From: Luyao Huang <lhuang(a)redhat.com&gt; 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(a)redhat.com&gt; Signed-off-by: Ján Tomko <jtomko(a)redhat.com&gt; --- src/qemu/qemu_command.c | 4 +++ src/qemu/qemu_hotplug.c | 70 +++++++++++++++++++++++++++++-------------------- 2 files changed, 46 insertions(+), 28 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 9b06a49..01c80c0 100644 --- 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 0628964..c7c2ea4 100644 --- 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; -- 2.3.6

The target type comparison in qemuDomainDetachChrDevice used the VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE enum, so virtio-serial addresses were not freed properly for channel devices. Call qemuDomainReleaseDeviceAddress uncoditionally and decide based on the address type instead of the target/device types. --- src/qemu/qemu_hotplug.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index c7c2ea4..11f983f 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -4169,13 +4169,7 @@ int qemuDomainDetachChrDevice(virQEMUDriverPtr driver, rc = qemuDomainWaitForDeviceRemoval(vm); if (rc == 0 || rc == 1) { - if (chr->targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_PCI) { - qemuDomainReleaseDeviceAddress(vm, &tmpChr->info, NULL); - } else if (chr->targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_USB) { - /* XXX */ - } else { - virDomainVirtioSerialAddrRelease(priv->vioserialaddrs, &tmpChr->info); - } + qemuDomainReleaseDeviceAddress(vm, &tmpChr->info, NULL); ret = qemuDomainRemoveChrDevice(driver, vm, tmpChr); } else { ret = 0; -- 2.3.6