Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> On 7/17/19 4:03 PM, Collin Walling wrote:

Refactor some code in qemuMonitorJSONGetCPUModelExpansion to be later used for the comparison and baseline functions.

This does not alter any functionality.

Signed-off-by: Collin Walling <walling@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_monitor_json.c | 173 ++++++++++++++++++++++++++++++------------- 1 file changed, 121 insertions(+), 52 deletions(-)

diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 8723ff4..f6bf7f2 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -5616,6 +5616,120 @@ qemuMonitorJSONParseCPUModelProperty(const char *key, return 0; }

+ +static virJSONValuePtr +qemuMonitorJSONMakeCPUModel(const char *model_name, + size_t nprops, + virCPUFeatureDefPtr props, + bool migratable) +{ + virJSONValuePtr value; + virJSONValuePtr feats = NULL; + size_t i; + + if (!(value = virJSONValueNewObject())) + goto cleanup; + + if (virJSONValueObjectAppendString(value, "name", model_name) < 0) + goto cleanup; + + if (nprops || !migratable) { + if (!(feats = virJSONValueNewObject())) + goto cleanup; + + for (i = 0; i < nprops; i++) { + char *name = props[i].name; + bool enabled = false; + + /* policy may be reported as -1 if the CPU def is a host model */ + if (props[i].policy == VIR_CPU_FEATURE_REQUIRE || + props[i].policy == VIR_CPU_FEATURE_FORCE || + props[i].policy == -1) + enabled = true; + + if (virJSONValueObjectAppendBoolean(feats, name, enabled) < 0) + goto cleanup; + } + + if (!migratable && + virJSONValueObjectAppendBoolean(feats, "migratable", false) < 0) { + goto cleanup; + } + + if (virJSONValueObjectAppend(value, "props", feats) < 0) + goto cleanup; + } + + return value; + + cleanup: + virJSONValueFree(value); + virJSONValueFree(feats); + return NULL; +} + + +static int +qemuMonitorJSONParseCPUModelData(virJSONValuePtr data, + virJSONValuePtr *cpu_model, + virJSONValuePtr *cpu_props, + const char **cpu_name, + const char *cmd_name) +{ + if (!(*cpu_model = virJSONValueObjectGetObject(data, "model"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("%s reply data was missing 'model'"), cmd_name); + return -1; + } + + if (!(*cpu_name = virJSONValueObjectGetString(*cpu_model, "name"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("%s reply data was missing 'name'"), cmd_name); + return -1; + } + + /* Some older CPU models do not report properties */ + *cpu_props = virJSONValueObjectGetObject(*cpu_model, "props"); + + return 0; +} + + +static int +qemuMonitorJSONParseCPUModel(const char *cpu_name, + virJSONValuePtr cpu_props, + qemuMonitorCPUModelInfoPtr *model_info) +{ + qemuMonitorCPUModelInfoPtr machine_model = NULL; + int ret = -1; + + if (VIR_ALLOC(machine_model) < 0) + goto cleanup; + + if (VIR_STRDUP(machine_model->name, cpu_name) < 0) + goto cleanup; + + if (cpu_props) { + size_t nprops = virJSONValueObjectKeysNumber(cpu_props); + + if (VIR_ALLOC_N(machine_model->props, nprops) < 0) + goto cleanup; + + if (virJSONValueObjectForeachKeyValue(cpu_props, + qemuMonitorJSONParseCPUModelProperty, + machine_model) < 0) + goto cleanup; + } + + VIR_STEAL_PTR(*model_info, machine_model); + ret = 0; + + cleanup: + qemuMonitorCPUModelInfoFree(machine_model); + return ret; +} + + int qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon, qemuMonitorCPUModelExpansionType type, @@ -5624,33 +5738,20 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon, qemuMonitorCPUModelInfoPtr *model_info) { int ret = -1; - virJSONValuePtr model = NULL; - virJSONValuePtr props = NULL; + virJSONValuePtr model; virJSONValuePtr cmd = NULL; virJSONValuePtr reply = NULL; virJSONValuePtr data; virJSONValuePtr cpu_model; - virJSONValuePtr cpu_props; - qemuMonitorCPUModelInfoPtr machine_model = NULL; - char const *cpu_name; + virJSONValuePtr cpu_props = NULL; + const char *cpu_name = ""; const char *typeStr = "";

*model_info = NULL;

- if (!(model = virJSONValueNewObject())) + if (!(model = qemuMonitorJSONMakeCPUModel(model_name, 0, NULL, migratable))) goto cleanup;

- if (virJSONValueObjectAppendString(model, "name", model_name) < 0) - goto cleanup; - - if (!migratable) { - if (!(props = virJSONValueNewObject()) || - virJSONValueObjectAppendBoolean(props, "migratable", false) < 0 || - virJSONValueObjectAppend(model, "props", props) < 0) - goto cleanup; - props = NULL; - } - retry: switch (type) { case QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC: @@ -5686,11 +5787,9 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon,

data = virJSONValueObjectGetObject(reply, "return");

- if (!(cpu_model = virJSONValueObjectGetObject(data, "model"))) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("query-cpu-model-expansion reply data was missing 'model'")); + if (qemuMonitorJSONParseCPUModelData(data, &cpu_model, &cpu_props, &cpu_name, + "query-cpu-model-expansion") < 0) goto cleanup; - }

/* QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC_FULL requests "full" expansion * on the result of the initial "static" expansion. @@ -5705,42 +5804,12 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon, goto retry; }

- if (!(cpu_name = virJSONValueObjectGetString(cpu_model, "name"))) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("query-cpu-model-expansion reply data was missing 'name'")); - goto cleanup; - } - - if (!(cpu_props = virJSONValueObjectGetObject(cpu_model, "props"))) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("query-cpu-model-expansion reply data was missing 'props'")); - goto cleanup; - } - - if (VIR_ALLOC(machine_model) < 0) - goto cleanup; - - if (VIR_STRDUP(machine_model->name, cpu_name) < 0) - goto cleanup; - - if (VIR_ALLOC_N(machine_model->props, virJSONValueObjectKeysNumber(cpu_props)) < 0) - goto cleanup; - - if (virJSONValueObjectForeachKeyValue(cpu_props, - qemuMonitorJSONParseCPUModelProperty, - machine_model) < 0) - goto cleanup; - - ret = 0; - *model_info = machine_model; - machine_model = NULL; + ret = qemuMonitorJSONParseCPUModel(cpu_name, cpu_props, model_info);

cleanup: - qemuMonitorCPUModelInfoFree(machine_model); virJSONValueFree(cmd); virJSONValueFree(reply); virJSONValueFree(model); - virJSONValueFree(props); return ret; }

-- 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

On Wed, Jul 17, 2019 at 10:03:22 -0400, Collin Walling wrote:

Refactor some code in qemuMonitorJSONGetCPUModelExpansion to be later used for the comparison and baseline functions.

You're mixing refactoring with adding new functionality. Splitting such changes into separate patches is better. Moving some parts of existing functions into new standalone functions is easier to review if the new code matches the old one. It's not a big deal in this case, but since you'll need to respin the series for other issues, you can also split this patch.

This does not alter any functionality.

Signed-off-by: Collin Walling <walling@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_monitor_json.c | 173 ++++++++++++++++++++++++++++++------------- 1 file changed, 121 insertions(+), 52 deletions(-)

diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 8723ff4..f6bf7f2 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -5616,6 +5616,120 @@ qemuMonitorJSONParseCPUModelProperty(const char *key, return 0; }

+ +static virJSONValuePtr +qemuMonitorJSONMakeCPUModel(const char *model_name, + size_t nprops, + virCPUFeatureDefPtr props,

Adding virCPUFeatureDefPtr parameter to this function should go into a separate patch. And for consistency, I believe we tend to pass the pointer first followed by number of items.

+ bool migratable) +{ + virJSONValuePtr value; + virJSONValuePtr feats = NULL; + size_t i; + + if (!(value = virJSONValueNewObject())) + goto cleanup; + + if (virJSONValueObjectAppendString(value, "name", model_name) < 0) + goto cleanup; + + if (nprops || !migratable) { + if (!(feats = virJSONValueNewObject())) + goto cleanup; + + for (i = 0; i < nprops; i++) { + char *name = props[i].name; + bool enabled = false; + + /* policy may be reported as -1 if the CPU def is a host model */ + if (props[i].policy == VIR_CPU_FEATURE_REQUIRE || + props[i].policy == VIR_CPU_FEATURE_FORCE || + props[i].policy == -1) + enabled = true;

The naming is quite confusing. The virCPUDef structure contains an array of features and you call it "props" while QEMU calls them "props" in the QMP protocol and your variable containing them is called "feats".

+ + if (virJSONValueObjectAppendBoolean(feats, name, enabled) < 0) + goto cleanup; + } + + if (!migratable && + virJSONValueObjectAppendBoolean(feats, "migratable", false) < 0) { + goto cleanup; + } + + if (virJSONValueObjectAppend(value, "props", feats) < 0) + goto cleanup; + } + + return value; + + cleanup:

