qemu: fix address allocation on chardev attach

Also check the device type when deciding what type the address should
be. Commit 9807c47 (aiming to fix another error in address allocation)
only checked the target type, but its value is different for different
device types. This resulted in an error when trying to attach
a channel with target type 'virtio':

error: Failed to attach device from channel-file.xml
error: internal error: virtio serial device has invalid address type

Make the logic for releasing the address dependent only on
* the address type
* whether it was allocated earlier
to avoid copying the device and target type checks.

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

Signed-off-by: Luyao Huang <lhuang@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index c8f8fe837416d3e2caa2babca6863663197fa717..25a7bc621440e0d43e36507593cdc1d12be78251 100644 (file)
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1837,6 +1837,10 @@ qemuDomainReleaseDeviceAddress(virDomainObjPtr vm,
                                             &info->addr.pci) < 0)
         VIR_WARN("Unable to release PCI address on %s",
                  NULLSTR(devstr));
+    if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_SERIAL &&
+        virDomainVirtioSerialAddrRelease(priv->vioserialaddrs, info) < 0)
+        VIR_WARN("Unable to release virtio-serial address on %s",
+                 NULLSTR(devstr));
 }
 
 

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 062896415ed2b57201b9a103104e89043121db4b..c7c2ea471c7c6e5fad3d1eee31097561b58b7ac2 100644 (file)
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1531,17 +1531,51 @@ qemuDomainChrRemove(virDomainDefPtr vmdef,
     return ret;
 }
 
+static int
+qemuDomainAttachChrDeviceAssignAddr(qemuDomainObjPrivatePtr priv,
+                                    virDomainChrDefPtr chr)
+{
+    if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE &&
+        chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO) {
+        if (virDomainVirtioSerialAddrAutoAssign(NULL, priv->vioserialaddrs,
+                                                &chr->info, true) < 0)
+            return -1;
+        return 1;
+
+    } else if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL &&
+               chr->targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_PCI) {
+        if (virDomainPCIAddressEnsureAddr(priv->pciaddrs, &chr->info) < 0)
+            return -1;
+        return 1;
+
+    } else if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL &&
+               chr->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO) {
+        if (virDomainVirtioSerialAddrAutoAssign(NULL, priv->vioserialaddrs,
+                                                &chr->info, false) < 0)
+            return -1;
+        return 1;
+    }
+
+    if (chr->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_SERIAL ||
+        chr->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                       _("Unsupported address type for character device"));
+        return -1;
+    }
+
+    return 0;
+}
+
 int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
                               virDomainObjPtr vm,
                               virDomainChrDefPtr chr)
 {
-    int ret = -1;
+    int ret = -1, rc;
     qemuDomainObjPrivatePtr priv = vm->privateData;
     virDomainDefPtr vmdef = vm->def;
     char *devstr = NULL;
     char *charAlias = NULL;
     bool need_release = false;
-    bool allowZero = false;
 
     if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {
         virReportError(VIR_ERR_OPERATION_INVALID, "%s",
@@ -1552,23 +1586,10 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
     if (qemuAssignDeviceChrAlias(vmdef, chr, -1) < 0)
         goto cleanup;
 
-    if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE &&
-        chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO)
-        allowZero = true;
-
-    if (chr->targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_PCI) {
-        if (virDomainPCIAddressEnsureAddr(priv->pciaddrs, &chr->info) < 0)
-            goto cleanup;
-    } else if (chr->targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_USB) {
-        /* XXX */
-    } else {
-        if (virDomainVirtioSerialAddrAutoAssign(NULL,
-                                                priv->vioserialaddrs,
-                                                &chr->info,
-                                                allowZero) < 0)
-            goto cleanup;
-    }
-    need_release = true;
+    if ((rc = qemuDomainAttachChrDeviceAssignAddr(priv, chr) < 0))
+        goto cleanup;
+    if (rc == 1)
+        need_release = true;
 
     if (qemuBuildChrDeviceStr(&devstr, vm->def, chr, priv->qemuCaps) < 0)
         goto cleanup;
@@ -1601,15 +1622,8 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
  cleanup:
     if (ret < 0 && virDomainObjIsActive(vm))
         qemuDomainChrInsertPreAllocCleanup(vm->def, chr);
-    if (ret < 0 && need_release) {
-        if (chr->targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_PCI) {
-            qemuDomainReleaseDeviceAddress(vm, &chr->info, NULL);
-        } else if (chr->targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_USB) {
-            /* XXX */
-        } else {
-            virDomainVirtioSerialAddrRelease(priv->vioserialaddrs, &chr->info);
-        }
-    }
+    if (ret < 0 && need_release)
+        qemuDomainReleaseDeviceAddress(vm, &chr->info, NULL);
     VIR_FREE(charAlias);
     VIR_FREE(devstr);
     return ret;