[libvirt] [PATCHv1 00/12] BaselineHypervisorCPU using QEMU QMP exchanges

Some architectures (S390) depend on QEMU to compute baseline CPU model and expand a models feature set. Interacting with QEMU requires starting the QEMU process and completing one or more query-cpu-model-baseline QMP exchanges with QEMU in addition to a query-cpu-model-expansion QMP exchange to expand all features in the model. See "s390x CPU models: exposing features" patch set on Qemu-devel for discussion of QEMU aspects. This is part of resolution of: https://bugzilla.redhat.com/show_bug.cgi?id=1511999 Signed-off-by: Chris Venteicher <cventeic@redhat.com> Chris Venteicher (12): qemu_monitor_json: qemuMonitorCPUModelInfo / JSON conversion qemu_monitor: Introduce qemuMonitorCPUModelInfoInit and qemuMonitorCPUModelInfoFreeContents qemu_monitor: Introduce qemuMonitorGetCPUModelBaseline (query-cpu-model-baseline) qemu_monitor: Indicate when CPUModelInfo props report migratablity qemu_monitor: CPUModelExpansion on CPUModel with both name and properties cpu_conf: Calculate virCPUDef List Length function qemu_capabilities: virCPUDef / qemuMonitorCPUModelInfo conversions qemu_capabilities: QMPCommandPtr without shadow qmperr qemu_capabilities: Persist QEMU instance over multiple QMP Cmds qemu_capabilities: Introduce virQEMUCapsNewQMPCommandConnection qemu_capabilities: Introduce virQEMUCapsQMPBaselineCPUModel (baseline using QEMU) qemu_driver: BaselineHypervisorCPU supports S390 using QEMU/QMP src/conf/cpu_conf.c | 15 ++ src/conf/cpu_conf.h | 3 + src/libvirt_private.syms | 1 + src/qemu/qemu_capabilities.c | 335 +++++++++++++++++++++++++++++------ src/qemu/qemu_capabilities.h | 33 ++++ src/qemu/qemu_driver.c | 66 ++++++- src/qemu/qemu_monitor.c | 90 +++++++++- src/qemu/qemu_monitor.h | 16 +- src/qemu/qemu_monitor_json.c | 226 +++++++++++++++++++---- src/qemu/qemu_monitor_json.h | 14 +- tests/cputest.c | 7 +- 11 files changed, 683 insertions(+), 123 deletions(-) -- 2.17.1

Bidirectional conversion functions between data structure and JSON. JSON of form: {"model": {"name": "IvyBridge", "props": {}}} --- src/qemu/qemu_monitor_json.c | 126 ++++++++++++++++++++++++++--------- 1 file changed, 96 insertions(+), 30 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index aa89ea7056..afbf000fa2 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -5358,6 +5358,101 @@ qemuMonitorJSONParseCPUModelProperty(const char *key, return 0; } + +/* model_json: {"name": "z13-base", "props": {}} + */ +static virJSONValuePtr +qemuMonitorJSONBuildCPUModelInfoToJSON(qemuMonitorCPUModelInfoPtr model) +{ + virJSONValuePtr cpu_props = NULL; + virJSONValuePtr model_json = NULL; + size_t i; + + if (!model) + goto cleanup; + + if (!(cpu_props = virJSONValueNewObject())) + goto cleanup; + + for (i = 0; i < model->nprops; i++) { + qemuMonitorCPUPropertyPtr prop = &(model->props[i]); + + switch (prop->type) { + case QEMU_MONITOR_CPU_PROPERTY_BOOLEAN: + if (virJSONValueObjectAppendBoolean(cpu_props, prop->name, + prop->value.boolean) < 0) + goto cleanup; + break; + + case QEMU_MONITOR_CPU_PROPERTY_STRING: + if (virJSONValueObjectAppendString(cpu_props, prop->name, + prop->value.string) < 0) + goto cleanup; + break; + + case QEMU_MONITOR_CPU_PROPERTY_NUMBER: + if (virJSONValueObjectAppendNumberLong(cpu_props, prop->name, + prop->value.number) < 0) + goto cleanup; + break; + + case QEMU_MONITOR_CPU_PROPERTY_LAST: + default: + virReportEnumRangeError(qemuMonitorCPUPropertyPtr, prop->type); + goto cleanup; + } + } + + ignore_value(virJSONValueObjectCreate(&model_json, "s:name", model->name, + "a:props", &cpu_props, NULL)); + + cleanup: + virJSONValueFree(cpu_props); + return model_json; +} + + +/* model_json: {"name": "IvyBridge", "props": {}} + */ +static qemuMonitorCPUModelInfoPtr +qemuMonitorJSONBuildCPUModelInfoFromJSON(virJSONValuePtr cpu_model) +{ + virJSONValuePtr cpu_props; + qemuMonitorCPUModelInfoPtr machine_model = NULL; + qemuMonitorCPUModelInfoPtr model = NULL; + char const *cpu_name; + + if (!(cpu_name = virJSONValueObjectGetString(cpu_model, "name"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Parsed JSON reply missing 'name'")); + goto cleanup; + } + + if (VIR_ALLOC(machine_model) < 0) + goto cleanup; + + if (VIR_STRDUP(machine_model->name, cpu_name) < 0) + goto cleanup; + + if ((cpu_props = virJSONValueObjectGetObject(cpu_model, "props"))) { + 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, machine_model); + + cleanup: + qemuMonitorCPUModelInfoFree(machine_model); + + return model; +} + int qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon, qemuMonitorCPUModelExpansionType type, @@ -5372,9 +5467,6 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon, 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; @@ -5447,38 +5539,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) + if (!(*model_info = qemuMonitorJSONBuildCPUModelInfoFromJSON(cpu_model))) goto cleanup; ret = 0; - *model_info = machine_model; - machine_model = NULL; cleanup: - qemuMonitorCPUModelInfoFree(machine_model); virJSONValueFree(cmd); virJSONValueFree(reply); virJSONValueFree(model); -- 2.17.1

Init - Initial with model name and empty properties FreeContents - Free model name and properties without freeing pointer --- src/qemu/qemu_monitor.c | 37 ++++++++++++++++++++++++++++++++++++- src/qemu/qemu_monitor.h | 4 ++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index d6771c1d52..16de54dac7 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3632,8 +3632,31 @@ qemuMonitorGetCPUModelExpansion(qemuMonitorPtr mon, } +int +qemuMonitorCPUModelInfoInit(const char *name, qemuMonitorCPUModelInfoPtr model) +{ + int ret = -1; + + if (!model) + goto cleanup; + + model->name = NULL; + model->nprops = 0; + model->props = NULL; + model->props_migratable_valid = false; + + if (VIR_STRDUP(model->name, name) < 0) + goto cleanup; + + ret = 0; + + cleanup: + return ret; +} + + void -qemuMonitorCPUModelInfoFree(qemuMonitorCPUModelInfoPtr model_info) +qemuMonitorCPUModelInfoFreeContents(qemuMonitorCPUModelInfoPtr model_info) { size_t i; @@ -3648,6 +3671,18 @@ qemuMonitorCPUModelInfoFree(qemuMonitorCPUModelInfoPtr model_info) VIR_FREE(model_info->props); VIR_FREE(model_info->name); + + model_info->name = NULL; + model_info->nprops = 0; + model_info->props = NULL; + model_info->props_migratable_valid = false; +} + + +void +qemuMonitorCPUModelInfoFree(qemuMonitorCPUModelInfoPtr model_info) +{ + qemuMonitorCPUModelInfoFreeContents(model_info); VIR_FREE(model_info); } diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index b3d62324b4..7dbc993f1b 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -1021,6 +1021,10 @@ int qemuMonitorGetCPUModelExpansion(qemuMonitorPtr mon, qemuMonitorCPUModelInfoPtr *model_info); void qemuMonitorCPUModelInfoFree(qemuMonitorCPUModelInfoPtr model_info); +void qemuMonitorCPUModelInfoFreeContents(qemuMonitorCPUModelInfoPtr model_info); + +int qemuMonitorCPUModelInfoInit(const char *name, qemuMonitorCPUModelInfoPtr model) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); qemuMonitorCPUModelInfoPtr qemuMonitorCPUModelInfoCopy(const qemuMonitorCPUModelInfo *orig); -- 2.17.1

On 06/22/2018 12:42 AM, Chris Venteicher wrote:
Init - Initial with model name and empty properties FreeContents - Free model name and properties without freeing pointer --- src/qemu/qemu_monitor.c | 37 ++++++++++++++++++++++++++++++++++++- src/qemu/qemu_monitor.h | 4 ++++ 2 files changed, 40 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index d6771c1d52..16de54dac7 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3632,8 +3632,31 @@ qemuMonitorGetCPUModelExpansion(qemuMonitorPtr mon, }
+int +qemuMonitorCPUModelInfoInit(const char *name, qemuMonitorCPUModelInfoPtr model) +{ + int ret = -1; + + if (!model) + goto cleanup; + + model->name = NULL; + model->nprops = 0; + model->props = NULL; + model->props_migratable_valid = false; + + if (VIR_STRDUP(model->name, name) < 0) + goto cleanup; + + ret = 0; + + cleanup: + return ret; +} +> + void -qemuMonitorCPUModelInfoFree(qemuMonitorCPUModelInfoPtr model_info) +qemuMonitorCPUModelInfoFreeContents(qemuMonitorCPUModelInfoPtr model_info) { size_t i;
@@ -3648,6 +3671,18 @@ qemuMonitorCPUModelInfoFree(qemuMonitorCPUModelInfoPtr model_info)
VIR_FREE(model_info->props); VIR_FREE(model_info->name); + + model_info->name = NULL; + model_info->nprops = 0; + model_info->props = NULL; + model_info->props_migratable_valid = false; +}
I thought VIR_FREE frees memory and then updates the pointer to NULL?
+ + +void +qemuMonitorCPUModelInfoFree(qemuMonitorCPUModelInfoPtr model_info) +{ + qemuMonitorCPUModelInfoFreeContents(model_info); VIR_FREE(model_info); }
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index b3d62324b4..7dbc993f1b 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -1021,6 +1021,10 @@ int qemuMonitorGetCPUModelExpansion(qemuMonitorPtr mon, qemuMonitorCPUModelInfoPtr *model_info);
void qemuMonitorCPUModelInfoFree(qemuMonitorCPUModelInfoPtr model_info); +void qemuMonitorCPUModelInfoFreeContents(qemuMonitorCPUModelInfoPtr model_info); + +int qemuMonitorCPUModelInfoInit(const char *name, qemuMonitorCPUModelInfoPtr model) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
qemuMonitorCPUModelInfoPtr qemuMonitorCPUModelInfoCopy(const qemuMonitorCPUModelInfo *orig);
-- Respectfully, - Collin Walling

Wrap QMP query-cpu-model-baseline command transaction with QEMU. --- src/qemu/qemu_monitor.c | 13 ++++++++ src/qemu/qemu_monitor.h | 5 +++ src/qemu/qemu_monitor_json.c | 65 ++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 7 ++++ 4 files changed, 90 insertions(+) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 16de54dac7..c5c640b0d8 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3738,6 +3738,19 @@ qemuMonitorCPUModelInfoCopy(const qemuMonitorCPUModelInfo *orig) return NULL; } +int +qemuMonitorGetCPUModelBaseline(qemuMonitorPtr mon, + qemuMonitorCPUModelInfoPtr model_a, + qemuMonitorCPUModelInfoPtr model_b, + qemuMonitorCPUModelInfoPtr *model_baseline) +{ + VIR_DEBUG("model_a->name=%s", model_a->name); + VIR_DEBUG("model_b->name=%s", model_b->name); + + QEMU_CHECK_MONITOR(mon); + + return qemuMonitorJSONGetCPUModelBaseline(mon, model_a, model_b, model_baseline); +} int qemuMonitorGetCommands(qemuMonitorPtr mon, diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 7dbc993f1b..15bebb2aaf 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -1029,6 +1029,11 @@ int qemuMonitorCPUModelInfoInit(const char *name, qemuMonitorCPUModelInfoPtr mod qemuMonitorCPUModelInfoPtr qemuMonitorCPUModelInfoCopy(const qemuMonitorCPUModelInfo *orig); +int qemuMonitorGetCPUModelBaseline(qemuMonitorPtr mon, + qemuMonitorCPUModelInfoPtr model_a, + qemuMonitorCPUModelInfoPtr model_b, + qemuMonitorCPUModelInfoPtr *model_baseline); + int qemuMonitorGetCommands(qemuMonitorPtr mon, char ***commands); int qemuMonitorGetEvents(qemuMonitorPtr mon, diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index afbf000fa2..729578414b 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -5553,6 +5553,71 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon, } +/* Note: *model_baseline == NULL && return == 0 if command not supported by QEMU + */ +int +qemuMonitorJSONGetCPUModelBaseline(qemuMonitorPtr mon, + qemuMonitorCPUModelInfoPtr model_a, + qemuMonitorCPUModelInfoPtr model_b, + qemuMonitorCPUModelInfoPtr *model_baseline) +{ + int ret = -1; + virJSONValuePtr cmd = NULL; + virJSONValuePtr reply = NULL; + virJSONValuePtr data = NULL; + virJSONValuePtr modela = NULL; + virJSONValuePtr modelb = NULL; + virJSONValuePtr cpu_model = NULL; + + *model_baseline = NULL; + + if (!(modela = qemuMonitorJSONBuildCPUModelInfoToJSON(model_a))) + goto cleanup; + + if (!(modelb = qemuMonitorJSONBuildCPUModelInfoToJSON(model_b))) + goto cleanup; + + if (!(cmd = qemuMonitorJSONMakeCommand("query-cpu-model-baseline", + "a:modela", &modela, + "a:modelb", &modelb, + NULL))) + goto cleanup; + + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; + + if (qemuMonitorJSONHasError(reply, "GenericError")) { + /* QEMU does not support query-cpu-model-baseline or cpu model */ + ret = 0; + goto cleanup; + } + + if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_OBJECT) < 0) + goto cleanup; + + data = virJSONValueObjectGetObject(reply, "return"); + + if (!(cpu_model = virJSONValueObjectGetObject(data, "model"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-cpu-model-baseline reply data was missing 'model'")); + goto cleanup; + } + + if (!(*model_baseline = qemuMonitorJSONBuildCPUModelInfoFromJSON(cpu_model))) + goto cleanup; + + ret = 0; + + cleanup: + virJSONValueFree(cmd); + virJSONValueFree(reply); + virJSONValueFree(modela); + virJSONValueFree(modelb); + + 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 6bc0dd3ad2..73e1cb6ace 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -367,6 +367,13 @@ int qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon, qemuMonitorCPUModelInfoPtr *model_info) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(5); +int qemuMonitorJSONGetCPUModelBaseline(qemuMonitorPtr mon, + qemuMonitorCPUModelInfoPtr model_a, + qemuMonitorCPUModelInfoPtr model_b, + qemuMonitorCPUModelInfoPtr *model_baseline) + ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4); + + int qemuMonitorJSONGetCommands(qemuMonitorPtr mon, char ***commands) ATTRIBUTE_NONNULL(2); -- 2.17.1

On 06/22/2018 12:42 AM, Chris Venteicher wrote:
Wrap QMP query-cpu-model-baseline command transaction with QEMU. --- src/qemu/qemu_monitor.c | 13 ++++++++ src/qemu/qemu_monitor.h | 5 +++ src/qemu/qemu_monitor_json.c | 65 ++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 7 ++++ 4 files changed, 90 insertions(+)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 16de54dac7..c5c640b0d8 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3738,6 +3738,19 @@ qemuMonitorCPUModelInfoCopy(const qemuMonitorCPUModelInfo *orig) return NULL; }
+int +qemuMonitorGetCPUModelBaseline(qemuMonitorPtr mon, + qemuMonitorCPUModelInfoPtr model_a, + qemuMonitorCPUModelInfoPtr model_b, + qemuMonitorCPUModelInfoPtr *model_baseline) +{ + VIR_DEBUG("model_a->name=%s", model_a->name); + VIR_DEBUG("model_b->name=%s", model_b->name);
It might be useful to also print out nprops here too, so debuggers can at least see that the features are at least present in the object.
+ + QEMU_CHECK_MONITOR(mon); + + return qemuMonitorJSONGetCPUModelBaseline(mon, model_a, model_b, model_baseline); +}
int qemuMonitorGetCommands(qemuMonitorPtr mon,
[...]
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index afbf000fa2..729578414b 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -5553,6 +5553,71 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon, }
+/* Note: *model_baseline == NULL && return == 0 if command not supported by QEMU + */ +int +qemuMonitorJSONGetCPUModelBaseline(qemuMonitorPtr mon, + qemuMonitorCPUModelInfoPtr model_a, + qemuMonitorCPUModelInfoPtr model_b, + qemuMonitorCPUModelInfoPtr *model_baseline) +{ + int ret = -1; + virJSONValuePtr cmd = NULL; + virJSONValuePtr reply = NULL; + virJSONValuePtr data = NULL; + virJSONValuePtr modela = NULL; + virJSONValuePtr modelb = NULL; + virJSONValuePtr cpu_model = NULL; + + *model_baseline = NULL; + + if (!(modela = qemuMonitorJSONBuildCPUModelInfoToJSON(model_a))) + goto cleanup; + + if (!(modelb = qemuMonitorJSONBuildCPUModelInfoToJSON(model_b))) + goto cleanup;
Small nit: maybe combine the above two into one if-statement? Other than that, this patch looks good to me. [...] -- Respectfully, - Collin Walling

Renamed variable in CPUModelInfo such that props_migratable_valid is true when properties in CPUModelInfo indicate if property is / isn't migratable. Property migratability is not part of QMP messages but rather is sometimes calculated within Libvirt and stored in CPUModelInfo properties. --- src/qemu/qemu_capabilities.c | 10 +++++----- src/qemu/qemu_monitor.c | 2 +- src/qemu/qemu_monitor.h | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 419208ad5c..0c5e4bb766 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2400,7 +2400,7 @@ virQEMUCapsProbeQMPHostCPU(virQEMUCapsPtr qemuCaps, } } - modelInfo->migratability = true; + modelInfo->props_migratable_valid = true; } VIR_STEAL_PTR(cpuData->info, modelInfo); @@ -2455,7 +2455,7 @@ virQEMUCapsGetCPUFeatures(virQEMUCapsPtr qemuCaps, } VIR_STEAL_PTR(*features, list); - if (migratable && !data->info->migratability) + if (migratable && !data->info->props_migratable_valid) ret = 1; else ret = 0; @@ -2854,7 +2854,7 @@ virQEMUCapsInitCPUModel(virQEMUCapsPtr qemuCaps, virQEMUCapsHostCPUDataPtr cpuData = virQEMUCapsGetHostCPUData(qemuCaps, type); int ret = 1; - if (migratable && cpuData->info && !cpuData->info->migratability) + if (migratable && cpuData->info && !cpuData->info->props_migratable_valid) return 1; if (ARCH_IS_S390(qemuCaps->arch)) { @@ -3037,7 +3037,7 @@ virQEMUCapsLoadHostCPUModelInfo(virQEMUCapsPtr qemuCaps, _("invalid migratability value for host CPU model")); goto cleanup; } - hostCPU->migratability = val == VIR_TRISTATE_BOOL_YES; + hostCPU->props_migratable_valid = val == VIR_TRISTATE_BOOL_YES; VIR_FREE(str); ctxt->node = hostCPUNode; @@ -3530,7 +3530,7 @@ virQEMUCapsFormatHostCPUModelInfo(virQEMUCapsPtr qemuCaps, virBufferAsprintf(buf, "<hostCPU type='%s' model='%s' migratability='%s'>\n", typeStr, model->name, - model->migratability ? "yes" : "no"); + model->props_migratable_valid ? "yes" : "no"); virBufferAdjustIndent(buf, 2); for (i = 0; i < model->nprops; i++) { diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index c5c640b0d8..3fd73e2cd1 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3702,7 +3702,7 @@ qemuMonitorCPUModelInfoCopy(const qemuMonitorCPUModelInfo *orig) if (VIR_STRDUP(copy->name, orig->name) < 0) goto error; - copy->migratability = orig->migratability; + copy->props_migratable_valid = orig->props_migratable_valid; copy->nprops = orig->nprops; for (i = 0; i < orig->nprops; i++) { diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 15bebb2aaf..08b787e28c 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -1005,7 +1005,7 @@ struct _qemuMonitorCPUModelInfo { char *name; size_t nprops; qemuMonitorCPUPropertyPtr props; - bool migratability; + bool props_migratable_valid; }; typedef enum { -- 2.17.1

Send both model name and a set of features/properties to QEMU for expansion rather than just the model name. --- src/qemu/qemu_capabilities.c | 33 +++++++++++++------ src/qemu/qemu_monitor.c | 38 ++++++++++++++++++---- src/qemu/qemu_monitor.h | 5 ++- src/qemu/qemu_monitor_json.c | 61 +++++++++++++++++++++++------------- src/qemu/qemu_monitor_json.h | 7 ++--- tests/cputest.c | 7 ++++- 6 files changed, 105 insertions(+), 46 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 0c5e4bb766..da6fb1f614 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2333,7 +2333,7 @@ virQEMUCapsProbeQMPHostCPU(virQEMUCapsPtr qemuCaps, qemuMonitorCPUModelInfoPtr modelInfo = NULL; qemuMonitorCPUModelInfoPtr nonMigratable = NULL; virHashTablePtr hash = NULL; - const char *model; + const char *model_name; qemuMonitorCPUModelExpansionType type; virDomainVirtType virtType; virQEMUCapsHostCPUDataPtr cpuData; @@ -2344,12 +2344,20 @@ virQEMUCapsProbeQMPHostCPU(virQEMUCapsPtr qemuCaps, if (tcg || !virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM)) { virtType = VIR_DOMAIN_VIRT_QEMU; - model = "max"; + model_name = "max"; } else { virtType = VIR_DOMAIN_VIRT_KVM; - model = "host"; + model_name = "host"; } + if ((VIR_ALLOC(modelInfo) < 0) || + (VIR_ALLOC(nonMigratable) < 0)) + goto cleanup; + + if ((qemuMonitorCPUModelInfoInit(model_name, modelInfo) < 0) || + (qemuMonitorCPUModelInfoInit(model_name, nonMigratable) < 0)) + goto cleanup; + cpuData = virQEMUCapsGetHostCPUData(qemuCaps, virtType); /* Some x86_64 features defined in cpu_map.xml use spelling which differ @@ -2362,15 +2370,20 @@ virQEMUCapsProbeQMPHostCPU(virQEMUCapsPtr qemuCaps, else type = QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC; - if (qemuMonitorGetCPUModelExpansion(mon, type, model, true, &modelInfo) < 0) - goto cleanup; - - /* Try to check migratability of each feature. */ - if (modelInfo && - qemuMonitorGetCPUModelExpansion(mon, type, model, false, - &nonMigratable) < 0) + if ((qemuMonitorGetCPUModelExpansion(mon, type, true, modelInfo) < 0) || + (qemuMonitorGetCPUModelExpansion(mon, type, false, nonMigratable) < 0)) goto cleanup; + /* Try to check migratability of each feature */ + /* Expansion 1 sets migratable features true + * Expansion 2 sets migratable and non-migratable features true + * (non-migratable set true only in some archs like X86) + * + * If delta between Expansion 1 and 2 exists... + * - both migratable and non-migratable features set prop->value = true + * - migratable features set prop->migatable = VIR_TRISTATE_BOOL_YES + * - non-migratable features set prop->migatable = VIR_TRISTATE_BOOL_NO + */ if (nonMigratable) { qemuMonitorCPUPropertyPtr prop; qemuMonitorCPUPropertyPtr nmProp; diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 3fd73e2cd1..558bc37aff 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3615,20 +3615,46 @@ qemuMonitorCPUDefInfoFree(qemuMonitorCPUDefInfoPtr cpu) } +/* + * type static: + * Expand to static base model + delta property changes + * Returned model is invariant and migration safe + * + * model_info->name = base model name + * model_info->props = features to +/- to base model to achive model_name + * + * type full: + * Expand model to enumerate all properties + * Returned model isn't guaranteed to be invariant or migration safe. + * + * model_info->name = base model name + * model_info->props = features to +/- to empty set to achive model_name + * + * type static_full: + * Expand to static base model + delta property changes (pass 0) + * Expand model to enumerate all properties (pass 1) + * Returned model is invariant and migration safe + * + * model_info->name = base model name + * model_info->props = features to +/- to empty set to achive model_name + * + * migratable_only: + * true: QEMU excludes non-migratable features + * false: QEMU includes non-migratable features for some archs like X86 + */ int qemuMonitorGetCPUModelExpansion(qemuMonitorPtr mon, qemuMonitorCPUModelExpansionType type, - const char *model_name, - bool migratable, - qemuMonitorCPUModelInfoPtr *model_info) + bool migratable_only, + qemuMonitorCPUModelInfoPtr model_info) { VIR_DEBUG("type=%d model_name=%s migratable=%d", - type, model_name, migratable); + type, (model_info ? NULLSTR(model_info->name):"NULL"), + migratable_only); QEMU_CHECK_MONITOR(mon); - return qemuMonitorJSONGetCPUModelExpansion(mon, type, model_name, - migratable, model_info); + return qemuMonitorJSONGetCPUModelExpansion(mon, type, migratable_only, model_info); } diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 08b787e28c..7165aa9076 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -1016,9 +1016,8 @@ typedef enum { int qemuMonitorGetCPUModelExpansion(qemuMonitorPtr mon, qemuMonitorCPUModelExpansionType type, - const char *model_name, - bool migratable, - qemuMonitorCPUModelInfoPtr *model_info); + bool migratable_only, + qemuMonitorCPUModelInfoPtr model_info); void qemuMonitorCPUModelInfoFree(qemuMonitorCPUModelInfoPtr model_info); void qemuMonitorCPUModelInfoFreeContents(qemuMonitorCPUModelInfoPtr model_info); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 729578414b..bcd7828e8d 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -5403,8 +5403,11 @@ qemuMonitorJSONBuildCPUModelInfoToJSON(qemuMonitorCPUModelInfoPtr model) } } - ignore_value(virJSONValueObjectCreate(&model_json, "s:name", model->name, - "a:props", &cpu_props, NULL)); + if (model->nprops > 0) + ignore_value(virJSONValueObjectCreate(&model_json, "s:name", model->name, + "a:props", &cpu_props, NULL)); + else + ignore_value(virJSONValueObjectCreate(&model_json, "s:name", model->name, NULL)); cleanup: virJSONValueFree(cpu_props); @@ -5456,35 +5459,43 @@ qemuMonitorJSONBuildCPUModelInfoFromJSON(virJSONValuePtr cpu_model) int qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon, qemuMonitorCPUModelExpansionType type, - const char *model_name, - bool migratable, - qemuMonitorCPUModelInfoPtr *model_info) + bool migratable_only, + qemuMonitorCPUModelInfoPtr model) { int ret = -1; - virJSONValuePtr model = NULL; - virJSONValuePtr props = NULL; + virJSONValuePtr json_model = NULL; virJSONValuePtr cmd = NULL; virJSONValuePtr reply = NULL; virJSONValuePtr data; virJSONValuePtr cpu_model; + qemuMonitorCPUModelInfoPtr expanded_model = NULL; + qemuMonitorCPUModelInfoPtr model_info = NULL; const char *typeStr = ""; - *model_info = NULL; + if (!(model_info = qemuMonitorCPUModelInfoCopy(model))) + return -1; - if (!(model = virJSONValueNewObject())) - goto cleanup; + qemuMonitorCPUModelInfoFreeContents(model); - if (virJSONValueObjectAppendString(model, "name", model_name) < 0) - goto cleanup; + if (!migratable_only) { + /* Add property to input CPUModelInfo causing QEMU to include + * non-migratable properties for some architectures like X86 */ + + qemuMonitorCPUProperty prop; + prop.type = QEMU_MONITOR_CPU_PROPERTY_BOOLEAN; + prop.value.boolean = false; + prop.migratable = false; + + if (VIR_STRDUP(prop.name, "migratable") < 0) + goto cleanup; - if (!migratable) { - if (!(props = virJSONValueNewObject()) || - virJSONValueObjectAppendBoolean(props, "migratable", false) < 0 || - virJSONValueObjectAppend(model, "props", props) < 0) + if (VIR_APPEND_ELEMENT(model_info->props, model_info->nprops, prop) < 0) goto cleanup; - props = NULL; } + if (!(json_model = qemuMonitorJSONBuildCPUModelInfoToJSON(model_info))) + goto cleanup; + retry: switch (type) { case QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC: @@ -5499,7 +5510,7 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon, if (!(cmd = qemuMonitorJSONMakeCommand("query-cpu-model-expansion", "s:type", typeStr, - "a:model", &model, + "a:model", &json_model, NULL))) goto cleanup; @@ -5530,7 +5541,9 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon, * on the result of the initial "static" expansion. */ if (type == QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC_FULL) { - if (!(model = virJSONValueCopy(cpu_model))) + virJSONValueFree(json_model); + + if (!(json_model = virJSONValueCopy(cpu_model))) goto cleanup; virJSONValueFree(cmd); @@ -5539,16 +5552,20 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon, goto retry; } - if (!(*model_info = qemuMonitorJSONBuildCPUModelInfoFromJSON(cpu_model))) + if (!(expanded_model = qemuMonitorJSONBuildCPUModelInfoFromJSON(cpu_model))) goto cleanup; + *model = *expanded_model; /* overwrite contents */ + ret = 0; cleanup: + VIR_FREE(expanded_model); /* Free structure but not reused contents */ + qemuMonitorCPUModelInfoFreeContents(model_info); + virJSONValueFree(cmd); virJSONValueFree(reply); - virJSONValueFree(model); - virJSONValueFree(props); + virJSONValueFree(json_model); return ret; } diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 73e1cb6ace..9950483c5c 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -362,10 +362,9 @@ int qemuMonitorJSONGetCPUDefinitions(qemuMonitorPtr mon, int qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon, qemuMonitorCPUModelExpansionType type, - const char *model_name, - bool migratable, - qemuMonitorCPUModelInfoPtr *model_info) - ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(5); + bool migratable_only, + qemuMonitorCPUModelInfoPtr model_info) + ATTRIBUTE_NONNULL(4); int qemuMonitorJSONGetCPUModelBaseline(qemuMonitorPtr mon, qemuMonitorCPUModelInfoPtr model_a, diff --git a/tests/cputest.c b/tests/cputest.c index baf2b3c648..27727aa29e 100644 --- a/tests/cputest.c +++ b/tests/cputest.c @@ -495,9 +495,14 @@ cpuTestMakeQEMUCaps(const struct data *data) if (!(testMon = qemuMonitorTestNewFromFile(json, driver.xmlopt, true))) goto error; + if ((VIR_ALLOC(model) < 0) || + (qemuMonitorCPUModelInfoInit("host", model) < 0)) + goto cleanup; + + if (qemuMonitorGetCPUModelExpansion(qemuMonitorTestGetMonitor(testMon), QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC, - "host", true, &model) < 0) + true, model) < 0) goto error; if (!(qemuCaps = virQEMUCapsNew())) -- 2.17.1

Quoting Chris Venteicher (2018-06-21 23:42:04)
Send both model name and a set of features/properties to QEMU for expansion rather than just the model name. --- src/qemu/qemu_capabilities.c | 33 +++++++++++++------ src/qemu/qemu_monitor.c | 38 ++++++++++++++++++---- src/qemu/qemu_monitor.h | 5 ++- src/qemu/qemu_monitor_json.c | 61 +++++++++++++++++++++++------------- src/qemu/qemu_monitor_json.h | 7 ++--- tests/cputest.c | 7 ++++- 6 files changed, 105 insertions(+), 46 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 0c5e4bb766..da6fb1f614 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2333,7 +2333,7 @@ virQEMUCapsProbeQMPHostCPU(virQEMUCapsPtr qemuCaps, qemuMonitorCPUModelInfoPtr modelInfo = NULL; qemuMonitorCPUModelInfoPtr nonMigratable = NULL; virHashTablePtr hash = NULL; - const char *model; + const char *model_name; qemuMonitorCPUModelExpansionType type; virDomainVirtType virtType; virQEMUCapsHostCPUDataPtr cpuData; @@ -2344,12 +2344,20 @@ virQEMUCapsProbeQMPHostCPU(virQEMUCapsPtr qemuCaps,
if (tcg || !virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM)) { virtType = VIR_DOMAIN_VIRT_QEMU; - model = "max"; + model_name = "max"; } else { virtType = VIR_DOMAIN_VIRT_KVM; - model = "host"; + model_name = "host"; }
+ if ((VIR_ALLOC(modelInfo) < 0) || + (VIR_ALLOC(nonMigratable) < 0)) + goto cleanup; + + if ((qemuMonitorCPUModelInfoInit(model_name, modelInfo) < 0) || + (qemuMonitorCPUModelInfoInit(model_name, nonMigratable) < 0)) + goto cleanup; + cpuData = virQEMUCapsGetHostCPUData(qemuCaps, virtType);
/* Some x86_64 features defined in cpu_map.xml use spelling which differ @@ -2362,15 +2370,20 @@ virQEMUCapsProbeQMPHostCPU(virQEMUCapsPtr qemuCaps, else type = QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC;
- if (qemuMonitorGetCPUModelExpansion(mon, type, model, true, &modelInfo) < 0) - goto cleanup; - - /* Try to check migratability of each feature. */ - if (modelInfo && - qemuMonitorGetCPUModelExpansion(mon, type, model, false, - &nonMigratable) < 0) + if ((qemuMonitorGetCPUModelExpansion(mon, type, true, modelInfo) < 0) || + (qemuMonitorGetCPUModelExpansion(mon, type, false, nonMigratable) < 0)) goto cleanup;
Missing the case where QEMU doesn't support the expansion here. These are not error cases but we shouldn't update info as if we succeded either. Also, no need to try to identify non-migratable features if expansion 2 did not populate "nonMigratable"
+ /* Try to check migratability of each feature */ + /* Expansion 1 sets migratable features true + * Expansion 2 sets migratable and non-migratable features true + * (non-migratable set true only in some archs like X86) + * + * If delta between Expansion 1 and 2 exists... + * - both migratable and non-migratable features set prop->value = true + * - migratable features set prop->migatable = VIR_TRISTATE_BOOL_YES + * - non-migratable features set prop->migatable = VIR_TRISTATE_BOOL_NO + */ if (nonMigratable) { qemuMonitorCPUPropertyPtr prop; qemuMonitorCPUPropertyPtr nmProp; diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 3fd73e2cd1..558bc37aff 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3615,20 +3615,46 @@ qemuMonitorCPUDefInfoFree(qemuMonitorCPUDefInfoPtr cpu) }
+/* + * type static: + * Expand to static base model + delta property changes + * Returned model is invariant and migration safe + * + * model_info->name = base model name + * model_info->props = features to +/- to base model to achive model_name + * + * type full: + * Expand model to enumerate all properties + * Returned model isn't guaranteed to be invariant or migration safe. + * + * model_info->name = base model name + * model_info->props = features to +/- to empty set to achive model_name + * + * type static_full: + * Expand to static base model + delta property changes (pass 0) + * Expand model to enumerate all properties (pass 1) + * Returned model is invariant and migration safe + * + * model_info->name = base model name + * model_info->props = features to +/- to empty set to achive model_name + * + * migratable_only: + * true: QEMU excludes non-migratable features + * false: QEMU includes non-migratable features for some archs like X86 + */ int qemuMonitorGetCPUModelExpansion(qemuMonitorPtr mon, qemuMonitorCPUModelExpansionType type, - const char *model_name, - bool migratable, - qemuMonitorCPUModelInfoPtr *model_info) + bool migratable_only, + qemuMonitorCPUModelInfoPtr model_info) { VIR_DEBUG("type=%d model_name=%s migratable=%d", - type, model_name, migratable); + type, (model_info ? NULLSTR(model_info->name):"NULL"), + migratable_only);
QEMU_CHECK_MONITOR(mon);
- return qemuMonitorJSONGetCPUModelExpansion(mon, type, model_name, - migratable, model_info); + return qemuMonitorJSONGetCPUModelExpansion(mon, type, migratable_only, model_info); }
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 08b787e28c..7165aa9076 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -1016,9 +1016,8 @@ typedef enum {
int qemuMonitorGetCPUModelExpansion(qemuMonitorPtr mon, qemuMonitorCPUModelExpansionType type, - const char *model_name, - bool migratable, - qemuMonitorCPUModelInfoPtr *model_info); + bool migratable_only, + qemuMonitorCPUModelInfoPtr model_info);
void qemuMonitorCPUModelInfoFree(qemuMonitorCPUModelInfoPtr model_info); void qemuMonitorCPUModelInfoFreeContents(qemuMonitorCPUModelInfoPtr model_info); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 729578414b..bcd7828e8d 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -5403,8 +5403,11 @@ qemuMonitorJSONBuildCPUModelInfoToJSON(qemuMonitorCPUModelInfoPtr model) } }
- ignore_value(virJSONValueObjectCreate(&model_json, "s:name", model->name, - "a:props", &cpu_props, NULL)); + if (model->nprops > 0) + ignore_value(virJSONValueObjectCreate(&model_json, "s:name", model->name, + "a:props", &cpu_props, NULL)); + else + ignore_value(virJSONValueObjectCreate(&model_json, "s:name", model->name, NULL));
cleanup: virJSONValueFree(cpu_props); @@ -5456,35 +5459,43 @@ qemuMonitorJSONBuildCPUModelInfoFromJSON(virJSONValuePtr cpu_model) int qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon, qemuMonitorCPUModelExpansionType type, - const char *model_name, - bool migratable, - qemuMonitorCPUModelInfoPtr *model_info) + bool migratable_only, + qemuMonitorCPUModelInfoPtr model) { int ret = -1; - virJSONValuePtr model = NULL; - virJSONValuePtr props = NULL; + virJSONValuePtr json_model = NULL; virJSONValuePtr cmd = NULL; virJSONValuePtr reply = NULL; virJSONValuePtr data; virJSONValuePtr cpu_model; + qemuMonitorCPUModelInfoPtr expanded_model = NULL; + qemuMonitorCPUModelInfoPtr model_info = NULL; const char *typeStr = "";
- *model_info = NULL; + if (!(model_info = qemuMonitorCPUModelInfoCopy(model))) + return -1;
- if (!(model = virJSONValueNewObject())) - goto cleanup; + qemuMonitorCPUModelInfoFreeContents(model);
- if (virJSONValueObjectAppendString(model, "name", model_name) < 0) - goto cleanup; + if (!migratable_only) { + /* Add property to input CPUModelInfo causing QEMU to include + * non-migratable properties for some architectures like X86 */ + + qemuMonitorCPUProperty prop; + prop.type = QEMU_MONITOR_CPU_PROPERTY_BOOLEAN; + prop.value.boolean = false; + prop.migratable = false; + + if (VIR_STRDUP(prop.name, "migratable") < 0) + goto cleanup;
- if (!migratable) { - if (!(props = virJSONValueNewObject()) || - virJSONValueObjectAppendBoolean(props, "migratable", false) < 0 || - virJSONValueObjectAppend(model, "props", props) < 0) + if (VIR_APPEND_ELEMENT(model_info->props, model_info->nprops, prop) < 0) goto cleanup; - props = NULL; }
+ if (!(json_model = qemuMonitorJSONBuildCPUModelInfoToJSON(model_info))) + goto cleanup; + retry: switch (type) { case QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC: @@ -5499,7 +5510,7 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon,
if (!(cmd = qemuMonitorJSONMakeCommand("query-cpu-model-expansion", "s:type", typeStr, - "a:model", &model, + "a:model", &json_model, NULL))) goto cleanup;
@@ -5530,7 +5541,9 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon, * on the result of the initial "static" expansion. */ if (type == QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC_FULL) { - if (!(model = virJSONValueCopy(cpu_model))) + virJSONValueFree(json_model); + + if (!(json_model = virJSONValueCopy(cpu_model))) goto cleanup;
virJSONValueFree(cmd); @@ -5539,16 +5552,20 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon, goto retry; }
- if (!(*model_info = qemuMonitorJSONBuildCPUModelInfoFromJSON(cpu_model))) + if (!(expanded_model = qemuMonitorJSONBuildCPUModelInfoFromJSON(cpu_model))) goto cleanup;
+ *model = *expanded_model; /* overwrite contents */ + ret = 0;
cleanup: + VIR_FREE(expanded_model); /* Free structure but not reused contents */ + qemuMonitorCPUModelInfoFreeContents(model_info); + virJSONValueFree(cmd); virJSONValueFree(reply); - virJSONValueFree(model); - virJSONValueFree(props); + virJSONValueFree(json_model); return ret; }
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 73e1cb6ace..9950483c5c 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -362,10 +362,9 @@ int qemuMonitorJSONGetCPUDefinitions(qemuMonitorPtr mon,
int qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon, qemuMonitorCPUModelExpansionType type, - const char *model_name, - bool migratable, - qemuMonitorCPUModelInfoPtr *model_info) - ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(5); + bool migratable_only, + qemuMonitorCPUModelInfoPtr model_info) + ATTRIBUTE_NONNULL(4);
int qemuMonitorJSONGetCPUModelBaseline(qemuMonitorPtr mon, qemuMonitorCPUModelInfoPtr model_a, diff --git a/tests/cputest.c b/tests/cputest.c index baf2b3c648..27727aa29e 100644 --- a/tests/cputest.c +++ b/tests/cputest.c @@ -495,9 +495,14 @@ cpuTestMakeQEMUCaps(const struct data *data) if (!(testMon = qemuMonitorTestNewFromFile(json, driver.xmlopt, true))) goto error;
+ if ((VIR_ALLOC(model) < 0) || + (qemuMonitorCPUModelInfoInit("host", model) < 0)) + goto cleanup; + + if (qemuMonitorGetCPUModelExpansion(qemuMonitorTestGetMonitor(testMon), QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC, - "host", true, &model) < 0) + true, model) < 0) goto error;
if (!(qemuCaps = virQEMUCapsNew())) -- 2.17.1
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

