[libvirt] [PATCH 3/5] qemu: Fix auto-adding PCI bridge when all slots are reserved

[Erik Skultety]

Commit 93c8ca tried to fix the issue with auto-adding of a PCI bridge
controller, it worked well when the slots were reserved by devices with
user defined addresses without any other devices with unspecified PCI addresses
present in the XML.
However, if another device with unspecified PCI addresses appeared in
the domain's XML, the same problem occurred as the BZ below references.

This patch fixes the issue by not creating any additional bridges when not
necessary.
Now let's say all the buses corresponding to the number of PCI controllers
present in the domain's XML got fully reserved by devices with user defined
addresses. If there are no other devices present in the XML, no need to
create both an additional PCI bus and a bridge controller.
If however there are still some PCI devices waiting to get a slot assigned
by qemuAssignDevicePCISlots, this means an additional bus needs to
be created along with a corresponding bridge controller.
This scenario now results in an reasonable error instead of generating wrong qemu
command line.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1132900
---
 src/qemu/qemu_command.c | 42 ++++++++++++++++++++++++++++++++++--------
 1 file changed, 34 insertions(+), 8 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index cdf02a6..99b06ee 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1472,6 +1472,9 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def,
         int nbuses = 0;
         size_t i;
         int rv;
+        int resflags = 0;
+        bool buses_reserved = false;
+
         virDomainPCIConnectFlags flags = VIR_PCI_CONNECT_TYPE_PCI;
 
         for (i = 0; i < def->ncontrollers; i++) {
@@ -1490,17 +1493,22 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def,
             /* 1st pass to figure out how many PCI bridges we need */
             if (!(addrs = qemuDomainPCIAddressSetCreate(def, nbuses, true)))
                 goto cleanup;
+
+            for (i = 0; i < nbuses; i++) {
+                if (qemuDomainPCIBusFullyReserved(&addrs->buses[i]))
+                    resflags |= 1 << i;
+            }
+            buses_reserved = (resflags && ~((~0) << nbuses));
+
             if (qemuAssignDevicePCISlots(def, qemuCaps, addrs) < 0)
                 goto cleanup;
 
-            for (i = 0; i < addrs->nbuses; i++) {
-                if (!qemuDomainPCIBusFullyReserved(&addrs->buses[i])) {
-
-                    /* Reserve 1 extra slot for a (potential) bridge */
-                    if (virDomainPCIAddressReserveNextSlot(addrs, &info, flags) < 0)
-                        goto cleanup;
-                }
-            }
+            /* Reserve 1 extra slot for a (potential) bridge only if buses
+             * are not fully reserved yet
+             */
+            if (!buses_reserved &&
+                virDomainPCIAddressReserveNextSlot(addrs, &info, flags) < 0)
+                goto cleanup;
 
             for (i = 1; i < addrs->nbuses; i++) {
                 virDomainPCIAddressBusPtr bus = &addrs->buses[i];
@@ -1532,6 +1540,24 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def,
             if (qemuAssignDevicePCISlots(def, qemuCaps, addrs) < 0)
                 goto cleanup;
         }
+
+        for (i = 0; i < def->ncontrollers; i++) {
+            /* check if every PCI bridge controller's ID is greater than
+             * the bus it is placed onto
+             */
+            virDomainControllerDefPtr cont = def->controllers[i];
+            int idx = cont->idx;
+            int bus = cont->info.addr.pci.bus;
+            if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI &&
+                cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE &&
+                idx <= bus) {
+                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                               _("failed to create PCI bridge "
+                                 "on bus %d: bus is fully reserved"),
+                               bus);
+                goto cleanup;
+            }
+        }
     }
 
     if (obj && obj->privateData) {
-- 
1.9.3