src/qemu/qemu_capabilities.c | 42 ++++++++++++++++-------------------- 1 file changed, 19 insertions(+), 23 deletions(-)
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@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.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
src/qemu/qemu_capabilities.c | 42 ++++++++++++++++--------------------
1 file changed, 19 insertions(+), 23 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 498348ad58..9017e8d920 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -5280,10 +5280,12 @@ virQEMUCapsCacheLookupDefault(virFileCachePtr cache,
const char **retMachine)
{
int virttype = VIR_DOMAIN_VIRT_NONE;
- int arch = virArchFromHost();
+ virArch hostarch = virArchFromHost();
+ virArch arch = hostarch;
virDomainVirtType capsType;
virQEMUCapsPtr qemuCaps = NULL;
virQEMUCapsPtr ret = NULL;
+ virArch arch_from_caps;
if (virttypeStr &&
(virttype = virDomainVirtTypeFromString(virttypeStr)) < 0) {
@@ -5299,31 +5301,25 @@ virQEMUCapsCacheLookupDefault(virFileCachePtr cache,
goto cleanup;
}
- if (binary) {
- virArch arch_from_caps;
+ if (!binary)
+ binary = virQEMUCapsGetDefaultEmulator(hostarch, arch);
- if (!(qemuCaps = virQEMUCapsCacheLookup(cache, binary)))
- goto cleanup;
+ if (!(qemuCaps = virQEMUCapsCacheLookup(cache, binary)))
+ goto cleanup;
- arch_from_caps = virQEMUCapsGetArch(qemuCaps);
+ arch_from_caps = virQEMUCapsGetArch(qemuCaps);
- if (arch_from_caps != arch &&
- !((ARCH_IS_X86(arch) && ARCH_IS_X86(arch_from_caps)) ||
- (ARCH_IS_PPC(arch) && ARCH_IS_PPC(arch_from_caps)) ||
- (ARCH_IS_ARM(arch) && ARCH_IS_ARM(arch_from_caps)) ||
- (ARCH_IS_S390(arch) && ARCH_IS_S390(arch_from_caps)))) {
- virReportError(VIR_ERR_INVALID_ARG,
- _("architecture from emulator '%s' doesn't "
- "match given architecture '%s'"),
- virArchToString(arch_from_caps),
- virArchToString(arch));
- goto cleanup;
- }
- } else {
- if (!(qemuCaps = virQEMUCapsCacheLookupByArch(cache, arch)))
- goto cleanup;
-
- binary = virQEMUCapsGetBinary(qemuCaps);
+ if (arch_from_caps != arch &&
+ !((ARCH_IS_X86(arch) && ARCH_IS_X86(arch_from_caps)) ||
+ (ARCH_IS_PPC(arch) && ARCH_IS_PPC(arch_from_caps)) ||
+ (ARCH_IS_ARM(arch) && ARCH_IS_ARM(arch_from_caps)) ||
+ (ARCH_IS_S390(arch) && ARCH_IS_S390(arch_from_caps)))) {
+ virReportError(VIR_ERR_INVALID_ARG,
+ _("architecture from emulator '%s' doesn't "
+ "match given architecture '%s'"),
+ virArchToString(arch_from_caps),
+ virArchToString(arch));
+ goto cleanup;
}
if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM))
--
2.23.0
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@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.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Fixes the problem over here, so:
Tested-by: Richard W.M. Jones <rjones@redhat.com>
Thanks,
Rich.
> ---
> src/qemu/qemu_capabilities.c | 42 ++++++++++++++++--------------------
> 1 file changed, 19 insertions(+), 23 deletions(-)
>
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 498348ad58..9017e8d920 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -5280,10 +5280,12 @@ virQEMUCapsCacheLookupDefault(virFileCachePtr cache,
> const char **retMachine)
> {
> int virttype = VIR_DOMAIN_VIRT_NONE;
> - int arch = virArchFromHost();
> + virArch hostarch = virArchFromHost();
> + virArch arch = hostarch;
> virDomainVirtType capsType;
> virQEMUCapsPtr qemuCaps = NULL;
> virQEMUCapsPtr ret = NULL;
> + virArch arch_from_caps;
>
> if (virttypeStr &&
> (virttype = virDomainVirtTypeFromString(virttypeStr)) < 0) {
> @@ -5299,31 +5301,25 @@ virQEMUCapsCacheLookupDefault(virFileCachePtr cache,
> goto cleanup;
> }
>
> - if (binary) {
> - virArch arch_from_caps;
> + if (!binary)
> + binary = virQEMUCapsGetDefaultEmulator(hostarch, arch);
>
> - if (!(qemuCaps = virQEMUCapsCacheLookup(cache, binary)))
> - goto cleanup;
> + if (!(qemuCaps = virQEMUCapsCacheLookup(cache, binary)))
> + goto cleanup;
>
> - arch_from_caps = virQEMUCapsGetArch(qemuCaps);
> + arch_from_caps = virQEMUCapsGetArch(qemuCaps);
>
> - if (arch_from_caps != arch &&
> - !((ARCH_IS_X86(arch) && ARCH_IS_X86(arch_from_caps)) ||
> - (ARCH_IS_PPC(arch) && ARCH_IS_PPC(arch_from_caps)) ||
> - (ARCH_IS_ARM(arch) && ARCH_IS_ARM(arch_from_caps)) ||
> - (ARCH_IS_S390(arch) && ARCH_IS_S390(arch_from_caps)))) {
> - virReportError(VIR_ERR_INVALID_ARG,
> - _("architecture from emulator '%s' doesn't "
> - "match given architecture '%s'"),
> - virArchToString(arch_from_caps),
> - virArchToString(arch));
> - goto cleanup;
> - }
> - } else {
> - if (!(qemuCaps = virQEMUCapsCacheLookupByArch(cache, arch)))
> - goto cleanup;
> -
> - binary = virQEMUCapsGetBinary(qemuCaps);
> + if (arch_from_caps != arch &&
> + !((ARCH_IS_X86(arch) && ARCH_IS_X86(arch_from_caps)) ||
> + (ARCH_IS_PPC(arch) && ARCH_IS_PPC(arch_from_caps)) ||
> + (ARCH_IS_ARM(arch) && ARCH_IS_ARM(arch_from_caps)) ||
> + (ARCH_IS_S390(arch) && ARCH_IS_S390(arch_from_caps)))) {
> + virReportError(VIR_ERR_INVALID_ARG,
> + _("architecture from emulator '%s' doesn't "
> + "match given architecture '%s'"),
> + virArchToString(arch_from_caps),
> + virArchToString(arch));
> + goto cleanup;
> }
>
> if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM))
> --
> 2.23.0
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine. Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/
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@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?
>Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
>---
> src/qemu/qemu_capabilities.c | 42 ++++++++++++++++--------------------
> 1 file changed, 19 insertions(+), 23 deletions(-)
>
>diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
>index 498348ad58..9017e8d920 100644
>--- a/src/qemu/qemu_capabilities.c
>+++ b/src/qemu/qemu_capabilities.c
>@@ -5280,10 +5280,12 @@ virQEMUCapsCacheLookupDefault(virFileCachePtr cache,
> const char **retMachine)
> {
> int virttype = VIR_DOMAIN_VIRT_NONE;
>- int arch = virArchFromHost();
>+ virArch hostarch = virArchFromHost();
>+ virArch arch = hostarch;
> virDomainVirtType capsType;
> virQEMUCapsPtr qemuCaps = NULL;
> virQEMUCapsPtr ret = NULL;
>+ virArch arch_from_caps;
>
> if (virttypeStr &&
> (virttype = virDomainVirtTypeFromString(virttypeStr)) < 0) {
>@@ -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.
Jano
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@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 :|
© 2016 - 2024 Red Hat, Inc.