[libvirt PATCH] qemu: fix missing error reports in capabilities probing

[Michal Privoznik]

On 6/15/20 3:56 PM, Daniel P. Berrangé wrote:
> The "virsh domcapabilities --arch ppc64" command will fail with no
> error message set if qemu-system-ppc64 is not currently installed.
> 
> This is because virQEMUCapsCacheLookup() does not report any error
> message if not capabilities can be obtained from the cache. Almost
> all methods calling this expected an error to be set on failure.
> 
> Once that's fixed though, we see a further bug which is that
> virQEMUCapsCacheLookupDefault() is passing a NULL binary path to
> virQEMUCapsCacheLookup(), so we need to catch that too.
> 
> Signed-off-by: Daniel P. Berrangé <berrange at redhat.com>
> ---
>   src/qemu/qemu_capabilities.c | 11 +++++++++++
>   src/qemu/qemu_domain.c       |  4 +++-
>   2 files changed, 14 insertions(+), 1 deletion(-)
> 


> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 2dad823a86..97096073e6 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -6068,8 +6068,10 @@ qemuDomainPostParseDataAlloc(const virDomainDef *def,
>       virQEMUDriverPtr driver = opaque;
>   
>       if (!(*parseOpaque = virQEMUCapsCacheLookup(driver->qemuCapsCache,
> -                                                def->emulator)))
> +                                                def->emulator))) {
> +        virResetLastError();
>           return 1;
> +    }
>   
>       return 0;
>   }
> 

I think we need to revisit this patch. In my testing, I have qemu built 
on a side from git and one domain that runs it. As I updated my system, 
but not rebuilt the qemu from git it no longer runs (fails to link):

~/work/qemu/qemu.git/x86_64-softmmu/qemu-system-x86_64: error while 
loading shared libraries: libnettle.so.7: cannot open shared object 
file: No such file or directory

This is expected. But, virsh start fails with:

error: Failed to start domain fedora
error: An error occurred, but the cause is unknown

I've tracked this down to the virResetLastError() in the hunk above. And 
it kind of makes sense - we failed to load capabilities on daemon 
startup (oh, yeah, daemon runs from git too, but it's been rebuilt) so 
we try again on domain startup. But now that I am reading the commit 
message it doesn't make much sense to me either. 'virsh domcapabilities' 
has nothing to do with PostParse callbacks, does it? Can it be that this 
error reset call is just misplaced?

Michal