[libvirt] [PATCH v1 0/4] RFC CPU Model Comparison via QMP

This is a preview of the CPU comparison (and soon baseline) patches. Comments are welcome on all aspects, as they will help guide the development of the baseline implementation. The first couple of patches simply refactor code. The first introduces an XML -> CPU def helper to be used to extract the CPU definition prior to sending it to the comparison functions. The second patch refactors some of the code from CPU model expansion to be later used by comparison and baseline. Currently, only one of the functions from the refactoring is reused for comparison. The other two will come into play when baseline is implemented. The third is the typical "new capability" patch to introduce query-cpu-model-comparison and test data for s390x. The last patch implements the qemuMonitorJSON function for query-cpu-model-comparison and hooks it up to virsh hypervisor-cpu-compare. This is posted as an RFC to make sure these patches are set in the correct direction before tackling the baseline patches. Thanks! Collin Walling (4): cpu_conf: xml to cpu definition parse helper qemu: monitor: helper functions for CPU models qemu_capabilities: introduce QEMU_CAPS_QUERY_CPU_MODEL_COMPARISON qemu: monitor: implement query-cpu-model-comparison src/conf/cpu_conf.c | 30 +++ src/conf/cpu_conf.h | 6 + src/cpu/cpu.c | 14 +- src/libvirt_private.syms | 1 + src/qemu/qemu_capabilities.c | 49 ++++ src/qemu/qemu_capabilities.h | 10 + src/qemu/qemu_driver.c | 10 + src/qemu/qemu_monitor.c | 22 ++ src/qemu/qemu_monitor.h | 10 + src/qemu/qemu_monitor_json.c | 223 +++++++++++++++--- src/qemu/qemu_monitor_json.h | 11 + .../caps_2.10.0.s390x.xml | 1 + .../caps_2.11.0.s390x.xml | 1 + .../caps_2.12.0.s390x.xml | 1 + .../qemucapabilitiesdata/caps_2.8.0.s390x.xml | 1 + .../qemucapabilitiesdata/caps_2.9.0.s390x.xml | 1 + .../qemucapabilitiesdata/caps_3.0.0.s390x.xml | 1 + 17 files changed, 343 insertions(+), 49 deletions(-) -- 2.20.1

