[libvirt] [PATCH v3 00/11] clean up after slot sharing patches
(rebase of the original patches to make review easier) Some of the patches that enabled sharing PCI slots by multiple pcie-root-ports made the idea of reserving an entire slot obsolete. To reduce confusion and misunderstandings, this patch series gets rid of the name "Slot" in all of the functions that reserve and release PCI addresses. Laine Stump (11): conf: fix fromConfig argument to virDomainPCIAddressReserveAddr() conf: fix fromConfig argument to virDomainPCIAddressValidate() conf: rename virDomainPCIAddressGetNextSlot() to ...GetNextAddr() conf: eliminate virDomainPCIAddressReserveNextSlot() qemu: replace virDomainPCIAddressReserveAddr with virDomainPCIAddressReserveSlot conf: make virDomainPCIAddressReserveAddr() a static function conf: rename virDomainPCIAddressReserveAddr() to ...Internal() conf: rename virDomainPCIAddressReserveSlot() to ...Addr() qemu: remove qemuDomainPCIAddressReserveNextAddr() qemu: rename qemuDomainPCIAddressReserveNextSlot() to ...Addr() conf: eliminate virDomainPCIAddressReleaseSlot() in favor of ...Addr() src/bhyve/bhyve_device.c | 26 ++++++---- src/conf/domain_addr.c | 60 +++++------------------ src/conf/domain_addr.h | 15 ------ src/libvirt_private.syms | 4 +- src/qemu/qemu_domain_address.c | 105 ++++++++++++++++++++--------------------- 5 files changed, 80 insertions(+), 130 deletions(-) -- 2.7.4
Although setting virDomainPCIAddressReserveAddr()'s fromConfig=true is correct when a PCI addres is coming from a domain's config, the *true* purpose of the fromConfig argument is to lower restrictions on what kind of device can plug into what kind of controller - if fromConfig is true, then a PCIE_DEVICE can plug into a slot that is marked as only compatible with PCI_DEVICE (and vice versa), and the HOTPLUG flag is ignored. For a long time there have been several calls to virDomainPCIAddressReserveAddr() that have fromConfig incorrectly set to false - it's correct that the addresses aren't coming from user config, but they are coming from hardcoded exceptions in libvirt that should, if anything, pay *even less* attention to following the pciConnectFlags (under the assumption that the libvirt programmer knew what they were doing). See commit b87703cf7 for an example of an actual bug caused by the incorrect setting of the "fromConfig" argument to virDomainPCIAddressReserveAddr(). Although they haven't resulted in any reported bugs, this patch corrects all the other incorrect settings of fromConfig in calls to virDomainPCIAddressReserveAddr(). --- src/conf/domain_addr.c | 2 +- src/qemu/qemu_domain_address.c | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index d60b1d9..babe2ad 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -591,7 +591,7 @@ virDomainPCIAddressReserveSlot(virDomainPCIAddressSetPtr addrs, virPCIDeviceAddressPtr addr, virDomainPCIConnectFlags flags) { - return virDomainPCIAddressReserveAddr(addrs, addr, flags, false); + return virDomainPCIAddressReserveAddr(addrs, addr, flags, true); } int diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 8e2ffe0..442d3ad 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -1303,7 +1303,7 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def, tmp_addr.slot = 0x1E; if (!virDomainPCIAddressSlotInUse(addrs, &tmp_addr)) { if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, - flags, false) < 0) + flags, true) < 0) goto cleanup; cont->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; cont->info.addr.pci.domain = 0; @@ -1327,12 +1327,12 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def, tmp_addr.function = 0; tmp_addr.multi = VIR_TRISTATE_SWITCH_ON; if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, - flags, false) < 0) + flags, true) < 0) goto cleanup; tmp_addr.function = 3; tmp_addr.multi = VIR_TRISTATE_SWITCH_ABSENT; if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, - flags, false) < 0) + flags, true) < 0) goto cleanup; } -- 2.7.4
On Mon, 2016-12-19 at 10:25 -0500, Laine Stump wrote:
Although setting virDomainPCIAddressReserveAddr()'s fromConfig=true is correct when a PCI addres is coming from a domain's config, the *true* purpose of the fromConfig argument is to lower restrictions on what kind of device can plug into what kind of controller - if fromConfig is true, then a PCIE_DEVICE can plug into a slot that is marked as only compatible with PCI_DEVICE (and vice versa), and the HOTPLUG flag is ignored. For a long time there have been several calls to virDomainPCIAddressReserveAddr() that have fromConfig incorrectly set to false - it's correct that the addresses aren't coming from user config, but they are coming from hardcoded exceptions in libvirt that should, if anything, pay *even less* attention to following the pciConnectFlags (under the assumption that the libvirt programmer knew what they were doing).
It seems to me that many issues with incorrect usage of the fromConfig parameter can be traced back to the fact that it's being assigned two related but not overlapping meanings: * the address being assigned or validated comes from the user configuration and error reporting should be worded accordingly * use more relaxed rules when assigning or validating the address, eg. allow legacy PCI devices to be plugged into PCIe slots and whatnot The former basically implies the latter, because we assume the user[1] knows what they are doing; however, the spots you're touching with this patch only call for the latter behavior. [...]
@@ -591,7 +591,7 @@ virDomainPCIAddressReserveSlot(virDomainPCIAddressSetPtr addrs, virPCIDeviceAddressPtr addr, virDomainPCIConnectFlags flags) { - return virDomainPCIAddressReserveAddr(addrs, addr, flags, false); + return virDomainPCIAddressReserveAddr(addrs, addr, flags, true); }
All uses of virDomainPCIAddressReserveSlot() in the QEMU driver are for devices we're adding ourselves with explicit PCI addresses, and those in the bhyve driver are for devices that come from the guest configuration, so this change should be safe. That said, it leads to a weird situation in which ReserveSlot() implies fromConfig=true and ReserveNextSlot() implies fromConfig=false, which is very confusing. I know you're going to rename and change both functions further down the series so it's not a big issue for now. ACK [1] Just like libvirt programmers ;) -- Andrea Bolognani / Red Hat / Virtualization
fromConfig should be true if the caller want virDomainPCIAddressValidate() to loosen restrictions on its interpretation of the pciConnectFlags. In particular, either PCI_DEVICE or PCIE_DEVICE will be counted as equivalent to both, and HOTPLUG will be ignored. In a few cases where libvirt was manually overriding automatic address assignment, it was setting fromConfig to false when validating the hardcoded manual override. This patch changes those to fromConfig=true as a preemptive strike against any future bugs that might otherwise surface. --- src/conf/domain_addr.c | 2 +- src/qemu/qemu_domain_address.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index babe2ad..50151ae 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -661,7 +661,7 @@ virDomainPCIAddressReleaseSlot(virDomainPCIAddressSetPtr addrs, if (!(addrStr = virDomainPCIAddressAsString(addr))) goto cleanup; - if (!virDomainPCIAddressValidate(addrs, addr, addrStr, flags, false)) + if (!virDomainPCIAddressValidate(addrs, addr, addrStr, flags, true)) goto cleanup; addrs->buses[addr->bus].slot[addr->slot].functions = 0; diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 442d3ad..59dbe22 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -1162,7 +1162,7 @@ qemuDomainValidateDevicePCISlotsPIIX3(virDomainDefPtr def, if (!(addrStr = virDomainPCIAddressAsString(&tmp_addr))) goto cleanup; if (!virDomainPCIAddressValidate(addrs, &tmp_addr, - addrStr, flags, false)) + addrStr, flags, true)) goto cleanup; if (virDomainPCIAddressSlotInUse(addrs, &tmp_addr)) { @@ -1351,7 +1351,7 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def, if (!(addrStr = virDomainPCIAddressAsString(&tmp_addr))) goto cleanup; if (!virDomainPCIAddressValidate(addrs, &tmp_addr, - addrStr, flags, false)) + addrStr, flags, true)) goto cleanup; if (virDomainPCIAddressSlotInUse(addrs, &tmp_addr)) { -- 2.7.4
With the advent of VIR_PCI_CONNECT_AGGREGATE_SLOT, the new name is more appropriate, since the address contained may be another address on the same slot as last time, not necessarily a new slot. --- src/conf/domain_addr.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index 50151ae..87ddd20 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -770,7 +770,7 @@ virDomainPCIAddressFindUnusedFunctionOnBus(virDomainPCIAddressBusPtr bus, static int ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) -virDomainPCIAddressGetNextSlot(virDomainPCIAddressSetPtr addrs, +virDomainPCIAddressGetNextAddr(virDomainPCIAddressSetPtr addrs, virPCIDeviceAddressPtr next_addr, int function, virDomainPCIConnectFlags flags) @@ -879,7 +879,7 @@ virDomainPCIAddressReserveNextAddr(virDomainPCIAddressSetPtr addrs, { virPCIDeviceAddress addr; - if (virDomainPCIAddressGetNextSlot(addrs, &addr, function, flags) < 0) + if (virDomainPCIAddressGetNextAddr(addrs, &addr, function, flags) < 0) return -1; if (virDomainPCIAddressReserveAddr(addrs, &addr, flags, false) < 0) -- 2.7.4
On Mon, 2016-12-19 at 10:25 -0500, Laine Stump wrote:
With the advent of VIR_PCI_CONNECT_AGGREGATE_SLOT, the new name is more appropriate, since the address contained may be another address
s/contained/returned/ ?
on the same slot as last time, not necessarily a new slot.
ACK -- Andrea Bolognani / Red Hat / Virtualization
Since we don't actually reserve an entire slot at a time anymore, the name of this function is just confusing, and it's almost identical in operation to virDomainPCIAddressReserveNextAddr() anyway, so remove the *Slot() function and replace calls to it with calls to *Addr(..., -1). --- src/bhyve/bhyve_device.c | 18 ++++++++++-------- src/conf/domain_addr.c | 11 +---------- src/conf/domain_addr.h | 5 ----- src/libvirt_private.syms | 1 - 4 files changed, 11 insertions(+), 24 deletions(-) diff --git a/src/bhyve/bhyve_device.c b/src/bhyve/bhyve_device.c index ca30e9f..4a1e1bd 100644 --- a/src/bhyve/bhyve_device.c +++ b/src/bhyve/bhyve_device.c @@ -100,19 +100,20 @@ bhyveAssignDevicePCISlots(virDomainDefPtr def, for (i = 0; i < def->nnets; i++) { if (!virDeviceInfoPCIAddressWanted(&def->nets[i]->info)) continue; - if (virDomainPCIAddressReserveNextSlot(addrs, - &def->nets[i]->info, - VIR_PCI_CONNECT_TYPE_PCI_DEVICE) < 0) + if (virDomainPCIAddressReserveNextAddr(addrs, &def->nets[i]->info, + VIR_PCI_CONNECT_TYPE_PCI_DEVICE, + -1) < 0) { goto error; + } } for (i = 0; i < def->ndisks; i++) { if (def->disks[i]->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI && !virPCIDeviceAddressIsEmpty(&def->disks[i]->info.addr.pci)) continue; - if (virDomainPCIAddressReserveNextSlot(addrs, - &def->disks[i]->info, - VIR_PCI_CONNECT_TYPE_PCI_DEVICE) < 0) + if (virDomainPCIAddressReserveNextAddr(addrs, &def->disks[i]->info, + VIR_PCI_CONNECT_TYPE_PCI_DEVICE, + -1) < 0) goto error; } @@ -122,9 +123,10 @@ bhyveAssignDevicePCISlots(virDomainDefPtr def, !virDeviceInfoPCIAddressWanted(&def->controllers[i]->info)) continue; - if (virDomainPCIAddressReserveNextSlot(addrs, + if (virDomainPCIAddressReserveNextAddr(addrs, &def->controllers[i]->info, - VIR_PCI_CONNECT_TYPE_PCI_DEVICE) < 0) + VIR_PCI_CONNECT_TYPE_PCI_DEVICE, + -1) < 0) goto error; } diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index 87ddd20..b02d52c 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -629,7 +629,7 @@ virDomainPCIAddressEnsureAddr(virDomainPCIAddressSetPtr addrs, ret = virDomainPCIAddressReserveAddr(addrs, &dev->addr.pci, flags, true); } else { - ret = virDomainPCIAddressReserveNextSlot(addrs, dev, flags); + ret = virDomainPCIAddressReserveNextAddr(addrs, dev, flags, -1); } cleanup: @@ -897,15 +897,6 @@ virDomainPCIAddressReserveNextAddr(virDomainPCIAddressSetPtr addrs, } -int -virDomainPCIAddressReserveNextSlot(virDomainPCIAddressSetPtr addrs, - virDomainDeviceInfoPtr dev, - virDomainPCIConnectFlags flags) -{ - return virDomainPCIAddressReserveNextAddr(addrs, dev, flags, -1); -} - - typedef struct { virPCIDeviceAddressPtr addr; bool isMulti; diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h index dd4cd5b..be13b1f 100644 --- a/src/conf/domain_addr.h +++ b/src/conf/domain_addr.h @@ -180,11 +180,6 @@ int virDomainPCIAddressReserveNextAddr(virDomainPCIAddressSetPtr addrs, int function) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); -int virDomainPCIAddressReserveNextSlot(virDomainPCIAddressSetPtr addrs, - virDomainDeviceInfoPtr dev, - virDomainPCIConnectFlags flags) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); - bool virDomainPCIAddressIsMulti(const virDomainDef *def, virPCIDeviceAddressPtr addr) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 0dd0af5..fbc00f9 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -103,7 +103,6 @@ virDomainPCIAddressIsMulti; virDomainPCIAddressReleaseSlot; virDomainPCIAddressReserveAddr; virDomainPCIAddressReserveNextAddr; -virDomainPCIAddressReserveNextSlot; virDomainPCIAddressReserveSlot; virDomainPCIAddressSetAlloc; virDomainPCIAddressSetFree; -- 2.7.4
On Mon, 2016-12-19 at 10:25 -0500, Laine Stump wrote:
Since we don't actually reserve an entire slot at a time anymore, the name of this function is just confusing, and it's almost identical in operation to virDomainPCIAddressReserveNextAddr() anyway, so remove the *Slot() function and replace calls to it with calls to *Addr(..., -1).
ACK -- Andrea Bolognani / Red Hat / Virtualization
All occurences of the former use fromConfig=true, and that's exactly how virDomainPCIAddressReserveSlot() calls virDomainPCIaddressReserveAddr(), so just use *Slot() so that *Addr() can be made static to conf/domain_addr.c (both functions will be renamed in upcoming patches). --- src/qemu/qemu_domain_address.c | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 59dbe22..9a78504 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -989,8 +989,8 @@ qemuDomainCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED, } } - if (virDomainPCIAddressReserveAddr(addrs, addr, - info->pciConnectFlags, true) < 0) { + if (virDomainPCIAddressReserveSlot(addrs, addr, + info->pciConnectFlags) < 0) { goto cleanup; } @@ -1279,9 +1279,11 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def, assign = true; } if (assign) { - if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, - flags, true) < 0) + if (virDomainPCIAddressReserveSlot(addrs, + &tmp_addr, flags) < 0) { goto cleanup; + } + cont->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; cont->info.addr.pci.domain = 0; cont->info.addr.pci.bus = 0; @@ -1302,9 +1304,11 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def, memset(&tmp_addr, 0, sizeof(tmp_addr)); tmp_addr.slot = 0x1E; if (!virDomainPCIAddressSlotInUse(addrs, &tmp_addr)) { - if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, - flags, true) < 0) + if (virDomainPCIAddressReserveSlot(addrs, + &tmp_addr, flags) < 0) { goto cleanup; + } + cont->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; cont->info.addr.pci.domain = 0; cont->info.addr.pci.bus = 0; @@ -1326,13 +1330,12 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def, tmp_addr.slot = 0x1F; tmp_addr.function = 0; tmp_addr.multi = VIR_TRISTATE_SWITCH_ON; - if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, - flags, true) < 0) + if (virDomainPCIAddressReserveSlot(addrs, &tmp_addr, flags) < 0) goto cleanup; + tmp_addr.function = 3; tmp_addr.multi = VIR_TRISTATE_SWITCH_ABSENT; - if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, - flags, true) < 0) + if (virDomainPCIAddressReserveSlot(addrs, &tmp_addr, flags) < 0) goto cleanup; } @@ -1630,10 +1633,10 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def, if (foundAddr) { /* Reserve this function on the slot we found */ - if (virDomainPCIAddressReserveAddr(addrs, &addr, - cont->info.pciConnectFlags, - true) < 0) + if (virDomainPCIAddressReserveSlot(addrs, &addr, + cont->info.pciConnectFlags) < 0) { goto error; + } cont->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; cont->info.addr.pci = addr; -- 2.7.4
On Mon, 2016-12-19 at 10:25 -0500, Laine Stump wrote:
All occurences of the former use fromConfig=true, and that's exactly how virDomainPCIAddressReserveSlot() calls virDomainPCIaddressReserveAddr(), so just use *Slot() so that *Addr() can be made static to conf/domain_addr.c (both functions will be renamed in upcoming patches).
ACK -- Andrea Bolognani / Red Hat / Virtualization
It is now only used in domain_addr.c. --- src/conf/domain_addr.c | 2 +- src/conf/domain_addr.h | 6 ------ src/libvirt_private.syms | 1 - 3 files changed, 1 insertion(+), 8 deletions(-) diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index b02d52c..bb51579 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -533,7 +533,7 @@ virDomainPCIAddressSlotInUse(virDomainPCIAddressSetPtr addrs, * automatically created by libvirt, so it is an internal error (not * XML). */ -int +static int ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) virDomainPCIAddressReserveAddr(virDomainPCIAddressSetPtr addrs, virPCIDeviceAddressPtr addr, virDomainPCIConnectFlags flags, diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h index be13b1f..2412ed3 100644 --- a/src/conf/domain_addr.h +++ b/src/conf/domain_addr.h @@ -150,12 +150,6 @@ int virDomainPCIAddressSetGrow(virDomainPCIAddressSetPtr addrs, virDomainPCIConnectFlags flags) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); -int virDomainPCIAddressReserveAddr(virDomainPCIAddressSetPtr addrs, - virPCIDeviceAddressPtr addr, - virDomainPCIConnectFlags flags, - bool fromConfig) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); - int virDomainPCIAddressReserveSlot(virDomainPCIAddressSetPtr addrs, virPCIDeviceAddressPtr addr, virDomainPCIConnectFlags flags) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index fbc00f9..04823d2 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -101,7 +101,6 @@ virDomainPCIAddressEnsureAddr; virDomainPCIAddressFlagsCompatible; virDomainPCIAddressIsMulti; virDomainPCIAddressReleaseSlot; -virDomainPCIAddressReserveAddr; virDomainPCIAddressReserveNextAddr; virDomainPCIAddressReserveSlot; virDomainPCIAddressSetAlloc; -- 2.7.4
This is in preparation for renaming virDomainPCIAddressReserveSlot() to virDomainPCIAddressReserveAddr(), which is a better description of what it does. --- src/conf/domain_addr.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index bb51579..24e3c7d 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -534,10 +534,10 @@ virDomainPCIAddressSlotInUse(virDomainPCIAddressSetPtr addrs, * XML). */ static int ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) -virDomainPCIAddressReserveAddr(virDomainPCIAddressSetPtr addrs, - virPCIDeviceAddressPtr addr, - virDomainPCIConnectFlags flags, - bool fromConfig) +virDomainPCIAddressReserveAddrInternal(virDomainPCIAddressSetPtr addrs, + virPCIDeviceAddressPtr addr, + virDomainPCIConnectFlags flags, + bool fromConfig) { int ret = -1; char *addrStr = NULL; @@ -591,7 +591,7 @@ virDomainPCIAddressReserveSlot(virDomainPCIAddressSetPtr addrs, virPCIDeviceAddressPtr addr, virDomainPCIConnectFlags flags) { - return virDomainPCIAddressReserveAddr(addrs, addr, flags, true); + return virDomainPCIAddressReserveAddrInternal(addrs, addr, flags, true); } int @@ -626,8 +626,8 @@ virDomainPCIAddressEnsureAddr(virDomainPCIAddressSetPtr addrs, addrStr, flags, true)) goto cleanup; - ret = virDomainPCIAddressReserveAddr(addrs, &dev->addr.pci, - flags, true); + ret = virDomainPCIAddressReserveAddrInternal(addrs, &dev->addr.pci, + flags, true); } else { ret = virDomainPCIAddressReserveNextAddr(addrs, dev, flags, -1); } @@ -882,7 +882,7 @@ virDomainPCIAddressReserveNextAddr(virDomainPCIAddressSetPtr addrs, if (virDomainPCIAddressGetNextAddr(addrs, &addr, function, flags) < 0) return -1; - if (virDomainPCIAddressReserveAddr(addrs, &addr, flags, false) < 0) + if (virDomainPCIAddressReserveAddrInternal(addrs, &addr, flags, false) < 0) return -1; addrs->lastaddr = addr; -- 2.7.4
On Mon, 2016-12-19 at 10:25 -0500, Laine Stump wrote:
This is in preparation for renaming virDomainPCIAddressReserveSlot() to virDomainPCIAddressReserveAddr(), which is a better description of what it does.
ACK -- Andrea Bolognani / Red Hat / Virtualization
This function doesn't actually reserve an entire slot any more, it reserves a single PCI address, so this name is more appropriate. --- src/bhyve/bhyve_device.c | 8 ++++++-- src/conf/domain_addr.c | 2 +- src/conf/domain_addr.h | 2 +- src/libvirt_private.syms | 2 +- src/qemu/qemu_domain_address.c | 24 ++++++++++++------------ 5 files changed, 21 insertions(+), 17 deletions(-) diff --git a/src/bhyve/bhyve_device.c b/src/bhyve/bhyve_device.c index 4a1e1bd..97e81e1 100644 --- a/src/bhyve/bhyve_device.c +++ b/src/bhyve/bhyve_device.c @@ -53,8 +53,10 @@ bhyveCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED, } } - if (virDomainPCIAddressReserveSlot(addrs, addr, VIR_PCI_CONNECT_TYPE_PCI_DEVICE) < 0) + if (virDomainPCIAddressReserveAddr(addrs, addr, + VIR_PCI_CONNECT_TYPE_PCI_DEVICE) < 0) { goto cleanup; + } ret = 0; cleanup: @@ -94,8 +96,10 @@ bhyveAssignDevicePCISlots(virDomainDefPtr def, memset(&lpc_addr, 0, sizeof(lpc_addr)); lpc_addr.slot = 0x1; - if (virDomainPCIAddressReserveSlot(addrs, &lpc_addr, VIR_PCI_CONNECT_TYPE_PCI_DEVICE) < 0) + if (virDomainPCIAddressReserveAddr(addrs, &lpc_addr, + VIR_PCI_CONNECT_TYPE_PCI_DEVICE) < 0) { goto error; + } for (i = 0; i < def->nnets; i++) { if (!virDeviceInfoPCIAddressWanted(&def->nets[i]->info)) diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index 24e3c7d..345c274 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -587,7 +587,7 @@ virDomainPCIAddressReserveAddrInternal(virDomainPCIAddressSetPtr addrs, int -virDomainPCIAddressReserveSlot(virDomainPCIAddressSetPtr addrs, +virDomainPCIAddressReserveAddr(virDomainPCIAddressSetPtr addrs, virPCIDeviceAddressPtr addr, virDomainPCIConnectFlags flags) { diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h index 2412ed3..dac73fa 100644 --- a/src/conf/domain_addr.h +++ b/src/conf/domain_addr.h @@ -150,7 +150,7 @@ int virDomainPCIAddressSetGrow(virDomainPCIAddressSetPtr addrs, virDomainPCIConnectFlags flags) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); -int virDomainPCIAddressReserveSlot(virDomainPCIAddressSetPtr addrs, +int virDomainPCIAddressReserveAddr(virDomainPCIAddressSetPtr addrs, virPCIDeviceAddressPtr addr, virDomainPCIConnectFlags flags) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 04823d2..1f54ea4 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -101,8 +101,8 @@ virDomainPCIAddressEnsureAddr; virDomainPCIAddressFlagsCompatible; virDomainPCIAddressIsMulti; virDomainPCIAddressReleaseSlot; +virDomainPCIAddressReserveAddr; virDomainPCIAddressReserveNextAddr; -virDomainPCIAddressReserveSlot; virDomainPCIAddressSetAlloc; virDomainPCIAddressSetFree; virDomainPCIAddressSetGrow; diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 9a78504..4673cc7 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -989,7 +989,7 @@ qemuDomainCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED, } } - if (virDomainPCIAddressReserveSlot(addrs, addr, + if (virDomainPCIAddressReserveAddr(addrs, addr, info->pciConnectFlags) < 0) { goto cleanup; } @@ -1143,7 +1143,7 @@ qemuDomainValidateDevicePCISlotsPIIX3(virDomainDefPtr def, if (addrs->nbuses) { memset(&tmp_addr, 0, sizeof(tmp_addr)); tmp_addr.slot = 1; - if (virDomainPCIAddressReserveSlot(addrs, &tmp_addr, flags) < 0) + if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags) < 0) goto cleanup; } @@ -1178,7 +1178,7 @@ qemuDomainValidateDevicePCISlotsPIIX3(virDomainDefPtr def, goto cleanup; } } else { - if (virDomainPCIAddressReserveSlot(addrs, &tmp_addr, flags) < 0) + if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags) < 0) goto cleanup; primaryVideo->info.addr.pci = tmp_addr; primaryVideo->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; @@ -1203,7 +1203,7 @@ qemuDomainValidateDevicePCISlotsPIIX3(virDomainDefPtr def, VIR_DEBUG("PCI address 0:0:2.0 in use, future addition of a video" " device will not be possible without manual" " intervention"); - } else if (virDomainPCIAddressReserveSlot(addrs, &tmp_addr, flags) < 0) { + } else if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags) < 0) { goto cleanup; } } @@ -1279,7 +1279,7 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def, assign = true; } if (assign) { - if (virDomainPCIAddressReserveSlot(addrs, + if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags) < 0) { goto cleanup; } @@ -1304,7 +1304,7 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def, memset(&tmp_addr, 0, sizeof(tmp_addr)); tmp_addr.slot = 0x1E; if (!virDomainPCIAddressSlotInUse(addrs, &tmp_addr)) { - if (virDomainPCIAddressReserveSlot(addrs, + if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags) < 0) { goto cleanup; } @@ -1330,12 +1330,12 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def, tmp_addr.slot = 0x1F; tmp_addr.function = 0; tmp_addr.multi = VIR_TRISTATE_SWITCH_ON; - if (virDomainPCIAddressReserveSlot(addrs, &tmp_addr, flags) < 0) + if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags) < 0) goto cleanup; tmp_addr.function = 3; tmp_addr.multi = VIR_TRISTATE_SWITCH_ABSENT; - if (virDomainPCIAddressReserveSlot(addrs, &tmp_addr, flags) < 0) + if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags) < 0) goto cleanup; } @@ -1369,7 +1369,7 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def, goto cleanup; } } else { - if (virDomainPCIAddressReserveSlot(addrs, &tmp_addr, flags) < 0) + if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags) < 0) goto cleanup; primaryVideo->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; primaryVideo->info.addr.pci = tmp_addr; @@ -1395,7 +1395,7 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def, " device will not be possible without manual" " intervention"); virResetLastError(); - } else if (virDomainPCIAddressReserveSlot(addrs, + } else if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags) < 0) { goto cleanup; } @@ -1417,7 +1417,7 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def, !virDeviceInfoPCIAddressWanted(&sound->info)) { continue; } - if (virDomainPCIAddressReserveSlot(addrs, &tmp_addr, flags) < 0) + if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags) < 0) goto cleanup; sound->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; @@ -1633,7 +1633,7 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def, if (foundAddr) { /* Reserve this function on the slot we found */ - if (virDomainPCIAddressReserveSlot(addrs, &addr, + if (virDomainPCIAddressReserveAddr(addrs, &addr, cont->info.pciConnectFlags) < 0) { goto error; } -- 2.7.4
On Mon, 2016-12-19 at 10:25 -0500, Laine Stump wrote:
This function doesn't actually reserve an entire slot any more, it reserves a single PCI address, so this name is more appropriate.
ACK -- Andrea Bolognani / Red Hat / Virtualization
This function is only called in two places, and the function itself is just adding a single argument and calling virDomainPCIAddressReserveNextAddr(), so we can remove it and instead call virDomainPCIAddressReserveNextAddr() directly. (The main motivation for doing this is to free up the name so that qemuDomainPCIAddressReserveNextSlot() can be renamed in the next patch, as its current name is now inaccurate and misleading). --- src/qemu/qemu_domain_address.c | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 4673cc7..a9ea266 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -890,21 +890,11 @@ qemuDomainFillDevicePCIConnectFlags(virDomainDefPtr def, static int -qemuDomainPCIAddressReserveNextAddr(virDomainPCIAddressSetPtr addrs, - virDomainDeviceInfoPtr dev, - unsigned int function) -{ - return virDomainPCIAddressReserveNextAddr(addrs, dev, - dev->pciConnectFlags, - function); -} - - -static int qemuDomainPCIAddressReserveNextSlot(virDomainPCIAddressSetPtr addrs, virDomainDeviceInfoPtr dev) { - return qemuDomainPCIAddressReserveNextAddr(addrs, dev, -1); + return virDomainPCIAddressReserveNextAddr(addrs, dev, + dev->pciConnectFlags, -1); } @@ -1643,8 +1633,9 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def, } else { /* This is the first part of the controller, so need * to find a free slot & then reserve this function */ - if (qemuDomainPCIAddressReserveNextAddr(addrs, &cont->info, - addr.function) < 0) { + if (virDomainPCIAddressReserveNextAddr(addrs, &cont->info, + cont->info.pciConnectFlags, + addr.function) < 0) { goto error; } -- 2.7.4
On Mon, 2016-12-19 at 10:25 -0500, Laine Stump wrote:
This function is only called in two places, and the function itself is just adding a single argument and calling virDomainPCIAddressReserveNextAddr(), so we can remove it and instead call virDomainPCIAddressReserveNextAddr() directly.
Empty line here.
(The main motivation for doing this is to free up the name so that qemuDomainPCIAddressReserveNextSlot() can be renamed in the next patch, as its current name is now inaccurate and misleading).
You can also lose the parentheses around the paragraph. ACK -- Andrea Bolognani / Red Hat / Virtualization
This function doesn't actually reserve an entire slot any more, it reserves a single PCI address, so this name is more appropriate. --- src/qemu/qemu_domain_address.c | 45 +++++++++++++++++++++--------------------- 1 file changed, 23 insertions(+), 22 deletions(-) diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index a9ea266..e6bba8a 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -890,7 +890,7 @@ qemuDomainFillDevicePCIConnectFlags(virDomainDefPtr def, static int -qemuDomainPCIAddressReserveNextSlot(virDomainPCIAddressSetPtr addrs, +qemuDomainPCIAddressReserveNextAddr(virDomainPCIAddressSetPtr addrs, virDomainDeviceInfoPtr dev) { return virDomainPCIAddressReserveNextAddr(addrs, dev, @@ -1157,7 +1157,7 @@ qemuDomainValidateDevicePCISlotsPIIX3(virDomainDefPtr def, if (virDomainPCIAddressSlotInUse(addrs, &tmp_addr)) { if (qemuDeviceVideoUsable) { - if (qemuDomainPCIAddressReserveNextSlot(addrs, + if (qemuDomainPCIAddressReserveNextAddr(addrs, &primaryVideo->info) < 0) { goto cleanup; } @@ -1349,7 +1349,7 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def, if (virDomainPCIAddressSlotInUse(addrs, &tmp_addr)) { if (qemuDeviceVideoUsable) { - if (qemuDomainPCIAddressReserveNextSlot(addrs, + if (qemuDomainPCIAddressReserveNextAddr(addrs, &primaryVideo->info) < 0) goto cleanup; } else { @@ -1510,7 +1510,7 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def, !virDeviceInfoPCIAddressWanted(&cont->info)) continue; - if (qemuDomainPCIAddressReserveNextSlot(addrs, &cont->info) < 0) + if (qemuDomainPCIAddressReserveNextAddr(addrs, &cont->info) < 0) goto error; } } @@ -1521,7 +1521,7 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def, /* Only support VirtIO-9p-pci so far. If that changes, * we might need to skip devices here */ - if (qemuDomainPCIAddressReserveNextSlot(addrs, &def->fss[i]->info) < 0) + if (qemuDomainPCIAddressReserveNextAddr(addrs, &def->fss[i]->info) < 0) goto error; } @@ -1538,7 +1538,7 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def, continue; } - if (qemuDomainPCIAddressReserveNextSlot(addrs, &net->info) < 0) + if (qemuDomainPCIAddressReserveNextAddr(addrs, &net->info) < 0) goto error; } @@ -1556,7 +1556,7 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def, continue; } - if (qemuDomainPCIAddressReserveNextSlot(addrs, &sound->info) < 0) + if (qemuDomainPCIAddressReserveNextAddr(addrs, &sound->info) < 0) goto error; } @@ -1642,7 +1642,7 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def, cont->info.addr.pci.multi = addr.multi; } } else { - if (qemuDomainPCIAddressReserveNextSlot(addrs, &cont->info) < 0) + if (qemuDomainPCIAddressReserveNextAddr(addrs, &cont->info) < 0) goto error; } } @@ -1674,7 +1674,7 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def, goto error; } - if (qemuDomainPCIAddressReserveNextSlot(addrs, &def->disks[i]->info) < 0) + if (qemuDomainPCIAddressReserveNextAddr(addrs, &def->disks[i]->info) < 0) goto error; } @@ -1688,7 +1688,7 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def, def->hostdevs[i]->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI_HOST) continue; - if (qemuDomainPCIAddressReserveNextSlot(addrs, + if (qemuDomainPCIAddressReserveNextAddr(addrs, def->hostdevs[i]->info) < 0) goto error; } @@ -1698,7 +1698,7 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def, def->memballoon->model == VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO && virDeviceInfoPCIAddressWanted(&def->memballoon->info)) { - if (qemuDomainPCIAddressReserveNextSlot(addrs, + if (qemuDomainPCIAddressReserveNextAddr(addrs, &def->memballoon->info) < 0) goto error; } @@ -1709,7 +1709,7 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def, !virDeviceInfoPCIAddressWanted(&def->rngs[i]->info)) continue; - if (qemuDomainPCIAddressReserveNextSlot(addrs, &def->rngs[i]->info) < 0) + if (qemuDomainPCIAddressReserveNextAddr(addrs, &def->rngs[i]->info) < 0) goto error; } @@ -1717,7 +1717,7 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def, if (def->watchdog && def->watchdog->model == VIR_DOMAIN_WATCHDOG_MODEL_I6300ESB && virDeviceInfoPCIAddressWanted(&def->watchdog->info)) { - if (qemuDomainPCIAddressReserveNextSlot(addrs, &def->watchdog->info) < 0) + if (qemuDomainPCIAddressReserveNextAddr(addrs, &def->watchdog->info) < 0) goto error; } @@ -1725,7 +1725,7 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def, * assigned address. */ if (def->nvideos > 0 && virDeviceInfoPCIAddressWanted(&def->videos[0]->info)) { - if (qemuDomainPCIAddressReserveNextSlot(addrs, &def->videos[0]->info) < 0) + if (qemuDomainPCIAddressReserveNextAddr(addrs, &def->videos[0]->info) < 0) goto error; } @@ -1733,7 +1733,7 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def, if (!virDeviceInfoPCIAddressWanted(&def->videos[i]->info)) continue; - if (qemuDomainPCIAddressReserveNextSlot(addrs, &def->videos[i]->info) < 0) + if (qemuDomainPCIAddressReserveNextAddr(addrs, &def->videos[i]->info) < 0) goto error; } @@ -1742,7 +1742,7 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def, if (!virDeviceInfoPCIAddressWanted(&def->shmems[i]->info)) continue; - if (qemuDomainPCIAddressReserveNextSlot(addrs, &def->shmems[i]->info) < 0) + if (qemuDomainPCIAddressReserveNextAddr(addrs, &def->shmems[i]->info) < 0) goto error; } for (i = 0; i < def->ninputs; i++) { @@ -1750,7 +1750,7 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def, !virDeviceInfoPCIAddressWanted(&def->inputs[i]->info)) continue; - if (qemuDomainPCIAddressReserveNextSlot(addrs, &def->inputs[i]->info) < 0) + if (qemuDomainPCIAddressReserveNextAddr(addrs, &def->inputs[i]->info) < 0) goto error; } for (i = 0; i < def->nparallels; i++) { @@ -1763,7 +1763,7 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def, !virDeviceInfoPCIAddressWanted(&chr->info)) continue; - if (qemuDomainPCIAddressReserveNextSlot(addrs, &chr->info) < 0) + if (qemuDomainPCIAddressReserveNextAddr(addrs, &chr->info) < 0) goto error; } for (i = 0; i < def->nchannels; i++) { @@ -1983,7 +1983,7 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def, } } if (!buses_reserved && - qemuDomainPCIAddressReserveNextSlot(addrs, &info) < 0) + qemuDomainPCIAddressReserveNextAddr(addrs, &info) < 0) goto cleanup; } @@ -2015,7 +2015,7 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def, /* if there isn't an empty pcie-root-port, this will * cause one to be added */ - if (qemuDomainPCIAddressReserveNextSlot(addrs, &info) < 0) + if (qemuDomainPCIAddressReserveNextAddr(addrs, &info) < 0) goto cleanup; } @@ -2055,9 +2055,10 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def, dev.data.controller = def->controllers[contIndex]; /* set connect flags so it will be properly addressed */ qemuDomainFillDevicePCIConnectFlags(def, &dev, qemuCaps, driver); - if (qemuDomainPCIAddressReserveNextSlot(addrs, - &dev.data.controller->info) < 0) + if (qemuDomainPCIAddressReserveNextAddr(addrs, + &dev.data.controller->info) < 0) { goto cleanup; + } } nbuses = addrs->nbuses; -- 2.7.4
On Mon, 2016-12-19 at 10:25 -0500, Laine Stump wrote:
This function doesn't actually reserve an entire slot any more, it reserves a single PCI address, so this name is more appropriate.
ACK -- Andrea Bolognani / Red Hat / Virtualization
Surprisingly there was a virDomainPCIAddressReleaseAddr() function already, but it was completely unused. Since we don't reserve entire slots at once any more, there is no need to release entire slots either, so we just replace the single call to virDomainPCIAddressReleaseSlot() with a call to virDomainPCIAddressReleaseAddr() and remove the now unused function. The keen observer may be concerned that ...Addr() doesn't call virDomainPCIAddressValidate(), as ...Slot() did. But really the validation was pointless anyway - if the device hadn't been suitable to be connected at that address, it would have failed validation before every being reserved in the first place, so by definition it will pass validation when it is being unplugged. (And anyway, even if something "bad" happened and we managed to have a device incorrectly at the given address, we would still want to be able to free it up for use by a device that *did* validate properly). --- src/conf/domain_addr.c | 25 ------------------------- src/conf/domain_addr.h | 4 ---- src/libvirt_private.syms | 2 +- src/qemu/qemu_domain_address.c | 2 +- 4 files changed, 2 insertions(+), 31 deletions(-) diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index 345c274..ebcc494 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -647,31 +647,6 @@ virDomainPCIAddressReleaseAddr(virDomainPCIAddressSetPtr addrs, return 0; } -int -virDomainPCIAddressReleaseSlot(virDomainPCIAddressSetPtr addrs, - virPCIDeviceAddressPtr addr) -{ - /* permit any kind of connection type in validation, since we - * already had it, and are giving it back. - */ - virDomainPCIConnectFlags flags = VIR_PCI_CONNECT_TYPES_MASK; - int ret = -1; - char *addrStr = NULL; - - if (!(addrStr = virDomainPCIAddressAsString(addr))) - goto cleanup; - - if (!virDomainPCIAddressValidate(addrs, addr, addrStr, flags, true)) - goto cleanup; - - addrs->buses[addr->bus].slot[addr->slot].functions = 0; - ret = 0; - cleanup: - VIR_FREE(addrStr); - return ret; -} - - virDomainPCIAddressSetPtr virDomainPCIAddressSetAlloc(unsigned int nbuses) { diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h index dac73fa..ee44c50 100644 --- a/src/conf/domain_addr.h +++ b/src/conf/domain_addr.h @@ -164,10 +164,6 @@ int virDomainPCIAddressReleaseAddr(virDomainPCIAddressSetPtr addrs, virPCIDeviceAddressPtr addr) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); -int virDomainPCIAddressReleaseSlot(virDomainPCIAddressSetPtr addrs, - virPCIDeviceAddressPtr addr) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); - int virDomainPCIAddressReserveNextAddr(virDomainPCIAddressSetPtr addrs, virDomainDeviceInfoPtr dev, virDomainPCIConnectFlags flags, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 1f54ea4..7607faa 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -100,7 +100,7 @@ virDomainPCIAddressBusSetModel; virDomainPCIAddressEnsureAddr; virDomainPCIAddressFlagsCompatible; virDomainPCIAddressIsMulti; -virDomainPCIAddressReleaseSlot; +virDomainPCIAddressReleaseAddr; virDomainPCIAddressReserveAddr; virDomainPCIAddressReserveNextAddr; virDomainPCIAddressSetAlloc; diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index e6bba8a..89bb794 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -2495,7 +2495,7 @@ qemuDomainReleaseDeviceAddress(virDomainObjPtr vm, devstr = info->alias; if (virDeviceInfoPCIAddressPresent(info) && - virDomainPCIAddressReleaseSlot(priv->pciaddrs, + virDomainPCIAddressReleaseAddr(priv->pciaddrs, &info->addr.pci) < 0) VIR_WARN("Unable to release PCI address on %s", NULLSTR(devstr)); -- 2.7.4
On Mon, 2016-12-19 at 10:25 -0500, Laine Stump wrote:
Surprisingly there was a virDomainPCIAddressReleaseAddr() function already, but it was completely unused. Since we don't reserve entire slots at once any more, there is no need to release entire slots either, so we just replace the single call to virDomainPCIAddressReleaseSlot() with a call to virDomainPCIAddressReleaseAddr() and remove the now unused function.
ACK -- Andrea Bolognani / Red Hat / Virtualization
participants (2)
-
Andrea Bolognani -
Laine Stump