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 available 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

@@ -527,11 +527,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).
  */
@@ -539,7 +537,6 @@ int
 virDomainPCIAddressReserveAddr(virDomainPCIAddressSetPtr addrs,
                                virPCIDeviceAddressPtr addr,
                                virDomainPCIConnectFlags flags,
-                               bool reserveEntireSlot,
                                bool fromConfig)
 {
     int ret = -1;
@@ -562,33 +559,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:
@@ -602,7 +579,7 @@ virDomainPCIAddressReserveSlot(virDomainPCIAddressSetPtr addrs,
                                virPCIDeviceAddressPtr addr,
                                virDomainPCIConnectFlags flags)
 {
-    return virDomainPCIAddressReserveAddr(addrs, addr, flags, true, false);
+    return virDomainPCIAddressReserveAddr(addrs, addr, flags, false);
 }
 
 int
@@ -637,8 +614,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);
     }
@@ -716,6 +693,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
@@ -743,6 +721,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)))
@@ -821,14 +805,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.
  */
@@ -836,17 +819,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;
@@ -866,7 +846,7 @@ virDomainPCIAddressReserveNextSlot(virDomainPCIAddressSetPtr addrs,
                                    virDomainDeviceInfoPtr dev,
                                    virDomainPCIConnectFlags flags)
 {
-    return virDomainPCIAddressReserveNextAddr(addrs, dev, flags, 0, true);
+    return virDomainPCIAddressReserveNextAddr(addrs, dev, flags, 0);
 }
 
 

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,

src/qemu/qemu_domain_address.c

@@ -937,12 +937,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);
 }
 
 
@@ -950,7 +949,7 @@ static int
 qemuDomainPCIAddressReserveNextSlot(virDomainPCIAddressSetPtr addrs,
                                     virDomainDeviceInfoPtr dev)
 {
-    return qemuDomainPCIAddressReserveNextAddr(addrs, dev, 0, true);
+    return qemuDomainPCIAddressReserveNextAddr(addrs, dev, 0);
 }
 
 
@@ -963,7 +962,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) &&
@@ -1036,12 +1034,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:
@@ -1329,7 +1325,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;
@@ -1352,7 +1348,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;
@@ -1375,13 +1371,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;
     }
 
@@ -1681,7 +1677,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;
@@ -1690,8 +1686,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;
                 }