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

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

/* 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; + }

+ } 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; + }

} }

} - return 0; +cleanup: + pciConfigClose(check, fd); + return ret; } static int @@ -598,12 +626,14 @@ pciGetParentDevice(pciDevice *dev, pciDevice **parent) */

@@ -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);

+ 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);