Fix save/restore migrate syntax for new KVM releases

ChangeLog

@@ -1,3 +1,25 @@
+Thu Jan 29 17:24:22 GMT 2009 Daniel P. Berrange <berrange@redhat.com>
+
+	Fix save/restore for new KVM releases
+	* src/domain_conf.h, src/lxc_driver.c, src/uml_driver.c:
+	Remove unused stdin_fd field from virDomainObjPtr
+	* src/qemu_conf.c, src/qemu_driver.c: Support new
+	migration options for save & restore, and fix deadlock
+	in save code.
+	* src/qemu_conf.h: Add more QEMU argv flags to various
+	migration options & describe existing flags
+	* src/util.c: Close original stdin file handle after
+	duping it onto STDIN_FILENO
+	* tests/qemuxml2argvtest.c: Test for various migrate
+	syntax options
+	* tests/qemuxml2argvdata/qemuxml2argv-migrate.args,
+	tests/qemuxml2argvdata/qemuxml2argv-migrate.xml,
+	tests/qemuxml2argvdata/qemuxml2argv-restore-v1.args,
+	tests/qemuxml2argvdata/qemuxml2argv-restore-v1.xml,
+	tests/qemuxml2argvdata/qemuxml2argv-restore-v2.args,
+	tests/qemuxml2argvdata/qemuxml2argv-restore-v2.xml: Data
+	files for testing migrate syntax options
+
 Thu Jan 29 17:15:18 GMT 2009 John Levon <john.levon@sun.com>
 
 	* src/libvirt.c: fix another printf("%s", NULL) case

src/domain_conf.h

@@ -462,7 +462,6 @@ typedef virDomainObj *virDomainObjPtr;
 struct _virDomainObj {
     virMutex lock;
 
-    int stdin_fd;
     int monitor;
     int monitor_watch;
     char *monitorpath;

src/lxc_driver.c

@@ -775,11 +775,10 @@ static int lxcControllerStart(virConnectPtr conn,
 
     ADD_ARG(NULL);
 
-    vm->stdin_fd = -1;
     FD_SET(appPty, &keepfd);
 
     if (virExec(conn, largv, NULL, &keepfd, &child,
-                vm->stdin_fd, &logfd, &logfd,
+                -1, &logfd, &logfd,
                 VIR_EXEC_NONE) < 0)
         goto cleanup;
 

src/qemu_conf.c

@@ -396,6 +396,27 @@ int qemudExtractVersionInfo(const char *qemu,
     if (kvm_version >= 74)
         flags |= QEMUD_CMD_FLAG_VNET_HDR;
 
+    /*
+     * Handling of -incoming arg with varying features
+     *  -incoming tcp    (kvm >= 79)
+     *  -incoming exec   (kvm >= 80)
+     *  -incoming stdio  (all earlier kvm)
+     *
+     * NB, there was a pre-kvm-79 'tcp' support, but it
+     * was broken, because it blocked the monitor console
+     * while waiting for data, so pretend it doesn't exist
+     *
+     * XXX when next QEMU release after 0.9.1 arrives,
+     * we'll need to add MIGRATE_QEMU_TCP/EXEC here too
+     */
+    if (kvm_version >= 79) {
+        flags |= QEMUD_CMD_FLAG_MIGRATE_QEMU_TCP;
+        if (kvm_version >= 80)
+            flags |= QEMUD_CMD_FLAG_MIGRATE_QEMU_EXEC;
+    } else if (kvm_version > 0) {
+        flags |= QEMUD_CMD_FLAG_MIGRATE_KVM_STDIO;
+    }
+
     if (retversion)
         *retversion = version;
     if (retflags)
@@ -677,6 +698,33 @@ int qemudBuildCommandLine(virConnectPtr conn,
 
     virUUIDFormat(vm->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)) {
+                qemudReportError(conn, NULL, NULL, VIR_ERR_NO_SUPPORT,
+                                 "%s", _("TCP migration is not supported with this QEMU binary"));
+                return -1;
+            }
+        } 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)) {
+                qemudReportError(conn, NULL, NULL, VIR_ERR_NO_SUPPORT,
+                                 "%s", _("STDIO migration is not supported with this QEMU binary"));
+                return -1;
+            }
+        } else if (STRPREFIX(migrateFrom, "exec")) {
+            if (!(qemuCmdFlags & QEMUD_CMD_FLAG_MIGRATE_QEMU_EXEC)) {
+                qemudReportError(conn, NULL, NULL, VIR_ERR_NO_SUPPORT,
+                                 "%s", _("STDIO migration is not supported with this QEMU binary"));
+                return -1;
+            }
+        }
+    }
+
     /* Need to explicitly disable KQEMU if
      * 1. Arch matches host arch
      * 2. Guest is 'qemu'

src/qemu_conf.h

@@ -40,15 +40,19 @@
 
 /* Internal flags to keep track of qemu command line capabilities */
 enum qemud_cmd_flags {
-    QEMUD_CMD_FLAG_KQEMU          = (1 << 0),
+    QEMUD_CMD_FLAG_KQEMU          = (1 << 0), /* Whether KQEMU is compiled in */
-    QEMUD_CMD_FLAG_VNC_COLON      = (1 << 1),
+    QEMUD_CMD_FLAG_VNC_COLON      = (1 << 1), /* Does the VNC take just port, or address + display */
-    QEMUD_CMD_FLAG_NO_REBOOT      = (1 << 2),
+    QEMUD_CMD_FLAG_NO_REBOOT      = (1 << 2), /* Is the -no-reboot flag available */
-    QEMUD_CMD_FLAG_DRIVE          = (1 << 3),
+    QEMUD_CMD_FLAG_DRIVE          = (1 << 3), /* Is the new -drive arg available */
-    QEMUD_CMD_FLAG_DRIVE_BOOT     = (1 << 4),
+    QEMUD_CMD_FLAG_DRIVE_BOOT     = (1 << 4), /* Does -drive support boot=on */
-    QEMUD_CMD_FLAG_NAME           = (1 << 5),
+    QEMUD_CMD_FLAG_NAME           = (1 << 5), /* Is the -name flag available */
-    QEMUD_CMD_FLAG_UUID           = (1 << 6),
+    QEMUD_CMD_FLAG_UUID           = (1 << 6), /* Is the -uuid flag available */
-    QEMUD_CMD_FLAG_DOMID          = (1 << 7), /* Xenner only */
+    QEMUD_CMD_FLAG_DOMID          = (1 << 7), /* Xenner only, special -domid flag available */
-    QEMUD_CMD_FLAG_VNET_HDR       = (1 << 8),
+    QEMUD_CMD_FLAG_VNET_HDR        = (1 << 8),
+    QEMUD_CMD_FLAG_MIGRATE_KVM_STDIO = (1 << 9),  /* Original migration code from KVM. Also had tcp, but we can't use that
+                                                   * since it had a design bug blocking the entire monitor console */
+    QEMUD_CMD_FLAG_MIGRATE_QEMU_TCP  = (1 << 10), /* New migration syntax after merge to QEMU with TCP transport */
+    QEMUD_CMD_FLAG_MIGRATE_QEMU_EXEC = (1 << 11), /* New migration syntax after merge to QEMU with EXEC transport */
 };
 
 /* Main driver state */

