From af91509dfc59e90262117b96a2a365d5ece29393 Mon Sep 17 00:00:00 2001
Message-Id: <af91509dfc59e90262117b96a2a365d5ece29393@dist-git>
From: Andrea Bolognani <abologna@redhat.com>
Date: Mon, 17 Jul 2017 12:09:03 +0200
Subject: [PATCH] conf: Simplify slot allocation

The current algorithm for slot allocation tries to be clever
and avoid looking at buses / slots more than once unless it's
necessary. Unfortunately that makes the code more complex,
and it will cause problem later on in some situations unless
even more complex code is added.

Since the performance gains are going to be pretty modest
anyway, we can just get rid of the extra complexity and use a
completely  straighforward implementation instead.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Laine Stump <laine@laine.org>
(cherry picked from commit 2bd0658d7cc886209a53bae23c20adbb7d4f91ac)

Bug: https://bugzilla.redhat.com/show_bug.cgi?id=1431193

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
---
 src/conf/domain_addr.c | 47 +++++++++--------------------------------------
 src/conf/domain_addr.h |  2 --
 2 files changed, 9 insertions(+), 40 deletions(-)

diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index b17bb61a1d..b8e5902c07 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -755,46 +755,34 @@ virDomainPCIAddressGetNextAddr(virDomainPCIAddressSetPtr addrs,
                                virDomainPCIConnectFlags flags,
                                int function)
 {
-    /* default to starting the search for a free slot from
-     * the first slot of domain 0 bus 0...
-     */
     virPCIDeviceAddress a = { 0 };
-    bool found = false;
 
     if (addrs->nbuses == 0) {
         virReportError(VIR_ERR_XML_ERROR, "%s", _("No PCI buses available"));
         goto error;
     }
 
-    /* ...unless this search is for the exact same type of device as
-     * last time, then continue the search from the slot where we
-     * found the previous match (it's possible there will still be a
-     * function available on that slot).
-     */
-    if (flags == addrs->lastFlags)
-        a = addrs->lastaddr;
-    else
-        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) {
-        if (virDomainPCIAddressFindUnusedFunctionOnBus(&addrs->buses[a.bus],
-                                                       &a, function,
+    /* "Begin at the beginning," the King said, very gravely, "and go on
+     * till you come to the end: then stop." */
+    for (a.bus = 0; a.bus < addrs->nbuses; a.bus++) {
+        virDomainPCIAddressBusPtr bus = &addrs->buses[a.bus];
+        bool found = false;
+
+        a.slot = bus->minSlot;
+
+        if (virDomainPCIAddressFindUnusedFunctionOnBus(bus, &a, function,
                                                        flags, &found) < 0) {
             goto error;
         }
 
         if (found)
             goto success;
-
-        /* nothing on this bus, go to the next bus */
-        if (++a.bus < addrs->nbuses)
-            a.slot = addrs->buses[a.bus].minSlot;
     }
 
     /* There were no free slots after the last used one */
@@ -805,20 +793,6 @@ virDomainPCIAddressGetNextAddr(virDomainPCIAddressSetPtr addrs,
         /* this device will use the first slot of the new bus */
         a.slot = addrs->buses[a.bus].minSlot;
         goto success;
-    } else if (flags == addrs->lastFlags) {
-        /* Check the buses from 0 up to the last used one */
-        for (a.bus = 0; a.bus <= addrs->lastaddr.bus; a.bus++) {
-            a.slot = addrs->buses[a.bus].minSlot;
-
-            if (virDomainPCIAddressFindUnusedFunctionOnBus(&addrs->buses[a.bus],
-                                                           &a, function,
-                                                           flags, &found) < 0) {
-                goto error;
-            }
-
-            if (found)
-                goto success;
-        }
     }
 
     virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -865,9 +839,6 @@ virDomainPCIAddressReserveNextAddr(virDomainPCIAddressSetPtr addrs,
     if (virDomainPCIAddressReserveAddrInternal(addrs, &addr, flags, false) < 0)
         return -1;
 
-    addrs->lastaddr = addr;
-    addrs->lastFlags = flags;
-
     if (!addrs->dryRun) {
         dev->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
         dev->addr.pci = addr;
diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h
index bb10f1abf3..a5afefda95 100644
--- a/src/conf/domain_addr.h
+++ b/src/conf/domain_addr.h
@@ -106,8 +106,6 @@ typedef virDomainPCIAddressBus *virDomainPCIAddressBusPtr;
 struct _virDomainPCIAddressSet {
     virDomainPCIAddressBus *buses;
     size_t nbuses;
-    virPCIDeviceAddress lastaddr;
-    virDomainPCIConnectFlags lastFlags;
     bool dryRun;          /* on a dry run, new buses are auto-added
                              and addresses aren't saved in device infos */
 };
-- 
2.13.3