Function returns number of virCPUDefPtrs in list --- src/conf/cpu_conf.c | 15 +++++++++++++++ src/conf/cpu_conf.h | 3 +++ src/libvirt_private.syms | 1 + 3 files changed, 19 insertions(+) diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c index 43a3ab5dcd..ff978ec083 100644 --- a/src/conf/cpu_conf.c +++ b/src/conf/cpu_conf.c @@ -1039,3 +1039,18 @@ virCPUDefListFree(virCPUDefPtr *cpus) VIR_FREE(cpus); } + + +/* + * Return number of virCPUDefPtrs in list + */ +size_t +virCPUDefListLength(virCPUDefPtr *cpus) +{ + size_t i = 0; + + while (cpus && cpus[i]) + i++; + + return i; +} diff --git a/src/conf/cpu_conf.h b/src/conf/cpu_conf.h index 9f2e7ee264..a0743e5d9b 100644 --- a/src/conf/cpu_conf.h +++ b/src/conf/cpu_conf.h @@ -227,4 +227,7 @@ virCPUDefListParse(const char **xmlCPUs, void virCPUDefListFree(virCPUDefPtr *cpus); +size_t +virCPUDefListLength(virCPUDefPtr *cpus); + #endif /* __VIR_CPU_CONF_H__ */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 98913a577a..66e74e3fb7 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -85,6 +85,7 @@ virCPUDefFreeFeatures; virCPUDefFreeModel; virCPUDefIsEqual; virCPUDefListFree; +virCPUDefListLength; virCPUDefListParse; virCPUDefParseXML; virCPUDefStealModel; -- 2.17.1

On 06/22/2018 12:42 AM, Chris Venteicher wrote:
Function returns number of virCPUDefPtrs in list --- src/conf/cpu_conf.c | 15 +++++++++++++++ src/conf/cpu_conf.h | 3 +++ src/libvirt_private.syms | 1 + 3 files changed, 19 insertions(+)
See my comment on 11/12. You may not need this anymore. -- Respectfully, - Collin Walling

Bi-directional conversion functions. Converts model / name and features / properties. --- src/qemu/qemu_capabilities.c | 137 +++++++++++++++++++++++++++++------ src/qemu/qemu_capabilities.h | 3 + 2 files changed, 118 insertions(+), 22 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index da6fb1f614..89a313b55d 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2737,7 +2737,7 @@ virQEMUCapsInitCPUModelS390(virQEMUCapsPtr qemuCaps, virCPUDefPtr cpu, bool migratable) { - size_t i; + virCPUDefPtr tmp = NULL; if (!modelInfo) { if (type == VIR_DOMAIN_VIRT_KVM) { @@ -2750,30 +2750,12 @@ virQEMUCapsInitCPUModelS390(virQEMUCapsPtr qemuCaps, return 2; } - if (VIR_STRDUP(cpu->model, modelInfo->name) < 0 || - VIR_ALLOC_N(cpu->features, modelInfo->nprops) < 0) + if (!(tmp = virQEMUCapsCPUModelInfoToCPUDef(migratable, modelInfo))) return -1; - cpu->nfeatures_max = modelInfo->nprops; - cpu->nfeatures = 0; - - for (i = 0; i < modelInfo->nprops; i++) { - virCPUFeatureDefPtr feature = cpu->features + cpu->nfeatures; - qemuMonitorCPUPropertyPtr prop = modelInfo->props + i; + *cpu = *tmp; - if (prop->type != QEMU_MONITOR_CPU_PROPERTY_BOOLEAN) - continue; - - if (VIR_STRDUP(feature->name, prop->name) < 0) - return -1; - - if (!prop->value.boolean || - (migratable && prop->migratable == VIR_TRISTATE_BOOL_NO)) - feature->policy = VIR_CPU_FEATURE_DISABLE; - else - feature->policy = VIR_CPU_FEATURE_REQUIRE; - cpu->nfeatures++; - } + VIR_FREE(tmp); return 0; } @@ -3526,6 +3508,117 @@ virQEMUCapsLoadCache(virArch hostArch, return ret; } +/* virCPUDef model => qemuMonitorCPUModelInfo name + * virCPUDef features => qemuMonitorCPUModelInfo boolean properties */ +qemuMonitorCPUModelInfoPtr +virQEMUCapsCPUModelInfoFromCPUDef(const virCPUDef *cpuDef) +{ + size_t i; + qemuMonitorCPUModelInfoPtr cpuModel = NULL; + qemuMonitorCPUModelInfoPtr ret = NULL; + + if (!cpuDef || (VIR_ALLOC(cpuModel) < 0)) + goto cleanup; + + VIR_DEBUG("cpuDef->model = %s", NULLSTR(cpuDef->model)); + + if (VIR_STRDUP(cpuModel->name, cpuDef->model) < 0 || + VIR_ALLOC_N(cpuModel->props, cpuDef->nfeatures) < 0) + goto cleanup; + + /* no visibility on migratability of properties / features */ + cpuModel->props_migratable_valid = false; + + cpuModel->nprops = 0; + + for (i = 0; i < cpuDef->nfeatures; i++) { + qemuMonitorCPUPropertyPtr prop = &(cpuModel->props[cpuModel->nprops]); + virCPUFeatureDefPtr feature = &(cpuDef->features[i]); + + if (!feature || !(feature->name)) + continue; + + if (VIR_STRDUP(prop->name, feature->name) < 0) + goto cleanup; + + prop->migratable = VIR_TRISTATE_BOOL_ABSENT; + prop->type = QEMU_MONITOR_CPU_PROPERTY_BOOLEAN; + + switch (feature->policy) { + case VIR_CPU_FEATURE_FORCE: + case VIR_CPU_FEATURE_REQUIRE: + prop->value.boolean = true; + break; + + case VIR_CPU_FEATURE_FORBID: + case VIR_CPU_FEATURE_DISABLE: + case VIR_CPU_FEATURE_OPTIONAL: + case VIR_CPU_FEATURE_LAST: + prop->value.boolean = false; + break; + } + + cpuModel->nprops += 1; + } + + VIR_STEAL_PTR(ret, cpuModel); + + cleanup: + qemuMonitorCPUModelInfoFree(cpuModel); + return ret; +} + + +/* qemuMonitorCPUModelInfo name => virCPUDef model + * qemuMonitorCPUModelInfo boolean properties => virCPUDef features + * + * migratable_only true: mark non-migratable features as disabled + * false: allow all features as required + */ +virCPUDefPtr +virQEMUCapsCPUModelInfoToCPUDef(bool migratable_only, qemuMonitorCPUModelInfoPtr model) +{ + virCPUDefPtr cpu = NULL; + virCPUDefPtr ret = NULL; + size_t i; + + if (!model || (VIR_ALLOC(cpu) < 0)) + goto cleanup; + + VIR_DEBUG("model->name= %s", NULLSTR(model->name)); + + if (VIR_STRDUP(cpu->model, model->name) < 0 || + VIR_ALLOC_N(cpu->features, model->nprops) < 0) + goto cleanup; + + cpu->nfeatures_max = model->nprops; + cpu->nfeatures = 0; + + for (i = 0; i < model->nprops; i++) { + virCPUFeatureDefPtr feature = cpu->features + cpu->nfeatures; + qemuMonitorCPUPropertyPtr prop = model->props + i; + + if (prop->type != QEMU_MONITOR_CPU_PROPERTY_BOOLEAN) + continue; + + if (VIR_STRDUP(feature->name, prop->name) < 0) + goto cleanup; + + if (!prop->value.boolean || + (migratable_only && prop->migratable == VIR_TRISTATE_BOOL_NO)) + feature->policy = VIR_CPU_FEATURE_DISABLE; + else + feature->policy = VIR_CPU_FEATURE_REQUIRE; + + cpu->nfeatures++; + } + + VIR_STEAL_PTR(ret, cpu); + + cleanup: + virCPUDefFree(cpu); + return ret; +} static void virQEMUCapsFormatHostCPUModelInfo(virQEMUCapsPtr qemuCaps, diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 3519a194e9..af263c9780 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -562,6 +562,9 @@ int virQEMUCapsGetMachineTypesCaps(virQEMUCapsPtr qemuCaps, void virQEMUCapsFilterByMachineType(virQEMUCapsPtr qemuCaps, const char *machineType); +qemuMonitorCPUModelInfoPtr virQEMUCapsCPUModelInfoFromCPUDef(const virCPUDef *cpuDef); +virCPUDefPtr virQEMUCapsCPUModelInfoToCPUDef(bool migratable_only, qemuMonitorCPUModelInfoPtr model); + virFileCachePtr virQEMUCapsCacheNew(const char *libDir, const char *cacheDir, uid_t uid, -- 2.17.1

Quoting Chris Venteicher (2018-06-21 23:42:06)
Bi-directional conversion functions. Converts model / name and features / properties. --- src/qemu/qemu_capabilities.c | 137 +++++++++++++++++++++++++++++------ src/qemu/qemu_capabilities.h | 3 + 2 files changed, 118 insertions(+), 22 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index da6fb1f614..89a313b55d 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2737,7 +2737,7 @@ virQEMUCapsInitCPUModelS390(virQEMUCapsPtr qemuCaps, virCPUDefPtr cpu, bool migratable) { - size_t i; + virCPUDefPtr tmp = NULL;
if (!modelInfo) { if (type == VIR_DOMAIN_VIRT_KVM) { @@ -2750,30 +2750,12 @@ virQEMUCapsInitCPUModelS390(virQEMUCapsPtr qemuCaps, return 2; }
- if (VIR_STRDUP(cpu->model, modelInfo->name) < 0 || - VIR_ALLOC_N(cpu->features, modelInfo->nprops) < 0) + if (!(tmp = virQEMUCapsCPUModelInfoToCPUDef(migratable, modelInfo))) return -1;
- cpu->nfeatures_max = modelInfo->nprops; - cpu->nfeatures = 0; - - for (i = 0; i < modelInfo->nprops; i++) { - virCPUFeatureDefPtr feature = cpu->features + cpu->nfeatures; - qemuMonitorCPUPropertyPtr prop = modelInfo->props + i; + *cpu = *tmp;
Can't overwrite the existing virCPUDef here. Copy over the model, vendor, vendor_id and features and preserve everything else in virCPUDef cpu.
- if (prop->type != QEMU_MONITOR_CPU_PROPERTY_BOOLEAN) - continue; - - if (VIR_STRDUP(feature->name, prop->name) < 0) - return -1; - - if (!prop->value.boolean || - (migratable && prop->migratable == VIR_TRISTATE_BOOL_NO)) - feature->policy = VIR_CPU_FEATURE_DISABLE; - else - feature->policy = VIR_CPU_FEATURE_REQUIRE; - cpu->nfeatures++; - } + VIR_FREE(tmp);
return 0; } @@ -3526,6 +3508,117 @@ virQEMUCapsLoadCache(virArch hostArch, return ret; }
+/* virCPUDef model => qemuMonitorCPUModelInfo name + * virCPUDef features => qemuMonitorCPUModelInfo boolean properties */ +qemuMonitorCPUModelInfoPtr +virQEMUCapsCPUModelInfoFromCPUDef(const virCPUDef *cpuDef) +{ + size_t i; + qemuMonitorCPUModelInfoPtr cpuModel = NULL; + qemuMonitorCPUModelInfoPtr ret = NULL; + + if (!cpuDef || (VIR_ALLOC(cpuModel) < 0)) + goto cleanup; + + VIR_DEBUG("cpuDef->model = %s", NULLSTR(cpuDef->model)); + + if (VIR_STRDUP(cpuModel->name, cpuDef->model) < 0 || + VIR_ALLOC_N(cpuModel->props, cpuDef->nfeatures) < 0) + goto cleanup; + + /* no visibility on migratability of properties / features */ + cpuModel->props_migratable_valid = false; + + cpuModel->nprops = 0; + + for (i = 0; i < cpuDef->nfeatures; i++) { + qemuMonitorCPUPropertyPtr prop = &(cpuModel->props[cpuModel->nprops]); + virCPUFeatureDefPtr feature = &(cpuDef->features[i]); + + if (!feature || !(feature->name)) + continue; + + if (VIR_STRDUP(prop->name, feature->name) < 0) + goto cleanup; + + prop->migratable = VIR_TRISTATE_BOOL_ABSENT; + prop->type = QEMU_MONITOR_CPU_PROPERTY_BOOLEAN; + + switch (feature->policy) { + case VIR_CPU_FEATURE_FORCE: + case VIR_CPU_FEATURE_REQUIRE:
Missing case where feature->policy == -1 (undefined) in modes of use where feature just being enumerated implies true.
+ prop->value.boolean = true; + break; + + case VIR_CPU_FEATURE_FORBID: + case VIR_CPU_FEATURE_DISABLE: + case VIR_CPU_FEATURE_OPTIONAL: + case VIR_CPU_FEATURE_LAST: + prop->value.boolean = false; + break; + } + + cpuModel->nprops += 1; + } + + VIR_STEAL_PTR(ret, cpuModel); + + cleanup: + qemuMonitorCPUModelInfoFree(cpuModel); + return ret; +} + + +/* qemuMonitorCPUModelInfo name => virCPUDef model + * qemuMonitorCPUModelInfo boolean properties => virCPUDef features + * + * migratable_only true: mark non-migratable features as disabled + * false: allow all features as required + */ +virCPUDefPtr +virQEMUCapsCPUModelInfoToCPUDef(bool migratable_only, qemuMonitorCPUModelInfoPtr model) +{ + virCPUDefPtr cpu = NULL; + virCPUDefPtr ret = NULL; + size_t i; + + if (!model || (VIR_ALLOC(cpu) < 0)) + goto cleanup; + + VIR_DEBUG("model->name= %s", NULLSTR(model->name)); + + if (VIR_STRDUP(cpu->model, model->name) < 0 || + VIR_ALLOC_N(cpu->features, model->nprops) < 0) + goto cleanup; + + cpu->nfeatures_max = model->nprops; + cpu->nfeatures = 0; + + for (i = 0; i < model->nprops; i++) { + virCPUFeatureDefPtr feature = cpu->features + cpu->nfeatures; + qemuMonitorCPUPropertyPtr prop = model->props + i; + + if (prop->type != QEMU_MONITOR_CPU_PROPERTY_BOOLEAN) + continue; + + if (VIR_STRDUP(feature->name, prop->name) < 0) + goto cleanup; + + if (!prop->value.boolean || + (migratable_only && prop->migratable == VIR_TRISTATE_BOOL_NO)) + feature->policy = VIR_CPU_FEATURE_DISABLE; + else + feature->policy = VIR_CPU_FEATURE_REQUIRE; + + cpu->nfeatures++; + } + + VIR_STEAL_PTR(ret, cpu); + + cleanup: + virCPUDefFree(cpu); + return ret; +}
static void virQEMUCapsFormatHostCPUModelInfo(virQEMUCapsPtr qemuCaps, diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 3519a194e9..af263c9780 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -562,6 +562,9 @@ int virQEMUCapsGetMachineTypesCaps(virQEMUCapsPtr qemuCaps, void virQEMUCapsFilterByMachineType(virQEMUCapsPtr qemuCaps, const char *machineType);
+qemuMonitorCPUModelInfoPtr virQEMUCapsCPUModelInfoFromCPUDef(const virCPUDef *cpuDef); +virCPUDefPtr virQEMUCapsCPUModelInfoToCPUDef(bool migratable_only, qemuMonitorCPUModelInfoPtr model); + virFileCachePtr virQEMUCapsCacheNew(const char *libDir, const char *cacheDir, uid_t uid, -- 2.17.1

Previously QMPCommandPtr (handle for issuing QMP commands) required an external char * qmperr to persist over the lifespan of QMPCommand to expose a QMP error string outside of QMPCommand. Specifically the external char *qmperr was required between calls to virQEMUCapsInitQMPCommandNew and virQEMUCapsInitQMPCommandAbort. This change allows the qmperr pointer to be maintained within the QMPCommand it's self avoiding the need to track the qmperr variable across a multi-function call lifespan of a QMPCommand. Q) Should we eliminate external qmperr completely as there seems to be no current use of qmperr outside the lifespan of QMPCommand in which an internal qmperr char pointer can persist and be used by anywhere we have access to the QMPCommandPtr? conversion functions removed '57d6df39bd'. --- src/qemu/qemu_capabilities.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 89a313b55d..f2202b652d 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -4242,6 +4242,7 @@ struct _virQEMUCapsInitQMPCommand { uid_t runUid; gid_t runGid; char **qmperr; + char *qmperr_internal; char *monarg; char *monpath; char *pidfile; @@ -4319,7 +4320,11 @@ virQEMUCapsInitQMPCommandNew(char *binary, cmd->runUid = runUid; cmd->runGid = runGid; - cmd->qmperr = qmperr; + + if (qmperr) + cmd->qmperr = qmperr; /* external storage */ + else + cmd->qmperr = &cmd->qmperr_internal; /* cmd internal storage */ /* the ".sock" sufix is important to avoid a possible clash with a qemu * domain called "capabilities" -- 2.17.1

Makes possible to start a single QEMU instance and use that one instance for multiple independent QMP commands without starting and stopping QEMU for each QMP command type. This allows functions outside qemu_capabilities (ex. qemuConnectBaselineHypervisorCPU in qemu_driver) requiring multiple different QMP messages to be sent to QEMU to issue those requests over a single QMP Command Channel without Starting and Stopping QEMU for each independent QMP Command usage. Now in Global Scope: struct virQEMUCapsInitQMPCommand virQEMUCapsInitQMPCommandFree --- src/qemu/qemu_capabilities.c | 21 +-------------------- src/qemu/qemu_capabilities.h | 21 +++++++++++++++++++++ 2 files changed, 22 insertions(+), 20 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index f2202b652d..2bbbfc83f3 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -4235,25 +4235,6 @@ virQEMUCapsInitQMPMonitorTCG(virQEMUCapsPtr qemuCaps ATTRIBUTE_UNUSED, } -typedef struct _virQEMUCapsInitQMPCommand virQEMUCapsInitQMPCommand; -typedef virQEMUCapsInitQMPCommand *virQEMUCapsInitQMPCommandPtr; -struct _virQEMUCapsInitQMPCommand { - char *binary; - uid_t runUid; - gid_t runGid; - char **qmperr; - char *qmperr_internal; - char *monarg; - char *monpath; - char *pidfile; - virCommandPtr cmd; - qemuMonitorPtr mon; - virDomainChrSourceDef config; - pid_t pid; - virDomainObjPtr vm; -}; - - static void virQEMUCapsInitQMPCommandAbort(virQEMUCapsInitQMPCommandPtr cmd) { @@ -4288,7 +4269,7 @@ virQEMUCapsInitQMPCommandAbort(virQEMUCapsInitQMPCommandPtr cmd) } -static void +void virQEMUCapsInitQMPCommandFree(virQEMUCapsInitQMPCommandPtr cmd) { if (!cmd) diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index af263c9780..c9618bb754 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -488,6 +488,27 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; +typedef struct _virQEMUCapsInitQMPCommand virQEMUCapsInitQMPCommand; +typedef virQEMUCapsInitQMPCommand *virQEMUCapsInitQMPCommandPtr; + +struct _virQEMUCapsInitQMPCommand { + char *binary; + uid_t runUid; + gid_t runGid; + char **qmperr; + char *qmperr_internal; + char *monarg; + char *monpath; + char *pidfile; + virCommandPtr cmd; + qemuMonitorPtr mon; + virDomainChrSourceDef config; + pid_t pid; + virDomainObjPtr vm; +}; + +void virQEMUCapsInitQMPCommandFree(virQEMUCapsInitQMPCommandPtr cmd); + typedef struct _virQEMUCaps virQEMUCaps; typedef virQEMUCaps *virQEMUCapsPtr; -- 2.17.1

Start and connect to QEMU so QMP commands can be performed. Isolates code for starting QEMU and establishing Monitor connections from code for obtaining capabilities so that arbitrary QMP commands can be exchanged with QEMU. --- src/qemu/qemu_capabilities.c | 40 +++++++++++++++++++++++++++++++++++- src/qemu/qemu_capabilities.h | 5 +++++ 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 2bbbfc83f3..658f88b327 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -4285,7 +4285,7 @@ virQEMUCapsInitQMPCommandFree(virQEMUCapsInitQMPCommandPtr cmd) static virQEMUCapsInitQMPCommandPtr -virQEMUCapsInitQMPCommandNew(char *binary, +virQEMUCapsInitQMPCommandNew(const char *binary, const char *libDir, uid_t runUid, gid_t runGid, @@ -4426,6 +4426,44 @@ virQEMUCapsInitQMPCommandRun(virQEMUCapsInitQMPCommandPtr cmd, } +/* Start and connect to QEMU so QMP commands can be performed. + */ +virQEMUCapsInitQMPCommandPtr +virQEMUCapsNewQMPCommandConnection(const char *exec, + const char *libDir, uid_t runUid, gid_t runGid, + bool forceTCG) +{ + virQEMUCapsInitQMPCommandPtr cmd = NULL; + virQEMUCapsInitQMPCommandPtr rtn_cmd = NULL; + + VIR_DEBUG("exec =%s", NULLSTR(exec)); + + /* Allocate and initialize QMPCommand structure */ + if (!(cmd = virQEMUCapsInitQMPCommandNew(exec, libDir, + runUid, runGid, NULL))) + goto cleanup; + + /* Spawn QEMU and establish connection for QMP commands */ + if (virQEMUCapsInitQMPCommandRun(cmd, forceTCG) != 0) + goto cleanup; + + /* Exit capabilities negotiation mode and enter QEMU command mode + * by issuing qmp_capabilities command to QEMU */ + if (qemuMonitorSetCapabilities(cmd->mon) < 0) { + VIR_DEBUG("Failed to set monitor capabilities %s", + virGetLastErrorMessage()); + goto cleanup; + } + + VIR_STEAL_PTR(rtn_cmd, cmd); + + cleanup: + virQEMUCapsInitQMPCommandFree(cmd); + + return rtn_cmd; +} + + static int virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps, const char *libDir, diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index c9618bb754..9a62b014ac 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -507,6 +507,11 @@ struct _virQEMUCapsInitQMPCommand { virDomainObjPtr vm; }; +virQEMUCapsInitQMPCommandPtr +virQEMUCapsNewQMPCommandConnection(const char *exec, + const char *libDir, uid_t runUid, gid_t runGid, + bool forceTCG); + void virQEMUCapsInitQMPCommandFree(virQEMUCapsInitQMPCommandPtr cmd); typedef struct _virQEMUCaps virQEMUCaps; -- 2.17.1

Baseline cpu model using QEMU/QMP query-cpu-model-baseline query-cpu-model-baseline only compares two CPUModels so multiple exchanges are needed to evaluate more than two CPUModels. --- src/qemu/qemu_capabilities.c | 89 ++++++++++++++++++++++++++++++++++++ src/qemu/qemu_capabilities.h | 4 ++ 2 files changed, 93 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 658f88b327..1646284588 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -5394,3 +5394,92 @@ virQEMUCapsStripMachineAliases(virQEMUCapsPtr qemuCaps) for (i = 0; i < qemuCaps->nmachineTypes; i++) VIR_FREE(qemuCaps->machineTypes[i].alias); } + + +/* in: + * cpus[0]->model = "z13-base"; + * cpus[0]->features[0].name = "xxx"; + * cpus[0]->features[1].name = "yyy"; + * *** + * cpus[n]->model = "s390x"; + * cpus[n]->features[0].name = "xxx"; + * cpus[n]->features[1].name = "yyy"; + * + * out: + * *baseline->model = "s390x"; + * *baseline->features[0].name = "yyy"; + * + * (ret==0) && (*baseline == NULL) if QMP baseline not supported by QEMU + */ +int +virQEMUCapsQMPBaselineCPUModel(virQEMUCapsInitQMPCommandPtr cmd, + virCPUDefPtr *cpus, + virCPUDefPtr *baseline) +{ + qemuMonitorCPUModelInfoPtr model_baseline = NULL; + qemuMonitorCPUModelInfoPtr new_model_baseline = NULL; + qemuMonitorCPUModelInfoPtr model_b = NULL; + bool migratable_only = true; + int ret = -1; + size_t i; + + int ncpus = virCPUDefListLength(cpus); + + VIR_DEBUG("ncpus = %i", ncpus); + + *baseline = NULL; + + if (!cpus || !cpus[0] || !cpus[1]) { + virReportError(VIR_ERR_INVALID_ARG, "%s", _("less than 2 cpus")); + goto cleanup; + } + + VIR_DEBUG("cpu[0]->model = %s", NULLSTR(cpus[0]->model)); + + if (!(model_baseline = virQEMUCapsCPUModelInfoFromCPUDef(cpus[0]))) + goto cleanup; + + for (i = 1; i < ncpus; i++) { + virCPUDefPtr cpu = cpus[i]; + + VIR_DEBUG("cpu[%lu]->model = %s", i, NULLSTR(cpu->model)); + + if (!(model_b = virQEMUCapsCPUModelInfoFromCPUDef(cpu))) + goto cleanup; + + if (!model_baseline || !model_b) { + virReportError(VIR_ERR_INVALID_ARG, "%s", _("cpu without content")); + goto cleanup; + } + + if (qemuMonitorGetCPUModelBaseline(cmd->mon, model_baseline, + model_b, &new_model_baseline) < 0) + goto cleanup; + + if (!new_model_baseline) { + VIR_DEBUG("QEMU didn't support baseline"); + ret = 0; + goto cleanup; + } + + qemuMonitorCPUModelInfoFree(model_baseline); + qemuMonitorCPUModelInfoFree(model_b); + + model_b = NULL; + + model_baseline = new_model_baseline; + } + + if (!(*baseline = virQEMUCapsCPUModelInfoToCPUDef(migratable_only, model_baseline))) + goto cleanup; + + VIR_DEBUG("baseline->model = %s", NULLSTR((*baseline)->model)); + + ret = 0; + + cleanup: + qemuMonitorCPUModelInfoFree(model_baseline); + qemuMonitorCPUModelInfoFree(model_b); + + return ret; +} diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 9a62b014ac..6098378f6c 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -591,6 +591,10 @@ void virQEMUCapsFilterByMachineType(virQEMUCapsPtr qemuCaps, qemuMonitorCPUModelInfoPtr virQEMUCapsCPUModelInfoFromCPUDef(const virCPUDef *cpuDef); virCPUDefPtr virQEMUCapsCPUModelInfoToCPUDef(bool migratable_only, qemuMonitorCPUModelInfoPtr model); +int virQEMUCapsQMPBaselineCPUModel(virQEMUCapsInitQMPCommandPtr cmd, + virCPUDefPtr *cpus, + virCPUDefPtr *baseline); + virFileCachePtr virQEMUCapsCacheNew(const char *libDir, const char *cacheDir, uid_t uid, -- 2.17.1

On 06/22/2018 12:42 AM, Chris Venteicher wrote:
Baseline cpu model using QEMU/QMP query-cpu-model-baseline
query-cpu-model-baseline only compares two CPUModels so multiple exchanges are needed to evaluate more than two CPUModels. --- src/qemu/qemu_capabilities.c | 89 ++++++++++++++++++++++++++++++++++++ src/qemu/qemu_capabilities.h | 4 ++ 2 files changed, 93 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 658f88b327..1646284588 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -5394,3 +5394,92 @@ virQEMUCapsStripMachineAliases(virQEMUCapsPtr qemuCaps) for (i = 0; i < qemuCaps->nmachineTypes; i++) VIR_FREE(qemuCaps->machineTypes[i].alias); } + + +/* in: + * cpus[0]->model = "z13-base"; + * cpus[0]->features[0].name = "xxx"; + * cpus[0]->features[1].name = "yyy"; + * *** + * cpus[n]->model = "s390x"; + * cpus[n]->features[0].name = "xxx"; + * cpus[n]->features[1].name = "yyy"; + * + * out: + * *baseline->model = "s390x"; + * *baseline->features[0].name = "yyy"; + * + * (ret==0) && (*baseline == NULL) if QMP baseline not supported by QEMU + */ +int +virQEMUCapsQMPBaselineCPUModel(virQEMUCapsInitQMPCommandPtr cmd, + virCPUDefPtr *cpus, + virCPUDefPtr *baseline) +{ + qemuMonitorCPUModelInfoPtr model_baseline = NULL; + qemuMonitorCPUModelInfoPtr new_model_baseline = NULL; + qemuMonitorCPUModelInfoPtr model_b = NULL; + bool migratable_only = true; + int ret = -1; + size_t i; + + int ncpus = virCPUDefListLength(cpus); + + VIR_DEBUG("ncpus = %i", ncpus); + + *baseline = NULL; + + if (!cpus || !cpus[0] || !cpus[1]) { + virReportError(VIR_ERR_INVALID_ARG, "%s", _("less than 2 cpus")); + goto cleanup; + } + + VIR_DEBUG("cpu[0]->model = %s", NULLSTR(cpus[0]->model)); + + if (!(model_baseline = virQEMUCapsCPUModelInfoFromCPUDef(cpus[0]))) + goto cleanup; + + for (i = 1; i < ncpus; i++) { + virCPUDefPtr cpu = cpus[i];
The list of cpus is null-terminated, so you should be able to loop while cpus[i] != NULL and axe the virCPUDefListLength function.
+ + VIR_DEBUG("cpu[%lu]->model = %s", i, NULLSTR(cpu->model)); + + if (!(model_b = virQEMUCapsCPUModelInfoFromCPUDef(cpu))) + goto cleanup; + + if (!model_baseline || !model_b) { + virReportError(VIR_ERR_INVALID_ARG, "%s", _("cpu without content")); + goto cleanup; + } + + if (qemuMonitorGetCPUModelBaseline(cmd->mon, model_baseline, + model_b, &new_model_baseline) < 0) + goto cleanup; + + if (!new_model_baseline) { + VIR_DEBUG("QEMU didn't support baseline"); + ret = 0; + goto cleanup; + } + + qemuMonitorCPUModelInfoFree(model_baseline); + qemuMonitorCPUModelInfoFree(model_b); + + model_b = NULL; + + model_baseline = new_model_baseline; + } + + if (!(*baseline = virQEMUCapsCPUModelInfoToCPUDef(migratable_only, model_baseline))) + goto cleanup; + + VIR_DEBUG("baseline->model = %s", NULLSTR((*baseline)->model)); + + ret = 0; + + cleanup: + qemuMonitorCPUModelInfoFree(model_baseline); + qemuMonitorCPUModelInfoFree(model_b); + + return ret; +} diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 9a62b014ac..6098378f6c 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -591,6 +591,10 @@ void virQEMUCapsFilterByMachineType(virQEMUCapsPtr qemuCaps, qemuMonitorCPUModelInfoPtr virQEMUCapsCPUModelInfoFromCPUDef(const virCPUDef *cpuDef); virCPUDefPtr virQEMUCapsCPUModelInfoToCPUDef(bool migratable_only, qemuMonitorCPUModelInfoPtr model);
+int virQEMUCapsQMPBaselineCPUModel(virQEMUCapsInitQMPCommandPtr cmd, + virCPUDefPtr *cpus, + virCPUDefPtr *baseline); + virFileCachePtr virQEMUCapsCacheNew(const char *libDir, const char *cacheDir, uid_t uid,
-- Respectfully, - Collin Walling

Quoting Collin Walling (2018-06-27 19:03:46)
On 06/22/2018 12:42 AM, Chris Venteicher wrote:
Baseline cpu model using QEMU/QMP query-cpu-model-baseline
query-cpu-model-baseline only compares two CPUModels so multiple exchanges are needed to evaluate more than two CPUModels. --- src/qemu/qemu_capabilities.c | 89 ++++++++++++++++++++++++++++++++++++ src/qemu/qemu_capabilities.h | 4 ++ 2 files changed, 93 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 658f88b327..1646284588 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -5394,3 +5394,92 @@ virQEMUCapsStripMachineAliases(virQEMUCapsPtr qemuCaps) for (i = 0; i < qemuCaps->nmachineTypes; i++) VIR_FREE(qemuCaps->machineTypes[i].alias); } + + +/* in: + * cpus[0]->model = "z13-base"; + * cpus[0]->features[0].name = "xxx"; + * cpus[0]->features[1].name = "yyy"; + * *** + * cpus[n]->model = "s390x"; + * cpus[n]->features[0].name = "xxx"; + * cpus[n]->features[1].name = "yyy"; + * + * out: + * *baseline->model = "s390x"; + * *baseline->features[0].name = "yyy"; + * + * (ret==0) && (*baseline == NULL) if QMP baseline not supported by QEMU + */ +int +virQEMUCapsQMPBaselineCPUModel(virQEMUCapsInitQMPCommandPtr cmd, + virCPUDefPtr *cpus, + virCPUDefPtr *baseline) +{ + qemuMonitorCPUModelInfoPtr model_baseline = NULL; + qemuMonitorCPUModelInfoPtr new_model_baseline = NULL; + qemuMonitorCPUModelInfoPtr model_b = NULL; + bool migratable_only = true; + int ret = -1; + size_t i; + + int ncpus = virCPUDefListLength(cpus); + + VIR_DEBUG("ncpus = %i", ncpus); + + *baseline = NULL; + + if (!cpus || !cpus[0] || !cpus[1]) { + virReportError(VIR_ERR_INVALID_ARG, "%s", _("less than 2 cpus")); + goto cleanup; + } + + VIR_DEBUG("cpu[0]->model = %s", NULLSTR(cpus[0]->model)); + + if (!(model_baseline = virQEMUCapsCPUModelInfoFromCPUDef(cpus[0]))) + goto cleanup;
This check duplicates check in for loop.
+ + for (i = 1; i < ncpus; i++) { + virCPUDefPtr cpu = cpus[i];
The list of cpus is null-terminated, so you should be able to loop while cpus[i] != NULL and axe the virCPUDefListLength function.
Yep. Can probably start with i=0 and init model_baseline in for loop too.
+ + VIR_DEBUG("cpu[%lu]->model = %s", i, NULLSTR(cpu->model)); + + if (!(model_b = virQEMUCapsCPUModelInfoFromCPUDef(cpu))) + goto cleanup; + This check duplicates check below.
+ if (!model_baseline || !model_b) { + virReportError(VIR_ERR_INVALID_ARG, "%s", _("cpu without content")); + goto cleanup; + } + + if (qemuMonitorGetCPUModelBaseline(cmd->mon, model_baseline, + model_b, &new_model_baseline) < 0) + goto cleanup; + + if (!new_model_baseline) { + VIR_DEBUG("QEMU didn't support baseline"); + ret = 0; + goto cleanup; + } + + qemuMonitorCPUModelInfoFree(model_baseline); + qemuMonitorCPUModelInfoFree(model_b); + + model_b = NULL; + + model_baseline = new_model_baseline; + } + + if (!(*baseline = virQEMUCapsCPUModelInfoToCPUDef(migratable_only, model_baseline))) + goto cleanup; + + VIR_DEBUG("baseline->model = %s", NULLSTR((*baseline)->model)); + + ret = 0; + + cleanup: + qemuMonitorCPUModelInfoFree(model_baseline); + qemuMonitorCPUModelInfoFree(model_b); + + return ret; +} diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 9a62b014ac..6098378f6c 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -591,6 +591,10 @@ void virQEMUCapsFilterByMachineType(virQEMUCapsPtr qemuCaps, qemuMonitorCPUModelInfoPtr virQEMUCapsCPUModelInfoFromCPUDef(const virCPUDef *cpuDef); virCPUDefPtr virQEMUCapsCPUModelInfoToCPUDef(bool migratable_only, qemuMonitorCPUModelInfoPtr model);
+int virQEMUCapsQMPBaselineCPUModel(virQEMUCapsInitQMPCommandPtr cmd, + virCPUDefPtr *cpus, + virCPUDefPtr *baseline); + virFileCachePtr virQEMUCapsCacheNew(const char *libDir, const char *cacheDir, uid_t uid,
-- Respectfully, - Collin Walling

Transient S390 configurations require using QEMU to compute CPU Model Baseline and to do CPU Feature Expansion. Start and use a single QEMU instance to do both the baseline and expansion transactions required by BaselineHypervisorCPU. --- src/qemu/qemu_driver.c | 66 ++++++++++++++++++++++++++++++++++++------ 1 file changed, 57 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 921aafcd79..868d6087a9 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -13419,10 +13419,13 @@ qemuConnectBaselineHypervisorCPU(virConnectPtr conn, virArch arch; virDomainVirtType virttype; virDomainCapsCPUModelsPtr cpuModels; - bool migratable; + bool migratable_only; virCPUDefPtr cpu = NULL; char *cpustr = NULL; char **features = NULL; + virQEMUCapsInitQMPCommandPtr cmd = NULL; + bool forceTCG = false; + qemuMonitorCPUModelInfoPtr modelInfo = NULL; virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES | VIR_CONNECT_BASELINE_CPU_MIGRATABLE, NULL); @@ -13430,8 +13433,6 @@ qemuConnectBaselineHypervisorCPU(virConnectPtr conn, if (virConnectBaselineHypervisorCPUEnsureACL(conn) < 0) goto cleanup; - migratable = !!(flags & VIR_CONNECT_BASELINE_CPU_MIGRATABLE); - if (!(cpus = virCPUDefListParse(xmlCPUs, ncpus, VIR_CPU_TYPE_AUTO))) goto cleanup; @@ -13444,6 +13445,19 @@ qemuConnectBaselineHypervisorCPU(virConnectPtr conn, if (!qemuCaps) goto cleanup; + /* QEMU can enumerate non-migratable cpu model features for some archs like x86 + * migratable_only == true: ask for and include only migratable features + * migratable_only == false: ask for and include all features + */ + migratable_only = !!(flags & VIR_CONNECT_BASELINE_CPU_MIGRATABLE); + + if (ARCH_IS_S390(arch)) { + /* QEMU for S390 arch only enumerates migratable features + * No reason to explicitly ask QEMU for or include non-migratable features + */ + migratable_only = true; + } + if (!(cpuModels = virQEMUCapsGetCPUDefinitions(qemuCaps, virttype)) || cpuModels->nmodels == 0) { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, @@ -13456,18 +13470,31 @@ qemuConnectBaselineHypervisorCPU(virConnectPtr conn, if (ARCH_IS_X86(arch)) { int rc = virQEMUCapsGetCPUFeatures(qemuCaps, virttype, - migratable, &features); + migratable_only, &features); if (rc < 0) goto cleanup; if (features && rc == 0) { /* We got only migratable features from QEMU if we asked for them, * no further filtering in virCPUBaseline is desired. */ - migratable = false; + migratable_only = false; } if (!(cpu = virCPUBaseline(arch, cpus, ncpus, cpuModels, - (const char **)features, migratable))) + (const char **)features, migratable_only))) goto cleanup; + } else if (ARCH_IS_S390(arch)) { + + const char *binary = virQEMUCapsGetBinary(qemuCaps); + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + + if (!(cmd = virQEMUCapsNewQMPCommandConnection(binary, cfg->libDir, + cfg->user, cfg->group, + forceTCG))) + goto cleanup; + + if (virQEMUCapsQMPBaselineCPUModel(cmd, cpus, &cpu) < 0) + goto cleanup; /* Content Error */ + } else { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, _("computing baseline hypervisor CPU is not supported " @@ -13477,9 +13504,28 @@ qemuConnectBaselineHypervisorCPU(virConnectPtr conn, cpu->fallback = VIR_CPU_FALLBACK_FORBID; - if ((flags & VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES) && - virCPUExpandFeatures(arch, cpu) < 0) - goto cleanup; + if (flags & VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES) { + if (ARCH_IS_X86(arch)) { + if (virCPUExpandFeatures(arch, cpu) < 0) + goto cleanup; + } else if (ARCH_IS_S390(arch)) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION)) + goto cleanup; + + if (!(modelInfo = virQEMUCapsCPUModelInfoFromCPUDef(cpu))) + goto cleanup; + + if (qemuMonitorGetCPUModelExpansion(cmd->mon, + QEMU_MONITOR_CPU_MODEL_EXPANSION_FULL, + migratable_only, modelInfo) < 0) + goto cleanup; + + virCPUDefFree(cpu); + + if (!(cpu = virQEMUCapsCPUModelInfoToCPUDef(migratable_only, modelInfo))) + goto cleanup; + } + } cpustr = virCPUDefFormat(cpu, NULL); @@ -13488,6 +13534,8 @@ qemuConnectBaselineHypervisorCPU(virConnectPtr conn, virCPUDefFree(cpu); virObjectUnref(qemuCaps); virStringListFree(features); + virQEMUCapsInitQMPCommandFree(cmd); + qemuMonitorCPUModelInfoFree(modelInfo); return cpustr; } -- 2.17.1

Quoting Chris Venteicher (2018-06-21 23:42:11)
Transient S390 configurations require using QEMU to compute CPU Model Baseline and to do CPU Feature Expansion.
Start and use a single QEMU instance to do both the baseline and expansion transactions required by BaselineHypervisorCPU. --- src/qemu/qemu_driver.c | 66 ++++++++++++++++++++++++++++++++++++------ 1 file changed, 57 insertions(+), 9 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 921aafcd79..868d6087a9 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -13419,10 +13419,13 @@ qemuConnectBaselineHypervisorCPU(virConnectPtr conn, virArch arch; virDomainVirtType virttype; virDomainCapsCPUModelsPtr cpuModels; - bool migratable; + bool migratable_only; virCPUDefPtr cpu = NULL; char *cpustr = NULL; char **features = NULL; + virQEMUCapsInitQMPCommandPtr cmd = NULL; + bool forceTCG = false; + qemuMonitorCPUModelInfoPtr modelInfo = NULL;
virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES | VIR_CONNECT_BASELINE_CPU_MIGRATABLE, NULL); @@ -13430,8 +13433,6 @@ qemuConnectBaselineHypervisorCPU(virConnectPtr conn, if (virConnectBaselineHypervisorCPUEnsureACL(conn) < 0) goto cleanup;
- migratable = !!(flags & VIR_CONNECT_BASELINE_CPU_MIGRATABLE); - if (!(cpus = virCPUDefListParse(xmlCPUs, ncpus, VIR_CPU_TYPE_AUTO))) goto cleanup;
@@ -13444,6 +13445,19 @@ qemuConnectBaselineHypervisorCPU(virConnectPtr conn, if (!qemuCaps) goto cleanup;
+ /* QEMU can enumerate non-migratable cpu model features for some archs like x86 + * migratable_only == true: ask for and include only migratable features + * migratable_only == false: ask for and include all features + */ + migratable_only = !!(flags & VIR_CONNECT_BASELINE_CPU_MIGRATABLE); + + if (ARCH_IS_S390(arch)) { + /* QEMU for S390 arch only enumerates migratable features + * No reason to explicitly ask QEMU for or include non-migratable features + */ + migratable_only = true; + } + if (!(cpuModels = virQEMUCapsGetCPUDefinitions(qemuCaps, virttype)) || cpuModels->nmodels == 0) { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, @@ -13456,18 +13470,31 @@ qemuConnectBaselineHypervisorCPU(virConnectPtr conn,
if (ARCH_IS_X86(arch)) { int rc = virQEMUCapsGetCPUFeatures(qemuCaps, virttype, - migratable, &features); + migratable_only, &features); if (rc < 0) goto cleanup; if (features && rc == 0) { /* We got only migratable features from QEMU if we asked for them, * no further filtering in virCPUBaseline is desired. */ - migratable = false; + migratable_only = false; }
if (!(cpu = virCPUBaseline(arch, cpus, ncpus, cpuModels, - (const char **)features, migratable))) + (const char **)features, migratable_only))) goto cleanup; + } else if (ARCH_IS_S390(arch)) { + + const char *binary = virQEMUCapsGetBinary(qemuCaps); + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + + if (!(cmd = virQEMUCapsNewQMPCommandConnection(binary, cfg->libDir, + cfg->user, cfg->group, + forceTCG))) + goto cleanup; + + if (virQEMUCapsQMPBaselineCPUModel(cmd, cpus, &cpu) < 0) + goto cleanup; /* Content Error */
Cover usecase where qemu doesn't support baseline and returns 0 but cpu is NULL.
+ } else { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, _("computing baseline hypervisor CPU is not supported " @@ -13477,9 +13504,28 @@ qemuConnectBaselineHypervisorCPU(virConnectPtr conn,
cpu->fallback = VIR_CPU_FALLBACK_FORBID;
- if ((flags & VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES) && - virCPUExpandFeatures(arch, cpu) < 0) - goto cleanup; + if (flags & VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES) { + if (ARCH_IS_X86(arch)) { + if (virCPUExpandFeatures(arch, cpu) < 0) + goto cleanup; + } else if (ARCH_IS_S390(arch)) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION)) + goto cleanup; + + if (!(modelInfo = virQEMUCapsCPUModelInfoFromCPUDef(cpu))) + goto cleanup; + Were commited to expanding the cpu definition and failure to do so is an error and should not result in updating the model in the output with an unexpanded feature set.
Don't let an error case or lack of support by QEMU for expansion return an unexpanded feature set when user asked for expanded.
+ if (qemuMonitorGetCPUModelExpansion(cmd->mon, + QEMU_MONITOR_CPU_MODEL_EXPANSION_FULL, + migratable_only, modelInfo) < 0) + goto cleanup; + + virCPUDefFree(cpu); +
Expansion enumerates all features and indicates true/false to indicate if feature is/isn't included. Baseline only returns a list of features that are included (true). Filter out all the features that are not included (false) prior to creating the cpu definition.
+ if (!(cpu = virQEMUCapsCPUModelInfoToCPUDef(migratable_only, modelInfo))) + goto cleanup; + } + }
cpustr = virCPUDefFormat(cpu, NULL);
@@ -13488,6 +13534,8 @@ qemuConnectBaselineHypervisorCPU(virConnectPtr conn, virCPUDefFree(cpu); virObjectUnref(qemuCaps); virStringListFree(features); + virQEMUCapsInitQMPCommandFree(cmd); + qemuMonitorCPUModelInfoFree(modelInfo);
return cpustr; } -- 2.17.1
participants (2)
-
Chris Venteicher
-
Collin Walling