[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

On 11/05/2020 15:14, Jan Beulich wrote:
On 11.05.2020 16:11, Julien Grall wrote:

On 11/05/2020 15:07, Jan Beulich wrote:
On 11.05.2020 15:57, Julien Grall wrote:

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


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

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

... 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.

For a first, AFAICT, your solution is not documented anywhere at all... It would be possible to document it thought.

However, having two .config for managing Xen options is not very great. You have to remember where to add the options and that you need to provide the two files if you want someone else to reproduce your setup.

So I would still like to push for a single .config.

As an aside, if you look at the placement of ./.config, it is meant to be applied for the full tree. XEN_CONFIG_EXPERT is a config specific to the hypervisor, so it is a bit counterintuitive to put it in that file.


Julien Grall



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.