On Mon, Jul 13, 2020 at 09:44:07PM +0200, Michal Privoznik wrote:
> 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@xxxxxxxxxx>
> > ---
> > 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?
I was attempting to preserve what I thought was existing behaviour.
I was seeing that virFileCacheLookup returned NULL, but didn't set
any error. So I added an error report in virQEMUCapsCacheLookup,
and thus in reurn clearer the eror in virQEMUCapsCacheLookup.
It looks like i was mistaken about virFileCacheLookup tough - it
does indeed set an error, but not in all cases. So I thin we need
to revert part of my commit.
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 :|
