[libvirt PATCH] qemu: fixing auto-detecting binary in domain capabilities

[Daniel P. Berrangé]

On Fri, Jan 17, 2020 at 02:54:53PM +0100, Ján Tomko wrote:
> On Fri, Jan 17, 2020 at 01:29:36PM +0000, Daniel P. Berrangé wrote:
> > The virConnectGetDomainCapabilities API accepts either a binary path
> > to the emulator, or desired guest arch. If guest arch is not given,
> > then the host arch is assumed.
> > 
> > In the case where the binary is not given, the code tried to find the
> > emulator binary in the existing list of cached emulator capabilities.
> > This is not valid since we switched to lazy population of the cache in:
> > 
> >  commit 3dd91af01f30c5bda6328454ef49f3afece755d6
> >  Author: Daniel P. Berrangé <berrange at redhat.com>
> >  Date:   Mon Dec 2 13:04:26 2019 +0000
> > 
> >    qemu: stop creating capabilities at driver startup
> > 
> > As a result of this change, if there are no persistent guests defined
> > using the requested guest architecture, virConnectGetDomainCapabilities
> > will fail to find an emulator binary.
> > 
> > The solution is to stop relying on the cached capabilities to find the
> > binary and instead use the same logic we use to pick default a binary
> > per arch when populating capabilities.
> > 
> 
> This does fix the problem for machines with no persistent guests, but
> breaks domcapabilities for machines where there is no QEMU in the
> default location - before this patch I'd get domcapabilities for /usr/libexec/qemu-git
> (taken from the persistent config), after I get an error:
> error: failed to get emulator capabilities
> error: Cannot check QEMU binary /usr/libexec/qemu-kvm: No such file or directory
> 
> Should we keep the cache lookup as a fallback or vice-versa?

Hmm, I think it is probably undesirable for the results of this
API to vary depending on whether you happened to have defined a
guest config. IOW, this difference is probably a good thing and
we should clarify that if no binary is passed, we're only looking
in the default location.

> > @@ -5299,31 +5301,25 @@ virQEMUCapsCacheLookupDefault(virFileCachePtr cache,
> >         goto cleanup;
> >     }
> > 
> > -    if (binary) {
> > -        virArch arch_from_caps;
> > +    if (!binary)
> > +        binary = virQEMUCapsGetDefaultEmulator(hostarch, arch);
> 
> virQEMUCapsGetDefaultEmulator returns an allocated string so it needs to
> be freed.

Opps, yes.


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 :|