qemu: Introduce inactive PCI device list

pciTrySecondaryBusReset checks if there is active device on the
same bus, however, qemu driver doesn't maintain an effective
list for the inactive devices, and it passes meaningless argument
for parameter "inactiveDevs". e.g. (qemuPrepareHostdevPCIDevices)

if (!(pcidevs = qemuGetPciHostDeviceList(hostdevs, nhostdevs)))
    return -1;

..skipped...

if (pciResetDevice(dev, driver->activePciHostdevs, pcidevs) < 0)
    goto reattachdevs;

NB, the "pcidevs" used above are extracted from domain def, and
thus one won't be able to attach a device of which bus has other
device even detached from host (nodedev-detach). To see more
details of the problem:

RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=773667

This patch is to resolve the problem by introducing an inactive
PCI device list (just like qemu_driver->activePciHostdevs), and
the whole logic is:

  * Add the device to inactive list during nodedev-dettach
  * Remove the device from inactive list during nodedev-reattach
  * Remove the device from inactive list during attach-device
    (for non-managed device)
  * Add the device to inactive list after detach-device, only
    if the device is not managed

With the above, we have a sufficient inactive PCI device list, and thus
we can use it for pciResetDevice. e.g.(qemuPrepareHostdevPCIDevices)

if (pciResetDevice(dev, driver->activePciHostdevs,
                   driver->inactivePciHostdevs) < 0)
    goto reattachdevs;

diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index d8d7915f7e565031fbefa05c66915e6f78c1640f..7d7982377cc1803ae3d973138c78c3f00a0a5fdc 100644 (file)
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -1,7 +1,7 @@
 /*
  * qemu_conf.h: QEMU configuration management
  *
- * Copyright (C) 2006-2007, 2009-2011 Red Hat, Inc.
+ * Copyright (C) 2006-2007, 2009-2012 Red Hat, Inc.
  * Copyright (C) 2006 Daniel P. Berrange
  *
  * This library is free software; you can redistribute it and/or
@@ -128,6 +128,9 @@ struct qemud_driver {
     pciDeviceList *activePciHostdevs;
     usbDeviceList *activeUsbHostdevs;
 
+    /* The devices which is are not in use by the host or any guest. */
+    pciDeviceList *inactivePciHostdevs;
+
     virBitmapPtr reservedVNCPorts;
 
     virSysinfoDefPtr hostsysinfo;

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 712f1fc457374a3ad89c7d6d801eeca58144040d..47e23802f767cac253c611b685a35d44b3e08fe9 100644 (file)
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -588,6 +588,9 @@ qemudStartup(int privileged) {
     if ((qemu_driver->activeUsbHostdevs = usbDeviceListNew()) == NULL)
         goto error;
 
+    if ((qemu_driver->inactivePciHostdevs = pciDeviceListNew()) == NULL)
+        goto error;
+
     if (privileged) {
         if (chown(qemu_driver->libDir, qemu_driver->user, qemu_driver->group) < 0) {
             virReportSystemError(errno,
@@ -778,6 +781,7 @@ qemudShutdown(void) {
 
     qemuDriverLock(qemu_driver);
     pciDeviceListFree(qemu_driver->activePciHostdevs);
+    pciDeviceListFree(qemu_driver->inactivePciHostdevs);
     usbDeviceListFree(qemu_driver->activeUsbHostdevs);
     virCapabilitiesFree(qemu_driver->caps);
 
@@ -9211,6 +9215,7 @@ qemudNodeDeviceDettach (virNodeDevicePtr dev)
     pciDevice *pci;
     unsigned domain, bus, slot, function;
     int ret = -1;
+    bool in_inactive_list = false;
 
     if (qemudNodeDeviceGetPciInfo(dev, &domain, &bus, &slot, &function) < 0)
         return -1;
@@ -9220,13 +9225,17 @@ qemudNodeDeviceDettach (virNodeDevicePtr dev)
         return -1;
 
     qemuDriverLock(driver);
-    if (pciDettachDevice(pci, driver->activePciHostdevs) < 0)
+    in_inactive_list = pciDeviceListFind(driver->inactivePciHostdevs, pci);
+
+    if (pciDettachDevice(pci, driver->activePciHostdevs,
+                         driver->inactivePciHostdevs) < 0)
         goto out;
 
     ret = 0;
 out:
     qemuDriverUnlock(driver);
-    pciFreeDevice(pci);
+    if (in_inactive_list)
+        pciFreeDevice(pci);
     return ret;
 }
 
@@ -9248,7 +9257,8 @@ qemudNodeDeviceReAttach (virNodeDevicePtr dev)
     pciDeviceReAttachInit(pci);
 
     qemuDriverLock(driver);
-    if (pciReAttachDevice(pci, driver->activePciHostdevs) < 0)
+    if (pciReAttachDevice(pci, driver->activePciHostdevs,
+                          driver->inactivePciHostdevs) < 0)
         goto out;
 
     ret = 0;
@@ -9275,7 +9285,8 @@ qemudNodeDeviceReset (virNodeDevicePtr dev)
 
     qemuDriverLock(driver);
 
-    if (pciResetDevice(pci, driver->activePciHostdevs, NULL) < 0)
+    if (pciResetDevice(pci, driver->activePciHostdevs,
+                       driver->inactivePciHostdevs) < 0)
         goto out;
 
     ret = 0;

diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c
index c7adb1d68097f30c3b4e9259b394bb1784b57e7c..b3cad8e7a0f2293c9f477b45a8a08e65a208076a 100644 (file)
--- a/src/qemu/qemu_hostdev.c
+++ b/src/qemu/qemu_hostdev.c
@@ -1,7 +1,7 @@
 /*
  * qemu_hostdev.c: QEMU hostdev management
  *
- * Copyright (C) 2006-2007, 2009-2011 Red Hat, Inc.
+ * Copyright (C) 2006-2007, 2009-2012 Red Hat, Inc.
  * Copyright (C) 2006 Daniel P. Berrange
  *
  * This library is free software; you can redistribute it and/or
@@ -212,7 +212,7 @@ int qemuPrepareHostdevPCIDevices(struct qemud_driver *driver,
     for (i = 0; i < pciDeviceListCount(pcidevs); i++) {
         pciDevice *dev = pciDeviceListGet(pcidevs, i);
         if (pciDeviceGetManaged(dev) &&
-            pciDettachDevice(dev, driver->activePciHostdevs) < 0)
+            pciDettachDevice(dev, driver->activePciHostdevs, NULL) < 0)
             goto reattachdevs;
     }
 
@@ -220,7 +220,8 @@ int qemuPrepareHostdevPCIDevices(struct qemud_driver *driver,
      * can safely reset them */
     for (i = 0; i < pciDeviceListCount(pcidevs); i++) {
         pciDevice *dev = pciDeviceListGet(pcidevs, i);
-        if (pciResetDevice(dev, driver->activePciHostdevs, pcidevs) < 0)
+        if (pciResetDevice(dev, driver->activePciHostdevs,
+                           driver->inactivePciHostdevs) < 0)
             goto reattachdevs;
     }
 
@@ -233,7 +234,13 @@ int qemuPrepareHostdevPCIDevices(struct qemud_driver *driver,
         }
     }
 
-    /* Loop 5: Now set the used_by_domain of the device in
+    /* Loop 5: Now remove the devices from inactive list. */
+    for (i = 0; i < pciDeviceListCount(pcidevs); i++) {
+         pciDevice *dev = pciDeviceListGet(pcidevs, i);
+         pciDeviceListDel(driver->inactivePciHostdevs, dev);
+    }
+
+    /* Loop 6: Now set the used_by_domain of the device in
      * driver->activePciHostdevs as domain name.
      */
     for (i = 0; i < pciDeviceListCount(pcidevs); i++) {
@@ -245,7 +252,7 @@ int qemuPrepareHostdevPCIDevices(struct qemud_driver *driver,
         pciDeviceSetUsedBy(activeDev, name);
     }
 
-    /* Loop 6: Now set the original states for hostdev def */
+    /* Loop 7: Now set the original states for hostdev def */
     for (i = 0; i < nhostdevs; i++) {
         pciDevice *dev;
         pciDevice *pcidev;
@@ -277,7 +284,7 @@ int qemuPrepareHostdevPCIDevices(struct qemud_driver *driver,
         pciFreeDevice(dev);
     }
 
-    /* Loop 7: Now steal all the devices from pcidevs */
+    /* Loop 8: Now steal all the devices from pcidevs */
     while (pciDeviceListCount(pcidevs) > 0) {
         pciDevice *dev = pciDeviceListGet(pcidevs, 0);
         pciDeviceListSteal(pcidevs, dev);
@@ -298,7 +305,7 @@ inactivedevs:
 reattachdevs:
     for (i = 0; i < pciDeviceListCount(pcidevs); i++) {
         pciDevice *dev = pciDeviceListGet(pcidevs, i);
-        pciReAttachDevice(dev, driver->activePciHostdevs);
+        pciReAttachDevice(dev, driver->activePciHostdevs, NULL);
     }
 
 cleanup:
@@ -445,8 +452,13 @@ void qemuReattachPciDevice(pciDevice *dev, struct qemud_driver *driver)
 {
     int retries = 100;
 
-    if (!pciDeviceGetManaged(dev))
+    /* If the device is not managed and was attached to guest
+     * successfully, it must have been inactive.
+     */
+    if (!pciDeviceGetManaged(dev)) {
+        pciDeviceListAdd(driver->inactivePciHostdevs, dev);
         return;
+    }
 
     while (pciWaitForDeviceCleanup(dev, "kvm_assigned_device")
            && retries) {
@@ -454,7 +466,8 @@ void qemuReattachPciDevice(pciDevice *dev, struct qemud_driver *driver)
         retries--;
     }
 
-    if (pciReAttachDevice(dev, driver->activePciHostdevs) < 0) {
+    if (pciReAttachDevice(dev, driver->activePciHostdevs,
+                          driver->inactivePciHostdevs) < 0) {
         virErrorPtr err = virGetLastError();
         VIR_ERROR(_("Failed to re-attach PCI device: %s"),
                   err ? err->message : _("unknown error"));
@@ -503,7 +516,8 @@ void qemuDomainReAttachHostdevDevices(struct qemud_driver *driver,
 
     for (i = 0; i < pciDeviceListCount(pcidevs); i++) {
         pciDevice *dev = pciDeviceListGet(pcidevs, i);
-        if (pciResetDevice(dev, driver->activePciHostdevs, pcidevs) < 0) {
+        if (pciResetDevice(dev, driver->activePciHostdevs,
+                           driver->inactivePciHostdevs) < 0) {
             virErrorPtr err = virGetLastError();
             VIR_ERROR(_("Failed to reset PCI device: %s"),
                       err ? err->message : _("unknown error"));

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index dc40d2f3f4aaf0521ee0d64826bb8156a00446a8..4b608397ac69ecd54afd7a40701f88a6a2ec7bc5 100644 (file)
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -2029,7 +2029,8 @@ qemuDomainDetachHostPciDevice(struct qemud_driver *driver,
                        detach->source.subsys.u.pci.function);
     if (pci) {
         activePci = pciDeviceListSteal(driver->activePciHostdevs, pci);
-        if (pciResetDevice(activePci, driver->activePciHostdevs, NULL))
+        if (pciResetDevice(activePci, driver->activePciHostdevs,
+                           driver->inactivePciHostdevs) == 0)
             qemuReattachPciDevice(activePci, driver);
         else
             ret = -1;

diff --git a/src/util/pci.c b/src/util/pci.c
index 17cde81bf921dc834e528570766bf491bebe449b..633dcd89ed500cb5e550e60e5fa5c06ba38043b4 100644 (file)
--- a/src/util/pci.c
+++ b/src/util/pci.c
@@ -1117,7 +1117,9 @@ cleanup:
 }
 
 int
-pciDettachDevice(pciDevice *dev, pciDeviceList *activeDevs)
+pciDettachDevice(pciDevice *dev,
+                 pciDeviceList *activeDevs,
+                 pciDeviceList *inactiveDevs)
 {
     const char *driver = pciFindStubDriver();
     if (!driver) {
@@ -1132,11 +1134,22 @@ pciDettachDevice(pciDevice *dev, pciDeviceList *activeDevs)
         return -1;
     }
 
-    return pciBindDeviceToStub(dev, driver);
+    if (pciBindDeviceToStub(dev, driver) < 0)
+        return -1;
+
+    /* Add the dev into list inactiveDevs */
+    if (inactiveDevs && !pciDeviceListFind(inactiveDevs, dev)) {
+        if (pciDeviceListAdd(inactiveDevs, dev) < 0)
+            return -1;
+    }
+
+    return 0;
 }
 
 int
-pciReAttachDevice(pciDevice *dev, pciDeviceList *activeDevs)
+pciReAttachDevice(pciDevice *dev,
+                  pciDeviceList *activeDevs,
+                  pciDeviceList *inactiveDevs)
 {
     const char *driver = pciFindStubDriver();
     if (!driver) {
@@ -1151,7 +1164,14 @@ pciReAttachDevice(pciDevice *dev, pciDeviceList *activeDevs)
         return -1;
     }
 
-    return pciUnbindDeviceFromStub(dev, driver);
+    if (pciUnbindDeviceFromStub(dev, driver) < 0)
+        return -1;
+
+    /* Steal the dev from list inactiveDevs */
+    if (inactiveDevs)
+        pciDeviceListSteal(inactiveDevs, dev);
+
+    return 0;
 }
 
 /* Certain hypervisors (like qemu/kvm) map the PCI bar(s) on

diff --git a/src/util/pci.h b/src/util/pci.h
index 5493e0aed0f90729170f63e2186713ff42c6ee97..25b5b662f9891211ad84242ffaf1ed519743defc 100644 (file)
--- a/src/util/pci.h
+++ b/src/util/pci.h
@@ -40,8 +40,12 @@ pciDevice *pciGetDevice      (unsigned       domain,
                               unsigned       function);
 void       pciFreeDevice     (pciDevice     *dev);
 const char *pciDeviceGetName (pciDevice     *dev);
-int        pciDettachDevice  (pciDevice     *dev, pciDeviceList *activeDevs);
-int        pciReAttachDevice (pciDevice     *dev, pciDeviceList *activeDevs);
+int        pciDettachDevice  (pciDevice     *dev,
+                              pciDeviceList *activeDevs,
+                              pciDeviceList *inactiveDevs);
+int        pciReAttachDevice (pciDevice     *dev,
+                              pciDeviceList *activeDevs,
+                              pciDeviceList *inactiveDevs);
 int        pciResetDevice    (pciDevice     *dev,
                               pciDeviceList *activeDevs,
                               pciDeviceList *inactiveDevs);

diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
index 20671c0d6af0804e5493491a6e118af05b773a63..520ec03a846e6e3cc74b24759b86c3b343934ab2 100644 (file)
--- a/src/xen/xen_driver.c
+++ b/src/xen/xen_driver.c
@@ -1985,7 +1985,7 @@ xenUnifiedNodeDeviceDettach (virNodeDevicePtr dev)
     if (!pci)
         return -1;
 
-    if (pciDettachDevice(pci, NULL) < 0)
+    if (pciDettachDevice(pci, NULL, NULL) < 0)
         goto out;
 
     ret = 0;
@@ -2075,7 +2075,7 @@ xenUnifiedNodeDeviceReAttach (virNodeDevicePtr dev)
         goto out;
     }
 
-    if (pciReAttachDevice(pci, NULL) < 0)
+    if (pciReAttachDevice(pci, NULL, NULL) < 0)
         goto out;
 
     ret = 0;