9:03 a.m.
Changelog:
v3
- rebased on master
v2
- numerous cleanups
- removed "policy fix function" and now properly check
for policy == -1 in the CPUDef -> JSON parser
- resolved some memory leaks
- added string arg to qemuMonitorJSONParseCPUModelData for
error message to print out proper command name
v1
- introduce baseline
- split patches into small chunks
- free'd lingering qemuMonitorCPUModelInfo pointer
- when converting from virCPUDef -> virJSON, consider
feature policy FORCED for enabled
___
To run these patches, execute the virsh hypervisor-cpu-compare or
hypervisor-cpu-baseline commands and pass an XML file describing one or
more CPU definition. You can use the definition from virsh domcapabilities
or from a guest XML. There is no need extract it from the file and place
it a new one, as the XML parser will look specifically for the CPU tags.
___
These patches hookup the virsh hypervisor-cpu-compare/baseline commands
for the s390x architecture. They take an XML file describing some CPU
definitions and passes the data to QEMU, where the actual CPU model
comparison / baseline calculation is handled (available since QEMU 2.8.5).
These calculations are compared against / baselined with the hypervisor
CPU model, which can be observed via the virsh domcapabilities command
for s390x.
When baselining CPU models and the user appends the --features argument
to the command, s390x will only report back features that supersede the
base model definition.
*NOTE* if the --features flag is intended to expand /ALL/ features
available to a CPU model (such as the huge list of features reported
by a full CPU model expansion), please let me know and I can resolve
this.
The first patch pulls some code out of the CPU Model Expansion JSON
function so that it can be later used for the Comparison and Baseline
JSON functions.
The rest of the patches follow this sequence:
- introduce JSON monitor functions
- introduce capability and update test files
- hook up monitor functions to virsh command
Patch 7 pulls out some code from the CPUDef XML parser to be
reused in the comparison hookup.
Thanks.
x86 and Power review by Daniel Henrique Barboza (thanks!)
Collin Walling (8):
qemu_monitor: helper functions for CPU models
qemu_monitor: implement query-cpu-model-baseline
qemu_capabilities: introduce QEMU_CAPS_QUERY_CPU_MODEL_BASELINE
qemu_driver: hook up query-cpu-model-baseline
qemu_monitor: implement query-cpu-model-comparison
qemu_capabilities: introduce QEMU_CAPS_QUERY_CPU_MODEL_COMPARISON
cpu_conf: xml to cpu definition parse helper
qemu_driver: hook up 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 | 136 +++++++++++
src/qemu/qemu_capabilities.h | 18 ++
src/qemu/qemu_driver.c | 38 +++
src/qemu/qemu_monitor.c | 44 ++++
src/qemu/qemu_monitor.h | 18 ++
src/qemu/qemu_monitor_json.c | 297 ++++++++++++++++++++---
src/qemu/qemu_monitor_json.h | 20 ++
tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml | 2 +
tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml | 2 +
tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml | 2 +
tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml | 2 +
tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml | 2 +
tests/qemucapabilitiesdata/caps_3.0.0.s390x.xml | 2 +
tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml | 2 +
18 files changed, 587 insertions(+), 49 deletions(-)
--
2.7.4
9:03 a.m.
New subject: [libvirt] [PATCH v4 1/8] qemu_monitor: helper functions for CPU models
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(a)linux.ibm.com>
Reviewed-by: Bjoern Walk <bwalk(a)linux.ibm.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413(a)gmail.com>
---
src/qemu/qemu_monitor_json.c | 173 ++++++++++++++++++++++++++++++-------------
1 file changed, 121 insertions(+), 52 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 8723ff4..f6bf7f2 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -5616,6 +5616,120 @@ 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 = false;
+
+ /* policy may be reported as -1 if the CPU def is a host model */
+ if (props[i].policy == VIR_CPU_FEATURE_REQUIRE ||
+ props[i].policy == VIR_CPU_FEATURE_FORCE ||
+ props[i].policy == -1)
+ enabled = true;
+
+ if (virJSONValueObjectAppendBoolean(feats, name, enabled) < 0)
+ goto cleanup;
+ }
+
+ if (!migratable &&
+ 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,
+ const char *cmd_name)
+{
+ if (!(*cpu_model = virJSONValueObjectGetObject(data, "model"))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("%s reply data was missing 'model'"),
cmd_name);
+ return -1;
+ }
+
+ if (!(*cpu_name = virJSONValueObjectGetString(*cpu_model, "name"))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("%s reply data was missing 'name'"),
cmd_name);
+ return -1;
+ }
+
+ /* Some older CPU models do not report properties */
+ *cpu_props = virJSONValueObjectGetObject(*cpu_model, "props");
+
+ 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) {
+ size_t nprops = virJSONValueObjectKeysNumber(cpu_props);
+
+ if (VIR_ALLOC_N(machine_model->props, nprops) < 0)
+ goto cleanup;
+
+ if (virJSONValueObjectForeachKeyValue(cpu_props,
+ qemuMonitorJSONParseCPUModelProperty,
+ machine_model) < 0)
+ goto cleanup;
+ }
+
+ VIR_STEAL_PTR(*model_info, machine_model);
+ ret = 0;
+
+ cleanup:
+ qemuMonitorCPUModelInfoFree(machine_model);
+ return ret;
+}
+
+
int
qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon,
qemuMonitorCPUModelExpansionType type,
@@ -5624,33 +5738,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:
@@ -5686,11 +5787,9 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon,
data = virJSONValueObjectGetObject(reply, "return");
- if (!(cpu_model = virJSONValueObjectGetObject(data, "model"))) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("query-cpu-model-expansion reply data was missing
'model'"));
+ if (qemuMonitorJSONParseCPUModelData(data, &cpu_model, &cpu_props,
&cpu_name,
+ "query-cpu-model-expansion") < 0)
goto cleanup;
- }
/* QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC_FULL requests "full" expansion
* on the result of the initial "static" expansion.
@@ -5705,42 +5804,12 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon,
goto retry;
}
- if (!(cpu_name = virJSONValueObjectGetString(cpu_model, "name"))) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("query-cpu-model-expansion reply data was missing
'name'"));
- goto cleanup;
- }
-
- if (!(cpu_props = virJSONValueObjectGetObject(cpu_model, "props"))) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("query-cpu-model-expansion reply data was missing
'props'"));
- goto cleanup;
- }
-
- if (VIR_ALLOC(machine_model) < 0)
- goto cleanup;
-
- if (VIR_STRDUP(machine_model->name, cpu_name) < 0)
- goto cleanup;
-
- if (VIR_ALLOC_N(machine_model->props, virJSONValueObjectKeysNumber(cpu_props))
< 0)
- goto cleanup;
-
- if (virJSONValueObjectForeachKeyValue(cpu_props,
- qemuMonitorJSONParseCPUModelProperty,
- machine_model) < 0)
- goto cleanup;
-
- ret = 0;
- *model_info = machine_model;
- machine_model = NULL;
+ ret = qemuMonitorJSONParseCPUModel(cpu_name, cpu_props, model_info);
cleanup:
- qemuMonitorCPUModelInfoFree(machine_model);
virJSONValueFree(cmd);
virJSONValueFree(reply);
virJSONValueFree(model);
- virJSONValueFree(props);
return ret;
}
--
2.7.4
11:18 a.m.
New subject: [libvirt] [PATCH v4 1/8] qemu_monitor: helper functions for CPU models
Reviewed-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com>
On 7/17/19 4:03 PM, Collin Walling wrote:
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(a)linux.ibm.com>
Reviewed-by: Bjoern Walk <bwalk(a)linux.ibm.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413(a)gmail.com>
---
src/qemu/qemu_monitor_json.c | 173 ++++++++++++++++++++++++++++++-------------
1 file changed, 121 insertions(+), 52 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 8723ff4..f6bf7f2 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -5616,6 +5616,120 @@ 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 = false;
+
+ /* policy may be reported as -1 if the CPU def is a host model */
+ if (props[i].policy == VIR_CPU_FEATURE_REQUIRE ||
+ props[i].policy == VIR_CPU_FEATURE_FORCE ||
+ props[i].policy == -1)
+ enabled = true;
+
+ if (virJSONValueObjectAppendBoolean(feats, name, enabled) < 0)
+ goto cleanup;
+ }
+
+ if (!migratable &&
+ 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,
+ const char *cmd_name)
+{
+ if (!(*cpu_model = virJSONValueObjectGetObject(data, "model"))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("%s reply data was missing 'model'"),
cmd_name);
+ return -1;
+ }
+
+ if (!(*cpu_name = virJSONValueObjectGetString(*cpu_model, "name"))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("%s reply data was missing 'name'"),
cmd_name);
+ return -1;
+ }
+
+ /* Some older CPU models do not report properties */
+ *cpu_props = virJSONValueObjectGetObject(*cpu_model, "props");
+
+ 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) {
+ size_t nprops = virJSONValueObjectKeysNumber(cpu_props);
+
+ if (VIR_ALLOC_N(machine_model->props, nprops) < 0)
+ goto cleanup;
+
+ if (virJSONValueObjectForeachKeyValue(cpu_props,
+ qemuMonitorJSONParseCPUModelProperty,
+ machine_model) < 0)
+ goto cleanup;
+ }
+
+ VIR_STEAL_PTR(*model_info, machine_model);
+ ret = 0;
+
+ cleanup:
+ qemuMonitorCPUModelInfoFree(machine_model);
+ return ret;
+}
+
+
int
qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon,
qemuMonitorCPUModelExpansionType type,
@@ -5624,33 +5738,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:
@@ -5686,11 +5787,9 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon,
data = virJSONValueObjectGetObject(reply, "return");
- if (!(cpu_model = virJSONValueObjectGetObject(data, "model"))) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("query-cpu-model-expansion reply data was missing
'model'"));
+ if (qemuMonitorJSONParseCPUModelData(data, &cpu_model, &cpu_props,
&cpu_name,
+ "query-cpu-model-expansion") < 0)
goto cleanup;
- }
/* QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC_FULL requests "full"
expansion
* on the result of the initial "static" expansion.
@@ -5705,42 +5804,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;
}
--
Mit freundlichen Grüßen/Kind regards
Boris Fiuczynski
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Matthias Hartmann
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
8:57 a.m.
New subject: [libvirt] [PATCH v4 1/8] qemu_monitor: helper functions for CPU models
On Wed, Jul 17, 2019 at 10:03:22 -0400, Collin Walling wrote:
Refactor some code in qemuMonitorJSONGetCPUModelExpansion to be
later used for the comparison and baseline functions.
You're mixing refactoring with adding new functionality. Splitting such
changes into separate patches is better. Moving some parts of existing
functions into new standalone functions is easier to review if the new
code matches the old one. It's not a big deal in this case, but since
you'll need to respin the series for other issues, you can also split
this patch.
This does not alter any functionality.
Signed-off-by: Collin Walling <walling(a)linux.ibm.com>
Reviewed-by: Bjoern Walk <bwalk(a)linux.ibm.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413(a)gmail.com>
---
src/qemu/qemu_monitor_json.c | 173 ++++++++++++++++++++++++++++++-------------
1 file changed, 121 insertions(+), 52 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 8723ff4..f6bf7f2 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -5616,6 +5616,120 @@ qemuMonitorJSONParseCPUModelProperty(const char *key,
return 0;
}
+
+static virJSONValuePtr
+qemuMonitorJSONMakeCPUModel(const char *model_name,
+ size_t nprops,
+ virCPUFeatureDefPtr props,
Adding virCPUFeatureDefPtr parameter to this function should go into a
separate patch. And for consistency, I believe we tend to pass the
pointer first followed by number of items.
+ 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 = false;
+
+ /* policy may be reported as -1 if the CPU def is a host model */
+ if (props[i].policy == VIR_CPU_FEATURE_REQUIRE ||
+ props[i].policy == VIR_CPU_FEATURE_FORCE ||
+ props[i].policy == -1)
+ enabled = true;
The naming is quite confusing. The virCPUDef structure contains an array
of features and you call it "props" while QEMU calls them "props" in
the
QMP protocol and your variable containing them is called "feats".
+
+ if (virJSONValueObjectAppendBoolean(feats, name, enabled) < 0)
+ goto cleanup;
+ }
+
+ if (!migratable &&
+ virJSONValueObjectAppendBoolean(feats, "migratable", false) <
0) {
+ goto cleanup;
+ }
+
+ if (virJSONValueObjectAppend(value, "props", feats) < 0)
+ goto cleanup;
+ }
+
+ return value;
+
+ cleanup:
"cleanup" label is used if the code following it is shared between both
success and failure paths. Since it's not the case here, you should call
it "error".
+ virJSONValueFree(value);
+ virJSONValueFree(feats);
+ return NULL;
+}
+
+
+static int
+qemuMonitorJSONParseCPUModelData(virJSONValuePtr data,
+ virJSONValuePtr *cpu_model,
cpu_model is not used anywhere outside this function, it's just a
temporary variable to store the container object for "model" and
"props".
+ virJSONValuePtr *cpu_props,
+ const char **cpu_name,
+ const char *cmd_name)
I'd move the cmd_name parameter to the second place after data so the
input parameters go first, followed by output parameters.
+{
+ if (!(*cpu_model = virJSONValueObjectGetObject(data, "model"))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("%s reply data was missing 'model'"),
cmd_name);
+ return -1;
+ }
+
+ if (!(*cpu_name = virJSONValueObjectGetString(*cpu_model, "name"))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("%s reply data was missing 'name'"),
cmd_name);
+ return -1;
+ }
+
+ /* Some older CPU models do not report properties */
+ *cpu_props = virJSONValueObjectGetObject(*cpu_model, "props");
This change should go into a separate patch with an explanation. Also,
it only applies to s390. Missing "props" should still result in an error
on x86. I think the callers should decide whether this is fatal or not.
+
+ 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) {
This is connected to making "props" optional.
+ size_t nprops = virJSONValueObjectKeysNumber(cpu_props);
+
+ if (VIR_ALLOC_N(machine_model->props, nprops) < 0)
+ goto cleanup;
+
+ if (virJSONValueObjectForeachKeyValue(cpu_props,
+ qemuMonitorJSONParseCPUModelProperty,
+ machine_model) < 0)
+ goto cleanup;
+ }
+
+ VIR_STEAL_PTR(*model_info, machine_model);
+ ret = 0;
+
+ cleanup:
+ qemuMonitorCPUModelInfoFree(machine_model);
+ return ret;
+}
+
+
int
qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon,
qemuMonitorCPUModelExpansionType type,
...
Jirka
9:03 a.m.
New subject: [libvirt] [PATCH v4 2/8] qemu_monitor: implement query-cpu-model-baseline
Interfaces with QEMU to baseline CPU models. The command takes two
CPU models, A and B, that are given a model name and an optional list
of CPU features. Through the query-cpu-model-baseline command issued
via QMP, a result is produced that contains a new baselined CPU model
that is guaranteed to run on both A and B.
Signed-off-by: Collin Walling <walling(a)linux.ibm.com>
Reviewed-by: Daniel Henrique Barboza <danielh413(a)gmail.com>
---
src/qemu/qemu_monitor.c | 22 ++++++++++++++++
src/qemu/qemu_monitor.h | 9 +++++++
src/qemu/qemu_monitor_json.c | 61 ++++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_monitor_json.h | 10 ++++++++
4 files changed, 102 insertions(+)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 731be2e..136d3e2 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -3648,6 +3648,28 @@ qemuMonitorGetCPUModelExpansion(qemuMonitorPtr mon,
}
+int
+qemuMonitorGetCPUModelBaseline(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 qemuMonitorJSONGetCPUModelBaseline(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 c41428b..4ec74f1 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -1084,6 +1084,15 @@ int qemuMonitorGetCPUModelExpansion(qemuMonitorPtr mon,
void qemuMonitorCPUModelInfoFree(qemuMonitorCPUModelInfoPtr model_info);
+int qemuMonitorGetCPUModelBaseline(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 f6bf7f2..b599ee6 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -5814,6 +5814,67 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon,
}
+int
+qemuMonitorJSONGetCPUModelBaseline(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;
+ virJSONValuePtr cpu_model;
+ virJSONValuePtr cpu_props = NULL;
+ const char *cpu_name = "";
+ qemuMonitorCPUModelInfoPtr baseline = 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-baseline",
+ "a:modela", &model_a,
+ "a:modelb", &model_b,
+ NULL)))
+ goto cleanup;
+
+ if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
+ goto cleanup;
+
+ if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_OBJECT) < 0)
+ goto cleanup;
+
+ data = virJSONValueObjectGetObject(reply, "return");
+
+ if (qemuMonitorJSONParseCPUModelData(data, &cpu_model, &cpu_props,
&cpu_name,
+ "query-cpu-model-baseline") < 0)
+ goto cleanup;
+
+ if (qemuMonitorJSONParseCPUModel(cpu_name, cpu_props, &baseline) < 0)
+ goto cleanup;
+
+ VIR_STEAL_PTR(*model_result, baseline);
+ ret = 0;
+
+ cleanup:
+ qemuMonitorCPUModelInfoFree(baseline);
+ 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 d0b519c..d3ee2fe 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -368,6 +368,16 @@ int qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon,
qemuMonitorCPUModelInfoPtr *model_info)
ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(5);
+int qemuMonitorJSONGetCPUModelBaseline(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.7.4
11:18 a.m.
New subject: [libvirt] [PATCH v4 2/8] qemu_monitor: implement query-cpu-model-baseline
On 7/17/19 4:03 PM, Collin Walling wrote:
Interfaces with QEMU to baseline CPU models. The command takes two
CPU models, A and B, that are given a model name and an optional list
of CPU features. Through the query-cpu-model-baseline command issued
via QMP, a result is produced that contains a new baselined CPU model
that is guaranteed to run on both A and B.
Signed-off-by: Collin Walling <walling(a)linux.ibm.com>
Reviewed-by: Daniel Henrique Barboza <danielh413(a)gmail.com>
---
src/qemu/qemu_monitor.c | 22 ++++++++++++++++
src/qemu/qemu_monitor.h | 9 +++++++
src/qemu/qemu_monitor_json.c | 61 ++++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_monitor_json.h | 10 ++++++++
4 files changed, 102 insertions(+)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 731be2e..136d3e2 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -3648,6 +3648,28 @@ qemuMonitorGetCPUModelExpansion(qemuMonitorPtr mon,
}
+int
+qemuMonitorGetCPUModelBaseline(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 qemuMonitorJSONGetCPUModelBaseline(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 c41428b..4ec74f1 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -1084,6 +1084,15 @@ int qemuMonitorGetCPUModelExpansion(qemuMonitorPtr mon,
void qemuMonitorCPUModelInfoFree(qemuMonitorCPUModelInfoPtr model_info);
+int qemuMonitorGetCPUModelBaseline(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 f6bf7f2..b599ee6 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -5814,6 +5814,67 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon,
}
+int
+qemuMonitorJSONGetCPUModelBaseline(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;
+ virJSONValuePtr cpu_model;
+ virJSONValuePtr cpu_props = NULL;
+ const char *cpu_name = "";
+ qemuMonitorCPUModelInfoPtr baseline = 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-baseline",
+ "a:modela", &model_a,
+ "a:modelb", &model_b,
+ NULL)))
+ goto cleanup;
+
+ if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
+ goto cleanup;
+
+ if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_OBJECT) < 0)
+ goto cleanup;
Did you test scenarios which contain unknown cpu model name or unknown
features? I have not yet tested this but please see my reply on patch 5.
+
+ data = virJSONValueObjectGetObject(reply, "return");
+
+ if (qemuMonitorJSONParseCPUModelData(data, &cpu_model, &cpu_props,
&cpu_name,
+ "query-cpu-model-baseline") < 0)
+ goto cleanup;
+
+ if (qemuMonitorJSONParseCPUModel(cpu_name, cpu_props, &baseline) < 0)
+ goto cleanup;
+
+ VIR_STEAL_PTR(*model_result, baseline);
+ ret = 0;
+
+ cleanup:
+ qemuMonitorCPUModelInfoFree(baseline);
Wouldn't
ret = qemuMonitorJSONParseCPUModel(cpu_name, cpu_props, model_result);
cleanup:
virJSONValueFree(cmd);
virJSONValueFree(reply);
virJSONValueFree(model_a);
virJSONValueFree(model_b);
return ret;
just do the trick as well since qemuMonitorJSONParseCPUModel already
handles the model_info allocation?
+ 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 d0b519c..d3ee2fe 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -368,6 +368,16 @@ int qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon,
qemuMonitorCPUModelInfoPtr *model_info)
ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(5);
+int qemuMonitorJSONGetCPUModelBaseline(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);
--
Mit freundlichen Grüßen/Kind regards
Boris Fiuczynski
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Matthias Hartmann
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
8:58 a.m.
New subject: [libvirt] [PATCH v4 2/8] qemu_monitor: implement query-cpu-model-baseline
On Wed, Jul 17, 2019 at 10:03:23 -0400, Collin Walling wrote:
Interfaces with QEMU to baseline CPU models. The command takes two
CPU models, A and B, that are given a model name and an optional list
of CPU features. Through the query-cpu-model-baseline command issued
via QMP, a result is produced that contains a new baselined CPU model
that is guaranteed to run on both A and B.
Signed-off-by: Collin Walling <walling(a)linux.ibm.com>
Reviewed-by: Daniel Henrique Barboza <danielh413(a)gmail.com>
---
src/qemu/qemu_monitor.c | 22 ++++++++++++++++
src/qemu/qemu_monitor.h | 9 +++++++
src/qemu/qemu_monitor_json.c | 61 ++++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_monitor_json.h | 10 ++++++++
4 files changed, 102 insertions(+)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 731be2e..136d3e2 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -3648,6 +3648,28 @@ qemuMonitorGetCPUModelExpansion(qemuMonitorPtr mon,
}
+int
+qemuMonitorGetCPUModelBaseline(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,
This is a mess. Model name, number of features, and the array of
features are all included in a virCPUDef structure. Are you going to
call this function from places where you don't actually have virCPUDef?
Passing just two virCPUDefPtr cpu1, and virCPUDefPtr cpu2 parameters
would be much nicer.
+ 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 qemuMonitorJSONGetCPUModelBaseline(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 c41428b..4ec74f1 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -1084,6 +1084,15 @@ int qemuMonitorGetCPUModelExpansion(qemuMonitorPtr mon,
void qemuMonitorCPUModelInfoFree(qemuMonitorCPUModelInfoPtr model_info);
+int qemuMonitorGetCPUModelBaseline(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 f6bf7f2..b599ee6 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -5814,6 +5814,67 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon,
}
+int
+qemuMonitorJSONGetCPUModelBaseline(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;
+ virJSONValuePtr cpu_model;
+ virJSONValuePtr cpu_props = NULL;
+ const char *cpu_name = "";
+ qemuMonitorCPUModelInfoPtr baseline = 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-baseline",
+ "a:modela", &model_a,
+ "a:modelb", &model_b,
+ NULL)))
+ goto cleanup;
+
+ if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
+ goto cleanup;
+
+ if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_OBJECT) < 0)
+ goto cleanup;
+
+ data = virJSONValueObjectGetObject(reply, "return");
+
+ if (qemuMonitorJSONParseCPUModelData(data, &cpu_model, &cpu_props,
&cpu_name,
+ "query-cpu-model-baseline") < 0)
+ goto cleanup;
+
+ if (qemuMonitorJSONParseCPUModel(cpu_name, cpu_props, &baseline) < 0)
+ goto cleanup;
+
+ VIR_STEAL_PTR(*model_result, baseline);
+ ret = 0;
As already mentioned by Boris, you can return baseline directly and you
don't need to worry about freeing it.
+
+ cleanup:
+ qemuMonitorCPUModelInfoFree(baseline);
+ virJSONValueFree(cmd);
+ virJSONValueFree(reply);
+ virJSONValueFree(model_a);
+ virJSONValueFree(model_b);
You can declare all four variables as
VIR_AUTOPTR(virJSONValue) cmd = NULL;
and then you can drop this cleanup section completely by replacing "goto
cleanup" with "return -1".
+ return ret;
+}
+
+
int qemuMonitorJSONGetCommands(qemuMonitorPtr mon,
char ***commands)
{
...
Jirka
9:03 a.m.
New subject: [libvirt] [PATCH v4 3/8] qemu_capabilities: introduce QEMU_CAPS_QUERY_CPU_MODEL_BASELINE
This capability enables CPU model baselining between two CPU
definitions via QMP.
Signed-off-by: Collin Walling <walling(a)linux.ibm.com>
Reviewed-by: Daniel Henrique Barboza <danielh413(a)gmail.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 +
tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml | 1 +
9 files changed, 10 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 6519246..b898113 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -536,6 +536,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
/* 335 */
"bochs-display",
+ "query-cpu-model-baseline",
);
@@ -981,6 +982,7 @@ struct virQEMUCapsStringFlags virQEMUCapsCommands[] = {
{ "blockdev-del", QEMU_CAPS_BLOCKDEV_DEL },
{ "query-current-machine", QEMU_CAPS_QUERY_CURRENT_MACHINE },
{ "block-dirty-bitmap-merge", QEMU_CAPS_BITMAP_MERGE },
+ { "query-cpu-model-baseline", QEMU_CAPS_QUERY_CPU_MODEL_BASELINE },
};
struct virQEMUCapsStringFlags virQEMUCapsMigration[] = {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 3cb56e6..8dde759 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -517,6 +517,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check
*/
/* 335 */
QEMU_CAPS_DEVICE_BOCHS_DISPLAY, /* -device bochs-display */
+ QEMU_CAPS_QUERY_CPU_MODEL_BASELINE, /* qmp query-cpu-model-baseline */
QEMU_CAPS_LAST /* this must always be the last item */
} virQEMUCapsFlags;
diff --git a/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml
b/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml
index 21b918f..e6f7e28 100644
--- a/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml
@@ -103,6 +103,7 @@
<flag name='egl-headless'/>
<flag name='zpci'/>
<flag name='iothread.poll-max-ns'/>
+ <flag name='query-cpu-model-baseline'/>
<version>2010000</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>39100805</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml
b/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml
index 6cb997d..48d7742 100644
--- a/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml
@@ -110,6 +110,7 @@
<flag name='egl-headless'/>
<flag name='zpci'/>
<flag name='iothread.poll-max-ns'/>
+ <flag name='query-cpu-model-baseline'/>
<version>2011000</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>39100806</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml
b/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml
index 2930381..381abd5 100644
--- a/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml
@@ -121,6 +121,7 @@
<flag name='memory-backend-memfd.hugetlb'/>
<flag name='iothread.poll-max-ns'/>
<flag name='memory-backend-file.align'/>
+ <flag name='query-cpu-model-baseline'/>
<version>2012000</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>39100807</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml
b/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml
index 5a2b740..9516a3b 100644
--- a/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml
@@ -92,6 +92,7 @@
<flag name='sdl-gl'/>
<flag name='vhost-vsock'/>
<flag name='zpci'/>
+ <flag name='query-cpu-model-baseline'/>
<version>2007093</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>39100764</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml
b/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml
index 72ae100..cf8bbb2 100644
--- a/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml
@@ -97,6 +97,7 @@
<flag name='vhost-vsock'/>
<flag name='zpci'/>
<flag name='iothread.poll-max-ns'/>
+ <flag name='query-cpu-model-baseline'/>
<version>2009000</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>39100765</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_3.0.0.s390x.xml
b/tests/qemucapabilitiesdata/caps_3.0.0.s390x.xml
index d511377..40b4a2b 100644
--- a/tests/qemucapabilitiesdata/caps_3.0.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_3.0.0.s390x.xml
@@ -123,6 +123,7 @@
<flag name='memory-backend-memfd.hugetlb'/>
<flag name='iothread.poll-max-ns'/>
<flag name='memory-backend-file.align'/>
+ <flag name='query-cpu-model-baseline'/>
<version>3000000</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>39100757</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml
b/tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml
index a1ac258..e72f447 100644
--- a/tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml
@@ -131,6 +131,7 @@
<flag name='query-current-machine'/>
<flag name='bitmap-merge'/>
<flag name='nbd-bitmap'/>
+ <flag name='query-cpu-model-baseline'/>
<version>4000000</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>39100758</microcodeVersion>
--
2.7.4
11:18 a.m.
New subject: [libvirt] [PATCH v4 3/8] qemu_capabilities: introduce QEMU_CAPS_QUERY_CPU_MODEL_BASELINE
Reviewed-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com>
On 7/17/19 4:03 PM, Collin Walling wrote:
This capability enables CPU model baselining between two CPU
definitions via QMP.
Signed-off-by: Collin Walling <walling(a)linux.ibm.com>
Reviewed-by: Daniel Henrique Barboza <danielh413(a)gmail.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 +
tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml | 1 +
9 files changed, 10 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 6519246..b898113 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -536,6 +536,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
/* 335 */
"bochs-display",
+ "query-cpu-model-baseline",
);
@@ -981,6 +982,7 @@ struct virQEMUCapsStringFlags virQEMUCapsCommands[] = {
{ "blockdev-del", QEMU_CAPS_BLOCKDEV_DEL },
{ "query-current-machine", QEMU_CAPS_QUERY_CURRENT_MACHINE },
{ "block-dirty-bitmap-merge", QEMU_CAPS_BITMAP_MERGE },
+ { "query-cpu-model-baseline", QEMU_CAPS_QUERY_CPU_MODEL_BASELINE },
};
struct virQEMUCapsStringFlags virQEMUCapsMigration[] = {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 3cb56e6..8dde759 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -517,6 +517,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check
*/
/* 335 */
QEMU_CAPS_DEVICE_BOCHS_DISPLAY, /* -device bochs-display */
+ QEMU_CAPS_QUERY_CPU_MODEL_BASELINE, /* qmp query-cpu-model-baseline */
QEMU_CAPS_LAST /* this must always be the last item */
} virQEMUCapsFlags;
diff --git a/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml
b/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml
index 21b918f..e6f7e28 100644
--- a/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml
@@ -103,6 +103,7 @@
<flag name='egl-headless'/>
<flag name='zpci'/>
<flag name='iothread.poll-max-ns'/>
+ <flag name='query-cpu-model-baseline'/>
<version>2010000</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>39100805</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml
b/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml
index 6cb997d..48d7742 100644
--- a/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml
@@ -110,6 +110,7 @@
<flag name='egl-headless'/>
<flag name='zpci'/>
<flag name='iothread.poll-max-ns'/>
+ <flag name='query-cpu-model-baseline'/>
<version>2011000</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>39100806</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml
b/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml
index 2930381..381abd5 100644
--- a/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml
@@ -121,6 +121,7 @@
<flag name='memory-backend-memfd.hugetlb'/>
<flag name='iothread.poll-max-ns'/>
<flag name='memory-backend-file.align'/>
+ <flag name='query-cpu-model-baseline'/>
<version>2012000</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>39100807</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml
b/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml
index 5a2b740..9516a3b 100644
--- a/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml
@@ -92,6 +92,7 @@
<flag name='sdl-gl'/>
<flag name='vhost-vsock'/>
<flag name='zpci'/>
+ <flag name='query-cpu-model-baseline'/>
<version>2007093</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>39100764</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml
b/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml
index 72ae100..cf8bbb2 100644
--- a/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml
@@ -97,6 +97,7 @@
<flag name='vhost-vsock'/>
<flag name='zpci'/>
<flag name='iothread.poll-max-ns'/>
+ <flag name='query-cpu-model-baseline'/>
<version>2009000</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>39100765</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_3.0.0.s390x.xml
b/tests/qemucapabilitiesdata/caps_3.0.0.s390x.xml
index d511377..40b4a2b 100644
--- a/tests/qemucapabilitiesdata/caps_3.0.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_3.0.0.s390x.xml
@@ -123,6 +123,7 @@
<flag name='memory-backend-memfd.hugetlb'/>
<flag name='iothread.poll-max-ns'/>
<flag name='memory-backend-file.align'/>
+ <flag name='query-cpu-model-baseline'/>
<version>3000000</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>39100757</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml
b/tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml
index a1ac258..e72f447 100644
--- a/tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml
@@ -131,6 +131,7 @@
<flag name='query-current-machine'/>
<flag name='bitmap-merge'/>
<flag name='nbd-bitmap'/>
+ <flag name='query-cpu-model-baseline'/>
<version>4000000</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>39100758</microcodeVersion>
--
Mit freundlichen Grüßen/Kind regards
Boris Fiuczynski
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Matthias Hartmann
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
8:58 a.m.
New subject: [libvirt] [PATCH v4 3/8] qemu_capabilities: introduce QEMU_CAPS_QUERY_CPU_MODEL_BASELINE
On Wed, Jul 17, 2019 at 10:03:24 -0400, Collin Walling wrote:
This capability enables CPU model baselining between two CPU
definitions via QMP.
Signed-off-by: Collin Walling <walling(a)linux.ibm.com>
Reviewed-by: Daniel Henrique Barboza <danielh413(a)gmail.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 +
tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml | 1 +
9 files changed, 10 insertions(+)
Reviewed-by: Jiri Denemark <jdenemar(a)redhat.com>
9:03 a.m.
New subject: [libvirt] [PATCH v4 4/8] qemu_driver: hook up query-cpu-model-baseline
This command is hooked into the virsh hypervisor-cpu-baseline command.
As such, the CPU models provided in the XML sent to the command will be
baselined with the CPU contained in the QEMU capabilities file for the
appropriate QEMU binary (for s390x, this CPU definition can be observed
via virsh domcapabilities).
Signed-off-by: Collin Walling <walling(a)linux.ibm.com>
Reviewed-by: Daniel Henrique Barboza <danielh413(a)gmail.com>
---
src/qemu/qemu_capabilities.c | 84 ++++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_capabilities.h | 7 ++++
src/qemu/qemu_driver.c | 27 ++++++++++++++
3 files changed, 118 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index b898113..ab337fa 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -5574,3 +5574,87 @@ virQEMUCapsStripMachineAliases(virQEMUCapsPtr qemuCaps)
for (i = 0; i < qemuCaps->nmachineTypes; i++)
VIR_FREE(qemuCaps->machineTypes[i].alias);
}
+
+
+static int
+virQEMUCapsStealCPUModelFromInfo(virCPUDefPtr dst,
+ qemuMonitorCPUModelInfoPtr *src)
+{
+ qemuMonitorCPUModelInfoPtr info = *src;
+ size_t i;
+
+ virCPUDefFreeModel(dst);
+
+ VIR_STEAL_PTR(dst->model, info->name);
+
+ for (i = 0; i < info->nprops; i++) {
+ char *name = info->props[i].name;
+ int policy = VIR_CPU_FEATURE_REQUIRE;
+
+ if (info->props[i].type == QEMU_MONITOR_CPU_PROPERTY_BOOLEAN &&
+ !info->props[i].value.boolean)
+ policy = VIR_CPU_FEATURE_DISABLE;
+
+ if (virCPUDefAddFeature(dst, name, policy) < 0)
+ goto error;
+ }
+
+ qemuMonitorCPUModelInfoFree(info);
+ *src = NULL;
+ return 0;
+
+ error:
+ virCPUDefFree(dst);
+ return -1;
+}
+
+
+virCPUDefPtr
+virQEMUCapsCPUModelBaseline(virQEMUCapsPtr qemuCaps,
+ const char *libDir,
+ uid_t runUid,
+ gid_t runGid,
+ int ncpus,
+ virCPUDefPtr *cpus)
+{
+ qemuMonitorCPUModelInfoPtr result = NULL;
+ qemuProcessQMPPtr proc = NULL;
+ virCPUDefPtr cpu = NULL;
+ virCPUDefPtr baseline = NULL;
+ size_t i;
+
+ if (VIR_ALLOC(cpu) < 0)
+ goto cleanup;
+
+ if (virCPUDefCopyModel(cpu, cpus[0], false))
+ goto cleanup;
+
+ if (!(proc = qemuProcessQMPNew(qemuCaps->binary, libDir,
+ runUid, runGid, false)))
+ goto cleanup;
+
+ if (qemuProcessQMPStart(proc) < 0)
+ goto cleanup;
+
+ for (i = 1; i < ncpus; i++) {
+ if (qemuMonitorGetCPUModelBaseline(proc->mon, cpu->model,
+ cpu->nfeatures, cpu->features,
+ cpus[i]->model, cpus[i]->nfeatures,
+ cpus[i]->features, &result) < 0)
+ goto cleanup;
+
+ if (virQEMUCapsStealCPUModelFromInfo(cpu, &result) < 0)
+ goto cleanup;
+ }
+
+ VIR_STEAL_PTR(baseline, cpu);
+
+ cleanup:
+ if (!baseline)
+ virQEMUCapsLogProbeFailure(qemuCaps->binary);
+
+ qemuMonitorCPUModelInfoFree(result);
+ qemuProcessQMPFree(proc);
+ virCPUDefFree(cpu);
+ return baseline;
+}
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 8dde759..b49c0b9 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -665,3 +665,10 @@ virQEMUCapsGetSEVCapabilities(virQEMUCapsPtr qemuCaps);
virArch virQEMUCapsArchFromString(const char *arch);
const char *virQEMUCapsArchToString(virArch arch);
+
+virCPUDefPtr virQEMUCapsCPUModelBaseline(virQEMUCapsPtr qemuCaps,
+ const char *libDir,
+ uid_t runUid,
+ gid_t runGid,
+ int ncpus,
+ virCPUDefPtr *cpus);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d3a144a..37b9c75 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -13553,6 +13553,7 @@ qemuConnectBaselineHypervisorCPU(virConnectPtr conn,
unsigned int flags)
{
virQEMUDriverPtr driver = conn->privateData;
+ virQEMUDriverConfigPtr config = driver->config;
virCPUDefPtr *cpus = NULL;
virQEMUCapsPtr qemuCaps = NULL;
virArch arch;
@@ -13607,6 +13608,32 @@ qemuConnectBaselineHypervisorCPU(virConnectPtr conn,
if (!(cpu = virCPUBaseline(arch, cpus, ncpus, cpuModels,
(const char **)features, migratable)))
goto cleanup;
+ } else if (ARCH_IS_S390(arch) &&
+ virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_CPU_MODEL_BASELINE)) {
+ /* Add a copy of the hypervisor CPU to the list */
+ virCPUDefPtr hvCPU, tmp;
+ size_t allocated = ncpus + 1;
+
+ hvCPU = virQEMUCapsGetHostModel(qemuCaps, virttype,
+ VIR_QEMU_CAPS_HOST_CPU_REPORTED);
+ if (VIR_ALLOC(tmp) < 0)
+ goto cleanup;
+
+ if (virCPUDefCopyModel(tmp, hvCPU, false))
+ goto cleanup;
+
+ if (VIR_INSERT_ELEMENT(cpus, ncpus, allocated, tmp) < 0)
+ goto cleanup;
+
+ ncpus++;
+
+ if (!(cpu = virQEMUCapsCPUModelBaseline(qemuCaps, config->libDir,
+ config->user, config->group,
+ ncpus, cpus)))
+ goto cleanup;
+
+ if (!(flags & VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES))
+ virCPUDefFreeFeatures(cpu);
} else {
virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
_("computing baseline hypervisor CPU is not supported "
--
2.7.4
11:18 a.m.
New subject: [libvirt] [PATCH v4 4/8] qemu_driver: hook up query-cpu-model-baseline
Besides the comment below
Reviewed-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com>
On 7/17/19 4:03 PM, Collin Walling wrote:
This command is hooked into the virsh hypervisor-cpu-baseline
command.
As such, the CPU models provided in the XML sent to the command will be
baselined with the CPU contained in the QEMU capabilities file for the
appropriate QEMU binary (for s390x, this CPU definition can be observed
via virsh domcapabilities).
Signed-off-by: Collin Walling <walling(a)linux.ibm.com>
Reviewed-by: Daniel Henrique Barboza <danielh413(a)gmail.com>
---
src/qemu/qemu_capabilities.c | 84 ++++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_capabilities.h | 7 ++++
src/qemu/qemu_driver.c | 27 ++++++++++++++
3 files changed, 118 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index b898113..ab337fa 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -5574,3 +5574,87 @@ virQEMUCapsStripMachineAliases(virQEMUCapsPtr qemuCaps)
for (i = 0; i < qemuCaps->nmachineTypes; i++)
VIR_FREE(qemuCaps->machineTypes[i].alias);
}
+
+
+static int
+virQEMUCapsStealCPUModelFromInfo(virCPUDefPtr dst,
+ qemuMonitorCPUModelInfoPtr *src)
+{
+ qemuMonitorCPUModelInfoPtr info = *src;
+ size_t i;
+
+ virCPUDefFreeModel(dst);
+
+ VIR_STEAL_PTR(dst->model, info->name);
+
+ for (i = 0; i < info->nprops; i++) {
+ char *name = info->props[i].name;
+ int policy = VIR_CPU_FEATURE_REQUIRE;
+
+ if (info->props[i].type == QEMU_MONITOR_CPU_PROPERTY_BOOLEAN &&
+ !info->props[i].value.boolean)
+ policy = VIR_CPU_FEATURE_DISABLE;
+
+ if (virCPUDefAddFeature(dst, name, policy) < 0)
+ goto error;
+ }
+
+ qemuMonitorCPUModelInfoFree(info);
+ *src = NULL;
+ return 0;
+
+ error:
+ virCPUDefFree(dst);
+ return -1;
+}
+
+
+virCPUDefPtr
+virQEMUCapsCPUModelBaseline(virQEMUCapsPtr qemuCaps,
+ const char *libDir,
+ uid_t runUid,
+ gid_t runGid,
+ int ncpus,
+ virCPUDefPtr *cpus)
+{
+ qemuMonitorCPUModelInfoPtr result = NULL;
+ qemuProcessQMPPtr proc = NULL;
+ virCPUDefPtr cpu = NULL;
+ virCPUDefPtr baseline = NULL;
+ size_t i;
+
+ if (VIR_ALLOC(cpu) < 0)
+ goto cleanup;
+
+ if (virCPUDefCopyModel(cpu, cpus[0], false))
+ goto cleanup;
+
+ if (!(proc = qemuProcessQMPNew(qemuCaps->binary, libDir,
+ runUid, runGid, false)))
+ goto cleanup;
+
+ if (qemuProcessQMPStart(proc) < 0)
+ goto cleanup;
+
+ for (i = 1; i < ncpus; i++) {
+ if (qemuMonitorGetCPUModelBaseline(proc->mon, cpu->model,
+ cpu->nfeatures, cpu->features,
+ cpus[i]->model, cpus[i]->nfeatures,
+ cpus[i]->features, &result) < 0)
+ goto cleanup;
+
+ if (virQEMUCapsStealCPUModelFromInfo(cpu, &result) < 0)
+ goto cleanup;
+ }
+
+ VIR_STEAL_PTR(baseline, cpu);
+
+ cleanup:
+ if (!baseline)
+ virQEMUCapsLogProbeFailure(qemuCaps->binary);
+
+ qemuMonitorCPUModelInfoFree(result);
Is qemuMonitorCPUModelInfoFree really
necessary? Are
qemuMonitorGetCPUModelBaseline and virQEMUCapsStealCPUModelFromInfo not
taking care of the mem allocation on the error paths that result in the
goto cleanup?
+ qemuProcessQMPFree(proc);
+ virCPUDefFree(cpu);
+ return baseline;
+}
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 8dde759..b49c0b9 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -665,3 +665,10 @@ virQEMUCapsGetSEVCapabilities(virQEMUCapsPtr qemuCaps);
virArch virQEMUCapsArchFromString(const char *arch);
const char *virQEMUCapsArchToString(virArch arch);
+
+virCPUDefPtr virQEMUCapsCPUModelBaseline(virQEMUCapsPtr qemuCaps,
+ const char *libDir,
+ uid_t runUid,
+ gid_t runGid,
+ int ncpus,
+ virCPUDefPtr *cpus);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d3a144a..37b9c75 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -13553,6 +13553,7 @@ qemuConnectBaselineHypervisorCPU(virConnectPtr conn,
unsigned int flags)
{
virQEMUDriverPtr driver = conn->privateData;
+ virQEMUDriverConfigPtr config = driver->config;
virCPUDefPtr *cpus = NULL;
virQEMUCapsPtr qemuCaps = NULL;
virArch arch;
@@ -13607,6 +13608,32 @@ qemuConnectBaselineHypervisorCPU(virConnectPtr conn,
if (!(cpu = virCPUBaseline(arch, cpus, ncpus, cpuModels,
(const char **)features, migratable)))
goto cleanup;
+ } else if (ARCH_IS_S390(arch) &&
+ virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_CPU_MODEL_BASELINE)) {
+ /* Add a copy of the hypervisor CPU to the list */
+ virCPUDefPtr hvCPU, tmp;
+ size_t allocated = ncpus + 1;
+
+ hvCPU = virQEMUCapsGetHostModel(qemuCaps, virttype,
+ VIR_QEMU_CAPS_HOST_CPU_REPORTED);
+ if (VIR_ALLOC(tmp) < 0)
+ goto cleanup;
+
+ if (virCPUDefCopyModel(tmp, hvCPU, false))
+ goto cleanup;
+
+ if (VIR_INSERT_ELEMENT(cpus, ncpus, allocated, tmp) < 0)
+ goto cleanup;
+
+ ncpus++;
+
+ if (!(cpu = virQEMUCapsCPUModelBaseline(qemuCaps, config->libDir,
+ config->user, config->group,
+ ncpus, cpus)))
+ goto cleanup;
+
+ if (!(flags & VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES))
+ virCPUDefFreeFeatures(cpu);
} else {
virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
_("computing baseline hypervisor CPU is not supported
"
--
Mit freundlichen Grüßen/Kind regards
Boris Fiuczynski
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Matthias Hartmann
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
8:58 a.m.
New subject: [libvirt] [PATCH v4 4/8] qemu_driver: hook up query-cpu-model-baseline
On Wed, Jul 17, 2019 at 10:03:25 -0400, Collin Walling wrote:
This command is hooked into the virsh hypervisor-cpu-baseline
command.
As such, the CPU models provided in the XML sent to the command will be
baselined with the CPU contained in the QEMU capabilities file for the
appropriate QEMU binary (for s390x, this CPU definition can be observed
via virsh domcapabilities).
This is not how the baseline API works. If a user wants the current host
CPU model to be taken in the baseline computation, they need to provide
it in the array of input CPUs by themselves.
Signed-off-by: Collin Walling <walling(a)linux.ibm.com>
Reviewed-by: Daniel Henrique Barboza <danielh413(a)gmail.com>
---
src/qemu/qemu_capabilities.c | 84 ++++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_capabilities.h | 7 ++++
src/qemu/qemu_driver.c | 27 ++++++++++++++
3 files changed, 118 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index b898113..ab337fa 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -5574,3 +5574,87 @@ virQEMUCapsStripMachineAliases(virQEMUCapsPtr qemuCaps)
for (i = 0; i < qemuCaps->nmachineTypes; i++)
VIR_FREE(qemuCaps->machineTypes[i].alias);
}
+
+
+static int
+virQEMUCapsStealCPUModelFromInfo(virCPUDefPtr dst,
+ qemuMonitorCPUModelInfoPtr *src)
+{
+ qemuMonitorCPUModelInfoPtr info = *src;
+ size_t i;
+
+ virCPUDefFreeModel(dst);
+
+ VIR_STEAL_PTR(dst->model, info->name);
+
+ for (i = 0; i < info->nprops; i++) {
+ char *name = info->props[i].name;
+ int policy = VIR_CPU_FEATURE_REQUIRE;
+
+ if (info->props[i].type == QEMU_MONITOR_CPU_PROPERTY_BOOLEAN &&
+ !info->props[i].value.boolean)
+ policy = VIR_CPU_FEATURE_DISABLE;
+
+ if (virCPUDefAddFeature(dst, name, policy) < 0)
+ goto error;
+ }
+
+ qemuMonitorCPUModelInfoFree(info);
+ *src = NULL;
+ return 0;
+
+ error:
+ virCPUDefFree(dst);
+ return -1;
+}
+
+
+virCPUDefPtr
+virQEMUCapsCPUModelBaseline(virQEMUCapsPtr qemuCaps,
+ const char *libDir,
+ uid_t runUid,
+ gid_t runGid,
+ int ncpus,
+ virCPUDefPtr *cpus)
This function does not belong to qemu_capabilities.c, it should be in
qemu_driver.c and with a different prefix, of course. And this applies
to the helper above too.
+{
+ qemuMonitorCPUModelInfoPtr result = NULL;
+ qemuProcessQMPPtr proc = NULL;
+ virCPUDefPtr cpu = NULL;
+ virCPUDefPtr baseline = NULL;
+ size_t i;
+
+ if (VIR_ALLOC(cpu) < 0)
+ goto cleanup;
+
+ if (virCPUDefCopyModel(cpu, cpus[0], false))
+ goto cleanup;
+
+ if (!(proc = qemuProcessQMPNew(qemuCaps->binary, libDir,
+ runUid, runGid, false)))
+ goto cleanup;
+
+ if (qemuProcessQMPStart(proc) < 0)
+ goto cleanup;
+
+ for (i = 1; i < ncpus; i++) {
+ if (qemuMonitorGetCPUModelBaseline(proc->mon, cpu->model,
+ cpu->nfeatures, cpu->features,
+ cpus[i]->model, cpus[i]->nfeatures,
+ cpus[i]->features, &result) < 0)
See my comment to 2/8. Wouldn't this be much better as
if (qemuMonitorGetCPUModelBaseline(proc->mon, cpu, cpus[i],
&result) < 0)
+ goto cleanup;
+
+ if (virQEMUCapsStealCPUModelFromInfo(cpu, &result) < 0)
+ goto cleanup;
+ }
+
+ VIR_STEAL_PTR(baseline, cpu);
+
+ cleanup:
+ if (!baseline)
+ virQEMUCapsLogProbeFailure(qemuCaps->binary);
There's no probing here. It's just an implementation of the API invoked
by libvirt client. No need to log anything here.
+
+ qemuMonitorCPUModelInfoFree(result);
As Boris already noticed, result cannot be non-NULL here. In fact, you
could even make the result variable local to the for loop.
+ qemuProcessQMPFree(proc);
+ virCPUDefFree(cpu);
+ return baseline;
+}
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 8dde759..b49c0b9 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -665,3 +665,10 @@ virQEMUCapsGetSEVCapabilities(virQEMUCapsPtr qemuCaps);
virArch virQEMUCapsArchFromString(const char *arch);
const char *virQEMUCapsArchToString(virArch arch);
+
+virCPUDefPtr virQEMUCapsCPUModelBaseline(virQEMUCapsPtr qemuCaps,
+ const char *libDir,
+ uid_t runUid,
+ gid_t runGid,
+ int ncpus,
+ virCPUDefPtr *cpus);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d3a144a..37b9c75 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -13553,6 +13553,7 @@ qemuConnectBaselineHypervisorCPU(virConnectPtr conn,
unsigned int flags)
{
virQEMUDriverPtr driver = conn->privateData;
+ virQEMUDriverConfigPtr config = driver->config;
You should use
VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver);
instead.
virCPUDefPtr *cpus = NULL;
virQEMUCapsPtr qemuCaps = NULL;
virArch arch;
@@ -13607,6 +13608,32 @@ qemuConnectBaselineHypervisorCPU(virConnectPtr conn,
if (!(cpu = virCPUBaseline(arch, cpus, ncpus, cpuModels,
(const char **)features, migratable)))
goto cleanup;
+ } else if (ARCH_IS_S390(arch) &&
+ virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_CPU_MODEL_BASELINE)) {
+ /* Add a copy of the hypervisor CPU to the list */
+ virCPUDefPtr hvCPU, tmp;
+ size_t allocated = ncpus + 1;
+
+ hvCPU = virQEMUCapsGetHostModel(qemuCaps, virttype,
+ VIR_QEMU_CAPS_HOST_CPU_REPORTED);
+ if (VIR_ALLOC(tmp) < 0)
+ goto cleanup;
+
+ if (virCPUDefCopyModel(tmp, hvCPU, false))
+ goto cleanup;
+
+ if (VIR_INSERT_ELEMENT(cpus, ncpus, allocated, tmp) < 0)
+ goto cleanup;
+
+ ncpus++;
All the code in this if statement down to here should go away.
+
+ if (!(cpu = virQEMUCapsCPUModelBaseline(qemuCaps, config->libDir,
+ config->user, config->group,
+ ncpus, cpus)))
+ goto cleanup;
+
+ if (!(flags & VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES))
+ virCPUDefFreeFeatures(cpu);
This seems to be wrong.
} else {
virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
_("computing baseline hypervisor CPU is not supported "
Jirka
9:03 a.m.
New subject: [libvirt] [PATCH v4 5/8] qemu_monitor: implement query-cpu-model-comparison
Interfaces with QEMU to compare CPU models. The command takes two
CPU models, A and B, that are given a model name and an optional list
of CPU features. Through the query-cpu-model-comparison command issued
via QMP, a result is produced that contains the comparison evaluation
string (identical, superset, subset, incompatible) as well as a list
of properties (aka CPU features) responsible for the subset or
superset result.
Signed-off-by: Collin Walling <walling(a)linux.ibm.com>
Reviewed-by: Daniel Henrique Barboza <danielh413(a)gmail.com>
---
src/qemu/qemu_monitor.c | 22 ++++++++++
src/qemu/qemu_monitor.h | 9 +++++
src/qemu/qemu_monitor_json.c | 95 ++++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_monitor_json.h | 10 +++++
4 files changed, 136 insertions(+)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 136d3e2..b16e149 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -3670,6 +3670,28 @@ qemuMonitorGetCPUModelBaseline(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 4ec74f1..f0cf70a 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -1093,6 +1093,15 @@ int qemuMonitorGetCPUModelBaseline(qemuMonitorPtr mon,
virCPUFeatureDefPtr model_b_props,
qemuMonitorCPUModelInfoPtr *model_result);
+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 b599ee6..3c33c63 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -5875,6 +5875,101 @@ qemuMonitorJSONGetCPUModelBaseline(qemuMonitorPtr mon,
}
+static int
+qemuMonitorJSONParseCPUModelPropName(size_t pos ATTRIBUTE_UNUSED,
+ virJSONValuePtr item,
+ void *opaque)
+{
+ if (!qemuMonitorJSONParseCPUModelProperty(virJSONValueGetString(item),
+ item, opaque))
+ return -1;
+
+ virJSONValueFree(item);
+ return 0;
+}
+
+
+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 d3ee2fe..f7a8e99 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -378,6 +378,16 @@ int qemuMonitorJSONGetCPUModelBaseline(qemuMonitorPtr mon,
qemuMonitorCPUModelInfoPtr *model_result)
ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(5) ATTRIBUTE_NONNULL(8);
+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.7.4
11:18 a.m.
New subject: [libvirt] [PATCH v4 5/8] qemu_monitor: implement query-cpu-model-comparison
On 7/17/19 4:03 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, incompatible) as well as a list
of properties (aka CPU features) responsible for the subset or
superset result.
Signed-off-by: Collin Walling <walling(a)linux.ibm.com>
Reviewed-by: Daniel Henrique Barboza <danielh413(a)gmail.com>
---
src/qemu/qemu_monitor.c | 22 ++++++++++
src/qemu/qemu_monitor.h | 9 +++++
src/qemu/qemu_monitor_json.c | 95 ++++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_monitor_json.h | 10 +++++
4 files changed, 136 insertions(+)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 136d3e2..b16e149 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -3670,6 +3670,28 @@ qemuMonitorGetCPUModelBaseline(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 4ec74f1..f0cf70a 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -1093,6 +1093,15 @@ int qemuMonitorGetCPUModelBaseline(qemuMonitorPtr mon,
virCPUFeatureDefPtr model_b_props,
qemuMonitorCPUModelInfoPtr *model_result);
+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 b599ee6..3c33c63 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -5875,6 +5875,101 @@ qemuMonitorJSONGetCPUModelBaseline(qemuMonitorPtr mon,
}
+static int
+qemuMonitorJSONParseCPUModelPropName(size_t pos ATTRIBUTE_UNUSED,
+ virJSONValuePtr item,
+ void *opaque)
+{
+ if (!qemuMonitorJSONParseCPUModelProperty(virJSONValueGetString(item),
+ item, opaque))
This need to be changed into
if (qemuMonitorJSONParseCPUModelProperty(virJSONValueGetString(item),
item, opaque) < 0)
+ return -1;
+
+ virJSONValueFree(item);
+ return 0;
+}
+
+
+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)
Did you test scenarios which contain unknown cpu model name or unknown
features?
This caused errors for me during testing that resulted in e.g.
error : qemuMonitorJSONCheckError:415 : internal error: unable to
execute QEMU command 'query-cpu-model-comparison': Property '.boris' not
found
warning : virQEMUCapsLogProbeFailure:4586 : Failed to probe
capabilities for /usr/bin/qemu-system-s390x: internal error: unable to
execute QEMU command 'query-cpu-model-comparison': Property '.boris' not
found
If I think about a scenario like: I run "virsh domcapabilties" on a new
machine with new cpu model and new cpu features, new libvirt, new qemu
and use "virsh hypervisor-cpu-compare" with the output xml on my old
machine with old cpu &feature, older libvirt and older qemu
I would expect to get as an answer: Incompatible
If my expectation to get incompatible is wrong please correct me but an
"internal error" is not what should occur.
What is the qemu specification for this?
+ 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 d3ee2fe..f7a8e99 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -378,6 +378,16 @@ int qemuMonitorJSONGetCPUModelBaseline(qemuMonitorPtr mon,
qemuMonitorCPUModelInfoPtr *model_result)
ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(5) ATTRIBUTE_NONNULL(8);
+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);
--
Mit freundlichen Grüßen/Kind regards
Boris Fiuczynski
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Matthias Hartmann
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
1:26 p.m.
New subject: [libvirt] [PATCH v4 5/8] qemu_monitor: implement query-cpu-model-comparison
On 7/24/19 12:18 PM, Boris Fiuczynski wrote:
On 7/17/19 4:03 PM, Collin Walling wrote:
[...]
> diff --git a/src/qemu/qemu_monitor_json.c
b/src/qemu/qemu_monitor_json.c
> index b599ee6..3c33c63 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -5875,6 +5875,101 @@
> qemuMonitorJSONGetCPUModelBaseline(qemuMonitorPtr mon,
> }
> +static int
> +qemuMonitorJSONParseCPUModelPropName(size_t pos ATTRIBUTE_UNUSED,
> + virJSONValuePtr item,
> + void *opaque)
> +{
> + if
> (!qemuMonitorJSONParseCPUModelProperty(virJSONValueGetString(item),
> + item, opaque))
This need to be changed into
if (qemuMonitorJSONParseCPUModelProperty(virJSONValueGetString(item),
item, opaque) < 0)
Yes, thank you for catching this one.
> + return -1;
> +
> + virJSONValueFree(item);
> + return 0;
> +}
> +
> +
> +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)
Did you test scenarios which contain unknown cpu model name or unknown
features?
This caused errors for me during testing that resulted in e.g.
error : qemuMonitorJSONCheckError:415 : internal error: unable to
execute QEMU command 'query-cpu-model-comparison': Property '.boris' not
found
warning : virQEMUCapsLogProbeFailure:4586 : Failed to probe
capabilities for /usr/bin/qemu-system-s390x: internal error: unable to
execute QEMU command 'query-cpu-model-comparison': Property '.boris' not
found
If I think about a scenario like: I run "virsh domcapabilties" on a new
machine with new cpu model and new cpu features, new libvirt, new qemu
and use "virsh hypervisor-cpu-compare" with the output xml on my old
machine with old cpu &feature, older libvirt and older qemu
I would expect to get as an answer: Incompatible
If my expectation to get incompatible is wrong please correct me but an
"internal error" is not what should occur.
What is the qemu specification for this?
Two routes:
1) Fix up the logic in virQEMUCapsCPUModelComparison
Please see my comments on patch 8.
2) Change the error class in the QEMU QMP code
The QMP code in QEMU currently reports a "GenericError" whenever
something goes wrong (such as an unknown CPU model or feature). A
possible solution to this would be to alter that code to set a specific
error class, have libvirt check for that specific error type in the
JSON, then handle it appropriately to print a cleaner message.
There is one drawback: we would technically be reporting this error
twice -- once in virsh, and another in the libvirt daemon (the internal
error) unless we wanted to suppress it.
This is one solution without too much messy code.
> + 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;
> +}
> +
> +
[...]
4:59 a.m.
New subject: [libvirt] [PATCH v4 5/8] qemu_monitor: implement query-cpu-model-comparison
On 7/25/19 8:26 PM, Collin Walling wrote:
>> +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)
>
> Did you test scenarios which contain unknown cpu model name or unknown
> features?
> This caused errors for me during testing that resulted in e.g.
> error : qemuMonitorJSONCheckError:415 : internal error: unable to
> execute QEMU command 'query-cpu-model-comparison': Property '.boris'
> not found
> warning : virQEMUCapsLogProbeFailure:4586 : Failed to probe
> capabilities for /usr/bin/qemu-system-s390x: internal error: unable to
> execute QEMU command 'query-cpu-model-comparison': Property '.boris'
> not found
>
> If I think about a scenario like: I run "virsh domcapabilties" on a
> new machine with new cpu model and new cpu features, new libvirt, new
> qemu and use "virsh hypervisor-cpu-compare" with the output xml on my
> old machine with old cpu &feature, older libvirt and older qemu
> I would expect to get as an answer: Incompatible
>
> If my expectation to get incompatible is wrong please correct me but
> an "internal error" is not what should occur.
> What is the qemu specification for this?
>
>
Two routes:
1) Fix up the logic in virQEMUCapsCPUModelComparison
Please see my comments on patch 8.
2) Change the error class in the QEMU QMP code
The QMP code in QEMU currently reports a "GenericError" whenever
something goes wrong (such as an unknown CPU model or feature). A
possible solution to this would be to alter that code to set a specific
error class, have libvirt check for that specific error type in the
JSON, then handle it appropriately to print a cleaner message.
There is one drawback: we would technically be reporting this error
twice -- once in virsh, and another in the libvirt daemon (the internal
error) unless we wanted to suppress it.
This is one solution without too much messy code.
Or you prevent the generic error by not making the call at all in the
case QEMU does not know CPU model or features. To be able to do this you
either need lists of all known CPU models and features (preferable) or
you need to ask QEMU if it knows the CPU model and features by use of
other QMP commands that provide a consumable response.
I would not simply turn an QEMU internal error into an answer
"incompatible" and I am not a fan of parsing thru QEMU internal errors
that turns the answer into "incompatible".
Regarding patch 8: The option you outline in patch 8 returns "CPU is
incompatible" for every QEMU error and not only "CPU model or feature"
unknown. Therefore I think that it is not a good idea.
--
Mit freundlichen Grüßen/Kind regards
Boris Fiuczynski
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Matthias Hartmann
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
8:43 a.m.
New subject: [libvirt] [PATCH v4 5/8] qemu_monitor: implement query-cpu-model-comparison
On Wed, Jul 24, 2019 at 18:18:27 +0200, Boris Fiuczynski wrote:
On 7/17/19 4:03 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, incompatible) as well as a list
> of properties (aka CPU features) responsible for the subset or
> superset result.
>
> Signed-off-by: Collin Walling <walling(a)linux.ibm.com>
> Reviewed-by: Daniel Henrique Barboza <danielh413(a)gmail.com>
> ---
> src/qemu/qemu_monitor.c | 22 ++++++++++
> src/qemu/qemu_monitor.h | 9 +++++
> src/qemu/qemu_monitor_json.c | 95 ++++++++++++++++++++++++++++++++++++++++++++
> src/qemu/qemu_monitor_json.h | 10 +++++
> 4 files changed, 136 insertions(+)
>
> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
> index 136d3e2..b16e149 100644
> --- a/src/qemu/qemu_monitor.c
> +++ b/src/qemu/qemu_monitor.c
> @@ -3670,6 +3670,28 @@ qemuMonitorGetCPUModelBaseline(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 4ec74f1..f0cf70a 100644
> --- a/src/qemu/qemu_monitor.h
> +++ b/src/qemu/qemu_monitor.h
> @@ -1093,6 +1093,15 @@ int qemuMonitorGetCPUModelBaseline(qemuMonitorPtr mon,
> virCPUFeatureDefPtr model_b_props,
> qemuMonitorCPUModelInfoPtr *model_result);
>
> +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 b599ee6..3c33c63 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -5875,6 +5875,101 @@ qemuMonitorJSONGetCPUModelBaseline(qemuMonitorPtr mon,
> }
>
>
> +static int
> +qemuMonitorJSONParseCPUModelPropName(size_t pos ATTRIBUTE_UNUSED,
> + virJSONValuePtr item,
> + void *opaque)
> +{
> + if (!qemuMonitorJSONParseCPUModelProperty(virJSONValueGetString(item),
> + item, opaque))
This need to be changed into
if (qemuMonitorJSONParseCPUModelProperty(virJSONValueGetString(item),
item, opaque) < 0)
> + return -1;
> +
> + virJSONValueFree(item);
> + return 0;
> +}
> +
> +
> +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)
Did you test scenarios which contain unknown cpu model name or unknown
features?
This caused errors for me during testing that resulted in e.g.
error : qemuMonitorJSONCheckError:415 : internal error: unable to
execute QEMU command 'query-cpu-model-comparison': Property '.boris' not
found
warning : virQEMUCapsLogProbeFailure:4586 : Failed to probe
capabilities for /usr/bin/qemu-system-s390x: internal error: unable to
execute QEMU command 'query-cpu-model-comparison': Property '.boris' not
found
If I think about a scenario like: I run "virsh domcapabilties" on a new
machine with new cpu model and new cpu features, new libvirt, new qemu
and use "virsh hypervisor-cpu-compare" with the output xml on my old
machine with old cpu &feature, older libvirt and older qemu
I would expect to get as an answer: Incompatible
If my expectation to get incompatible is wrong please correct me but an
"internal error" is not what should occur.
What is the qemu specification for this?
Actually returning error in such case is the correct way and also
consistent with how it works for x86. The only difference is we report
these errors ourselves for x86 while s390 would report the error from
QEMU. The QEMU error is not exactly nice, but it is correct. So adding
any code which would allow libvirt detect these errors and report better
error messages would be a nice enhancement, but it's not really
necessary.
Jirka
9:13 a.m.
New subject: [libvirt] [PATCH v4 5/8] qemu_monitor: implement query-cpu-model-comparison
On Wed, Jul 17, 2019 at 10:03:26 -0400, 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, incompatible) as well as a list
of properties (aka CPU features) responsible for the subset or
superset result.
Signed-off-by: Collin Walling <walling(a)linux.ibm.com>
Reviewed-by: Daniel Henrique Barboza <danielh413(a)gmail.com>
---
src/qemu/qemu_monitor.c | 22 ++++++++++
src/qemu/qemu_monitor.h | 9 +++++
src/qemu/qemu_monitor_json.c | 95 ++++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_monitor_json.h | 10 +++++
4 files changed, 136 insertions(+)
Most of the comments from 2/8 are applicable here as well.
Jirka
9:03 a.m.
New subject: [libvirt] [PATCH v4 6/8] qemu_capabilities: introduce QEMU_CAPS_QUERY_CPU_MODEL_COMPARISON
This capability enables CPU model comparison between two CPU
definitions via QMP.
Signed-off-by: Collin Walling <walling(a)linux.ibm.com>
Reviewed-by: Daniel Henrique Barboza <danielh413(a)gmail.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 +
tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml | 1 +
9 files changed, 10 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index ab337fa..a99ebcb 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -537,6 +537,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
/* 335 */
"bochs-display",
"query-cpu-model-baseline",
+ "query-cpu-model-comparison",
);
@@ -983,6 +984,7 @@ struct virQEMUCapsStringFlags virQEMUCapsCommands[] = {
{ "query-current-machine", QEMU_CAPS_QUERY_CURRENT_MACHINE },
{ "block-dirty-bitmap-merge", QEMU_CAPS_BITMAP_MERGE },
{ "query-cpu-model-baseline", QEMU_CAPS_QUERY_CPU_MODEL_BASELINE },
+ { "query-cpu-model-comparison", QEMU_CAPS_QUERY_CPU_MODEL_COMPARISON },
};
struct virQEMUCapsStringFlags virQEMUCapsMigration[] = {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index b49c0b9..8afa05c 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -518,6 +518,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check
*/
/* 335 */
QEMU_CAPS_DEVICE_BOCHS_DISPLAY, /* -device bochs-display */
QEMU_CAPS_QUERY_CPU_MODEL_BASELINE, /* qmp query-cpu-model-baseline */
+ QEMU_CAPS_QUERY_CPU_MODEL_COMPARISON, /* qmp query-cpu-model-comparison */
QEMU_CAPS_LAST /* this must always be the last item */
} virQEMUCapsFlags;
diff --git a/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml
b/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml
index e6f7e28..058eb32 100644
--- a/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml
@@ -104,6 +104,7 @@
<flag name='zpci'/>
<flag name='iothread.poll-max-ns'/>
<flag name='query-cpu-model-baseline'/>
+ <flag name='query-cpu-model-comparison'/>
<version>2010000</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>39100805</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml
b/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml
index 48d7742..4256131 100644
--- a/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml
@@ -111,6 +111,7 @@
<flag name='zpci'/>
<flag name='iothread.poll-max-ns'/>
<flag name='query-cpu-model-baseline'/>
+ <flag name='query-cpu-model-comparison'/>
<version>2011000</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>39100806</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml
b/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml
index 381abd5..48be96d 100644
--- a/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml
@@ -122,6 +122,7 @@
<flag name='iothread.poll-max-ns'/>
<flag name='memory-backend-file.align'/>
<flag name='query-cpu-model-baseline'/>
+ <flag name='query-cpu-model-comparison'/>
<version>2012000</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>39100807</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml
b/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml
index 9516a3b..5b04416 100644
--- a/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml
@@ -93,6 +93,7 @@
<flag name='vhost-vsock'/>
<flag name='zpci'/>
<flag name='query-cpu-model-baseline'/>
+ <flag name='query-cpu-model-comparison'/>
<version>2007093</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>39100764</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml
b/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml
index cf8bbb2..d5feb7b 100644
--- a/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml
@@ -98,6 +98,7 @@
<flag name='zpci'/>
<flag name='iothread.poll-max-ns'/>
<flag name='query-cpu-model-baseline'/>
+ <flag name='query-cpu-model-comparison'/>
<version>2009000</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>39100765</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_3.0.0.s390x.xml
b/tests/qemucapabilitiesdata/caps_3.0.0.s390x.xml
index 40b4a2b..27befc4 100644
--- a/tests/qemucapabilitiesdata/caps_3.0.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_3.0.0.s390x.xml
@@ -124,6 +124,7 @@
<flag name='iothread.poll-max-ns'/>
<flag name='memory-backend-file.align'/>
<flag name='query-cpu-model-baseline'/>
+ <flag name='query-cpu-model-comparison'/>
<version>3000000</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>39100757</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml
b/tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml
index e72f447..8c8dc68 100644
--- a/tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml
@@ -132,6 +132,7 @@
<flag name='bitmap-merge'/>
<flag name='nbd-bitmap'/>
<flag name='query-cpu-model-baseline'/>
+ <flag name='query-cpu-model-comparison'/>
<version>4000000</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>39100758</microcodeVersion>
--
2.7.4
11:18 a.m.
New subject: [libvirt] [PATCH v4 6/8] qemu_capabilities: introduce QEMU_CAPS_QUERY_CPU_MODEL_COMPARISON
Reviewed-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com>
On 7/17/19 4:03 PM, Collin Walling wrote:
This capability enables CPU model comparison between two CPU
definitions via QMP.
Signed-off-by: Collin Walling <walling(a)linux.ibm.com>
Reviewed-by: Daniel Henrique Barboza <danielh413(a)gmail.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 +
tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml | 1 +
9 files changed, 10 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index ab337fa..a99ebcb 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -537,6 +537,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
/* 335 */
"bochs-display",
"query-cpu-model-baseline",
+ "query-cpu-model-comparison",
);
@@ -983,6 +984,7 @@ struct virQEMUCapsStringFlags virQEMUCapsCommands[] = {
{ "query-current-machine", QEMU_CAPS_QUERY_CURRENT_MACHINE },
{ "block-dirty-bitmap-merge", QEMU_CAPS_BITMAP_MERGE },
{ "query-cpu-model-baseline", QEMU_CAPS_QUERY_CPU_MODEL_BASELINE },
+ { "query-cpu-model-comparison", QEMU_CAPS_QUERY_CPU_MODEL_COMPARISON },
};
struct virQEMUCapsStringFlags virQEMUCapsMigration[] = {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index b49c0b9..8afa05c 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -518,6 +518,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check
*/
/* 335 */
QEMU_CAPS_DEVICE_BOCHS_DISPLAY, /* -device bochs-display */
QEMU_CAPS_QUERY_CPU_MODEL_BASELINE, /* qmp query-cpu-model-baseline */
+ QEMU_CAPS_QUERY_CPU_MODEL_COMPARISON, /* qmp query-cpu-model-comparison */
QEMU_CAPS_LAST /* this must always be the last item */
} virQEMUCapsFlags;
diff --git a/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml
b/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml
index e6f7e28..058eb32 100644
--- a/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml
@@ -104,6 +104,7 @@
<flag name='zpci'/>
<flag name='iothread.poll-max-ns'/>
<flag name='query-cpu-model-baseline'/>
+ <flag name='query-cpu-model-comparison'/>
<version>2010000</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>39100805</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml
b/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml
index 48d7742..4256131 100644
--- a/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml
@@ -111,6 +111,7 @@
<flag name='zpci'/>
<flag name='iothread.poll-max-ns'/>
<flag name='query-cpu-model-baseline'/>
+ <flag name='query-cpu-model-comparison'/>
<version>2011000</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>39100806</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml
b/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml
index 381abd5..48be96d 100644
--- a/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml
@@ -122,6 +122,7 @@
<flag name='iothread.poll-max-ns'/>
<flag name='memory-backend-file.align'/>
<flag name='query-cpu-model-baseline'/>
+ <flag name='query-cpu-model-comparison'/>
<version>2012000</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>39100807</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml
b/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml
index 9516a3b..5b04416 100644
--- a/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml
@@ -93,6 +93,7 @@
<flag name='vhost-vsock'/>
<flag name='zpci'/>
<flag name='query-cpu-model-baseline'/>
+ <flag name='query-cpu-model-comparison'/>
<version>2007093</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>39100764</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml
b/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml
index cf8bbb2..d5feb7b 100644
--- a/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml
@@ -98,6 +98,7 @@
<flag name='zpci'/>
<flag name='iothread.poll-max-ns'/>
<flag name='query-cpu-model-baseline'/>
+ <flag name='query-cpu-model-comparison'/>
<version>2009000</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>39100765</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_3.0.0.s390x.xml
b/tests/qemucapabilitiesdata/caps_3.0.0.s390x.xml
index 40b4a2b..27befc4 100644
--- a/tests/qemucapabilitiesdata/caps_3.0.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_3.0.0.s390x.xml
@@ -124,6 +124,7 @@
<flag name='iothread.poll-max-ns'/>
<flag name='memory-backend-file.align'/>
<flag name='query-cpu-model-baseline'/>
+ <flag name='query-cpu-model-comparison'/>
<version>3000000</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>39100757</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml
b/tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml
index e72f447..8c8dc68 100644
--- a/tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml
@@ -132,6 +132,7 @@
<flag name='bitmap-merge'/>
<flag name='nbd-bitmap'/>
<flag name='query-cpu-model-baseline'/>
+ <flag name='query-cpu-model-comparison'/>
<version>4000000</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>39100758</microcodeVersion>
--
Mit freundlichen Grüßen/Kind regards
Boris Fiuczynski
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Matthias Hartmann
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
9:14 a.m.
New subject: [libvirt] [PATCH v4 6/8] qemu_capabilities: introduce QEMU_CAPS_QUERY_CPU_MODEL_COMPARISON
On Wed, Jul 17, 2019 at 10:03:27 -0400, Collin Walling wrote:
This capability enables CPU model comparison between two CPU
definitions via QMP.
Signed-off-by: Collin Walling <walling(a)linux.ibm.com>
Reviewed-by: Daniel Henrique Barboza <danielh413(a)gmail.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 +
tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml | 1 +
9 files changed, 10 insertions(+)
Reviewed-by: Jiri Denemark <jdenemar(a)redhat.com>
9:03 a.m.
New subject: [libvirt] [PATCH v4 7/8] cpu_conf: xml to cpu definition parse helper
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(a)linux.ibm.com>
Reviewed-by: Bjoern Walk <bwalk(a)linux.ibm.com>
Reviewed-by: Daniel Henrique Barboza <danielh413(a)gmail.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 7d16a05..c587aff 100644
--- a/src/conf/cpu_conf.c
+++ b/src/conf/cpu_conf.c
@@ -268,6 +268,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 19ce816..a0efac8 100644
--- a/src/conf/cpu_conf.h
+++ b/src/conf/cpu_conf.h
@@ -183,6 +183,12 @@ virCPUDefPtr
virCPUDefCopyWithoutModel(const virCPUDef *cpu);
int
+virCPUDefParseXMLHelper(const char *xml,
+ const char *xpath,
+ virCPUType type,
+ virCPUDefPtr *cpu);
+
+int
virCPUDefParseXML(xmlXPathContextPtr ctxt,
const char *xpath,
virCPUType mode,
diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c
index b89462c..31ee887 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 4e77cf5..ea5edab 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -90,6 +90,7 @@ virCPUDefIsEqual;
virCPUDefListFree;
virCPUDefListParse;
virCPUDefParseXML;
+virCPUDefParseXMLHelper;
virCPUDefStealModel;
virCPUDefUpdateFeature;
virCPUModeTypeToString;
--
2.7.4
11:18 a.m.
New subject: [libvirt] [PATCH v4 7/8] cpu_conf: xml to cpu definition parse helper
Reviewed-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com>
On 7/17/19 4:03 PM, Collin Walling wrote:
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(a)linux.ibm.com>
Reviewed-by: Bjoern Walk <bwalk(a)linux.ibm.com>
Reviewed-by: Daniel Henrique Barboza <danielh413(a)gmail.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 7d16a05..c587aff 100644
--- a/src/conf/cpu_conf.c
+++ b/src/conf/cpu_conf.c
@@ -268,6 +268,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 19ce816..a0efac8 100644
--- a/src/conf/cpu_conf.h
+++ b/src/conf/cpu_conf.h
@@ -183,6 +183,12 @@ virCPUDefPtr
virCPUDefCopyWithoutModel(const virCPUDef *cpu);
int
+virCPUDefParseXMLHelper(const char *xml,
+ const char *xpath,
+ virCPUType type,
+ virCPUDefPtr *cpu);
+
+int
virCPUDefParseXML(xmlXPathContextPtr ctxt,
const char *xpath,
virCPUType mode,
diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c
index b89462c..31ee887 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 4e77cf5..ea5edab 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -90,6 +90,7 @@ virCPUDefIsEqual;
virCPUDefListFree;
virCPUDefListParse;
virCPUDefParseXML;
+virCPUDefParseXMLHelper;
virCPUDefStealModel;
virCPUDefUpdateFeature;
virCPUModeTypeToString;
--
Mit freundlichen Grüßen/Kind regards
Boris Fiuczynski
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Matthias Hartmann
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
9:27 a.m.
New subject: [libvirt] [PATCH v4 7/8] cpu_conf: xml to cpu definition parse helper
On Wed, Jul 17, 2019 at 10:03:28 -0400, Collin Walling wrote:
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(a)linux.ibm.com>
Reviewed-by: Bjoern Walk <bwalk(a)linux.ibm.com>
Reviewed-by: Daniel Henrique Barboza <danielh413(a)gmail.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 7d16a05..c587aff 100644
--- a/src/conf/cpu_conf.c
+++ b/src/conf/cpu_conf.c
@@ -268,6 +268,36 @@ virCPUDefCopy(const virCPUDef *cpu)
}
+int
+virCPUDefParseXMLHelper(const char *xml,
I would call this virCPUDefParseXMLString for consistency with other
parsing functions. If there are several variants of the parsing function
with different input types, the String suffix is used for the variant
that parses an XML string.
+ const char *xpath,
And the callers of this function should not ever need the xpath
parameter. It should go away.
+ 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 ".").
...
Jirka
9:03 a.m.
New subject: [libvirt] [PATCH v4 8/8] qemu_driver: hook up query-cpu-model-comparison
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(a)linux.ibm.com>
Reviewed-by: Daniel Henrique Barboza <danielh413(a)gmail.com>
---
src/qemu/qemu_capabilities.c | 48 ++++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_capabilities.h | 9 +++++++++
src/qemu/qemu_driver.c | 11 ++++++++++
3 files changed, 68 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index a99ebcb..20cb82b 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -5660,3 +5660,51 @@ virQEMUCapsCPUModelBaseline(virQEMUCapsPtr qemuCaps,
virCPUDefFree(cpu);
return baseline;
}
+
+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 = NULL;
+ 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);
+
+ qemuMonitorCPUModelInfoFree(result);
+ qemuProcessQMPFree(proc);
+ return ret;
+}
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 8afa05c..a62499f 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -673,3 +673,12 @@ virCPUDefPtr virQEMUCapsCPUModelBaseline(virQEMUCapsPtr qemuCaps,
gid_t runGid,
int ncpus,
virCPUDefPtr *cpus);
+
+virCPUCompareResult
+virQEMUCapsCPUModelComparison(virQEMUCapsPtr qemuCaps,
+ const char *libDir,
+ uid_t runUid,
+ gid_t runGid,
+ virCPUDefPtr cpu_a,
+ virCPUDefPtr cpu_b,
+ bool failIncompatible);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 37b9c75..3e49c04 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -13446,9 +13446,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;
@@ -13483,6 +13485,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 "
@@ -13490,6 +13500,7 @@ qemuConnectCompareHypervisorCPU(virConnectPtr conn,
}
cleanup:
+ virCPUDefFree(cpu);
virObjectUnref(qemuCaps);
return ret;
}
--
2.7.4
11:18 a.m.
New subject: [libvirt] [PATCH v4 8/8] qemu_driver: hook up query-cpu-model-comparison
Reviewed-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com>
On 7/17/19 4:03 PM, Collin Walling wrote:
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(a)linux.ibm.com>
Reviewed-by: Daniel Henrique Barboza <danielh413(a)gmail.com>
---
src/qemu/qemu_capabilities.c | 48 ++++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_capabilities.h | 9 +++++++++
src/qemu/qemu_driver.c | 11 ++++++++++
3 files changed, 68 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index a99ebcb..20cb82b 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -5660,3 +5660,51 @@ virQEMUCapsCPUModelBaseline(virQEMUCapsPtr qemuCaps,
virCPUDefFree(cpu);
return baseline;
}
+
+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 = NULL;
+ 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);
+
+ qemuMonitorCPUModelInfoFree(result);
+ qemuProcessQMPFree(proc);
+ return ret;
+}
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 8afa05c..a62499f 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -673,3 +673,12 @@ virCPUDefPtr virQEMUCapsCPUModelBaseline(virQEMUCapsPtr qemuCaps,
gid_t runGid,
int ncpus,
virCPUDefPtr *cpus);
+
+virCPUCompareResult
+virQEMUCapsCPUModelComparison(virQEMUCapsPtr qemuCaps,
+ const char *libDir,
+ uid_t runUid,
+ gid_t runGid,
+ virCPUDefPtr cpu_a,
+ virCPUDefPtr cpu_b,
+ bool failIncompatible);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 37b9c75..3e49c04 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -13446,9 +13446,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;
@@ -13483,6 +13485,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
"
@@ -13490,6 +13500,7 @@ qemuConnectCompareHypervisorCPU(virConnectPtr conn,
}
cleanup:
+ virCPUDefFree(cpu);
virObjectUnref(qemuCaps);
return ret;
}
--
Mit freundlichen Grüßen/Kind regards
Boris Fiuczynski
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Matthias Hartmann
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
1:26 p.m.
New subject: [libvirt] [PATCH v4 8/8] qemu_driver: hook up query-cpu-model-comparison
[...]
> +
> +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 = NULL;
Set ret = VIR_CPU_COMPARE_INCOMPATIBLE
> + 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;
and change this:
> +
> + 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);
To this:
cleanup:
if (failIncompatible && ret == VIR_CPU_COMPARE_INCOMPATIBLE) {
ret = VIR_CPU_COMPARE_ERROR;
virReportError(VIR_ERR_CPU_INCOMPATIBLE, NULL);
virQEMUCapsLogProbeFailure(qemuCaps->binary);
}
> +
> + qemuMonitorCPUModelInfoFree(result);
> + qemuProcessQMPFree(proc);
> + return ret;
> +}
And now the output will look like this when the xml contains an
erroneous CPU model or feature:
virsh hypervisor-cpu-compare cpufail.xml
CPU described in cpufail.xml is incompatible with the CPU provided by
hypervisor on the host
virsh hypervisor-cpu-compare cpufail.xml --error
error: Failed to compare hypervisor CPU with cpufail.xml
error: the CPU is incompatible with host CPU
If this output is not acceptable, then perhaps we should further explore
option 2 that I described on patch 5.
[...]
4:59 a.m.
New subject: [libvirt] [PATCH v4 8/8] qemu_driver: hook up query-cpu-model-comparison
On 7/25/19 8:26 PM, Collin Walling wrote:
[...]
>> +
>> +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 = NULL;
Set ret = VIR_CPU_COMPARE_INCOMPATIBLE
>> + 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;
and change this:
>> +
>> + 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);
To this:
cleanup:
if (failIncompatible && ret == VIR_CPU_COMPARE_INCOMPATIBLE) {
ret = VIR_CPU_COMPARE_ERROR;
virReportError(VIR_ERR_CPU_INCOMPATIBLE, NULL);
virQEMUCapsLogProbeFailure(qemuCaps->binary);
}
>> +
>> + qemuMonitorCPUModelInfoFree(result);
>> + qemuProcessQMPFree(proc);
>> + return ret;
>> +}
And now the output will look like this when the xml contains an
erroneous CPU model or feature:
virsh hypervisor-cpu-compare cpufail.xml
CPU described in cpufail.xml is incompatible with the CPU provided by
hypervisor on the host
virsh hypervisor-cpu-compare cpufail.xml --error
error: Failed to compare hypervisor CPU with cpufail.xml
error: the CPU is incompatible with host CPU
If this output is not acceptable, then perhaps we should further explore
option 2 that I described on patch 5.
[...]
Please see my response in patch 5.
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
--
Mit freundlichen Grüßen/Kind regards
Boris Fiuczynski
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Matthias Hartmann
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
9:33 a.m.
New subject: [libvirt] [PATCH v4 8/8] qemu_driver: hook up query-cpu-model-comparison
On Wed, Jul 17, 2019 at 10:03:29 -0400, Collin Walling wrote:
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(a)linux.ibm.com>
Reviewed-by: Daniel Henrique Barboza <danielh413(a)gmail.com>
---
src/qemu/qemu_capabilities.c | 48 ++++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_capabilities.h | 9 +++++++++
src/qemu/qemu_driver.c | 11 ++++++++++
3 files changed, 68 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index a99ebcb..20cb82b 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -5660,3 +5660,51 @@ virQEMUCapsCPUModelBaseline(virQEMUCapsPtr qemuCaps,
virCPUDefFree(cpu);
return baseline;
}
+
+virCPUCompareResult
+virQEMUCapsCPUModelComparison(virQEMUCapsPtr qemuCaps,
+ const char *libDir,
+ uid_t runUid,
+ gid_t runGid,
+ virCPUDefPtr cpu_a,
+ virCPUDefPtr cpu_b,
+ bool failIncompatible)
This function should go into qemu_driver.c and its name should change
accordingly.
+{
+ qemuProcessQMPPtr proc = NULL;
+ qemuMonitorCPUModelInfoPtr result = NULL;
+ 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);
Again, no logging here, please. The reported error is already going to
be logged anyway. And mainly since this is called from inside an API
invoked by a client, the client will get the error. Probing is different
as there's no API involved, QEMU binaries are probed when libvirtd
starts.
+
+ qemuMonitorCPUModelInfoFree(result);
+ qemuProcessQMPFree(proc);
+ return ret;
+}
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 8afa05c..a62499f 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -673,3 +673,12 @@ virCPUDefPtr virQEMUCapsCPUModelBaseline(virQEMUCapsPtr qemuCaps,
gid_t runGid,
int ncpus,
virCPUDefPtr *cpus);
+
+virCPUCompareResult
+virQEMUCapsCPUModelComparison(virQEMUCapsPtr qemuCaps,
+ const char *libDir,
+ uid_t runUid,
+ gid_t runGid,
+ virCPUDefPtr cpu_a,
+ virCPUDefPtr cpu_b,
+ bool failIncompatible);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 37b9c75..3e49c04 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -13446,9 +13446,11 @@ qemuConnectCompareHypervisorCPU(virConnectPtr conn,
{
int ret = VIR_CPU_COMPARE_ERROR;
virQEMUDriverPtr driver = conn->privateData;
+ virQEMUDriverConfigPtr config = driver->config;
VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver);
virQEMUCapsPtr qemuCaps = NULL;
bool failIncompatible;
virCPUDefPtr hvCPU;
+ virCPUDefPtr cpu;
virArch arch;
virDomainVirtType virttype;
@@ -13483,6 +13485,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
"
@@ -13490,6 +13500,7 @@ qemuConnectCompareHypervisorCPU(virConnectPtr conn,
}
cleanup:
+ virCPUDefFree(cpu);
virObjectUnref(qemuCaps);
return ret;
}
Jirka
9:06 a.m.
First, let me apologize for such a late review. I'll try my best to
review your series earlier next time.
On Wed, Jul 17, 2019 at 10:03:21 -0400, Collin Walling wrote:
When baselining CPU models and the user appends the --features
argument
to the command, s390x will only report back features that supersede the
base model definition.
*NOTE* if the --features flag is intended to expand /ALL/ features
available to a CPU model (such as the huge list of features reported
by a full CPU model expansion), please let me know and I can resolve
this.
I'm not sure how well this fits s390 world, but the semantics of
VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES flag is to report all CPU
features which are hidden behind the CPU model. That is, all feature
which you'd get when starting QEMU with just the CPU model name and no
additional features. The extra features should not be touched at all.
Specifically, removing them when the flag is not used is wrong.
To me this looks like the flag should really result in running
query-cpu-model-expansion (but likely the "static" one rather then
"full" expansion) on the baseline CPU model and reporting the enabled
features along with those already included in the baseline feature set.
Jirka
2:32 p.m.
On 8/20/19 10:06 AM, Jiri Denemark wrote:
First, let me apologize for such a late review. I'll try my best
to
review your series earlier next time.
Your review is greatly appreciated! I haven't replied to your other
posts on this series as my comments were mostly acknowledgements rather
than discussion pieces. I'm working through them.
On Wed, Jul 17, 2019 at 10:03:21 -0400, Collin Walling wrote:
> When baselining CPU models and the user appends the --features argument
> to the command, s390x will only report back features that supersede the
> base model definition.
>
> *NOTE* if the --features flag is intended to expand /ALL/ features
> available to a CPU model (such as the huge list of features reported
> by a full CPU model expansion), please let me know and I can resolve
> this.
I'm not sure how well this fits s390 world, but the semantics of
VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES flag is to report all CPU
features which are hidden behind the CPU model. That is, all feature
which you'd get when starting QEMU with just the CPU model name and no
additional features. The extra features should not be touched at all.
Specifically, removing them when the flag is not used is wrong.
To me this looks like the flag should really result in running
query-cpu-model-expansion (but likely the "static" one rather then
"full" expansion) on the baseline CPU model and reporting the enabled
features along with those already included in the baseline feature set.
Actually, query-cpu-model-baseline will return a CPU model along with a
feature set. The features returned are the same as those produced from a
static expansion on the model.
Correct me if I am wrong here: virsh should report features *only* if the
--features flag is present. Otherwise, we only report the model name (which
can be accomplished by stripping the result of all reported features).
Jirka
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Thank you for your review!
--
Respectfully,
- Collin Walling
10:22 a.m.
On Tue, Sep 03, 2019 at 15:32:34 -0400, Collin Walling wrote:
On 8/20/19 10:06 AM, Jiri Denemark wrote:
> First, let me apologize for such a late review. I'll try my best to
> review your series earlier next time.
>
Your review is greatly appreciated! I haven't replied to your other
posts on this series as my comments were mostly acknowledgements rather
than discussion pieces. I'm working through them.
> On Wed, Jul 17, 2019 at 10:03:21 -0400, Collin Walling wrote:
>> When baselining CPU models and the user appends the --features argument
>> to the command, s390x will only report back features that supersede the
>> base model definition.
>>
>> *NOTE* if the --features flag is intended to expand /ALL/ features
>> available to a CPU model (such as the huge list of features reported
>> by a full CPU model expansion), please let me know and I can resolve
>> this.
>
> I'm not sure how well this fits s390 world, but the semantics of
> VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES flag is to report all CPU
> features which are hidden behind the CPU model. That is, all feature
> which you'd get when starting QEMU with just the CPU model name and no
> additional features. The extra features should not be touched at all.
> Specifically, removing them when the flag is not used is wrong.
>
> To me this looks like the flag should really result in running
> query-cpu-model-expansion (but likely the "static" one rather then
> "full" expansion) on the baseline CPU model and reporting the enabled
> features along with those already included in the baseline feature set.
>
Actually, query-cpu-model-baseline will return a CPU model along with a
feature set. The features returned are the same as those produced from a
static expansion on the model.
Correct me if I am wrong here: virsh should report features *only* if the
--features flag is present. Otherwise, we only report the model name (which
can be accomplished by stripping the result of all reported features).
No, this is wrong in general (although it maybe correct for s390, I
don't know).
Let me explain with some hypothetical CPU models.
Model1 (when used as -cpu Model1) will enable features f1, f2, f3.
Model2 will enable f1, f2, f3, f4, f5.
Model3 will enable f1, f2, f3, f4, f6.
Running baseline command for [Model1, Model2, Model3] should return
Model1 with no additional features as [f1, f2, f3] are in only ones
common to all three models and they are all enabled by Model1.
However, baseline of [Model2, Model3] should return Model1 + f4. The
common set of features is [f1, f2, f3, f4], but Model1 would enable [f1,
f2, f3] only, which means we need to add f4 explicitly.
With --features, even the features enabled implicitly by a specific CPU
model should be returned. That is, the first result would be Model1 +
[f1, f2, f3] and the second result would be Model1 + [f1, f2, f3, f4].
In other words, stripping all features if no --features option is used
is wrong. Only features implicitly enabled by the CPU model should be
stripped.
Specifically, baseline without --features on the following CPU
definition
<cpu mode='custom' match='exact'>
<model>Model1</model>
<feature name='f4' policy='require'/>
</cpu>
should be the same definition (i.e., Model1 + f4).
The question is whether you can enabled extra features on s390 or you
can only use plain CPU models with no additional features. If additional
features are allowed, I'd guess baseline without features should return
the result of the QMP command minus the features a static expansion of
the CPU model itself would return (this is assuming f4 will be included
in the result we get from QEMU for Model1 + f4). When --features is
used, the result from QEMU should be returned.
I hope it's clear now. However, I'm not sure how CPU models work on
s390 and I'd be happy if you could explain it to me. Especially, whether
something similar to what I described can happen there.
Jirka
10:38 a.m.
On 10.09.19 17:22, Jiri Denemark wrote:
On Tue, Sep 03, 2019 at 15:32:34 -0400, Collin Walling wrote:
> On 8/20/19 10:06 AM, Jiri Denemark wrote:
>> First, let me apologize for such a late review. I'll try my best to
>> review your series earlier next time.
>>
>
> Your review is greatly appreciated! I haven't replied to your other
> posts on this series as my comments were mostly acknowledgements rather
> than discussion pieces. I'm working through them.
>
>> On Wed, Jul 17, 2019 at 10:03:21 -0400, Collin Walling wrote:
>>> When baselining CPU models and the user appends the --features argument
>>> to the command, s390x will only report back features that supersede the
>>> base model definition.
>>>
>>> *NOTE* if the --features flag is intended to expand /ALL/ features
>>> available to a CPU model (such as the huge list of features reported
>>> by a full CPU model expansion), please let me know and I can resolve
>>> this.
>>
>> I'm not sure how well this fits s390 world, but the semantics of
>> VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES flag is to report all CPU
>> features which are hidden behind the CPU model. That is, all feature
>> which you'd get when starting QEMU with just the CPU model name and no
>> additional features. The extra features should not be touched at all.
>> Specifically, removing them when the flag is not used is wrong.
>>
>> To me this looks like the flag should really result in running
>> query-cpu-model-expansion (but likely the "static" one rather then
>> "full" expansion) on the baseline CPU model and reporting the enabled
>> features along with those already included in the baseline feature set.
>>
>
> Actually, query-cpu-model-baseline will return a CPU model along with a
> feature set. The features returned are the same as those produced from a
> static expansion on the model.
>
> Correct me if I am wrong here: virsh should report features *only* if the
> --features flag is present. Otherwise, we only report the model name (which
> can be accomplished by stripping the result of all reported features).
No, this is wrong in general (although it maybe correct for s390, I
don't know).
David here: Not correct for s390x
Let me explain with some hypothetical CPU models.
Model1 (when used as -cpu Model1) will enable features f1, f2, f3.
Model2 will enable f1, f2, f3, f4, f5.
Model3 will enable f1, f2, f3, f4, f6.
Running baseline command for [Model1, Model2, Model3] should return
Model1 with no additional features as [f1, f2, f3] are in only ones
common to all three models and they are all enabled by Model1.
However, baseline of [Model2, Model3] should return Model1 + f4. The
common set of features is [f1, f2, f3, f4], but Model1 would enable [f1,
f2, f3] only, which means we need to add f4 explicitly.
With --features, even the features enabled implicitly by a specific CPU
model should be returned. That is, the first result would be Model1 +
[f1, f2, f3] and the second result would be Model1 + [f1, f2, f3, f4].
In other words, stripping all features if no --features option is used
is wrong. Only features implicitly enabled by the CPU model should be
stripped.
Specifically, baseline without --features on the following CPU
definition
<cpu mode='custom' match='exact'>
<model>Model1</model>
<feature name='f4' policy='require'/>
</cpu>
should be the same definition (i.e., Model1 + f4).
s390x will fallback to base models when baselining/expanding ("minimum
feature set we expect to be around for a certain cpu generation", which
is independent of the QEMU version and will never change). But if you're
already using base models, it works as you would expect it.
E.g.
Model1 is [f1, f2, f3]
Model1-base is [f1, f2]
Model2 is [f1, f2, f3, f4]
Model2-base is [f1, f2, f3]
Baselining Model1 with Model2 would result in
Model1-base + f3
For this, QMP "query-cpu-model-baseline" can be used.
Note: When expending cpu models (e.g. host), one would already always
get base models. So there, it would work completely as you expect it.
Now, to achieve the "--features" part, simply throw the result of
"query-cpu-model-baseline" into "query-cpu-model-expansion" with
"CpuModelExpansionType" == "full"
The question is whether you can enabled extra features on s390 or you
can only use plain CPU models with no additional features. If additional
features are allowed, I'd guess baseline without features should return
the result of the QMP command minus the features a static expansion of
the CPU model itself would return (this is assuming f4 will be included
in the result we get from QEMU for Model1 + f4). When --features is
used, the result from QEMU should be returned.
We do have plenty of extra cpu features on s390x. *Plenty* :)
I hope it's clear now. However, I'm not sure how CPU models work on
s390 and I'd be happy if you could explain it to me. Especially, whether
something similar to what I described can happen there.
Jirka
--
Thanks,
David / dhildenb
2:59 p.m.
On 9/10/19 11:38 AM, David Hildenbrand wrote:
On 10.09.19 17:22, Jiri Denemark wrote:
> On Tue, Sep 03, 2019 at 15:32:34 -0400, Collin Walling wrote:
>> On 8/20/19 10:06 AM, Jiri Denemark wrote:
>>> First, let me apologize for such a late review. I'll try my best to
>>> review your series earlier next time.
>>>
>>
>> Your review is greatly appreciated! I haven't replied to your other
>> posts on this series as my comments were mostly acknowledgements rather
>> than discussion pieces. I'm working through them.
>>
>>> On Wed, Jul 17, 2019 at 10:03:21 -0400, Collin Walling wrote:
>>>> When baselining CPU models and the user appends the --features argument
>>>> to the command, s390x will only report back features that supersede the
>>>> base model definition.
>>>>
>>>> *NOTE* if the --features flag is intended to expand /ALL/ features
>>>> available to a CPU model (such as the huge list of features reported
>>>> by a full CPU model expansion), please let me know and I can resolve
>>>> this.
>>>
>>> I'm not sure how well this fits s390 world, but the semantics of
>>> VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES flag is to report all CPU
>>> features which are hidden behind the CPU model. That is, all feature
>>> which you'd get when starting QEMU with just the CPU model name and no
>>> additional features. The extra features should not be touched at all.
>>> Specifically, removing them when the flag is not used is wrong.
>>>
>>> To me this looks like the flag should really result in running
>>> query-cpu-model-expansion (but likely the "static" one rather then
>>> "full" expansion) on the baseline CPU model and reporting the
enabled
>>> features along with those already included in the baseline feature set.
>>>
>>
>> Actually, query-cpu-model-baseline will return a CPU model along with a
>> feature set. The features returned are the same as those produced from a
>> static expansion on the model.
>>
>> Correct me if I am wrong here: virsh should report features *only* if the
>> --features flag is present. Otherwise, we only report the model name (which
>> can be accomplished by stripping the result of all reported features).
>
> No, this is wrong in general (although it maybe correct for s390, I
> don't know).
David here: Not correct for s390x
>
> Let me explain with some hypothetical CPU models.
>
> Model1 (when used as -cpu Model1) will enable features f1, f2, f3.
> Model2 will enable f1, f2, f3, f4, f5.
> Model3 will enable f1, f2, f3, f4, f6.
>
> Running baseline command for [Model1, Model2, Model3] should return
> Model1 with no additional features as [f1, f2, f3] are in only ones
> common to all three models and they are all enabled by Model1.
>
> However, baseline of [Model2, Model3] should return Model1 + f4. The
> common set of features is [f1, f2, f3, f4], but Model1 would enable [f1,
> f2, f3] only, which means we need to add f4 explicitly.
>
> With --features, even the features enabled implicitly by a specific CPU
> model should be returned. That is, the first result would be Model1 +
> [f1, f2, f3] and the second result would be Model1 + [f1, f2, f3, f4].
>
> In other words, stripping all features if no --features option is used
> is wrong. Only features implicitly enabled by the CPU model should be
> stripped.
>
> Specifically, baseline without --features on the following CPU
> definition
>
> <cpu mode='custom' match='exact'>
> <model>Model1</model>
> <feature name='f4' policy='require'/>
> </cpu>
>
> should be the same definition (i.e., Model1 + f4).
s390x will fallback to base models when baselining/expanding ("minimum
feature set we expect to be around for a certain cpu generation", which
is independent of the QEMU version and will never change). But if you're
already using base models, it works as you would expect it.
E.g.
Model1 is [f1, f2, f3]
Model1-base is [f1, f2]
Model2 is [f1, f2, f3, f4]
Model2-base is [f1, f2, f3]
Baselining Model1 with Model2 would result in
Model1-base + f3
For this, QMP "query-cpu-model-baseline" can be used.
Note: When expending cpu models (e.g. host), one would already always
get base models. So there, it would work completely as you expect it.
Now, to achieve the "--features" part, simply throw the result of
"query-cpu-model-baseline" into "query-cpu-model-expansion" with
"CpuModelExpansionType" == "full"
>
> The question is whether you can enabled extra features on s390 or you
> can only use plain CPU models with no additional features. If additional
> features are allowed, I'd guess baseline without features should return
> the result of the QMP command minus the features a static expansion of
> the CPU model itself would return (this is assuming f4 will be included
> in the result we get from QEMU for Model1 + f4). When --features is
> used, the result from QEMU should be returned.
We do have plenty of extra cpu features on s390x. *Plenty* :)
Indeed, there's quite a few.
>
> I hope it's clear now. However, I'm not sure how CPU models work on
> s390 and I'd be happy if you could explain it to me. Especially, whether
> something similar to what I described can happen there.
>
> Jirka
>
Yes, both you and David have provided an excellent explanation of what's
expected (from both perspectives). Thank you both. I'll make the
appropriate changes.
1900
days inactive
1955
days old
35 comments
4 participants
participants (4)
-
Boris Fiuczynski -
Collin Walling -
David Hildenbrand -
Jiri Denemark