Refactor some code in qemuMonitorJSONGetCPUModelExpansion to be later used for the comparison and baseline functions. Signed-off-by: Collin Walling <walling(a)linux.ibm.com&gt; Reviewed-by: Bjoern Walk <bwalk(a)linux.ibm.com&gt; Reviewed-by: Daniel Henrique Barboza <danielhb413(a)gmail.com&gt; --- src/qemu/qemu_monitor_json.c | 143 ++++++++++++++++++++++++++++--------------- 1 file changed, 94 insertions(+), 49 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index e4404f0..7219d14 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -5780,6 +5780,96 @@ qemuMonitorJSONParseCPUModelProperty(const char *key, return 0; } + +static virJSONValuePtr +qemuMonitorJSONMakeCPUModel(const char *model_name, + bool migratable) +{ + virJSONValuePtr model = NULL; + virJSONValuePtr props = NULL; + + if (!(model = virJSONValueNewObject())) + goto error; + + if (virJSONValueObjectAppendString(model, "name", model_name) < 0) + goto error; + + if (!migratable) { + if (!(props = virJSONValueNewObject()) || + virJSONValueObjectAppendBoolean(props, "migratable", false) < 0 || + virJSONValueObjectAppend(model, "props", props) < 0) + goto error; + props = NULL; + } + + return model; + + error: + virJSONValueFree(model); + virJSONValueFree(props); + return NULL; +} + + +static int +qemuMonitorJSONParseCPUModelData(virJSONValuePtr data, + virJSONValuePtr *cpu_model, + virJSONValuePtr *cpu_props, + const char **cpu_name) +{ + if (!(*cpu_model = virJSONValueObjectGetObject(data, "model"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-cpu-model-expansion reply data was missing 'model'")); + return -1; + } + + if (!(*cpu_name = virJSONValueObjectGetString(*cpu_model, "name"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-cpu-model-expansion reply data was missing 'name'")); + return -1; + } + + if (!(*cpu_props = virJSONValueObjectGetObject(*cpu_model, "props"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-cpu-model-expansion reply data was missing 'props'")); + return -1; + } + + 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 (VIR_ALLOC_N(machine_model->props, virJSONValueObjectKeysNumber(cpu_props)) < 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, @@ -5789,32 +5879,19 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon, { int ret = -1; virJSONValuePtr model = NULL; - virJSONValuePtr props = NULL; virJSONValuePtr cmd = NULL; virJSONValuePtr reply = NULL; virJSONValuePtr data; virJSONValuePtr cpu_model; virJSONValuePtr cpu_props; - qemuMonitorCPUModelInfoPtr machine_model = NULL; char const *cpu_name; const char *typeStr = ""; *model_info = NULL; - if (!(model = virJSONValueNewObject())) - goto cleanup; - - if (virJSONValueObjectAppendString(model, "name", model_name) < 0) + if (!(model = qemuMonitorJSONMakeCPUModel(model_name, migratable))) 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: @@ -5850,11 +5927,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) < 0) goto cleanup; - } /* QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC_FULL requests "full" expansion * on the result of the initial "static" expansion. @@ -5869,42 +5944,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; } -- 2.7.4

With refactoring most of the expansion function, let's take care of some additional cleanups. Signed-off-by: Collin Walling <walling(a)linux.ibm.com&gt; --- src/qemu/qemu_monitor_json.c | 37 ++++++++++++++----------------------- 1 file changed, 14 insertions(+), 23 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 7219d14..3282593 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -5877,20 +5877,19 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon, bool migratable, qemuMonitorCPUModelInfoPtr *model_info) { - int ret = -1; - virJSONValuePtr model = NULL; - virJSONValuePtr cmd = NULL; - virJSONValuePtr reply = NULL; + VIR_AUTOPTR(virJSONValue) model = NULL; + VIR_AUTOPTR(virJSONValue) cmd = NULL; + VIR_AUTOPTR(virJSONValue) reply = NULL; virJSONValuePtr data; virJSONValuePtr cpu_model; - virJSONValuePtr cpu_props; - char const *cpu_name; + virJSONValuePtr cpu_props = NULL; + const char *cpu_name = ""; const char *typeStr = ""; *model_info = NULL; if (!(model = qemuMonitorJSONMakeCPUModel(model_name, migratable))) - goto cleanup; + return -1; retry: switch (type) { @@ -5908,35 +5907,33 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon, "s:type", typeStr, "a:model", &model, NULL))) - goto cleanup; + return -1; if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) - goto cleanup; + return -1; /* Even though query-cpu-model-expansion is advertised by query-commands it * may just return GenericError if it is not implemented for the requested * guest architecture or it is not supported in the host environment. */ - if (qemuMonitorJSONHasError(reply, "GenericError")) { - ret = 0; - goto cleanup; - } + if (qemuMonitorJSONHasError(reply, "GenericError")) + return 0; if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_OBJECT) < 0) - goto cleanup; + return -1; data = virJSONValueObjectGetObject(reply, "return"); if (qemuMonitorJSONParseCPUModelData(data, &cpu_model, &cpu_props, &cpu_name) < 0) - goto cleanup; + return -1; /* QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC_FULL requests "full" expansion * on the result of the initial "static" expansion. */ if (type == QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC_FULL) { if (!(model = virJSONValueCopy(cpu_model))) - goto cleanup; + return -1; virJSONValueFree(cmd); virJSONValueFree(reply); @@ -5944,13 +5941,7 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon, goto retry; } - ret = qemuMonitorJSONParseCPUModel(cpu_name, cpu_props, model_info); - - cleanup: - virJSONValueFree(cmd); - virJSONValueFree(reply); - virJSONValueFree(model); - return ret; + return qemuMonitorJSONParseCPUModel(cpu_name, cpu_props, model_info); } -- 2.7.4

When expanding a CPU model via query-cpu-model-expansion, any features that were a part of the original model are discarded. For exmaple, when expanding modelA with features f1, f2, a full expansion may reveal feature f3, but the expanded model will not include f1 or f2. Let's pass a virCPUDefPtr to the expansion function in preparation for taking features into consideration. Signed-off-by: Collin Walling <walling(a)linux.ibm.com&gt; --- src/qemu/qemu_capabilities.c | 9 +++++++-- src/qemu/qemu_monitor.c | 7 +++---- src/qemu/qemu_monitor.h | 2 +- src/qemu/qemu_monitor_json.c | 8 ++++---- src/qemu/qemu_monitor_json.h | 2 +- tests/cputest.c | 7 ++++++- 6 files changed, 22 insertions(+), 13 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 9b19930..bc0ac3d 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2497,6 +2497,7 @@ virQEMUCapsProbeQMPHostCPU(virQEMUCapsPtr qemuCaps, qemuMonitorCPUModelInfoPtr nonMigratable = NULL; virHashTablePtr hash = NULL; const char *model; + virCPUDefPtr cpu; qemuMonitorCPUModelExpansionType type; virDomainVirtType virtType; int ret = -1; @@ -2512,6 +2513,9 @@ virQEMUCapsProbeQMPHostCPU(virQEMUCapsPtr qemuCaps, model = "host"; } + if (VIR_ALLOC(cpu) < 0 || VIR_STRDUP(cpu->model, model) < 0) + goto cleanup; + /* Some x86_64 features defined in cpu_map.xml use spelling which differ * from the one preferred by QEMU. Static expansion would give us only the * preferred spelling. With new QEMU we always use the QEMU's canonical @@ -2525,12 +2529,12 @@ virQEMUCapsProbeQMPHostCPU(virQEMUCapsPtr qemuCaps, else type = QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC; - if (qemuMonitorGetCPUModelExpansion(mon, type, model, true, &modelInfo) < 0) + if (qemuMonitorGetCPUModelExpansion(mon, type, cpu, true, &modelInfo) < 0) goto cleanup; /* Try to check migratability of each feature. */ if (modelInfo && - qemuMonitorGetCPUModelExpansion(mon, type, model, false, + qemuMonitorGetCPUModelExpansion(mon, type, cpu, false, &nonMigratable) < 0) goto cleanup; @@ -2574,6 +2578,7 @@ virQEMUCapsProbeQMPHostCPU(virQEMUCapsPtr qemuCaps, virHashFree(hash); qemuMonitorCPUModelInfoFree(nonMigratable); qemuMonitorCPUModelInfoFree(modelInfo); + virCPUDefFree(cpu); return ret; } diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index aa230b3..7b454c2 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3716,16 +3716,15 @@ qemuMonitorCPUDefInfoFree(qemuMonitorCPUDefInfoPtr cpu) int qemuMonitorGetCPUModelExpansion(qemuMonitorPtr mon, qemuMonitorCPUModelExpansionType type, - const char *model_name, + virCPUDefPtr cpu, bool migratable, qemuMonitorCPUModelInfoPtr *model_info) { - VIR_DEBUG("type=%d model_name=%s migratable=%d", - type, model_name, migratable); + VIR_DEBUG("type=%d cpu=%p migratable=%d", type, cpu, migratable); QEMU_CHECK_MONITOR(mon); - return qemuMonitorJSONGetCPUModelExpansion(mon, type, model_name, + return qemuMonitorJSONGetCPUModelExpansion(mon, type, cpu, migratable, model_info); } diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 70000a1..672b4f9 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -1157,7 +1157,7 @@ typedef enum { int qemuMonitorGetCPUModelExpansion(qemuMonitorPtr mon, qemuMonitorCPUModelExpansionType type, - const char *model_name, + virCPUDefPtr cpu, bool migratable, qemuMonitorCPUModelInfoPtr *model_info); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 3282593..3c6c330 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -5782,7 +5782,7 @@ qemuMonitorJSONParseCPUModelProperty(const char *key, static virJSONValuePtr -qemuMonitorJSONMakeCPUModel(const char *model_name, +qemuMonitorJSONMakeCPUModel(virCPUDefPtr cpu, bool migratable) { virJSONValuePtr model = NULL; @@ -5791,7 +5791,7 @@ qemuMonitorJSONMakeCPUModel(const char *model_name, if (!(model = virJSONValueNewObject())) goto error; - if (virJSONValueObjectAppendString(model, "name", model_name) < 0) + if (virJSONValueObjectAppendString(model, "name", cpu->model) < 0) goto error; if (!migratable) { @@ -5873,7 +5873,7 @@ qemuMonitorJSONParseCPUModel(const char *cpu_name, int qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon, qemuMonitorCPUModelExpansionType type, - const char *model_name, + virCPUDefPtr cpu, bool migratable, qemuMonitorCPUModelInfoPtr *model_info) { @@ -5888,7 +5888,7 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon, *model_info = NULL; - if (!(model = qemuMonitorJSONMakeCPUModel(model_name, migratable))) + if (!(model = qemuMonitorJSONMakeCPUModel(cpu, migratable))) return -1; retry: diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 61e64e8..bdccd36 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -390,7 +390,7 @@ int qemuMonitorJSONGetCPUDefinitions(qemuMonitorPtr mon, int qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon, qemuMonitorCPUModelExpansionType type, - const char *model_name, + virCPUDefPtr cpu, bool migratable, qemuMonitorCPUModelInfoPtr *model_info) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(5); diff --git a/tests/cputest.c b/tests/cputest.c index 7037bcc..3c17ed4 100644 --- a/tests/cputest.c +++ b/tests/cputest.c @@ -481,6 +481,7 @@ cpuTestMakeQEMUCaps(const struct data *data) virQEMUCapsPtr qemuCaps = NULL; qemuMonitorTestPtr testMon = NULL; qemuMonitorCPUModelInfoPtr model = NULL; + virCPUDefPtr cpu = NULL; char *json = NULL; if (virAsprintf(&json, "%s/cputestdata/%s-cpuid-%s.json", @@ -490,9 +491,12 @@ cpuTestMakeQEMUCaps(const struct data *data) if (!(testMon = qemuMonitorTestNewFromFile(json, driver.xmlopt, true))) goto error; + if (VIR_ALLOC(cpu) < 0 || VIR_STRDUP(cpu->model, "host") < 0) + goto cleanup; + if (qemuMonitorGetCPUModelExpansion(qemuMonitorTestGetMonitor(testMon), QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC, - "host", true, &model) < 0) + cpu, true, &model) < 0) goto error; if (!(qemuCaps = virQEMUCapsNew())) @@ -515,6 +519,7 @@ cpuTestMakeQEMUCaps(const struct data *data) cleanup: qemuMonitorCPUModelInfoFree(model); qemuMonitorTestFree(testMon); + virCPUDefFree(cpu); VIR_FREE(json); return qemuCaps; -- 2.7.4

query-cpu-model-baseline/comparison will accept a list of features as part of the command. Since CPUs may be defined with CPU feature policies, let's parse it to the appropriate boolean that the QMP command expects. A feature that is set to required, force, or if it is a hypervisor CPU feature (-1), then set the property value to true. Otherwise (optional, disabled) set the value to false. Signed-off-by: Collin Walling <walling(a)linux.ibm.com&gt; --- src/qemu/qemu_monitor_json.c | 30 +++++++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 3c6c330..77113c0 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -5787,6 +5787,7 @@ qemuMonitorJSONMakeCPUModel(virCPUDefPtr cpu, { virJSONValuePtr model = NULL; virJSONValuePtr props = NULL; + size_t i; if (!(model = virJSONValueNewObject())) goto error; @@ -5794,12 +5795,31 @@ qemuMonitorJSONMakeCPUModel(virCPUDefPtr cpu, if (virJSONValueObjectAppendString(model, "name", cpu->model) < 0) goto error; - if (!migratable) { - if (!(props = virJSONValueNewObject()) || - virJSONValueObjectAppendBoolean(props, "migratable", false) < 0 || - virJSONValueObjectAppend(model, "props", props) < 0) + if (cpu->nfeatures || !migratable) { + if (!(props = virJSONValueNewObject())) + goto error; + + for (i = 0; i < cpu->nfeatures; i++) { + char *name = cpu->features[i].name; + bool enabled = false; + + /* policy may be reported as -1 if the CPU def is a host model */ + if (cpu->features[i].policy == VIR_CPU_FEATURE_REQUIRE || + cpu->features[i].policy == VIR_CPU_FEATURE_FORCE || + cpu->features[i].policy == -1) + enabled = true; + + if (virJSONValueObjectAppendBoolean(props, name, enabled) < 0) + goto error; + } + + if (!migratable && + virJSONValueObjectAppendBoolean(props, "migratable", false) < 0) { + goto error; + } + + if (virJSONValueObjectAppend(model, "props", props) < 0) goto error; - props = NULL; } return model; -- 2.7.4

Some older s390 CPU models (e.g. z900) will not report props as a response from query-cpu-model-expansion. As such, we should make the props field optional when parsing the return data from the QMP response. Signed-off-by: Collin Walling <walling(a)linux.ibm.com&gt; --- src/qemu/qemu_capabilities.c | 10 ++++++++-- src/qemu/qemu_monitor.c | 4 +++- src/qemu/qemu_monitor.h | 1 + src/qemu/qemu_monitor_json.c | 24 ++++++++++++++++-------- src/qemu/qemu_monitor_json.h | 3 ++- tests/cputest.c | 6 +++++- 6 files changed, 35 insertions(+), 13 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index bc0ac3d..3020530 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2500,6 +2500,7 @@ virQEMUCapsProbeQMPHostCPU(virQEMUCapsPtr qemuCaps, virCPUDefPtr cpu; qemuMonitorCPUModelExpansionType type; virDomainVirtType virtType; + bool fail_no_props = true; int ret = -1; if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION)) @@ -2529,12 +2530,17 @@ virQEMUCapsProbeQMPHostCPU(virQEMUCapsPtr qemuCaps, else type = QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC; - if (qemuMonitorGetCPUModelExpansion(mon, type, cpu, true, &modelInfo) < 0) + /* Older s390 models do not report a feature set */ + if (ARCH_IS_S390(qemuCaps->arch)) + fail_no_props = false; + + if (qemuMonitorGetCPUModelExpansion(mon, type, cpu, true, fail_no_props, + &modelInfo) < 0) goto cleanup; /* Try to check migratability of each feature. */ if (modelInfo && - qemuMonitorGetCPUModelExpansion(mon, type, cpu, false, + qemuMonitorGetCPUModelExpansion(mon, type, cpu, false, fail_no_props, &nonMigratable) < 0) goto cleanup; diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 7b454c2..6bda620 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3718,6 +3718,7 @@ qemuMonitorGetCPUModelExpansion(qemuMonitorPtr mon, qemuMonitorCPUModelExpansionType type, virCPUDefPtr cpu, bool migratable, + bool fail_no_props, qemuMonitorCPUModelInfoPtr *model_info) { VIR_DEBUG("type=%d cpu=%p migratable=%d", type, cpu, migratable); @@ -3725,7 +3726,8 @@ qemuMonitorGetCPUModelExpansion(qemuMonitorPtr mon, QEMU_CHECK_MONITOR(mon); return qemuMonitorJSONGetCPUModelExpansion(mon, type, cpu, - migratable, model_info); + migratable, fail_no_props, + model_info); } diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 672b4f9..c9cb3bd 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -1159,6 +1159,7 @@ int qemuMonitorGetCPUModelExpansion(qemuMonitorPtr mon, qemuMonitorCPUModelExpansionType type, virCPUDefPtr cpu, bool migratable, + bool fail_no_props, qemuMonitorCPUModelInfoPtr *model_info); void qemuMonitorCPUModelInfoFree(qemuMonitorCPUModelInfoPtr model_info); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 77113c0..ddcf425 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -5833,6 +5833,7 @@ qemuMonitorJSONMakeCPUModel(virCPUDefPtr cpu, static int qemuMonitorJSONParseCPUModelData(virJSONValuePtr data, + bool fail_no_props, virJSONValuePtr *cpu_model, virJSONValuePtr *cpu_props, const char **cpu_name) @@ -5849,7 +5850,8 @@ qemuMonitorJSONParseCPUModelData(virJSONValuePtr data, return -1; } - if (!(*cpu_props = virJSONValueObjectGetObject(*cpu_model, "props"))) { + if (!(*cpu_props = virJSONValueObjectGetObject(*cpu_model, "props")) && + fail_no_props) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("query-cpu-model-expansion reply data was missing 'props'")); return -1; @@ -5873,13 +5875,17 @@ qemuMonitorJSONParseCPUModel(const char *cpu_name, 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 (cpu_props) { + size_t nprops = virJSONValueObjectKeysNumber(cpu_props); - if (virJSONValueObjectForeachKeyValue(cpu_props, - qemuMonitorJSONParseCPUModelProperty, - machine_model) < 0) - goto cleanup; + 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; @@ -5895,6 +5901,7 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon, qemuMonitorCPUModelExpansionType type, virCPUDefPtr cpu, bool migratable, + bool fail_no_props, qemuMonitorCPUModelInfoPtr *model_info) { VIR_AUTOPTR(virJSONValue) model = NULL; @@ -5945,7 +5952,8 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon, data = virJSONValueObjectGetObject(reply, "return"); if (qemuMonitorJSONParseCPUModelData(data, - &cpu_model, &cpu_props, &cpu_name) < 0) + fail_no_props, &cpu_model, &cpu_props, + &cpu_name) < 0) return -1; /* QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC_FULL requests "full" expansion diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index bdccd36..682710f 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -392,8 +392,9 @@ int qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon, qemuMonitorCPUModelExpansionType type, virCPUDefPtr cpu, bool migratable, + bool fail_no_props, qemuMonitorCPUModelInfoPtr *model_info) - ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(5); + ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(6); int qemuMonitorJSONGetCommands(qemuMonitorPtr mon, char ***commands) diff --git a/tests/cputest.c b/tests/cputest.c index 3c17ed4..e3937c6 100644 --- a/tests/cputest.c +++ b/tests/cputest.c @@ -482,6 +482,7 @@ cpuTestMakeQEMUCaps(const struct data *data) qemuMonitorTestPtr testMon = NULL; qemuMonitorCPUModelInfoPtr model = NULL; virCPUDefPtr cpu = NULL; + bool fail_no_props = true; char *json = NULL; if (virAsprintf(&json, "%s/cputestdata/%s-cpuid-%s.json", @@ -494,9 +495,12 @@ cpuTestMakeQEMUCaps(const struct data *data) if (VIR_ALLOC(cpu) < 0 || VIR_STRDUP(cpu->model, "host") < 0) goto cleanup; + if (ARCH_IS_S390(data->arch)) + fail_no_props = false; + if (qemuMonitorGetCPUModelExpansion(qemuMonitorTestGetMonitor(testMon), QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC, - cpu, true, &model) < 0) + cpu, true, fail_no_props, &model) < 0) goto error; if (!(qemuCaps = virQEMUCapsNew())) -- 2.7.4

Modify the error messages in qemuMonitorJSONParseCPUModelData to print the command name provided to the function. Signed-off-by: Collin Walling <walling(a)linux.ibm.com&gt; --- src/qemu/qemu_monitor_json.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index ddcf425..13ee323 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -5833,27 +5833,28 @@ qemuMonitorJSONMakeCPUModel(virCPUDefPtr cpu, static int qemuMonitorJSONParseCPUModelData(virJSONValuePtr data, + const char *cmd_name, bool fail_no_props, virJSONValuePtr *cpu_model, virJSONValuePtr *cpu_props, const char **cpu_name) { if (!(*cpu_model = virJSONValueObjectGetObject(data, "model"))) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("query-cpu-model-expansion reply data was missing '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", - _("query-cpu-model-expansion reply data was missing 'name'")); + virReportError(VIR_ERR_INTERNAL_ERROR, + _("%s reply data was missing 'name'"), cmd_name); return -1; } if (!(*cpu_props = virJSONValueObjectGetObject(*cpu_model, "props")) && fail_no_props) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("query-cpu-model-expansion reply data was missing 'props'")); + virReportError(VIR_ERR_INTERNAL_ERROR, + _("%s reply data was missing 'props'"), cmd_name); return -1; } @@ -5951,7 +5952,7 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon, data = virJSONValueObjectGetObject(reply, "return"); - if (qemuMonitorJSONParseCPUModelData(data, + if (qemuMonitorJSONParseCPUModelData(data, "query-cpu-model-expansion", fail_no_props, &cpu_model, &cpu_props, &cpu_name) < 0) return -1; -- 2.7.4

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(a)linux.ibm.com&gt; Reviewed-by: Daniel Henrique Barboza <danielh413(a)gmail.com&gt; --- src/qemu/qemu_monitor.c | 14 ++++++++++++++ src/qemu/qemu_monitor.h | 5 +++++ src/qemu/qemu_monitor_json.c | 42 ++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 6 ++++++ 4 files changed, 67 insertions(+) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 6bda620..834eaf2 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3731,6 +3731,20 @@ qemuMonitorGetCPUModelExpansion(qemuMonitorPtr mon, } +int +qemuMonitorGetCPUModelBaseline(qemuMonitorPtr mon, + virCPUDefPtr cpu_a, + virCPUDefPtr cpu_b, + qemuMonitorCPUModelInfoPtr *baseline) +{ + VIR_DEBUG("cpu_a=%p cpu_b=%p", cpu_a, cpu_b); + + QEMU_CHECK_MONITOR(mon); + + return qemuMonitorJSONGetCPUModelBaseline(mon, cpu_a, cpu_b, baseline); +} + + void qemuMonitorCPUModelInfoFree(qemuMonitorCPUModelInfoPtr model_info) { diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index c9cb3bd..8974ba8 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -1164,6 +1164,11 @@ int qemuMonitorGetCPUModelExpansion(qemuMonitorPtr mon, void qemuMonitorCPUModelInfoFree(qemuMonitorCPUModelInfoPtr model_info); +int qemuMonitorGetCPUModelBaseline(qemuMonitorPtr mon, + virCPUDefPtr cpu_a, + virCPUDefPtr cpu_b, + qemuMonitorCPUModelInfoPtr *baseline); + qemuMonitorCPUModelInfoPtr qemuMonitorCPUModelInfoCopy(const qemuMonitorCPUModelInfo *orig); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 13ee323..679531d 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -5974,6 +5974,48 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon, } +int +qemuMonitorJSONGetCPUModelBaseline(qemuMonitorPtr mon, + virCPUDefPtr cpu_a, + virCPUDefPtr cpu_b, + qemuMonitorCPUModelInfoPtr *baseline) +{ + VIR_AUTOPTR(virJSONValue) model_a = NULL; + VIR_AUTOPTR(virJSONValue) model_b = NULL; + VIR_AUTOPTR(virJSONValue) cmd = NULL; + VIR_AUTOPTR(virJSONValue) reply = NULL; + virJSONValuePtr data; + virJSONValuePtr cpu_model; + virJSONValuePtr cpu_props = NULL; + const char *cpu_name = ""; + + if (!(model_a = qemuMonitorJSONMakeCPUModel(cpu_a, true)) || + !(model_b = qemuMonitorJSONMakeCPUModel(cpu_b, true))) + return -1; + + if (!(cmd = qemuMonitorJSONMakeCommand("query-cpu-model-baseline", + "a:modela", &model_a, + "a:modelb", &model_b, + NULL))) + return -1; + + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + return -1; + + if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_OBJECT) < 0) + return -1; + + data = virJSONValueObjectGetObject(reply, "return"); + + if (qemuMonitorJSONParseCPUModelData(data, "query-cpu-model-baseline", + false, &cpu_model, &cpu_props, + &cpu_name) < 0) + return -1; + + return qemuMonitorJSONParseCPUModel(cpu_name, cpu_props, baseline); +} + + int qemuMonitorJSONGetCommands(qemuMonitorPtr mon, char ***commands) { diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 682710f..77ea41b 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -396,6 +396,12 @@ int qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon, qemuMonitorCPUModelInfoPtr *model_info) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(6); +int qemuMonitorJSONGetCPUModelBaseline(qemuMonitorPtr mon, + virCPUDefPtr cpu_a, + virCPUDefPtr cpu_b, + qemuMonitorCPUModelInfoPtr *baseline) + ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4); + int qemuMonitorJSONGetCommands(qemuMonitorPtr mon, char ***commands) ATTRIBUTE_NONNULL(2); -- 2.7.4

This capability enables baselining of CPU models via QMP. Signed-off-by: Collin Walling <walling(a)linux.ibm.com&gt; Reviewed-by: Daniel Henrique Barboza <danielh413(a)gmail.com&gt; Reviewed-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com&gt; Reviewed-by: Jiri Denemark <jdenemar(a)redhat.com&gt; --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_3.0.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml | 1 + 9 files changed, 10 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 3020530..6fa8354 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -539,6 +539,7 @@ VIR_ENUM_IMPL(virQEMUCaps, "migration-file-drop-cache", "net-socket-dgram", "dbus-vmstate", + "query-cpu-model-baseline", ); @@ -985,6 +986,7 @@ struct virQEMUCapsStringFlags virQEMUCapsCommands[] = { { "blockdev-del", QEMU_CAPS_BLOCKDEV_DEL }, { "query-current-machine", QEMU_CAPS_QUERY_CURRENT_MACHINE }, { "block-dirty-bitmap-merge", QEMU_CAPS_BITMAP_MERGE }, + { "query-cpu-model-baseline", QEMU_CAPS_QUERY_CPU_MODEL_BASELINE }, }; struct virQEMUCapsStringFlags virQEMUCapsMigration[] = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 54f9115..2c10e00 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -520,6 +520,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ QEMU_CAPS_MIGRATION_FILE_DROP_CACHE, /* migration with disk cache on is safe for type='file' disks */ QEMU_CAPS_NET_SOCKET_DGRAM, /* -net socket,fd= with dgram socket */ QEMU_CAPS_DBUS_VMSTATE, /* -object dbus-vmstate */ + QEMU_CAPS_QUERY_CPU_MODEL_BASELINE, /* qmp query-cpu-model-baseline */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml index 21b918f..e6f7e28 100644 --- a/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml @@ -103,6 +103,7 @@ <flag name='egl-headless'/> <flag name='zpci'/> <flag name='iothread.poll-max-ns'/> + <flag name='query-cpu-model-baseline'/> <version>2010000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>39100805</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml index 6cb997d..48d7742 100644 --- a/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml @@ -110,6 +110,7 @@ <flag name='egl-headless'/> <flag name='zpci'/> <flag name='iothread.poll-max-ns'/> + <flag name='query-cpu-model-baseline'/> <version>2011000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>39100806</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml index 2930381..381abd5 100644 --- a/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml @@ -121,6 +121,7 @@ <flag name='memory-backend-memfd.hugetlb'/> <flag name='iothread.poll-max-ns'/> <flag name='memory-backend-file.align'/> + <flag name='query-cpu-model-baseline'/> <version>2012000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>39100807</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml index 5a2b740..9516a3b 100644 --- a/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml @@ -92,6 +92,7 @@ <flag name='sdl-gl'/> <flag name='vhost-vsock'/> <flag name='zpci'/> + <flag name='query-cpu-model-baseline'/> <version>2007093</version> <kvmVersion>0</kvmVersion> <microcodeVersion>39100764</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml index 72ae100..cf8bbb2 100644 --- a/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml @@ -97,6 +97,7 @@ <flag name='vhost-vsock'/> <flag name='zpci'/> <flag name='iothread.poll-max-ns'/> + <flag name='query-cpu-model-baseline'/> <version>2009000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>39100765</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_3.0.0.s390x.xml b/tests/qemucapabilitiesdata/caps_3.0.0.s390x.xml index d511377..40b4a2b 100644 --- a/tests/qemucapabilitiesdata/caps_3.0.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_3.0.0.s390x.xml @@ -123,6 +123,7 @@ <flag name='memory-backend-memfd.hugetlb'/> <flag name='iothread.poll-max-ns'/> <flag name='memory-backend-file.align'/> + <flag name='query-cpu-model-baseline'/> <version>3000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>39100757</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml b/tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml index 6cad000..4909bcc 100644 --- a/tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml @@ -133,6 +133,7 @@ <flag name='nbd-bitmap'/> <flag name='migration-file-drop-cache'/> <flag name='net-socket-dgram'/> + <flag name='query-cpu-model-baseline'/> <version>4000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>39100758</microcodeVersion> -- 2.7.4

On Thu, Sep 19, 2019 at 16:25:00 -0400, Collin Walling wrote: We usually initialize ret to -1 and set it to zero at the very end when everything is done rather than changing it to -1 on each error path. This would cause double free in the caller. Adding cleanup label here would be better. You can avoid this by using VIR_STEAL_PTR(info, *src) at the beginning. I think I mentioned in my previous review (probably not in this exact context, though) we usually pass an array followed by the number of elements. Extra empty line. I think this comment would be better placed as a documentation of the new function. You could steal from baseline to ret, free baseline unconditionally and return ret to get rid of the error label. Three extra empty lines in this hunk. With the following patch squashed in Reviewed-by: Jiri Denemark <jdenemar(a)redhat.com&gt; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b25e554358..b772a920e3 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -13739,32 +13739,41 @@ qemuConnectBaselineCPU(virConnectPtr conn ATTRIBUTE_UNUSED, } +/** + * qemuConnectStealCPUModelFromInfo: + * + * Consumes @src and replaces the content of @dst with CPU model name and + * features from @src. When this function returns (both with success or + * failure), @src is freed. + */ static int qemuConnectStealCPUModelFromInfo(virCPUDefPtr dst, qemuMonitorCPUModelInfoPtr *src) { - qemuMonitorCPUModelInfoPtr info = *src; + qemuMonitorCPUModelInfoPtr info; size_t i; - int ret = 0; + int ret = -1; virCPUDefFreeModel(dst); + VIR_STEAL_PTR(info, *src); VIR_STEAL_PTR(dst->model, info->name); for (i = 0; i < info->nprops; i++) { char *name = info->props[i].name; - if (info->props[i].type == QEMU_MONITOR_CPU_PROPERTY_BOOLEAN && - info->props[i].value.boolean && - virCPUDefAddFeature(dst, name, VIR_CPU_FEATURE_REQUIRE) < 0) { - virCPUDefFree(dst); - ret = -1; - break; - } + if (info->props[i].type != QEMU_MONITOR_CPU_PROPERTY_BOOLEAN || + !info->props[i].value.boolean) + continue; + + if (virCPUDefAddFeature(dst, name, VIR_CPU_FEATURE_REQUIRE) < 0) + goto cleanup; } + ret = 0; + + cleanup: qemuMonitorCPUModelInfoFree(info); - *src = NULL; return ret; } @@ -13774,10 +13783,11 @@ qemuConnectCPUModelBaseline(virQEMUCapsPtr qemuCaps, const char *libDir, uid_t runUid, gid_t runGid, - int ncpus, - virCPUDefPtr *cpus) + virCPUDefPtr *cpus, + int ncpus) { qemuProcessQMPPtr proc; + virCPUDefPtr ret = NULL; virCPUDefPtr baseline = NULL; qemuMonitorCPUModelInfoPtr result = NULL; size_t i; @@ -13790,29 +13800,26 @@ qemuConnectCPUModelBaseline(virQEMUCapsPtr qemuCaps, goto cleanup; if (VIR_ALLOC(baseline) < 0) - goto error; + goto cleanup; if (virCPUDefCopyModel(baseline, cpus[0], false)) - goto error; + goto cleanup; for (i = 1; i < ncpus; i++) { - if (qemuMonitorGetCPUModelBaseline(proc->mon, baseline, cpus[i], &result) < 0) - goto error; + goto cleanup; - /* result is freed regardless of this function's success */ if (qemuConnectStealCPUModelFromInfo(baseline, &result) < 0) - goto error; + goto cleanup; } + VIR_STEAL_PTR(ret, baseline); + cleanup: qemuProcessQMPFree(proc); - return baseline; - - error: virCPUDefFree(baseline); - goto cleanup; + return ret; } @@ -13882,16 +13889,12 @@ 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)) { - if (!(cpu = qemuConnectCPUModelBaseline(qemuCaps, cfg->libDir, cfg->user, cfg->group, - ncpus, - cpus))) + cpus, ncpus))) goto cleanup; - } else { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, _("computing baseline hypervisor CPU is not supported "

Perform a full CPU model expansion on the result of the baselined model name when the features flag is present. Signed-off-by: Collin Walling <walling(a)linux.ibm.com&gt; --- src/qemu/qemu_driver.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2a5a3ca..93f1767 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -13845,6 +13845,7 @@ qemuConnectCPUModelBaseline(virQEMUCapsPtr qemuCaps, const char *libDir, uid_t runUid, gid_t runGid, + bool expand_features, int ncpus, virCPUDefPtr *cpus) { @@ -13877,6 +13878,16 @@ qemuConnectCPUModelBaseline(virQEMUCapsPtr qemuCaps, goto error; } + if (expand_features) { + if (qemuMonitorGetCPUModelExpansion(proc->mon, + QEMU_MONITOR_CPU_MODEL_EXPANSION_FULL, + baseline, true, false, &result) < 0) + goto error; + + if (qemuConnectStealCPUModelFromInfo(baseline, &result) < 0) + goto error; + } + cleanup: qemuProcessQMPFree(proc); return baseline; @@ -13957,9 +13968,11 @@ qemuConnectBaselineHypervisorCPU(virConnectPtr conn, } else if (ARCH_IS_S390(arch) && virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_CPU_MODEL_BASELINE)) { + bool expand_features = (flags & VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES); + if (!(cpu = qemuConnectCPUModelBaseline(qemuCaps, cfg->libDir, cfg->user, cfg->group, - ncpus, + expand_features, ncpus, cpus))) goto cleanup; -- 2.7.4

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). The list of properties (aka CPU features) that is returned from the QMP response is ignored. 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 | 14 ++++++++++++++ src/qemu/qemu_monitor.h | 5 +++++ src/qemu/qemu_monitor_json.c | 42 ++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 6 ++++++ 4 files changed, 67 insertions(+) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 834eaf2..76b5547 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3745,6 +3745,20 @@ qemuMonitorGetCPUModelBaseline(qemuMonitorPtr mon, } +int +qemuMonitorGetCPUModelComparison(qemuMonitorPtr mon, + virCPUDefPtr cpu_a, + virCPUDefPtr cpu_b, + char **result) +{ + VIR_DEBUG("cpu_a=%p cpu_b=%p", cpu_a, cpu_b); + + QEMU_CHECK_MONITOR(mon); + + return qemuMonitorJSONGetCPUModelComparison(mon, cpu_a, cpu_b, result); +} + + void qemuMonitorCPUModelInfoFree(qemuMonitorCPUModelInfoPtr model_info) { diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 8974ba8..6f46931 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -1169,6 +1169,11 @@ int qemuMonitorGetCPUModelBaseline(qemuMonitorPtr mon, virCPUDefPtr cpu_b, qemuMonitorCPUModelInfoPtr *baseline); +int qemuMonitorGetCPUModelComparison(qemuMonitorPtr mon, + virCPUDefPtr cpu_a, + virCPUDefPtr cpu_b, + char **result); + qemuMonitorCPUModelInfoPtr qemuMonitorCPUModelInfoCopy(const qemuMonitorCPUModelInfo *orig); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 679531d..d8f7668 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -6016,6 +6016,48 @@ qemuMonitorJSONGetCPUModelBaseline(qemuMonitorPtr mon, } +int +qemuMonitorJSONGetCPUModelComparison(qemuMonitorPtr mon, + virCPUDefPtr cpu_a, + virCPUDefPtr cpu_b, + char **result) +{ + VIR_AUTOPTR(virJSONValue) model_a = NULL; + VIR_AUTOPTR(virJSONValue) model_b = NULL; + VIR_AUTOPTR(virJSONValue) cmd = NULL; + VIR_AUTOPTR(virJSONValue) reply = NULL; + const char *data_result; + virJSONValuePtr data; + + if (!(model_a = qemuMonitorJSONMakeCPUModel(cpu_a, true)) || + !(model_b = qemuMonitorJSONMakeCPUModel(cpu_b, true))) + return -1; + + if (!(cmd = qemuMonitorJSONMakeCommand("query-cpu-model-comparison", + "a:modela", &model_a, + "a:modelb", &model_b, + NULL))) + return -1; + + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + return -1; + + if (qemuMonitorJSONCheckError(cmd, reply) < 0) + return -1; + + data = virJSONValueObjectGetObject(reply, "return"); + + if (!(data_result = virJSONValueObjectGetString(data, "result"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-cpu-model-comparison reply data was missing " + "'result'")); + return -1; + } + + return VIR_STRDUP(*result, data_result); +} + + int qemuMonitorJSONGetCommands(qemuMonitorPtr mon, char ***commands) { diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 77ea41b..52b24f0 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -402,6 +402,12 @@ int qemuMonitorJSONGetCPUModelBaseline(qemuMonitorPtr mon, qemuMonitorCPUModelInfoPtr *baseline) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4); +int qemuMonitorJSONGetCPUModelComparison(qemuMonitorPtr mon, + virCPUDefPtr cpu_a, + virCPUDefPtr cpu_b, + char **result) + ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); + int qemuMonitorJSONGetCommands(qemuMonitorPtr mon, char ***commands) ATTRIBUTE_NONNULL(2); -- 2.7.4

This capability enables comparison of CPU models via QMP. Signed-off-by: Collin Walling <walling(a)linux.ibm.com&gt; Reviewed-by: Daniel Henrique Barboza <danielh413(a)gmail.com&gt; Reviewed-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com&gt; Reviewed-by: Jiri Denemark <jdenemar(a)redhat.com&gt; --- src/qemu/qemu_capabilities.c | 4 ++++ src/qemu/qemu_capabilities.h | 3 +++ tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_3.0.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml | 1 + 9 files changed, 14 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 6fa8354..b89bcea 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -540,6 +540,9 @@ VIR_ENUM_IMPL(virQEMUCaps, "net-socket-dgram", "dbus-vmstate", "query-cpu-model-baseline", + + /* 340 */ + "query-cpu-model-comparison", ); @@ -987,6 +990,7 @@ struct virQEMUCapsStringFlags virQEMUCapsCommands[] = { { "query-current-machine", QEMU_CAPS_QUERY_CURRENT_MACHINE }, { "block-dirty-bitmap-merge", QEMU_CAPS_BITMAP_MERGE }, { "query-cpu-model-baseline", QEMU_CAPS_QUERY_CPU_MODEL_BASELINE }, + { "query-cpu-model-comparison", QEMU_CAPS_QUERY_CPU_MODEL_COMPARISON }, }; struct virQEMUCapsStringFlags virQEMUCapsMigration[] = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 2c10e00..2a274f9 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -522,6 +522,9 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ QEMU_CAPS_DBUS_VMSTATE, /* -object dbus-vmstate */ QEMU_CAPS_QUERY_CPU_MODEL_BASELINE, /* qmp query-cpu-model-baseline */ + /* 340 */ + QEMU_CAPS_QUERY_CPU_MODEL_COMPARISON, /* qmp query-cpu-model-comparison */ + QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml index e6f7e28..058eb32 100644 --- a/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml @@ -104,6 +104,7 @@ <flag name='zpci'/> <flag name='iothread.poll-max-ns'/> <flag name='query-cpu-model-baseline'/> + <flag name='query-cpu-model-comparison'/> <version>2010000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>39100805</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml index 48d7742..4256131 100644 --- a/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml @@ -111,6 +111,7 @@ <flag name='zpci'/> <flag name='iothread.poll-max-ns'/> <flag name='query-cpu-model-baseline'/> + <flag name='query-cpu-model-comparison'/> <version>2011000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>39100806</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml index 381abd5..48be96d 100644 --- a/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml @@ -122,6 +122,7 @@ <flag name='iothread.poll-max-ns'/> <flag name='memory-backend-file.align'/> <flag name='query-cpu-model-baseline'/> + <flag name='query-cpu-model-comparison'/> <version>2012000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>39100807</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml index 9516a3b..5b04416 100644 --- a/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml @@ -93,6 +93,7 @@ <flag name='vhost-vsock'/> <flag name='zpci'/> <flag name='query-cpu-model-baseline'/> + <flag name='query-cpu-model-comparison'/> <version>2007093</version> <kvmVersion>0</kvmVersion> <microcodeVersion>39100764</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml index cf8bbb2..d5feb7b 100644 --- a/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml @@ -98,6 +98,7 @@ <flag name='zpci'/> <flag name='iothread.poll-max-ns'/> <flag name='query-cpu-model-baseline'/> + <flag name='query-cpu-model-comparison'/> <version>2009000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>39100765</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_3.0.0.s390x.xml b/tests/qemucapabilitiesdata/caps_3.0.0.s390x.xml index 40b4a2b..27befc4 100644 --- a/tests/qemucapabilitiesdata/caps_3.0.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_3.0.0.s390x.xml @@ -124,6 +124,7 @@ <flag name='iothread.poll-max-ns'/> <flag name='memory-backend-file.align'/> <flag name='query-cpu-model-baseline'/> + <flag name='query-cpu-model-comparison'/> <version>3000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>39100757</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml b/tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml index 4909bcc..dc98baf 100644 --- a/tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml @@ -134,6 +134,7 @@ <flag name='migration-file-drop-cache'/> <flag name='net-socket-dgram'/> <flag name='query-cpu-model-baseline'/> + <flag name='query-cpu-model-comparison'/> <version>4000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>39100758</microcodeVersion> -- 2.7.4

On Thu, Sep 19, 2019 at 16:25:05 -0400, Collin Walling wrote: ... This needs to be initialized to NULL. Some extra empty lines. Reviewed-by: Jiri Denemark <jdenemar(a)redhat.com&gt;

On Thu, Sep 19, 2019 at 16:25:06 -0400, Collin Walling wrote: This is not really related to CPU compatibility. Except for taking a CPU model (or feature below) from a newer QEMU and comparing it on an old one. But this is indistinguishable from an incorrect input. For this reason and for consistency among all architectures we should just report this as a normal error (e.g., VIR_ERR_CONFIG_UNSUPPORTED). Ditto. That said, you don't need to pass failIncompatible to qemuConnectCheckCPUModel... and you can drop this conditional. Also in all four cases you should use qemuConnectCheckCPUModel() < 0 check. Jirka