[libvirt] [PATCH 1/2] qemu: fix address allocation on chardev attach
John Ferlan
jferlan at redhat.com
Wed Jul 1 10:21:07 UTC 2015
On 06/30/2015 10:26 AM, Ján Tomko wrote:
> From: Luyao Huang <lhuang at redhat.com>
>
> 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 at redhat.com>
> Signed-off-by: Ján Tomko <jtomko at redhat.com>
> ---
> 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))
Coverity wasn't too happy about the placement of the parentheses here -
making things DEAD after here... Including the ability to set need_release
The parens should be (priv, chr)) < 0)
John
> + 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;
>
More information about the libvir-list
mailing list