qemuhotplugtest: Introduce test for chardev hotplug

The test is currently testing just device update function. However,
chardev hotplug is implemented just for device attach and detach. This
fact means, the test needs to be rewritten (the majority of the code is
still shared). Moreover, we are now able to pass VM among multiple test
runs. So for instance, while we add a device in the first run, we can
remove it in the second run.

tests/qemuhotplugtest.c

@@ -33,11 +33,20 @@
 
 static virQEMUDriver driver;
 
+enum {
+    ATTACH,
+    DETACH,
+    UPDATE
+};
+
 struct qemuHotplugTestData {
     const char *domain_filename;
     const char *device_filename;
     bool fail;
     const char *const *mon;
+    int action;
+    bool keep;
+    virDomainObjPtr vm;
 };
 
 static int
@@ -46,6 +55,7 @@ qemuHotplugCreateObjects(virDomainXMLOptionPtr xmlopt,
                          const char *filename)
 {
     int ret = -1;
+    qemuDomainObjPrivatePtr priv = NULL;
 
     if (!(*vm = virDomainObjNew(xmlopt)))
         goto cleanup;
@@ -57,11 +67,84 @@ qemuHotplugCreateObjects(virDomainXMLOptionPtr xmlopt,
                                              0)))
         goto cleanup;
 
+    priv = (*vm)->privateData;
+
+    if (!(priv->qemuCaps = virQEMUCapsNew()))
+        goto cleanup;
+
+    /* for attach & detach qemu must support -device */
+    virQEMUCapsSet(priv->qemuCaps, QEMU_CAPS_DEVICE);
+
     ret = 0;
 cleanup:
     return ret;
 }
 
+static int
+testQemuHotplugAttach(virDomainObjPtr vm,
+                      virDomainDeviceDefPtr dev)
+{
+    int ret = -1;
+
+    switch (dev->type) {
+    case VIR_DOMAIN_DEVICE_CHR:
+        ret = qemuDomainAttachChrDevice(&driver, vm, dev->data.chr);
+        break;
+    default:
+        if (virTestGetVerbose())
+            fprintf(stderr, "device type '%s' cannot be attached",
+                    virDomainDeviceTypeToString(dev->type));
+        break;
+    }
+
+    return ret;
+}
+
+static int
+testQemuHotplugDetach(virDomainObjPtr vm,
+                      virDomainDeviceDefPtr dev)
+{
+    int ret = -1;
+
+    switch (dev->type) {
+    case VIR_DOMAIN_DEVICE_CHR:
+        ret = qemuDomainDetachChrDevice(&driver, vm, dev->data.chr);
+        break;
+    default:
+        if (virTestGetVerbose())
+            fprintf(stderr, "device type '%s' cannot be attached",
+                    virDomainDeviceTypeToString(dev->type));
+        break;
+    }
+
+    return ret;
+}
+
+static int
+testQemuHotplugUpdate(virDomainObjPtr vm,
+                      virDomainDeviceDefPtr dev)
+{
+    int ret = -1;
+
+    /* XXX Ideally, we would call qemuDomainUpdateDeviceLive here.  But that
+     * would require us to provide virConnectPtr and virDomainPtr (they're used
+     * in case of updating a disk device. So for now, we will proceed with
+     * breaking the function into pieces. If we ever learn how to fake those
+     * required object, we can replace this code then. */
+    switch (dev->type) {
+    case VIR_DOMAIN_DEVICE_GRAPHICS:
+        ret = qemuDomainChangeGraphics(&driver, vm, dev->data.graphics);
+        break;
+    default:
+        if (virTestGetVerbose())
+            fprintf(stderr, "device type '%s' cannot be updated",
+                    virDomainDeviceTypeToString(dev->type));
+        break;
+    }
+
+    return ret;
+}
+
 static int
 testQemuHotplug(const void *data)
 {
@@ -72,6 +155,7 @@ testQemuHotplug(const void *data)
     char *device_xml = NULL;
     const char *const *tmp;
     bool fail = test->fail;
+    bool keep = test->keep;
     virDomainObjPtr vm = NULL;
     virDomainDeviceDefPtr dev = NULL;
     virCapsPtr caps = NULL;
@@ -87,17 +171,18 @@ testQemuHotplug(const void *data)
     if (!(caps = virQEMUDriverGetCapabilities(&driver, false)))
         goto cleanup;
 
-    if (qemuHotplugCreateObjects(driver.xmlopt, &vm, domain_filename) < 0)
-        goto cleanup;
-
-    priv = vm->privateData;
+    if (test->vm) {
+        vm = test->vm;
+    } else {
+        if (qemuHotplugCreateObjects(driver.xmlopt, &vm, domain_filename) < 0)
+            goto cleanup;
+    }
 
     if (virtTestLoadFile(device_filename, &device_xml) < 0)
         goto cleanup;
 
     if (!(dev = virDomainDeviceDefParse(device_xml, vm->def,
-                                        caps, driver.xmlopt,
-                                        VIR_DOMAIN_XML_INACTIVE)))
+                                        caps, driver.xmlopt, 0)))
         goto cleanup;
 
     /* Now is the best time to feed the spoofed monitor with predefined
@@ -117,6 +202,7 @@ testQemuHotplug(const void *data)
             goto cleanup;
     }
 
+    priv = vm->privateData;
     priv->mon = qemuMonitorTestGetMonitor(test_mon);
     priv->monJSON = true;
 
@@ -125,20 +211,22 @@ testQemuHotplug(const void *data)
      * tries to lock it again */
     virObjectUnlock(priv->mon);
 
