Re: [PATCH 08/11] conf: Prefer pcie-to-pci-bridge to dmi-to-pci-bridge

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 2018-04-03 at 19:13 -0400, John Ferlan wrote:
[...]
> >              needDMIToPCIBridge = true;
> > +            needPCIeToPCIBridge = true;
> >              for (i = 0; i < addrs->nbuses; i++) {
> >                  if (addrs->buses[i].flags & VIR_PCI_CONNECT_TYPE_PCI_BRIDGE) {
> >                      needDMIToPCIBridge = false;
> > +                    needPCIeToPCIBridge = false;
> >                      break;
> >                  }
> >              }
> > -            if (needDMIToPCIBridge && add == 1) {
> > +
> > +            /* Prefer pcie-to-pci-bridge, fall back to dmi-to-pci-bridge */
> > +            if (addrs->isPCIeToPCIBridgeSupported)
> > +                needDMIToPCIBridge = false;
> > +            else
> > +                needPCIeToPCIBridge = false;
> 
> The above seems a bit extra work and is a bit hard to read...  Could the
> previous for loop change to use a new bool "needXToPCIBridge"...
> 
> Then, I think it would just be:
> 
>     if (addrs->isPCIeToPCIBridgeSupported)
>         needPCIeToPCIBridge = needXToPCIBridge;
>     else
>         needDMIToPCIBridge = needXToPCIBridge;
> 
> with the following just being if (needXToPCIBridge && add == 1)
> 
> What you have works, just seems to be overkill or maybe I'm missing
> something subtle ;-).

I don't think you missed something, both version should work just
as fine. I happen to find my version easier to read than yours, so
I'd stick with that if you're okay with it - I guess it's just a
matter of preference at the end of the day...

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux