| Message ID | 6613fdc72f9e7c4a7eb29ae73c19db810501944f.1582203228.git.pawel@invisiblethingslab.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [XEN,v2] libxl: wait for console path before firing console_available | expand |
On Thu, Feb 20, 2020 at 02:31:03PM +0100, Paweł Marczewski wrote: > If we skip the bootloader, the TTY path will be set for xenconsoled. > However, there is no guarantee that this will happen by the time we > want to call the console_available callback, so we have to wait. > > Signed-off-by: Paweł Marczewski <pawel@invisiblethingslab.com> With minor fix below: Reviewed-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> > --- > Changed since v1: > * use xswait mechanism to add a timeout > > As mentioned before, this is to fix a race condition that appears when > using libxl via libvirt and not using bootloader (console_available > fires too early). > > I have tested the patch on Qubes OS 4.1 (with Xen 4.13), and it seems > to solve the problem. I also checked the timeout: when xenconsoled is > stopped, libxl waits for 10 seconds and then aborts domain creation. > > tools/libxl/libxl_create.c | 43 +++++++++++++++++++++++++++++++++--- > tools/libxl/libxl_internal.h | 1 + > 2 files changed, 41 insertions(+), 3 deletions(-) > > diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c > index 3a7364e2ac..4b150d92b9 100644 > --- a/tools/libxl/libxl_create.c > +++ b/tools/libxl/libxl_create.c > @@ -1190,6 +1190,33 @@ static void domcreate_console_available(libxl__egc *egc, > dcs->aop_console_how.for_event)); > } > > +static void console_xswait_callback(libxl__egc *egc, libxl__xswait_state *xswa, > + int rc, const char *p) > +{ > + EGC_GC; > + libxl__domain_create_state *dcs = CONTAINER_OF(xswa, *dcs, console_xswait); > + char *dompath = libxl__xs_get_dompath(gc, dcs->guest_domid); > + char *tty_path = GCSPRINTF("%s/console/tty", dompath); > + char *tty; > + > + if (rc) { > + if (rc == ERROR_TIMEDOUT) > + LOG(ERROR, "%s: timed out", xswa->what); > + libxl__xswait_stop(gc, xswa); > + domcreate_complete(egc, dcs, rc); > + return; > + } > + > + tty = libxl__xs_read(gc, XBT_NULL, tty_path); > + > + if (tty && tty[0] != '\0') { > + libxl__xswait_stop(gc, xswa); > + > + domcreate_console_available(egc, dcs); > + domcreate_complete(egc, dcs, 0); > + } > +} > + > static void domcreate_bootloader_done(libxl__egc *egc, > libxl__bootloader_state *bl, > int rc) > @@ -1728,9 +1755,18 @@ static void domcreate_attach_devices(libxl__egc *egc, > return; > } > > - domcreate_console_available(egc, dcs); > - > - domcreate_complete(egc, dcs, 0); > + dcs->console_xswait.ao = ao; > + dcs->console_xswait.what = GCSPRINTF("domain %d console tty", domid); > + dcs->console_xswait.path = GCSPRINTF("%s/console/tty", > + libxl__xs_get_dompath(gc, domid)); > + dcs->console_xswait.timeout_ms = 10 * 1000; Better not use explicit value _here_, but a constant in some header. I think LIBXL_INIT_TIMEOUT is a good fit here. > + dcs->console_xswait.callback = console_xswait_callback; > + ret = libxl__xswait_start(gc, &dcs->console_xswait); > + if (ret) { > + LOG(ERROR, "unable to set up watch for domain %d console path", > + domid); > + goto error_out; > + } > > return; > > @@ -1861,6 +1897,7 @@ static int do_domain_create(libxl_ctx *ctx, libxl_domain_config *d_config, > > libxl__ao_progress_gethow(&cdcs->dcs.aop_console_how, aop_console_how); > cdcs->domid_out = domid; > + libxl__xswait_init(&cdcs->dcs.console_xswait); > > initiate_domain_create(egc, &cdcs->dcs); > > diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h > index 4936446069..d8129417dc 100644 > --- a/tools/libxl/libxl_internal.h > +++ b/tools/libxl/libxl_internal.h > @@ -4180,6 +4180,7 @@ struct libxl__domain_create_state { > /* necessary if the domain creation failed and we have to destroy it */ > libxl__domain_destroy_state dds; > libxl__multidev multidev; > + libxl__xswait_state console_xswait; > }; > > _hidden int libxl__device_nic_set_devids(libxl__gc *gc,
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c index 3a7364e2ac..4b150d92b9 100644 --- a/tools/libxl/libxl_create.c +++ b/tools/libxl/libxl_create.c @@ -1190,6 +1190,33 @@ static void domcreate_console_available(libxl__egc *egc, dcs->aop_console_how.for_event)); } +static void console_xswait_callback(libxl__egc *egc, libxl__xswait_state *xswa, + int rc, const char *p) +{ + EGC_GC; + libxl__domain_create_state *dcs = CONTAINER_OF(xswa, *dcs, console_xswait); + char *dompath = libxl__xs_get_dompath(gc, dcs->guest_domid); + char *tty_path = GCSPRINTF("%s/console/tty", dompath); + char *tty; + + if (rc) { + if (rc == ERROR_TIMEDOUT) + LOG(ERROR, "%s: timed out", xswa->what); + libxl__xswait_stop(gc, xswa); + domcreate_complete(egc, dcs, rc); + return; + } + + tty = libxl__xs_read(gc, XBT_NULL, tty_path); + + if (tty && tty[0] != '\0') { + libxl__xswait_stop(gc, xswa); + + domcreate_console_available(egc, dcs); + domcreate_complete(egc, dcs, 0); + } +} + static void domcreate_bootloader_done(libxl__egc *egc, libxl__bootloader_state *bl, int rc) @@ -1728,9 +1755,18 @@ static void domcreate_attach_devices(libxl__egc *egc, return; } - domcreate_console_available(egc, dcs); - - domcreate_complete(egc, dcs, 0); + dcs->console_xswait.ao = ao; + dcs->console_xswait.what = GCSPRINTF("domain %d console tty", domid); + dcs->console_xswait.path = GCSPRINTF("%s/console/tty", + libxl__xs_get_dompath(gc, domid)); + dcs->console_xswait.timeout_ms = 10 * 1000; + dcs->console_xswait.callback = console_xswait_callback; + ret = libxl__xswait_start(gc, &dcs->console_xswait); + if (ret) { + LOG(ERROR, "unable to set up watch for domain %d console path", + domid); + goto error_out; + } return; @@ -1861,6 +1897,7 @@ static int do_domain_create(libxl_ctx *ctx, libxl_domain_config *d_config, libxl__ao_progress_gethow(&cdcs->dcs.aop_console_how, aop_console_how); cdcs->domid_out = domid; + libxl__xswait_init(&cdcs->dcs.console_xswait); initiate_domain_create(egc, &cdcs->dcs); diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h index 4936446069..d8129417dc 100644 --- a/tools/libxl/libxl_internal.h +++ b/tools/libxl/libxl_internal.h @@ -4180,6 +4180,7 @@ struct libxl__domain_create_state { /* necessary if the domain creation failed and we have to destroy it */ libxl__domain_destroy_state dds; libxl__multidev multidev; + libxl__xswait_state console_xswait; }; _hidden int libxl__device_nic_set_devids(libxl__gc *gc,
If we skip the bootloader, the TTY path will be set for xenconsoled. However, there is no guarantee that this will happen by the time we want to call the console_available callback, so we have to wait. Signed-off-by: Paweł Marczewski <pawel@invisiblethingslab.com> --- Changed since v1: * use xswait mechanism to add a timeout As mentioned before, this is to fix a race condition that appears when using libxl via libvirt and not using bootloader (console_available fires too early). I have tested the patch on Qubes OS 4.1 (with Xen 4.13), and it seems to solve the problem. I also checked the timeout: when xenconsoled is stopped, libxl waits for 10 seconds and then aborts domain creation. tools/libxl/libxl_create.c | 43 +++++++++++++++++++++++++++++++++--- tools/libxl/libxl_internal.h | 1 + 2 files changed, 41 insertions(+), 3 deletions(-)