[libvirt] [PATCH 0/6] Address assignment fixes
Fixes a crash on usb-serial hotplug and missing addresses after libvirtd restart, and makes some code more readable. Ján Tomko (6): Add 'FromCache' to virDomainVirtioSerialAddrAutoAssign Introduce virDomainVirtioSerialAddrAutoAssign again Return directly from qemuDomainAttachChrDeviceAssignAddr Also create the USB address cache for domains with all the USB addresses Fix crash on usb-serial hotplug Do not try to release virtio serial addresses src/conf/domain_addr.c | 41 +++++++++++++++++++++++++++++++++---- src/conf/domain_addr.h | 14 +++++++++++-- src/libvirt_private.syms | 2 ++ src/qemu/qemu_domain_address.c | 22 ++++++++++++++++---- src/qemu/qemu_hotplug.c | 46 +++++++++++++++++------------------------- 5 files changed, 87 insertions(+), 38 deletions(-) -- 2.7.3
We dropped the cache from QEMU's private domain object. Assume the callers do not have the cache by default and use a longer name for the internal ones that do. This makes the shorter 'virDomainVirtioSerialAddrAutoAssign' name availabe for a function that will not require the cache. --- src/conf/domain_addr.c | 8 ++++---- src/conf/domain_addr.h | 8 ++++---- src/libvirt_private.syms | 2 +- src/qemu/qemu_domain_address.c | 6 ++++-- src/qemu/qemu_hotplug.c | 8 ++++---- 5 files changed, 17 insertions(+), 15 deletions(-) diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index 1e96fe9..065baa7 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -1154,10 +1154,10 @@ virDomainVirtioSerialAddrNextFromController(virDomainVirtioSerialAddrSetPtr addr * or assign a virtio serial address to the device */ int -virDomainVirtioSerialAddrAutoAssign(virDomainDefPtr def, - virDomainVirtioSerialAddrSetPtr addrs, - virDomainDeviceInfoPtr info, - bool allowZero) +virDomainVirtioSerialAddrAutoAssignFromCache(virDomainDefPtr def, + virDomainVirtioSerialAddrSetPtr addrs, + virDomainDeviceInfoPtr info, + bool allowZero) { bool portOnly = info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_SERIAL; if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_SERIAL && diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h index 0072a08..b0fadc1 100644 --- a/src/conf/domain_addr.h +++ b/src/conf/domain_addr.h @@ -222,10 +222,10 @@ virDomainVirtioSerialAddrSetCreateFromDomain(virDomainDefPtr def) bool virDomainVirtioSerialAddrIsComplete(virDomainDeviceInfoPtr info); int -virDomainVirtioSerialAddrAutoAssign(virDomainDefPtr def, - virDomainVirtioSerialAddrSetPtr addrs, - virDomainDeviceInfoPtr info, - bool allowZero) +virDomainVirtioSerialAddrAutoAssignFromCache(virDomainDefPtr def, + virDomainVirtioSerialAddrSetPtr addrs, + virDomainDeviceInfoPtr info, + bool allowZero) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); int diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index bf503a5..8bc2584 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -123,7 +123,7 @@ virDomainUSBAddressSetAddHub; virDomainUSBAddressSetCreate; virDomainUSBAddressSetFree; virDomainVirtioSerialAddrAssign; -virDomainVirtioSerialAddrAutoAssign; +virDomainVirtioSerialAddrAutoAssignFromCache; virDomainVirtioSerialAddrIsComplete; virDomainVirtioSerialAddrRelease; virDomainVirtioSerialAddrReserve; diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index dc67d51..7d92e0b 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -122,7 +122,8 @@ qemuDomainAssignVirtioSerialAddresses(virDomainDefPtr def) if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE && chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO && !virDomainVirtioSerialAddrIsComplete(&chr->info) && - virDomainVirtioSerialAddrAutoAssign(def, addrs, &chr->info, true) < 0) + virDomainVirtioSerialAddrAutoAssignFromCache(def, addrs, + &chr->info, true) < 0) goto cleanup; } @@ -131,7 +132,8 @@ qemuDomainAssignVirtioSerialAddresses(virDomainDefPtr def) if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL && chr->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO && !virDomainVirtioSerialAddrIsComplete(&chr->info) && - virDomainVirtioSerialAddrAutoAssign(def, addrs, &chr->info, false) < 0) + virDomainVirtioSerialAddrAutoAssignFromCache(def, addrs, + &chr->info, false) < 0) goto cleanup; } diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 2cb2267..7bd38ab 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1648,8 +1648,8 @@ qemuDomainAttachChrDeviceAssignAddr(virDomainDefPtr def, if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE && chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO) { - if (virDomainVirtioSerialAddrAutoAssign(NULL, vioaddrs, - &chr->info, true) < 0) + if (virDomainVirtioSerialAddrAutoAssignFromCache(NULL, vioaddrs, + &chr->info, true) < 0) goto cleanup; ret = 1; @@ -1667,8 +1667,8 @@ qemuDomainAttachChrDeviceAssignAddr(virDomainDefPtr def, } else if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL && chr->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO) { - if (virDomainVirtioSerialAddrAutoAssign(NULL, vioaddrs, - &chr->info, false) < 0) + if (virDomainVirtioSerialAddrAutoAssignFromCache(NULL, vioaddrs, + &chr->info, false) < 0) goto cleanup; ret = 1; } -- 2.7.3
On 10/21/2016 09:58 AM, Ján Tomko wrote:
We dropped the cache from QEMU's private domain object.
s/We dropped the cache/Commit id '?????' dropped the cache Is it commit id '19a148b7c8'?
Assume the callers do not have the cache by default and use a longer name for the internal ones that do.
This makes the shorter 'virDomainVirtioSerialAddrAutoAssign' name availabe for a function that will not require the cache. --- src/conf/domain_addr.c | 8 ++++---- src/conf/domain_addr.h | 8 ++++---- src/libvirt_private.syms | 2 +- src/qemu/qemu_domain_address.c | 6 ++++-- src/qemu/qemu_hotplug.c | 8 ++++---- 5 files changed, 17 insertions(+), 15 deletions(-)
ACK with the reference... John
This time do not require an address cache as a parameter. Simplify qemuDomainAttachChrDeviceAssignAddr to not generate the virtio serial address cache for devices of other types. --- src/conf/domain_addr.c | 21 +++++++++++++++++++++ src/conf/domain_addr.h | 6 ++++++ src/libvirt_private.syms | 1 + src/qemu/qemu_hotplug.c | 11 ++--------- 4 files changed, 30 insertions(+), 9 deletions(-) diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index 065baa7..92a5516 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -1167,6 +1167,27 @@ virDomainVirtioSerialAddrAutoAssignFromCache(virDomainDefPtr def, return virDomainVirtioSerialAddrAssign(def, addrs, info, allowZero, portOnly); } +int +virDomainVirtioSerialAddrAutoAssign(virDomainDefPtr def, + virDomainDeviceInfoPtr info, + bool allowZero) +{ + virDomainVirtioSerialAddrSetPtr addrs = NULL; + int ret = -1; + + if (!(addrs = virDomainVirtioSerialAddrSetCreateFromDomain(def))) + goto cleanup; + + if (virDomainVirtioSerialAddrAutoAssignFromCache(def, addrs, info, allowZero) < 0) + goto cleanup; + + ret = 0; + + cleanup: + virDomainVirtioSerialAddrSetFree(addrs); + return ret; +} + int virDomainVirtioSerialAddrAssign(virDomainDefPtr def, diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h index b0fadc1..141f83b 100644 --- a/src/conf/domain_addr.h +++ b/src/conf/domain_addr.h @@ -229,6 +229,12 @@ virDomainVirtioSerialAddrAutoAssignFromCache(virDomainDefPtr def, ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); int +virDomainVirtioSerialAddrAutoAssign(virDomainDefPtr def, + virDomainDeviceInfoPtr info, + bool allowZero) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + +int virDomainVirtioSerialAddrAssign(virDomainDefPtr def, virDomainVirtioSerialAddrSetPtr addrs, virDomainDeviceInfoPtr info, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 8bc2584..ce9c4c4 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -123,6 +123,7 @@ virDomainUSBAddressSetAddHub; virDomainUSBAddressSetCreate; virDomainUSBAddressSetFree; virDomainVirtioSerialAddrAssign; +virDomainVirtioSerialAddrAutoAssign; virDomainVirtioSerialAddrAutoAssignFromCache; virDomainVirtioSerialAddrIsComplete; virDomainVirtioSerialAddrRelease; diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 7bd38ab..cbdcd81 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1641,15 +1641,10 @@ qemuDomainAttachChrDeviceAssignAddr(virDomainDefPtr def, virDomainChrDefPtr chr) { int ret = -1; - virDomainVirtioSerialAddrSetPtr vioaddrs = NULL; - - if (!(vioaddrs = virDomainVirtioSerialAddrSetCreateFromDomain(def))) - goto cleanup; if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE && chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO) { - if (virDomainVirtioSerialAddrAutoAssignFromCache(NULL, vioaddrs, - &chr->info, true) < 0) + if (virDomainVirtioSerialAddrAutoAssign(def, &chr->info, true) < 0) goto cleanup; ret = 1; @@ -1667,8 +1662,7 @@ qemuDomainAttachChrDeviceAssignAddr(virDomainDefPtr def, } else if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL && chr->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO) { - if (virDomainVirtioSerialAddrAutoAssignFromCache(NULL, vioaddrs, - &chr->info, false) < 0) + if (virDomainVirtioSerialAddrAutoAssign(def, &chr->info, false) < 0) goto cleanup; ret = 1; } @@ -1686,7 +1680,6 @@ qemuDomainAttachChrDeviceAssignAddr(virDomainDefPtr def, ret = 0; cleanup: - virDomainVirtioSerialAddrSetFree(vioaddrs); return ret; } -- 2.7.3
On 10/21/2016 09:58 AM, Ján Tomko wrote:
This time do not require an address cache as a parameter.
Essentially reworking commit id '925fa4b90', right? Provide the reference...
Simplify qemuDomainAttachChrDeviceAssignAddr to not generate the virtio serial address cache for devices of other types. --- src/conf/domain_addr.c | 21 +++++++++++++++++++++ src/conf/domain_addr.h | 6 ++++++ src/libvirt_private.syms | 1 + src/qemu/qemu_hotplug.c | 11 ++--------- 4 files changed, 30 insertions(+), 9 deletions(-)
ACK w/ the reference John
This function should never need a cleanup section. --- src/qemu/qemu_hotplug.c | 28 ++++++++++------------------ 1 file changed, 10 insertions(+), 18 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index cbdcd81..407ae73 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1640,47 +1640,39 @@ qemuDomainAttachChrDeviceAssignAddr(virDomainDefPtr def, qemuDomainObjPrivatePtr priv, virDomainChrDefPtr chr) { - int ret = -1; - if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE && chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO) { if (virDomainVirtioSerialAddrAutoAssign(def, &chr->info, true) < 0) - goto cleanup; - ret = 1; + 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) - goto cleanup; - ret = 1; + return -1; + return 1; } else if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL && chr->targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_USB) { if (virDomainUSBAddressEnsure(priv->usbaddrs, &chr->info) < 0) - goto cleanup; - ret = 1; + 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(def, &chr->info, false) < 0) - goto cleanup; - ret = 1; + return -1; + return 1; } - if (ret == 1) - goto cleanup; - 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")); - goto cleanup; + return -1; } - ret = 0; - - cleanup: - return ret; + return 0; } int qemuDomainAttachChrDevice(virQEMUDriverPtr driver, -- 2.7.3
On 10/21/2016 09:58 AM, Ján Tomko wrote:
This function should never need a cleanup section. --- src/qemu/qemu_hotplug.c | 28 ++++++++++------------------ 1 file changed, 10 insertions(+), 18 deletions(-)
ACK John
When starting a new domain, we allocate the USB addresses and keep an address cache in the domain object's private data. However this data is lost on libvirtd restart. Also generate the address cache if all the addresses have been specified, so that devices hotplugged after libvirtd restart also get theirs assigned. https://bugzilla.redhat.com/show_bug.cgi?id=1387666 --- src/conf/domain_addr.c | 12 ++++++++++++ src/conf/domain_addr.h | 4 ++++ src/libvirt_private.syms | 1 + src/qemu/qemu_domain_address.c | 16 ++++++++++++++-- 4 files changed, 31 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index 92a5516..34fa01b 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -1797,6 +1797,18 @@ virDomainUSBAddressAssign(virDomainUSBAddressSetPtr addrs, int +virDomainUSBAddressPresent(virDomainDeviceInfoPtr info, + void *data ATTRIBUTE_UNUSED) +{ + if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB && + virDomainUSBAddressPortIsValid(info->addr.usb.port)) + return 0; + + return -1; +} + + +int virDomainUSBAddressReserve(virDomainDeviceInfoPtr info, void *data) { diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h index 141f83b..ee8dc04 100644 --- a/src/conf/domain_addr.h +++ b/src/conf/domain_addr.h @@ -299,6 +299,10 @@ virDomainUSBAddressCountAllPorts(virDomainDefPtr def); void virDomainUSBAddressSetFree(virDomainUSBAddressSetPtr addrs); int +virDomainUSBAddressPresent(virDomainDeviceInfoPtr info, + void *data) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); +int virDomainUSBAddressReserve(virDomainDeviceInfoPtr info, void *data) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index ce9c4c4..cbc97f7 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -116,6 +116,7 @@ virDomainUSBAddressEnsure; virDomainUSBAddressPortFormat; virDomainUSBAddressPortFormatBuf; virDomainUSBAddressPortIsValid; +virDomainUSBAddressPresent; virDomainUSBAddressRelease; virDomainUSBAddressReserve; virDomainUSBAddressSetAddControllers; diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 7d92e0b..a6001ad 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -1710,12 +1710,24 @@ qemuDomainUSBAddressAddHubs(virDomainDefPtr def) static int qemuDomainAssignUSBAddresses(virDomainDefPtr def, - virDomainObjPtr obj) + virDomainObjPtr obj, + bool newDomain) { int ret = -1; virDomainUSBAddressSetPtr addrs = NULL; qemuDomainObjPrivatePtr priv = NULL; + if (!newDomain) { + /* only create the address cache for: + * new domains + * domains that already have all the addresses specified + * otherwise libvirt's attempt to recreate the USB topology via + * QEMU command line might fail */ + if (virDomainUSBDeviceDefForeach(def, virDomainUSBAddressPresent, NULL, + false) < 0) + return 0; + } + if (!(addrs = virDomainUSBAddressSetCreate())) goto cleanup; @@ -1772,7 +1784,7 @@ qemuDomainAssignAddresses(virDomainDefPtr def, if (qemuDomainAssignPCIAddresses(def, qemuCaps, obj) < 0) return -1; - if (newDomain && qemuDomainAssignUSBAddresses(def, obj) < 0) + if (qemuDomainAssignUSBAddresses(def, obj, newDomain) < 0) return -1; return 0; -- 2.7.3
$SUBJ... The "Also" just seems misplaced. especially with the "Also generate the..." below... How about "At Reconnect, recreate the USB address cache" ? On 10/21/2016 09:58 AM, Ján Tomko wrote:
When starting a new domain, we allocate the USB addresses and keep an address cache in the domain object's private data.
However this data is lost on libvirtd restart.
The qemuProcessReconnect will call qemuDomainAssignAddresses w/ newDomain = false, so this code needs to "repopulate" the usbaddr cache properly, but can only properly do so if all the USB addresses for the domain were specified in the domain XML. In this path are we getting the status XML file which "should" have all the addresses, right?
Also generate the address cache if all the addresses have been specified, so that devices hotplugged after libvirtd restart also get theirs assigned.
https://bugzilla.redhat.com/show_bug.cgi?id=1387666 --- src/conf/domain_addr.c | 12 ++++++++++++ src/conf/domain_addr.h | 4 ++++ src/libvirt_private.syms | 1 + src/qemu/qemu_domain_address.c | 16 ++++++++++++++-- 4 files changed, 31 insertions(+), 2 deletions(-)
ACK for the patch, just validating/checking my assumptions on the commit message. John
On Tue, Oct 25, 2016 at 08:11:31AM -0400, John Ferlan wrote:
$SUBJ...
The "Also" just seems misplaced.
especially with the "Also generate the..." below...
Me fail English? That's unpossible!
How about "At Reconnect, recreate the USB address cache"
?
That is not exhaustive, but probably better as the commit summary. What I meant to say is that now the address cache is created not only for new domains, but also for those that already have all the addresses assigned. Whether they were assigned by the user, or by libvirt.
On 10/21/2016 09:58 AM, Ján Tomko wrote:
When starting a new domain, we allocate the USB addresses and keep an address cache in the domain object's private data.
However this data is lost on libvirtd restart.
The qemuProcessReconnect will call qemuDomainAssignAddresses w/ newDomain = false, so this code needs to "repopulate" the usbaddr cache properly, but can only properly do so if all the USB addresses for the domain were specified in the domain XML.
In this path are we getting the status XML file which "should" have all the addresses, right?
Not necessarily. The whole point of these conditions is to prevent libvirt from assigning USB addresses to the old domains that had at least one USB device without an address, because if we specify it on QEMU command line, QEMU might auto-add a hub that is not reflected in domain XML. So domains that were defined with libvirt new enough or that had their addresses assigned manually will get an address cache here, the rest will not.
Also generate the address cache if all the addresses have been specified, so that devices hotplugged after libvirtd restart also get theirs assigned.
https://bugzilla.redhat.com/show_bug.cgi?id=1387666 --- src/conf/domain_addr.c | 12 ++++++++++++ src/conf/domain_addr.h | 4 ++++ src/libvirt_private.syms | 1 + src/qemu/qemu_domain_address.c | 16 ++++++++++++++-- 4 files changed, 31 insertions(+), 2 deletions(-)
ACK for the patch, just validating/checking my assumptions on the commit message.
Thanks, I have rewritten the commit summary and pushed the series. Jan
For domains with no USB address cache, we should not attempt to generate a USB address. https://bugzilla.redhat.com/show_bug.cgi?id=1387665 --- src/qemu/qemu_hotplug.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 407ae73..9e9073d 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1652,7 +1652,8 @@ qemuDomainAttachChrDeviceAssignAddr(virDomainDefPtr def, return -1; return 1; - } else if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL && + } else if (priv->usbaddrs && + chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL && chr->targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_USB) { if (virDomainUSBAddressEnsure(priv->usbaddrs, &chr->info) < 0) return -1; -- 2.7.3
On 10/21/2016 09:58 AM, Ján Tomko wrote:
For domains with no USB address cache, we should not attempt to generate a USB address.
https://bugzilla.redhat.com/show_bug.cgi?id=1387665 --- src/qemu/qemu_hotplug.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
ACK John Yet another case where ATTRIBUTE_NONNULL(#arg) doesn't really work if the passed argument is assigned to NULL <sigh> (as opposed to the checks made if someone passed 'NULL' in the argument.
Return 0 instead of 1, so that qemuDomainAttachChrDevice does not assume the address neeeds to be released on error. No functional change, since qemuDomainReleaseDeviceAddress has been a noop for virtio serial addresses since the address cache was removed. --- src/qemu/qemu_hotplug.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 9e9073d..d432616 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1635,6 +1635,10 @@ qemuDomainChrRemove(virDomainDefPtr vmdef, return ret; } +/* Returns 1 if the address will need to be released later, + * -1 on error + * 0 otherwise + */ static int qemuDomainAttachChrDeviceAssignAddr(virDomainDefPtr def, qemuDomainObjPrivatePtr priv, @@ -1644,7 +1648,7 @@ qemuDomainAttachChrDeviceAssignAddr(virDomainDefPtr def, chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO) { if (virDomainVirtioSerialAddrAutoAssign(def, &chr->info, true) < 0) return -1; - return 1; + return 0; } else if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL && chr->targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_PCI) { @@ -1663,7 +1667,7 @@ qemuDomainAttachChrDeviceAssignAddr(virDomainDefPtr def, chr->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO) { if (virDomainVirtioSerialAddrAutoAssign(def, &chr->info, false) < 0) return -1; - return 1; + return 0; } if (chr->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_SERIAL || -- 2.7.3
On 10/21/2016 09:58 AM, Ján Tomko wrote:
Return 0 instead of 1, so that qemuDomainAttachChrDevice does not assume the address neeeds to be released on error.
No functional change, since qemuDomainReleaseDeviceAddress has been a noop for virtio serial addresses since the address cache was removed.
s/./ in commit id '19a148b7'. ACK, John
--- src/qemu/qemu_hotplug.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 9e9073d..d432616 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1635,6 +1635,10 @@ qemuDomainChrRemove(virDomainDefPtr vmdef, return ret; }
+/* Returns 1 if the address will need to be released later, + * -1 on error + * 0 otherwise + */
Something that should have been there from the start!
static int qemuDomainAttachChrDeviceAssignAddr(virDomainDefPtr def, qemuDomainObjPrivatePtr priv, @@ -1644,7 +1648,7 @@ qemuDomainAttachChrDeviceAssignAddr(virDomainDefPtr def, chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO) { if (virDomainVirtioSerialAddrAutoAssign(def, &chr->info, true) < 0) return -1; - return 1; + return 0;
} else if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL && chr->targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_PCI) { @@ -1663,7 +1667,7 @@ qemuDomainAttachChrDeviceAssignAddr(virDomainDefPtr def, chr->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO) { if (virDomainVirtioSerialAddrAutoAssign(def, &chr->info, false) < 0) return -1; - return 1; + return 0; }
if (chr->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_SERIAL ||
participants (2)
-
John Ferlan -
Ján Tomko