[PATCH] qemu: fix cannot attach a virtio channel

[Luyao Huang <lhuang@xxxxxxxxxx>]

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

When attach a channel which target type is virtio, we will
get an error:

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

This issue was introduced in commit 9807c4.
We didn't check the chr device type is serial then check
if the device target type is pci-serial, but not all the
Chr device is a serial type so we should check the device
type before check the target type to avoid assign wrong
address to other device type chr device.

Also most of chr device do not need {pci, virtio-serial} address
in qemu, we just get the address for the device which needed.

Signed-off-by: Luyao Huang <lhuang@xxxxxxxxxx>
---
 src/qemu/qemu_hotplug.c | 72 +++++++++++++++++++++++++++++++------------------
 1 file changed, 46 insertions(+), 26 deletions(-)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 3562de6..4d60513 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1531,6 +1531,48 @@ 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 &&
+        virDomainVirtioSerialAddrAutoAssign(NULL, priv->vioserialaddrs,
+                                            &chr->info, true) < 0)
+        return -1;
+
+    if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL &&
+        chr->targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_PCI &&
+        (chr->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE ||
+         chr->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) &&
+        virDomainPCIAddressEnsureAddr(priv->pciaddrs, &chr->info) < 0)
+        return -1;
+
+    if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL &&
+        chr->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO &&
+        virDomainVirtioSerialAddrAutoAssign(NULL, priv->vioserialaddrs,
+                                            &chr->info, false) < 0)
+        return -1;
+
+    return 0;
+}
+
+static void
+qemuDomainAttachChrDeviceReleaseAddr(qemuDomainObjPrivatePtr priv,
+                                     virDomainObjPtr vm,
+                                     virDomainChrDefPtr chr)
+{
+    if ((chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE &&
+         chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO) ||
+        (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL &&
+         chr->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO))
+        virDomainVirtioSerialAddrRelease(priv->vioserialaddrs, &chr->info);
+
+    if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL &&
+        chr->targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_PCI)
+        qemuDomainReleaseDeviceAddress(vm, &chr->info, NULL);
+}
+
 int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
                               virDomainObjPtr vm,
                               virDomainChrDefPtr chr)
@@ -1541,7 +1583,6 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
     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,22 +1593,8 @@ 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;
-    }
+    if (qemuDomainAttachChrDeviceAssignAddr(priv, chr) < 0)
+        goto cleanup;
     need_release = true;
 
     if (qemuBuildChrDeviceStr(&devstr, vm->def, chr, priv->qemuCaps) < 0)
@@ -1601,15 +1628,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)
+        qemuDomainAttachChrDeviceReleaseAddr(priv, vm, chr);
     VIR_FREE(charAlias);
     VIR_FREE(devstr);
     return ret;
-- 
1.8.3.1

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list