[Xen-devel] [XEN PATCH v2] libxl: wait for console path before firing console_available

* [Xen-devel] [XEN PATCH v2] libxl: wait for console path before firing console_available
@ 2020-02-20 13:31 Paweł Marczewski
  2020-02-28 17:30 ` Marek Marczykowski-Górecki
  0 siblings, 1 reply; 2+ messages in thread
From: Paweł Marczewski @ 2020-02-20 13:31 UTC (permalink / raw)
  To: xen-devel
  Cc: Anthony PERARD, Paweł Marczewski, Ian Jackson,
	Marek Marczykowski-Górecki, Wei Liu

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-devel

^ permalink raw reply related	[flat|nested] 2+ messages in thread