Re: [libvirt] [PATCH v4 5/8] qemu_monitor: implement query-cpu-model-comparison

On 7/17/19 4:03 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, incompatible) as well as a list > of properties (aka CPU features) responsible for the subset or > superset result. > > Signed-off-by: Collin Walling <walling(a)linux.ibm.com&gt; > Reviewed-by: Daniel Henrique Barboza <danielh413(a)gmail.com&gt; > --- > src/qemu/qemu_monitor.c | 22 ++++++++++ > src/qemu/qemu_monitor.h | 9 +++++ > src/qemu/qemu_monitor_json.c | 95 ++++++++++++++++++++++++++++++++++++++++++++ > src/qemu/qemu_monitor_json.h | 10 +++++ > 4 files changed, 136 insertions(+) > > diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c > index 136d3e2..b16e149 100644 > --- a/src/qemu/qemu_monitor.c > +++ b/src/qemu/qemu_monitor.c > @@ -3670,6 +3670,28 @@ qemuMonitorGetCPUModelBaseline(qemuMonitorPtr mon, > } > > > +int > +qemuMonitorGetCPUModelComparison(qemuMonitorPtr mon, > + const char *model_a_name, > + int model_a_nprops, > + virCPUFeatureDefPtr model_a_props, > + const char *model_b_name, > + int model_b_nprops, > + virCPUFeatureDefPtr model_b_props, > + qemuMonitorCPUModelInfoPtr *model_result) > +{ > + VIR_DEBUG("model_a_name=%s model_a_nprops=%d model_b_name=%s model_b_nprops=%d", > + model_a_name, model_a_nprops, model_b_name, model_b_nprops); > + > + QEMU_CHECK_MONITOR(mon); > + > + return qemuMonitorJSONGetCPUModelComparison(mon, model_a_name, model_a_nprops, > + model_a_props, model_b_name, > + model_b_nprops, model_b_props, > + model_result); > +} > + > + > void > qemuMonitorCPUModelInfoFree(qemuMonitorCPUModelInfoPtr model_info) > { > diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h > index 4ec74f1..f0cf70a 100644 > --- a/src/qemu/qemu_monitor.h > +++ b/src/qemu/qemu_monitor.h > @@ -1093,6 +1093,15 @@ int qemuMonitorGetCPUModelBaseline(qemuMonitorPtr mon, > virCPUFeatureDefPtr model_b_props, > qemuMonitorCPUModelInfoPtr *model_result); > > +int qemuMonitorGetCPUModelComparison(qemuMonitorPtr mon, > + const char *model_a_name, > + int model_a_nprops, > + virCPUFeatureDefPtr model_a_props, > + const char *model_b_name, > + int model_b_nprops, > + virCPUFeatureDefPtr model_b_props, > + qemuMonitorCPUModelInfoPtr *model_result); > + > qemuMonitorCPUModelInfoPtr > qemuMonitorCPUModelInfoCopy(const qemuMonitorCPUModelInfo *orig); > > diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c > index b599ee6..3c33c63 100644 > --- a/src/qemu/qemu_monitor_json.c > +++ b/src/qemu/qemu_monitor_json.c > @@ -5875,6 +5875,101 @@ qemuMonitorJSONGetCPUModelBaseline(qemuMonitorPtr mon, > } > > > +static int > +qemuMonitorJSONParseCPUModelPropName(size_t pos ATTRIBUTE_UNUSED, > + virJSONValuePtr item, > + void *opaque) > +{ > + if (!qemuMonitorJSONParseCPUModelProperty(virJSONValueGetString(item), > + item, opaque)) This need to be changed into if (qemuMonitorJSONParseCPUModelProperty(virJSONValueGetString(item), item, opaque) < 0) > + return -1; > + > + virJSONValueFree(item); > + return 0; > +} > + > + > +int > +qemuMonitorJSONGetCPUModelComparison(qemuMonitorPtr mon, > + const char *model_a_name, > + int model_a_nprops, > + virCPUFeatureDefPtr model_a_props, > + const char *model_b_name, > + int model_b_nprops, > + virCPUFeatureDefPtr model_b_props, > + qemuMonitorCPUModelInfoPtr *model_result) > +{ > + int ret = -1; > + virJSONValuePtr model_a; > + virJSONValuePtr model_b = NULL; > + virJSONValuePtr cmd = NULL; > + virJSONValuePtr reply = NULL; > + virJSONValuePtr data; > + const char *result_name; > + virJSONValuePtr props; > + qemuMonitorCPUModelInfoPtr compare = NULL; > + > + if (!(model_a = qemuMonitorJSONMakeCPUModel(model_a_name, model_a_nprops, > + model_a_props, true)) || > + !(model_b = qemuMonitorJSONMakeCPUModel(model_b_name, model_b_nprops, > + model_b_props, true))) > + goto cleanup; > + > + if (!(cmd = qemuMonitorJSONMakeCommand("query-cpu-model-comparison", > + "a:modela", &model_a, > + "a:modelb", &model_b, > + NULL))) > + goto cleanup; > + > + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) > + goto cleanup; > + > + if (qemuMonitorJSONCheckError(cmd, reply) < 0) Did you test scenarios which contain unknown cpu model name or unknown features? This caused errors for me during testing that resulted in e.g. error : qemuMonitorJSONCheckError:415 : internal error: unable to execute QEMU command 'query-cpu-model-comparison': Property '.boris' not found warning : virQEMUCapsLogProbeFailure:4586 : Failed to probe capabilities for /usr/bin/qemu-system-s390x: internal error: unable to execute QEMU command 'query-cpu-model-comparison': Property '.boris' not found If I think about a scenario like: I run "virsh domcapabilties" on a new machine with new cpu model and new cpu features, new libvirt, new qemu and use "virsh hypervisor-cpu-compare" with the output xml on my old machine with old cpu &feature, older libvirt and older qemu I would expect to get as an answer: Incompatible If my expectation to get incompatible is wrong please correct me but an "internal error" is not what should occur. What is the qemu specification for this?