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@linux.ibm.com>
Reviewed-by: Daniel Henrique Barboza <danielh413@gmail.com>
---
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))
+ 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)
+ goto cleanup;
+
+ data = virJSONValueObjectGetObject(reply, "return");
+
+ if (!(result_name = virJSONValueObjectGetString(data, "result"))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("query-cpu-model-comparison reply data was missing "
+ "'result'"));
+ goto cleanup;
+ }
+
+ if (!(props = virJSONValueObjectGetArray(data, "responsible-properties"))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("query-cpu-model-comparison reply data was missing "
+ "'responsible-properties'"));
+ goto cleanup;
+ }
+
+ if (VIR_ALLOC(compare) < 0)
+ goto cleanup;
+
+ if (VIR_STRDUP(compare->name, result_name) < 0)
+ goto cleanup;
+
+ if (VIR_ALLOC_N(compare->props, virJSONValueArraySize(props)) < 0)
+ goto cleanup;
+
+ if (virJSONValueArrayForeachSteal(props,
+ qemuMonitorJSONParseCPUModelPropName,
+ compare) < 0)
+ goto cleanup;
+
+ VIR_STEAL_PTR(*model_result, compare);
+ ret = 0;
+
+ cleanup:
+ qemuMonitorCPUModelInfoFree(compare);
+ virJSONValueFree(cmd);
+ virJSONValueFree(reply);
+ virJSONValueFree(model_a);
+ virJSONValueFree(model_b);
+ return ret;
+}
+
+
int qemuMonitorJSONGetCommands(qemuMonitorPtr mon,
char ***commands)
{
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index d3ee2fe..f7a8e99 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -378,6 +378,16 @@ int qemuMonitorJSONGetCPUModelBaseline(qemuMonitorPtr mon,
qemuMonitorCPUModelInfoPtr *model_result)
ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(5) ATTRIBUTE_NONNULL(8);
+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)
+ ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(5) ATTRIBUTE_NONNULL(8);
+
int qemuMonitorJSONGetCommands(qemuMonitorPtr mon,
char ***commands)
ATTRIBUTE_NONNULL(2);
--
2.7.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
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@linux.ibm.com>
> Reviewed-by: Daniel Henrique Barboza <danielh413@gmail.com>
> ---
> 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?
> + goto cleanup;
> +
> + data = virJSONValueObjectGetObject(reply, "return");
> +
> + if (!(result_name = virJSONValueObjectGetString(data, "result"))) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("query-cpu-model-comparison reply data was missing "
> + "'result'"));
> + goto cleanup;
> + }
> +
> + if (!(props = virJSONValueObjectGetArray(data, "responsible-properties"))) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("query-cpu-model-comparison reply data was missing "
> + "'responsible-properties'"));
> + goto cleanup;
> + }
> +
> + if (VIR_ALLOC(compare) < 0)
> + goto cleanup;
> +
> + if (VIR_STRDUP(compare->name, result_name) < 0)
> + goto cleanup;
> +
> + if (VIR_ALLOC_N(compare->props, virJSONValueArraySize(props)) < 0)
> + goto cleanup;
> +
> + if (virJSONValueArrayForeachSteal(props,
> + qemuMonitorJSONParseCPUModelPropName,
> + compare) < 0)
> + goto cleanup;
> +
> + VIR_STEAL_PTR(*model_result, compare);
> + ret = 0;
> +
> + cleanup:
> + qemuMonitorCPUModelInfoFree(compare);
> + virJSONValueFree(cmd);
> + virJSONValueFree(reply);
> + virJSONValueFree(model_a);
> + virJSONValueFree(model_b);
> + return ret;
> +}
> +
> +
> int qemuMonitorJSONGetCommands(qemuMonitorPtr mon,
> char ***commands)
> {
> diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
> index d3ee2fe..f7a8e99 100644
> --- a/src/qemu/qemu_monitor_json.h
> +++ b/src/qemu/qemu_monitor_json.h
> @@ -378,6 +378,16 @@ int qemuMonitorJSONGetCPUModelBaseline(qemuMonitorPtr mon,
> qemuMonitorCPUModelInfoPtr *model_result)
> ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(5) ATTRIBUTE_NONNULL(8);
>
> +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)
> + ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(5) ATTRIBUTE_NONNULL(8);
> +
> int qemuMonitorJSONGetCommands(qemuMonitorPtr mon,
> char ***commands)
> ATTRIBUTE_NONNULL(2);
>
--
Mit freundlichen Grüßen/Kind regards
Boris Fiuczynski
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Matthias Hartmann
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 7/24/19 12:18 PM, Boris Fiuczynski wrote:
> On 7/17/19 4:03 PM, Collin Walling wrote:
[...]
>> 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)
>
Yes, thank you for catching this one.
>> + 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?
>
>
Two routes:
1) Fix up the logic in virQEMUCapsCPUModelComparison
Please see my comments on patch 8.
2) Change the error class in the QEMU QMP code
The QMP code in QEMU currently reports a "GenericError" whenever
something goes wrong (such as an unknown CPU model or feature). A
possible solution to this would be to alter that code to set a specific
error class, have libvirt check for that specific error type in the
JSON, then handle it appropriately to print a cleaner message.
There is one drawback: we would technically be reporting this error
twice -- once in virsh, and another in the libvirt daemon (the internal
error) unless we wanted to suppress it.
This is one solution without too much messy code.
>> + goto cleanup;
>> +
>> + data = virJSONValueObjectGetObject(reply, "return");
>> +
>> + if (!(result_name = virJSONValueObjectGetString(data, "result"))) {
>> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> + _("query-cpu-model-comparison reply data was
>> missing "
>> + "'result'"));
>> + goto cleanup;
>> + }
>> +
>> + if (!(props = virJSONValueObjectGetArray(data,
>> "responsible-properties"))) {
>> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> + _("query-cpu-model-comparison reply data was
>> missing "
>> + "'responsible-properties'"));
>> + goto cleanup;
>> + }
>> +
>> + if (VIR_ALLOC(compare) < 0)
>> + goto cleanup;
>> +
>> + if (VIR_STRDUP(compare->name, result_name) < 0)
>> + goto cleanup;
>> +
>> + if (VIR_ALLOC_N(compare->props, virJSONValueArraySize(props)) < 0)
>> + goto cleanup;
>> +
>> + if (virJSONValueArrayForeachSteal(props,
>> +
>> qemuMonitorJSONParseCPUModelPropName,
>> + compare) < 0)
>> + goto cleanup;
>> +
>> + VIR_STEAL_PTR(*model_result, compare);
>> + ret = 0;
>> +
>> + cleanup:
>> + qemuMonitorCPUModelInfoFree(compare);
>> + virJSONValueFree(cmd);
>> + virJSONValueFree(reply);
>> + virJSONValueFree(model_a);
>> + virJSONValueFree(model_b);
>> + return ret;
>> +}
>> +
>> +
[...]
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 7/25/19 8:26 PM, Collin Walling wrote:
>>> +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?
>>
>>
>
> Two routes:
>
> 1) Fix up the logic in virQEMUCapsCPUModelComparison
>
> Please see my comments on patch 8.
>
> 2) Change the error class in the QEMU QMP code
>
> The QMP code in QEMU currently reports a "GenericError" whenever
> something goes wrong (such as an unknown CPU model or feature). A
> possible solution to this would be to alter that code to set a specific
> error class, have libvirt check for that specific error type in the
> JSON, then handle it appropriately to print a cleaner message.
>
> There is one drawback: we would technically be reporting this error
> twice -- once in virsh, and another in the libvirt daemon (the internal
> error) unless we wanted to suppress it.
>
> This is one solution without too much messy code.
>
Or you prevent the generic error by not making the call at all in the
case QEMU does not know CPU model or features. To be able to do this you
either need lists of all known CPU models and features (preferable) or
you need to ask QEMU if it knows the CPU model and features by use of
other QMP commands that provide a consumable response.
I would not simply turn an QEMU internal error into an answer
"incompatible" and I am not a fan of parsing thru QEMU internal errors
that turns the answer into "incompatible".
Regarding patch 8: The option you outline in patch 8 returns "CPU is
incompatible" for every QEMU error and not only "CPU model or feature"
unknown. Therefore I think that it is not a good idea.
--
Mit freundlichen Grüßen/Kind regards
Boris Fiuczynski
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Matthias Hartmann
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Wed, Jul 24, 2019 at 18:18:27 +0200, Boris Fiuczynski wrote:
> 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@linux.ibm.com>
> > Reviewed-by: Daniel Henrique Barboza <danielh413@gmail.com>
> > ---
> > 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?
Actually returning error in such case is the correct way and also
consistent with how it works for x86. The only difference is we report
these errors ourselves for x86 while s390 would report the error from
QEMU. The QEMU error is not exactly nice, but it is correct. So adding
any code which would allow libvirt detect these errors and report better
error messages would be a nice enhancement, but it's not really
necessary.
Jirka
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Wed, Jul 17, 2019 at 10:03:26 -0400, 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@linux.ibm.com> > Reviewed-by: Daniel Henrique Barboza <danielh413@gmail.com> > --- > 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(+) Most of the comments from 2/8 are applicable here as well. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2026 Red Hat, Inc.