[PATCH] libxl: wait for QEMU exit before restarting a domain

[Jiaqing Zhao <Zhao.Jiaqing@xxxxxxx>]

The PID-based device model destruction path returns immediately after
sending SIGHUP, before QEMU has necessarily exited. When an HVM guest
reboots with VNC enabled, the new QEMU process may fail to bind the VNC
port because the old process still holds it.

Wait for QEMU to exit in a child process after singaling it for at most
60 seconds. If waiting fails or times out, log the error and retain the
existing domain destruction behaviour.

Signed-off-by: Jiaqing Zhao <Zhao.Jiaqing@xxxxxxx>
---
Cc: Anthony PERARD <anthony.perard@xxxxxxxxxx>
Cc: Juergen Gross <jgross@xxxxxxxx>
Cc: Roger Pau Monné <roger@xxxxxxxxxxxxxx>
Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>
Cc: Penny Zheng <penny.zheng@xxxxxxx>
---
 tools/libs/light/libxl_aoutils.c  |  8 ++++--
 tools/libs/light/libxl_dm.c       | 45 ++++++++++++++++++++++++++-----
 tools/libs/light/libxl_domain.c   |  2 +-
 tools/libs/light/libxl_internal.h |  3 ++-
 4 files changed, 48 insertions(+), 10 deletions(-)