Implement an XML to virCPUDefPtr helper that handles the ctxt prerequisite for virCPUDefParseXML. This does not alter any functionality. Signed-off-by: Collin Walling <walling@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com> --- src/conf/cpu_conf.c | 30 ++++++++++++++++++++++++++++++ src/conf/cpu_conf.h | 6 ++++++ src/cpu/cpu.c | 14 +------------- src/libvirt_private.syms | 1 + 4 files changed, 38 insertions(+), 13 deletions(-) diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c index 33c8b99e49..cf19edcded 100644 --- a/src/conf/cpu_conf.c +++ b/src/conf/cpu_conf.c @@ -260,6 +260,36 @@ virCPUDefCopy(const virCPUDef *cpu) } +int +virCPUDefParseXMLHelper(const char *xml, + const char *xpath, + virCPUType type, + virCPUDefPtr *cpu) +{ + xmlDocPtr doc = NULL; + xmlXPathContextPtr ctxt = NULL; + int ret = -1; + + if (!xml) { + virReportError(VIR_ERR_INVALID_ARG, "%s", _("missing CPU definition")); + goto cleanup; + } + + if (!(doc = virXMLParseStringCtxt(xml, _("(CPU_definition)"), &ctxt))) + goto cleanup; + + if (virCPUDefParseXML(ctxt, xpath, type, cpu) < 0) + goto cleanup; + + ret = 0; + + cleanup: + xmlFreeDoc(doc); + xmlXPathFreeContext(ctxt); + return ret; +} + + /* * Parses CPU definition XML from a node pointed to by @xpath. If @xpath is * NULL, the current node of @ctxt is used (i.e., it is a shortcut to "."). diff --git a/src/conf/cpu_conf.h b/src/conf/cpu_conf.h index c98db65693..8d06b6f79d 100644 --- a/src/conf/cpu_conf.h +++ b/src/conf/cpu_conf.h @@ -181,6 +181,12 @@ virCPUDefCopy(const virCPUDef *cpu); virCPUDefPtr virCPUDefCopyWithoutModel(const virCPUDef *cpu); +int +virCPUDefParseXMLHelper(const char *xml, + const char *xpath, + virCPUType type, + virCPUDefPtr *cpu); + int virCPUDefParseXML(xmlXPathContextPtr ctxt, const char *xpath, diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c index a223ff06e8..6eca956038 100644 --- a/src/cpu/cpu.c +++ b/src/cpu/cpu.c @@ -111,31 +111,19 @@ virCPUCompareXML(virArch arch, const char *xml, bool failIncompatible) { - xmlDocPtr doc = NULL; - xmlXPathContextPtr ctxt = NULL; virCPUDefPtr cpu = NULL; virCPUCompareResult ret = VIR_CPU_COMPARE_ERROR; VIR_DEBUG("arch=%s, host=%p, xml=%s", virArchToString(arch), host, NULLSTR(xml)); - if (!xml) { - virReportError(VIR_ERR_INVALID_ARG, "%s", _("missing CPU definition")); - goto cleanup; - } - - if (!(doc = virXMLParseStringCtxt(xml, _("(CPU_definition)"), &ctxt))) - goto cleanup; - - if (virCPUDefParseXML(ctxt, NULL, VIR_CPU_TYPE_AUTO, &cpu) < 0) + if (virCPUDefParseXMLHelper(xml, NULL, VIR_CPU_TYPE_AUTO, &cpu) < 0) goto cleanup; ret = virCPUCompare(arch, host, cpu, failIncompatible); cleanup: virCPUDefFree(cpu); - xmlXPathFreeContext(ctxt); - xmlFreeDoc(doc); return ret; } diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 94c823d39d..00904d87d7 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -88,6 +88,7 @@ virCPUDefIsEqual; virCPUDefListFree; virCPUDefListParse; virCPUDefParseXML; +virCPUDefParseXMLHelper; virCPUDefStealModel; virCPUDefUpdateFeature; virCPUModeTypeToString; -- 2.20.1

Refactor some code in qemuMonitorJSONGetCPUModelExpansion to be later used for the comparison and baseline functions. This does not alter any functionality. Signed-off-by: Collin Walling <walling@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com> --- src/qemu/qemu_monitor_json.c | 166 ++++++++++++++++++++++++----------- 1 file changed, 114 insertions(+), 52 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 71c452b25b..992cec9efb 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -5513,6 +5513,114 @@ qemuMonitorJSONParseCPUModelProperty(const char *key, return 0; } + +static virJSONValuePtr +qemuMonitorJSONMakeCPUModel(const char *model_name, + size_t nprops, + virCPUFeatureDefPtr props, + bool migratable) +{ + virJSONValuePtr value; + virJSONValuePtr feats = NULL; + size_t i; + + if (!(value = virJSONValueNewObject())) + goto cleanup; + + if (virJSONValueObjectAppendString(value, "name", model_name) < 0) + goto cleanup; + + if (nprops || !migratable) { + if (!(feats = virJSONValueNewObject())) + goto cleanup; + + for (i = 0; i < nprops; i++) { + char *name = props[i].name; + bool enabled = props[i].policy & VIR_CPU_FEATURE_REQUIRE; + + if (virJSONValueObjectAppendBoolean(feats, name, enabled) < 0) + goto cleanup; + } + + if (!migratable) { + if (virJSONValueObjectAppendBoolean(feats, "migratable", false) < 0) + goto cleanup; + } + + if (virJSONValueObjectAppend(value, "props", feats) < 0) + goto cleanup; + } + + return value; + + cleanup: + virJSONValueFree(value); + virJSONValueFree(feats); + return NULL; +} + + +static int +qemuMonitorJSONParseCPUModelData(virJSONValuePtr data, + virJSONValuePtr *cpu_model, + virJSONValuePtr *cpu_props, + const char **cpu_name) +{ + if (!(*cpu_model = virJSONValueObjectGetObject(data, "model"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("QMP command reply data was missing 'model'")); + return -1; + } + + if (!(*cpu_name = virJSONValueObjectGetString(*cpu_model, "name"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("QMP command reply data was missing 'name'")); + return -1; + } + + if (!(*cpu_props = virJSONValueObjectGetObject(*cpu_model, "props"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("QMP command reply data was missing 'props'")); + return -1; + } + + return 0; +} + + +static int +qemuMonitorJSONParseCPUModel(const char *cpu_name, + virJSONValuePtr cpu_props, + qemuMonitorCPUModelInfoPtr *model_info) +{ + qemuMonitorCPUModelInfoPtr machine_model = NULL; + int ret = -1; + + if (VIR_ALLOC(machine_model) < 0) + goto cleanup; + + if (VIR_STRDUP(machine_model->name, cpu_name) < 0) + goto cleanup; + + if (cpu_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_info, machine_model); + ret = 0; + + cleanup: + qemuMonitorCPUModelInfoFree(machine_model); + return ret; +} + + int qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon, qemuMonitorCPUModelExpansionType type, @@ -5521,33 +5629,20 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon, qemuMonitorCPUModelInfoPtr *model_info) { int ret = -1; - virJSONValuePtr model = NULL; - virJSONValuePtr props = NULL; + virJSONValuePtr model; virJSONValuePtr cmd = NULL; virJSONValuePtr reply = NULL; virJSONValuePtr data; virJSONValuePtr cpu_model; - virJSONValuePtr cpu_props; - qemuMonitorCPUModelInfoPtr machine_model = NULL; - char const *cpu_name; + virJSONValuePtr cpu_props = NULL; + const char *cpu_name = ""; const char *typeStr = ""; *model_info = NULL; - if (!(model = virJSONValueNewObject())) + if (!(model = qemuMonitorJSONMakeCPUModel(model_name, 0, NULL, migratable))) goto cleanup; - if (virJSONValueObjectAppendString(model, "name", model_name) < 0) - goto cleanup; - - if (!migratable) { - if (!(props = virJSONValueNewObject()) || - virJSONValueObjectAppendBoolean(props, "migratable", false) < 0 || - virJSONValueObjectAppend(model, "props", props) < 0) - goto cleanup; - props = NULL; - } - retry: switch (type) { case QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC: @@ -5583,11 +5678,8 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon, data = virJSONValueObjectGetObject(reply, "return"); - if (!(cpu_model = virJSONValueObjectGetObject(data, "model"))) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("query-cpu-model-expansion reply data was missing 'model'")); + if (qemuMonitorJSONParseCPUModelData(data, &cpu_model, &cpu_props, &cpu_name) < 0) goto cleanup; - } /* QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC_FULL requests "full" expansion * on the result of the initial "static" expansion. @@ -5602,42 +5694,12 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon, goto retry; } - if (!(cpu_name = virJSONValueObjectGetString(cpu_model, "name"))) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("query-cpu-model-expansion reply data was missing 'name'")); - goto cleanup; - } - - if (!(cpu_props = virJSONValueObjectGetObject(cpu_model, "props"))) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("query-cpu-model-expansion reply data was missing 'props'")); - goto cleanup; - } - - if (VIR_ALLOC(machine_model) < 0) - goto cleanup; - - if (VIR_STRDUP(machine_model->name, cpu_name) < 0) - goto cleanup; - - if (VIR_ALLOC_N(machine_model->props, virJSONValueObjectKeysNumber(cpu_props)) < 0) - goto cleanup; - - if (virJSONValueObjectForeachKeyValue(cpu_props, - qemuMonitorJSONParseCPUModelProperty, - machine_model) < 0) - goto cleanup; - - ret = 0; - *model_info = machine_model; - machine_model = NULL; + ret = qemuMonitorJSONParseCPUModel(cpu_name, cpu_props, model_info); cleanup: - qemuMonitorCPUModelInfoFree(machine_model); virJSONValueFree(cmd); virJSONValueFree(reply); virJSONValueFree(model); - virJSONValueFree(props); return ret; } -- 2.20.1

This capability enables CPU model comparison between two CPU definitions via QMP. Signed-off-by: Collin Walling <walling@linux.ibm.com> --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_3.0.0.s390x.xml | 1 + 8 files changed, 9 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index f8ea66b577..6163fd5399 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -524,6 +524,7 @@ VIR_ENUM_IMPL(virQEMUCaps, "scsi-disk.device_id", "virtio-pci-non-transitional", "overcommit", + "query-cpu-model-comparison", ); @@ -969,6 +970,7 @@ struct virQEMUCapsStringFlags virQEMUCapsCommands[] = { { "query-cpus-fast", QEMU_CAPS_QUERY_CPUS_FAST }, { "qom-list-properties", QEMU_CAPS_QOM_LIST_PROPERTIES }, { "blockdev-del", QEMU_CAPS_BLOCKDEV_DEL }, + { "query-cpu-model-comparison", QEMU_CAPS_QUERY_CPU_MODEL_COMPARISON }, }; struct virQEMUCapsStringFlags virQEMUCapsMigration[] = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 23ecef8c63..0cc3d4d512 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -506,6 +506,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ QEMU_CAPS_SCSI_DISK_DEVICE_ID, /* 'device_id' property of scsi disk */ QEMU_CAPS_VIRTIO_PCI_TRANSITIONAL, /* virtio *-pci-{non-}transitional devices */ QEMU_CAPS_OVERCOMMIT, /* -overcommit */ + QEMU_CAPS_QUERY_CPU_MODEL_COMPARISON, /* qmp query-cpu-model-comparison */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml index 21b918f8d4..cc26e42bc1 100644 --- a/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml @@ -103,6 +103,7 @@ <flag name='egl-headless'/> <flag name='zpci'/> <flag name='iothread.poll-max-ns'/> + <flag name='query-cpu-model-comparison'/> <version>2010000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>39100805</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml index 6cb997d299..eb75c43e6c 100644 --- a/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml @@ -110,6 +110,7 @@ <flag name='egl-headless'/> <flag name='zpci'/> <flag name='iothread.poll-max-ns'/> + <flag name='query-cpu-model-comparison'/> <version>2011000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>39100806</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml index 2930381068..6b7f83cb0e 100644 --- a/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml @@ -121,6 +121,7 @@ <flag name='memory-backend-memfd.hugetlb'/> <flag name='iothread.poll-max-ns'/> <flag name='memory-backend-file.align'/> + <flag name='query-cpu-model-comparison'/> <version>2012000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>39100807</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml index 5a2b7408db..030640d189 100644 --- a/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml @@ -92,6 +92,7 @@ <flag name='sdl-gl'/> <flag name='vhost-vsock'/> <flag name='zpci'/> + <flag name='query-cpu-model-comparison'/> <version>2007093</version> <kvmVersion>0</kvmVersion> <microcodeVersion>39100764</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml index 72ae100a76..af2062d4c1 100644 --- a/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml @@ -97,6 +97,7 @@ <flag name='vhost-vsock'/> <flag name='zpci'/> <flag name='iothread.poll-max-ns'/> + <flag name='query-cpu-model-comparison'/> <version>2009000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>39100765</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_3.0.0.s390x.xml b/tests/qemucapabilitiesdata/caps_3.0.0.s390x.xml index d511377262..aab7934386 100644 --- a/tests/qemucapabilitiesdata/caps_3.0.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_3.0.0.s390x.xml @@ -123,6 +123,7 @@ <flag name='memory-backend-memfd.hugetlb'/> <flag name='iothread.poll-max-ns'/> <flag name='memory-backend-file.align'/> + <flag name='query-cpu-model-comparison'/> <version>3000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>39100757</microcodeVersion> -- 2.20.1

