Commit 1787cc167906f3f ("libs/guest: Move the guest ABI check earlier
into xc_dom_parse_image()") broke starting the xenstore stubdom. This
is due to a rather special way the xenstore stubdom domain config is
being initialized: in order to support both, PV and PVH stubdom,
init-xenstore-domain is using xc_dom_parse_image() to find the correct
domain type. Unfortunately above commit requires xc_dom_boot_xen_init()
to have been called before using xc_dom_parse_image(). This requires
the domid, which is known only after xc_domain_create(), which requires
the domain type.
In order to break this circular dependency, call xc_dom_boot_xen_init()
with an arbitrary domid first, and then set dom->guest_domid later.
Fixes: 1787cc167906f3f ("libs/guest: Move the guest ABI check earlier into xc_dom_parse_image()")
Signed-off-by: Juergen Gross <jgross@suse.com>
Release-acked-by: Ian Jackson <iwj@xenproject.org>
---
tools/helpers/init-xenstore-domain.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/tools/helpers/init-xenstore-domain.c b/tools/helpers/init-xenstore-domain.c
index 6836002f0b..a79662bd1b 100644
--- a/tools/helpers/init-xenstore-domain.c
+++ b/tools/helpers/init-xenstore-domain.c
@@ -132,6 +132,13 @@ static int build(xc_interface *xch)
}
}
+ rv = xc_dom_boot_xen_init(dom, xch, domid);
+ if ( rv )
+ {
+ fprintf(stderr, "xc_dom_boot_xen_init failed\n");
+ goto err;
+ }
+
dom->container_type = XC_DOM_HVM_CONTAINER;
rv = xc_dom_parse_image(dom);
if ( rv )
@@ -214,16 +221,11 @@ static int build(xc_interface *xch)
else
snprintf(cmdline, 512, "--event %d --internal-db", rv);
+ dom->guest_domid = domid;
dom->cmdline = xc_dom_strdup(dom, cmdline);
dom->xenstore_domid = domid;
dom->console_evtchn = console_evtchn;
- rv = xc_dom_boot_xen_init(dom, xch, domid);
- if ( rv )
- {
- fprintf(stderr, "xc_dom_boot_xen_init failed\n");
- goto err;
- }
rv = xc_dom_mem_init(dom, memory);
if ( rv )
{
--
2.26.2
On 04/11/2021 14:42, Juergen Gross wrote: > Commit 1787cc167906f3f ("libs/guest: Move the guest ABI check earlier > into xc_dom_parse_image()") broke starting the xenstore stubdom. This > is due to a rather special way the xenstore stubdom domain config is > being initialized: in order to support both, PV and PVH stubdom, > init-xenstore-domain is using xc_dom_parse_image() to find the correct > domain type. Unfortunately above commit requires xc_dom_boot_xen_init() > to have been called before using xc_dom_parse_image(). This requires > the domid, which is known only after xc_domain_create(), which requires > the domain type. > > In order to break this circular dependency, call xc_dom_boot_xen_init() > with an arbitrary domid first, and then set dom->guest_domid later. > > Fixes: 1787cc167906f3f ("libs/guest: Move the guest ABI check earlier into xc_dom_parse_image()") > Signed-off-by: Juergen Gross <jgross@suse.com> > Release-acked-by: Ian Jackson <iwj@xenproject.org> This is all rather nasty, and really highlights problems with the libxenguest abi, because you really ought to be able to ask "what ELF properties do I have on my hand" without an implicit "and try to start building a VM using it" on the side. I agree this is probably the best thing to do right now. Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
On Thu, Nov 4, 2021 at 11:00 AM Andrew Cooper <andrew.cooper3@citrix.com> wrote: > > On 04/11/2021 14:42, Juergen Gross wrote: > > Commit 1787cc167906f3f ("libs/guest: Move the guest ABI check earlier > > into xc_dom_parse_image()") broke starting the xenstore stubdom. This > > is due to a rather special way the xenstore stubdom domain config is > > being initialized: in order to support both, PV and PVH stubdom, > > init-xenstore-domain is using xc_dom_parse_image() to find the correct > > domain type. Unfortunately above commit requires xc_dom_boot_xen_init() > > to have been called before using xc_dom_parse_image(). This requires > > the domid, which is known only after xc_domain_create(), which requires > > the domain type. > > > > In order to break this circular dependency, call xc_dom_boot_xen_init() > > with an arbitrary domid first, and then set dom->guest_domid later. > > > > Fixes: 1787cc167906f3f ("libs/guest: Move the guest ABI check earlier into xc_dom_parse_image()") > > Signed-off-by: Juergen Gross <jgross@suse.com> > > Release-acked-by: Ian Jackson <iwj@xenproject.org> > > This is all rather nasty, and really highlights problems with the > libxenguest abi, because you really ought to be able to ask "what ELF > properties do I have on my hand" without an implicit "and try to start > building a VM using it" on the side. > > I agree this is probably the best thing to do right now. Yes, this is probably the best change before release. If xc_dom_allocate() filled in dom->xen_version & dom->xen_caps - i.e. move that from xc_dom_boot_xen_init() - then I think this patch wouldn't be necessary. But there could be side effects of such a change. Regards, Jason
On 04.11.21 17:33, Jason Andryuk wrote: > On Thu, Nov 4, 2021 at 11:00 AM Andrew Cooper <andrew.cooper3@citrix.com> wrote: >> >> On 04/11/2021 14:42, Juergen Gross wrote: >>> Commit 1787cc167906f3f ("libs/guest: Move the guest ABI check earlier >>> into xc_dom_parse_image()") broke starting the xenstore stubdom. This >>> is due to a rather special way the xenstore stubdom domain config is >>> being initialized: in order to support both, PV and PVH stubdom, >>> init-xenstore-domain is using xc_dom_parse_image() to find the correct >>> domain type. Unfortunately above commit requires xc_dom_boot_xen_init() >>> to have been called before using xc_dom_parse_image(). This requires >>> the domid, which is known only after xc_domain_create(), which requires >>> the domain type. >>> >>> In order to break this circular dependency, call xc_dom_boot_xen_init() >>> with an arbitrary domid first, and then set dom->guest_domid later. >>> >>> Fixes: 1787cc167906f3f ("libs/guest: Move the guest ABI check earlier into xc_dom_parse_image()") >>> Signed-off-by: Juergen Gross <jgross@suse.com> >>> Release-acked-by: Ian Jackson <iwj@xenproject.org> >> >> This is all rather nasty, and really highlights problems with the >> libxenguest abi, because you really ought to be able to ask "what ELF >> properties do I have on my hand" without an implicit "and try to start >> building a VM using it" on the side. >> >> I agree this is probably the best thing to do right now. > > Yes, this is probably the best change before release. > > If xc_dom_allocate() filled in dom->xen_version & dom->xen_caps - i.e. > move that from xc_dom_boot_xen_init() - then I think this patch > wouldn't be necessary. But there could be side effects of such a > change. This is a nice idea for a cleanup patch after 4.16 has been branched. Juergen
© 2016 - 2024 Red Hat, Inc.