[libvirt] [PATCH 3/3] qemu_monitor: query-cpu-model-baseline QMP command

[Chris Venteicher]

On 04/16/2018 03:02 AM, Ján Tomko wrote:
> On Mon, Apr 16, 2018 at 01:06:58AM -0500, Chris Venteicher wrote:
>> Function qemuMonitorGetCPUModelBaseline exposed to carry out a QMP
>> query-cpu-model-baseline transaction with QEMU.
>>
>> QEMU determines a baseline CPU Model from two input CPU Models to
>> complete the query-cpu-model-baseline transaction.
>> ---
>> src/qemu/qemu_monitor.c      | 16 +++++++++++++
>> src/qemu/qemu_monitor.h      |  5 ++++
>> src/qemu/qemu_monitor_json.c | 56 
>> ++++++++++++++++++++++++++++++++++++++++++++
>> src/qemu/qemu_monitor_json.h |  7 ++++++
>> 4 files changed, 84 insertions(+)
>>
>> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
>> index 7b647525b..9db9d4b81 100644
>> --- a/src/qemu/qemu_monitor.c
>> +++ b/src/qemu/qemu_monitor.c
>> @@ -3874,6 +3874,22 @@ qemuMonitorCPUModelInfoCopy(const 
>> qemuMonitorCPUModelInfo *orig)
>>     return NULL;
>> }
>>
>> +int
>> +qemuMonitorGetCPUModelBaseline(qemuMonitorPtr mon,
>> +                               qemuMonitorCPUModelInfoPtr model_a,
>> +                               qemuMonitorCPUModelInfoPtr model_b,
>> +                               qemuMonitorCPUModelInfoPtr 
>> *model_baseline)
>> +{
>> +    if (model_a)
>> +        VIR_DEBUG("model_a->name=%s", model_a->name);
>> +
>> +    if (model_b)
>> +        VIR_DEBUG("model_b->name=%s", model_b->name);
>> +
>> +    QEMU_CHECK_MONITOR_JSON(mon);
>> +
>> +    return qemuMonitorJSONGetCPUModelBaseline(mon, model_a, model_b, 
>> model_baseline);
>> +}
>>
>> int
>> qemuMonitorGetCommands(qemuMonitorPtr mon,
>> diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
>> index d04148e56..c7a80ca63 100644
>> --- a/src/qemu/qemu_monitor.h
>> +++ b/src/qemu/qemu_monitor.h
>> @@ -1079,6 +1079,11 @@ void 
>> qemuMonitorCPUModelInfoFree(qemuMonitorCPUModelInfoPtr model_info);
>> qemuMonitorCPUModelInfoPtr
>> qemuMonitorCPUModelInfoCopy(const qemuMonitorCPUModelInfo *orig);
>>
>> +int qemuMonitorGetCPUModelBaseline(qemuMonitorPtr mon,
>> +                                   qemuMonitorCPUModelInfoPtr model_a,
>> +                                   qemuMonitorCPUModelInfoPtr model_b,
>> +                                   qemuMonitorCPUModelInfoPtr 
>> *model_baseline);
>> +
>> int qemuMonitorGetCommands(qemuMonitorPtr mon,
>>                            char ***commands);
>> int qemuMonitorGetEvents(qemuMonitorPtr mon,
>> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
>> index 320d4601e..e03f6091c 100644
>> --- a/src/qemu/qemu_monitor_json.c
>> +++ b/src/qemu/qemu_monitor_json.c
>> @@ -5544,6 +5544,62 @@ 
>> qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon,
>>     return ret;
>> }
>>
>> +int
>> +qemuMonitorJSONGetCPUModelBaseline(qemuMonitorPtr mon,
>> +                                   qemuMonitorCPUModelInfoPtr model_a,
>> +                                   qemuMonitorCPUModelInfoPtr model_b,
>> +                                   qemuMonitorCPUModelInfoPtr 
>> *model_baseline)
>> +{
>> +    int ret = -1;
>> +    virJSONValuePtr cmd   = NULL;
>> +    virJSONValuePtr reply = NULL;
>> +    virJSONValuePtr data  = NULL;
>> +    virJSONValuePtr modela = NULL;
>> +    virJSONValuePtr modelb = NULL;
>
> Please do not try to align the =.
>
>> +
>> +    *model_baseline = NULL;
>> +
>> +    if (qemuMonitorJSONBuildCPUModelInfoJSON(model_a, &modela) < 0)
>> +        goto cleanup;
>> +
>> +    if (qemuMonitorJSONBuildCPUModelInfoJSON(model_b, &modelb) < 0)
>> +        goto cleanup;
>> +
>> +    if (!(cmd = qemuMonitorJSONMakeCommand("query-cpu-model-baseline",
>> +                                           "a:modela", &modela,
>> +                                           "a:modelb", &modelb,
>> +                                           NULL)))
>> +        goto cleanup;
>> +
>> +
>> +    if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
>> +        goto cleanup;
>> +
>> +    /* Urgh, some QEMU architectures have query-cpu-model-baseline
>> +     * command but return 'GenericError' with string "Not supported",
>> +     * instead of simply omitting the command entirely
>> +     */
>> +    if (qemuMonitorJSONHasError(reply, "GenericError"))
>> +        goto cleanup;
>
> Missing virReportError.
>
> Ideally, on error all libvirt functions would either call virReportError
> or be quiet in all possible exit paths.
Please see next version of patch set...
Returning 0 instead of -1 in response to reply "GenericError" to be 
consistent with other commands.

Otherwise, I think I am mirroring other functions in the file in terms 
of when virReportError is called.
Possible exception is error while parsing "return" json within 
qemuMonitorJSONBuildCPUModelInfoFromJSON... But virReportErrors are 
generated within that function if there found.

Please advise with specifics places I where I should be adding 
virReportErrors if you still see the problem in version 2 of the patch set.

Chris

>
> Jano
>
>
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20180418/1d792d97/attachment-0001.htm>