| Message ID | 20200430142548.23751-3-julien@xen.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | xen: Allow EXPERT mode to be selected from the menuconfig directly | expand |
On 30.04.2020 16:25, Julien Grall wrote: > 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 of one of the make > command (even a clean), then .config will get rewritten. This may lead > to a rather frustrating experience as it is difficult to diagnostic the > issue. Is / will this still be true after Anthony's rework of the build system? Right now we already have clean-targets := %clean no-dot-config-targets := $(clean-targets) \ ... > A lot of the options behind EXPERT would benefit to get more tested in > order to be mark as fully supported in the future. Anyone intending to get an EXPERT-only option fully supported will need to do focused testing; I don't think we can expect to move items out of this category just because more people happen to test something every now and then. > In order to make easier to experiment, the option EXPERT can now be > selected from the menuconfig rather than make command line. This does > not change the fact a kernel with EXPERT mode selected will not be > security supported. 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. > --- a/xen/Kconfig > +++ b/xen/Kconfig > @@ -35,7 +35,15 @@ config DEFCONFIG_LIST > default ARCH_DEFCONFIG > > config EXPERT > - def_bool y if "$(XEN_CONFIG_EXPERT)" = "y" > + bool "Configure standard Xen features (expert users)" > + help > + This option allows certain base Xen options and settings > + to be disabled or tweaked. This is for specialized environments > + which can tolerate a "non-standard" Xen. > + Only use this if you really know what you are doing. > + Xen binaries built with this option enabled are not security > + supported. > + default n I don't think the last line is needed. Jan
Hi Jan, On 30/04/2020 15:50, Jan Beulich wrote: > On 30.04.2020 16:25, Julien Grall wrote: >> 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 of one of the make >> command (even a clean), then .config will get rewritten. This may lead >> to a rather frustrating experience as it is difficult to diagnostic the >> issue. > > Is / will this still be true after Anthony's rework of the build > system? Right now we already have > > clean-targets := %clean > no-dot-config-targets := $(clean-targets) \ > ... I haven't tried Anthony's rework yet. But I guess the problem would be the same if you forget to add XEN_CONFIG_EXPERT=y on make. > >> A lot of the options behind EXPERT would benefit to get more tested in >> order to be mark as fully supported in the future. > > Anyone intending to get an EXPERT-only option fully supported will > need to do focused testing; I don't think we can expect to move > items out of this category just because more people happen to test > something every now and then. I didn't imply this was the only condition to get a feature security suported. I merely pointed out that more testing would help to harden the code. If you make difficult to access an option then it will be less tested and take longer to get it security supported. > >> In order to make easier to experiment, the option EXPERT can now be >> selected from the menuconfig rather than make command line. This does >> not change the fact a kernel with EXPERT mode selected will not be >> security supported. > > 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. Some of the options behind EXPERT mode don't make sense. For instance, how adding a built-in command line requires to be expert? I understand we don't want to support it, but I don't see any reason to make the user's life more difficult here. Cheers,
On 30.04.2020 17:35, Julien Grall wrote: > On 30/04/2020 15:50, Jan Beulich wrote: >> On 30.04.2020 16:25, Julien Grall wrote: >>> 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 of one of the make >>> command (even a clean), then .config will get rewritten. This may lead >>> to a rather frustrating experience as it is difficult to diagnostic the >>> issue. >> >> Is / will this still be true after Anthony's rework of the build >> system? Right now we already have >> >> clean-targets := %clean >> no-dot-config-targets := $(clean-targets) \ >> ... > > I haven't tried Anthony's rework yet. But I guess the problem would > be the same if you forget to add XEN_CONFIG_EXPERT=y on make. Why? xen/.config would get re-written only if kconfig got run in the first place. It is my understanding that no-dot-config-targets exist to avoid including .config, and as a result make won't find a need anymore to cause it to re-made if out of date. Jan
Hi Jan, On 04/05/2020 10:18, Jan Beulich wrote: > On 30.04.2020 17:35, Julien Grall wrote: >> On 30/04/2020 15:50, Jan Beulich wrote: >>> On 30.04.2020 16:25, Julien Grall wrote: >>>> 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 of one of the make >>>> command (even a clean), then .config will get rewritten. This may lead >>>> to a rather frustrating experience as it is difficult to diagnostic the >>>> issue. >>> >>> Is / will this still be true after Anthony's rework of the build >>> system? Right now we already have >>> >>> clean-targets := %clean >>> no-dot-config-targets := $(clean-targets) \ >>> ... >> >> I haven't tried Anthony's rework yet. But I guess the problem would >> be the same if you forget to add XEN_CONFIG_EXPERT=y on make. > > Why? xen/.config would get re-written only if kconfig got run in > the first place. It is my understanding that no-dot-config-targets > exist to avoid including .config, and as a result make won't find > a need anymore to cause it to re-made if out of date. kconfig may be executed because you change one of the */Kconfig file. So if you happen to forget XEN_CONFIG_EXPERT=y on your build command line, then you will have your .config rewritten without expert options. Cheers,
On 04.05.2020 11:30, Julien Grall wrote: > Hi Jan, > > On 04/05/2020 10:18, Jan Beulich wrote: >> On 30.04.2020 17:35, Julien Grall wrote: >>> On 30/04/2020 15:50, Jan Beulich wrote: >>>> On 30.04.2020 16:25, Julien Grall wrote: >>>>> 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 of one of the make >>>>> command (even a clean), then .config will get rewritten. This may lead >>>>> to a rather frustrating experience as it is difficult to diagnostic the >>>>> issue. >>>> >>>> Is / will this still be true after Anthony's rework of the build >>>> system? Right now we already have >>>> >>>> clean-targets := %clean >>>> no-dot-config-targets := $(clean-targets) \ >>>> ... >>> >>> I haven't tried Anthony's rework yet. But I guess the problem would >>> be the same if you forget to add XEN_CONFIG_EXPERT=y on make. >> >> Why? xen/.config would get re-written only if kconfig got run in >> the first place. It is my understanding that no-dot-config-targets >> exist to avoid including .config, and as a result make won't find >> a need anymore to cause it to re-made if out of date. > > kconfig may be executed because you change one of the */Kconfig file. > So if you happen to forget XEN_CONFIG_EXPERT=y on your build command > line, then you will have your .config rewritten without expert options. That's still a build system issue then (if this is really what happens): Dependencies of xen/.config shouldn't be evaluated as long as it doesn't get used. Jan
Hi Jan, On 04/05/2020 10:37, Jan Beulich wrote: > On 04.05.2020 11:30, Julien Grall wrote: >> Hi Jan, >> >> On 04/05/2020 10:18, Jan Beulich wrote: >>> On 30.04.2020 17:35, Julien Grall wrote: >>>> On 30/04/2020 15:50, Jan Beulich wrote: >>>>> On 30.04.2020 16:25, Julien Grall wrote: >>>>>> 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 of one of the make >>>>>> command (even a clean), then .config will get rewritten. This may lead >>>>>> to a rather frustrating experience as it is difficult to diagnostic the >>>>>> issue. >>>>> >>>>> Is / will this still be true after Anthony's rework of the build >>>>> system? Right now we already have >>>>> >>>>> clean-targets := %clean >>>>> no-dot-config-targets := $(clean-targets) \ >>>>> ... >>>> >>>> I haven't tried Anthony's rework yet. But I guess the problem would >>>> be the same if you forget to add XEN_CONFIG_EXPERT=y on make. >>> >>> Why? xen/.config would get re-written only if kconfig got run in >>> the first place. It is my understanding that no-dot-config-targets >>> exist to avoid including .config, and as a result make won't find >>> a need anymore to cause it to re-made if out of date. >> >> kconfig may be executed because you change one of the */Kconfig file. >> So if you happen to forget XEN_CONFIG_EXPERT=y on your build command >> line, then you will have your .config rewritten without expert options. > > That's still a build system issue then (if this is really what happens): > Dependencies of xen/.config shouldn't be evaluated as long as it doesn't > get used. I am not sure to understand what you mean by "doesn't get used here". When you build Xen, xen/.config is a dependency for the auto-generated header. So 'make' will actually check whether there are any modification in */Kconfig. A user would also expect that any modification in a */Kconfig will be picked by 'make' when building the hypervisor. This is how it works in Linux and I see no reason to diverge in Xen for this. Cheers,
On 04.05.2020 11:54, Julien Grall wrote: > Hi Jan, > > On 04/05/2020 10:37, Jan Beulich wrote: >> On 04.05.2020 11:30, Julien Grall wrote: >>> Hi Jan, >>> >>> On 04/05/2020 10:18, Jan Beulich wrote: >>>> On 30.04.2020 17:35, Julien Grall wrote: >>>>> On 30/04/2020 15:50, Jan Beulich wrote: >>>>>> On 30.04.2020 16:25, Julien Grall wrote: >>>>>>> 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 of one of the make >>>>>>> command (even a clean), then .config will get rewritten. This may lead >>>>>>> to a rather frustrating experience as it is difficult to diagnostic the >>>>>>> issue. >>>>>> >>>>>> Is / will this still be true after Anthony's rework of the build >>>>>> system? Right now we already have >>>>>> >>>>>> clean-targets := %clean >>>>>> no-dot-config-targets := $(clean-targets) \ >>>>>> ... >>>>> >>>>> I haven't tried Anthony's rework yet. But I guess the problem would >>>>> be the same if you forget to add XEN_CONFIG_EXPERT=y on make. >>>> >>>> Why? xen/.config would get re-written only if kconfig got run in >>>> the first place. It is my understanding that no-dot-config-targets >>>> exist to avoid including .config, and as a result make won't find >>>> a need anymore to cause it to re-made if out of date. >>> >>> kconfig may be executed because you change one of the */Kconfig file. >>> So if you happen to forget XEN_CONFIG_EXPERT=y on your build command >>> line, then you will have your .config rewritten without expert options. >> >> That's still a build system issue then (if this is really what happens): >> Dependencies of xen/.config shouldn't be evaluated as long as it doesn't >> get used. > > I am not sure to understand what you mean by "doesn't get used here". When you build Xen, xen/.config is a dependency for the auto-generated header. So 'make' will actually check whether there are any modification in */Kconfig. But you were talking about "make clean", weren't you? Jan
On 04/05/2020 11:18, Jan Beulich wrote: > On 04.05.2020 11:54, Julien Grall wrote: >> Hi Jan, >> >> On 04/05/2020 10:37, Jan Beulich wrote: >>> On 04.05.2020 11:30, Julien Grall wrote: >>>> Hi Jan, >>>> >>>> On 04/05/2020 10:18, Jan Beulich wrote: >>>>> On 30.04.2020 17:35, Julien Grall wrote: >>>>>> On 30/04/2020 15:50, Jan Beulich wrote: >>>>>>> On 30.04.2020 16:25, Julien Grall wrote: >>>>>>>> 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 of one of the make >>>>>>>> command (even a clean), then .config will get rewritten. This may lead >>>>>>>> to a rather frustrating experience as it is difficult to diagnostic the >>>>>>>> issue. >>>>>>> >>>>>>> Is / will this still be true after Anthony's rework of the build >>>>>>> system? Right now we already have >>>>>>> >>>>>>> clean-targets := %clean >>>>>>> no-dot-config-targets := $(clean-targets) \ >>>>>>> ... >>>>>> >>>>>> I haven't tried Anthony's rework yet. But I guess the problem would >>>>>> be the same if you forget to add XEN_CONFIG_EXPERT=y on make. >>>>> >>>>> Why? xen/.config would get re-written only if kconfig got run in >>>>> the first place. It is my understanding that no-dot-config-targets >>>>> exist to avoid including .config, and as a result make won't find >>>>> a need anymore to cause it to re-made if out of date. >>>> >>>> kconfig may be executed because you change one of the */Kconfig file. >>>> So if you happen to forget XEN_CONFIG_EXPERT=y on your build command >>>> line, then you will have your .config rewritten without expert options. >>> >>> That's still a build system issue then (if this is really what happens): >>> Dependencies of xen/.config shouldn't be evaluated as long as it doesn't >>> get used. >> >> I am not sure to understand what you mean by "doesn't get used here". When you build Xen, xen/.config is a dependency for the auto-generated header. So 'make' will actually check whether there are any modification in */Kconfig. > > But you were talking about "make clean", weren't you? In the commit message yes... You asked whether this was true and I answered I didn't get a chance to test Anthony's rework. However, I also pointed out that it wouldn't solve a simple 'make' issue. I considered that your 'why?' were related to the simple 'make'. Cheers,
> On Apr 30, 2020, at 3:50 PM, Jan Beulich <JBeulich@suse.com> wrote: > >> In order to make easier to experiment, the option EXPERT can now be >> selected from the menuconfig rather than make command line. This does >> not change the fact a kernel with EXPERT mode selected will not be >> security supported. > > 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. 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. -George
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@suse.com> 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.
Ian.
Hi George, On 04/05/2020 12:10, George Dunlap wrote: > > >> On Apr 30, 2020, at 3:50 PM, Jan Beulich <JBeulich@suse.com> wrote: >> >>> In order to make easier to experiment, the option EXPERT can now be >>> selected from the menuconfig rather than make command line. This does >>> not change the fact a kernel with EXPERT mode selected will not be >>> security supported. >> >> 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. > > 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. Assuming I reword the commit message regarding "make clean", could I consider this as an acked-by? Cheers,
Hi Ian, 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@suse.com> 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? :) Cheers,
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@suse.com> 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 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.
Alternatively, if you can get agreement from others on the wording of
the commit message, you may put my ack on the patch.
HTH,
Ian.
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@suse.com> 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. A lot of the options behind EXPERT would benefit to be more accessible so user can experiment with it and voice any concern before they are fully be supported. So rather than making really difficult to experiment or tweak your Xen (for instance by adding a built-in command line), this option can now be selected from the menuconfig. This doesn't change the fact a Xen with EXPERT mode selected will not be security supported. " Cheers,
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@suse.com> 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. Jan
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@suse.com> 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? But yes, there are ways to make it permanent. But that involves hacking Xen 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. The compiler is another option that would be worth to move in the Kconfig. I have stopped counting the number of time I mixed up between Arm64, Arm32 and x86 compilers when building Xen... Cheers,
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@suse.com> 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 > 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.) Jan
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@suse.com> 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? Cheers,
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@suse.com> 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
> On May 11, 2020, at 3:11 PM, Julien Grall <julien@xen.org> wrote: > > [CAUTION - EXTERNAL EMAIL] DO NOT reply, click links, or open attachments unless you have verified the sender and know the content is safe. > > Hi, > > On 11/05/2020 15:07, Jan Beulich wrote: >> On 11.05.2020 15:57, Julien Grall wrote: >>> >>> 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... Oh, is that not documented? It’s quite venerable at this point — if it’s not documented that should change. (Although I guess there’s an argument that everything which would be included there should be added to either KConfig or configure.) I don’t think I would have thought to add XEN_CONFIG_EXPERT=y to .config to prevent the issue Julien is seeing. That maybe part of the reason why this doesn’t bother you as much as it does him. From a UI perspective, I think that’s a poor UI — enabling CONFIG_EXPERT from the menuconfig is more discoverable and more in line with what people expect. -George
On 11/05/2020 15:14, Jan Beulich wrote: > 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@suse.com> 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. 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. Cheers,
> On May 11, 2020, at 2:30 PM, Julien Grall <julien@xen.org> wrote: > > [CAUTION - EXTERNAL EMAIL] DO NOT reply, click links, or open attachments unless you have verified the sender and know the content is safe. > > 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@suse.com> 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. > > A lot of the options behind EXPERT would benefit to be more accessible so user can experiment with it and voice any concern before they are fully be supported. > > So rather than making really difficult to experiment or tweak your Xen (for instance by adding a built-in command line), this option can now be selected from the menuconfig. > > This doesn't change the fact a Xen with EXPERT mode selected will not be security supported. > " How about this, clarifying that top-level .config is an option, but that it’s still better to put it in menuconfig? (Also note a number of grammar tweaks.) — EXPERT mode is currently used to gate any options that are in technical preview or not security supported. At the moment, this is selected by adding XEN_CONFIG_EXPERT=y on the make command line, or to the (currently undocumented) top-level .config file. This makes the option very unintuitive to use: If the user forgets to add the option when (re)building or when using menuconfig, then xen/.config will be silently rewritten, leading to behavior which is very difficult to diagnose. Adding XEN_CONFIG_EXPERT=y to the top-level .config is not obvious behavior, particularly as the file is undocumented. A lot of the options behind EXPERT would benefit from being more accessible so users can experiment with them and voice any concerns before they are fully supported. To make this option more discoverable and consistent to use, make it possible to select it from the menuconfig. This doesn't change the fact a Xen with EXPERT mode selected will not be security supported. — -George
Hi George, On 11/05/2020 16:27, George Dunlap wrote: > > >> On May 11, 2020, at 2:30 PM, Julien Grall <julien@xen.org> wrote: >> >> [CAUTION - EXTERNAL EMAIL] DO NOT reply, click links, or open attachments unless you have verified the sender and know the content is safe. >> >> 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@suse.com> 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. >> >> A lot of the options behind EXPERT would benefit to be more accessible so user can experiment with it and voice any concern before they are fully be supported. >> >> So rather than making really difficult to experiment or tweak your Xen (for instance by adding a built-in command line), this option can now be selected from the menuconfig. >> >> This doesn't change the fact a Xen with EXPERT mode selected will not be security supported. >> " > > How about this, clarifying that top-level .config is an option, but that it’s still better to put it in menuconfig? (Also note a number of grammar tweaks.) > > — > > EXPERT mode is currently used to gate any options that are in technical > preview or not security supported. At the moment, this is selected by adding XEN_CONFIG_EXPERT=y on the make command line, or to the (currently undocumented) top-level .config file. > > This makes the option very unintuitive to use: If the user forgets to add the option when (re)building or when using menuconfig, then xen/.config will be silently rewritten, leading to behavior which is very difficult to diagnose. Adding XEN_CONFIG_EXPERT=y to the top-level .config is not obvious behavior, particularly as the file is undocumented. > > A lot of the options behind EXPERT would benefit from being more accessible so users can experiment with them and voice any concerns before they are fully supported. > > To make this option more discoverable and consistent to use, make it possible to select it from the menuconfig. > > This doesn't change the fact a Xen with EXPERT mode selected will not be security supported. I am happy this wording. Cheers,
Julien Grall writes ("Re: [PATCH RESEND 2/2] xen: Allow EXPERT mode to be selected from the menuconfig directly"):
> I am happy this wording.
Thanks for engaging with the commit message issue.
This patch is:
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
with any of the suggested commit message wordings. I trust your
judgement to pick a suitable wording.
Ian.
Jan Beulich writes ("Re: [PATCH RESEND 2/2] xen: Allow EXPERT mode to be selected from the menuconfig directly"):
> 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.
Thanks for this clear statement of your objection. I'm afraid I don't
agree. Even though .config exists (and is even used by osstest, so I
know about it) I don't think it is as good as having it in
menuconfig.
I agree with George's comments:
| From a UI perspective, I think that’s a poor UI — enabling
| CONFIG_EXPERT from the menuconfig is more discoverable and more in
| line with what people expect.
and I haven't seen good reasons (in my opinion) for diverging from
that discoverability and expectation.
So I have given my ack in the other mail.
Regards,
Ian.
On 11.05.2020 19:14, Ian Jackson wrote: > Jan Beulich writes ("Re: [PATCH RESEND 2/2] xen: Allow EXPERT mode to be selected from the menuconfig directly"): >> 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. > > Thanks for this clear statement of your objection. I'm afraid I don't > agree. Even though .config exists (and is even used by osstest, so I > know about it) I don't think it is as good as having it in > menuconfig. But you realize that my objection is (was) more towards the reasoning behind the change, than towards the change itself. If, as a community, we decide to undo what we might now call a mistake, and if we're ready to deal with the consequences, so be it. Jan
Hi, On 12/05/2020 08:18, Jan Beulich wrote: > On 11.05.2020 19:14, Ian Jackson wrote: >> Jan Beulich writes ("Re: [PATCH RESEND 2/2] xen: Allow EXPERT mode to be selected from the menuconfig directly"): >>> 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. >> >> Thanks for this clear statement of your objection. I'm afraid I don't >> agree. Even though .config exists (and is even used by osstest, so I >> know about it) I don't think it is as good as having it in >> menuconfig. > > But you realize that my objection is (was) more towards the reasoning > behind the change, than towards the change itself. If, as a community, > we decide to undo what we might now call a mistake, and if we're ready > to deal with the consequences, so be it. Would you mind to explain the fall out you expect from this patch? Are you worry more people may contact security@xen.org for non-security issue? Cheers,
On 12.05.2020 12:08, Julien Grall wrote: > On 12/05/2020 08:18, Jan Beulich wrote: >> On 11.05.2020 19:14, Ian Jackson wrote: >>> Jan Beulich writes ("Re: [PATCH RESEND 2/2] xen: Allow EXPERT mode to be selected from the menuconfig directly"): >>>> 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. >>> >>> Thanks for this clear statement of your objection. I'm afraid I don't >>> agree. Even though .config exists (and is even used by osstest, so I >>> know about it) I don't think it is as good as having it in >>> menuconfig. >> >> But you realize that my objection is (was) more towards the reasoning >> behind the change, than towards the change itself. If, as a community, >> we decide to undo what we might now call a mistake, and if we're ready >> to deal with the consequences, so be it. > > Would you mind to explain the fall out you expect from this patch? Are > you worry more people may contact security@xen.org for non-security issue? That's one possible thing that might happen. But even more generally the likelihood will increase that people report issues without paying attention that they depend on their choice of configuration. We'll have to both take this into consideration and ask back for the specific .config they've used. Jan
Hi Jan, On 12/05/2020 11:15, Jan Beulich wrote: > On 12.05.2020 12:08, Julien Grall wrote: >> On 12/05/2020 08:18, Jan Beulich wrote: >>> On 11.05.2020 19:14, Ian Jackson wrote: >>>> Jan Beulich writes ("Re: [PATCH RESEND 2/2] xen: Allow EXPERT mode to be selected from the menuconfig directly"): >>>>> 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. >>>> >>>> Thanks for this clear statement of your objection. I'm afraid I don't >>>> agree. Even though .config exists (and is even used by osstest, so I >>>> know about it) I don't think it is as good as having it in >>>> menuconfig. >>> >>> But you realize that my objection is (was) more towards the reasoning >>> behind the change, than towards the change itself. If, as a community, >>> we decide to undo what we might now call a mistake, and if we're ready >>> to deal with the consequences, so be it. >> >> Would you mind to explain the fall out you expect from this patch? Are >> you worry more people may contact security@xen.org for non-security issue? > > That's one possible thing that might happen. But even more generally > the likelihood will increase that people report issues without paying > attention that they depend on their choice of configuration. I agree that you are going to get more report because there are more users to try new things. So inevitently, you will get more incomplete report. This is always the downside of allowing more flexibility. But we also need to look at the upside. I can see 2 advantages: 1) It will be easier to try upcoming features (e.g Argo). The more testing and input, the more chance a feature will be a success. 2) It will be easier to tailor Xen (such as built-in command line). In both cases, you make Xen more compelling because you allow to experiment and make it more flexible. IHMO, this is one of the best way to attract users and possible new contributors/reviewers to Xen community. > We'll > have to both take this into consideration and ask back for the > specific .config they've used. Correct me if I am wrong, but this is not very specific to EXPERT mode. You can already select different options that will affect the behavior of the hypervisor. For instance, on x86, you can disable PV guest support. How do you figure that out today without asking the .config? Cheers,
On 12.05.2020 13:00, Julien Grall wrote: > Hi Jan, > > On 12/05/2020 11:15, Jan Beulich wrote: >> On 12.05.2020 12:08, Julien Grall wrote: >>> On 12/05/2020 08:18, Jan Beulich wrote: >>>> On 11.05.2020 19:14, Ian Jackson wrote: >>>>> Jan Beulich writes ("Re: [PATCH RESEND 2/2] xen: Allow EXPERT mode to be selected from the menuconfig directly"): >>>>>> 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. >>>>> >>>>> Thanks for this clear statement of your objection. I'm afraid I don't >>>>> agree. Even though .config exists (and is even used by osstest, so I >>>>> know about it) I don't think it is as good as having it in >>>>> menuconfig. >>>> >>>> But you realize that my objection is (was) more towards the reasoning >>>> behind the change, than towards the change itself. If, as a community, >>>> we decide to undo what we might now call a mistake, and if we're ready >>>> to deal with the consequences, so be it. >>> >>> Would you mind to explain the fall out you expect from this patch? Are >>> you worry more people may contact security@xen.org for non-security issue? >> >> That's one possible thing that might happen. But even more generally >> the likelihood will increase that people report issues without paying >> attention that they depend on their choice of configuration. > I agree that you are going to get more report because there are more > users to try new things. So inevitently, you will get more incomplete > report. This is always the downside of allowing more flexibility. > > But we also need to look at the upside. I can see 2 advantages: > 1) It will be easier to try upcoming features (e.g Argo). The more > testing and input, the more chance a feature will be a success. > 2) It will be easier to tailor Xen (such as built-in command line). > > In both cases, you make Xen more compelling because you allow to > experiment and make it more flexible. IHMO, this is one of the best way > to attract users and possible new contributors/reviewers to Xen community. I'm fully aware of the upsides. >> We'll >> have to both take this into consideration and ask back for the >> specific .config they've used. > Correct me if I am wrong, but this is not very specific to EXPERT mode. > You can already select different options that will affect the behavior > of the hypervisor. For instance, on x86, you can disable PV guest > support. How do you figure that out today without asking the .config? I didn't say this is a new problem; I indicated this is going to become more likely to be one. Jan
> On May 12, 2020, at 12:03 PM, Jan Beulich <jbeulich@suse.com> wrote: > > [CAUTION - EXTERNAL EMAIL] DO NOT reply, click links, or open attachments unless you have verified the sender and know the content is safe. > > On 12.05.2020 13:00, Julien Grall wrote: >> Hi Jan, >> >> On 12/05/2020 11:15, Jan Beulich wrote: >>> On 12.05.2020 12:08, Julien Grall wrote: >>>> On 12/05/2020 08:18, Jan Beulich wrote: >>>>> On 11.05.2020 19:14, Ian Jackson wrote: >>>>>> Jan Beulich writes ("Re: [PATCH RESEND 2/2] xen: Allow EXPERT mode to be selected from the menuconfig directly"): >>>>>>> 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. >>>>>> >>>>>> Thanks for this clear statement of your objection. I'm afraid I don't >>>>>> agree. Even though .config exists (and is even used by osstest, so I >>>>>> know about it) I don't think it is as good as having it in >>>>>> menuconfig. >>>>> >>>>> But you realize that my objection is (was) more towards the reasoning >>>>> behind the change, than towards the change itself. If, as a community, >>>>> we decide to undo what we might now call a mistake, and if we're ready >>>>> to deal with the consequences, so be it. >>>> >>>> Would you mind to explain the fall out you expect from this patch? Are >>>> you worry more people may contact security@xen.org for non-security issue? >>> >>> That's one possible thing that might happen. But even more generally >>> the likelihood will increase that people report issues without paying >>> attention that they depend on their choice of configuration. >> I agree that you are going to get more report because there are more >> users to try new things. So inevitently, you will get more incomplete >> report. This is always the downside of allowing more flexibility. >> >> But we also need to look at the upside. I can see 2 advantages: >> 1) It will be easier to try upcoming features (e.g Argo). The more >> testing and input, the more chance a feature will be a success. >> 2) It will be easier to tailor Xen (such as built-in command line). >> >> In both cases, you make Xen more compelling because you allow to >> experiment and make it more flexible. IHMO, this is one of the best way >> to attract users and possible new contributors/reviewers to Xen community. > > I'm fully aware of the upsides. > >>> We'll >>> have to both take this into consideration and ask back for the >>> specific .config they've used. >> Correct me if I am wrong, but this is not very specific to EXPERT mode. >> You can already select different options that will affect the behavior >> of the hypervisor. For instance, on x86, you can disable PV guest >> support. How do you figure that out today without asking the .config? > > I didn't say this is a new problem; I indicated this is going to > become more likely to be one. I feel like there’s a misunderstanding here — Jan, are you simply explaining yourself and/or making sure that we all understand the implications of our choice? Or are you arguing against acceptance in an implicitly Nack-ing manner? I understood Jan to be doing the former; and that as such with Ian’s ack, this patch (with the modified commit message) can go in. -George
On 12.05.2020 13:05, George Dunlap wrote: > > >> On May 12, 2020, at 12:03 PM, Jan Beulich <jbeulich@suse.com> wrote: >> >> [CAUTION - EXTERNAL EMAIL] DO NOT reply, click links, or open attachments unless you have verified the sender and know the content is safe. >> >> On 12.05.2020 13:00, Julien Grall wrote: >>> Hi Jan, >>> >>> On 12/05/2020 11:15, Jan Beulich wrote: >>>> On 12.05.2020 12:08, Julien Grall wrote: >>>>> On 12/05/2020 08:18, Jan Beulich wrote: >>>>>> On 11.05.2020 19:14, Ian Jackson wrote: >>>>>>> Jan Beulich writes ("Re: [PATCH RESEND 2/2] xen: Allow EXPERT mode to be selected from the menuconfig directly"): >>>>>>>> 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. >>>>>>> >>>>>>> Thanks for this clear statement of your objection. I'm afraid I don't >>>>>>> agree. Even though .config exists (and is even used by osstest, so I >>>>>>> know about it) I don't think it is as good as having it in >>>>>>> menuconfig. >>>>>> >>>>>> But you realize that my objection is (was) more towards the reasoning >>>>>> behind the change, than towards the change itself. If, as a community, >>>>>> we decide to undo what we might now call a mistake, and if we're ready >>>>>> to deal with the consequences, so be it. >>>>> >>>>> Would you mind to explain the fall out you expect from this patch? Are >>>>> you worry more people may contact security@xen.org for non-security issue? >>>> >>>> That's one possible thing that might happen. But even more generally >>>> the likelihood will increase that people report issues without paying >>>> attention that they depend on their choice of configuration. >>> I agree that you are going to get more report because there are more >>> users to try new things. So inevitently, you will get more incomplete >>> report. This is always the downside of allowing more flexibility. >>> >>> But we also need to look at the upside. I can see 2 advantages: >>> 1) It will be easier to try upcoming features (e.g Argo). The more >>> testing and input, the more chance a feature will be a success. >>> 2) It will be easier to tailor Xen (such as built-in command line). >>> >>> In both cases, you make Xen more compelling because you allow to >>> experiment and make it more flexible. IHMO, this is one of the best way >>> to attract users and possible new contributors/reviewers to Xen community. >> >> I'm fully aware of the upsides. >> >>>> We'll >>>> have to both take this into consideration and ask back for the >>>> specific .config they've used. >>> Correct me if I am wrong, but this is not very specific to EXPERT mode. >>> You can already select different options that will affect the behavior >>> of the hypervisor. For instance, on x86, you can disable PV guest >>> support. How do you figure that out today without asking the .config? >> >> I didn't say this is a new problem; I indicated this is going to >> become more likely to be one. > > I feel like there’s a misunderstanding here — Jan, are you simply > explaining yourself and/or making sure that we all understand the > implications of our choice? Or are you arguing against acceptance > in an implicitly Nack-ing manner? The former - it would have seemed impolite if I hadn't replied to Julien's question. > I understood Jan to be doing the former; and that as such with > Ian’s ack, this patch (with the modified commit message) can go in. Indeed. Looks like I'm the only one anyway to be concerned of the extra effort. Jan
diff --git a/xen/Kconfig b/xen/Kconfig index 120b5f412993..34c318bfa2c7 100644 --- a/xen/Kconfig +++ b/xen/Kconfig @@ -35,7 +35,15 @@ config DEFCONFIG_LIST default ARCH_DEFCONFIG config EXPERT - def_bool y if "$(XEN_CONFIG_EXPERT)" = "y" + bool "Configure standard Xen features (expert users)" + help + This option allows certain base Xen options and settings + to be disabled or tweaked. This is for specialized environments + which can tolerate a "non-standard" Xen. + Only use this if you really know what you are doing. + Xen binaries built with this option enabled are not security + supported. + default n config LTO bool "Link Time Optimisation" diff --git a/xen/Makefile b/xen/Makefile index 2b1dacb49754..286f374b549f 100644 --- a/xen/Makefile +++ b/xen/Makefile @@ -11,7 +11,6 @@ export XEN_DOMAIN ?= $(shell ([ -x /bin/dnsdomainname ] && /bin/dnsdomainname) | export XEN_BUILD_DATE ?= $(shell LC_ALL=C date) export XEN_BUILD_TIME ?= $(shell LC_ALL=C date +%T) export XEN_BUILD_HOST ?= $(shell hostname) -export XEN_CONFIG_EXPERT ?= n # Best effort attempt to find a python interpreter, defaulting to Python 3 if # available. Fall back to just `python` if `which` is nowhere to be found.