Re: [libvirt] [PATCH v1 4/4] qemu: monitor: implement query-cpu-model-comparison

On 4/15/19 5:59 PM, Collin Walling wrote: > Interfaces with QEMU to compare CPU models. The command takes two > CPU models, A and B, that are given a model name and an optional list > of CPU features. Through the query-cpu-model-comparison command issued > via QMP, a result is produced that contains the comparison evaluation > string (identical, superset, subset [s390x specific], incompatible) > as well as a list of properties (aka CPU features) responsible for > the subset or superset result. > > Though the "responsible properties" are captured, they are not > reported by libvirt. > > This command is hooked into the virsh hypervisor-cpu-compare command. > As such, the CPU model XML provided to the command will be compared > to the hypervisor CPU contained in the QEMU capabilities file for the > appropriate QEMU binary (for s390x, this CPU definition can be observed > via virsh domcapabilities). > > s390x can report that the first model (A) is a subset of the second > (B). If A is the hypervisor CPU and a subset of B (the CPU contained > in the XML), then B would not be able to run on this machine and thus > we will report that CPU model B is incompatible. > > Signed-off-by: Collin Walling <walling(a)linux.ibm.com&gt; > --- >   src/qemu/qemu_capabilities.c | 47 +++++++++++++++++++ >   src/qemu/qemu_capabilities.h |  9 ++++ >   src/qemu/qemu_driver.c       | 10 ++++ >   src/qemu/qemu_monitor.c      | 22 +++++++++ >   src/qemu/qemu_monitor.h      | 10 ++++ >   src/qemu/qemu_monitor_json.c | 89 ++++++++++++++++++++++++++++++++++++ >   src/qemu/qemu_monitor_json.h | 11 +++++ >   7 files changed, 198 insertions(+) > > diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c > index 6163fd5399..1e4b05d535 100644 > --- a/src/qemu/qemu_capabilities.c > +++ b/src/qemu/qemu_capabilities.c > @@ -5400,3 +5400,50 @@ virQEMUCapsStripMachineAliases(virQEMUCapsPtr > qemuCaps) >       for (i = 0; i < qemuCaps->nmachineTypes; i++) >           VIR_FREE(qemuCaps->machineTypes[i].alias); >   } > + > +virCPUCompareResult > +virQEMUCapsCPUModelComparison(virQEMUCapsPtr qemuCaps, > +                              const char *libDir, > +                              uid_t runUid, > +                              gid_t runGid, > +                              virCPUDefPtr cpu_a, > +                              virCPUDefPtr cpu_b, > +                              bool failIncompatible) > +{ > +    qemuProcessQMPPtr proc = NULL; > +    qemuMonitorCPUModelInfoPtr result; > +    int ret = -1; > + > +    if (!(proc = qemuProcessQMPNew(qemuCaps->binary, libDir, > +                                   runUid, runGid, false))) > +        goto cleanup; > + > +    if (qemuProcessQMPStart(proc) < 0) > +        goto cleanup; > + > +    if (qemuMonitorGetCPUModelComparison(proc->mon, cpu_a->model, > +                                         cpu_a->nfeatures, > cpu_a->features, > +                                         cpu_b->model, cpu_b->nfeatures, > +                                         cpu_b->features, &result) < 0) > +        goto cleanup; > + > +    if (STREQ(result->name, "incompatible") || > +        STREQ(result->name, "subset")) > +        ret = VIR_CPU_COMPARE_INCOMPATIBLE; > +    else if (STREQ(result->name, "identical")) > +        ret = VIR_CPU_COMPARE_IDENTICAL; > +    else if (STREQ(result->name, "superset")) > +        ret = VIR_CPU_COMPARE_SUPERSET; > + > +    if (failIncompatible && ret == VIR_CPU_COMPARE_INCOMPATIBLE) { > +        ret = VIR_CPU_COMPARE_ERROR; > +        virReportError(VIR_ERR_CPU_INCOMPATIBLE, NULL); > +    } > + > + cleanup: > +    if (ret < 0) > +        virQEMUCapsLogProbeFailure(qemuCaps->binary); > + > +    qemuProcessQMPFree(proc); Don't you need to free 'result' here as well? This pointer is being passed down to 'qemuMonitorJSONGetCPUModelComparison', receiving the value of 'compare', which is VIR_ALLOC'd inside that function.