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

Re: [libvirt] [PATCH 1/4] qemu: Automatically choose usable GIC version



On Tue, 2016-05-10 at 18:42 -0400, Cole Robinson wrote:
> > 
> > +        if (virQEMUCapsFillDomainCaps(caps, qemuCaps, NULL, 0) < 0)
> > +            goto cleanup;
> > +
> > +        gic = &(caps->gic);
> > +
> > +        /* Pick the best GIC version from those available */
> > +        if (gic->supported) {
> > +            virGICVersion version;
> > +
> > +            VIR_DEBUG("Looking for usable GIC version in domain capabilities");
> > +            for (version = VIR_GIC_VERSION_LAST - 1;
> > +                 version > VIR_GIC_VERSION_NONE;
> > +                 version--) {
> > +                if (VIR_DOMAIN_CAPS_ENUM_IS_SET(gic->version, version)) {
> > +
> > +                    VIR_DEBUG("Using GIC version %s",
> > +                              virGICVersionTypeToString(version));
> > +                    def->gic_version = version;
> > +                    break;
> > +                }
> > +            }
> >          }
> 
> Hmm that's a bit of a new pattern... it seems the only thing you really need
> from domcaps is the bit of logic we encode via
> virQEMUCapsFillDomainFeatureGICCaps. Maybe break that logic out into a public
> function and call it here, rather than spinning up domcaps for a small bit of
> info? Or is there more to it?

Nothing more to it :)

Do you mean I should make virQEMUCapsFillDomainFeatureGICCaps()
public and use it here to fill only the part of the domain
capabilities I'm actually going to use, or create a new function
altogether?

Because right now I'm not seeing a way to do the latter without
introducing some code duplication or making things quite a bit
uglier... Maybe I'm just tired :)

-- 
Andrea Bolognani
Software Engineer - Virtualization Team


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