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

[Jiri Denemark]

On Wed, Dec 05, 2012 at 10:42:14 +0800, li guang wrote:
> 在 2012-12-04二的 23:23 +0100，Jiri Denemark写道：
...
> > @@ -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.

Yes, they can be. But the purpose of this patch was to change the way PCI
config files are used. Combining that with other changes would make the patch
(which is already pretty big) harder to review.

...
> > @@ -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

Well, the line is being replaced with the following to lines by this patch:

> > +    pciWrite32(dev, cfgfd, dev->pci_pm_cap_pos + PCI_PM_CTRL,
> > +               ctl | PCI_PM_CTRL_STATE_D3hot);

Jirka