src/qemu_driver.c

@@ -130,7 +130,8 @@ static void qemudDispatchVMEvent(int watch,
 static int qemudStartVMDaemon(virConnectPtr conn,
                               struct qemud_driver *driver,
                               virDomainObjPtr vm,
-                              const char *migrateFrom);
+                              const char *migrateFrom,
+                              int stdin_fd);
 
 static void qemudShutdownVMDaemon(virConnectPtr conn,
                                   struct qemud_driver *driver,
@@ -237,7 +238,7 @@ qemudAutostartConfigs(struct qemud_driver *driver) {
         virDomainObjLock(vm);
         if (vm->autostart &&
             !virDomainIsActive(vm)) {
-            int ret = qemudStartVMDaemon(conn, driver, vm, NULL);
+            int ret = qemudStartVMDaemon(conn, driver, vm, NULL, -1);
             if (ret < 0) {
                 virErrorPtr err = virGetLastError();
                 qemudLog(QEMUD_ERR, _("Failed to autostart VM '%s': %s\n"),
@@ -1050,7 +1051,8 @@ static virDomainPtr qemudDomainLookupByName(virConnectPtr conn,
 static int qemudStartVMDaemon(virConnectPtr conn,
                               struct qemud_driver *driver,
                               virDomainObjPtr vm,
-                              const char *migrateFrom) {
+                              const char *migrateFrom,
+                              int stdin_fd) {
     const char **argv = NULL, **tmp;
     const char **progenv = NULL;
     int i, ret;
@@ -1161,7 +1163,7 @@ static int qemudStartVMDaemon(virConnectPtr conn,
         FD_SET(tapfds[i], &keepfd);
 
     ret = virExec(conn, argv, progenv, &keepfd, &child,
-                  vm->stdin_fd, &vm->logfile, &vm->logfile,
+                  stdin_fd, &vm->logfile, &vm->logfile,
                   VIR_EXEC_NONBLOCK | VIR_EXEC_DAEMON);
 
     /* wait for qemu process to to show up */
@@ -1811,7 +1813,7 @@ static virDomainPtr qemudDomainCreate(virConnectPtr conn, const char *xml,
 
     def = NULL;
 
-    if (qemudStartVMDaemon(conn, driver, vm, NULL) < 0) {
+    if (qemudStartVMDaemon(conn, driver, vm, NULL, -1) < 0) {
         virDomainRemoveInactive(&driver->domains,
                                 vm);
         vm = NULL;
@@ -2287,11 +2289,14 @@ static int qemudDomainSave(virDomainPtr dom,
     /* Pause */
     if (vm->state == VIR_DOMAIN_RUNNING) {
         header.was_running = 1;
-        if (qemudDomainSuspend(dom) != 0) {
+        if (qemudMonitorCommand(vm, "stop", &info) < 0) {
             qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
-                             "%s", _("failed to pause domain"));
+                             "%s", _("suspend operation failed"));
             goto cleanup;
         }
+        vm->state = VIR_DOMAIN_PAUSED;
+        qemudDebug("Reply %s", info);
+        VIR_FREE(info);
     }
 
     /* Get XML for the domain */
@@ -2678,10 +2683,14 @@ static int qemudDomainRestore(virConnectPtr conn,
     vm = virDomainFindByUUID(&driver->domains, def->uuid);
     if (!vm)
         vm = virDomainFindByName(&driver->domains, def->name);
-    if (vm && virDomainIsActive(vm)) {
+    if (vm) {
-        qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED,
+        if (virDomainIsActive(vm)) {
-                         _("domain is already active as '%s'"), vm->def->name);
+            qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED,
-        goto cleanup;
+                             _("domain is already active as '%s'"), vm->def->name);
+            goto cleanup;
+        } else {
+            virDomainObjUnlock(vm);
+        }
     }
 
     if (!(vm = virDomainAssignDef(conn,
@@ -2694,11 +2703,9 @@ static int qemudDomainRestore(virConnectPtr conn,
     def = NULL;
 
     /* Set the migration source and start it up. */
-    vm->stdin_fd = fd;
+    ret = qemudStartVMDaemon(conn, driver, vm, "stdio", fd);
-    ret = qemudStartVMDaemon(conn, driver, vm, "stdio");
     close(fd);
     fd = -1;
-    vm->stdin_fd = -1;
     if (ret < 0) {
         qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED,
                          "%s", _("failed to start VM"));
@@ -2827,7 +2834,7 @@ static int qemudDomainStart(virDomainPtr dom) {
         goto cleanup;
     }
 
-    ret = qemudStartVMDaemon(dom->conn, driver, vm, NULL);
+    ret = qemudStartVMDaemon(dom->conn, driver, vm, NULL, -1);
     if (ret != -1)
         event = virDomainEventNewFromObj(vm,
                                          VIR_DOMAIN_EVENT_STARTED,
@@ -4141,7 +4148,7 @@ qemudDomainMigratePrepare2 (virConnectPtr dconn,
      * -incoming tcp:0.0.0.0:port
      */
     snprintf (migrateFrom, sizeof (migrateFrom), "tcp:0.0.0.0:%d", this_port);
-    if (qemudStartVMDaemon (dconn, driver, vm, migrateFrom) < 0) {
+    if (qemudStartVMDaemon (dconn, driver, vm, migrateFrom, -1) < 0) {
         qemudReportError (dconn, NULL, NULL, VIR_ERR_OPERATION_FAILED,
                           "%s", _("failed to start listening VM"));
         if (!vm->persistent) {

src/uml_driver.c

@@ -811,13 +811,12 @@ static int umlStartVMDaemon(virConnectPtr conn,
                  errno, strerror(errno));
 
     vm->monitor = -1;
-    vm->stdin_fd = -1;
 
     for (i = 0 ; i < ntapfds ; i++)
         FD_SET(tapfds[i], &keepfd);
 
     ret = virExec(conn, argv, progenv, &keepfd, &pid,
-                  vm->stdin_fd, &logfd, &logfd,
+                  -1, &logfd, &logfd,
                   VIR_EXEC_DAEMON);
     close(logfd);
 

src/util.c

@@ -407,6 +407,8 @@ __virExec(virConnectPtr conn,
         _exit(1);
     }
 
+    if (infd > 0)
+        close(infd);
     close(null);
     if (childout > 0)
         close(childout);

tests/qemuxml2argvdata/qemuxml2argv-migrate.args

@@ -0,0 +1 @@
+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu-kvm -S -M pc -m 214 -smp 1 -nographic -monitor pty -pidfile /nowhere/QEMUGuest1.pid -no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none -usb -incoming tcp:10.0.0.1:5000

tests/qemuxml2argvdata/qemuxml2argv-migrate.xml

@@ -0,0 +1,22 @@
+<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-kvm</emulator>
+    <disk type='block' device='disk'>
+      <source dev='/dev/HostVG/QEMUGuest1'/>
+      <target dev='hda' bus='ide'/>
+    </disk>
+  </devices>
+</domain>

tests/qemuxml2argvdata/qemuxml2argv-restore-v1.args

@@ -0,0 +1 @@
+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu-kvm -S -M pc -m 214 -smp 1 -nographic -monitor pty -pidfile /nowhere/QEMUGuest1.pid -no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none -usb -incoming stdio

tests/qemuxml2argvdata/qemuxml2argv-restore-v1.xml

@@ -0,0 +1,22 @@
+<domain type='kvm'>
+  <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-kvm</emulator>
+    <disk type='block' device='disk'>
+      <source dev='/dev/HostVG/QEMUGuest1'/>
+      <target dev='hda' bus='ide'/>
+    </disk>
+  </devices>
+</domain>

tests/qemuxml2argvdata/qemuxml2argv-restore-v2.args

@@ -0,0 +1 @@
+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu-kvm -S -M pc -m 214 -smp 1 -nographic -monitor pty -pidfile /nowhere/QEMUGuest1.pid -no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none -usb -incoming exec:cat

tests/qemuxml2argvdata/qemuxml2argv-restore-v2.xml

@@ -0,0 +1,22 @@
+<domain type='kvm'>
+  <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-kvm</emulator>
+    <disk type='block' device='disk'>
+      <source dev='/dev/HostVG/QEMUGuest1'/>
+      <target dev='hda' bus='ide'/>
+    </disk>
+  </devices>
+</domain>

tests/qemuxml2argvtest.c

@@ -24,7 +24,8 @@ static struct qemud_driver driver;
 
 static int testCompareXMLToArgvFiles(const char *xml,
                                      const char *cmd,
-                                     int extraFlags) {
+                                     int extraFlags,
+                                     const char *migrateFrom) {
     char argvData[MAX_FILE];
     char *expectargv = &(argvData[0]);
     char *actualargv = NULL;
@@ -56,7 +57,7 @@ static int testCompareXMLToArgvFiles(const char *xml,
 
     if (qemudBuildCommandLine(NULL, &driver,
                               &vm, flags, &argv, &qenv,
-                              NULL, NULL, NULL) < 0)
+                              NULL, NULL, migrateFrom) < 0)
         goto fail;
 
     len = 1; /* for trailing newline */
@@ -122,6 +123,7 @@ static int testCompareXMLToArgvFiles(const char *xml,
 struct testInfo {
     const char *name;
     int extraFlags;
+    const char *migrateFrom;
 };
 
 static int testCompareXMLToArgvHelper(const void *data) {
@@ -132,7 +134,7 @@ static int testCompareXMLToArgvHelper(const void *data) {
              abs_srcdir, info->name);
     snprintf(args, PATH_MAX, "%s/qemuxml2argvdata/qemuxml2argv-%s.args",
              abs_srcdir, info->name);
-    return testCompareXMLToArgvFiles(xml, args, info->extraFlags);
+    return testCompareXMLToArgvFiles(xml, args, info->extraFlags, info->migrateFrom);
 }
 
 
@@ -159,14 +161,17 @@ mymain(int argc, char **argv)
     if((driver.stateDir = strdup("/nowhere")) == NULL)
         return EXIT_FAILURE;
 
-#define DO_TEST(name, extraFlags)                                       \
+#define DO_TEST_FULL(name, extraFlags, migrateFrom)                     \
     do {                                                                \
-        struct testInfo info = { name, extraFlags };                    \
+        const struct testInfo info = { name, extraFlags, migrateFrom }; \
         if (virtTestRun("QEMU XML-2-ARGV " name,                        \
                         1, testCompareXMLToArgvHelper, &info) < 0)      \
             ret = -1;                                                   \
     } while (0)
 
+#define DO_TEST(name, extraFlags)                       \
+        DO_TEST_FULL(name, extraFlags, NULL)
+
     setenv("PATH", "/bin", 1);
     setenv("USER", "test", 1);
     setenv("LOGNAME", "test", 1);
@@ -228,6 +233,11 @@ mymain(int argc, char **argv)
 
     DO_TEST("hostdev-pci-address", 0);
 
+    DO_TEST_FULL("restore-v1", QEMUD_CMD_FLAG_MIGRATE_KVM_STDIO, "stdio");
+    DO_TEST_FULL("restore-v2", QEMUD_CMD_FLAG_MIGRATE_QEMU_EXEC, "stdio");
+    DO_TEST_FULL("restore-v2", QEMUD_CMD_FLAG_MIGRATE_QEMU_EXEC, "exec:cat");
+    DO_TEST_FULL("migrate", QEMUD_CMD_FLAG_MIGRATE_QEMU_TCP, "tcp:10.0.0.1:5000");
+
     virCapabilitiesFree(driver.caps);
 
     return(ret==0 ? EXIT_SUCCESS : EXIT_FAILURE);