[PATCH] qemu: capabilities: Remove check for /usr/libexec/qemu-kvm

[Daniel P. Berrangé]

On Wed, Mar 30, 2022 at 09:59:11AM -0700, Andrea Bolognani wrote:
> On Mon, Mar 28, 2022 at 02:38:38PM -0600, Jim Fehlig wrote:
> > A downstream packaging bug resulted in a scenario where no aarch64 emulator
> > binary was installed on a kvm host. Querying capabilities on the host
> > succeeds and the capabilities correctly report that no <guest>'s are
> > supported, but the following error is logged
> >
> > libvirtd: Cannot check QEMU binary /usr/libexec/qemu-kvm: No such file or directory
> >
> > This error is confusing and not very helpful. Additionally, comments in the
> > associated code note that /usr/libexec/qemu-kvm is disto-specific, which
> > suggests the logic is better suited for a downstream patch. Removing the
> > check for /usr/libexec/qemu-kvm leaves virQEMUCapsGetDefaultEmulator() as
> > nothing more than a needless wrapper around virQEMUCapsFindBinaryForArch.
> > Drop virQEMUCapsGetDefaultEmulator() and call virQEMUCapsFindBinaryForArch()
> > directly in its place, which squelches the unhelpful error.
> 
> I agree that the message being logged is not very useful, but I don't
> think the approach you take here is the correct one: we want upstream
> libvirt to work out of the box when built on a variety of distros,
> including RHEL and derivatives, and your patch breaks that.
> 
> I think the diff you're looking for is more along the lines of the
> (completely untested) one below. You can then rename
> virQEMUCapsFindBinaryForArch() to virQEMUCapsGetDefaultEmulator() in
> a second patch to avoid leaving unnecessary wrappers around.
> 
> 
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 6b4ed08499..e35a89f944 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -945,6 +945,12 @@ virQEMUCapsFindBinaryForArch(virArch hostarch,
>              return ret;
>      }
> 
> +    if (virQEMUCapsGuestIsNative(hostarch, guestarch)) {
> +        if ((ret = virFindFileInPath("/usr/libexec/qemu-kvm")) != NULL) {
> +            return ret;
> +        }
> +    }
> +
>      return ret;
>  }

That overwrites the existing 'ret' value that we want to keep
when qemu-kvm isn't present. Needs

   if (!ret && virQEMUCapsGuestIsNative(hostarch, guestarch)) {
     ...
     


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