qemu: use -incoming fd:n to avoid qemu holding fd indefinitely

https://bugzilla.redhat.com/show_bug.cgi?id=620363

When using -incoming stdio or -incoming exec:, qemu keeps the
stdin fd open long after the migration is complete.  Not to
mention that exec:cat is horribly inefficient, by doubling the
I/O and going through a popen interface in qemu.

The new -incoming fd: of qemu 0.12.0 closes the fd after using
it, and allows us to bypass an intermediary cat process for
less I/O.

* src/qemu/qemu_command.h (qemuBuildCommandLine): Add parameter.
* src/qemu/qemu_command.c (qemuBuildCommandLine): Support
migration via fd: when possible.  Consolidate migration handling
into one spot, now that it is more complex.
* src/qemu/qemu_driver.c (qemudStartVMDaemon): Update caller.
* tests/qemuxml2argvtest.c (mymain): Likewise.
* tests/qemuxml2argvdata/qemuxml2argv-restore-v2-fd.args: New file.
* tests/qemuxml2argvdata/qemuxml2argv-restore-v2-fd.xml: Likewise.

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index d36a30a7c2f5b9b969eb8b002fcdd909d9ec38e3..24acc10894e16050d536b98c36e09acdd20513ca 100644 (file)
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2503,6 +2503,7 @@ qemuBuildCommandLine(virConnectPtr conn,
                      bool monitor_json,
                      unsigned long long qemuCmdFlags,
                      const char *migrateFrom,
+                     int migrateFd,
                      virDomainSnapshotObjPtr current_snapshot,
                      enum virVMOperationType vmop)
 {
@@ -2530,33 +2531,6 @@ qemuBuildCommandLine(virConnectPtr conn,
 
     virUUIDFormat(def->uuid, uuid);
 
-    /* Migration is very annoying due to wildly varying syntax & capabilities
-     * over time of KVM / QEMU codebases
-     */
-    if (migrateFrom) {
-        if (STRPREFIX(migrateFrom, "tcp")) {
-            if (!(qemuCmdFlags & QEMUD_CMD_FLAG_MIGRATE_QEMU_TCP)) {
-                qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-                                "%s", _("TCP migration is not supported with this QEMU binary"));
-                return NULL;
-            }
-        } else if (STREQ(migrateFrom, "stdio")) {
-            if (qemuCmdFlags & QEMUD_CMD_FLAG_MIGRATE_QEMU_EXEC) {
-                migrateFrom = "exec:cat";
-            } else if (!(qemuCmdFlags & QEMUD_CMD_FLAG_MIGRATE_KVM_STDIO)) {
-                qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-                                "%s", _("STDIO migration is not supported with this QEMU binary"));
-                return NULL;
-            }
-        } else if (STRPREFIX(migrateFrom, "exec")) {
-            if (!(qemuCmdFlags & QEMUD_CMD_FLAG_MIGRATE_QEMU_EXEC)) {
-                qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-                                "%s", _("STDIO migration is not supported with this QEMU binary"));
-                return NULL;
-            }
-        }
-    }
-
     emulator = def->emulator;
 
     /*
@@ -3956,8 +3930,58 @@ qemuBuildCommandLine(virConnectPtr conn,
         }
     }
 
-    if (migrateFrom)
-        virCommandAddArgList(cmd, "-incoming", migrateFrom, NULL);
+    /* Migration is very annoying due to wildly varying syntax &
+     * capabilities over time of KVM / QEMU codebases.
+     */
+    if (migrateFrom) {
+        virCommandAddArg(cmd, "-incoming");
+        if (STRPREFIX(migrateFrom, "tcp")) {
+            if (!(qemuCmdFlags & QEMUD_CMD_FLAG_MIGRATE_QEMU_TCP)) {
+                qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                                "%s", _("TCP migration is not supported with "
+                                        "this QEMU binary"));
+                goto error;
+            }
+            virCommandAddArg(cmd, migrateFrom);
+        } else if (STREQ(migrateFrom, "stdio")) {
+            if (qemuCmdFlags & QEMUD_CMD_FLAG_MIGRATE_QEMU_FD) {
+                virCommandAddArgFormat(cmd, "fd:%d", migrateFd);
+                virCommandPreserveFD(cmd, migrateFd);
+            } else if (qemuCmdFlags & QEMUD_CMD_FLAG_MIGRATE_QEMU_EXEC) {
+                virCommandAddArg(cmd, "exec:cat");
+                virCommandSetInputFD(cmd, migrateFd);
+            } else if (qemuCmdFlags & QEMUD_CMD_FLAG_MIGRATE_KVM_STDIO) {
+                virCommandAddArg(cmd, migrateFrom);
+                virCommandSetInputFD(cmd, migrateFd);
+            } else {
+                qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                                "%s", _("STDIO migration is not supported "
+                                        "with this QEMU binary"));
+                goto error;
+            }
+        } else if (STRPREFIX(migrateFrom, "exec")) {
+            if (!(qemuCmdFlags & QEMUD_CMD_FLAG_MIGRATE_QEMU_EXEC)) {
+                qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                                "%s", _("EXEC migration is not supported "
+                                        "with this QEMU binary"));
+                goto error;
+            }
+            virCommandAddArg(cmd, migrateFrom);
+        } else if (STRPREFIX(migrateFrom, "fd")) {
+            if (!(qemuCmdFlags & QEMUD_CMD_FLAG_MIGRATE_QEMU_FD)) {
+                qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                                "%s", _("FD migration is not supported "
+                                        "with this QEMU binary"));
+                goto error;
+            }
+            virCommandAddArg(cmd, migrateFrom);
+            virCommandPreserveFD(cmd, migrateFd);
+        } else {
+            qemuReportError(VIR_ERR_INTERNAL_ERROR,
+                            "%s", _("unknown migration protocol"));
+            goto error;
+        }
+    }
 
     /* QEMU changed its default behavior to not include the virtio balloon
      * device.  Explicitly request it to ensure it will be present.

diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index 9c07aca496dab30e5a8e2d0a59d5059c968cc096..e41025932400167155fe93af383825d6c3f7524d 100644 (file)
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -44,6 +44,7 @@ virCommandPtr qemuBuildCommandLine(virConnectPtr conn,
                                    bool monitor_json,
                                    unsigned long long qemuCmdFlags,
                                    const char *migrateFrom,
+                                   int migrateFd,
                                    virDomainSnapshotObjPtr current_snapshot,
                                    enum virVMOperationType vmop)
     ATTRIBUTE_NONNULL(1);

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 2beb26826404f89c95e55ae44952ffbe31d51d01..fb821a59377bf2fbb805e5616056ba6d101c10a5 100644 (file)
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2777,7 +2777,7 @@ static int qemudStartVMDaemon(virConnectPtr conn,
     vm->def->id = driver->nextvmid++;
     if (!(cmd = qemuBuildCommandLine(conn, driver, vm->def, priv->monConfig,
                                      priv->monJSON != 0, qemuCmdFlags,
-                                     migrateFrom,
+                                     migrateFrom, stdin_fd,
                                      vm->current_snapshot, vmop)))
         goto cleanup;
 
@@ -2827,9 +2827,6 @@ static int qemudStartVMDaemon(virConnectPtr conn,
     VIR_WARN("Executing %s", vm->def->emulator);
     virCommandSetPreExecHook(cmd, qemudSecurityHook, &hookData);
 
-    if (stdin_fd != -1)
-        virCommandSetInputFD(cmd, stdin_fd);
-
     virCommandSetOutputFD(cmd, &logfile);
     virCommandSetErrorFD(cmd, &logfile);
     virCommandNonblockingFDs(cmd);
@@ -6091,7 +6088,7 @@ static char *qemuDomainXMLToNative(virConnectPtr conn,
 
     if (!(cmd = qemuBuildCommandLine(conn, driver, def,
                                      &monConfig, false, qemuCmdFlags,
-                                     NULL, NULL, VIR_VM_OP_NO_OP)))
+                                     NULL, -1, NULL, VIR_VM_OP_NO_OP)))
         goto cleanup;
 
     ret = virCommandToString(cmd);

diff --git a/tests/qemuxml2argvdata/qemuxml2argv-restore-v2-fd.args b/tests/qemuxml2argvdata/qemuxml2argv-restore-v2-fd.args
new file mode 100644 (file)
index 0000000..5464d37
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-restore-v2-fd.args
@@ -0,0 +1 @@
+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none -usb -incoming fd:7

diff --git a/tests/qemuxml2argvdata/qemuxml2argv-restore-v2-fd.xml b/tests/qemuxml2argvdata/qemuxml2argv-restore-v2-fd.xml
new file mode 100644 (file)
index 0000000..ed91e37
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-restore-v2-fd.xml
@@ -0,0 +1,25 @@
+<domain type='qemu'>
+  <name>QEMUGuest1</name>
+  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+  <memory>219200</memory>
+  <currentMemory>219200</currentMemory>
+  <vcpu>1</vcpu>
+  <os>
+    <type arch='i686' machine='pc'>hvm</type>
+    <boot dev='hd'/>
+  </os>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <devices>
+    <emulator>/usr/bin/qemu</emulator>
+    <disk type='block' device='disk'>
+      <source dev='/dev/HostVG/QEMUGuest1'/>
+      <target dev='hda' bus='ide'/>
+      <address type='drive' controller='0' bus='0' unit='0'/>
+    </disk>
+    <controller type='ide' index='0'/>
+    <memballoon model='virtio'/>
+  </devices>
+</domain>

diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index ee5eb02c388aabdc6cb04b1f38f309623db38ee2..9f74777f50aafbad4120d13f55022a8d99356db1 100644 (file)
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -31,6 +31,7 @@ static int testCompareXMLToArgvFiles(const char *xml,
                                      const char *cmdline,
                                      unsigned long long extraFlags,
                                      const char *migrateFrom,
+                                     int migrateFd,
                                      bool expectError) {
     char argvData[MAX_FILE];
     char *expectargv = &(argvData[0]);
@@ -112,7 +113,8 @@ static int testCompareXMLToArgvFiles(const char *xml,
 
     if (!(cmd = qemuBuildCommandLine(conn, &driver,
                                      vmdef, &monitor_chr, false, flags,
-                                     migrateFrom, NULL, VIR_VM_OP_CREATE)))
+                                     migrateFrom, migrateFd, NULL,
+                                     VIR_VM_OP_CREATE)))
         goto fail;
 
     if (!!virGetLastError() != expectError) {
@@ -160,6 +162,7 @@ struct testInfo {
     const char *name;
     unsigned long long extraFlags;
     const char *migrateFrom;
+    int migrateFd;
     bool expectError;
 };
 
@@ -172,7 +175,8 @@ static int testCompareXMLToArgvHelper(const void *data) {
     snprintf(args, PATH_MAX, "%s/qemuxml2argvdata/qemuxml2argv-%s.args",
              abs_srcdir, info->name);
     return testCompareXMLToArgvFiles(xml, args, info->extraFlags,
-                                     info->migrateFrom, info->expectError);
+                                     info->migrateFrom, info->migrateFd,
+                                     info->expectError);
 }
 
 
@@ -217,10 +221,10 @@ mymain(int argc, char **argv)
     if (cpuMapOverride(map) < 0)
         return EXIT_FAILURE;
 
-# define DO_TEST_FULL(name, extraFlags, migrateFrom, expectError)       \
+# define DO_TEST_FULL(name, extraFlags, migrateFrom, migrateFd, expectError) \
     do {                                                                \
         const struct testInfo info = {                                  \
-            name, extraFlags, migrateFrom, expectError                  \
+            name, extraFlags, migrateFrom, migrateFd, expectError       \
         };                                                              \
         if (virtTestRun("QEMU XML-2-ARGV " name,                        \
                         1, testCompareXMLToArgvHelper, &info) < 0)      \
@@ -228,7 +232,7 @@ mymain(int argc, char **argv)
     } while (0)
 
 # define DO_TEST(name, extraFlags, expectError)                         \
-        DO_TEST_FULL(name, extraFlags, NULL, expectError)
+    DO_TEST_FULL(name, extraFlags, NULL, -1, expectError)
 
     /* Unset or set all envvars here that are copied in qemudBuildCommandLine
      * using ADD_ENV_COPY, otherwise these tests may fail due to unexpected
@@ -424,10 +428,18 @@ mymain(int argc, char **argv)
     DO_TEST("hostdev-pci-address-device", QEMUD_CMD_FLAG_PCIDEVICE |
             QEMUD_CMD_FLAG_DEVICE | QEMUD_CMD_FLAG_NODEFCONFIG, false);
 
-    DO_TEST_FULL("restore-v1", QEMUD_CMD_FLAG_MIGRATE_KVM_STDIO, "stdio", false);
-    DO_TEST_FULL("restore-v2", QEMUD_CMD_FLAG_MIGRATE_QEMU_EXEC, "stdio", false);
-    DO_TEST_FULL("restore-v2", QEMUD_CMD_FLAG_MIGRATE_QEMU_EXEC, "exec:cat", false);
-    DO_TEST_FULL("migrate", QEMUD_CMD_FLAG_MIGRATE_QEMU_TCP, "tcp:10.0.0.1:5000", false);
+    DO_TEST_FULL("restore-v1", QEMUD_CMD_FLAG_MIGRATE_KVM_STDIO, "stdio", 7,
+                 false);
+    DO_TEST_FULL("restore-v2", QEMUD_CMD_FLAG_MIGRATE_QEMU_EXEC, "stdio", 7,
+                 false);
+    DO_TEST_FULL("restore-v2", QEMUD_CMD_FLAG_MIGRATE_QEMU_EXEC, "exec:cat", 7,
+                 false);
+    DO_TEST_FULL("restore-v2-fd", QEMUD_CMD_FLAG_MIGRATE_QEMU_FD, "stdio", 7,
+                 false);
+    DO_TEST_FULL("restore-v2-fd", QEMUD_CMD_FLAG_MIGRATE_QEMU_FD, "fd:7", 7,
+                 false);
+    DO_TEST_FULL("migrate", QEMUD_CMD_FLAG_MIGRATE_QEMU_TCP,
+                 "tcp:10.0.0.1:5000", -1, false);
 
     DO_TEST("qemu-ns", 0, false);