[libvirt] [PATCH v2 2/8] conf: eliminate concept of "reserveEntireSlot"

setting reserveEntireSlot really accomplishes nothing - instead of going to the trouble of computing the value for reserveEntireSlot and then possibly setting *all* functions of the slot as in-use, we can just set the in-use bit only for the specific function being used by a device. Later we will know from the context (the PCI connect flags, and whether we are reserving a specific address or asking for "the next available") whether or not it is okay to allocate other functions on the same slot. Although it's not used yet, we allow specifying "-1" for the function number when looking for the "next available slot" - this is going to end up meaning "return the lowest avaialable function in the slot, but since we currently only provide a function from an otherwise unused slot, "-1" ends up meaning "0". --- src/conf/domain_addr.c | 76 ++++++++++++++++-------------------------- src/conf/domain_addr.h | 4 +-- src/qemu/qemu_domain_address.c | 33 ++++++++---------- 3 files changed, 43 insertions(+), 70 deletions(-) diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index adc51f9..e0328f0 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -469,11 +469,9 @@ virDomainPCIAddressSlotInUse(virDomainPCIAddressSetPtr addrs, /* - * Reserve a slot (or just one function) for a device. If - * reserveEntireSlot is true, all functions for the slot are reserved, - * otherwise only one. If fromConfig is true, the address being - * requested came directly from the config and errors should be worded - * appropriately. If fromConfig is false, the address was + * Reserve a function in a slot. If fromConfig is true, the address + * being requested came directly from the config and errors should be + * worded appropriately. If fromConfig is false, the address was * automatically created by libvirt, so it is an internal error (not * XML). */ @@ -481,7 +479,6 @@ int virDomainPCIAddressReserveAddr(virDomainPCIAddressSetPtr addrs, virPCIDeviceAddressPtr addr, virDomainPCIConnectFlags flags, - bool reserveEntireSlot, bool fromConfig) { int ret = -1; @@ -504,33 +501,13 @@ virDomainPCIAddressReserveAddr(virDomainPCIAddressSetPtr addrs, bus = &addrs->buses[addr->bus]; - if (reserveEntireSlot) { - if (bus->slot[addr->slot].functions) { - virReportError(errType, - _("Attempted double use of PCI slot %s " - "(may need \"multifunction='on'\" for " - "device on function 0)"), addrStr); - goto cleanup; - } - bus->slot[addr->slot].functions = 0xFF; /* reserve all functions of slot */ - VIR_DEBUG("Reserving PCI slot %s (multifunction='off')", addrStr); - } else { - if (bus->slot[addr->slot].functions & (1 << addr->function)) { - if (addr->function == 0) { - virReportError(errType, - _("Attempted double use of PCI Address %s"), - addrStr); - } else { - virReportError(errType, - _("Attempted double use of PCI Address %s " - "(may need \"multifunction='on'\" " - "for device on function 0)"), addrStr); - } - goto cleanup; - } - bus->slot[addr->slot].functions |= (1 << addr->function); - VIR_DEBUG("Reserving PCI address %s", addrStr); + if (bus->slot[addr->slot].functions & (1 << addr->function)) { + virReportError(errType, _("Attempted double use of PCI Address %s"), + addrStr); + goto cleanup; } + bus->slot[addr->slot].functions |= (1 << addr->function); + VIR_DEBUG("Reserving PCI address %s", addrStr); ret = 0; cleanup: @@ -544,7 +521,7 @@ virDomainPCIAddressReserveSlot(virDomainPCIAddressSetPtr addrs, virPCIDeviceAddressPtr addr, virDomainPCIConnectFlags flags) { - return virDomainPCIAddressReserveAddr(addrs, addr, flags, true, false); + return virDomainPCIAddressReserveAddr(addrs, addr, flags, false); } int @@ -579,8 +556,8 @@ virDomainPCIAddressEnsureAddr(virDomainPCIAddressSetPtr addrs, addrStr, flags, true)) goto cleanup; - ret = virDomainPCIAddressReserveAddr(addrs, &dev->addr.pci, flags, - true, true); + ret = virDomainPCIAddressReserveAddr(addrs, &dev->addr.pci, + flags, true); } else { ret = virDomainPCIAddressReserveNextSlot(addrs, dev, flags); } @@ -659,6 +636,7 @@ virDomainPCIAddressSetFree(virDomainPCIAddressSetPtr addrs) static int ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) virDomainPCIAddressGetNextSlot(virDomainPCIAddressSetPtr addrs, virPCIDeviceAddressPtr next_addr, + int function, virDomainPCIConnectFlags flags) { /* default to starting the search for a free slot from @@ -686,6 +664,12 @@ virDomainPCIAddressGetNextSlot(virDomainPCIAddressSetPtr addrs, a.slot = addrs->buses[0].minSlot; } + /* if the caller asks for "any function", give them function 0 */ + if (function == -1) + a.function = 0; + else + a.function = function; + while (a.bus < addrs->nbuses) { VIR_FREE(addrStr); if (!(addrStr = virDomainPCIAddressAsString(&a))) @@ -764,14 +748,13 @@ virDomainPCIAddressGetNextSlot(virDomainPCIAddressSetPtr addrs, * @dev: virDomainDeviceInfo that should get the new address. * @flags: CONNECT_TYPE flags for the device that needs an address. * @function: which function on the slot to mark as reserved - * (if @reserveEntireSlot is false) - * @reserveEntireSlot: true to reserve all functions on the new slot, - * false to reserve just @function * * Find the next *completely unreserved* slot with compatible - * connection @flags, mark either one function or the entire - * slot as in-use (according to @function and @reserveEntireSlot), - * and set @dev->addr.pci with this newly reserved address. + * connection @flags, mark one function of the slot as in-use + * (according to @function), then set @dev->addr.pci with this newly + * reserved address. If @function is -1, then the lowest unused + * function of the slot will be reserved (and since we only look for + * completely unused slots, that means "0"). * * returns 0 on success, or -1 on failure. */ @@ -779,17 +762,14 @@ int virDomainPCIAddressReserveNextAddr(virDomainPCIAddressSetPtr addrs, virDomainDeviceInfoPtr dev, virDomainPCIConnectFlags flags, - unsigned int function, - bool reserveEntireSlot) + int function) { virPCIDeviceAddress addr; - if (virDomainPCIAddressGetNextSlot(addrs, &addr, flags) < 0) + if (virDomainPCIAddressGetNextSlot(addrs, &addr, function, flags) < 0) return -1; - addr.function = reserveEntireSlot ? 0 : function; - - if (virDomainPCIAddressReserveAddr(addrs, &addr, flags, reserveEntireSlot, false) < 0) + if (virDomainPCIAddressReserveAddr(addrs, &addr, flags, false) < 0) return -1; addrs->lastaddr = addr; @@ -809,7 +789,7 @@ virDomainPCIAddressReserveNextSlot(virDomainPCIAddressSetPtr addrs, virDomainDeviceInfoPtr dev, virDomainPCIConnectFlags flags) { - return virDomainPCIAddressReserveNextAddr(addrs, dev, flags, 0, true); + return virDomainPCIAddressReserveNextAddr(addrs, dev, flags, 0); } diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h index 7df0502..9c08274 100644 --- a/src/conf/domain_addr.h +++ b/src/conf/domain_addr.h @@ -141,7 +141,6 @@ int virDomainPCIAddressSetGrow(virDomainPCIAddressSetPtr addrs, int virDomainPCIAddressReserveAddr(virDomainPCIAddressSetPtr addrs, virPCIDeviceAddressPtr addr, virDomainPCIConnectFlags flags, - bool reserveEntireSlot, bool fromConfig) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); @@ -166,8 +165,7 @@ int virDomainPCIAddressReleaseSlot(virDomainPCIAddressSetPtr addrs, int virDomainPCIAddressReserveNextAddr(virDomainPCIAddressSetPtr addrs, virDomainDeviceInfoPtr dev, virDomainPCIConnectFlags flags, - unsigned int function, - bool reserveEntireSlot) + int function) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); int virDomainPCIAddressReserveNextSlot(virDomainPCIAddressSetPtr addrs, diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index a3dfd63..37ce675 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -863,12 +863,11 @@ qemuDomainFillDevicePCIConnectFlags(virDomainDefPtr def, static int qemuDomainPCIAddressReserveNextAddr(virDomainPCIAddressSetPtr addrs, virDomainDeviceInfoPtr dev, - unsigned int function, - bool reserveEntireSlot) + unsigned int function) { return virDomainPCIAddressReserveNextAddr(addrs, dev, dev->pciConnectFlags, - function, reserveEntireSlot); + function); } @@ -876,7 +875,7 @@ static int qemuDomainPCIAddressReserveNextSlot(virDomainPCIAddressSetPtr addrs, virDomainDeviceInfoPtr dev) { - return qemuDomainPCIAddressReserveNextAddr(addrs, dev, 0, true); + return qemuDomainPCIAddressReserveNextAddr(addrs, dev, 0); } @@ -889,7 +888,6 @@ qemuDomainCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED, virDomainPCIAddressSetPtr addrs = opaque; int ret = -1; virPCIDeviceAddressPtr addr = &info->addr.pci; - bool entireSlot; if (!virDeviceInfoPCIAddressPresent(info) || ((device->type == VIR_DOMAIN_DEVICE_HOSTDEV) && @@ -961,12 +959,10 @@ qemuDomainCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED, } } - entireSlot = (addr->function == 0 && - addr->multi != VIR_TRISTATE_SWITCH_ON); - - if (virDomainPCIAddressReserveAddr(addrs, addr, info->pciConnectFlags, - entireSlot, true) < 0) + if (virDomainPCIAddressReserveAddr(addrs, addr, + info->pciConnectFlags, true) < 0) { goto cleanup; + } ret = 0; cleanup: @@ -1287,7 +1283,7 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def, } if (assign) { if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, - flags, false, true) < 0) + flags, true) < 0) goto cleanup; cont->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; cont->info.addr.pci.domain = 0; @@ -1310,7 +1306,7 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def, tmp_addr.slot = 0x1E; if (!virDomainPCIAddressSlotInUse(addrs, &tmp_addr)) { if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, - flags, true, false) < 0) + flags, false) < 0) goto cleanup; cont->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; cont->info.addr.pci.domain = 0; @@ -1333,13 +1329,13 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def, tmp_addr.slot = 0x1F; tmp_addr.function = 0; tmp_addr.multi = VIR_TRISTATE_SWITCH_ON; - if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags, - false, false) < 0) + if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, + flags, false) < 0) goto cleanup; tmp_addr.function = 3; tmp_addr.multi = VIR_TRISTATE_SWITCH_ABSENT; - if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags, - false, false) < 0) + if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, + flags, false) < 0) goto cleanup; } @@ -1639,7 +1635,7 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def, /* Reserve this function on the slot we found */ if (virDomainPCIAddressReserveAddr(addrs, &addr, cont->info.pciConnectFlags, - false, true) < 0) + true) < 0) goto error; cont->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; @@ -1648,8 +1644,7 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def, /* 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, - false) < 0) { + addr.function) < 0) { goto error; } -- 2.7.4