Re: [PATCH RESEND 2/2] xen: Allow EXPERT mode to be selected from the menuconfig directly

[Jan Beulich <jbeulich@xxxxxxxx>]

On 11.05.2020 16:11, Julien Grall wrote:
> Hi,
> 
> On 11/05/2020 15:07, Jan Beulich wrote:
>> On 11.05.2020 15:57, Julien Grall wrote:
>>> Hi,
>>>
>>> On 11/05/2020 14:40, Jan Beulich wrote:
>>>> On 11.05.2020 15:30, Julien Grall wrote:
>>>>> Hi Ian,
>>>>>
>>>>> Thank you for the clarification.
>>>>>
>>>>> On 07/05/2020 18:01, Ian Jackson wrote:
>>>>>> Julien Grall writes ("Re: [PATCH RESEND 2/2] xen: Allow EXPERT mode to 
>>>>>> be selected from the menuconfig directly"):
>>>>>>> On 04/05/2020 13:34, Ian Jackson wrote:
>>>>>>>> George Dunlap writes ("Re: [PATCH RESEND 2/2] xen: Allow EXPERT mode 
>>>>>>>> to be selected from the menuconfig directly"):
>>>>>>>>> On Apr 30, 2020, at 3:50 PM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
>>>>>>>>>> Well, if I'm not mis-remembering it was on purpose to make it more
>>>>>>>>>> difficult for people to declare themselves "experts". FAOD I'm not
>>>>>>>>>> meaning to imply I don't see and accept the frustration aspect you
>>>>>>>>>> mention further up. The two need to be carefully weighed against
>>>>>>>>>> one another.
>>>>>>>>
>>>>>>>> Yes, it was on purpose.  However, I had my doubts at the time and
>>>>>>>> I think experience has shown that this was a mistake.
>>>>>>>>
>>>>>>>>> I don’t think we need to make it difficult for people to declare
>>>>>>>>> themselves experts, particularly as “all” it means at the moment is,
>>>>>>>>> “Can build something which is not security supported”.  People who
>>>>>>>>> are building their own hypervisors are already pretty well advanced;
>>>>>>>>> I think we can let them shoot themselves in the foot if they want
>>>>>>>>> to.
>>>>>>>>
>>>>>>>> Precisely.
>>>>>>>
>>>>>>> Can I consider this as an Acked-by? :)
>>>>>>
>>>>>> I am happy with the principle of the change.  I haven't reviewed the
>>>>>> details of the commit message etc.
>>>>>>
>>>>>> I reviewed the thread and there were two concernes raised:
>>>>>>
>>>>>>     * The question of principle.  I disagree with this concern
>>>>>>       because I approve of principle of the patch.
>>>>>>
>>>>>>     * Some detail about the precise justificaton as written in
>>>>>>       the commit message, regarding `clean' targets.  Apparently the
>>>>>>       assertion may not be completely true.  I haven't seen a proposed
>>>>>>       alternative wording.
>>>>>
>>>>> I have checked the latest staging, the `clean` target doesn't trash
>>>>> .config anymore.
>>>>>
>>>>>>
>>>>>> I don't feel I should ack a controversial patch with an unresolved
>>>>>> wording issue.  Can you tell me what your proposed wording is ?
>>>>>> To avoid blocking this change I would be happy to review your wording
>>>>>> and see if it meets my reading of the stated objection.
>>>>>
>>>>> Here a suggested rewording:
>>>>>
>>>>> "EXPERT mode is currently used to gate any options that are in technical
>>>>> preview or not security supported At the moment, the only way to select
>>>>> it is to use XEN_CONFIG_EXPERT=y on the make command line.
>>>>>
>>>>> However, if the user forget to add the option when (re)building or when
>>>>> using menuconfig, then .config will get rewritten. This may lead to a
>>>>> rather frustrating experience as it is difficult to diagnostic the
>>>>> issue.
>>>>
>>>> To me this looks very similar to e.g. not suitably overriding the
>>>> default toolchain binaries, if one has a need to build with newer
>>>> ones than what a distro provides. According to some of my routinely
>>>> built configs both can be done by putting suitable entries into
>>>> ./.config (not xen/.config), removing the need to remember adding
>>>> either to the make command line.
>>>
>>> I have never heard of ./.config before. So what are you referring to?
>>
>> I'm referring to this line in ./Config.mk:
>>
>> -include $(XEN_ROOT)/.config
> 
> Great another undocumented way to do things...
> 
>>
>>> But yes, there are ways to make it permanent. But that involves hacking
>>> Xen source.
>>
>> Why would there be any need for a source modification? Just like
>> xen/.config, ./.config is not considered part of the source.
>>
>>> This is not a very great approach because if you need to
>>> bisect, then you have to remember to apply the change everytime. It also
>>> doesn't work if you have to build for multiple different target from the
>>> same source.
>>
>> Why wouldn't it? I'm doing exactly this, far beyond just x86 and
>> Arm builds, and it all works fine. (It would work even better
>> with out-of-tree builds, but it looks like Anthony is getting us
>> there.)
> 
> ... let me ask it differently. Are you concerned of my wording or by the 
> fact we make easier to a user to EXPERT mode?

I'm trying to make the point that your patch, to me, looks to be
trying to overcome a problem for which we have had a solution all
the time.

Jan