-    /* XXX Ideally, we would call qemuDomainUpdateDeviceLive here.  But that
-     * would require us to provide virConnectPtr and virDomainPtr (they're used
-     * in case of updating a disk device. So for now, we will proceed with
-     * breaking the function into pieces. If we ever learn how to fake those
-     * required object, we can replace this code then. */
-    switch (dev->type) {
-    case VIR_DOMAIN_DEVICE_GRAPHICS:
-        ret = qemuDomainChangeGraphics(&driver, vm, dev->data.graphics);
+    switch (test->action) {
+    case ATTACH:
+        ret = testQemuHotplugAttach(vm, dev);
+        if (!ret) {
+            /* avoid @dev double free on success,
+             * as @dev is part of vm->def now */
+            dev = NULL;
+        }
         break;
-    default:
-        if (virTestGetVerbose())
-            fprintf(stderr, "device type '%s' cannot be updated",
-                    virDomainDeviceTypeToString(dev->type));
+
+    case DETACH:
+        ret = testQemuHotplugDetach(vm, dev);
         break;
+
+    case UPDATE:
+        ret = testQemuHotplugUpdate(vm, dev);
     }
 
 cleanup:
@@ -148,7 +236,12 @@ cleanup:
     /* don't dispose test monitor with VM */
     if (priv)
         priv->mon = NULL;
-    virObjectUnref(vm);
+    if (keep) {
+        test->vm = vm;
+    } else {
+        virObjectUnref(vm);
+        test->vm = NULL;
+    }
     virDomainDeviceDefFree(dev);
     virObjectUnref(caps);
     qemuMonitorTestFree(test_mon);
@@ -159,6 +252,7 @@ static int
 mymain(void)
 {
     int ret = 0;
+    struct qemuHotplugTestData data = {0};
 
 #if !WITH_YAJL
     fputs("libvirt not compiled with yajl, skipping this test\n", stderr);
@@ -180,26 +274,52 @@ mymain(void)
     if (VIR_STRDUP_QUIET(driver.config->spicePassword, "123456") < 0)
         return EXIT_FAILURE;
 
-#define DO_TEST(file, dev, fial, ...) \
-    do { \
-        const char *my_mon[] = { __VA_ARGS__, NULL}; \
-        struct qemuHotplugTestData data =                                    \
-            {.domain_filename = file, .device_filename = dev, .fail = fial,  \
-             .mon = my_mon}; \
-        if (virtTestRun(#file, 1, testQemuHotplug, &data) < 0)               \
-            ret = -1;                                                        \
+#define DO_TEST(file, dev, fial, kep, ...)                                  \
+        const char *my_mon[] = { __VA_ARGS__, NULL};                        \
+        data.domain_filename = file;                                        \
+        data.device_filename = dev;                                         \
+        data.fail = fial;                                                   \
+        data.mon = my_mon;                                                  \
+        data.keep = kep;                                                    \
+        if (virtTestRun(#file, 1, testQemuHotplug, &data) < 0)              \
+            ret = -1;                                                       \
+
+#define DO_TEST_ATTACH(file, dev, fial, kep, ...)                           \
+    do {                                                                    \
+        data.action = ATTACH;                                               \
+        DO_TEST(file, dev, fial, kep, __VA_ARGS__)                          \
     } while (0)
 
-    DO_TEST("graphics-spice", "graphics-spice-nochange", false, NULL);
-    DO_TEST("graphics-spice-timeout", "graphics-spice-timeout-nochange", false,
-            "set_password", "{\"return\":{}}", "expire_password", "{\"return\":{}}");
-    DO_TEST("graphics-spice-timeout", "graphics-spice-timeout-password", false,
-            "set_password", "{\"return\":{}}", "expire_password", "{\"return\":{}}");
-    DO_TEST("graphics-spice", "graphics-spice-listen", true, NULL);
-    DO_TEST("graphics-spice-listen-network", "graphics-spice-listen-network", false,
-            "set_password", "{\"return\":{}}", "expire_password", "{\"return\":{}}");
-    /* Strange huh? Currently, only graphics can be testet :-P */
-    DO_TEST("disk-cdrom", "disk-cdrom-nochange", true, NULL);
+#define DO_TEST_DETACH(file, dev, fial, kep, ...)                           \
+    do {                                                                    \
+        data.action = DETACH;                                               \
+        DO_TEST(file, dev, fial, kep, __VA_ARGS__)                          \
+    } while (0)
+
+#define DO_TEST_UPDATE(file, dev, fial, kep, ...)                           \
+    do {                                                                    \
+        data.action = UPDATE;                                               \
+        DO_TEST(file, dev, fial, kep, __VA_ARGS__)                          \
+    } while (0)
+
+    DO_TEST_UPDATE("graphics-spice", "graphics-spice-nochange", false, false, NULL);
+    DO_TEST_UPDATE("graphics-spice-timeout", "graphics-spice-timeout-nochange", false, false,
+                   "set_password", "{\"return\":{}}", "expire_password", "{\"return\":{}}");
+    DO_TEST_UPDATE("graphics-spice-timeout", "graphics-spice-timeout-password", false, false,
+                   "set_password", "{\"return\":{}}", "expire_password", "{\"return\":{}}");
+    DO_TEST_UPDATE("graphics-spice", "graphics-spice-listen", true, false, NULL);
+    DO_TEST_UPDATE("graphics-spice-listen-network", "graphics-spice-listen-network", false, false,
+                   "set_password", "{\"return\":{}}", "expire_password", "{\"return\":{}}");
+    /* Strange huh? Currently, only graphics can be updated :-P */
+    DO_TEST_UPDATE("disk-cdrom", "disk-cdrom-nochange", true, false, NULL);
+
+    DO_TEST_ATTACH("console-compat-2", "console-virtio", false, true,
+                   "chardev-add", "{\"return\": {\"pty\": \"/dev/pts/26\"}}",
+                   "device_add", "{\"return\": {}}");
+
+    DO_TEST_DETACH("console-compat-2", "console-virtio", false, false,
+                   "device_del", "{\"return\": {}}",
+                   "chardev-remove", "{\"return\": {}}");
 
     virObjectUnref(driver.caps);
     virObjectUnref(driver.xmlopt);

tests/qemuhotplugtestdata/qemuhotplug-console-virtio.xml

@@ -0,0 +1,5 @@
+    <console type='pty'>
+      <source path='/dev/pts/26'/>
+      <target type='virtio' port='1'/>
+      <alias name='console1'/>
+    </console>

tests/qemuxml2argvdata/qemuxml2argv-console-compat-2.xml

@@ -0,0 +1,122 @@
+<domain type='kvm' id='2'>
+  <name>f17</name>
+  <uuid>a1cd52eb-d37f-4717-fc6e-972f0774f4c9</uuid>
+  <memory unit='KiB'>1048576</memory>
+  <currentMemory unit='KiB'>1048576</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <resource>
+    <partition>/machine</partition>
+  </resource>
+  <os>
+    <type arch='x86_64' machine='pc-i440fx-1.4'>hvm</type>
+    <boot dev='hd'/>
+  </os>
+  <features>
+    <acpi/>
+    <apic/>
+    <pae/>
+  </features>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>restart</on_crash>
+  <pm>
+    <suspend-to-mem enabled='yes'/>
+    <suspend-to-disk enabled='yes'/>
+  </pm>
+  <devices>
+    <emulator>/usr/bin/qemu-system-x86_64</emulator>
+    <disk type='file' device='disk'>
+      <driver name='qemu' type='qcow2' cache='none'/>
+      <source file='/var/lib/libvirt/images/f17.qcow2'/>
+      <target dev='vda' bus='virtio'/>
+      <alias name='virtio-disk0'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/>
+    </disk>
+    <disk type='file' device='cdrom'>
+      <driver name='qemu' type='raw' cache='none'/>
+      <source file='/home/user/tmp/Fedora-17-x86_64-Live-KDE.iso'/>
+      <target dev='hdc' bus='ide'/>
+      <readonly/>
+      <alias name='ide0-1-0'/>
+      <address type='drive' controller='0' bus='1' target='0' unit='0'/>
+    </disk>
+    <controller type='ide' index='0'>
+      <alias name='ide0'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/>
+    </controller>
+    <controller type='usb' index='0'>
+      <alias name='usb0'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
+    </controller>
+    <controller type='pci' index='0' model='pci-root'>
+      <alias name='pci0'/>
+    </controller>
+    <controller type='virtio-serial' index='0'>
+      <alias name='virtio-serial0'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x0'/>
+    </controller>
+    <interface type='network'>
+      <mac address='52:54:00:ea:35:6f'/>
+      <source network='default'/>
+      <target dev='vnet0'/>
+      <model type='virtio'/>
+      <bandwidth>
+        <inbound average='4000' peak='8000' floor='200' burst='1024'/>
+        <outbound average='4000' peak='8000' burst='1024'/>
+      </bandwidth>
+      <alias name='net0'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
+    </interface>
+    <serial type='pty'>
+      <source path='/dev/pts/22'/>
+      <target type='isa-serial' port='0'/>
+      <alias name='serial0'/>
+    </serial>
+    <serial type='pty'>
+      <source path='/dev/pts/25'/>
+      <target port='0'/>
+      <alias name='serial1'/>
+    </serial>
+    <serial type='tcp'>
+      <source mode='bind' host='0.0.0.0' service='2445'/>
+      <protocol type='raw'/>
+      <target port='1'/>
+      <alias name='serial2'/>
+    </serial>
+    <console type='pty' tty='/dev/pts/22'>
+      <source path='/dev/pts/22'/>
+      <target type='serial' port='0'/>
+      <alias name='serial0'/>
+    </console>
+    <channel type='unix'>
+      <source mode='bind' path='/var/lib/libvirt/qemu/f17x86_64.agent'/>
+      <target type='virtio' name='org.qemu.guest_agent.0'/>
+      <alias name='channel0'/>
+      <address type='virtio-serial' controller='0' bus='0' port='1'/>
+    </channel>
+    <input type='tablet' bus='usb'>
+      <alias name='input0'/>
+    </input>
+    <input type='mouse' bus='ps2'/>
+    <graphics type='vnc' port='5900' autoport='yes' listen='0.0.0.0'>
+      <listen type='address' address='0.0.0.0'/>
+    </graphics>
+    <sound model='ich6'>
+      <alias name='sound0'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/>
+    </sound>
+    <video>
+      <model type='cirrus' vram='9216' heads='1'/>
+      <alias name='video0'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/>
+    </video>
+    <memballoon model='virtio'>
+      <alias name='balloon0'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/>
+    </memballoon>
+  </devices>
+  <seclabel type='static' model='dac' relabel='no'>
+    <label>root:root</label>
+  </seclabel>
+</domain>