Re: [libvirt] [PATCH v2 4/4] util: Do not keep PCI device config file open

5 Dec 2012

在 2012-12-04二的 23:23 +0100，Jiri Denemark写道：
...
Directly open and close PCI config file in the APIs that need it rather
than keeping the file open for the whole life of PCI device structure.
---
 src/util/pci.c | 265 +++++++++++++++++++++++++++++++++------------------------
 1 file changed, 156 insertions(+), 109 deletions(-)

diff --git a/src/util/pci.c b/src/util/pci.c
index e410245..8bded78 100644
[snip]
     /* otherwise, SRIOV allows VFs to be on different busses then their PFs.
      * In this case, what we need to do is look for the "best" match; i.e.
@@ -557,25 +572,38 @@ pciIsParent(pciDevice *dev, pciDevice *check, void *data)
         if (*best == NULL) {
             *best = pciGetDevice(check->domain, check->bus, check->slot,
                                  check->function);
-            if (*best == NULL)
-                return -1;
-        }
-        else {
       --    **     --
+            if (*best == NULL) {
+                ret = -1;
+                goto cleanup;
+            }
--first--
...
+        } else {
             /* OK, we had already recorded a previous "best" match for the
              * parent.  See if the current device is more restrictive than the
              * best, and if so, make it the new best
              */
-            if (secondary > pciRead8(*best, PCI_SECONDARY_BUS)) {
+            int bestfd;
+            uint8_t best_secondary;
+
+            if ((bestfd = pciConfigOpen(*best, false)) < 0)
+                goto cleanup;
+            best_secondary = pciRead8(*best, bestfd, PCI_SECONDARY_BUS);
+            pciConfigClose(*best, bestfd);
+
+            if (secondary > best_secondary) {
                 pciFreeDevice(*best);
                 *best = pciGetDevice(check->domain, check->bus, check->slot,
                                      check->function);
-                if (*best == NULL)
-                    return -1;
       --    **     --
+                if (*best == NULL) {
+                    ret = -1;
+                    goto cleanup;
+                }
--second--
...
}
         }
logically, the 2 'if (*best == NULL) {'
        can be combined and plcaced here.
...
}
-    return 0;
+cleanup:
+    pciConfigClose(check, fd);
+    return ret;
 }
static int
@@ -598,12 +626,14 @@ pciGetParentDevice(pciDevice *dev, pciDevice **parent)
  */
[snip]
...
@@ -669,7 +704,7 @@ out:
  * above we require the device supports a full internal reset.
  */
 static int
-pciTryPowerManagementReset(pciDevice *dev)
+pciTryPowerManagementReset(pciDevice *dev, int cfgfd)
 {
     uint8_t config_space[PCI_CONF_LEN];
     uint32_t ctl;
@@ -678,7 +713,7 @@ pciTryPowerManagementReset(pciDevice *dev)
         return -1;
/* Save and restore the device's config space. */
-    if (pciRead(dev, 0, &config_space[0], PCI_CONF_LEN) < 0) {
+    if (pciRead(dev, cfgfd, 0, &config_space[0], PCI_CONF_LEN) < 0) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        _("Failed to read PCI config space for %s"),
                        dev->name);
@@ -687,18 +722,20 @@ pciTryPowerManagementReset(pciDevice *dev)
VIR_DEBUG("%s %s: doing a power management reset", dev->id, dev->name);
-    ctl = pciRead32(dev, dev->pci_pm_cap_pos + PCI_PM_CTRL);
+    ctl = pciRead32(dev, cfgfd, dev->pci_pm_cap_pos + PCI_PM_CTRL);
     ctl &= ~PCI_PM_CTRL_STATE_MASK;
-    pciWrite32(dev, dev->pci_pm_cap_pos + PCI_PM_CTRL, ctl|PCI_PM_CTRL_STATE_D3hot);
seems more than 80 characters
...
+    pciWrite32(dev, cfgfd, dev->pci_pm_cap_pos + PCI_PM_CTRL,
+               ctl | PCI_PM_CTRL_STATE_D3hot);
usleep(10 * 1000); /* sleep 10ms */
-    pciWrite32(dev, dev->pci_pm_cap_pos + PCI_PM_CTRL, ctl|PCI_PM_CTRL_STATE_D0);
+    pciWrite32(dev, cfgfd, dev->pci_pm_cap_pos + PCI_PM_CTRL,
+               ctl | PCI_PM_CTRL_STATE_D0);
usleep(10 * 1000); /* sleep 10ms */
-    if (pciWrite(dev, 0, &config_space[0], PCI_CONF_LEN) < 0) {
+    if (pciWrite(dev, cfgfd, 0, &config_space[0], PCI_CONF_LEN) < 0) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        _("Failed to restore PCI config space for %s"),
                        dev->name);
[snip]
-- 
regards!
li guang

    