tools/libxl/libxl_create.c | 43 +++++++++++++++++++++++++++++++++--- tools/libxl/libxl_internal.h | 1 + 2 files changed, 41 insertions(+), 3 deletions(-)
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(-)
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,
--
2.21.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-develOn 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,
--
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
© 2016 - 2024 Red Hat, Inc.