[libvirt] [PATCH 11/22] qemu: Implement virConnectCompareHypervisorCPU

[Jiri Denemark]

On Fri, May 25, 2018 at 13:50:19 -0400, Collin Walling wrote:
> Sorry for the delay. I've been experiencing issues with the mail server :(
> 
> On 05/16/2018 04:39 AM, Jiri Denemark wrote:
> > Signed-off-by: Jiri Denemark <jdenemar at redhat.com>
> > ---
> >  src/qemu/qemu_driver.c | 60 ++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 60 insertions(+)
> > 
> > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> > index 6c086b9ef8..4b48afdad1 100644
> > --- a/src/qemu/qemu_driver.c
> > +++ b/src/qemu/qemu_driver.c
> > @@ -13100,6 +13100,65 @@ qemuConnectCompareCPU(virConnectPtr conn,
> >  }
> >  
> >  
> > +static int
> > +qemuConnectCompareHypervisorCPU(virConnectPtr conn,
> > +                                const char *emulator,
> > +                                const char *archStr,
> > +                                const char *machine,
> > +                                const char *virttypeStr,
> > +                                const char *xmlCPU,
> > +                                unsigned int flags)
> > +{
> > +    int ret = VIR_CPU_COMPARE_ERROR;
> > +    virQEMUDriverPtr driver = conn->privateData;
> > +    virQEMUCapsPtr qemuCaps = NULL;
> > +    bool failIncompatible;
> > +    virCPUDefPtr hvCPU;
> > +    virArch arch;
> > +    virDomainVirtType virttype;
> > +
> > +    virCheckFlags(VIR_CONNECT_COMPARE_CPU_FAIL_INCOMPATIBLE,
> > +                  VIR_CPU_COMPARE_ERROR);
> > +
> > +    if (virConnectCompareHypervisorCPUEnsureACL(conn) < 0)
> > +        goto cleanup;
> > +
> > +    failIncompatible = !!(flags & VIR_CONNECT_COMPARE_CPU_FAIL_INCOMPATIBLE);
> > +
> > +    qemuCaps = virQEMUCapsCacheLookupDefault(driver->qemuCapsCache,
> > +                                             emulator,
> > +                                             archStr,
> > +                                             virttypeStr,
> > +                                             machine,
> > +                                             &arch, &virttype, NULL);
> > +    if (!qemuCaps)
> > +        goto cleanup;
> > +
> > +    hvCPU = virQEMUCapsGetHostModel(qemuCaps, virttype,
> > +                                    VIR_QEMU_CAPS_HOST_CPU_REPORTED);
> 
> nit: add a blank line here
> 
> > +    if (!hvCPU || hvCPU->fallback != VIR_CPU_FALLBACK_FORBID) {
> > +        virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
> > +                       _("QEMU '%s' does not support reporting CPU model for "
> > +                         "virttype '%s'"),
> > +                       virQEMUCapsGetBinary(qemuCaps),
> > +                       virDomainVirtTypeToString(virttype));
> > +        goto cleanup;
> > +    }
> > +
> > +    if (ARCH_IS_X86(arch)) {
> > +        ret = virCPUCompareXML(arch, hvCPU, xmlCPU, failIncompatible);
> > +    } else {
> > +        virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
> > +                       _("comparing hypervisor CPUs is not supported for "
> > +                         "arch %s"), virArchToString(arch));
> 
> At first glance, this message makes me think that this function is for "comparing two hypervisor CPUs".
> Perhaps the message should say "comparing with the hypervisor CPU is not supported for arch %s" instead? 
> I think that makes it more clear that the other CPU in question is not (necessarily) a hypervisor CPU.

Yeah, that's definitely better.

Jirka