Re: [libvirt PATCH 08/20] qemu: Introduce virQEMUCapsAccelStr

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

 



On Fri, Jan 21, 2022 at 09:20:42AM -0800, Andrea Bolognani wrote:
> On Fri, Jan 21, 2022 at 04:57:34PM +0000, Daniel P. Berrangé wrote:
> > On Mon, Jan 17, 2022 at 11:52:48AM +0100, Andrea Bolognani wrote:
> > > +static const char *
> > > +virQEMUCapsAccelStr(virDomainVirtType type)
> > > +{
> > > +    if (type == VIR_DOMAIN_VIRT_QEMU)
> > > +        return "tcg";
> > > +
> > > +    return virDomainVirtTypeToString(type);
> > > +}
> >
> > I don't like this use of virDomainVirtTypeToString since the
> > vast majority of values it returns are not valid QEMU accelerators.
> >
> > I'd say we should just directly return the virt types we actually
> > expect to get - the invalid ones will have already been filtered
> > out. ie
> >
> >  virQEMUCapsAccelStr(virDomainVirtType type)
> >  {
> >      if (type == VIR_DOMAIN_VIRT_KVM)
> >          return "kvm";
> >      else
> >          return "tcg";
> >  }
> >
> > that is still easily extended to add hvf and so on, without being
> > misleading about supporting any virt type
> 
> The original version works because, as you note, we already make sure
> much earlier[1] that only values that correspond to valid QEMU
> accelerators can be used, but I'm okay with being more explicit.
> 
> I'd rather use a switch statement though, that way next time an
> accelerator has to be added there will be fewer spots where we can
> easily forget to add handling for it. Does that sound good?

I don't mind.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|




[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