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

[Collin Walling]

On 4/17/19 4:19 PM, Daniel Henrique Barboza wrote:
> 
> 
> 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 at linux.ibm.com>
>> ---
>>   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.
> 
> 
> 
> 

You are correct. I there were a few pointers I forgot to free up 
littered around these patches. I hope to have them all resolved before
posting the next version.

[...]