| Message ID | 1452781693-28525-1-git-send-email-cardoe@cardoe.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
I have to confess I'm quite confused now. Maybe there are many
underlying disagreements here but mostly I seem befogged. However,
here are some principles I currently believe in for how this should
all work:
* It should be possible to enable, or disable, all of the following
things by pulling one handle:
- XSM hypervisor support
- FLASK hypervisor support
- installation of the FLASK policy in /boot
- presence of the Xen+XSM boot entry
* FAOD it is IMO OK for some of those things to be configurable
separately, but there should be one most obvious way to enable, or
disable, them all together.
* This single handle should be used by osstest, so that osstest is
testing the most usual build method. (Corollary: current code in
osstest that conditionally copies the policy is a bodge which would
ideally go away.)
* I do not have a strong opinion about whether FLASK policy-handling
tools ought to be always built. I see no harm in building them
always and I can see arguments in favour.
* Users should not get boot config options that are definitely not
going to work (see above). (Or at least, not unless they try
hard.)
* The hypervisor maintainers object to autoconf. This is fine. But
it means that if we want to have a single configuration option
which affects both hypervisor and tools (at least, by default),
that it should be possible for tools config options to at least
inherit their defaults from Kconfig.
* Perhaps this should be done by having the tools configure run some
Kconfig. If so there should be an autoconf command line option to
suppress this.
* Alternatively, perhaps configure should be changed so that it can
set Kconfig settings which the hypervisor build will pick up.
* IMO it is fine to put the Xen Kconfig config file in /boot. Given
the existing state of the rest of the universe, this seems like the
best place for me.
* osstest's replacement Xen grub menu entry generator ought to be on
an upstream track. This is necessary because osstest ought to be
using features that users will get too.
* I don't have a clear design proposal for the above but I think Doug
can probably provide one. I'm hoping this is more a matter of
thinking carefully than of extensive build system programming!
* If a fix for this is hard to achieve for some reason I will be
content with patches which make things no worse, overall, than they
were before Christmas :-). (By which I do not mean that I demand a
Pareto improvement.)
Is any of this of any use ?
Thanks,
Ian.
(no less confused after writing this than I was before)
On 1/14/16 10:27 AM, Ian Jackson wrote: > I have to confess I'm quite confused now. Maybe there are many > underlying disagreements here but mostly I seem befogged. However, > here are some principles I currently believe in for how this should > all work: > > * It should be possible to enable, or disable, all of the following > things by pulling one handle: > - XSM hypervisor support > - FLASK hypervisor support > - installation of the FLASK policy in /boot > - presence of the Xen+XSM boot entry I think this is where the issue lies. There have always been multiple switches to pull and different assumptions where made. - XSM in the hypervisor was enabled by XSM_ENABLE at the top level. Currently it is enabled by the CONFIG_XSM Kconfig switch in the xen/ directory. - FLASK in the hypervisor was enabled by FLASK_ENABLE at the top level. It did not depend on XSM_ENABLE in the Makefile rules (but the code did ifdef it out since it needed XSM_ENABLE). Currently it is enabled by the CONFIG_FLASK Kconfig switch in the xen/ directory. It depends on CONFIG_XSM. - The FLASK policy installation is controlled in the tools/ by --enable-xsmpolicy (which is the default) however the policy COULDN'T build without FLASK_ENABLE being set at the top level but you could disable XSM_ENABLE and it would still build. (which is what my patch tried to make more explicit in osstest) - Xen+XSM boot entry doesn't exist today but with storing the state of the hypervisor in the .config we'll be able to make this smart by saying "Xen supports FLASK so load in the policy since the policy is present" > > * FAOD it is IMO OK for some of those things to be configurable > separately, but there should be one most obvious way to enable, or > disable, them all together. That's fine but we need to make the pull switch explicitly set all the knobs correctly and not rely on implicit defaults. > > * This single handle should be used by osstest, so that osstest is > testing the most usual build method. (Corollary: current code in > osstest that conditionally copies the policy is a bodge which would > ideally go away.) Will it be if we document it that people need to provide a Kconfig file to enable it and then configure with --enable-xsmpolicy? > > * I do not have a strong opinion about whether FLASK policy-handling > tools ought to be always built. I see no harm in building them > always and I can see arguments in favour. > > * Users should not get boot config options that are definitely not > going to work (see above). (Or at least, not unless they try > hard.) I agree and that's more likely to happen now with the Kconfig file being saved. > > * The hypervisor maintainers object to autoconf. This is fine. But > it means that if we want to have a single configuration option > which affects both hypervisor and tools (at least, by default), > that it should be possible for tools config options to at least > inherit their defaults from Kconfig. So that was my question early on in the Kconfig bits if it the .config bits should drag up to the top level or just be contained into xen/ and I was told to keep it in xen/ hence for the split. > > * Perhaps this should be done by having the tools configure run some > Kconfig. If so there should be an autoconf command line option to > suppress this. > > * Alternatively, perhaps configure should be changed so that it can > set Kconfig settings which the hypervisor build will pick up. I'm not sure of the best approach here. It will require a little bit of tinkering. > > * IMO it is fine to put the Xen Kconfig config file in /boot. Given > the existing state of the rest of the universe, this seems like the > best place for me. I agree with you. But Jan disagreed. I submitted a patch both ways because progress was better than bikeshedding and he merged the patch into /usr/lib/debug. I'd be in favor of seeing it moved to /boot. > > * osstest's replacement Xen grub menu entry generator ought to be on > an upstream track. This is necessary because osstest ought to be > using features that users will get too. I agree here as well. I've got a few patches that I'm planning on submitting to grub as well to make the options a little clearer and the defaults do the right thing in more cases. > > * I don't have a clear design proposal for the above but I think Doug > can probably provide one. I'm hoping this is more a matter of > thinking carefully than of extensive build system programming! > > * If a fix for this is hard to achieve for some reason I will be > content with patches which make things no worse, overall, than they > were before Christmas :-). (By which I do not mean that I demand a > Pareto improvement.) > > Is any of this of any use ? > > Thanks, > Ian. > (no less confused after writing this than I was before) > The take away I see here is whatever path is taken we just need to be a bit more explicit with what does what or what is called what to avoid potential breakages.
Doug Goldstein writes ("Re: [Xen-devel] [xen-unstable test] 77945: regressions - FAIL [and 2 more messages]"):
> On 1/14/16 10:27 AM, Ian Jackson wrote:
> > Is any of this of any use ?
> >
> > Thanks,
> > Ian.
> > (no less confused after writing this than I was before)
>
> The take away I see here is whatever path is taken we just need to be a
> bit more explicit with what does what or what is called what to avoid
> potential breakages.
Heh. Does this mean that we should expect more or different patches
from you ? I see your conversation with Ian Campbell about at least
one of the osstest patches is ongoing...
Ian.
On Thu, 2016-01-14 at 17:18 +0000, Ian Jackson wrote: > Doug Goldstein writes ("Re: [Xen-devel] [xen-unstable test] 77945: > regressions - FAIL [and 2 more messages]"): > > On 1/14/16 10:27 AM, Ian Jackson wrote: > > > Is any of this of any use ? > > > > > > Thanks, > > > Ian. > > > (no less confused after writing this than I was before) > > > > The take away I see here is whatever path is taken we just need to be a > > bit more explicit with what does what or what is called what to avoid > > potential breakages. > > Heh. Does this mean that we should expect more or different patches > from you ? I see your conversation with Ian Campbell about at least > one of the osstest patches is ongoing... I was confused about which .config that was touching (.config vs xen/.config), so I think my feedback was largely nonsense. Although in the light of what was written in this thread I'm not sure what the next step is WRT that patch either. Ian.
On 1/14/16 11:18 AM, Ian Jackson wrote: > Doug Goldstein writes ("Re: [Xen-devel] [xen-unstable test] 77945: regressions - FAIL [and 2 more messages]"): >> On 1/14/16 10:27 AM, Ian Jackson wrote: >>> Is any of this of any use ? >>> >>> Thanks, >>> Ian. >>> (no less confused after writing this than I was before) >> >> The take away I see here is whatever path is taken we just need to be a >> bit more explicit with what does what or what is called what to avoid >> potential breakages. > > Heh. Does this mean that we should expect more or different patches > from you ? I see your conversation with Ian Campbell about at least > one of the osstest patches is ongoing... > > Ian. > So I stand behind the two submitted patches for osstest because I feel that it makes it more explicit. However, I understand your point that you would prefer 1 knob and it be the same knob that users use so I'm willing to do the work necessarily to get the code behaving how you would prefer but I would hope we make the code more explicit is all. So basically, if the patches are thumbs downed I'll change.
On 1/14/16 8:28 AM, Doug Goldstein wrote: > If the test should build with XSM then supply --enable-xsmpolicy to the > tools/configure script otherwise provide --disable-xsmpolicy. This will > allow the default to change from --enable-xsmpolicy to > --disable-xsmpolicy in the Xen tree without breaking OSSTest. > > Signed-off-by: Doug Goldstein <cardoe@cardoe.com> > --- > ts-xen-build | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/ts-xen-build b/ts-xen-build > index bc4e41a..4812dff 100755 > --- a/ts-xen-build > +++ b/ts-xen-build > @@ -111,6 +111,7 @@ END > sub build () { > my $xend_opt= $r{enable_xend} =~ m/true/ ? "--enable-xend" : "--disable-xend"; > my $ovmf_opt= $r{enable_ovmf} =~ m/true/ ? "--enable-ovmf" : "--disable-ovmf"; > + my $xsm_opt= $r{enable_xsm} =~ m/true/ ? "--enable-xsmpolicy" : "--disable-xsmpolicy"; > > my $configure_prefix = $r{cmdprefix_configure} // ''; > my $make_prefix = $r{cmdprefix_make} // ''; > @@ -123,8 +124,11 @@ sub build () { > if grep -q -- $ovmf_opt tools/configure ; then > ovmf=$ovmf_opt > fi > + if grep -q -- $xsm_opt tools/configure ; then > + xsm=$xsm_opt > + fi > END > - $configure_prefix ./configure --sysconfdir=/etc \$xend \$ovmf > + $configure_prefix ./configure --sysconfdir=/etc \$xend \$ovmf \$xsm > END > fi > END > So this patch should definitely wait until a direction is selected on the other submitted XSM/FLASK patches for the tools.
>>> On 14.01.16 at 17:27, <Ian.Jackson@eu.citrix.com> wrote: > I have to confess I'm quite confused now. Maybe there are many > underlying disagreements here but mostly I seem befogged. However, > here are some principles I currently believe in for how this should > all work: > > * It should be possible to enable, or disable, all of the following > things by pulling one handle: > - XSM hypervisor support > - FLASK hypervisor support > - installation of the FLASK policy in /boot > - presence of the Xen+XSM boot entry > > * FAOD it is IMO OK for some of those things to be configurable > separately, but there should be one most obvious way to enable, or > disable, them all together. Both of these would be nice, but I don't view this a requirement. > * Users should not get boot config options that are definitely not > going to work (see above). (Or at least, not unless they try > hard.) Agreed. > * The hypervisor maintainers object to autoconf. This is fine. But > it means that if we want to have a single configuration option > which affects both hypervisor and tools (at least, by default), > that it should be possible for tools config options to at least > inherit their defaults from Kconfig. > > * Perhaps this should be done by having the tools configure run some > Kconfig. If so there should be an autoconf command line option to > suppress this. > > * Alternatively, perhaps configure should be changed so that it can > set Kconfig settings which the hypervisor build will pick up. All of these fall into the same area as the first two above - would be nice, but not strict requirement. In particular with the various subtrees there's no requirement for both tools and hypervisor to be built at all in the same tree. (In fact on my only left 32-bit host this is what happens - the 32-bit build tree only gets tools built, and the 64-bit build tree only gets the hypervisor built.) > * IMO it is fine to put the Xen Kconfig config file in /boot. Given > the existing state of the rest of the universe, this seems like the > best place for me. I continue to disagree, but I wouldn't outright veto someone else committing a change to this effect. I won't, however, commit such a change myself (or only with an Explicitly-not-acked-by: tag). Jan
On Thu, 2016-01-14 at 16:27 +0000, Ian Jackson wrote: > * I don't have a clear design proposal for the above but I think Doug > can probably provide one. I'm hoping this is more a matter of > thinking carefully than of extensive build system programming! I think we should: 1) Move /usr/lib/debug/xen-4.7-unstable.config to /boot. I previously didn't care about what path it was, but the usecase of having grub be able to react to the config (see below) is a strong reason to have it in /boot IMHO. Jan has said he won't veto such a change, AFAICT everyone else is happy with it. 2) Assume that grub (specifically the patch in http://savannah.gnu.org/bugs /?43420 and as used by osstest today) will at some point be modified to look at /boot/xenconfig-$version to decide whether to create an XSM entry or not instead of the presence of /boot/xenpolicy-$version. This step belongs here logically but chronologically could come much later since osstest will do the right thing even if there is a spurious /boot/xenpolicy-$version file (which is to say it will ignore the spurious entry and boot the right thing). 3) Have tools/* always build the FLASK+XSM tools _and_ the FLASK policy and to always install both. Any related configure options can go away and we no longer need to worry about synchronising the configuration of the tools and xen trees, this is desirable because we would prefer to have one set of tools which gracefully handles differing hypervisor configurations over needing different sets of tools (FLASK+XSM was one of the few exceptions to that rule AFAICT). I think with this plan there is no need to modify osstest.git, since it already does the right thing (which is, it sets XSM for Xen builds, which in turn enables FLASK and it does nothing for tools/* which is correct once #3 above has happened). The only downside is a spurious /boot/xenpolicy-$version installed when the corresponding Xen binary doesn't support XSM, however given the assumption in #2 (which implies the user will never see a spurious grub entry, which is the important thing) and the fact that it avoids the complexity of having tools/* rely in some way on xen/.config I think that is a worthwhile trade-off. Hopefully this simplifies a bunch of the arguments we have been having and provides a path forwards? Objections? Doug, I think you have the majority of #1 and #3 already in some form or other? #2 is not on the critical path and can come later. Ian.
Ian Campbell writes ("Re: [Xen-devel] [xen-unstable test] 77945: regressions - FAIL [and 2 more messages]"):
...
> The only downside is a spurious /boot/xenpolicy-$version installed when the
> corresponding Xen binary doesn't support XSM, however given the assumption
> in #2 (which implies the user will never see a spurious grub entry, which
> is the important thing) and the fact that it avoids the complexity of
> having tools/* rely in some way on xen/.config I think that is a worthwhile
> trade-off.
I like this plan and I think this downside is quite tolerable.
Ian.
>>> On 15.01.16 at 18:06, <ian.campbell@citrix.com> wrote: > On Thu, 2016-01-14 at 16:27 +0000, Ian Jackson wrote: >> * I don't have a clear design proposal for the above but I think Doug >> can probably provide one. I'm hoping this is more a matter of >> thinking carefully than of extensive build system programming! > > I think we should: > > 1) Move /usr/lib/debug/xen-4.7-unstable.config to /boot. I previously > didn't care about what path it was, but the usecase of having grub be able > to react to the config (see below) is a strong reason to have it in /boot > IMHO. Jan has said he won't veto such a change, AFAICT everyone else is > happy with it. > > 2) Assume that grub (specifically the patch in http://savannah.gnu.org/bugs > /?43420 and as used by osstest today) will at some point be modified to > look at /boot/xenconfig-$version to decide whether to create an XSM entry > or not instead of the presence of /boot/xenpolicy-$version. This step > belongs here logically but chronologically could come much later since > osstest will do the right thing even if there is a spurious > /boot/xenpolicy-$version file (which is to say it will ignore the spurious > entry and boot the right thing). > > 3) Have tools/* always build the FLASK+XSM tools _and_ the FLASK policy and > to always install both. Any related configure options can go away and we no > longer need to worry about synchronising the configuration of the tools and > xen trees, this is desirable because we would prefer to have one set of > tools which gracefully handles differing hypervisor configurations over > needing different sets of tools (FLASK+XSM was one of the few exceptions to > that rule AFAICT). > > I think with this plan there is no need to modify osstest.git, since it > already does the right thing (which is, it sets XSM for Xen builds, which > in turn enables FLASK and it does nothing for tools/* which is correct once > #3 above has happened). > > The only downside is a spurious /boot/xenpolicy-$version installed when the > corresponding Xen binary doesn't support XSM, however given the assumption > in #2 (which implies the user will never see a spurious grub entry, which > is the important thing) and the fact that it avoids the complexity of > having tools/* rely in some way on xen/.config I think that is a worthwhile > trade-off. > > Hopefully this simplifies a bunch of the arguments we have been having and > provides a path forwards? > > Objections? My opinion on 1 and 2 is known; 3 seems like a good step to me. Jan
On 1/15/16 11:06 AM, Ian Campbell wrote: > On Thu, 2016-01-14 at 16:27 +0000, Ian Jackson wrote: >> * I don't have a clear design proposal for the above but I think Doug >> can probably provide one. I'm hoping this is more a matter of >> thinking carefully than of extensive build system programming! > > I think we should: > > 1) Move /usr/lib/debug/xen-4.7-unstable.config to /boot. I previously > didn't care about what path it was, but the usecase of having grub be able > to react to the config (see below) is a strong reason to have it in /boot > IMHO. Jan has said he won't veto such a change, AFAICT everyone else is > happy with it. > > 2) Assume that grub (specifically the patch in http://savannah.gnu.org/bugs > /?43420 and as used by osstest today) will at some point be modified to > look at /boot/xenconfig-$version to decide whether to create an XSM entry > or not instead of the presence of /boot/xenpolicy-$version. This step > belongs here logically but chronologically could come much later since > osstest will do the right thing even if there is a spurious > /boot/xenpolicy-$version file (which is to say it will ignore the spurious > entry and boot the right thing). > > 3) Have tools/* always build the FLASK+XSM tools _and_ the FLASK policy and > to always install both. Any related configure options can go away and we no > longer need to worry about synchronising the configuration of the tools and > xen trees, this is desirable because we would prefer to have one set of > tools which gracefully handles differing hypervisor configurations over > needing different sets of tools (FLASK+XSM was one of the few exceptions to > that rule AFAICT). > > I think with this plan there is no need to modify osstest.git, since it > already does the right thing (which is, it sets XSM for Xen builds, which > in turn enables FLASK and it does nothing for tools/* which is correct once > #3 above has happened). > > The only downside is a spurious /boot/xenpolicy-$version installed when the > corresponding Xen binary doesn't support XSM, however given the assumption > in #2 (which implies the user will never see a spurious grub entry, which > is the important thing) and the fact that it avoids the complexity of > having tools/* rely in some way on xen/.config I think that is a worthwhile > trade-off. > > Hopefully this simplifies a bunch of the arguments we have been having and > provides a path forwards? > > Objections? > > Doug, I think you have the majority of #1 and #3 already in some form or > other? #2 is not on the critical path and can come later. > > Ian. > Yes. No objections here. Yes I've submitted a patch for #1 and #3 earlier. They'll need to be rebased but that's easy.
On 15/01/16 17:15, Jan Beulich wrote: >>>> On 15.01.16 at 18:06, <ian.campbell@citrix.com> wrote: >> On Thu, 2016-01-14 at 16:27 +0000, Ian Jackson wrote: >>> * I don't have a clear design proposal for the above but I think Doug >>> can probably provide one. I'm hoping this is more a matter of >>> thinking carefully than of extensive build system programming! >> I think we should: >> >> 1) Move /usr/lib/debug/xen-4.7-unstable.config to /boot. I previously >> didn't care about what path it was, but the usecase of having grub be able >> to react to the config (see below) is a strong reason to have it in /boot >> IMHO. Jan has said he won't veto such a change, AFAICT everyone else is >> happy with it. >> >> 2) Assume that grub (specifically the patch in http://savannah.gnu.org/bugs >> /?43420 and as used by osstest today) will at some point be modified to >> look at /boot/xenconfig-$version to decide whether to create an XSM entry >> or not instead of the presence of /boot/xenpolicy-$version. This step >> belongs here logically but chronologically could come much later since >> osstest will do the right thing even if there is a spurious >> /boot/xenpolicy-$version file (which is to say it will ignore the spurious >> entry and boot the right thing). >> >> 3) Have tools/* always build the FLASK+XSM tools _and_ the FLASK policy and >> to always install both. Any related configure options can go away and we no >> longer need to worry about synchronising the configuration of the tools and >> xen trees, this is desirable because we would prefer to have one set of >> tools which gracefully handles differing hypervisor configurations over >> needing different sets of tools (FLASK+XSM was one of the few exceptions to >> that rule AFAICT). >> >> I think with this plan there is no need to modify osstest.git, since it >> already does the right thing (which is, it sets XSM for Xen builds, which >> in turn enables FLASK and it does nothing for tools/* which is correct once >> #3 above has happened). >> >> The only downside is a spurious /boot/xenpolicy-$version installed when the >> corresponding Xen binary doesn't support XSM, however given the assumption >> in #2 (which implies the user will never see a spurious grub entry, which >> is the important thing) and the fact that it avoids the complexity of >> having tools/* rely in some way on xen/.config I think that is a worthwhile >> trade-off. >> >> Hopefully this simplifies a bunch of the arguments we have been having and >> provides a path forwards? >> >> Objections? > My opinion on 1 and 2 is known; 3 seems like a good step to me. FWIW, I also prefer option 3. It lends itself better to a toolstack which functions in the same way, irrespective of hypervisor configuration. ~Andrew
On Fri, 2016-01-15 at 17:24 +0000, Andrew Cooper wrote: > On 15/01/16 17:15, Jan Beulich wrote: > > > > > On 15.01.16 at 18:06, <ian.campbell@citrix.com> wrote: > > > On Thu, 2016-01-14 at 16:27 +0000, Ian Jackson wrote: > > > > * I don't have a clear design proposal for the above but I think Doug > > > > can probably provide one. I'm hoping this is more a matter of > > > > thinking carefully than of extensive build system programming! > > > I think we should: > > > > > > 1) Move /usr/lib/debug/xen-4.7-unstable.config to /boot. I previously > > > didn't care about what path it was, but the usecase of having grub be able > > > to react to the config (see below) is a strong reason to have it in /boot > > > IMHO. Jan has said he won't veto such a change, AFAICT everyone else is > > > happy with it. > > > > > > 2) Assume that grub (specifically the patch in http://savannah.gnu.org/bugs > > > /?43420 and as used by osstest today) will at some point be modified to > > > look at /boot/xenconfig-$version to decide whether to create an XSM entry > > > or not instead of the presence of /boot/xenpolicy-$version. This step > > > belongs here logically but chronologically could come much later since > > > osstest will do the right thing even if there is a spurious > > > /boot/xenpolicy-$version file (which is to say it will ignore the spurious > > > entry and boot the right thing). > > > > > > 3) Have tools/* always build the FLASK+XSM tools _and_ the FLASK policy and > > > to always install both. Any related configure options can go away and we no > > > longer need to worry about synchronising the configuration of the tools and > > > xen trees, this is desirable because we would prefer to have one set of > > > tools which gracefully handles differing hypervisor configurations over > > > needing different sets of tools (FLASK+XSM was one of the few exceptions to > > > that rule AFAICT). > > > > > > I think with this plan there is no need to modify osstest.git, since it > > > already does the right thing (which is, it sets XSM for Xen builds, which > > > in turn enables FLASK and it does nothing for tools/* which is correct once > > > #3 above has happened). > > > > > > The only downside is a spurious /boot/xenpolicy-$version installed when the > > > corresponding Xen binary doesn't support XSM, however given the assumption > > > in #2 (which implies the user will never see a spurious grub entry, which > > > is the important thing) and the fact that it avoids the complexity of > > > having tools/* rely in some way on xen/.config I think that is a worthwhile > > > trade-off. > > > > > > Hopefully this simplifies a bunch of the arguments we have been having and > > > provides a path forwards? > > > > > > Objections? > > My opinion on 1 and 2 is known; 3 seems like a good step to me. > > FWIW, I also prefer option 3. It lends itself better to a toolstack > which functions in the same way, irrespective of hypervisor configuration. To be clear: These are not options, they are steps in a plan, to be followed in order. Ian.
On 1/14/16 8:28 AM, Doug Goldstein wrote: > If the test should build with XSM then supply --enable-xsmpolicy to the > tools/configure script otherwise provide --disable-xsmpolicy. This will > allow the default to change from --enable-xsmpolicy to > --disable-xsmpolicy in the Xen tree without breaking OSSTest. > > Signed-off-by: Doug Goldstein <cardoe@cardoe.com> > --- > ts-xen-build | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/ts-xen-build b/ts-xen-build > index bc4e41a..4812dff 100755 > --- a/ts-xen-build > +++ b/ts-xen-build > @@ -111,6 +111,7 @@ END > sub build () { > my $xend_opt= $r{enable_xend} =~ m/true/ ? "--enable-xend" : "--disable-xend"; > my $ovmf_opt= $r{enable_ovmf} =~ m/true/ ? "--enable-ovmf" : "--disable-ovmf"; > + my $xsm_opt= $r{enable_xsm} =~ m/true/ ? "--enable-xsmpolicy" : "--disable-xsmpolicy"; > > my $configure_prefix = $r{cmdprefix_configure} // ''; > my $make_prefix = $r{cmdprefix_make} // ''; > @@ -123,8 +124,11 @@ sub build () { > if grep -q -- $ovmf_opt tools/configure ; then > ovmf=$ovmf_opt > fi > + if grep -q -- $xsm_opt tools/configure ; then > + xsm=$xsm_opt > + fi > END > - $configure_prefix ./configure --sysconfdir=/etc \$xend \$ovmf > + $configure_prefix ./configure --sysconfdir=/etc \$xend \$ovmf \$xsm > END > fi > END > Given the support of the maintainers and committers for the proposal [1], this patch should NOT be applied. [1] http://lists.xenproject.org/archives/html/xen-devel/2016-01/msg01796.html
>>> On 15.01.16 at 18:42, <ian.campbell@citrix.com> wrote: > On Fri, 2016-01-15 at 17:24 +0000, Andrew Cooper wrote: >> On 15/01/16 17:15, Jan Beulich wrote: >> > > > > On 15.01.16 at 18:06, <ian.campbell@citrix.com> wrote: >> > > On Thu, 2016-01-14 at 16:27 +0000, Ian Jackson wrote: >> > > > * I don't have a clear design proposal for the above but I think Doug >> > > > can probably provide one. I'm hoping this is more a matter of >> > > > thinking carefully than of extensive build system programming! >> > > I think we should: >> > > >> > > 1) Move /usr/lib/debug/xen-4.7-unstable.config to /boot. I previously >> > > didn't care about what path it was, but the usecase of having grub be able >> > > to react to the config (see below) is a strong reason to have it in /boot >> > > IMHO. Jan has said he won't veto such a change, AFAICT everyone else is >> > > happy with it. >> > > >> > > 2) Assume that grub (specifically the patch in http://savannah.gnu.org/bugs >> > > /?43420 and as used by osstest today) will at some point be modified to >> > > look at /boot/xenconfig-$version to decide whether to create an XSM entry >> > > or not instead of the presence of /boot/xenpolicy-$version. This step >> > > belongs here logically but chronologically could come much later since >> > > osstest will do the right thing even if there is a spurious >> > > /boot/xenpolicy-$version file (which is to say it will ignore the spurious >> > > entry and boot the right thing). >> > > >> > > 3) Have tools/* always build the FLASK+XSM tools _and_ the FLASK policy and >> > > to always install both. Any related configure options can go away and we no >> > > longer need to worry about synchronising the configuration of the tools and >> > > xen trees, this is desirable because we would prefer to have one set of >> > > tools which gracefully handles differing hypervisor configurations over >> > > needing different sets of tools (FLASK+XSM was one of the few exceptions to >> > > that rule AFAICT). >> > > >> > > I think with this plan there is no need to modify osstest.git, since it >> > > already does the right thing (which is, it sets XSM for Xen builds, which >> > > in turn enables FLASK and it does nothing for tools/* which is correct once >> > > #3 above has happened). >> > > >> > > The only downside is a spurious /boot/xenpolicy-$version installed when the >> > > corresponding Xen binary doesn't support XSM, however given the assumption >> > > in #2 (which implies the user will never see a spurious grub entry, which >> > > is the important thing) and the fact that it avoids the complexity of >> > > having tools/* rely in some way on xen/.config I think that is a worthwhile >> > > trade-off. >> > > >> > > Hopefully this simplifies a bunch of the arguments we have been having and >> > > provides a path forwards? >> > > >> > > Objections? >> > My opinion on 1 and 2 is known; 3 seems like a good step to me. >> >> FWIW, I also prefer option 3. It lends itself better to a toolstack >> which functions in the same way, irrespective of hypervisor configuration. > > To be clear: These are not options, they are steps in a plan, to be > followed in order. "Not options" - indeed, but "in order"? At least 3 seems independent of both 1 and 2. Jan
On 18/01/2016 07:49, Jan Beulich wrote: >>>> On 15.01.16 at 18:42, <ian.campbell@citrix.com> wrote: >> On Fri, 2016-01-15 at 17:24 +0000, Andrew Cooper wrote: >>> On 15/01/16 17:15, Jan Beulich wrote: >>>>>>> On 15.01.16 at 18:06, <ian.campbell@citrix.com> wrote: >>>>> On Thu, 2016-01-14 at 16:27 +0000, Ian Jackson wrote: >>>>>> * I don't have a clear design proposal for the above but I think Doug >>>>>> can probably provide one. I'm hoping this is more a matter of >>>>>> thinking carefully than of extensive build system programming! >>>>> I think we should: >>>>> >>>>> 1) Move /usr/lib/debug/xen-4.7-unstable.config to /boot. I previously >>>>> didn't care about what path it was, but the usecase of having grub be able >>>>> to react to the config (see below) is a strong reason to have it in /boot >>>>> IMHO. Jan has said he won't veto such a change, AFAICT everyone else is >>>>> happy with it. >>>>> >>>>> 2) Assume that grub (specifically the patch in http://savannah.gnu.org/bugs >>>>> /?43420 and as used by osstest today) will at some point be modified to >>>>> look at /boot/xenconfig-$version to decide whether to create an XSM entry >>>>> or not instead of the presence of /boot/xenpolicy-$version. This step >>>>> belongs here logically but chronologically could come much later since >>>>> osstest will do the right thing even if there is a spurious >>>>> /boot/xenpolicy-$version file (which is to say it will ignore the spurious >>>>> entry and boot the right thing). >>>>> >>>>> 3) Have tools/* always build the FLASK+XSM tools _and_ the FLASK policy and >>>>> to always install both. Any related configure options can go away and we no >>>>> longer need to worry about synchronising the configuration of the tools and >>>>> xen trees, this is desirable because we would prefer to have one set of >>>>> tools which gracefully handles differing hypervisor configurations over >>>>> needing different sets of tools (FLASK+XSM was one of the few exceptions to >>>>> that rule AFAICT). >>>>> >>>>> I think with this plan there is no need to modify osstest.git, since it >>>>> already does the right thing (which is, it sets XSM for Xen builds, which >>>>> in turn enables FLASK and it does nothing for tools/* which is correct once >>>>> #3 above has happened). >>>>> >>>>> The only downside is a spurious /boot/xenpolicy-$version installed when the >>>>> corresponding Xen binary doesn't support XSM, however given the assumption >>>>> in #2 (which implies the user will never see a spurious grub entry, which >>>>> is the important thing) and the fact that it avoids the complexity of >>>>> having tools/* rely in some way on xen/.config I think that is a worthwhile >>>>> trade-off. >>>>> >>>>> Hopefully this simplifies a bunch of the arguments we have been having and >>>>> provides a path forwards? >>>>> >>>>> Objections? >>>> My opinion on 1 and 2 is known; 3 seems like a good step to me. >>> FWIW, I also prefer option 3. It lends itself better to a toolstack >>> which functions in the same way, irrespective of hypervisor configuration. >> To be clear: These are not options, they are steps in a plan, to be >> followed in order. > "Not options" - indeed, but "in order"? At least 3 seems independent > of both 1 and 2. 3 is required to unblock OSSTest and needs to happen ASAP. That is the absolute priority at this point. ~Andrew
On Mon, 2016-01-18 at 00:49 -0700, Jan Beulich wrote: > > > > On 15.01.16 at 18:42, <ian.campbell@citrix.com> wrote: > > On Fri, 2016-01-15 at 17:24 +0000, Andrew Cooper wrote: > > > On 15/01/16 17:15, Jan Beulich wrote: > > > > > > > On 15.01.16 at 18:06, <ian.campbell@citrix.com> wrote: > > > > > On Thu, 2016-01-14 at 16:27 +0000, Ian Jackson wrote: > > > > > > * I don't have a clear design proposal for the above but I > > > > > > think Doug > > > > > > can probably provide one. I'm hoping this is more a matter > > > > > > of > > > > > > thinking carefully than of extensive build system > > > > > > programming! > > > > > I think we should: > > > > > > > > > > 1) Move /usr/lib/debug/xen-4.7-unstable.config to /boot. I > > > > > previously > > > > > didn't care about what path it was, but the usecase of having > > > > > grub be able > > > > > to react to the config (see below) is a strong reason to have it > > > > > in /boot > > > > > IMHO. Jan has said he won't veto such a change, AFAICT everyone > > > > > else is > > > > > happy with it. > > > > > > > > > > 2) Assume that grub (specifically the patch in http://savannah.gn > > > > > u.org/bugs > > > > > /?43420 and as used by osstest today) will at some point be > > > > > modified to > > > > > look at /boot/xenconfig-$version to decide whether to create an > > > > > XSM entry > > > > > or not instead of the presence of /boot/xenpolicy-$version. This > > > > > step > > > > > belongs here logically but chronologically could come much later > > > > > since > > > > > osstest will do the right thing even if there is a spurious > > > > > /boot/xenpolicy-$version file (which is to say it will ignore the > > > > > spurious > > > > > entry and boot the right thing). > > > > > > > > > > 3) Have tools/* always build the FLASK+XSM tools _and_ the FLASK > > > > > policy and > > > > > to always install both. Any related configure options can go away > > > > > and we no > > > > > longer need to worry about synchronising the configuration of the > > > > > tools and > > > > > xen trees, this is desirable because we would prefer to have one > > > > > set of > > > > > tools which gracefully handles differing hypervisor > > > > > configurations over > > > > > needing different sets of tools (FLASK+XSM was one of the few > > > > > exceptions to > > > > > that rule AFAICT). > > > > > > > > > > I think with this plan there is no need to modify osstest.git, > > > > > since it > > > > > already does the right thing (which is, it sets XSM for Xen > > > > > builds, which > > > > > in turn enables FLASK and it does nothing for tools/* which is > > > > > correct once > > > > > #3 above has happened). > > > > > > > > > > The only downside is a spurious /boot/xenpolicy-$version > > > > > installed when the > > > > > corresponding Xen binary doesn't support XSM, however given the > > > > > assumption > > > > > in #2 (which implies the user will never see a spurious grub > > > > > entry, which > > > > > is the important thing) and the fact that it avoids the > > > > > complexity of > > > > > having tools/* rely in some way on xen/.config I think that is a > > > > > worthwhile > > > > > trade-off. > > > > > > > > > > Hopefully this simplifies a bunch of the arguments we have been > > > > > having and > > > > > provides a path forwards? > > > > > > > > > > Objections? > > > > My opinion on 1 and 2 is known; 3 seems like a good step to me. > > > > > > FWIW, I also prefer option 3. It lends itself better to a toolstack > > > which functions in the same way, irrespective of hypervisor > > > configuration. > > > > To be clear: These are not options, they are steps in a plan, to be > > followed in order. > > "Not options" - indeed, but "in order"? At least 3 seems independent > of both 1 and 2. Without #1 we cannot make the assumption in #2. Without (the assumption of eventually doing) #2 we cannot do #3 because that would result in spurious boot menu entries for users (i.e. ones which claim to enable XSM but boot an XSM incapable Xen) which several of us feel we should avoid. Ian.
>>> On 18.01.16 at 10:41, <ian.campbell@citrix.com> wrote: > On Mon, 2016-01-18 at 00:49 -0700, Jan Beulich wrote: >> > > > On 15.01.16 at 18:42, <ian.campbell@citrix.com> wrote: >> > On Fri, 2016-01-15 at 17:24 +0000, Andrew Cooper wrote: >> > > On 15/01/16 17:15, Jan Beulich wrote: >> > > > > > > On 15.01.16 at 18:06, <ian.campbell@citrix.com> wrote: >> > > > > On Thu, 2016-01-14 at 16:27 +0000, Ian Jackson wrote: >> > > > > > * I don't have a clear design proposal for the above but I >> > > > > > think Doug >> > > > > > can probably provide one. I'm hoping this is more a matter >> > > > > > of >> > > > > > thinking carefully than of extensive build system >> > > > > > programming! >> > > > > I think we should: >> > > > > >> > > > > 1) Move /usr/lib/debug/xen-4.7-unstable.config to /boot. I >> > > > > previously >> > > > > didn't care about what path it was, but the usecase of having >> > > > > grub be able >> > > > > to react to the config (see below) is a strong reason to have it >> > > > > in /boot >> > > > > IMHO. Jan has said he won't veto such a change, AFAICT everyone >> > > > > else is >> > > > > happy with it. >> > > > > >> > > > > 2) Assume that grub (specifically the patch in http://savannah.gn >> > > > > u.org/bugs >> > > > > /?43420 and as used by osstest today) will at some point be >> > > > > modified to >> > > > > look at /boot/xenconfig-$version to decide whether to create an >> > > > > XSM entry >> > > > > or not instead of the presence of /boot/xenpolicy-$version. This >> > > > > step >> > > > > belongs here logically but chronologically could come much later >> > > > > since >> > > > > osstest will do the right thing even if there is a spurious >> > > > > /boot/xenpolicy-$version file (which is to say it will ignore the >> > > > > spurious >> > > > > entry and boot the right thing). >> > > > > >> > > > > 3) Have tools/* always build the FLASK+XSM tools _and_ the FLASK >> > > > > policy and >> > > > > to always install both. Any related configure options can go away >> > > > > and we no >> > > > > longer need to worry about synchronising the configuration of the >> > > > > tools and >> > > > > xen trees, this is desirable because we would prefer to have one >> > > > > set of >> > > > > tools which gracefully handles differing hypervisor >> > > > > configurations over >> > > > > needing different sets of tools (FLASK+XSM was one of the few >> > > > > exceptions to >> > > > > that rule AFAICT). >> > > > > >> > > > > I think with this plan there is no need to modify osstest.git, >> > > > > since it >> > > > > already does the right thing (which is, it sets XSM for Xen >> > > > > builds, which >> > > > > in turn enables FLASK and it does nothing for tools/* which is >> > > > > correct once >> > > > > #3 above has happened). >> > > > > >> > > > > The only downside is a spurious /boot/xenpolicy-$version >> > > > > installed when the >> > > > > corresponding Xen binary doesn't support XSM, however given the >> > > > > assumption >> > > > > in #2 (which implies the user will never see a spurious grub >> > > > > entry, which >> > > > > is the important thing) and the fact that it avoids the >> > > > > complexity of >> > > > > having tools/* rely in some way on xen/.config I think that is a >> > > > > worthwhile >> > > > > trade-off. >> > > > > >> > > > > Hopefully this simplifies a bunch of the arguments we have been >> > > > > having and >> > > > > provides a path forwards? >> > > > > >> > > > > Objections? >> > > > My opinion on 1 and 2 is known; 3 seems like a good step to me. >> > > >> > > FWIW, I also prefer option 3. It lends itself better to a toolstack >> > > which functions in the same way, irrespective of hypervisor >> > > configuration. >> > >> > To be clear: These are not options, they are steps in a plan, to be >> > followed in order. >> >> "Not options" - indeed, but "in order"? At least 3 seems independent >> of both 1 and 2. > > Without #1 we cannot make the assumption in #2. > > Without (the assumption of eventually doing) #2 we cannot do #3 because > that would result in spurious boot menu entries for users (i.e. ones which > claim to enable XSM but boot an XSM incapable Xen) which several of us feel > we should avoid. Ugly. Could we live with that until #1 and #2 get put in place? Otherwise it looks very much like reverting the two Kconfig conversion patches is the only possible solution at this point... Jan
diff --git a/ts-xen-build b/ts-xen-build index bc4e41a..4812dff 100755 --- a/ts-xen-build +++ b/ts-xen-build @@ -111,6 +111,7 @@ END sub build () { my $xend_opt= $r{enable_xend} =~ m/true/ ? "--enable-xend" : "--disable-xend"; my $ovmf_opt= $r{enable_ovmf} =~ m/true/ ? "--enable-ovmf" : "--disable-ovmf"; + my $xsm_opt= $r{enable_xsm} =~ m/true/ ? "--enable-xsmpolicy" : "--disable-xsmpolicy"; my $configure_prefix = $r{cmdprefix_configure} // ''; my $make_prefix = $r{cmdprefix_make} // ''; @@ -123,8 +124,11 @@ sub build () { if grep -q -- $ovmf_opt tools/configure ; then ovmf=$ovmf_opt fi + if grep -q -- $xsm_opt tools/configure ; then + xsm=$xsm_opt + fi END - $configure_prefix ./configure --sysconfdir=/etc \$xend \$ovmf + $configure_prefix ./configure --sysconfdir=/etc \$xend \$ovmf \$xsm END fi END
If the test should build with XSM then supply --enable-xsmpolicy to the tools/configure script otherwise provide --disable-xsmpolicy. This will allow the default to change from --enable-xsmpolicy to --disable-xsmpolicy in the Xen tree without breaking OSSTest. Signed-off-by: Doug Goldstein <cardoe@cardoe.com> --- ts-xen-build | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)