[XEN,v4] libxl: wait for console path before firing console_available

Commit Message

Paweł Marczewski March 3, 2020, 1:28 p.m. UTC

If the path doesn't become available after LIBXL_INIT_TIMEOUT
seconds, fail the domain creation.

If we skip the bootloader, the TTY path will be set by 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 <[e-mail]>
Reviewed-by: Marek Marczykowski-Górecki <[e-mail]>
---
Changed since v3 (after review by Anthony Perard):
  * use libxl__console_tty_path
  * move console_xswait_callback to proper place in file
  * refactor console_xswait_callback so that it has a single
    "out" section
  * move xswait_init to proper function (initiate_domain_create)
  * add xswait_stop to cleanup (domcreate_complete)
  * mention in commit message that we fail after a timeout
Changed since v2 (after review by Marek Marczykowski-Górecki):
  * replace hardcoded value with LIBXL_INIT_TIMEOUT
Changed since v1:
  * use xswait mechanism to add a timeout

 tools/libxl/libxl_console.c  |  4 +--
 tools/libxl/libxl_create.c   | 50 ++++++++++++++++++++++++++++++++++--
 tools/libxl/libxl_internal.h |  3 +++
 3 files changed, 53 insertions(+), 4 deletions(-)

Comments

Anthony PERARD March 3, 2020, 3:51 p.m. UTC | #1

On Tue, Mar 03, 2020 at 02:28:20PM +0100, Paweł Marczewski wrote:
> If the path doesn't become available after LIBXL_INIT_TIMEOUT
> seconds, fail the domain creation.
> 
> If we skip the bootloader, the TTY path will be set by 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 <[e-mail]>
> Reviewed-by: Marek Marczykowski-Górecki <[e-mail]>

Reviewed-by: Anthony PERARD <[e-mail]>

Thanks!

Wei Liu March 4, 2020, 10:27 a.m. UTC | #2

On Tue, Mar 03, 2020 at 03:51:04PM +0000, Anthony PERARD wrote:
> On Tue, Mar 03, 2020 at 02:28:20PM +0100, Paweł Marczewski wrote:
> > If the path doesn't become available after LIBXL_INIT_TIMEOUT
> > seconds, fail the domain creation.
> > 
> > If we skip the bootloader, the TTY path will be set by 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 <[e-mail]>
> > Reviewed-by: Marek Marczykowski-Górecki <[e-mail]>
> 
> Reviewed-by: Anthony PERARD <[e-mail]>

Applied. Thanks.

Wei.

Patch

diff --git a/tools/libxl/libxl_console.c b/tools/libxl/libxl_console.c
index 088a455b52..047d23d7ae 100644
--- a/tools/libxl/libxl_console.c
+++ b/tools/libxl/libxl_console.c
@@ -16,8 +16,8 @@ 
 
 #include "libxl_internal.h"
 
-static int libxl__console_tty_path(libxl__gc *gc, uint32_t domid, int cons_num,
-                                   libxl_console_type type, char **tty_path)
+int libxl__console_tty_path(libxl__gc *gc, uint32_t domid, int cons_num,
+                            libxl_console_type type, char **tty_path)
 {
     int rc;
     char *dom_path;
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 3a7364e2ac..ada942bc8d 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -846,6 +846,8 @@  static void domcreate_devmodel_started(libxl__egc *egc,
 static void domcreate_attach_devices(libxl__egc *egc,
                                      libxl__multidev *multidev,
                                      int ret);
+static void console_xswait_callback(libxl__egc *egc, libxl__xswait_state *xswa,
+                                    int rc, const char *p);
 
 /* Our own function to clean up and call the user's callback.
  * The final call in the sequence. */
@@ -1151,6 +1153,8 @@  static void initiate_domain_create(libxl__egc *egc,
     if (ret)
         goto error_out;
 
+    libxl__xswait_init(&dcs->console_xswait);
+
     if (restore_fd >= 0 || dcs->soft_reset) {
         LOGD(DEBUG, domid, "restoring, not running bootloader");
         domcreate_bootloader_done(egc, &dcs->bl, 0);
@@ -1705,6 +1709,7 @@  static void domcreate_attach_devices(libxl__egc *egc,
     int domid = dcs->guest_domid;
     libxl_domain_config *const d_config = dcs->guest_config;
     const libxl__device_type *dt;
+    char *tty_path;
 
     if (ret) {
         LOGD(ERROR, domid, "unable to add %s devices",
@@ -1728,9 +1733,24 @@  static void domcreate_attach_devices(libxl__egc *egc,
         return;
     }
 
-    domcreate_console_available(egc, dcs);
+    ret = libxl__console_tty_path(gc, domid, 0, LIBXL_CONSOLE_TYPE_PV, &tty_path);
+    if (ret) {
+        LOG(ERROR, "failed to get domain %d console tty path",
+            domid);
+        goto error_out;
+    }
 
-    domcreate_complete(egc, dcs, 0);
+    dcs->console_xswait.ao = ao;
+    dcs->console_xswait.what = GCSPRINTF("domain %d console tty", domid);
+    dcs->console_xswait.path = tty_path;
+    dcs->console_xswait.timeout_ms = LIBXL_INIT_TIMEOUT * 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 tty path",
+            domid);
+        goto error_out;
+    }
 
     return;
 
@@ -1739,6 +1759,30 @@  error_out:
     domcreate_complete(egc, dcs, ret);
 }
 
+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);
+
+    if (rc) {
+        if (rc == ERROR_TIMEDOUT)
+            LOG(ERROR, "%s: timed out", xswa->what);
+        goto out;
+    }
+
+    if (p && p[0] != '\0') {
+        domcreate_console_available(egc, dcs);
+        goto out;
+    }
+
+    return;
+
+out:
+    libxl__xswait_stop(gc, xswa);
+    domcreate_complete(egc, dcs, rc);
+}
+
 static void domcreate_complete(libxl__egc *egc,
                                libxl__domain_create_state *dcs,
                                int rc)
@@ -1747,6 +1791,8 @@  static void domcreate_complete(libxl__egc *egc,
     libxl_domain_config *const d_config = dcs->guest_config;
     libxl_domain_config *d_config_saved = &dcs->guest_config_saved;
 
+    libxl__xswait_stop(gc, &dcs->console_xswait);
+
     libxl__domain_build_state_dispose(&dcs->build_state);
 
     if (!rc && d_config->b_info.exec_ssidref)
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 4936446069..19d11cc6d6 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1517,6 +1517,8 @@  _hidden char *libxl__domain_device_libxl_path(libxl__gc *gc, uint32_t domid, uin
                                               libxl__device_kind device_kind);
 _hidden int libxl__parse_backend_path(libxl__gc *gc, const char *path,
                                       libxl__device *dev);
+_hidden int libxl__console_tty_path(libxl__gc *gc, uint32_t domid, int cons_num,
+                                    libxl_console_type type, char **tty_path);
 _hidden int libxl__device_destroy(libxl__gc *gc, libxl__device *dev);
 _hidden int libxl__wait_for_backend(libxl__gc *gc, const char *be_path,
                                     const char *state);
@@ -4180,6 +4182,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,