Interfaces with QEMU to compare CPU models. The command takes two CPU models, A and B, that are given a model name and an optional list of CPU features. Through the query-cpu-model-comparison command issued via QMP, a result is produced that contains the comparison evaluation string (identical, superset, subset [s390x specific], incompatible) as well as a list of properties (aka CPU features) responsible for the subset or superset result. Though the "responsible properties" are captured, they are not reported by libvirt. This command is hooked into the virsh hypervisor-cpu-compare command. As such, the CPU model XML provided to the command will be compared to the hypervisor CPU contained in the QEMU capabilities file for the appropriate QEMU binary (for s390x, this CPU definition can be observed via virsh domcapabilities). s390x can report that the first model (A) is a subset of the second (B). If A is the hypervisor CPU and a subset of B (the CPU contained in the XML), then B would not be able to run on this machine and thus we will report that CPU model B is incompatible. Signed-off-by: Collin Walling <walling@linux.ibm.com> --- src/qemu/qemu_capabilities.c | 47 +++++++++++++++++++ src/qemu/qemu_capabilities.h | 9 ++++ src/qemu/qemu_driver.c | 10 ++++ src/qemu/qemu_monitor.c | 22 +++++++++ src/qemu/qemu_monitor.h | 10 ++++ src/qemu/qemu_monitor_json.c | 89 ++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 11 +++++ 7 files changed, 198 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 6163fd5399..1e4b05d535 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -5400,3 +5400,50 @@ virQEMUCapsStripMachineAliases(virQEMUCapsPtr qemuCaps) for (i = 0; i < qemuCaps->nmachineTypes; i++) VIR_FREE(qemuCaps->machineTypes[i].alias); } + +virCPUCompareResult +virQEMUCapsCPUModelComparison(virQEMUCapsPtr qemuCaps, + const char *libDir, + uid_t runUid, + gid_t runGid, + virCPUDefPtr cpu_a, + virCPUDefPtr cpu_b, + bool failIncompatible) +{ + qemuProcessQMPPtr proc = NULL; + qemuMonitorCPUModelInfoPtr result; + int ret = -1; + + if (!(proc = qemuProcessQMPNew(qemuCaps->binary, libDir, + runUid, runGid, false))) + goto cleanup; + + if (qemuProcessQMPStart(proc) < 0) + goto cleanup; + + if (qemuMonitorGetCPUModelComparison(proc->mon, cpu_a->model, + cpu_a->nfeatures, cpu_a->features, + cpu_b->model, cpu_b->nfeatures, + cpu_b->features, &result) < 0) + goto cleanup; + + if (STREQ(result->name, "incompatible") || + STREQ(result->name, "subset")) + ret = VIR_CPU_COMPARE_INCOMPATIBLE; + else if (STREQ(result->name, "identical")) + ret = VIR_CPU_COMPARE_IDENTICAL; + else if (STREQ(result->name, "superset")) + ret = VIR_CPU_COMPARE_SUPERSET; + + if (failIncompatible && ret == VIR_CPU_COMPARE_INCOMPATIBLE) { + ret = VIR_CPU_COMPARE_ERROR; + virReportError(VIR_ERR_CPU_INCOMPATIBLE, NULL); + } + + cleanup: + if (ret < 0) + virQEMUCapsLogProbeFailure(qemuCaps->binary); + + qemuProcessQMPFree(proc); + return ret; +} diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 0cc3d4d512..aa27b18d32 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -647,4 +647,13 @@ virQEMUCapsGetSEVCapabilities(virQEMUCapsPtr qemuCaps); virArch virQEMUCapsArchFromString(const char *arch); const char *virQEMUCapsArchToString(virArch arch); +virCPUCompareResult +virQEMUCapsCPUModelComparison(virQEMUCapsPtr qemuCaps, + const char *libDir, + uid_t runUid, + gid_t runGid, + virCPUDefPtr cpu_a, + virCPUDefPtr cpu_b, + bool failIncompatible); + #endif /* LIBVIRT_QEMU_CAPABILITIES_H */ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 4510b0ce60..2084da1b9d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -13463,9 +13463,11 @@ qemuConnectCompareHypervisorCPU(virConnectPtr conn, { int ret = VIR_CPU_COMPARE_ERROR; virQEMUDriverPtr driver = conn->privateData; + virQEMUDriverConfigPtr config = driver->config; virQEMUCapsPtr qemuCaps = NULL; bool failIncompatible; virCPUDefPtr hvCPU; + virCPUDefPtr cpu; virArch arch; virDomainVirtType virttype; @@ -13500,6 +13502,14 @@ qemuConnectCompareHypervisorCPU(virConnectPtr conn, if (ARCH_IS_X86(arch)) { ret = virCPUCompareXML(arch, hvCPU, xmlCPU, failIncompatible); + } else if (ARCH_IS_S390(arch) && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_CPU_MODEL_COMPARISON)) { + if (virCPUDefParseXMLHelper(xmlCPU, NULL, VIR_CPU_TYPE_AUTO, &cpu) < 0) + goto cleanup; + + ret = virQEMUCapsCPUModelComparison(qemuCaps, config->libDir, + config->user, config->group, + hvCPU, cpu, failIncompatible); } else { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, _("comparing with the hypervisor CPU is not supported " diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index babcbde878..5fd132a268 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3684,6 +3684,28 @@ qemuMonitorGetCPUModelExpansion(qemuMonitorPtr mon, } +int +qemuMonitorGetCPUModelComparison(qemuMonitorPtr mon, + const char *model_a_name, + int model_a_nprops, + virCPUFeatureDefPtr model_a_props, + const char *model_b_name, + int model_b_nprops, + virCPUFeatureDefPtr model_b_props, + qemuMonitorCPUModelInfoPtr *model_result) +{ + VIR_DEBUG("model_a_name=%s model_a_nprops=%d model_b_name=%s model_b_nprops=%d", + model_a_name, model_a_nprops, model_b_name, model_b_nprops); + + QEMU_CHECK_MONITOR(mon); + + return qemuMonitorJSONGetCPUModelComparison(mon, model_a_name, model_a_nprops, + model_a_props, model_b_name, + model_b_nprops, model_b_props, + model_result); +} + + void qemuMonitorCPUModelInfoFree(qemuMonitorCPUModelInfoPtr model_info) { diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index caf62af5e2..0a3ee493b1 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -1066,6 +1066,16 @@ int qemuMonitorGetCPUModelExpansion(qemuMonitorPtr mon, void qemuMonitorCPUModelInfoFree(qemuMonitorCPUModelInfoPtr model_info); +int +qemuMonitorGetCPUModelComparison(qemuMonitorPtr mon, + const char *model_a_name, + int model_a_nprops, + virCPUFeatureDefPtr model_a_props, + const char *model_b_name, + int model_b_nprops, + virCPUFeatureDefPtr model_b_props, + qemuMonitorCPUModelInfoPtr *model_result); + qemuMonitorCPUModelInfoPtr qemuMonitorCPUModelInfoCopy(const qemuMonitorCPUModelInfo *orig); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 992cec9efb..97cb7191e1 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -5704,6 +5704,95 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon, } +static int +qemuMonitorJSONParseCPUModelPropName(size_t pos ATTRIBUTE_UNUSED, + virJSONValuePtr item, + void *opaque) +{ + return qemuMonitorJSONParseCPUModelProperty(virJSONValueGetString(item), + item, opaque); +} + + +int +qemuMonitorJSONGetCPUModelComparison(qemuMonitorPtr mon, + const char *model_a_name, + int model_a_nprops, + virCPUFeatureDefPtr model_a_props, + const char *model_b_name, + int model_b_nprops, + virCPUFeatureDefPtr model_b_props, + qemuMonitorCPUModelInfoPtr *model_result) +{ + int ret = -1; + virJSONValuePtr model_a; + virJSONValuePtr model_b = NULL; + virJSONValuePtr cmd = NULL; + virJSONValuePtr reply = NULL; + virJSONValuePtr data; + const char *result_name; + virJSONValuePtr props; + qemuMonitorCPUModelInfoPtr compare = NULL; + + if (!(model_a = qemuMonitorJSONMakeCPUModel(model_a_name, model_a_nprops, model_a_props, true)) || + !(model_b = qemuMonitorJSONMakeCPUModel(model_b_name, model_b_nprops, model_b_props, true))) + goto cleanup; + + if (!(cmd = qemuMonitorJSONMakeCommand("query-cpu-model-comparison", + "a:modela", &model_a, + "a:modelb", &model_b, + NULL))) + goto cleanup; + + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; + + if (qemuMonitorJSONCheckError(cmd, reply) < 0) + goto cleanup; + + data = virJSONValueObjectGetObject(reply, "return"); + + if (!(result_name = virJSONValueObjectGetString(data, "result"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-cpu-model-comparison reply data was missing " + "'result'")); + goto cleanup; + } + + if (!(props = virJSONValueObjectGetArray(data, "responsible-properties"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-cpu-model-comparison reply data was missing " + "'responsible-properties'")); + goto cleanup; + } + + if (VIR_ALLOC(compare) < 0) + goto cleanup; + + if (VIR_STRDUP(compare->name, result_name) < 0) + goto cleanup; + + if (VIR_ALLOC_N(compare->props, virJSONValueArraySize(props)) < 0) + goto cleanup; + + if (virJSONValueArrayForeachSteal(props, + qemuMonitorJSONParseCPUModelPropName, + compare) < 0) + goto cleanup; + + VIR_STEAL_PTR(*model_result, compare); + ret = 0; + + cleanup: + qemuMonitorCPUModelInfoFree(compare); + virJSONValueFree(cmd); + virJSONValueFree(reply); + virJSONValueFree(model_a); + virJSONValueFree(model_b); + return ret; +} + + int qemuMonitorJSONGetCommands(qemuMonitorPtr mon, char ***commands) { diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index c10513da15..e69be46dd5 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -367,6 +367,17 @@ int qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon, qemuMonitorCPUModelInfoPtr *model_info) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(5); +int +qemuMonitorJSONGetCPUModelComparison(qemuMonitorPtr mon, + const char *model_a_name, + int model_a_nprops, + virCPUFeatureDefPtr model_a_props, + const char *model_b_name, + int model_b_nprops, + virCPUFeatureDefPtr model_b_props, + qemuMonitorCPUModelInfoPtr *model_result) + ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(5) ATTRIBUTE_NONNULL(8); + int qemuMonitorJSONGetCommands(qemuMonitorPtr mon, char ***commands) ATTRIBUTE_NONNULL(2); -- 2.20.1

