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@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
