[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[libvirt] [PATCH 06/13] Rewrite way QEMU PCI addresses are allocated



The current QEMU code allocates PCI addresses incrementally starting
at 4. This is not satisfactory because the user may have given some
addresses in their XML config, which need to be skipped over when
allocating addresses to remaining devices.

It is thus neccessary to maintain a list of already allocated PCI
addresses and then only allocate ones that remain unused. This is
also required for domain device hotplug to work properly later.

* src/qemu/qemu_conf.c, src/qemu/qemu_conf.h: Add APIs for creating
  list of existing PCI addresses, and allocating new addresses.
  Refactor address assignment to use this code
* src/qemu/qemu_driver.c: Pull PCI address assignment up into the
  qemuStartVMDaemon() method, as a prelude to moving it into the
  'define' method. Update list of allocated addresses when connecting
  to a running VM at daemon startup.
* tests/qemuxml2argvtest.c, tests/qemuargv2xmltest.c,
  tests/qemuxml2xmltest.c: Remove USB product test since all
  passthrough is done based on address
* tests/qemuxml2argvdata/qemuxml2argv-hostdev-usb-product.args,
  tests/qemuxml2argvdata/qemuxml2argv-hostdev-usb-product.xml: Kil
  unused data files
---
 src/qemu/qemu_conf.c                               |  228 +++++++++++++++++---
 src/qemu/qemu_conf.h                               |   12 +
 src/qemu/qemu_driver.c                             |   23 ++
 tests/qemuargv2xmltest.c                           |    1 -
 .../qemuxml2argv-hostdev-usb-product.args          |    1 -
 .../qemuxml2argv-hostdev-usb-product.xml           |   36 ---
 tests/qemuxml2argvtest.c                           |   13 +-
 tests/qemuxml2xmltest.c                            |    1 -
 8 files changed, 245 insertions(+), 70 deletions(-)
 delete mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-usb-product.args
 delete mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-usb-product.xml

diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index bbf49fd..9bfd181 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1588,28 +1588,174 @@ qemuAssignDeviceAliases(virDomainDefPtr def)
 }
 
 
-static void
-qemuAssignDevicePCISlot(virDomainDeviceInfoPtr info,
-                        int slot)
+#define QEMU_PCI_ADDRESS_LAST_SLOT 31
+struct _qemuDomainPCIAddressSet {
+    virHashTablePtr used;
+    int nextslot;
+    /* XXX add domain, bus later when QEMU allows > 1 */
+};
+
+
+static char *qemuPCIAddressAsString(virDomainDeviceInfoPtr dev)
 {
-    info->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
-    info->addr.pci.domain = 0;
-    info->addr.pci.bus = 0;
-    info->addr.pci.slot = slot;
-    info->addr.pci.function = 0;
+    char *addr;
+
+    if (virAsprintf(&addr, "%d:%d:%d",
+                    dev->addr.pci.domain,
+                    dev->addr.pci.bus,
+                    dev->addr.pci.slot) < 0) {
+        virReportOOMError(NULL);
+        return NULL;
+    }
+    return addr;
 }
 
