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@xxxxxxxxxx>
> > ---
> > 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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
