[Xen-devel] [PATCH 08/35] libxl: Replace libxl__qmp_initializations by ev_qmp calls

From: Anthony PERARD <anthony.perard@citrix.com>
To: <xen-devel@lists.xenproject.org>
Cc: Anthony PERARD <anthony.perard@citrix.com>,
	Ian Jackson <ian.jackson@eu.citrix.com>, Wei Liu <wl@xen.org>
Subject: [Xen-devel] [PATCH 08/35] libxl: Replace libxl__qmp_initializations by ev_qmp calls
Date: Fri, 2 Aug 2019 16:35:39 +0100	[thread overview]
Message-ID: <20190802153606.32061-9-anthony.perard@citrix.com> (raw)
In-Reply-To: <20190802153606.32061-1-anthony.perard@citrix.com>

Setup a timeout of 10s for all the commands. It used to be about 5s
per commands.

The order of command is changed, we call 'query-vnc' before
'change-vnc-password', but that should not matter. That makes it
easier to call 'change-vnc-password' conditionally.

Also 'change' command is replaced by 'change-vnc-password'
because 'change' is deprecated. The new command is available in all
QEMU versions that also have Xen support.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 tools/libxl/libxl_dm.c       | 211 ++++++++++++++++++++++++++++++++++-
 tools/libxl/libxl_internal.h |   8 +-
 tools/libxl/libxl_qmp.c      | 169 ----------------------------
 3 files changed, 212 insertions(+), 176 deletions(-)

diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 5a2e349776..bb5339784e 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -2063,11 +2063,13 @@ static int libxl__write_stub_dmargs(libxl__gc *gc,
 static void dmss_init(libxl__dm_spawn_state *dmss)
 {
     libxl__ev_qmp_init(&dmss->qmp);
+    libxl__ev_time_init(&dmss->timeout);
 }
 
 static void dmss_dispose(libxl__gc *gc, libxl__dm_spawn_state *dmss)
 {
     libxl__ev_qmp_dispose(gc, &dmss->qmp);
+    libxl__ev_time_deregister(gc, &dmss->timeout);
 }
 
 static void spawn_stubdom_pvqemu_cb(libxl__egc *egc,
@@ -2462,6 +2464,16 @@ static void device_model_qmp_cb(libxl__egc *egc, libxl__ev_qmp *ev,
 static void device_model_spawn_outcome(libxl__egc *egc,
                                        libxl__dm_spawn_state *dmss,
                                        int rc);
+static void device_model_postconfig_chardev(libxl__egc *egc,
+    libxl__ev_qmp *qmp, const libxl__json_object *response, int rc);
+static void device_model_postconfig_vnc(libxl__egc *egc,
+    libxl__ev_qmp *qmp, const libxl__json_object *response, int rc);
+static void device_model_postconfig_vnc_passwd(libxl__egc *egc,
+    libxl__ev_qmp *qmp, const libxl__json_object *response, int rc);
+static void devise_model_postconfig_timeout(libxl__egc *egc,
+    libxl__ev_time *ev, const struct timeval *requested_abs, int rc);
+static void device_model_postconfig_done(libxl__egc *egc,
+    libxl__dm_spawn_state *dmss, int rc);
 
 void libxl__spawn_local_dm(libxl__egc *egc, libxl__dm_spawn_state *dmss)
 {
@@ -2763,12 +2775,209 @@ static void device_model_spawn_outcome(libxl__egc *egc,
         }
     }
 
+    /* Check if spawn failed */
+    if (rc) goto out;
+
     if (d_config->b_info.device_model_version
             == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN) {
-        libxl__qmp_initializations(gc, dmss->guest_domid, d_config);
+        rc = libxl__ev_time_register_rel(ao, &dmss->timeout,
+                                         devise_model_postconfig_timeout,
+                                         LIBXL_QMP_CMD_TIMEOUT * 1000);
+        if (rc) goto out;
+        dmss->qmp.ao = ao;
+        dmss->qmp.domid = dmss->guest_domid;
+        dmss->qmp.payload_fd = -1;
+        dmss->qmp.callback = device_model_postconfig_chardev;
+        rc = libxl__ev_qmp_send(gc, &dmss->qmp, "query-chardev", NULL);
+        if (rc) goto out;
+        return;
     }
 
  out:
+    device_model_postconfig_done(egc, dmss, rc); /* must be last */
+}
+
+static void device_model_postconfig_chardev(libxl__egc *egc,
+    libxl__ev_qmp *qmp, const libxl__json_object *response, int rc)
+{
+    EGC_GC;
+    libxl__dm_spawn_state *dmss = CONTAINER_OF(qmp, *dmss, qmp);
+    const libxl__json_object *item = NULL;
+    const libxl__json_object *o = NULL;
+    int i = 0;
+    const char *label;
+    const char *filename;
+    int port;
+    char *endptr;
+    const char *dompath;
+    const char serial[] = "serial";
+    const size_t seriall = sizeof(serial) - 1;
+    const char pty[] = "pty:";
+    const size_t ptyl = sizeof(pty) - 1;
+
+    if (rc) goto out;
+
+    /*
+     * query-chardev response:
+     * [{ 'label': 'str',
+     *    'filename': 'str',
+     *    'frontend-open': 'bool' }, ...]
+     */
+
+    for (i = 0; (item = libxl__json_array_get(response, i)); i++) {
+        o = libxl__json_map_get("label", item, JSON_STRING);
+        if (!o) goto protocol_error;
+        label = libxl__json_object_get_string(o);
+
+        /* Check if the "label" start with "serial". */
+        if (!label || strncmp(label, serial, seriall))
+            continue;
+        port = strtol(label + seriall, &endptr, 10);
+        if (*(label + seriall) == '\0' || *endptr != '\0') {
+            LOGD(ERROR, qmp->domid,
+                 "Invalid serial port number: %s", label);
+            rc = ERROR_QEMU_API;
+            goto out;
+        }
+
+        o = libxl__json_map_get("filename", item, JSON_STRING);
+        if (!o) goto protocol_error;
+        filename = libxl__json_object_get_string(o);
+
+        /* Check if filename start with "pty:" */
+        if (!filename || strncmp(filename, pty, ptyl))
+            continue;
+
+        dompath = libxl__xs_get_dompath(gc, qmp->domid);
+        if (!dompath) {
+            rc = ERROR_FAIL;
+            goto out;
+        }
+        rc = libxl__xs_printf(gc, XBT_NULL,
+                              GCSPRINTF("%s/serial/%d/tty", dompath, port),
+                              "%s", filename + ptyl);
+        if (rc) goto out;
+    }
+
+    qmp->callback = device_model_postconfig_vnc;
+    rc = libxl__ev_qmp_send(gc, qmp, "query-vnc", NULL);
+    if (rc) goto out;
+    return;
+
+protocol_error:
+    rc = ERROR_QEMU_API;
+    LOGD(ERROR, qmp->domid,
+         "unexpected response to QMP cmd 'query-chardev', received:\n%s",
+         JSON(response));
+out:
+    device_model_postconfig_done(egc, dmss, rc); /* must be last */
+}
+
+static void device_model_postconfig_vnc(libxl__egc *egc,
+    libxl__ev_qmp *qmp, const libxl__json_object *response, int rc)
+{
+    EGC_GC;
+    libxl__dm_spawn_state *dmss = CONTAINER_OF(qmp, *dmss, qmp);
+    const libxl_vnc_info *vnc = libxl__dm_vnc(dmss->guest_config);
+    const libxl__json_object *o;
+    libxl__json_object *args = NULL;
+
+    if (rc) goto out;
+
+    /*
+     * query-vnc response:
+     * { 'enabled': 'bool', '*host': 'str', '*service': 'str' }
+     */
+
+    o = libxl__json_map_get("enabled", response, JSON_BOOL);
+    if (!o) goto protocol_error;
+    if (libxl__json_object_get_bool(o)) {
+        const char *addr, *port;
+        const char *dompath;
+
+        o = libxl__json_map_get("host", response, JSON_STRING);
+        if (!o) goto protocol_error;
+        addr = libxl__json_object_get_string(o);
+        o = libxl__json_map_get("service", response, JSON_STRING);
+        if (!o) goto protocol_error;
+        port = libxl__json_object_get_string(o);
+
+        dompath = libxl__xs_get_dompath(gc, qmp->domid);
+        if (!dompath) {
+            rc = ERROR_FAIL;
+            goto out;
+        }
+        rc = libxl__xs_printf(gc, XBT_NULL,
+                              GCSPRINTF("%s/console/vnc-listen", dompath),
+                              "%s", addr);
+        if (rc) goto out;
+        rc = libxl__xs_printf(gc, XBT_NULL,
+                              GCSPRINTF("%s/console/vnc-port", dompath),
+                              "%s", port);
+        if (rc) goto out;
+    }
+
+    if (vnc && vnc->passwd) {
+        qmp->callback = device_model_postconfig_vnc_passwd;
+        libxl__qmp_param_add_string(gc, &args, "password", vnc->passwd);
+        rc = libxl__ev_qmp_send(gc, qmp, "change-vnc-password", args);
+        if (rc) goto out;
+        return;
+    }
+
+    rc = 0;
+    goto out;
+
+protocol_error:
+    rc = ERROR_QEMU_API;
+    LOGD(ERROR, qmp->domid,
+         "unexpected response to QMP cmd 'query-vnc', received:\n%s",
+         JSON(response));
+out:
+    device_model_postconfig_done(egc, dmss, rc); /* must be last */
+}
+
+static void device_model_postconfig_vnc_passwd(libxl__egc *egc,
+    libxl__ev_qmp *qmp, const libxl__json_object *response, int rc)
+{
+    EGC_GC;
+    libxl__dm_spawn_state *dmss = CONTAINER_OF(qmp, *dmss, qmp);
+    const libxl_vnc_info *vnc = libxl__dm_vnc(dmss->guest_config);
+    const char *dompath;
+
+    if (rc) goto out;
+
+    dompath = libxl__xs_get_dompath(gc, qmp->domid);
+    if (!dompath) {
+        rc = ERROR_FAIL;
+        goto out;
+    }
+    rc = libxl__xs_printf(gc, XBT_NULL,
+                          GCSPRINTF("%s/console/vnc-pass", dompath),
+                          "%s", vnc->passwd);
+
+out:
+    device_model_postconfig_done(egc, dmss, rc); /* must be last */
+}
+
+void devise_model_postconfig_timeout(libxl__egc *egc, libxl__ev_time *ev,
+                                     const struct timeval *requested_abs,
+                                     int rc)
+{
+    libxl__dm_spawn_state *dmss = CONTAINER_OF(ev, *dmss, timeout);
+    device_model_postconfig_done(egc, dmss, rc); /* must be last */
+}
+
+
+static void device_model_postconfig_done(libxl__egc *egc,
+                                         libxl__dm_spawn_state *dmss,
+                                         int rc)
+{
+    EGC_GC;
+
+    if (rc)
+        LOGD(ERROR, dmss->guest_domid,
+             "Post DM startup configs failed, rc=%d", rc);
     dmss_dispose(gc, dmss);
     dmss->callback(egc, dmss, rc);
 }
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index effc1c5bf9..5e09e602ed 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -100,6 +100,7 @@
 #define LIBXL_DEVICE_MODEL_START_TIMEOUT 60
 #define LIBXL_DEVICE_MODEL_SAVE_FILE XEN_LIB_DIR "/qemu-save" /* .$domid */
 #define LIBXL_DEVICE_MODEL_RESTORE_FILE XEN_LIB_DIR "/qemu-resume" /* .$domid */
+#define LIBXL_QMP_CMD_TIMEOUT 10
 #define LIBXL_STUBDOM_START_TIMEOUT 30
 #define LIBXL_QEMU_BODGE_TIMEOUT 2
 #define LIBXL_XENCONSOLE_LIMIT 1048576
@@ -1946,8 +1947,6 @@ _hidden libxl__qmp_handler *libxl__qmp_initialize(libxl__gc *gc,
 _hidden int libxl__qmp_run_command_flexarray(libxl__gc *gc, int domid,
                                              const char *cmd,
                                              flexarray_t *array);
-/* ask to QEMU the serial port information and store it in xenstore. */
-_hidden int libxl__qmp_query_serial(libxl__qmp_handler *qmp);
 _hidden int libxl__qmp_pci_add(libxl__gc *gc, int d, libxl_device_pci *pcidev);
 _hidden int libxl__qmp_pci_del(libxl__gc *gc, int domid,
                                libxl_device_pci *pcidev);
@@ -1993,10 +1992,6 @@ _hidden void libxl__qmp_close(libxl__qmp_handler *qmp);
  * nothing happen */
 _hidden void libxl__qmp_cleanup(libxl__gc *gc, uint32_t domid);
 
-/* this helper calls qmp_initialize, query_serial and qmp_close */
-_hidden int libxl__qmp_initializations(libxl__gc *gc, uint32_t domid,
-                                       const libxl_domain_config *guest_config);
-
 /* `data' should contain a byte to send.
  * When dealing with a non-blocking fd, it returns
  *   ERROR_NOT_READY on EWOULDBLOCK
@@ -3953,6 +3948,7 @@ struct libxl__dm_spawn_state {
     /* mixed - spawn.ao must be initialised by user; rest is private: */
     libxl__spawn_state spawn;
     libxl__ev_qmp qmp;
+    libxl__ev_time timeout;
     /* filled in by user, must remain valid: */
     uint32_t guest_domid; /* domain being served */
     libxl_domain_config *guest_config;
diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
index f1529925ee..0d6aedcc7d 100644
--- a/tools/libxl/libxl_qmp.c
+++ b/tools/libxl/libxl_qmp.c
@@ -143,121 +143,6 @@ static const int QMP_SOCKET_CONNECT_TIMEOUT = 5;
  * QMP callbacks functions
  */
 
-static int store_serial_port_info(libxl__qmp_handler *qmp,
-                                  const char *chardev,
-                                  int port)
-{
-    GC_INIT(qmp->ctx);
-    char *path = NULL;
-    int ret = 0;
-
-    if (!(chardev && strncmp("pty:", chardev, 4) == 0)) {
-        return 0;
-    }
-
-    path = libxl__xs_get_dompath(gc, qmp->domid);
-    path = GCSPRINTF("%s/serial/%d/tty", path, port);
-
-    ret = libxl__xs_printf(gc, XBT_NULL, path, "%s", chardev + 4);
-
-    GC_FREE;
-    return ret;
-}
-
-static int register_serials_chardev_callback(libxl__qmp_handler *qmp,
-                                             const libxl__json_object *o,
-                                             void *unused)
-{
-    const libxl__json_object *obj = NULL;
-    const libxl__json_object *label = NULL;
-    const char *s = NULL;
-    int i = 0;
-    const char *chardev = NULL;
-    int ret = 0;
-
-    for (i = 0; (obj = libxl__json_array_get(o, i)); i++) {
-        if (!libxl__json_object_is_map(obj))
-            continue;
-        label = libxl__json_map_get("label", obj, JSON_STRING);
-        s = libxl__json_object_get_string(label);
-
-        if (s && strncmp("serial", s, strlen("serial")) == 0) {
-            const libxl__json_object *filename = NULL;
-            char *endptr = NULL;
-            int port_number;
-
-            filename = libxl__json_map_get("filename", obj, JSON_STRING);
-            chardev = libxl__json_object_get_string(filename);
-
-            s += strlen("serial");
-            port_number = strtol(s, &endptr, 10);
-            if (*s == 0 || *endptr != 0) {
-                LIBXL__LOGD(qmp->ctx, LIBXL__LOG_ERROR, qmp->domid,
-                            "Invalid serial port number: %s", s);
-                return -1;
-            }
-            ret = store_serial_port_info(qmp, chardev, port_number);
-            if (ret) {
-                LIBXL__LOGD_ERRNO(qmp->ctx, LIBXL__LOG_ERROR, qmp->domid,
-                                  "Failed to store serial port information"
-                                  " in xenstore");
-                return ret;
-            }
-        }
-    };
-
-    return ret;
-}
-
-static int qmp_write_domain_console_item(libxl__gc *gc, int domid,
-                                         const char *item, const char *value)
-{
-    char *path;
-
-    path = libxl__xs_get_dompath(gc, domid);
-    path = GCSPRINTF("%s/console/%s", path, item);
-
-    return libxl__xs_printf(gc, XBT_NULL, path, "%s", value);
-}
-
-static int qmp_register_vnc_callback(libxl__qmp_handler *qmp,
-                                     const libxl__json_object *o,
-                                     void *unused)
-{
-    GC_INIT(qmp->ctx);
-    const libxl__json_object *obj;
-    const char *addr, *port;
-    int rc = -1;
-
-    if (!libxl__json_object_is_map(o)) {
-        goto out;
-    }
-
-    obj = libxl__json_map_get("enabled", o, JSON_BOOL);
-    if (!obj || !libxl__json_object_get_bool(obj)) {
-        rc = 0;
-        goto out;
-    }
-
-    obj = libxl__json_map_get("host", o, JSON_STRING);
-    addr = libxl__json_object_get_string(obj);
-    obj = libxl__json_map_get("service", o, JSON_STRING);
-    port = libxl__json_object_get_string(obj);
-
-    if (!addr || !port) {
-        LOGD(ERROR, qmp->domid, "Failed to retrieve VNC connect information.");
-        goto out;
-    }
-
-    rc = qmp_write_domain_console_item(gc, qmp->domid, "vnc-listen", addr);
-    if (!rc)
-        rc = qmp_write_domain_console_item(gc, qmp->domid, "vnc-port", port);
-
-out:
-    GC_FREE;
-    return rc;
-}
-
 static int qmp_capabilities_callback(libxl__qmp_handler *qmp,
                                      const libxl__json_object *o, void *unused)
 {
@@ -851,20 +736,6 @@ void libxl__qmp_cleanup(libxl__gc *gc, uint32_t domid)
     }
 }
 
-int libxl__qmp_query_serial(libxl__qmp_handler *qmp)
-{
-    return qmp_synchronous_send(qmp, "query-chardev", NULL,
-                                register_serials_chardev_callback,
-                                NULL, qmp->timeout);
-}
-
-static int qmp_query_vnc(libxl__qmp_handler *qmp)
-{
-    return qmp_synchronous_send(qmp, "query-vnc", NULL,
-                                qmp_register_vnc_callback,
-                                NULL, qmp->timeout);
-}
-
 static int pci_add_callback(libxl__qmp_handler *qmp,
                             const libxl__json_object *response, void *opaque)
 {
@@ -1085,24 +956,6 @@ int libxl__qmp_restore(libxl__gc *gc, int domid, const char *state_file)
                            NULL, NULL);
 }
 
-static int qmp_change(libxl__gc *gc, libxl__qmp_handler *qmp,
-                      char *device, char *target, char *arg)
-{
-    libxl__json_object *args = NULL;
-    int rc = 0;
-
-    libxl__qmp_param_add_string(gc, &args, "device", device);
-    libxl__qmp_param_add_string(gc, &args, "target", target);
-    if (arg) {
-        libxl__qmp_param_add_string(gc, &args, "arg", arg);
-    }
-
-    rc = qmp_synchronous_send(qmp, "change", args,
-                              NULL, NULL, qmp->timeout);
-
-    return rc;
-}
-
 int libxl__qmp_resume(libxl__gc *gc, int domid)
 {
     return qmp_run_command(gc, domid, "cont", NULL, NULL, NULL);
@@ -1293,28 +1146,6 @@ int libxl_qemu_monitor_command(libxl_ctx *ctx, uint32_t domid,
     return AO_INPROGRESS;
 }
 
-int libxl__qmp_initializations(libxl__gc *gc, uint32_t domid,
-                               const libxl_domain_config *guest_config)
-{
-    const libxl_vnc_info *vnc = libxl__dm_vnc(guest_config);
-    libxl__qmp_handler *qmp = NULL;
-    int ret = 0;
-
-    qmp = libxl__qmp_initialize(gc, domid);
-    if (!qmp)
-        return -1;
-    ret = libxl__qmp_query_serial(qmp);
-    if (!ret && vnc && vnc->passwd) {
-        ret = qmp_change(gc, qmp, "vnc", "password", vnc->passwd);
-        qmp_write_domain_console_item(gc, domid, "vnc-pass", vnc->passwd);
-    }
-    if (!ret) {
-        ret = qmp_query_vnc(qmp);
-    }
-    libxl__qmp_close(qmp);
-    return ret;
-}
-
 
 /*
  * Functions using libxl__ev_qmp
-- 
Anthony PERARD


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