-static void
-qemuAssignDevicePCISlots(virDomainDefPtr def)
+
+static int qemuCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED,
+                                 virDomainDeviceInfoPtr dev,
+                                 void *opaque)
+{
+    qemuDomainPCIAddressSetPtr addrs = opaque;
+
+    if (dev->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
+        char *addr = qemuPCIAddressAsString(dev);
+
+        if (virHashAddEntry(addrs->used, addr, addr) < 0) {
+            VIR_FREE(addr);
+            return -1;
+        }
+    }
+
+    return 0;
+}
+
+
+qemuDomainPCIAddressSetPtr qemuDomainPCIAddressSetCreate(virDomainDefPtr def)
+{
+    qemuDomainPCIAddressSetPtr addrs;
+
+    if (VIR_ALLOC(addrs) < 0)
+        goto no_memory;
+
+    if (!(addrs->used = virHashCreate(10)))
+        goto no_memory;
+
+    if (virDomainDeviceInfoIterate(def, qemuCollectPCIAddress, addrs) < 0)
+        goto error;
+
+    return addrs;
+
+no_memory:
+    virReportOOMError(NULL);
+error:
+    qemuDomainPCIAddressSetFree(addrs);
+    return NULL;
+}
+
+int qemuDomainPCIAddressReserve(qemuDomainPCIAddressSetPtr addrs,
+                                int slot)
+{
+    virDomainDeviceInfo dev;
+    char *addr;
+
+    dev.addr.pci.domain = 0;
+    dev.addr.pci.bus = 0;
+    dev.addr.pci.slot = slot;
+
+    addr = qemuPCIAddressAsString(&dev);
+    if (!addr)
+        return -1;
+
+    if (virHashLookup(addrs->used, addr)) {
+        qemudReportError(NULL, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+                         _("unable to reserve PCI address %s"), addr);
+        VIR_FREE(addr);
+        return -1;
+    }
+
+    if (virHashAddEntry(addrs->used, addr, addr)) {
+        VIR_FREE(addr);
+        return -1;
+    }
+
+    return 0;
+}
+
+
+static void qemuDomainPCIAddressSetFreeEntry(void *payload, const char *name ATTRIBUTE_UNUSED)
+{
+    VIR_FREE(payload);
+}
+
+
+void qemuDomainPCIAddressSetFree(qemuDomainPCIAddressSetPtr addrs)
+{
+    if (!addrs)
+        return;
+
+    virHashFree(addrs->used, qemuDomainPCIAddressSetFreeEntry);
+}
+
+
+int qemuDomainPCIAddressSetNextAddr(qemuDomainPCIAddressSetPtr addrs,
+                                    virDomainDeviceInfoPtr dev)
 {
     int i;
-    /*
-     * slot = 0 -> Host bridge
-     * slot = 1 -> PIIX3 (ISA bridge, IDE controller, something else unknown, USB controller)
-     * slot = 2 -> VGA
-     * slot = 3 -> VirtIO Balloon
-     */
-    int nextslot = 4;
+
+    for (i = addrs->nextslot ; i <= QEMU_PCI_ADDRESS_LAST_SLOT ; i++) {
+        virDomainDeviceInfo maybe;
+        char *addr;
+
+        memset(&maybe, 0, sizeof(maybe));
+        maybe.addr.pci.domain = 0;
+        maybe.addr.pci.bus = 0;
+        maybe.addr.pci.slot = i;
+
+        addr = qemuPCIAddressAsString(&maybe);
+
+        if (virHashLookup(addrs->used, addr)) {
+            VIR_FREE(addr);
+            continue;
+        }
+
+        if (virHashAddEntry(addrs->used, addr, addr) < 0) {
+            VIR_FREE(addr);
+            return -1;
+        }
+
+        dev->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
+        dev->addr.pci.domain = 0;
+        dev->addr.pci.bus = 0;
+        dev->addr.pci.slot = i;
+
+        addrs->nextslot = i + 1;
+
+        return 0;
+    }
+
+    qemudReportError(NULL, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+                     "%s", _("No more available PCI addresses"));
+    return -1;
+}
+
+
+int
+qemuAssignDevicePCISlots(virDomainDefPtr def, qemuDomainPCIAddressSetPtr addrs)
+{
+    int i;
+
+    /* Host bridge */
+    if (qemuDomainPCIAddressReserve(addrs, 0) < 0)
+        goto error;
+    /* PIIX3 (ISA bridge, IDE controller, something else unknown, USB controller) */
+    if (qemuDomainPCIAddressReserve(addrs, 1) < 0)
+        goto error;
+    /* VGA */
+    if (qemuDomainPCIAddressReserve(addrs, 2) < 0)
+        goto error;
+    /* VirtIO Balloon */
+    if (qemuDomainPCIAddressReserve(addrs, 3) < 0)
+        goto error;
 
     for (i = 0; i < def->ndisks ; i++) {
         if (def->disks[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)
@@ -1619,13 +1765,15 @@ qemuAssignDevicePCISlots(virDomainDefPtr def)
         if (def->disks[i]->bus != VIR_DOMAIN_DISK_BUS_VIRTIO)
             continue;
 
-        qemuAssignDevicePCISlot(&def->disks[i]->info, nextslot++);
+        if (qemuDomainPCIAddressSetNextAddr(addrs, &def->disks[i]->info) < 0)
+            goto error;
     }
 
     for (i = 0; i < def->nnets ; i++) {
         if (def->nets[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)
             continue;
-        qemuAssignDevicePCISlot(&def->nets[i]->info, nextslot++);
+        if (qemuDomainPCIAddressSetNextAddr(addrs, &def->nets[i]->info) < 0)
+            goto error;
     }
 
     for (i = 0; i < def->nsounds ; i++) {
@@ -1636,7 +1784,8 @@ qemuAssignDevicePCISlots(virDomainDefPtr def)
             def->sounds[i]->model == VIR_DOMAIN_SOUND_MODEL_PCSPK)
             continue;
 
-        qemuAssignDevicePCISlot(&def->sounds[i]->info, nextslot++);
+        if (qemuDomainPCIAddressSetNextAddr(addrs, &def->sounds[i]->info) < 0)
+            goto error;
     }
 
     for (i = 0; i < def->nhostdevs ; i++) {
@@ -1646,14 +1795,23 @@ qemuAssignDevicePCISlots(virDomainDefPtr def)
             def->hostdevs[i]->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI)
             continue;
 
-        qemuAssignDevicePCISlot(&def->hostdevs[i]->info, nextslot++);
+        if (qemuDomainPCIAddressSetNextAddr(addrs, &def->hostdevs[i]->info) < 0)
+            goto error;
     }
     for (i = 0; i < def->nvideos ; i++) {
+        if (def->videos[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)
+            continue;
         /* First VGA is hardcoded slot=2 */
-        if (i == 0)
-            qemuAssignDevicePCISlot(&def->videos[i]->info, 2);
-        else
-            qemuAssignDevicePCISlot(&def->videos[i]->info, nextslot++);
+        if (i == 0) {
+            def->videos[i]->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
+            def->videos[i]->info.addr.pci.domain = 0;
+            def->videos[i]->info.addr.pci.bus = 0;
+            def->videos[i]->info.addr.pci.slot = 2;
+            def->videos[i]->info.addr.pci.function = 0;
+        } else {
+            if (qemuDomainPCIAddressSetNextAddr(addrs, &def->videos[i]->info) < 0)
+                goto error;
+        }
     }
     for (i = 0; i < def->ncontrollers ; i++) {
         if (def->controllers[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)
@@ -1665,10 +1823,14 @@ qemuAssignDevicePCISlots(virDomainDefPtr def)
         /* First IDE controller lives on the PIIX3 at slot=1, function=1 */
         if (def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_IDE &&
             def->controllers[i]->idx == 0) {
-            qemuAssignDevicePCISlot(&def->controllers[i]->info, 1);
+            def->controllers[i]->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
+            def->controllers[i]->info.addr.pci.domain = 0;
+            def->controllers[i]->info.addr.pci.bus = 0;
+            def->controllers[i]->info.addr.pci.slot = 1;
             def->controllers[i]->info.addr.pci.function = 1;
         } else {
-            qemuAssignDevicePCISlot(&def->controllers[i]->info, nextslot++);
+            if (qemuDomainPCIAddressSetNextAddr(addrs, &def->controllers[i]->info) < 0)
+                goto error;
         }
     }
     for (i = 0; i < def->ninputs ; i++) {
@@ -1684,9 +1846,16 @@ qemuAssignDevicePCISlots(virDomainDefPtr def)
         /* Nada - none are PCI based (yet) */
         /* XXX virtio-serial will need one */
     }
-    if (def->watchdog) {
-        qemuAssignDevicePCISlot(&def->watchdog->info, nextslot++);
+    if (def->watchdog &&
+        def->watchdog->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
+        if (qemuDomainPCIAddressSetNextAddr(addrs, &def->watchdog->info) < 0)
+            goto error;
     }
+
+    return 0;
+
+error:
+    return -1;
 }
 
 
@@ -2885,7 +3054,6 @@ int qemudBuildCommandLine(virConnectPtr conn,
 
     if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) {
         qemuAssignDeviceAliases(def);
-        qemuAssignDevicePCISlots(def);
     } else {
         qemuAssignDiskAliases(def, qemuCmdFlags);
     }
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index c3b196e..bcddf3c 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -146,6 +146,8 @@ struct qemud_driver {
     pciDeviceList *activePciHostdevs;
 };
 
+typedef struct _qemuDomainPCIAddressSet qemuDomainPCIAddressSet;
+typedef qemuDomainPCIAddressSet *qemuDomainPCIAddressSetPtr;
 
 /* Port numbers used for KVM migration. */
 #define QEMUD_MIGRATION_FIRST_PORT 49152
@@ -272,4 +274,14 @@ virDomainDefPtr qemuParseCommandLineString(virConnectPtr conn,
                                            virCapsPtr caps,
                                            const char *args);
 
+qemuDomainPCIAddressSetPtr qemuDomainPCIAddressSetCreate(virDomainDefPtr def);
+int qemuDomainPCIAddressReserve(qemuDomainPCIAddressSetPtr addrs,
+                                int slot);
+int qemuDomainPCIAddressSetNextAddr(qemuDomainPCIAddressSetPtr addrs,
+                                    virDomainDeviceInfoPtr dev);
+void qemuDomainPCIAddressSetFree(qemuDomainPCIAddressSetPtr addrs);
+int  qemuAssignDevicePCISlots(virDomainDefPtr def, qemuDomainPCIAddressSetPtr addrs);
+
+
+
 #endif /* __QEMUD_CONF_H */
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 25e60c2..099e678 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -93,6 +93,8 @@ struct _qemuDomainObjPrivate {
 
     int nvcpupids;
     int *vcpupids;
+
+    qemuDomainPCIAddressSetPtr pciaddrs;
 };
 
 static int qemudShutdown(void);
@@ -146,6 +148,7 @@ static void qemuDomainObjPrivateFree(void *data)
 {
     qemuDomainObjPrivatePtr priv = data;
 
+    qemuDomainPCIAddressSetFree(priv->pciaddrs);
     virDomainChrDefFree(priv->monConfig);
     VIR_FREE(priv->vcpupids);
 
@@ -843,11 +846,14 @@ qemuReconnectDomain(void *payload, const char *name ATTRIBUTE_UNUSED, void *opaq
 {
     virDomainObjPtr obj = payload;
     struct qemud_driver *driver = opaque;
+    qemuDomainObjPrivatePtr priv;
 
     virDomainObjLock(obj);
 
     VIR_DEBUG("Reconnect monitor to %p '%s'", obj, obj->def->name);
 
+    priv = obj->privateData;
+
     /* XXX check PID liveliness & EXE path */
     if (qemuConnectMonitor(obj) < 0)
         goto error;
@@ -856,6 +862,9 @@ qemuReconnectDomain(void *payload, const char *name ATTRIBUTE_UNUSED, void *opaq
         goto error;
     }
 
+    if (!(priv->pciaddrs = qemuDomainPCIAddressSetCreate(obj->def)))
+        goto error;
+
     if (driver->securityDriver &&
         driver->securityDriver->domainReserveSecurityLabel &&
         driver->securityDriver->domainReserveSecurityLabel(NULL, obj) < 0)
@@ -2635,6 +2644,18 @@ static int qemudStartVMDaemon(virConnectPtr conn,
         goto cleanup;
     }
 
+    if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) {
+        if (priv->pciaddrs) {
+            qemuDomainPCIAddressSetFree(priv->pciaddrs);
+            priv->pciaddrs = NULL;
+        }
+        if (!(priv->pciaddrs = qemuDomainPCIAddressSetCreate(vm->def)))
+            goto cleanup;
+
+        if (qemuAssignDevicePCISlots(vm->def, priv->pciaddrs) < 0)
+            goto cleanup;
+    }
+
     vm->def->id = driver->nextvmid++;
     if (qemudBuildCommandLine(conn, driver, vm->def, priv->monConfig,
                               priv->monJSON, qemuCmdFlags, &argv, &progenv,
@@ -2867,6 +2888,8 @@ static void qemudShutdownVMDaemon(virConnectPtr conn,
 
     virDomainDefClearPCIAddresses(vm->def);
     virDomainDefClearDeviceAliases(vm->def);
+    qemuDomainPCIAddressSetFree(priv->pciaddrs);
+    priv->pciaddrs = NULL;
 
     qemuDomainReAttachHostDevices(conn, driver, vm->def);
 
diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c
index 8326c57..1f1914b 100644
--- a/tests/qemuargv2xmltest.c
+++ b/tests/qemuargv2xmltest.c
@@ -213,7 +213,6 @@ mymain(int argc, char **argv)
     DO_TEST("sound", 0);
     DO_TEST("watchdog", 0);
 
-    DO_TEST("hostdev-usb-product", 0);
     DO_TEST("hostdev-usb-address", 0);
 
     DO_TEST("hostdev-pci-address", QEMUD_CMD_FLAG_PCIDEVICE);
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-usb-product.args b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-usb-product.args
deleted file mode 100644
index 6084745..0000000
--- a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-usb-product.args
+++ /dev/null
@@ -1 +0,0 @@
-LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none -usb -usbdevice host:0204:6025 -usbdevice host:1234:0000
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-usb-product.xml b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-usb-product.xml
deleted file mode 100644
index 3dc8eeb..0000000
--- a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-usb-product.xml
+++ /dev/null
@@ -1,36 +0,0 @@
-<domain type='qemu'>
-  <name>QEMUGuest1</name>
-  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
-  <memory>219200</memory>
-  <currentMemory>219200</currentMemory>
-  <vcpu>1</vcpu>
-  <os>
-    <type arch='i686' machine='pc'>hvm</type>
-    <boot dev='hd'/>
-  </os>
-  <clock offset='utc'/>
-  <on_poweroff>destroy</on_poweroff>
-  <on_reboot>restart</on_reboot>
-  <on_crash>destroy</on_crash>
-  <devices>
-    <emulator>/usr/bin/qemu</emulator>
-    <disk type='block' device='disk'>
-      <source dev='/dev/HostVG/QEMUGuest1'/>
-      <target dev='hda' bus='ide'/>
-      <address type='drive' controller='0' bus='0' unit='0'/>
-    </disk>
-    <controller type='ide' index='0'/>
-    <hostdev mode='subsystem' type='usb' managed='no'>
-      <source>
-        <vendor id='0x0204'/>
-        <product id='0x6025'/>
-      </source>
-    </hostdev>
-    <hostdev mode='subsystem' type='usb' managed='no'>
-      <source>
-        <vendor id='0x1234'/>
-        <product id='0x0000'/>
-      </source>
-    </hostdev>
-  </devices>
-</domain>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index fc237c2..90c1cb0 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -62,6 +62,18 @@ static int testCompareXMLToArgvFiles(const char *xml,
     if (qemudCanonicalizeMachine(&driver, vmdef) < 0)
         goto fail;
 
+    if (flags & QEMUD_CMD_FLAG_DEVICE) {
+        qemuDomainPCIAddressSetPtr pciaddrs;
+        if (!(pciaddrs = qemuDomainPCIAddressSetCreate(vmdef)))
+            goto fail;
+
+        if (qemuAssignDevicePCISlots(vmdef, pciaddrs) < 0)
+            goto fail;
+
+        qemuDomainPCIAddressSetFree(pciaddrs);
+    }
+
+
     if (qemudBuildCommandLine(NULL, &driver,
                               vmdef, &monitor_chr, 0, flags,
                               &argv, &qenv,
@@ -303,7 +315,6 @@ mymain(int argc, char **argv)
     DO_TEST("sound", 0);
     DO_TEST("sound-device", QEMUD_CMD_FLAG_DEVICE);
 
-    DO_TEST("hostdev-usb-product", 0);
     DO_TEST("hostdev-usb-address", 0);
     DO_TEST("hostdev-usb-address-device", QEMUD_CMD_FLAG_DEVICE);
     DO_TEST("hostdev-pci-address", QEMUD_CMD_FLAG_PCIDEVICE);
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 0302696..5b706bb 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -134,7 +134,6 @@ mymain(int argc, char **argv)
     DO_TEST("console-compat");
     DO_TEST("channel-guestfwd");
 
-    DO_TEST("hostdev-usb-product");
     DO_TEST("hostdev-usb-address");
     DO_TEST("hostdev-pci-address");
 
-- 
1.6.5.2


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]