[libvirt] [PATCH 07/22] qemu_capabilities: Introduce virQEMUCapsCacheLookupDefault

[Jiri Denemark]

On Tue, May 22, 2018 at 18:24:57 -0400, Collin Walling wrote:
> On 05/16/2018 04:39 AM, Jiri Denemark wrote:
> > virConnectGetDomainCapabilities needs to lookup QEMU capabilities
> > matching a specified binary, architecture, virt type, and machine type
> > while using default values when any of the parameters are not provided
> > by the user. Let's extract the lookup code into
> > virQEMUCapsCacheLookupDefault to make it reusable.
> > 
> > Signed-off-by: Jiri Denemark <jdenemar at redhat.com>
> > ---
> >  src/qemu/qemu_capabilities.c | 118 +++++++++++++++++++++++++++++++++++
> >  src/qemu/qemu_capabilities.h |   8 +++
> >  src/qemu/qemu_driver.c       |  86 ++++---------------------
> >  3 files changed, 137 insertions(+), 75 deletions(-)
> > 
> 
> [...]
> 
> >  
> > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> > index 9037818e2a..6c086b9ef8 100644
> > --- a/src/qemu/qemu_driver.c
> > +++ b/src/qemu/qemu_driver.c
> > @@ -19249,10 +19249,9 @@ qemuConnectGetDomainCapabilities(virConnectPtr conn,
> >      char *ret = NULL;
> >      virQEMUDriverPtr driver = conn->privateData;
> >      virQEMUCapsPtr qemuCaps = NULL;
> > -    int virttype = VIR_DOMAIN_VIRT_NONE;
> > -    virDomainVirtType capsType;
> > +    virArch arch;
> > +    virDomainVirtType virttype;
> >      virDomainCapsPtr domCaps = NULL;
> > -    int arch = virArchFromHost(); /* virArch */
> >      virQEMUDriverConfigPtr cfg = NULL;
> >      virCapsPtr caps = NULL;
> >  
> > @@ -19266,80 +19265,17 @@ qemuConnectGetDomainCapabilities(virConnectPtr conn,
> >      if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
> >          goto cleanup;
> >  
> > -    if (virttype_str &&
> > -        (virttype = virDomainVirtTypeFromString(virttype_str)) < 0) {
> > -        virReportError(VIR_ERR_INVALID_ARG,
> > -                       _("unknown virttype: %s"),
> > -                       virttype_str);
> > +    qemuCaps = virQEMUCapsCacheLookupDefault(driver->qemuCapsCache,
> > +                                             emulatorbin,
> > +                                             arch_str,
> > +                                             virttype_str,
> > +                                             machine,
> > +                                             &arch, &virttype, &machine);
> > +    if (!qemuCaps)
> >          goto cleanup;
> > -    }
> >  
> > -    if (arch_str && (arch = virArchFromString(arch_str)) == VIR_ARCH_NONE) {
> > -        virReportError(VIR_ERR_INVALID_ARG,
> > -                       _("unknown architecture: %s"),
> > -                       arch_str);
> > -        goto cleanup;
> > -    }
> > -
> > -    if (emulatorbin) {
> > -        virArch arch_from_caps;
> > -
> > -        if (!(qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache,
> > -                                                emulatorbin)))
> > -            goto cleanup;
> > -
> > -        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;
> > -        }
> 
> Are all these checks necessary? Can't you get away with just checking arch_from_caps != arch?

Yes, because i686 != x86_64, while ARCH_IS_X86(i686) &&
ARCH_IS_X86(x86_64) is true.

Jirka