qemu: don't retry connect() if doing FD passing

Since libvirt called bind() and listen() on the UNIX socket, it is
guaranteed that connect() will immediately succeed, if QEMU is running
normally. It will only fail if QEMU has closed the monitor socket by
mistake or if QEMU has exited, letting the kernel close it.

With this in mind we can remove the retry loop and timeout when
connecting to the QEMU monitor if we are doing FD passing. Libvirt can
go straight to sending the QMP greeting and will simply block waiting
for a reply until QEMU is ready.

Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index a0ad7f03365a5c26f8fbad05e2825eb8d938b08d..f65681bc5e5552d2f16f9a63aa65b9740e27005d 100644 (file)
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -4260,7 +4260,7 @@ virQEMUCapsInitQMPCommandRun(virQEMUCapsInitQMPCommandPtr cmd,
 
     cmd->vm->pid = cmd->pid;
 
-    if (!(cmd->mon = qemuMonitorOpen(cmd->vm, &cmd->config, true,
+    if (!(cmd->mon = qemuMonitorOpen(cmd->vm, &cmd->config, true, true,
                                      0, &callbacks, NULL)))
         goto ignore;
 

diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index b0c63c68d378ec6d6efdabe22f06831a880a6c4c..69aea6a850bb5da30ec0edf43a525cfc2ee72208 100644 (file)
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -334,6 +334,7 @@ qemuMonitorDispose(void *obj)
 static int
 qemuMonitorOpenUnix(const char *monitor,
                     pid_t cpid,
+                    bool retry,
                     unsigned long long timeout)
 {
     struct sockaddr_un addr;
@@ -355,31 +356,39 @@ qemuMonitorOpenUnix(const char *monitor,
         goto error;
     }
 
-    if (virTimeBackOffStart(&timebackoff, 1, timeout * 1000) < 0)
-        goto error;
-    while (virTimeBackOffWait(&timebackoff)) {
-        ret = connect(monfd, (struct sockaddr *)&addr, sizeof(addr));
-
-        if (ret == 0)
-            break;
+    if (retry) {
+        if (virTimeBackOffStart(&timebackoff, 1, timeout * 1000) < 0)
+            goto error;
+        while (virTimeBackOffWait(&timebackoff)) {
+            ret = connect(monfd, (struct sockaddr *)&addr, sizeof(addr));
 
-        if ((errno == ENOENT || errno == ECONNREFUSED) &&
-            (!cpid || virProcessKill(cpid, 0) == 0)) {
-            /* ENOENT       : Socket may not have shown up yet
-             * ECONNREFUSED : Leftover socket hasn't been removed yet */
-            continue;
-        }
+            if (ret == 0)
+                break;
 
-        virReportSystemError(errno, "%s",
-                             _("failed to connect to monitor socket"));
-        goto error;
+            if ((errno == ENOENT || errno == ECONNREFUSED) &&
+                (!cpid || virProcessKill(cpid, 0) == 0)) {
+                /* ENOENT       : Socket may not have shown up yet
+                 * ECONNREFUSED : Leftover socket hasn't been removed yet */
+                continue;
+            }
 
-    }
+            virReportSystemError(errno, "%s",
+                                 _("failed to connect to monitor socket"));
+            goto error;
+        }
 
-    if (ret != 0) {
-        virReportSystemError(errno, "%s",
-                             _("monitor socket did not show up"));
-        goto error;
+        if (ret != 0) {
+            virReportSystemError(errno, "%s",
+                                 _("monitor socket did not show up"));
+            goto error;
+        }
+    } else {
+        ret = connect(monfd, (struct sockaddr *) &addr, sizeof(addr));
+        if (ret < 0) {
+            virReportSystemError(errno, "%s",
+                                 _("failed to connect to monitor socket"));
+            goto error;
+        }
     }
 
     return monfd;
@@ -893,6 +902,7 @@ qemuMonitorPtr
 qemuMonitorOpen(virDomainObjPtr vm,
                 virDomainChrSourceDefPtr config,
                 bool json,
+                bool retry,
                 unsigned long long timeout,
                 qemuMonitorCallbacksPtr cb,
                 void *opaque)
@@ -907,7 +917,7 @@ qemuMonitorOpen(virDomainObjPtr vm,
     case VIR_DOMAIN_CHR_TYPE_UNIX:
         hasSendFD = true;
         if ((fd = qemuMonitorOpenUnix(config->data.nix.path,
-                                      vm->pid, timeout)) < 0)
+                                      vm->pid, retry, timeout)) < 0)
             return NULL;
         break;
 

diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 9894eba4d0683e99d1a27a9168c4ca6b4d76a757..43843721bfd69d25b95d21da69d3777d50e4a110 100644 (file)
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -313,6 +313,7 @@ char *qemuMonitorUnescapeArg(const char *in);
 qemuMonitorPtr qemuMonitorOpen(virDomainObjPtr vm,
                                virDomainChrSourceDefPtr config,
                                bool json,
+                               bool retry,
                                unsigned long long timeout,
                                qemuMonitorCallbacksPtr cb,
                                void *opaque)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 3e97971543c9b41efd3472e5592aa6cadc54fc50..d53d6b04fa226de8f0d960d2850fe9fc1216ae3f 100644 (file)
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -1679,7 +1679,7 @@ qemuProcessInitMonitor(virQEMUDriverPtr driver,
 
 static int
 qemuConnectMonitor(virQEMUDriverPtr driver, virDomainObjPtr vm, int asyncJob,
-                   qemuDomainLogContextPtr logCtxt)
+                   bool retry, qemuDomainLogContextPtr logCtxt)
 {
     qemuDomainObjPrivatePtr priv = vm->privateData;
     qemuMonitorPtr mon = NULL;
@@ -1710,6 +1710,7 @@ qemuConnectMonitor(virQEMUDriverPtr driver, virDomainObjPtr vm, int asyncJob,
     mon = qemuMonitorOpen(vm,
                           monConfig,
                           priv->monJSON,
+                          retry,
                           timeout,
                           &monitorCallbacks,
                           driver);
@@ -2087,17 +2088,23 @@ qemuProcessWaitForMonitor(virQEMUDriverPtr driver,
 {
     int ret = -1;
     virHashTablePtr info = NULL;
-    qemuDomainObjPrivatePtr priv;
+    qemuDomainObjPrivatePtr priv = vm->privateData;
+    bool retry = true;
+
+    if (priv->qemuCaps &&
+        virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_CHARDEV_FD_PASS))
+        retry = false;
 
-    VIR_DEBUG("Connect monitor to %p '%s'", vm, vm->def->name);
-    if (qemuConnectMonitor(driver, vm, asyncJob, logCtxt) < 0)
+    VIR_DEBUG("Connect monitor to vm=%p name='%s' retry=%d",
+              vm, vm->def->name, retry);
+
+    if (qemuConnectMonitor(driver, vm, asyncJob, retry, logCtxt) < 0)
         goto cleanup;
 
     /* Try to get the pty path mappings again via the monitor. This is much more
      * reliable if it's available.
      * Note that the monitor itself can be on a pty, so we still need to try the
      * log output method. */
-    priv = vm->privateData;
     if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
         goto cleanup;
     ret = qemuMonitorGetChardevInfo(priv->mon, &info);
@@ -7416,6 +7423,7 @@ qemuProcessReconnect(void *opaque)
     unsigned int stopFlags = 0;
     bool jobStarted = false;
     virCapsPtr caps = NULL;
+    bool retry = true;
 
     VIR_FREE(data);
 
@@ -7446,10 +7454,15 @@ qemuProcessReconnect(void *opaque)
      * allowReboot in status XML and we need to initialize it. */
     qemuProcessPrepareAllowReboot(obj);
 
-    VIR_DEBUG("Reconnect monitor to %p '%s'", obj, obj->def->name);
+    if (priv->qemuCaps &&
+        virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_CHARDEV_FD_PASS))
+        retry = false;
+
+    VIR_DEBUG("Reconnect monitor to def=%p name='%s' retry=%d",
+              obj, obj->def->name, retry);
 
     /* XXX check PID liveliness & EXE path */
-    if (qemuConnectMonitor(driver, obj, QEMU_ASYNC_JOB_NONE, NULL) < 0)
+    if (qemuConnectMonitor(driver, obj, QEMU_ASYNC_JOB_NONE, retry, NULL) < 0)
         goto error;
 
     if (qemuHostdevUpdateActiveDomainDevices(driver, obj->def) < 0)

diff --git a/tests/qemumonitortestutils.c b/tests/qemumonitortestutils.c
index 62f68ee699921233b849619b6945fcaa6d505d50..789eb721969264e652a96e578e424515d43bff5f 100644 (file)
--- a/tests/qemumonitortestutils.c
+++ b/tests/qemumonitortestutils.c
@@ -1252,6 +1252,7 @@ qemuMonitorTestNew(bool json,
     if (!(test->mon = qemuMonitorOpen(test->vm,
                                       &src,
                                       json,
+                                      true,
                                       0,
                                       &qemuMonitorTestCallbacks,
                                       driver)))