| Message ID | 20200205093724.2854-1-pdurrant@amazon.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | libxl: fix assertion failure in stub domain creation | expand |
On Wed, Feb 05, 2020 at 09:37:24AM +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(). > > Fixes: 75259239d85d ("libxl_create: make 'soft reset' explicit") > Signed-off-by: Paul Durrant <pdurrant@amazon.com> > --- > tools/libxl/libxl_dm.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > 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; How this works? INVALID_DOMID seems to be directly feed to xc_domain_create(), which is using XEN_DOMCTL_createdomain. But a comment in domctl.h for XEN_DOMCTL_createdomain read: 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. So, is xc_domain_create going to create a new domain with domid==INVALID_DOMID? Thanks,
> -----Original Message----- > From: Anthony PERARD <anthony.perard@citrix.com> > Sent: 05 February 2020 10:47 > To: Durrant, Paul <pdurrant@amazon.co.uk> > Cc: xen-devel@lists.xenproject.org; Ian Jackson > <ian.jackson@eu.citrix.com>; Wei Liu <wl@xen.org>; Roger Pau Monné > <roger.pau@citrix.com> > Subject: Re: [PATCH] libxl: fix assertion failure in stub domain creation > > On Wed, Feb 05, 2020 at 09:37:24AM +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(). > > > > Fixes: 75259239d85d ("libxl_create: make 'soft reset' explicit") > > Signed-off-by: Paul Durrant <pdurrant@amazon.com> > > --- > > tools/libxl/libxl_dm.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > 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; > > How this works? INVALID_DOMID seems to be directly feed to > xc_domain_create(), which is using XEN_DOMCTL_createdomain. > But a comment in domctl.h for XEN_DOMCTL_createdomain read: > 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. > So, is xc_domain_create going to create a new domain with > domid==INVALID_DOMID? > As it happens, no. That comment is wrong. It should read "If it is not set to a valid value, an id is auto-allocated and returned". Paul
On Wed, Feb 05, 2020 at 10:50:46AM +0000, Durrant, Paul wrote: > > From: Anthony PERARD <anthony.perard@citrix.com> > > On Wed, Feb 05, 2020 at 09:37:24AM +0000, Paul Durrant wrote: > > > > > > - sdss->pvqemu.guest_domid = 0; > > > + sdss->pvqemu.guest_domid = INVALID_DOMID; > > > > How this works? INVALID_DOMID seems to be directly feed to > > xc_domain_create(), which is using XEN_DOMCTL_createdomain. > > But a comment in domctl.h for XEN_DOMCTL_createdomain read: > > 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. > > So, is xc_domain_create going to create a new domain with > > domid==INVALID_DOMID? > > > > As it happens, no. That comment is wrong. It should read "If it is not set to a valid value, an id is auto-allocated and returned". Ok, then: Acked-by: Anthony PERARD <anthony.perard@citrix.com> Thanks,
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;
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(). 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> 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 +- 1 file changed, 1 insertion(+), 1 deletion(-)