Message ID | 20200205112412.19414-1-pdurrant@amazon.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [v2] libxl: fix assertion failure in stub domain creation | expand |
On Wed, Feb 05, 2020 at 11:24:12AM +0000, Paul Durrant wrote: > An assertion in libxl__domain_make(): > > 'soft_reset || *domid == INVALID_DOMID' > > does not hold true for stub domain creation, where soft_reset is false > but the passed in domid == 0. This is easily fixed by changing the > initializer in libxl__spawn_stub_dm(). > > NOTE: The comment for XEN_DOMCTL_createdomain in domctl.h is changed to > reflect reality. > > Fixes: 75259239d85d ("libxl_create: make 'soft reset' explicit") > Signed-off-by: Paul Durrant <pdurrant@amazon.com> Acked-by: Wei Liu <wl@xen.org> I will wait until end of today in case others want to comment on the domctl.h change. I will also carry over Anthony's ack from v1 while committing. Wei. > --- > Cc: Ian Jackson <ian.jackson@eu.citrix.com> > Cc: Wei Liu <wl@xen.org> > Cc: Anthony PERARD <anthony.perard@citrix.com> > Cc: Roger Pau Monné <roger.pau@citrix.com> > Cc: Andrew Cooper <andrew.cooper3@citrix.com> > Cc: George Dunlap <George.Dunlap@eu.citrix.com> > Cc: Jan Beulich <jbeulich@suse.com> > Cc: Julien Grall <julien@xen.org> > Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > Cc: Stefano Stabellini <sstabellini@kernel.org> > > An example of the assertion failure can be seen at: > > http://logs.test-lab.xenproject.org/osstest/logs/146726/test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm/10.ts-debian-hvm-install.log > --- > tools/libxl/libxl_dm.c | 2 +- > xen/include/public/domctl.h | 3 ++- > 2 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c > index f758daf3b6..3b1da90167 100644 > --- a/tools/libxl/libxl_dm.c > +++ b/tools/libxl/libxl_dm.c > @@ -2127,7 +2127,7 @@ void libxl__spawn_stub_dm(libxl__egc *egc, libxl__stub_dm_spawn_state *sdss) > goto out; > } > > - sdss->pvqemu.guest_domid = 0; > + sdss->pvqemu.guest_domid = INVALID_DOMID; > > libxl_domain_create_info_init(&dm_config->c_info); > dm_config->c_info.type = LIBXL_DOMAIN_TYPE_PV; > diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h > index 2bb7397923..fec6f6fdd1 100644 > --- a/xen/include/public/domctl.h > +++ b/xen/include/public/domctl.h > @@ -42,7 +42,8 @@ > > /* > * NB. xen_domctl.domain is an IN/OUT parameter for this operation. > - * If it is specified as zero, an id is auto-allocated and returned. > + * If it is specified as an invalid value (0 or >= DOMID_FIRST_RESERVED), > + * an id is auto-allocated and returned. > */ > /* XEN_DOMCTL_createdomain */ > struct xen_domctl_createdomain { > -- > 2.20.1 >
On 05.02.2020 12:24, Paul Durrant wrote: > --- a/xen/include/public/domctl.h > +++ b/xen/include/public/domctl.h > @@ -42,7 +42,8 @@ > > /* > * NB. xen_domctl.domain is an IN/OUT parameter for this operation. > - * If it is specified as zero, an id is auto-allocated and returned. > + * If it is specified as an invalid value (0 or >= DOMID_FIRST_RESERVED), > + * an id is auto-allocated and returned. > */ > /* XEN_DOMCTL_createdomain */ > struct xen_domctl_createdomain { Acked-by: Jan Beulich <jbeulich@suse.com>
On Wed, Feb 05, 2020 at 11:24:12AM +0000, Paul Durrant wrote: > NOTE: The comment for XEN_DOMCTL_createdomain in domctl.h is changed to > reflect reality. Surely, this change deserve to be in its own patch, isn't it?
On Wed, Feb 05, 2020 at 11:29:30AM +0000, Anthony PERARD wrote: > On Wed, Feb 05, 2020 at 11:24:12AM +0000, Paul Durrant wrote: > > NOTE: The comment for XEN_DOMCTL_createdomain in domctl.h is changed to > > reflect reality. > > Surely, this change deserve to be in its own patch, isn't it? Ideally I would want that too. But this patch is trivial so I'm not too fussed either way. Wei. > > -- > Anthony PERARD
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c index f758daf3b6..3b1da90167 100644 --- a/tools/libxl/libxl_dm.c +++ b/tools/libxl/libxl_dm.c @@ -2127,7 +2127,7 @@ void libxl__spawn_stub_dm(libxl__egc *egc, libxl__stub_dm_spawn_state *sdss) goto out; } - sdss->pvqemu.guest_domid = 0; + sdss->pvqemu.guest_domid = INVALID_DOMID; libxl_domain_create_info_init(&dm_config->c_info); dm_config->c_info.type = LIBXL_DOMAIN_TYPE_PV; diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h index 2bb7397923..fec6f6fdd1 100644 --- a/xen/include/public/domctl.h +++ b/xen/include/public/domctl.h @@ -42,7 +42,8 @@ /* * NB. xen_domctl.domain is an IN/OUT parameter for this operation. - * If it is specified as zero, an id is auto-allocated and returned. + * If it is specified as an invalid value (0 or >= DOMID_FIRST_RESERVED), + * an id is auto-allocated and returned. */ /* XEN_DOMCTL_createdomain */ struct xen_domctl_createdomain {
An assertion in libxl__domain_make(): 'soft_reset || *domid == INVALID_DOMID' does not hold true for stub domain creation, where soft_reset is false but the passed in domid == 0. This is easily fixed by changing the initializer in libxl__spawn_stub_dm(). NOTE: The comment for XEN_DOMCTL_createdomain in domctl.h is changed to reflect reality. Fixes: 75259239d85d ("libxl_create: make 'soft reset' explicit") Signed-off-by: Paul Durrant <pdurrant@amazon.com> --- Cc: Ian Jackson <ian.jackson@eu.citrix.com> Cc: Wei Liu <wl@xen.org> Cc: Anthony PERARD <anthony.perard@citrix.com> Cc: Roger Pau Monné <roger.pau@citrix.com> Cc: Andrew Cooper <andrew.cooper3@citrix.com> Cc: George Dunlap <George.Dunlap@eu.citrix.com> Cc: Jan Beulich <jbeulich@suse.com> Cc: Julien Grall <julien@xen.org> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> Cc: Stefano Stabellini <sstabellini@kernel.org> An example of the assertion failure can be seen at: http://logs.test-lab.xenproject.org/osstest/logs/146726/test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm/10.ts-debian-hvm-install.log --- tools/libxl/libxl_dm.c | 2 +- xen/include/public/domctl.h | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-)