On 4/15/19 5:59 PM, Collin Walling wrote:
Interfaces with QEMU to compare CPU models. The command takes two CPU models, A and B, that are given a model name and an optional list of CPU features. Through the query-cpu-model-comparison command issued via QMP, a result is produced that contains the comparison evaluation string (identical, superset, subset [s390x specific], incompatible) as well as a list of properties (aka CPU features) responsible for the subset or superset result.
Though the "responsible properties" are captured, they are not reported by libvirt.
This command is hooked into the virsh hypervisor-cpu-compare command. As such, the CPU model XML provided to the command will be compared to the hypervisor CPU contained in the QEMU capabilities file for the appropriate QEMU binary (for s390x, this CPU definition can be observed via virsh domcapabilities).
s390x can report that the first model (A) is a subset of the second (B). If A is the hypervisor CPU and a subset of B (the CPU contained in the XML), then B would not be able to run on this machine and thus we will report that CPU model B is incompatible.
Signed-off-by: Collin Walling <walling@linux.ibm.com> --- src/qemu/qemu_capabilities.c | 47 +++++++++++++++++++ src/qemu/qemu_capabilities.h | 9 ++++ src/qemu/qemu_driver.c | 10 ++++ src/qemu/qemu_monitor.c | 22 +++++++++ src/qemu/qemu_monitor.h | 10 ++++ src/qemu/qemu_monitor_json.c | 89 ++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 11 +++++ 7 files changed, 198 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 6163fd5399..1e4b05d535 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -5400,3 +5400,50 @@ virQEMUCapsStripMachineAliases(virQEMUCapsPtr qemuCaps) for (i = 0; i < qemuCaps->nmachineTypes; i++) VIR_FREE(qemuCaps->machineTypes[i].alias); } + +virCPUCompareResult +virQEMUCapsCPUModelComparison(virQEMUCapsPtr qemuCaps, + const char *libDir, + uid_t runUid, + gid_t runGid, + virCPUDefPtr cpu_a, + virCPUDefPtr cpu_b, + bool failIncompatible) +{ + qemuProcessQMPPtr proc = NULL; + qemuMonitorCPUModelInfoPtr result; + int ret = -1; + + if (!(proc = qemuProcessQMPNew(qemuCaps->binary, libDir, + runUid, runGid, false))) + goto cleanup; + + if (qemuProcessQMPStart(proc) < 0) + goto cleanup; + + if (qemuMonitorGetCPUModelComparison(proc->mon, cpu_a->model, + cpu_a->nfeatures, cpu_a->features, + cpu_b->model, cpu_b->nfeatures, + cpu_b->features, &result) < 0) + goto cleanup; + + if (STREQ(result->name, "incompatible") || + STREQ(result->name, "subset")) + ret = VIR_CPU_COMPARE_INCOMPATIBLE; + else if (STREQ(result->name, "identical")) + ret = VIR_CPU_COMPARE_IDENTICAL; + else if (STREQ(result->name, "superset")) + ret = VIR_CPU_COMPARE_SUPERSET; + + if (failIncompatible && ret == VIR_CPU_COMPARE_INCOMPATIBLE) { + ret = VIR_CPU_COMPARE_ERROR; + virReportError(VIR_ERR_CPU_INCOMPATIBLE, NULL); + } + + cleanup: + if (ret < 0) + virQEMUCapsLogProbeFailure(qemuCaps->binary); + + qemuProcessQMPFree(proc);
Don't you need to free 'result' here as well? This pointer is being passed down to 'qemuMonitorJSONGetCPUModelComparison', receiving the value of 'compare', which is VIR_ALLOC'd inside that function.
+ return ret; +} diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 0cc3d4d512..aa27b18d32 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -647,4 +647,13 @@ virQEMUCapsGetSEVCapabilities(virQEMUCapsPtr qemuCaps); virArch virQEMUCapsArchFromString(const char *arch); const char *virQEMUCapsArchToString(virArch arch);
+virCPUCompareResult +virQEMUCapsCPUModelComparison(virQEMUCapsPtr qemuCaps, + const char *libDir, + uid_t runUid, + gid_t runGid, + virCPUDefPtr cpu_a, + virCPUDefPtr cpu_b, + bool failIncompatible); + #endif /* LIBVIRT_QEMU_CAPABILITIES_H */ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 4510b0ce60..2084da1b9d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -13463,9 +13463,11 @@ qemuConnectCompareHypervisorCPU(virConnectPtr conn, { int ret = VIR_CPU_COMPARE_ERROR; virQEMUDriverPtr driver = conn->privateData; + virQEMUDriverConfigPtr config = driver->config; virQEMUCapsPtr qemuCaps = NULL; bool failIncompatible; virCPUDefPtr hvCPU; + virCPUDefPtr cpu; virArch arch; virDomainVirtType virttype;
@@ -13500,6 +13502,14 @@ qemuConnectCompareHypervisorCPU(virConnectPtr conn,
if (ARCH_IS_X86(arch)) { ret = virCPUCompareXML(arch, hvCPU, xmlCPU, failIncompatible); + } else if (ARCH_IS_S390(arch) && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_CPU_MODEL_COMPARISON)) { + if (virCPUDefParseXMLHelper(xmlCPU, NULL, VIR_CPU_TYPE_AUTO, &cpu) < 0) + goto cleanup; + + ret = virQEMUCapsCPUModelComparison(qemuCaps, config->libDir, + config->user, config->group, + hvCPU, cpu, failIncompatible); } else { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, _("comparing with the hypervisor CPU is not supported " diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index babcbde878..5fd132a268 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3684,6 +3684,28 @@ qemuMonitorGetCPUModelExpansion(qemuMonitorPtr mon, }
+int +qemuMonitorGetCPUModelComparison(qemuMonitorPtr mon, + const char *model_a_name, + int model_a_nprops, + virCPUFeatureDefPtr model_a_props, + const char *model_b_name, + int model_b_nprops, + virCPUFeatureDefPtr model_b_props, + qemuMonitorCPUModelInfoPtr *model_result) +{ + VIR_DEBUG("model_a_name=%s model_a_nprops=%d model_b_name=%s model_b_nprops=%d", + model_a_name, model_a_nprops, model_b_name, model_b_nprops); + + QEMU_CHECK_MONITOR(mon); + + return qemuMonitorJSONGetCPUModelComparison(mon, model_a_name, model_a_nprops, + model_a_props, model_b_name, + model_b_nprops, model_b_props, + model_result); +} + + void qemuMonitorCPUModelInfoFree(qemuMonitorCPUModelInfoPtr model_info) { diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index caf62af5e2..0a3ee493b1 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -1066,6 +1066,16 @@ int qemuMonitorGetCPUModelExpansion(qemuMonitorPtr mon,
void qemuMonitorCPUModelInfoFree(qemuMonitorCPUModelInfoPtr model_info);
+int +qemuMonitorGetCPUModelComparison(qemuMonitorPtr mon, + const char *model_a_name, + int model_a_nprops, + virCPUFeatureDefPtr model_a_props, + const char *model_b_name, + int model_b_nprops, + virCPUFeatureDefPtr model_b_props, + qemuMonitorCPUModelInfoPtr *model_result); + qemuMonitorCPUModelInfoPtr qemuMonitorCPUModelInfoCopy(const qemuMonitorCPUModelInfo *orig);
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 992cec9efb..97cb7191e1 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -5704,6 +5704,95 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon, }
+static int +qemuMonitorJSONParseCPUModelPropName(size_t pos ATTRIBUTE_UNUSED, + virJSONValuePtr item, + void *opaque) +{ + return qemuMonitorJSONParseCPUModelProperty(virJSONValueGetString(item), + item, opaque); +} + + +int +qemuMonitorJSONGetCPUModelComparison(qemuMonitorPtr mon, + const char *model_a_name, + int model_a_nprops, + virCPUFeatureDefPtr model_a_props, + const char *model_b_name, + int model_b_nprops, + virCPUFeatureDefPtr model_b_props, + qemuMonitorCPUModelInfoPtr *model_result) +{ + int ret = -1; + virJSONValuePtr model_a; + virJSONValuePtr model_b = NULL; + virJSONValuePtr cmd = NULL; + virJSONValuePtr reply = NULL; + virJSONValuePtr data; + const char *result_name; + virJSONValuePtr props; + qemuMonitorCPUModelInfoPtr compare = NULL; + + if (!(model_a = qemuMonitorJSONMakeCPUModel(model_a_name, model_a_nprops, model_a_props, true)) || + !(model_b = qemuMonitorJSONMakeCPUModel(model_b_name, model_b_nprops, model_b_props, true))) + goto cleanup; + + if (!(cmd = qemuMonitorJSONMakeCommand("query-cpu-model-comparison", + "a:modela", &model_a, + "a:modelb", &model_b, + NULL))) + goto cleanup; + + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; + + if (qemuMonitorJSONCheckError(cmd, reply) < 0) + goto cleanup; + + data = virJSONValueObjectGetObject(reply, "return"); + + if (!(result_name = virJSONValueObjectGetString(data, "result"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-cpu-model-comparison reply data was missing " + "'result'")); + goto cleanup; + } + + if (!(props = virJSONValueObjectGetArray(data, "responsible-properties"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-cpu-model-comparison reply data was missing " + "'responsible-properties'")); + goto cleanup; + } + + if (VIR_ALLOC(compare) < 0) + goto cleanup; + + if (VIR_STRDUP(compare->name, result_name) < 0) + goto cleanup; + + if (VIR_ALLOC_N(compare->props, virJSONValueArraySize(props)) < 0) + goto cleanup; + + if (virJSONValueArrayForeachSteal(props, + qemuMonitorJSONParseCPUModelPropName, + compare) < 0) + goto cleanup; + + VIR_STEAL_PTR(*model_result, compare); + ret = 0; + + cleanup: + qemuMonitorCPUModelInfoFree(compare); + virJSONValueFree(cmd); + virJSONValueFree(reply); + virJSONValueFree(model_a); + virJSONValueFree(model_b); + return ret; +} + + int qemuMonitorJSONGetCommands(qemuMonitorPtr mon, char ***commands) { diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index c10513da15..e69be46dd5 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -367,6 +367,17 @@ int qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon, qemuMonitorCPUModelInfoPtr *model_info) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(5);
+int +qemuMonitorJSONGetCPUModelComparison(qemuMonitorPtr mon, + const char *model_a_name, + int model_a_nprops, + virCPUFeatureDefPtr model_a_props, + const char *model_b_name, + int model_b_nprops, + virCPUFeatureDefPtr model_b_props, + qemuMonitorCPUModelInfoPtr *model_result) + ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(5) ATTRIBUTE_NONNULL(8); + int qemuMonitorJSONGetCommands(qemuMonitorPtr mon, char ***commands) ATTRIBUTE_NONNULL(2);

On 4/17/19 4:19 PM, Daniel Henrique Barboza wrote:
On 4/15/19 5:59 PM, Collin Walling wrote:
Interfaces with QEMU to compare CPU models. The command takes two CPU models, A and B, that are given a model name and an optional list of CPU features. Through the query-cpu-model-comparison command issued via QMP, a result is produced that contains the comparison evaluation string (identical, superset, subset [s390x specific], incompatible) as well as a list of properties (aka CPU features) responsible for the subset or superset result.
Though the "responsible properties" are captured, they are not reported by libvirt.
This command is hooked into the virsh hypervisor-cpu-compare command. As such, the CPU model XML provided to the command will be compared to the hypervisor CPU contained in the QEMU capabilities file for the appropriate QEMU binary (for s390x, this CPU definition can be observed via virsh domcapabilities).
s390x can report that the first model (A) is a subset of the second (B). If A is the hypervisor CPU and a subset of B (the CPU contained in the XML), then B would not be able to run on this machine and thus we will report that CPU model B is incompatible.
Signed-off-by: Collin Walling <walling@linux.ibm.com> --- src/qemu/qemu_capabilities.c | 47 +++++++++++++++++++ src/qemu/qemu_capabilities.h | 9 ++++ src/qemu/qemu_driver.c | 10 ++++ src/qemu/qemu_monitor.c | 22 +++++++++ src/qemu/qemu_monitor.h | 10 ++++ src/qemu/qemu_monitor_json.c | 89 ++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 11 +++++ 7 files changed, 198 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 6163fd5399..1e4b05d535 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -5400,3 +5400,50 @@ virQEMUCapsStripMachineAliases(virQEMUCapsPtr qemuCaps) for (i = 0; i < qemuCaps->nmachineTypes; i++) VIR_FREE(qemuCaps->machineTypes[i].alias); } + +virCPUCompareResult +virQEMUCapsCPUModelComparison(virQEMUCapsPtr qemuCaps, + const char *libDir, + uid_t runUid, + gid_t runGid, + virCPUDefPtr cpu_a, + virCPUDefPtr cpu_b, + bool failIncompatible) +{ + qemuProcessQMPPtr proc = NULL; + qemuMonitorCPUModelInfoPtr result; + int ret = -1; + + if (!(proc = qemuProcessQMPNew(qemuCaps->binary, libDir, + runUid, runGid, false))) + goto cleanup; + + if (qemuProcessQMPStart(proc) < 0) + goto cleanup; + + if (qemuMonitorGetCPUModelComparison(proc->mon, cpu_a->model, + cpu_a->nfeatures, cpu_a->features, + cpu_b->model, cpu_b->nfeatures, + cpu_b->features, &result) < 0) + goto cleanup; + + if (STREQ(result->name, "incompatible") || + STREQ(result->name, "subset")) + ret = VIR_CPU_COMPARE_INCOMPATIBLE; + else if (STREQ(result->name, "identical")) + ret = VIR_CPU_COMPARE_IDENTICAL; + else if (STREQ(result->name, "superset")) + ret = VIR_CPU_COMPARE_SUPERSET; + + if (failIncompatible && ret == VIR_CPU_COMPARE_INCOMPATIBLE) { + ret = VIR_CPU_COMPARE_ERROR; + virReportError(VIR_ERR_CPU_INCOMPATIBLE, NULL); + } + + cleanup: + if (ret < 0) + virQEMUCapsLogProbeFailure(qemuCaps->binary); + + qemuProcessQMPFree(proc);
Don't you need to free 'result' here as well? This pointer is being passed down to 'qemuMonitorJSONGetCPUModelComparison', receiving the value of 'compare', which is VIR_ALLOC'd inside that function.
You are correct. I there were a few pointers I forgot to free up littered around these patches. I hope to have them all resolved before posting the next version. [...]

On 4/15/19 5:59 PM, Collin Walling wrote:
This is a preview of the CPU comparison (and soon baseline) patches. Comments are welcome on all aspects, as they will help guide the development of the baseline implementation.
The first couple of patches simply refactor code. The first introduces an XML -> CPU def helper to be used to extract the CPU definition prior to sending it to the comparison functions. The second patch refactors some of the code from CPU model expansion to be later used by comparison and baseline. Currently, only one of the functions from the refactoring is reused for comparison. The other two will come into play when baseline is implemented.
The third is the typical "new capability" patch to introduce query-cpu-model-comparison and test data for s390x.
The last patch implements the qemuMonitorJSON function for query-cpu-model-comparison and hooks it up to virsh hypervisor-cpu-compare.
This is posted as an RFC to make sure these patches are set in the correct direction before tackling the baseline patches.
Tested in both x86 (working as expected, falling back to virCPUCompareXML) and in Power 9 (failing "as expected" - I'll see if I can understand if there is a viable reason of why Power hosts can't handle it like x86). Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Thanks!
Collin Walling (4): cpu_conf: xml to cpu definition parse helper qemu: monitor: helper functions for CPU models qemu_capabilities: introduce QEMU_CAPS_QUERY_CPU_MODEL_COMPARISON qemu: monitor: implement query-cpu-model-comparison
src/conf/cpu_conf.c | 30 +++ src/conf/cpu_conf.h | 6 + src/cpu/cpu.c | 14 +- src/libvirt_private.syms | 1 + src/qemu/qemu_capabilities.c | 49 ++++ src/qemu/qemu_capabilities.h | 10 + src/qemu/qemu_driver.c | 10 + src/qemu/qemu_monitor.c | 22 ++ src/qemu/qemu_monitor.h | 10 + src/qemu/qemu_monitor_json.c | 223 +++++++++++++++--- src/qemu/qemu_monitor_json.h | 11 + .../caps_2.10.0.s390x.xml | 1 + .../caps_2.11.0.s390x.xml | 1 + .../caps_2.12.0.s390x.xml | 1 + .../qemucapabilitiesdata/caps_2.8.0.s390x.xml | 1 + .../qemucapabilitiesdata/caps_2.9.0.s390x.xml | 1 + .../qemucapabilitiesdata/caps_3.0.0.s390x.xml | 1 + 17 files changed, 343 insertions(+), 49 deletions(-)

On 4/17/19 4:25 PM, Daniel Henrique Barboza wrote:
On 4/15/19 5:59 PM, Collin Walling wrote:
This is a preview of the CPU comparison (and soon baseline) patches. Comments are welcome on all aspects, as they will help guide the development of the baseline implementation.
The first couple of patches simply refactor code. The first introduces an XML -> CPU def helper to be used to extract the CPU definition prior to sending it to the comparison functions. The second patch refactors some of the code from CPU model expansion to be later used by comparison and baseline. Currently, only one of the functions from the refactoring is reused for comparison. The other two will come into play when baseline is implemented.
The third is the typical "new capability" patch to introduce query-cpu-model-comparison and test data for s390x.
The last patch implements the qemuMonitorJSON function for query-cpu-model-comparison and hooks it up to virsh hypervisor-cpu-compare.
This is posted as an RFC to make sure these patches are set in the correct direction before tackling the baseline patches.
Tested in both x86 (working as expected, falling back to virCPUCompareXML) and in Power 9 (failing "as expected" - I'll see if I can understand if there is a viable reason of why Power hosts can't handle it like x86).
Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Much appreciated! Thanks for testing and reviewing.
Thanks!
Collin Walling (4): cpu_conf: xml to cpu definition parse helper qemu: monitor: helper functions for CPU models qemu_capabilities: introduce QEMU_CAPS_QUERY_CPU_MODEL_COMPARISON qemu: monitor: implement query-cpu-model-comparison
src/conf/cpu_conf.c | 30 +++ src/conf/cpu_conf.h | 6 + src/cpu/cpu.c | 14 +- src/libvirt_private.syms | 1 + src/qemu/qemu_capabilities.c | 49 ++++ src/qemu/qemu_capabilities.h | 10 + src/qemu/qemu_driver.c | 10 + src/qemu/qemu_monitor.c | 22 ++ src/qemu/qemu_monitor.h | 10 + src/qemu/qemu_monitor_json.c | 223 +++++++++++++++--- src/qemu/qemu_monitor_json.h | 11 + .../caps_2.10.0.s390x.xml | 1 + .../caps_2.11.0.s390x.xml | 1 + .../caps_2.12.0.s390x.xml | 1 + .../qemucapabilitiesdata/caps_2.8.0.s390x.xml | 1 + .../qemucapabilitiesdata/caps_2.9.0.s390x.xml | 1 + .../qemucapabilitiesdata/caps_3.0.0.s390x.xml | 1 + 17 files changed, 343 insertions(+), 49 deletions(-)
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
participants (2)
-
Collin Walling
-
Daniel Henrique Barboza