"cleanup" label is used if the code following it is shared between both success and failure paths. Since it's not the case here, you should call it "error".

+ virJSONValueFree(value); + virJSONValueFree(feats); + return NULL; +} + + +static int +qemuMonitorJSONParseCPUModelData(virJSONValuePtr data, + virJSONValuePtr *cpu_model,

cpu_model is not used anywhere outside this function, it's just a temporary variable to store the container object for "model" and "props".

+ virJSONValuePtr *cpu_props, + const char **cpu_name, + const char *cmd_name)

I'd move the cmd_name parameter to the second place after data so the input parameters go first, followed by output parameters.

+{ + if (!(*cpu_model = virJSONValueObjectGetObject(data, "model"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("%s reply data was missing 'model'"), cmd_name); + return -1; + } + + if (!(*cpu_name = virJSONValueObjectGetString(*cpu_model, "name"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("%s reply data was missing 'name'"), cmd_name); + return -1; + } + + /* Some older CPU models do not report properties */ + *cpu_props = virJSONValueObjectGetObject(*cpu_model, "props");

This change should go into a separate patch with an explanation. Also, it only applies to s390. Missing "props" should still result in an error on x86. I think the callers should decide whether this is fatal or not.

+ + return 0; +} + + +static int +qemuMonitorJSONParseCPUModel(const char *cpu_name, + virJSONValuePtr cpu_props, + qemuMonitorCPUModelInfoPtr *model_info) +{ + qemuMonitorCPUModelInfoPtr machine_model = NULL; + int ret = -1; + + if (VIR_ALLOC(machine_model) < 0) + goto cleanup; + + if (VIR_STRDUP(machine_model->name, cpu_name) < 0) + goto cleanup; + + if (cpu_props) {

This is connected to making "props" optional.

+ size_t nprops = virJSONValueObjectKeysNumber(cpu_props); + + if (VIR_ALLOC_N(machine_model->props, nprops) < 0) + goto cleanup; + + if (virJSONValueObjectForeachKeyValue(cpu_props, + qemuMonitorJSONParseCPUModelProperty, + machine_model) < 0) + goto cleanup; + } + + VIR_STEAL_PTR(*model_info, machine_model); + ret = 0; + + cleanup: + qemuMonitorCPUModelInfoFree(machine_model); + return ret; +} + + int qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon, qemuMonitorCPUModelExpansionType type, ...

Jirka

On 7/17/19 4:03 PM, Collin Walling wrote:

Interfaces with QEMU to baseline 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-baseline command issued via QMP, a result is produced that contains a new baselined CPU model that is guaranteed to run on both A and B.

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 | 61 ++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 10 ++++++++ 4 files changed, 102 insertions(+)

diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 731be2e..136d3e2 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3648,6 +3648,28 @@ qemuMonitorGetCPUModelExpansion(qemuMonitorPtr mon, }

+int +qemuMonitorGetCPUModelBaseline(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 qemuMonitorJSONGetCPUModelBaseline(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 c41428b..4ec74f1 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -1084,6 +1084,15 @@ int qemuMonitorGetCPUModelExpansion(qemuMonitorPtr mon,

void qemuMonitorCPUModelInfoFree(qemuMonitorCPUModelInfoPtr model_info);

+int qemuMonitorGetCPUModelBaseline(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 f6bf7f2..b599ee6 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -5814,6 +5814,67 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon, }

+int +qemuMonitorJSONGetCPUModelBaseline(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; + virJSONValuePtr cpu_model; + virJSONValuePtr cpu_props = NULL; + const char *cpu_name = ""; + qemuMonitorCPUModelInfoPtr baseline = 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-baseline", + "a:modela", &model_a, + "a:modelb", &model_b, + NULL))) + goto cleanup; + + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; + + if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_OBJECT) < 0) + goto cleanup;

Did you test scenarios which contain unknown cpu model name or unknown features? I have not yet tested this but please see my reply on patch 5.

+ + data = virJSONValueObjectGetObject(reply, "return"); + + if (qemuMonitorJSONParseCPUModelData(data, &cpu_model, &cpu_props, &cpu_name, + "query-cpu-model-baseline") < 0) + goto cleanup; + + if (qemuMonitorJSONParseCPUModel(cpu_name, cpu_props, &baseline) < 0) + goto cleanup; + + VIR_STEAL_PTR(*model_result, baseline); + ret = 0; + + cleanup: + qemuMonitorCPUModelInfoFree(baseline); Wouldn't

ret = qemuMonitorJSONParseCPUModel(cpu_name, cpu_props, model_result); cleanup: virJSONValueFree(cmd); virJSONValueFree(reply); virJSONValueFree(model_a); virJSONValueFree(model_b); return ret; just do the trick as well since qemuMonitorJSONParseCPUModel already handles the model_info allocation?

+ 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 d0b519c..d3ee2fe 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -368,6 +368,16 @@ int qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon, qemuMonitorCPUModelInfoPtr *model_info) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(5);

+int qemuMonitorJSONGetCPUModelBaseline(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

Besides the comment below Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> On 7/17/19 4:03 PM, Collin Walling wrote:

This command is hooked into the virsh hypervisor-cpu-baseline command. As such, the CPU models provided in the XML sent to the command will be baselined with the CPU contained in the QEMU capabilities file for the appropriate QEMU binary (for s390x, this CPU definition can be observed via virsh domcapabilities).

Signed-off-by: Collin Walling <walling@linux.ibm.com> Reviewed-by: Daniel Henrique Barboza <danielh413@gmail.com> --- src/qemu/qemu_capabilities.c | 84 ++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_capabilities.h | 7 ++++ src/qemu/qemu_driver.c | 27 ++++++++++++++ 3 files changed, 118 insertions(+)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index b898113..ab337fa 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -5574,3 +5574,87 @@ virQEMUCapsStripMachineAliases(virQEMUCapsPtr qemuCaps) for (i = 0; i < qemuCaps->nmachineTypes; i++) VIR_FREE(qemuCaps->machineTypes[i].alias); } + + +static int +virQEMUCapsStealCPUModelFromInfo(virCPUDefPtr dst, + qemuMonitorCPUModelInfoPtr *src) +{ + qemuMonitorCPUModelInfoPtr info = *src; + size_t i; + + virCPUDefFreeModel(dst); + + VIR_STEAL_PTR(dst->model, info->name); + + for (i = 0; i < info->nprops; i++) { + char *name = info->props[i].name; + int policy = VIR_CPU_FEATURE_REQUIRE; + + if (info->props[i].type == QEMU_MONITOR_CPU_PROPERTY_BOOLEAN && + !info->props[i].value.boolean) + policy = VIR_CPU_FEATURE_DISABLE; + + if (virCPUDefAddFeature(dst, name, policy) < 0) + goto error; + } + + qemuMonitorCPUModelInfoFree(info); + *src = NULL; + return 0; + + error: + virCPUDefFree(dst); + return -1; +} + + +virCPUDefPtr +virQEMUCapsCPUModelBaseline(virQEMUCapsPtr qemuCaps, + const char *libDir, + uid_t runUid, + gid_t runGid, + int ncpus, + virCPUDefPtr *cpus) +{ + qemuMonitorCPUModelInfoPtr result = NULL; + qemuProcessQMPPtr proc = NULL; + virCPUDefPtr cpu = NULL; + virCPUDefPtr baseline = NULL; + size_t i; + + if (VIR_ALLOC(cpu) < 0) + goto cleanup; + + if (virCPUDefCopyModel(cpu, cpus[0], false)) + goto cleanup; + + if (!(proc = qemuProcessQMPNew(qemuCaps->binary, libDir, + runUid, runGid, false))) + goto cleanup; + + if (qemuProcessQMPStart(proc) < 0) + goto cleanup; + + for (i = 1; i < ncpus; i++) { + if (qemuMonitorGetCPUModelBaseline(proc->mon, cpu->model, + cpu->nfeatures, cpu->features, + cpus[i]->model, cpus[i]->nfeatures, + cpus[i]->features, &result) < 0) + goto cleanup; + + if (virQEMUCapsStealCPUModelFromInfo(cpu, &result) < 0) + goto cleanup; + } + + VIR_STEAL_PTR(baseline, cpu); + + cleanup: + if (!baseline) + virQEMUCapsLogProbeFailure(qemuCaps->binary); + + qemuMonitorCPUModelInfoFree(result); Is qemuMonitorCPUModelInfoFree really necessary? Are qemuMonitorGetCPUModelBaseline and virQEMUCapsStealCPUModelFromInfo not taking care of the mem allocation on the error paths that result in the goto cleanup?

+ qemuProcessQMPFree(proc); + virCPUDefFree(cpu); + return baseline; +} diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 8dde759..b49c0b9 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -665,3 +665,10 @@ virQEMUCapsGetSEVCapabilities(virQEMUCapsPtr qemuCaps);

virArch virQEMUCapsArchFromString(const char *arch); const char *virQEMUCapsArchToString(virArch arch); + +virCPUDefPtr virQEMUCapsCPUModelBaseline(virQEMUCapsPtr qemuCaps, + const char *libDir, + uid_t runUid, + gid_t runGid, + int ncpus, + virCPUDefPtr *cpus); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d3a144a..37b9c75 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -13553,6 +13553,7 @@ qemuConnectBaselineHypervisorCPU(virConnectPtr conn, unsigned int flags) { virQEMUDriverPtr driver = conn->privateData; + virQEMUDriverConfigPtr config = driver->config; virCPUDefPtr *cpus = NULL; virQEMUCapsPtr qemuCaps = NULL; virArch arch; @@ -13607,6 +13608,32 @@ qemuConnectBaselineHypervisorCPU(virConnectPtr conn, if (!(cpu = virCPUBaseline(arch, cpus, ncpus, cpuModels, (const char **)features, migratable))) goto cleanup; + } else if (ARCH_IS_S390(arch) && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_CPU_MODEL_BASELINE)) { + /* Add a copy of the hypervisor CPU to the list */ + virCPUDefPtr hvCPU, tmp; + size_t allocated = ncpus + 1; + + hvCPU = virQEMUCapsGetHostModel(qemuCaps, virttype, + VIR_QEMU_CAPS_HOST_CPU_REPORTED); + if (VIR_ALLOC(tmp) < 0) + goto cleanup; + + if (virCPUDefCopyModel(tmp, hvCPU, false)) + goto cleanup; + + if (VIR_INSERT_ELEMENT(cpus, ncpus, allocated, tmp) < 0) + goto cleanup; + + ncpus++; + + if (!(cpu = virQEMUCapsCPUModelBaseline(qemuCaps, config->libDir, + config->user, config->group, + ncpus, cpus))) + goto cleanup; + + if (!(flags & VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES)) + virCPUDefFreeFeatures(cpu); } else { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, _("computing baseline hypervisor CPU is not supported "

-- 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

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; +} + +

[...]

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

Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> On 7/17/19 4:03 PM, Collin Walling wrote:

Implement an XML to virCPUDefPtr helper that handles the ctxt prerequisite for virCPUDefParseXML.

This does not alter any functionality.

Signed-off-by: Collin Walling <walling@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com> Reviewed-by: Daniel Henrique Barboza <danielh413@gmail.com> --- src/conf/cpu_conf.c | 30 ++++++++++++++++++++++++++++++ src/conf/cpu_conf.h | 6 ++++++ src/cpu/cpu.c | 14 +------------- src/libvirt_private.syms | 1 + 4 files changed, 38 insertions(+), 13 deletions(-)

diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c index 7d16a05..c587aff 100644 --- a/src/conf/cpu_conf.c +++ b/src/conf/cpu_conf.c @@ -268,6 +268,36 @@ virCPUDefCopy(const virCPUDef *cpu) }

+int +virCPUDefParseXMLHelper(const char *xml, + const char *xpath, + virCPUType type, + virCPUDefPtr *cpu) +{ + xmlDocPtr doc = NULL; + xmlXPathContextPtr ctxt = NULL; + int ret = -1; + + if (!xml) { + virReportError(VIR_ERR_INVALID_ARG, "%s", _("missing CPU definition")); + goto cleanup; + } + + if (!(doc = virXMLParseStringCtxt(xml, _("(CPU_definition)"), &ctxt))) + goto cleanup; + + if (virCPUDefParseXML(ctxt, xpath, type, cpu) < 0) + goto cleanup; + + ret = 0; + + cleanup: + xmlFreeDoc(doc); + xmlXPathFreeContext(ctxt); + return ret; +} + + /* * Parses CPU definition XML from a node pointed to by @xpath. If @xpath is * NULL, the current node of @ctxt is used (i.e., it is a shortcut to "."). diff --git a/src/conf/cpu_conf.h b/src/conf/cpu_conf.h index 19ce816..a0efac8 100644 --- a/src/conf/cpu_conf.h +++ b/src/conf/cpu_conf.h @@ -183,6 +183,12 @@ virCPUDefPtr virCPUDefCopyWithoutModel(const virCPUDef *cpu);

int +virCPUDefParseXMLHelper(const char *xml, + const char *xpath, + virCPUType type, + virCPUDefPtr *cpu); + +int virCPUDefParseXML(xmlXPathContextPtr ctxt, const char *xpath, virCPUType mode, diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c index b89462c..31ee887 100644 --- a/src/cpu/cpu.c +++ b/src/cpu/cpu.c @@ -111,31 +111,19 @@ virCPUCompareXML(virArch arch, const char *xml, bool failIncompatible) { - xmlDocPtr doc = NULL; - xmlXPathContextPtr ctxt = NULL; virCPUDefPtr cpu = NULL; virCPUCompareResult ret = VIR_CPU_COMPARE_ERROR;

VIR_DEBUG("arch=%s, host=%p, xml=%s", virArchToString(arch), host, NULLSTR(xml));

- if (!xml) { - virReportError(VIR_ERR_INVALID_ARG, "%s", _("missing CPU definition")); - goto cleanup; - } - - if (!(doc = virXMLParseStringCtxt(xml, _("(CPU_definition)"), &ctxt))) - goto cleanup; - - if (virCPUDefParseXML(ctxt, NULL, VIR_CPU_TYPE_AUTO, &cpu) < 0) + if (virCPUDefParseXMLHelper(xml, NULL, VIR_CPU_TYPE_AUTO, &cpu) < 0) goto cleanup;

ret = virCPUCompare(arch, host, cpu, failIncompatible);

cleanup: virCPUDefFree(cpu); - xmlXPathFreeContext(ctxt); - xmlFreeDoc(doc);

return ret; } diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 4e77cf5..ea5edab 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -90,6 +90,7 @@ virCPUDefIsEqual; virCPUDefListFree; virCPUDefListParse; virCPUDefParseXML; +virCPUDefParseXMLHelper; virCPUDefStealModel; virCPUDefUpdateFeature; virCPUModeTypeToString;

-- 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

On Wed, Jul 17, 2019 at 10:03:29 -0400, Collin Walling wrote:

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@linux.ibm.com> Reviewed-by: Daniel Henrique Barboza <danielh413@gmail.com> --- src/qemu/qemu_capabilities.c | 48 ++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_capabilities.h | 9 +++++++++ src/qemu/qemu_driver.c | 11 ++++++++++ 3 files changed, 68 insertions(+)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index a99ebcb..20cb82b 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -5660,3 +5660,51 @@ virQEMUCapsCPUModelBaseline(virQEMUCapsPtr qemuCaps, virCPUDefFree(cpu); return baseline; } + +virCPUCompareResult +virQEMUCapsCPUModelComparison(virQEMUCapsPtr qemuCaps, + const char *libDir, + uid_t runUid, + gid_t runGid, + virCPUDefPtr cpu_a, + virCPUDefPtr cpu_b, + bool failIncompatible)

This function should go into qemu_driver.c and its name should change accordingly.

+{ + qemuProcessQMPPtr proc = NULL; + qemuMonitorCPUModelInfoPtr result = NULL; + 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);

Again, no logging here, please. The reported error is already going to be logged anyway. And mainly since this is called from inside an API invoked by a client, the client will get the error. Probing is different as there's no API involved, QEMU binaries are probed when libvirtd starts.

+ + qemuMonitorCPUModelInfoFree(result); + qemuProcessQMPFree(proc); + return ret; +} diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 8afa05c..a62499f 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -673,3 +673,12 @@ virCPUDefPtr virQEMUCapsCPUModelBaseline(virQEMUCapsPtr qemuCaps, gid_t runGid, int ncpus, virCPUDefPtr *cpus); + +virCPUCompareResult +virQEMUCapsCPUModelComparison(virQEMUCapsPtr qemuCaps, + const char *libDir, + uid_t runUid, + gid_t runGid, + virCPUDefPtr cpu_a, + virCPUDefPtr cpu_b, + bool failIncompatible); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 37b9c75..3e49c04 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -13446,9 +13446,11 @@ qemuConnectCompareHypervisorCPU(virConnectPtr conn, { int ret = VIR_CPU_COMPARE_ERROR; virQEMUDriverPtr driver = conn->privateData; + virQEMUDriverConfigPtr config = driver->config;

VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver);

virQEMUCapsPtr qemuCaps = NULL; bool failIncompatible; virCPUDefPtr hvCPU; + virCPUDefPtr cpu; virArch arch; virDomainVirtType virttype;

@@ -13483,6 +13485,14 @@ qemuConnectCompareHypervisorCPU(virConnectPtr conn,

if (ARCH_IS_X86(arch)) { ret = virCPUCompareXML(arch, hvCPU, xmlCPU, failIncompatible); + } else if (ARCH_IS_S390(arch) && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_CPU_MODEL_COMPARISON)) { + if (virCPUDefParseXMLHelper(xmlCPU, NULL, VIR_CPU_TYPE_AUTO, &cpu) < 0) + goto cleanup; + + ret = virQEMUCapsCPUModelComparison(qemuCaps, config->libDir, + config->user, config->group, + hvCPU, cpu, failIncompatible); } else { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, _("comparing with the hypervisor CPU is not supported " @@ -13490,6 +13500,7 @@ qemuConnectCompareHypervisorCPU(virConnectPtr conn, }

cleanup: + virCPUDefFree(cpu); virObjectUnref(qemuCaps); return ret; }

Jirka