diff --git a/tools/libs/light/libxl_aoutils.c b/tools/libs/light/libxl_aoutils.c
index c2d42e7cac..977a2705a8 100644
--- a/tools/libs/light/libxl_aoutils.c
+++ b/tools/libs/light/libxl_aoutils.c
@@ -628,10 +628,11 @@ void libxl__kill(libxl__gc *gc, pid_t pid, int sig, const 
char *what)
 
 /* Generic function to signal (HUP) a pid stored in xenstore */
 int libxl__kill_xs_path(libxl__gc *gc, const char *xs_path_pid,
-                        const char *what)
+                        const char *what, pid_t *pid_r)
 {
     const char *xs_pid;
-    int ret, pid;
+    int ret;
+    pid_t pid;
 
     ret = libxl__xs_read_checked(gc, XBT_NULL, xs_path_pid, &xs_pid);
     if (ret || !xs_pid) {
@@ -640,6 +641,9 @@ int libxl__kill_xs_path(libxl__gc *gc, const char 
*xs_path_pid,
         goto out;
     }
     pid = atoi(xs_pid);
+    if (pid_r) {
+        *pid_r = pid;
+    }
 
     ret = kill(pid, SIGHUP);
     if (ret < 0 && errno == ESRCH) {
diff --git a/tools/libs/light/libxl_dm.c b/tools/libs/light/libxl_dm.c
index 511ec76a65..ac1db9fe6a 100644
--- a/tools/libs/light/libxl_dm.c
+++ b/tools/libs/light/libxl_dm.c
@@ -2859,7 +2859,7 @@ static void device_model_probe_spawn_outcome(libxl__egc 
*egc,
 
     /* Ensure our QEMU command line probe is killed. */
     rc = libxl__kill_xs_path(gc, dmss->spawn.pidpath,
-                             "qemu command-line probe");
+                             "qemu command-line probe", NULL);
     if (rc) {
         LOGD(WARN, dmss->guest_domid,
              "Killing qemu command-line probe pid from path %s",
@@ -3420,9 +3420,10 @@ out:
 }
 
 /* Generic function to signal a Qemu instance to exit */
-static int kill_device_model(libxl__gc *gc, const char *xs_path_pid)
+static int kill_device_model(libxl__gc *gc, const char *xs_path_pid,
+                             pid_t *pid_r)
 {
-    return libxl__kill_xs_path(gc, xs_path_pid, "Device Model");
+    return libxl__kill_xs_path(gc, xs_path_pid, "Device Model", pid_r);
 }
 
 /* Helper to destroy a Qdisk backend */
@@ -3433,7 +3434,7 @@ int libxl__destroy_qemu_xenpv_backend(libxl__gc *gc, 
uint32_t domid)
 
     pid_path = GCSPRINTF("libxl/%u/qdisk-backend-pid", domid);
 
-    rc = kill_device_model(gc, pid_path);
+    rc = kill_device_model(gc, pid_path, NULL);
     if (rc)
         goto out;
 
@@ -3454,6 +3455,21 @@ static void kill_device_model_uid_cb(libxl__egc *egc,
                                      libxl__ev_child *destroyer,
                                      pid_t pid, int status);
 
+/* Run in a child so waiting for QEMU does not block the libxl event loop. */
+static int wait_device_model_pid_child(libxl__gc *gc, uint32_t domid, pid_t 
pid)
+{
+    unsigned int i;
+
+    for (i = 0; i < LIBXL_DEVICE_MODEL_STOP_TIMEOUT * 10; i++) {
+        if (kill(pid, 0) && errno == ESRCH)
+            return 0;
+        usleep(100000);
+    }
+
+    LOGD(ERROR, domid, "Timed out waiting for device model [%d] to exit", pid);
+    return ERROR_TIMEDOUT;
+}
+
 /*
  * If we have a uid, we shouldn't kill by pid.  This is because a
  * hostile QEMU might have exited, in which case the pid we have may
@@ -3477,6 +3493,7 @@ void libxl__destroy_device_model(libxl__egc *egc,
     int domid = ddms->domid;
     char *path;
     const char *dm_kill_uid_str = NULL;
+    pid_t dm_pid;
     int reaper_pid;
 
     ddms->rc = 0;
@@ -3547,12 +3564,28 @@ void libxl__destroy_device_model(libxl__egc *egc,
     LOGD(DEBUG, domid, "Didn't find dm UID; destroying by pid");
 
     path = GCSPRINTF("/local/domain/%d/image/device-model-pid", domid);
-    rc = kill_device_model(gc, path);
-
+    rc = kill_device_model(gc, path, &dm_pid);
     if (rc) {
         ACCUMULATE_RC(ddms->rc);
         LOGD(ERROR, domid, "Killing device model pid from path %s", path);
+        goto out;
+    }
+
+    LOGD(DEBUG, domid, "Waiting for device model [%d] to exit", dm_pid);
+    reaper_pid = libxl__ev_child_fork(gc, &ddms->destroyer,
+                                      kill_device_model_uid_cb);
+    if (reaper_pid < 0) {
+        rc = ERROR_FAIL;
+        ACCUMULATE_RC(ddms->rc);
+        LOGD(ERROR, domid, "Unable to fork device model exit waiter");
+        goto out;
     }
+    if (!reaper_pid) {
+        rc = wait_device_model_pid_child(gc, domid, dm_pid);
+        assert(rc <= 0 && rc >= -125);
+        _exit(-rc);
+    }
+    return;
 
 out:
     /*
diff --git a/tools/libs/light/libxl_domain.c b/tools/libs/light/libxl_domain.c
index 37fcd92871..6e332cfed7 100644
--- a/tools/libs/light/libxl_domain.c
+++ b/tools/libs/light/libxl_domain.c
@@ -1270,7 +1270,7 @@ static void dm_destroy_cb(libxl__egc *egc,
         libxl__stubdomain_is_linux_running(gc, target_domid)) {
         char *path = GCSPRINTF("/local/domain/%d/image/qmp-proxy-pid", domid);
 
-        libxl__kill_xs_path(gc, path, "QMP Proxy");
+        libxl__kill_xs_path(gc, path, "QMP Proxy", NULL);
         /* qmp-proxy for stubdom registers target_domid's QMP sockets. */
         libxl__qmp_cleanup(gc, target_domid);
     }
diff --git a/tools/libs/light/libxl_internal.h 
b/tools/libs/light/libxl_internal.h
index b65e0064b9..f4fde8009e 100644
--- a/tools/libs/light/libxl_internal.h
+++ b/tools/libs/light/libxl_internal.h
@@ -97,6 +97,7 @@
 /* QEMU may be slow to load and start due to a bug in Linux where the I/O
  * subsystem sometime produce high latency under load. */
 #define LIBXL_DEVICE_MODEL_START_TIMEOUT 60
+#define LIBXL_DEVICE_MODEL_STOP_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
@@ -2659,7 +2660,7 @@ bool libxl__async_exec_inuse(const 
libxl__async_exec_state *aes);
 _hidden void libxl__kill(libxl__gc *gc, pid_t pid, int sig, const char *what);
 /* kill SIGHUP a pid stored in xenstore */
 _hidden int libxl__kill_xs_path(libxl__gc *gc, const char *xs_path_pid,
-                                const char *what);
+                                const char *what, pid_t *pid_r);
 
 /*----- device addition/removal -----*/
 
-- 
2.53.0