Re: [libvirt] [PATCH v2 11/33] qemu: Store more types in qemuMonitorCPUModelInfo

While query-cpu-model-expansion returns only boolean features on s390, but x86_64 reports some integer and string properties which we are interested in. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; --- Notes: Version 2: - no change src/qemu/qemu_capabilities.c | 84 ++++++++++++++++-------- src/qemu/qemu_monitor.c | 22 ++++++- src/qemu/qemu_monitor.h | 23 +++++-- src/qemu/qemu_monitor_json.c | 37 ++++++++--- tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml | 7 ++ 5 files changed, 133 insertions(+), 40 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index aab336954..466852d13 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -3074,14 +3074,16 @@ virQEMUCapsInitCPUModelS390(virQEMUCapsPtr qemuCaps, cpu->nfeatures = 0; for (i = 0; i < modelInfo->nprops; i++) { - if (VIR_STRDUP(cpu->features[i].name, modelInfo->props[i].name) < 0) + virCPUFeatureDefPtr feature = cpu->features + cpu->nfeatures; + qemuMonitorCPUPropertyPtr prop = modelInfo->props + i; + + if (prop->type != QEMU_MONITOR_CPU_PROPERTY_BOOLEAN) + continue;

+ + if (VIR_STRDUP(feature->name, prop->name) < 0) return -1; - - if (modelInfo->props[i].supported) - cpu->features[i].policy = VIR_CPU_FEATURE_REQUIRE; - else - cpu->features[i].policy = VIR_CPU_FEATURE_DISABLE; - + feature->policy = prop->value.boolean ? VIR_CPU_FEATURE_REQUIRE + : VIR_CPU_FEATURE_DISABLE; cpu->nfeatures++; } @@ -3154,7 +3156,6 @@ static int virQEMUCapsLoadHostCPUModelInfo(virQEMUCapsPtr qemuCaps, xmlXPathContextPtr ctxt) { - char *str = NULL; xmlNodePtr hostCPUNode; xmlNodePtr *featureNodes = NULL; xmlNodePtr oldnode = ctxt->node; @@ -3187,30 +3188,47 @@ virQEMUCapsLoadHostCPUModelInfo(virQEMUCapsPtr qemuCaps, hostCPU->nprops = n; for (i = 0; i < n; i++) { - hostCPU->props[i].name = virXMLPropString(featureNodes[i], "name"); - if (!hostCPU->props[i].name) { + qemuMonitorCPUPropertyPtr prop = hostCPU->props + i; + ctxt->node = featureNodes[i]; + + if (!(prop->name = virXMLPropString(ctxt->node, "name"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("missing 'name' attribute for a host CPU" " model property in QEMU capabilities cache")); goto cleanup; }

- if (!(str = virXMLPropString(featureNodes[i], "boolean"))) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("missing 'boolean' attribute for a host CPU" - " model property in QEMU capabilities cache")); - goto cleanup; - } - if (STREQ(str, "yes")) { - hostCPU->props[i].supported = true; - } else if (STREQ(str, "no")) { - hostCPU->props[i].supported = false; + if (virXPathBoolean("boolean(./@boolean)", ctxt)) { + if (virXPathBoolean("./@boolean='yes'", ctxt)) + prop->value.boolean = true; + prop->type = QEMU_MONITOR_CPU_PROPERTY_BOOLEAN; + } else if (virXPathBoolean("boolean(./@string)", ctxt)) { + prop->value.string = virXMLPropString(ctxt->node, "string"); + if (!prop->value.string) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("invalid string value for '%s' host CPU " + "model property in QEMU capabilities cache"), + prop->name); + goto cleanup; + } + prop->type = QEMU_MONITOR_CPU_PROPERTY_STRING; + } else if (virXPathBoolean("boolean(./@ull)", ctxt)) {

+ if (virXPathULongLong("string(./@ull)", ctxt, + &prop->value.ull) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("invalid integer value for '%s' host CPU "

+ "model property in QEMU capabilities cache"), + prop->name); + goto cleanup; + } + prop->type = QEMU_MONITOR_CPU_PROPERTY_ULL;

} else { virReportError(VIR_ERR_INTERNAL_ERROR, - _("invalid boolean value: '%s'"), str); + _("missing value for '%s' host CPU model " + "property in QEMU capabilities cache"), + prop->name);

goto cleanup; } - VIR_FREE(str); } } @@ -3220,7 +3238,6 @@ virQEMUCapsLoadHostCPUModelInfo(virQEMUCapsPtr qemuCaps, cleanup: ctxt->node = oldnode; - VIR_FREE(str); VIR_FREE(featureNodes); qemuMonitorCPUModelInfoFree(hostCPU); return ret; @@ -3552,9 +3569,24 @@ virQEMUCapsFormatHostCPUModelInfo(virQEMUCapsPtr qemuCaps, virBufferAdjustIndent(buf, 2); for (i = 0; i < model->nprops; i++) { - virBufferAsprintf(buf, "<property name='%s' boolean='%s'/>\n", - model->props[i].name, - model->props[i].supported ? "yes" : "no"); + qemuMonitorCPUPropertyPtr prop = model->props + i; + + switch (prop->type) { + case QEMU_MONITOR_CPU_PROPERTY_BOOLEAN: + virBufferAsprintf(buf, "<property name='%s' boolean='%s'/>\n", + prop->name, prop->value.boolean ? "yes" : "no"); + break; + + case QEMU_MONITOR_CPU_PROPERTY_STRING: + virBufferAsprintf(buf, "<property name='%s' ", prop->name); + virBufferEscapeString(buf, "string='%s'/>\n", prop->value.string); + break; + + case QEMU_MONITOR_CPU_PROPERTY_ULL: + virBufferAsprintf(buf, "<property name='%s' ull='%llu'/>\n", + prop->name, prop->value.ull); + break; + }

} virBufferAdjustIndent(buf, -2); diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index b15207a69..a434b234b 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3661,8 +3661,11 @@ qemuMonitorCPUModelInfoFree(qemuMonitorCPUModelInfoPtr model_info) if (!model_info) return; - for (i = 0; i < model_info->nprops; i++) + for (i = 0; i < model_info->nprops; i++) { VIR_FREE(model_info->props[i].name); + if (model_info->props[i].type == QEMU_MONITOR_CPU_PROPERTY_STRING) + VIR_FREE(model_info->props[i].value.string); + } VIR_FREE(model_info->props); VIR_FREE(model_info->name); @@ -3691,7 +3694,22 @@ qemuMonitorCPUModelInfoCopy(const qemuMonitorCPUModelInfo *orig) if (VIR_STRDUP(copy->props[i].name, orig->props[i].name) < 0) goto error; - copy->props[i].supported = orig->props[i].supported; + copy->props[i].type = orig->props[i].type; + switch (orig->props[i].type) { + case QEMU_MONITOR_CPU_PROPERTY_BOOLEAN: + copy->props[i].value.boolean = orig->props[i].value.boolean; + break; + + case QEMU_MONITOR_CPU_PROPERTY_STRING: + if (VIR_STRDUP(copy->props[i].value.string, + orig->props[i].value.string) < 0) + goto error; + break; + + case QEMU_MONITOR_CPU_PROPERTY_ULL: + copy->props[i].value.ull = orig->props[i].value.ull; + break; + } } return copy; diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 8811d8501..112f041f1 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -921,16 +921,31 @@ int qemuMonitorGetCPUDefinitions(qemuMonitorPtr mon, qemuMonitorCPUDefInfoPtr **cpus); void qemuMonitorCPUDefInfoFree(qemuMonitorCPUDefInfoPtr cpu); +typedef enum { + QEMU_MONITOR_CPU_PROPERTY_BOOLEAN,

+ QEMU_MONITOR_CPU_PROPERTY_STRING, + QEMU_MONITOR_CPU_PROPERTY_ULL,

+} qemuMonitorCPUPropertyType; + +typedef struct _qemuMonitorCPUProperty qemuMonitorCPUProperty; +typedef qemuMonitorCPUProperty *qemuMonitorCPUPropertyPtr; +struct _qemuMonitorCPUProperty { + char *name; + qemuMonitorCPUPropertyType type; + union { + bool boolean; + char *string; + unsigned long long ull; + } value; +}; + typedef struct _qemuMonitorCPUModelInfo qemuMonitorCPUModelInfo; typedef qemuMonitorCPUModelInfo *qemuMonitorCPUModelInfoPtr; struct _qemuMonitorCPUModelInfo { char *name; size_t nprops; - struct { - char *name; - bool supported; - } *props; + qemuMonitorCPUPropertyPtr props; }; int qemuMonitorGetCPUModelExpansion(qemuMonitorPtr mon, diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 1d281af48..415761525 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -4983,18 +4983,39 @@ qemuMonitorJSONParseCPUModelProperty(const char *key, void *opaque) { qemuMonitorCPUModelInfoPtr machine_model = opaque; - size_t n = machine_model->nprops; - bool supported; + qemuMonitorCPUPropertyPtr prop; - if (virJSONValueGetBoolean(value, &supported) < 0) + prop = machine_model->props + machine_model->nprops; + + switch ((virJSONType) value->type) { + case VIR_JSON_TYPE_STRING: + if (VIR_STRDUP(prop->value.string, virJSONValueGetString(value)) < 0) + return -1; + prop->type = QEMU_MONITOR_CPU_PROPERTY_STRING; + break; + + case VIR_JSON_TYPE_NUMBER: + /* Ignore numbers which cannot be parsed as unsigned long long */ + if (virJSONValueGetNumberUlong(value, &prop->value.ull) < 0) + return 0; + prop->type = QEMU_MONITOR_CPU_PROPERTY_ULL; + break; + + case VIR_JSON_TYPE_BOOLEAN: + virJSONValueGetBoolean(value, &prop->value.boolean); + prop->type = QEMU_MONITOR_CPU_PROPERTY_BOOLEAN; + break; + + case VIR_JSON_TYPE_OBJECT: + case VIR_JSON_TYPE_ARRAY: + case VIR_JSON_TYPE_NULL: return 0; - - if (VIR_STRDUP(machine_model->props[n].name, key) < 0) - return -1; - - machine_model->props[n].supported = supported; + } machine_model->nprops++; + if (VIR_STRDUP(prop->name, key) < 0) + return -1; + return 0; } diff --git a/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml index c13e8318f..32368e648 100644 --- a/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml @@ -222,11 +222,13 @@ <property name='avx512cd' boolean='no'/> <property name='decodeassists' boolean='no'/> <property name='sse4.1' boolean='yes'/> + <property name='family' ull='6'/> <property name='avx512f' boolean='no'/> <property name='msr' boolean='yes'/> <property name='mce' boolean='yes'/> <property name='mca' boolean='yes'/> <property name='xcrypt' boolean='no'/> + <property name='min-level' ull='13'/> <property name='xgetbv1' boolean='yes'/> <property name='cid' boolean='no'/> <property name='ds' boolean='no'/> @@ -242,6 +244,7 @@ <property name='xcrypt-en' boolean='no'/> <property name='pn' boolean='no'/> <property name='dca' boolean='no'/> + <property name='vendor' string='GenuineIntel'/> <property name='pku' boolean='no'/> <property name='smx' boolean='no'/> <property name='cmp-legacy' boolean='no'/> @@ -287,6 +290,7 @@ <property name='sse4.2' boolean='yes'/> <property name='pge' boolean='yes'/> <property name='pdcm' boolean='no'/> + <property name='model' ull='94'/> <property name='movbe' boolean='yes'/> <property name='nrip-save' boolean='no'/> <property name='ssse3' boolean='yes'/> @@ -297,6 +301,7 @@ <property name='fma' boolean='yes'/> <property name='cx16' boolean='yes'/> <property name='de' boolean='yes'/> + <property name='stepping' ull='3'/> <property name='xsave' boolean='yes'/> <property name='clflush' boolean='yes'/> <property name='skinit' boolean='no'/> @@ -334,6 +339,7 @@ <property name='sep' boolean='yes'/> <property name='nodeid-msr' boolean='no'/> <property name='misalignsse' boolean='no'/> + <property name='min-xlevel' ull='2147483656'/> <property name='bmi1' boolean='yes'/> <property name='bmi2' boolean='yes'/> <property name='kvm-pv-unhalt' boolean='yes'/> @@ -363,6 +369,7 @@ <property name='pse36' boolean='yes'/> <property name='tbm' boolean='no'/> <property name='wdt' boolean='no'/> + <property name='model-id' string='Intel(R) Xeon(R) CPU E3-1245 v5 @ 3.50GHz'/> <property name='sha-ni' boolean='no'/> <property name='abm' boolean='yes'/> <property name='avx512pf' boolean='no'/>