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

[li guang]

在 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