4:26 p.m.
Some architectures (S390) depend on QEMU to compute baseline CPU model and
expand a models feature set.
Interacting with QEMU requires starting the QEMU process and completing one or
more query-cpu-model-baseline QMP exchanges with QEMU in addition to a
query-cpu-model-expansion QMP exchange to expand all features in the model.
See "s390x CPU models: exposing features" patch set on Qemu-devel for
discussion
of QEMU aspects.
This is part of resolution of: https://bugzilla.redhat.com/show_bug.cgi?id=1511999
Version 3 attempts to address all issues from V1 and V2 including making the
QEMU process activation for QMP Queries generic (not specific to capabilities)
and moving that code from qemu_capabilities to qemu_process.
I attempted to make the new qemu_process functions consistent with the functions
but mostly ended up reusing the guts of the previous functions from qemu_capabilities.
Of interest may be the choice to reuse a domain structure to hold the Qmp Query
process state and connection information. Also, pay attention to the use of a large
random number to uniquely identify decoupled processes in terms of sockets and
file system footprint. If you believe it's worth the effort I think there are some
pre-existing library functions to establish files with unique identifiers in a
thread safe way.
The last patch may also be interesting and is based on past discussion of QEMU cpu
expansion only returning migratable features except for one x86 case where
non-migratable features are explicitly requested. The patch records that features
are migratable based on QEMU only returning migratable features. The main result
is the CPU xml for S390 CPU's contains the same migratability info the X86 currently
does. The testcases were updated to reflect the change. Is this ok?
Unlike the previous versions every patch should compile independently if applied
in sequence.
Thanks,
Chris
Chris Venteicher (6):
qemu_monitor: Introduce qemuMonitorCPUModelInfoNew
qemu_monitor: Introduce qemuMonitorCPUModelInfo / JSON conversion
qemu_monitor: qemuMonitorGetCPUModelExpansion inputs and outputs
CPUModelInfo
qemu_process: Use common processes mgmt funcs for all QMP query types
qemu_driver: BaselineHypervisorCPU support for S390
qemu_monitor: Default props to migratable when expanding cpu model
src/qemu/qemu_capabilities.c | 559 ++++++++----------
src/qemu/qemu_capabilities.h | 4 +
src/qemu/qemu_driver.c | 143 ++++-
src/qemu/qemu_monitor.c | 199 ++++++-
src/qemu/qemu_monitor.h | 29 +-
src/qemu/qemu_monitor_json.c | 210 +++++--
src/qemu/qemu_monitor_json.h | 13 +-
src/qemu/qemu_process.c | 398 +++++++++++++
src/qemu/qemu_process.h | 35 ++
tests/cputest.c | 11 +-
.../caps_2.10.0.s390x.xml | 60 +-
.../caps_2.11.0.s390x.xml | 58 +-
.../caps_2.12.0.s390x.xml | 56 +-
.../qemucapabilitiesdata/caps_2.8.0.s390x.xml | 32 +-
.../qemucapabilitiesdata/caps_2.9.0.s390x.xml | 34 +-
.../qemucapabilitiesdata/caps_3.0.0.s390x.xml | 64 +-
tests/qemucapabilitiestest.c | 7 +
17 files changed, 1375 insertions(+), 537 deletions(-)
--
2.17.1
4:26 p.m.
New subject: [libvirt] [PATCH v3 1/6] qemu_monitor: Introduce qemuMonitorCPUModelInfoNew
A helper function allocates an initializes model name
in CPU Model Info structs.
Signed-off-by: Chris Venteicher <cventeic(a)redhat.com>
---
src/qemu/qemu_capabilities.c | 2 +-
src/qemu/qemu_monitor.c | 32 +++++++++++++++++++++++++++-----
src/qemu/qemu_monitor.h | 2 ++
3 files changed, 30 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index e228f52ec0..e60a4b369e 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -3022,7 +3022,7 @@ virQEMUCapsLoadHostCPUModelInfo(virQEMUCapsPtr qemuCaps,
goto cleanup;
}
- if (VIR_ALLOC(hostCPU) < 0)
+ if (!(hostCPU = qemuMonitorCPUModelInfoNew(NULL)))
goto cleanup;
if (!(hostCPU->name = virXMLPropString(hostCPUNode, "model"))) {
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 7f7013e115..45a4568fcc 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -3670,6 +3670,31 @@ qemuMonitorGetCPUModelExpansion(qemuMonitorPtr mon,
}
+qemuMonitorCPUModelInfoPtr
+qemuMonitorCPUModelInfoNew(const char *name)
+{
+ qemuMonitorCPUModelInfoPtr ret = NULL;
+ qemuMonitorCPUModelInfoPtr model;
+
+ if (VIR_ALLOC(model) < 0)
+ return NULL;
+
+ model->name = NULL;
+ model->nprops = 0;
+ model->props = NULL;
+ model->migratability = false;
+
+ if (VIR_STRDUP(model->name, name) < 0)
+ goto cleanup;
+
+ VIR_STEAL_PTR(ret, model);
+
+ cleanup:
+ qemuMonitorCPUModelInfoFree(model);
+ return ret;
+}
+
+
void
qemuMonitorCPUModelInfoFree(qemuMonitorCPUModelInfoPtr model_info)
{
@@ -3693,18 +3718,15 @@ qemuMonitorCPUModelInfoFree(qemuMonitorCPUModelInfoPtr
model_info)
qemuMonitorCPUModelInfoPtr
qemuMonitorCPUModelInfoCopy(const qemuMonitorCPUModelInfo *orig)
{
- qemuMonitorCPUModelInfoPtr copy;
+ qemuMonitorCPUModelInfoPtr copy = NULL;
size_t i;
- if (VIR_ALLOC(copy) < 0)
+ if (!orig || !(copy = qemuMonitorCPUModelInfoNew(orig->name)))
goto error;
if (VIR_ALLOC_N(copy->props, orig->nprops) < 0)
goto error;
- if (VIR_STRDUP(copy->name, orig->name) < 0)
- goto error;
-
copy->migratability = orig->migratability;
copy->nprops = orig->nprops;
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 48b142a4f4..d87b5a4ec0 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -1042,6 +1042,8 @@ int qemuMonitorGetCPUModelExpansion(qemuMonitorPtr mon,
void qemuMonitorCPUModelInfoFree(qemuMonitorCPUModelInfoPtr model_info);
+qemuMonitorCPUModelInfoPtr qemuMonitorCPUModelInfoNew(const char *name);
+
qemuMonitorCPUModelInfoPtr
qemuMonitorCPUModelInfoCopy(const qemuMonitorCPUModelInfo *orig);
--
2.17.1
4:26 p.m.
New subject: [libvirt] [PATCH v3 2/6] qemu_monitor: Introduce qemuMonitorCPUModelInfo / JSON conversion
Conversion functions are used convert CPUModelInfo structs into
QMP JSON and the reverse.
QMP JSON is of form:
{"model": {"name": "IvyBridge", "props": {}}}
qemuMonitorCPUModelInfoBoolPropAdd is used to add boolean properties to
CPUModelInfo struct.
qemuMonitorJSONGetCPUModelExpansion makes full use of conversions and
propAdd in prep to support input of full cpu model in future.
Signed-off-by: Chris Venteicher <cventeic(a)redhat.com>
---
src/qemu/qemu_monitor.c | 24 ++++++
src/qemu/qemu_monitor.h | 5 ++
src/qemu/qemu_monitor_json.c | 154 +++++++++++++++++++++++++----------
3 files changed, 138 insertions(+), 45 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 45a4568fcc..801c072eff 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -3764,6 +3764,30 @@ qemuMonitorCPUModelInfoCopy(const qemuMonitorCPUModelInfo *orig)
}
+int
+qemuMonitorCPUModelInfoBoolPropAdd(qemuMonitorCPUModelInfoPtr model,
+ const char *prop_name,
+ bool prop_value)
+{
+ int ret = -1;
+ qemuMonitorCPUProperty prop;
+ prop.type = QEMU_MONITOR_CPU_PROPERTY_BOOLEAN;
+ prop.value.boolean = prop_value;
+
+ if (VIR_STRDUP(prop.name, prop_name) < 0)
+ goto cleanup;
+
+ if (VIR_APPEND_ELEMENT(model->props, model->nprops, prop) < 0)
+ goto cleanup;
+
+ ret = 0;
+
+ cleanup:
+ VIR_FREE(prop.name);
+ return ret;
+}
+
+
int
qemuMonitorGetCommands(qemuMonitorPtr mon,
char ***commands)
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index d87b5a4ec0..0a09590ed1 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -1047,6 +1047,11 @@ qemuMonitorCPUModelInfoPtr qemuMonitorCPUModelInfoNew(const char
*name);
qemuMonitorCPUModelInfoPtr
qemuMonitorCPUModelInfoCopy(const qemuMonitorCPUModelInfo *orig);
+int qemuMonitorCPUModelInfoBoolPropAdd(qemuMonitorCPUModelInfoPtr model,
+ const char *prop_name,
+ bool prop_value)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+
int qemuMonitorGetCommands(qemuMonitorPtr mon,
char ***commands);
int qemuMonitorGetEvents(qemuMonitorPtr mon,
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 3de298c9e2..f20e9e9379 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -5505,6 +5505,101 @@ qemuMonitorJSONParseCPUModelProperty(const char *key,
return 0;
}
+
+/* model_json: {"name": "z13-base", "props": {}}
+ */
+static virJSONValuePtr
+qemuMonitorJSONBuildCPUModelInfoToJSON(qemuMonitorCPUModelInfoPtr model)
+{
+ virJSONValuePtr cpu_props = NULL;
+ virJSONValuePtr model_json = NULL;
+ size_t i;
+
+ if (!model)
+ goto cleanup;
+
+ if (model->nprops > 0 && !(cpu_props = virJSONValueNewObject()))
+ goto cleanup;
+
+ for (i = 0; i < model->nprops; i++) {
+ qemuMonitorCPUPropertyPtr prop = &(model->props[i]);
+
+ switch (prop->type) {
+ case QEMU_MONITOR_CPU_PROPERTY_BOOLEAN:
+ if (virJSONValueObjectAppendBoolean(cpu_props, prop->name,
+ prop->value.boolean) < 0)
+ goto cleanup;
+ break;
+
+ case QEMU_MONITOR_CPU_PROPERTY_STRING:
+ if (virJSONValueObjectAppendString(cpu_props, prop->name,
+ prop->value.string) < 0)
+ goto cleanup;
+ break;
+
+ case QEMU_MONITOR_CPU_PROPERTY_NUMBER:
+ if (virJSONValueObjectAppendNumberLong(cpu_props, prop->name,
+ prop->value.number) < 0)
+ goto cleanup;
+ break;
+
+ case QEMU_MONITOR_CPU_PROPERTY_LAST:
+ default:
+ virReportEnumRangeError(qemuMonitorCPUPropertyPtr, prop->type);
+ goto cleanup;
+ }
+ }
+
+ ignore_value(virJSONValueObjectCreate(&model_json, "s:name",
model->name,
+ "A:props", &cpu_props, NULL));
+
+ cleanup:
+ virJSONValueFree(cpu_props);
+ return model_json;
+}
+
+
+/* model_json: {"name": "IvyBridge", "props": {}}
+ */
+static qemuMonitorCPUModelInfoPtr
+qemuMonitorJSONBuildCPUModelInfoFromJSON(virJSONValuePtr cpu_model)
+{
+ virJSONValuePtr cpu_props;
+ qemuMonitorCPUModelInfoPtr model = NULL;
+ qemuMonitorCPUModelInfoPtr ret = NULL;
+ char const *cpu_name;
+
+ if (!(cpu_name = virJSONValueObjectGetString(cpu_model, "name"))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Parsed JSON reply missing 'name'"));
+ goto cleanup;
+ }
+
+ if (VIR_ALLOC(model) < 0)
+ goto cleanup;
+
+ if (VIR_STRDUP(model->name, cpu_name) < 0)
+ goto cleanup;
+
+ if ((cpu_props = virJSONValueObjectGetObject(cpu_model, "props"))) {
+ if (VIR_ALLOC_N(model->props,
+ virJSONValueObjectKeysNumber(cpu_props)) < 0)
+ goto cleanup;
+
+ if (virJSONValueObjectForeachKeyValue(cpu_props,
+ qemuMonitorJSONParseCPUModelProperty,
+ model) < 0)
+ goto cleanup;
+ }
+
+ VIR_STEAL_PTR(ret, model);
+
+ cleanup:
+ qemuMonitorCPUModelInfoFree(model);
+
+ return ret;
+}
+
int
qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon,
qemuMonitorCPUModelExpansionType type,
@@ -5513,32 +5608,25 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon,
qemuMonitorCPUModelInfoPtr *model_info)
{
int ret = -1;
- virJSONValuePtr model = NULL;
- virJSONValuePtr props = NULL;
+ virJSONValuePtr json_model_in = NULL;
virJSONValuePtr cmd = NULL;
virJSONValuePtr reply = NULL;
virJSONValuePtr data;
virJSONValuePtr cpu_model;
- virJSONValuePtr cpu_props;
- qemuMonitorCPUModelInfoPtr machine_model = NULL;
- char const *cpu_name;
+ qemuMonitorCPUModelInfoPtr model_in = NULL;
const char *typeStr = "";
*model_info = NULL;
- if (!(model = virJSONValueNewObject()))
+ if (!(model_in = qemuMonitorCPUModelInfoNew(model_name)))
goto cleanup;
- if (virJSONValueObjectAppendString(model, "name", model_name) < 0)
+ if (!migratable &&
+ qemuMonitorCPUModelInfoBoolPropAdd(model_in, "migratable", false) <
0)
goto cleanup;
- if (!migratable) {
- if (!(props = virJSONValueNewObject()) ||
- virJSONValueObjectAppendBoolean(props, "migratable", false) < 0
||
- virJSONValueObjectAppend(model, "props", props) < 0)
- goto cleanup;
- props = NULL;
- }
+ if (!(json_model_in = qemuMonitorJSONBuildCPUModelInfoToJSON(model_in)))
+ goto cleanup;
retry:
switch (type) {
@@ -5554,7 +5642,7 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon,
if (!(cmd = qemuMonitorJSONMakeCommand("query-cpu-model-expansion",
"s:type", typeStr,
- "a:model", &model,
+ "a:model", &json_model_in,
NULL)))
goto cleanup;
@@ -5585,7 +5673,9 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon,
* on the result of the initial "static" expansion.
*/
if (type == QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC_FULL) {
- if (!(model = virJSONValueCopy(cpu_model)))
+ virJSONValueFree(json_model_in);
+
+ if (!(json_model_in = virJSONValueCopy(cpu_model)))
goto cleanup;
virJSONValueFree(cmd);
@@ -5594,42 +5684,16 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon,
goto retry;
}
- if (!(cpu_name = virJSONValueObjectGetString(cpu_model, "name"))) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("query-cpu-model-expansion reply data was missing
'name'"));
- goto cleanup;
- }
-
- if (!(cpu_props = virJSONValueObjectGetObject(cpu_model, "props"))) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("query-cpu-model-expansion reply data was missing
'props'"));
- goto cleanup;
- }
-
- if (VIR_ALLOC(machine_model) < 0)
- goto cleanup;
-
- if (VIR_STRDUP(machine_model->name, cpu_name) < 0)
- goto cleanup;
-
- if (VIR_ALLOC_N(machine_model->props, virJSONValueObjectKeysNumber(cpu_props))
< 0)
- goto cleanup;
-
- if (virJSONValueObjectForeachKeyValue(cpu_props,
- qemuMonitorJSONParseCPUModelProperty,
- machine_model) < 0)
+ if (!(*model_info = qemuMonitorJSONBuildCPUModelInfoFromJSON(cpu_model)))
goto cleanup;
ret = 0;
- *model_info = machine_model;
- machine_model = NULL;
cleanup:
- qemuMonitorCPUModelInfoFree(machine_model);
+ qemuMonitorCPUModelInfoFree(model_in);
virJSONValueFree(cmd);
virJSONValueFree(reply);
- virJSONValueFree(model);
- virJSONValueFree(props);
+ virJSONValueFree(json_model_in);
return ret;
}
--
2.17.1
4:26 p.m.
New subject: [libvirt] [PATCH v3 3/6] qemu_monitor: qemuMonitorGetCPUModelExpansion inputs and outputs CPUModelInfo
A Full CPUModelInfo structure with props is sent to QEMU for expansion.
virQEMUCapsProbeQMPHostCPU migratability logic partitioned into new function
for clarity.
Signed-off-by: Chris Venteicher <cventeic(a)redhat.com>
---
src/qemu/qemu_capabilities.c | 125 ++++++++++++++++++++++++-----------
src/qemu/qemu_monitor.c | 47 +++++++++++--
src/qemu/qemu_monitor.h | 5 +-
src/qemu/qemu_monitor_json.c | 20 ++++--
src/qemu/qemu_monitor_json.h | 6 +-
tests/cputest.c | 11 ++-
6 files changed, 156 insertions(+), 58 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index e60a4b369e..d38530ca80 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -2352,15 +2352,82 @@ virQEMUCapsProbeQMPCPUDefinitions(virQEMUCapsPtr qemuCaps,
return 0;
}
+/* virQEMUCapsMigratablePropsDiff
+ * @migratable: migratable props=true, non-migratable & unsupported props=false
+ * @nonMigratable: migratable & non-migratable props = true, unsupported props =
false
+ * @augmented: prop->migratable = VIR_TRISTATE_BOOL_{YES/NO} base on diff
+ *
+ * Use differences in Expanded CPUModelInfo inputs
+ * to augment with prop->migratable in CPUModelInfo output
+ */
+static int
+virQEMUCapsMigratablePropsDiff(qemuMonitorCPUModelInfoPtr migratable,
+ qemuMonitorCPUModelInfoPtr nonMigratable,
+ qemuMonitorCPUModelInfoPtr *augmented)
+{
+ int ret = -1;
+ qemuMonitorCPUModelInfoPtr tmp;
+ qemuMonitorCPUPropertyPtr prop;
+ qemuMonitorCPUPropertyPtr mProp;
+ qemuMonitorCPUPropertyPtr nmProp;
+ virHashTablePtr hash = NULL;
+ size_t i;
+
+ *augmented = NULL;
+
+ if (!(tmp = qemuMonitorCPUModelInfoCopy(migratable)))
+ goto cleanup;
+
+ if (!nonMigratable)
+ goto done;
+
+ if (!(hash = virHashCreate(0, NULL)))
+ goto cleanup;
+
+ for (i = 0; i < tmp->nprops; i++) {
+ prop = tmp->props + i;
+
+ if (virHashAddEntry(hash, prop->name, prop) < 0)
+ goto cleanup;
+ }
+
+ for (i = 0; i < nonMigratable->nprops; i++) {
+ nmProp = nonMigratable->props + i;
+
+ if (!(mProp = virHashLookup(hash, nmProp->name)) ||
+ mProp->type != QEMU_MONITOR_CPU_PROPERTY_BOOLEAN ||
+ mProp->type != nmProp->type)
+ continue; /* In non-migratable list but not in migratable list */
+
+ if (mProp->value.boolean) {
+ mProp->migratable = VIR_TRISTATE_BOOL_YES;
+ } else if (nmProp->value.boolean) {
+ mProp->value.boolean = true;
+ mProp->migratable = VIR_TRISTATE_BOOL_NO;
+ }
+ }
+
+ tmp->migratability = true;
+
+ done:
+ VIR_STEAL_PTR(*augmented, tmp);
+ ret = 0;
+
+ cleanup:
+ qemuMonitorCPUModelInfoFree(tmp);
+ virHashFree(hash);
+ return ret;
+}
static int
virQEMUCapsProbeQMPHostCPU(virQEMUCapsPtr qemuCaps,
qemuMonitorPtr mon,
bool tcg)
{
- qemuMonitorCPUModelInfoPtr modelInfo = NULL;
+ qemuMonitorCPUModelInfoPtr input;
+ qemuMonitorCPUModelInfoPtr migratable = NULL;
qemuMonitorCPUModelInfoPtr nonMigratable = NULL;
- virHashTablePtr hash = NULL;
+ qemuMonitorCPUModelInfoPtr augmented = NULL;
const char *model;
qemuMonitorCPUModelExpansionType type;
virDomainVirtType virtType;
@@ -2380,6 +2447,8 @@ virQEMUCapsProbeQMPHostCPU(virQEMUCapsPtr qemuCaps,
cpuData = virQEMUCapsGetHostCPUData(qemuCaps, virtType);
+ cpuData->info = NULL;
+
/* Some x86_64 features defined in cpu_map.xml use spelling which differ
* from the one preferred by QEMU. Static expansion would give us only the
* preferred spelling, thus we need to do a full expansion on the result of
@@ -2390,54 +2459,30 @@ virQEMUCapsProbeQMPHostCPU(virQEMUCapsPtr qemuCaps,
else
type = QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC;
- if (qemuMonitorGetCPUModelExpansion(mon, type, model, true, &modelInfo) < 0)
+ if (!(input = qemuMonitorCPUModelInfoNew(model)) ||
+ qemuMonitorGetCPUModelExpansion(mon, type, true, input, &migratable) < 0)
goto cleanup;
- /* Try to check migratability of each feature. */
- if (modelInfo &&
- qemuMonitorGetCPUModelExpansion(mon, type, model, false,
- &nonMigratable) < 0)
+ if (!migratable) {
+ ret = 0; /* Qemu can't expand the model name, exit without error */
goto cleanup;
+ }
- if (nonMigratable) {
- qemuMonitorCPUPropertyPtr prop;
- qemuMonitorCPUPropertyPtr nmProp;
- size_t i;
-
- if (!(hash = virHashCreate(0, NULL)))
- goto cleanup;
-
- for (i = 0; i < modelInfo->nprops; i++) {
- prop = modelInfo->props + i;
- if (virHashAddEntry(hash, prop->name, prop) < 0)
- goto cleanup;
- }
-
- for (i = 0; i < nonMigratable->nprops; i++) {
- nmProp = nonMigratable->props + i;
- if (!(prop = virHashLookup(hash, nmProp->name)) ||
- prop->type != QEMU_MONITOR_CPU_PROPERTY_BOOLEAN ||
- prop->type != nmProp->type)
- continue;
-
- if (prop->value.boolean) {
- prop->migratable = VIR_TRISTATE_BOOL_YES;
- } else if (nmProp->value.boolean) {
- prop->value.boolean = true;
- prop->migratable = VIR_TRISTATE_BOOL_NO;
- }
- }
+ /* Try to check migratability of each feature. */
+ if (qemuMonitorGetCPUModelExpansion(mon, type, false, input, &nonMigratable) <
0)
+ goto cleanup;
- modelInfo->migratability = true;
- }
+ if (virQEMUCapsMigratablePropsDiff(migratable, nonMigratable, &augmented) <
0)
+ goto cleanup;
- VIR_STEAL_PTR(cpuData->info, modelInfo);
+ VIR_STEAL_PTR(cpuData->info, augmented);
ret = 0;
cleanup:
- virHashFree(hash);
+ qemuMonitorCPUModelInfoFree(input);
+ qemuMonitorCPUModelInfoFree(migratable);
qemuMonitorCPUModelInfoFree(nonMigratable);
- qemuMonitorCPUModelInfoFree(modelInfo);
+ qemuMonitorCPUModelInfoFree(augmented);
return ret;
}
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 801c072eff..c64b3ad38a 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -3653,20 +3653,57 @@ qemuMonitorCPUDefInfoFree(qemuMonitorCPUDefInfoPtr cpu)
}
+/**
+ * qemuMonitorGetCPUModelExpansion:
+ * @mon:
+ * @type: qemuMonitorCPUModelExpansionType
+ * @migratable: Prompt QEMU to include non-migratable features for X86 models if false.
+ * @input: Non-expanded input model
+ * @expansion: Expanded output model (or NULL if QEMU rejects model / request)
+ *
+ * CPU property lists are computed from CPUModelInfo structures by 1) converting
+ * model->name into a property list using a static lookup table and 2) adding or
+ * removing properties in the resulting list from model->props.
+ *
+ * This function uses QEMU to identify a base model name that most closely
+ * matches the migratable CPU properties in the input CPU Model.
+ *
+ * This function also uses QEMU to enumerate model->props in various ways based
+ * on the qemuMonitorCPUModelExpansionType.
+ *
+ * full_input_props = LookupProps(input->name) then +/- input->props
+ *
+ * migratable_input_props = full_input_props - non_migratable_input_props
+ *
+ * base_model = FindClosestBaseModel(migratable_input_props)
+ *
+ * expansion->name = base_model->name
+ *
+ * @type == QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC
+ * expansion->props = props to +/- to base_model to approximate
migratable_input_props
+ *
+ * @type == QEMU_MONITOR_CPU_MODEL_EXPANSION_FULL
+ * expansion->props = full_input_props
+ *
+ * @type == QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC_FULL
+ * expansion->props = migratable_input_props
+ *
+ * Returns 0 in case of success, -1 in case of failure
+ */
int
qemuMonitorGetCPUModelExpansion(qemuMonitorPtr mon,
qemuMonitorCPUModelExpansionType type,
- const char *model_name,
bool migratable,
- qemuMonitorCPUModelInfoPtr *model_info)
+ qemuMonitorCPUModelInfoPtr input,
+ qemuMonitorCPUModelInfoPtr *expansion
+ )
{
VIR_DEBUG("type=%d model_name=%s migratable=%d",
- type, model_name, migratable);
+ type, input->name, migratable);
QEMU_CHECK_MONITOR(mon);
- return qemuMonitorJSONGetCPUModelExpansion(mon, type, model_name,
- migratable, model_info);
+ return qemuMonitorJSONGetCPUModelExpansion(mon, type, migratable, input, expansion);
}
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 0a09590ed1..d3efd37099 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -1036,9 +1036,10 @@ typedef enum {
int qemuMonitorGetCPUModelExpansion(qemuMonitorPtr mon,
qemuMonitorCPUModelExpansionType type,
- const char *model_name,
bool migratable,
- qemuMonitorCPUModelInfoPtr *model_info);
+ qemuMonitorCPUModelInfoPtr input,
+ qemuMonitorCPUModelInfoPtr *expansion)
+ ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(5);
void qemuMonitorCPUModelInfoFree(qemuMonitorCPUModelInfoPtr model_info);
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index f20e9e9379..4e6e220a8c 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -5600,12 +5600,17 @@ qemuMonitorJSONBuildCPUModelInfoFromJSON(virJSONValuePtr
cpu_model)
return ret;
}
+
+/* return:
+ * -1 - Execution Failure
+ * 0 - Success
+ */
int
qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon,
qemuMonitorCPUModelExpansionType type,
- const char *model_name,
bool migratable,
- qemuMonitorCPUModelInfoPtr *model_info)
+ qemuMonitorCPUModelInfoPtr input,
+ qemuMonitorCPUModelInfoPtr *expansion)
{
int ret = -1;
virJSONValuePtr json_model_in = NULL;
@@ -5613,12 +5618,13 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon,
virJSONValuePtr reply = NULL;
virJSONValuePtr data;
virJSONValuePtr cpu_model;
+ qemuMonitorCPUModelInfoPtr expanded_model = NULL;
qemuMonitorCPUModelInfoPtr model_in = NULL;
const char *typeStr = "";
- *model_info = NULL;
+ *expansion = NULL;
- if (!(model_in = qemuMonitorCPUModelInfoNew(model_name)))
+ if (!(model_in = qemuMonitorCPUModelInfoCopy(input)))
goto cleanup;
if (!migratable &&
@@ -5684,13 +5690,17 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon,
goto retry;
}
- if (!(*model_info = qemuMonitorJSONBuildCPUModelInfoFromJSON(cpu_model)))
+ if (!(expanded_model = qemuMonitorJSONBuildCPUModelInfoFromJSON(cpu_model)))
goto cleanup;
+ VIR_STEAL_PTR(*expansion, expanded_model);
ret = 0;
cleanup:
+ VIR_FREE(expanded_model); /* Free structure but not reused contents */
qemuMonitorCPUModelInfoFree(model_in);
+ qemuMonitorCPUModelInfoFree(expanded_model);
+
virJSONValueFree(cmd);
virJSONValueFree(reply);
virJSONValueFree(json_model_in);
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index da267b15b0..2d5679094e 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -367,10 +367,10 @@ int qemuMonitorJSONGetCPUDefinitions(qemuMonitorPtr mon,
int qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon,
qemuMonitorCPUModelExpansionType type,
- const char *model_name,
bool migratable,
- qemuMonitorCPUModelInfoPtr *model_info)
- ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(5);
+ qemuMonitorCPUModelInfoPtr input,
+ qemuMonitorCPUModelInfoPtr *expansion)
+ ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(5);
int qemuMonitorJSONGetCommands(qemuMonitorPtr mon,
char ***commands)
diff --git a/tests/cputest.c b/tests/cputest.c
index 339119c63f..c438a8d09e 100644
--- a/tests/cputest.c
+++ b/tests/cputest.c
@@ -483,6 +483,7 @@ cpuTestMakeQEMUCaps(const struct data *data)
virQEMUCapsPtr qemuCaps = NULL;
qemuMonitorTestPtr testMon = NULL;
qemuMonitorCPUModelInfoPtr model = NULL;
+ qemuMonitorCPUModelInfoPtr expansion = NULL;
char *json = NULL;
if (virAsprintf(&json, "%s/cputestdata/%s-cpuid-%s.json",
@@ -492,9 +493,12 @@ cpuTestMakeQEMUCaps(const struct data *data)
if (!(testMon = qemuMonitorTestNewFromFile(json, driver.xmlopt, true)))
goto error;
+ if (!(model = qemuMonitorCPUModelInfoNew("host")))
+ goto cleanup;
+
if (qemuMonitorGetCPUModelExpansion(qemuMonitorTestGetMonitor(testMon),
QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC,
- "host", true, &model) < 0)
+ true, model, &expansion) < 0)
goto error;
if (!(qemuCaps = virQEMUCapsNew()))
@@ -506,8 +510,8 @@ cpuTestMakeQEMUCaps(const struct data *data)
virQEMUCapsSet(qemuCaps, QEMU_CAPS_QUERY_CPU_DEFINITIONS);
virQEMUCapsSetArch(qemuCaps, data->arch);
- virQEMUCapsSetCPUModelInfo(qemuCaps, VIR_DOMAIN_VIRT_KVM, model);
- model = NULL;
+ virQEMUCapsSetCPUModelInfo(qemuCaps, VIR_DOMAIN_VIRT_KVM, expansion);
+ expansion = NULL;
if (virQEMUCapsProbeQMPCPUDefinitions(qemuCaps,
qemuMonitorTestGetMonitor(testMon),
@@ -516,6 +520,7 @@ cpuTestMakeQEMUCaps(const struct data *data)
cleanup:
qemuMonitorCPUModelInfoFree(model);
+ qemuMonitorCPUModelInfoFree(expansion);
qemuMonitorTestFree(testMon);
VIR_FREE(json);
--
2.17.1
8:33 a.m.
New subject: [libvirt] [PATCH v3 3/6] qemu_monitor: qemuMonitorGetCPUModelExpansion inputs and outputs CPUModelInfo
On Thu, Sep 27, 2018 at 16:26:42 -0500, Chris Venteicher wrote:
A Full CPUModelInfo structure with props is sent to QEMU for
expansion.
virQEMUCapsProbeQMPHostCPU migratability logic partitioned into new function
for clarity.
Unrelated changes should go into separate patches. Please, split this
patch in two. Mixing several separable changes in a single patch makes
it harder to review.
Signed-off-by: Chris Venteicher <cventeic(a)redhat.com>
---
src/qemu/qemu_capabilities.c | 125 ++++++++++++++++++++++++-----------
src/qemu/qemu_monitor.c | 47 +++++++++++--
src/qemu/qemu_monitor.h | 5 +-
src/qemu/qemu_monitor_json.c | 20 ++++--
src/qemu/qemu_monitor_json.h | 6 +-
tests/cputest.c | 11 ++-
6 files changed, 156 insertions(+), 58 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index e60a4b369e..d38530ca80 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -2352,15 +2352,82 @@ virQEMUCapsProbeQMPCPUDefinitions(virQEMUCapsPtr qemuCaps,
return 0;
}
+/* virQEMUCapsMigratablePropsDiff
+ * @migratable: migratable props=true, non-migratable & unsupported props=false
+ * @nonMigratable: migratable & non-migratable props = true, unsupported props =
false
+ * @augmented: prop->migratable = VIR_TRISTATE_BOOL_{YES/NO} base on diff
+ *
+ * Use differences in Expanded CPUModelInfo inputs
+ * to augment with prop->migratable in CPUModelInfo output
Could you use normal language to describe the parameters and what the
function is supposed to do. Having to read the documentation over and
over and thinking hard about it to grasp what the function is supposed
to do is even worse than studying the code itself.
The function and parameters names look like they give better idea about
what's going on here than the comments which are supposed to explain
them.
+ */
+static int
+virQEMUCapsMigratablePropsDiff(qemuMonitorCPUModelInfoPtr migratable,
+ qemuMonitorCPUModelInfoPtr nonMigratable,
+ qemuMonitorCPUModelInfoPtr *augmented)
...
cleanup:
- virHashFree(hash);
+ qemuMonitorCPUModelInfoFree(input);
+ qemuMonitorCPUModelInfoFree(migratable);
qemuMonitorCPUModelInfoFree(nonMigratable);
- qemuMonitorCPUModelInfoFree(modelInfo);
+ qemuMonitorCPUModelInfoFree(augmented);
return ret;
}
This is where the first patch would end. Although you'd need to change
it a bit to account for the missing changes which will go in later as a
second patch.
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 801c072eff..c64b3ad38a 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -3653,20 +3653,57 @@ qemuMonitorCPUDefInfoFree(qemuMonitorCPUDefInfoPtr cpu)
}
+/**
+ * qemuMonitorGetCPUModelExpansion:
+ * @mon:
+ * @type: qemuMonitorCPUModelExpansionType
+ * @migratable: Prompt QEMU to include non-migratable features for X86 models if false.
+ * @input: Non-expanded input model
+ * @expansion: Expanded output model (or NULL if QEMU rejects model / request)
+ *
+ * CPU property lists are computed from CPUModelInfo structures by 1) converting
+ * model->name into a property list using a static lookup table and 2) adding or
+ * removing properties in the resulting list from model->props.
+ *
+ * This function uses QEMU to identify a base model name that most closely
+ * matches the migratable CPU properties in the input CPU Model.
+ *
+ * This function also uses QEMU to enumerate model->props in various ways based
+ * on the qemuMonitorCPUModelExpansionType.
+ *
+ * full_input_props = LookupProps(input->name) then +/- input->props
+ *
+ * migratable_input_props = full_input_props - non_migratable_input_props
+ *
+ * base_model = FindClosestBaseModel(migratable_input_props)
+ *
+ * expansion->name = base_model->name
+ *
+ * @type == QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC
+ * expansion->props = props to +/- to base_model to approximate
migratable_input_props
+ *
+ * @type == QEMU_MONITOR_CPU_MODEL_EXPANSION_FULL
+ * expansion->props = full_input_props
+ *
+ * @type == QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC_FULL
+ * expansion->props = migratable_input_props
Same comment as for the other doc text above. I know what the function
is supposed to do, but still I find it hard to understand what you
wanted to say with all the text above.
+ *
+ * Returns 0 in case of success, -1 in case of failure
+ */
This applies to almost all functions in libvirt. There's no sense in
documenting it unless you add more documentation for that function and
mention return values for completeness.
int
qemuMonitorGetCPUModelExpansion(qemuMonitorPtr mon,
qemuMonitorCPUModelExpansionType type,
- const char *model_name,
bool migratable,
- qemuMonitorCPUModelInfoPtr *model_info)
+ qemuMonitorCPUModelInfoPtr input,
+ qemuMonitorCPUModelInfoPtr *expansion
+ )
{
VIR_DEBUG("type=%d model_name=%s migratable=%d",
- type, model_name, migratable);
+ type, input->name, migratable);
QEMU_CHECK_MONITOR(mon);
- return qemuMonitorJSONGetCPUModelExpansion(mon, type, model_name,
- migratable, model_info);
+ return qemuMonitorJSONGetCPUModelExpansion(mon, type, migratable, input,
expansion);
}
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 0a09590ed1..d3efd37099 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -1036,9 +1036,10 @@ typedef enum {
int qemuMonitorGetCPUModelExpansion(qemuMonitorPtr mon,
qemuMonitorCPUModelExpansionType type,
- const char *model_name,
bool migratable,
- qemuMonitorCPUModelInfoPtr *model_info);
+ qemuMonitorCPUModelInfoPtr input,
+ qemuMonitorCPUModelInfoPtr *expansion)
+ ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(5);
void qemuMonitorCPUModelInfoFree(qemuMonitorCPUModelInfoPtr model_info);
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index f20e9e9379..4e6e220a8c 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -5600,12 +5600,17 @@ qemuMonitorJSONBuildCPUModelInfoFromJSON(virJSONValuePtr
cpu_model)
return ret;
}
+
+/* return:
+ * -1 - Execution Failure
+ * 0 - Success
+ */
This applies to almost all functions in libvirt. There's no sense in
documenting it unless you add more documentation for that function and
mention return values for completeness.
int
qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon,
qemuMonitorCPUModelExpansionType type,
- const char *model_name,
bool migratable,
- qemuMonitorCPUModelInfoPtr *model_info)
+ qemuMonitorCPUModelInfoPtr input,
+ qemuMonitorCPUModelInfoPtr *expansion)
{
int ret = -1;
virJSONValuePtr json_model_in = NULL;
Jirka
4:26 p.m.
New subject: [libvirt] [PATCH v3 4/6] qemu_process: Use common processes mgmt funcs for all QMP query types
Generalized QEMU process management functions supporting all forms of
QMP queries including but no limited to capabilities queries.
QEMU process instances can be maintained and used for multiple different QMP
queries, of the same or different types, as required to complete a
specific libvirt task.
Support concurrent QEMU process instances for QMP queries,
using the same or different qemu binaries.
All process mgmt functions are removed from qemu_capabilities and
re-implemented in qemu_process in a generic, non capabilities specific,
manner consistent with existing qemu_process mgmt functions.
Concept of qmp_query domain is used to contain process state through the
an entire process lifecycle similar to how a VM domains contain process
state in existing process functions.
Monitor callbacks were not used in original process management code for
capabilities and are eliminated in new generalized process code for QMP
queries.
QMP exchange to exit capabilities negotiation mode and enter command mode
is the sole responsibility of the QMP query process code.
Signed-off-by: Chris Venteicher <cventeic(a)redhat.com>
---
src/qemu/qemu_capabilities.c | 304 +++++---------------------
src/qemu/qemu_monitor.c | 4 +-
src/qemu/qemu_process.c | 398 +++++++++++++++++++++++++++++++++++
src/qemu/qemu_process.h | 35 +++
tests/qemucapabilitiestest.c | 7 +
5 files changed, 489 insertions(+), 259 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index d38530ca80..5b1e4f1bbd 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -47,6 +47,7 @@
#define __QEMU_CAPSPRIV_H_ALLOW__
#include "qemu_capspriv.h"
#include "qemu_qapi.h"
+#include "qemu_process.h"
#include <fcntl.h>
#include <sys/stat.h>
@@ -3960,18 +3961,6 @@ virQEMUCapsIsValid(void *data,
}
-static void virQEMUCapsMonitorNotify(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
- virDomainObjPtr vm ATTRIBUTE_UNUSED,
- void *opaque ATTRIBUTE_UNUSED)
-{
-}
-
-static qemuMonitorCallbacks callbacks = {
- .eofNotify = virQEMUCapsMonitorNotify,
- .errorNotify = virQEMUCapsMonitorNotify,
-};
-
-
/**
* virQEMUCapsInitQMPArch:
* @qemuCaps: QEMU capabilities
@@ -4067,13 +4056,6 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps,
/* @mon is supposed to be locked by callee */
- if (qemuMonitorSetCapabilities(mon) < 0) {
- VIR_DEBUG("Failed to set monitor capabilities %s",
- virGetLastErrorMessage());
- ret = 0;
- goto cleanup;
- }
-
if (qemuMonitorGetVersion(mon,
&major, &minor, µ,
&package) < 0) {
@@ -4247,13 +4229,6 @@ virQEMUCapsInitQMPMonitorTCG(virQEMUCapsPtr qemuCaps
ATTRIBUTE_UNUSED,
{
int ret = -1;
- if (qemuMonitorSetCapabilities(mon) < 0) {
- VIR_DEBUG("Failed to set monitor capabilities %s",
- virGetLastErrorMessage());
- ret = 0;
- goto cleanup;
- }
-
if (virQEMUCapsProbeQMPCPUDefinitions(qemuCaps, mon, true) < 0)
goto cleanup;
@@ -4266,251 +4241,75 @@ virQEMUCapsInitQMPMonitorTCG(virQEMUCapsPtr qemuCaps
ATTRIBUTE_UNUSED,
}
-typedef struct _virQEMUCapsInitQMPCommand virQEMUCapsInitQMPCommand;
-typedef virQEMUCapsInitQMPCommand *virQEMUCapsInitQMPCommandPtr;
-struct _virQEMUCapsInitQMPCommand {
- char *binary;
- uid_t runUid;
- gid_t runGid;
- char **qmperr;
- char *monarg;
- char *monpath;
- char *pidfile;
- virCommandPtr cmd;
- qemuMonitorPtr mon;
- virDomainChrSourceDef config;
- pid_t pid;
- virDomainObjPtr vm;
-};
-
-
-static void
-virQEMUCapsInitQMPCommandAbort(virQEMUCapsInitQMPCommandPtr cmd)
-{
- if (cmd->mon)
- virObjectUnlock(cmd->mon);
- qemuMonitorClose(cmd->mon);
- cmd->mon = NULL;
-
- virCommandAbort(cmd->cmd);
- virCommandFree(cmd->cmd);
- cmd->cmd = NULL;
-
- if (cmd->monpath)
- unlink(cmd->monpath);
-
- virDomainObjEndAPI(&cmd->vm);
-
- if (cmd->pid != 0) {
- char ebuf[1024];
-
- VIR_DEBUG("Killing QMP caps process %lld", (long long)cmd->pid);
- if (virProcessKill(cmd->pid, SIGKILL) < 0 && errno != ESRCH)
- VIR_ERROR(_("Failed to kill process %lld: %s"),
- (long long)cmd->pid,
- virStrerror(errno, ebuf, sizeof(ebuf)));
-
- VIR_FREE(*cmd->qmperr);
- }
- if (cmd->pidfile)
- unlink(cmd->pidfile);
- cmd->pid = 0;
-}
-
-
-static void
-virQEMUCapsInitQMPCommandFree(virQEMUCapsInitQMPCommandPtr cmd)
-{
- if (!cmd)
- return;
-
- virQEMUCapsInitQMPCommandAbort(cmd);
- VIR_FREE(cmd->binary);
- VIR_FREE(cmd->monpath);
- VIR_FREE(cmd->monarg);
- VIR_FREE(cmd->pidfile);
- VIR_FREE(cmd);
-}
-
-
-static virQEMUCapsInitQMPCommandPtr
-virQEMUCapsInitQMPCommandNew(char *binary,
- const char *libDir,
- uid_t runUid,
- gid_t runGid,
- char **qmperr)
-{
- virQEMUCapsInitQMPCommandPtr cmd = NULL;
-
- if (VIR_ALLOC(cmd) < 0)
- goto error;
-
- if (VIR_STRDUP(cmd->binary, binary) < 0)
- goto error;
-
- cmd->runUid = runUid;
- cmd->runGid = runGid;
- cmd->qmperr = qmperr;
-
- /* the ".sock" sufix is important to avoid a possible clash with a qemu
- * domain called "capabilities"
- */
- if (virAsprintf(&cmd->monpath, "%s/%s", libDir,
- "capabilities.monitor.sock") < 0)
- goto error;
- if (virAsprintf(&cmd->monarg, "unix:%s,server,nowait",
cmd->monpath) < 0)
- goto error;
-
- /* ".pidfile" suffix is used rather than ".pid" to avoid a
possible clash
- * with a qemu domain called "capabilities"
- * Normally we'd use runDir for pid files, but because we're using
- * -daemonize we need QEMU to be allowed to create them, rather
- * than libvirtd. So we're using libDir which QEMU can write to
- */
- if (virAsprintf(&cmd->pidfile, "%s/%s", libDir,
"capabilities.pidfile") < 0)
- goto error;
-
- virPidFileForceCleanupPath(cmd->pidfile);
-
- cmd->config.type = VIR_DOMAIN_CHR_TYPE_UNIX;
- cmd->config.data.nix.path = cmd->monpath;
- cmd->config.data.nix.listen = false;
-
- return cmd;
-
- error:
- virQEMUCapsInitQMPCommandFree(cmd);
- return NULL;
-}
-
-
-/* Returns -1 on fatal error,
- * 0 on success,
- * 1 when probing QEMU failed
- */
static int
-virQEMUCapsInitQMPCommandRun(virQEMUCapsInitQMPCommandPtr cmd,
- bool forceTCG)
+virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
+ const char *libDir,
+ uid_t runUid,
+ gid_t runGid)
{
- virDomainXMLOptionPtr xmlopt = NULL;
- const char *machine;
- int status = 0;
int ret = -1;
+ virDomainObjPtr dom = NULL;
+ virDomainObjPtr dom_kvm = NULL;
+ qemuMonitorPtr mon = NULL;
+ qemuMonitorPtr mon_kvm = NULL;
+ bool forceTCG = false;
- if (forceTCG)
- machine = "none,accel=tcg";
- else
- machine = "none,accel=kvm:tcg";
-
- VIR_DEBUG("Try to probe capabilities of '%s' via QMP, machine %s",
- cmd->binary, machine);
+ VIR_DEBUG("qemuCaps->binary=%s, libDir=%s, runUid=%u, runGid=%u ",
+ NULLSTR(qemuCaps->binary), NULLSTR(libDir), runUid, runGid);
- /*
- * We explicitly need to use -daemonize here, rather than
- * virCommandDaemonize, because we need to synchronize
- * with QEMU creating its monitor socket API. Using
- * daemonize guarantees control won't return to libvirt
- * until the socket is present.
- */
- cmd->cmd = virCommandNewArgList(cmd->binary,
- "-S",
- "-no-user-config",
- "-nodefaults",
- "-nographic",
- "-machine", machine,
- "-qmp", cmd->monarg,
- "-pidfile", cmd->pidfile,
- "-daemonize",
- NULL);
- virCommandAddEnvPassCommon(cmd->cmd);
- virCommandClearCaps(cmd->cmd);
- virCommandSetGID(cmd->cmd, cmd->runGid);
- virCommandSetUID(cmd->cmd, cmd->runUid);
-
- virCommandSetErrorBuffer(cmd->cmd, cmd->qmperr);
-
- /* Log, but otherwise ignore, non-zero status. */
- if (virCommandRun(cmd->cmd, &status) < 0)
+ if (!(dom = qmpDomainObjNew(qemuCaps->binary, forceTCG, libDir, runUid, runGid)))
goto cleanup;
- if (status != 0) {
- VIR_DEBUG("QEMU %s exited with status %d: %s",
- cmd->binary, status, *cmd->qmperr);
- goto ignore;
- }
+ if (qemuProcessStartQmp(dom) < 0)
+ goto cleanup;
- if (virPidFileReadPath(cmd->pidfile, &cmd->pid) < 0) {
- VIR_DEBUG("Failed to read pidfile %s", cmd->pidfile);
- goto ignore;
+ if (!(mon = QMP_DOMAIN_MONITOR(dom))) {
+ ret = 0; /* Failure probing QEMU not considered error case */
+ goto cleanup;
}
- if (!(xmlopt = virDomainXMLOptionNew(NULL, NULL, NULL, NULL, NULL)) ||
- !(cmd->vm = virDomainObjNew(xmlopt)))
+ /* Pull capabilities from QEMU */
+ if (virQEMUCapsInitQMPMonitor(qemuCaps, mon) < 0)
goto cleanup;
- cmd->vm->pid = cmd->pid;
-
- if (!(cmd->mon = qemuMonitorOpen(cmd->vm, &cmd->config, true, true,
- 0, &callbacks, NULL)))
- goto ignore;
-
- virObjectLock(cmd->mon);
-
- ret = 0;
-
- cleanup:
- if (!cmd->mon)
- virQEMUCapsInitQMPCommandAbort(cmd);
- virObjectUnref(xmlopt);
-
- return ret;
-
- ignore:
- ret = 1;
- goto cleanup;
-}
-
-
-static int
-virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
- const char *libDir,
- uid_t runUid,
- gid_t runGid,
- char **qmperr)
-{
- virQEMUCapsInitQMPCommandPtr cmd = NULL;
- int ret = -1;
- int rc;
+ /* Pull capabilities again if KVM supported */
+ if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM)) {
- if (!(cmd = virQEMUCapsInitQMPCommandNew(qemuCaps->binary, libDir,
- runUid, runGid, qmperr)))
- goto cleanup;
+ qemuProcessStopQmp(dom);
+ virDomainObjEndAPI(&dom);
- if ((rc = virQEMUCapsInitQMPCommandRun(cmd, false)) != 0) {
- if (rc == 1)
- ret = 0;
- goto cleanup;
- }
+ forceTCG = true;
+ dom_kvm = qmpDomainObjNew(qemuCaps->binary, forceTCG, libDir, runUid,
runGid);
- if (virQEMUCapsInitQMPMonitor(qemuCaps, cmd->mon) < 0)
- goto cleanup;
+ if (qemuProcessStartQmp(dom_kvm) < 0)
+ goto cleanup;
- if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM)) {
- virQEMUCapsInitQMPCommandAbort(cmd);
- if ((rc = virQEMUCapsInitQMPCommandRun(cmd, true)) != 0) {
- if (rc == 1)
- ret = 0;
+ if (!(mon_kvm = QMP_DOMAIN_MONITOR(dom_kvm))) {
+ ret = 0; /* Failure probing QEMU not considered error case */
goto cleanup;
}
- if (virQEMUCapsInitQMPMonitorTCG(qemuCaps, cmd->mon) < 0)
+ if (virQEMUCapsInitQMPMonitorTCG(qemuCaps, mon_kvm) < 0)
goto cleanup;
}
ret = 0;
cleanup:
- virQEMUCapsInitQMPCommandFree(cmd);
+ if (!mon) {
+ char *err = QMP_DOMAIN_ERROR(dom) ? QMP_DOMAIN_ERROR(dom) : _("unknown
error");
+
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Failed to probe QEMU binary with QMP: %s"), err);
+ }
+
+ qemuProcessStopQmp(dom);
+ virDomainObjEndAPI(&dom);
+ qemuProcessStopQmp(dom_kvm);
+ virDomainObjEndAPI(&dom_kvm);
+
+ VIR_DEBUG("ret=%i", ret);
+
return ret;
}
@@ -4546,7 +4345,6 @@ virQEMUCapsNewForBinaryInternal(virArch hostArch,
{
virQEMUCapsPtr qemuCaps;
struct stat sb;
- char *qmperr = NULL;
if (!(qemuCaps = virQEMUCapsNew()))
goto error;
@@ -4573,15 +4371,8 @@ virQEMUCapsNewForBinaryInternal(virArch hostArch,
goto error;
}
- if (virQEMUCapsInitQMP(qemuCaps, libDir, runUid, runGid, &qmperr) < 0) {
- virQEMUCapsLogProbeFailure(binary);
- goto error;
- }
-
- if (!qemuCaps->usedQMP) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Failed to probe QEMU binary with QMP: %s"),
- qmperr ? qmperr : _("unknown error"));
+ if (virQEMUCapsInitQMP(qemuCaps, libDir, runUid, runGid) < 0 ||
+ !qemuCaps->usedQMP) {
virQEMUCapsLogProbeFailure(binary);
goto error;
}
@@ -4600,7 +4391,6 @@ virQEMUCapsNewForBinaryInternal(virArch hostArch,
}
cleanup:
- VIR_FREE(qmperr);
return qemuCaps;
error:
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index c64b3ad38a..507710d241 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -813,12 +813,12 @@ qemuMonitorOpenInternal(virDomainObjPtr vm,
{
qemuMonitorPtr mon;
- if (!cb->eofNotify) {
+ if (cb && !cb->eofNotify) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("EOF notify callback must be supplied"));
return NULL;
}
- if (!cb->errorNotify) {
+ if (cb && !cb->errorNotify) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Error notify callback must be supplied"));
return NULL;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 29b0ba1590..c8a34fc37e 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8068,3 +8068,401 @@ qemuProcessReconnectAll(virQEMUDriverPtr driver)
struct qemuProcessReconnectData data = {.driver = driver};
virDomainObjListForEach(driver->domains, qemuProcessReconnectHelper, &data);
}
+
+
+static void *
+qmpDomainObjPrivateAlloc(void *opaque)
+{
+ qmpDomainObjPrivatePtr priv;
+
+ if (VIR_ALLOC(priv) < 0)
+ return NULL;
+
+ VIR_DEBUG("priv=%p opaque=%p", priv, opaque);
+
+ return priv;
+}
+
+
+static void
+qmpDomainObjPrivateFree(void *data)
+{
+ qmpDomainObjPrivatePtr priv = data;
+
+ VIR_DEBUG("priv=%p, priv->mon=%p", priv, (priv ? priv->mon : NULL));
+
+ if (!priv)
+ return;
+
+ /* This should never be non-NULL if we get here, but just in case... */
+ if (priv->mon) {
+ VIR_ERROR(_("Unexpected QEMU monitor still active during domain
deletion"));
+ qemuMonitorClose(priv->mon);
+ priv->mon = NULL;
+ }
+
+ VIR_FREE(priv->libDir);
+ VIR_FREE(priv->monpath);
+ VIR_FREE(priv->monarg);
+ VIR_FREE(priv->pidfile);
+ VIR_FREE(priv->qmperr);
+ VIR_FREE(priv);
+}
+
+/**
+ * qmpDomainObjNew:
+ * @binary: Qemu binary
+ * @forceTCG: Force TCG mode if true
+ * @libDir: Directory for process and connection artifacts
+ * @runUid: UserId for Qemu Process
+ * @runGid: GroupId for Qemu Process
+ *
+ * Allocate and initialize domain structure encapsulating
+ * QEMU Process state and monitor connection to QEMU
+ * for completing QMP Queries.
+ */
+virDomainObjPtr
+qmpDomainObjNew(const char *binary,
+ bool forceTCG,
+ const char *libDir,
+ uid_t runUid, gid_t runGid)
+{
+ virDomainXMLOptionPtr xmlopt = NULL;
+ virDomainObjPtr ret = NULL;
+ virDomainObjPtr dom = NULL;
+ qmpDomainObjPrivatePtr priv = NULL;
+
+ virDomainXMLPrivateDataCallbacks callbacks = {
+ .alloc = qmpDomainObjPrivateAlloc,
+ .free = qmpDomainObjPrivateFree,
+ };
+
+ VIR_DEBUG("exec=%s, libDir=%s, runUid=%u, runGid=%u ",
+ NULLSTR(binary), NULLSTR(libDir), runUid, runGid);
+
+ xmlopt = virDomainXMLOptionNew(NULL,
+ &callbacks,
+ NULL, NULL, NULL);
+
+ if (!xmlopt ||
+ !(dom = virDomainObjNew(xmlopt)))
+ goto cleanup;
+
+ if (VIR_ALLOC(dom->def) < 0 ||
+ VIR_STRDUP(dom->def->name, "QmpQuery") < 0 ||
+ VIR_STRDUP(dom->def->emulator, binary) < 0)
+ goto cleanup;
+
+ priv = QMP_DOMAIN_PRIVATE(dom);
+
+ priv->forceTCG = forceTCG;
+
+ if (VIR_STRDUP(priv->libDir, libDir) < 0)
+ goto cleanup;
+
+ priv->runUid = runUid;
+ priv->runGid = runGid;
+
+ VIR_STEAL_PTR(ret, dom);
+
+ cleanup:
+ virDomainObjEndAPI(&dom);
+ return ret;
+}
+
+
+/* Initialize configuration and paths prior to starting QEMU
+ */
+static int
+qemuProcessInitQmp(virDomainObjPtr dom)
+{
+ int ret = -1;
+ qmpDomainObjPrivatePtr priv = QMP_DOMAIN_PRIVATE(dom);
+
+ /* Allow multiple QEMU Procs and make clashes very unlikely */
+ unsigned int seed = time(NULL);
+ dom->def->id = rand_r(&seed) % 1000000;
+
+ VIR_DEBUG("Beginning VM startup process"
+ "dom=%p name=%s emulator=%s, id=%d",
+ dom, dom->def->name, dom->def->emulator, dom->def->id);
+
+ /* the ".sock" sufix is important to avoid a possible clash with a qemu
+ * domain called "capabilities"
+ */
+ if (virAsprintf(&priv->monpath, "%s/%s.%d.%s",
+ priv->libDir, dom->def->name, dom->def->id,
+ "monitor.sock") < 0)
+ goto cleanup;
+
+ if (virAsprintf(&priv->monarg, "unix:%s,server,nowait",
priv->monpath) < 0)
+ goto cleanup;
+
+ /* Normally we'd use runDir for pid files, but because we're using
+ * -daemonize we need QEMU to be allowed to create them, rather
+ * than libvirtd. So we're using libDir which QEMU can write to
+ */
+ if (virAsprintf(&priv->pidfile, "%s/%s.%d.%s",
+ priv->libDir, dom->def->name, dom->def->id,
+ "pidfile") < 0)
+ goto cleanup;
+
+ virPidFileForceCleanupPath(priv->pidfile);
+
+ ret = 0;
+
+ cleanup:
+ VIR_DEBUG("ret=%i", ret);
+ return ret;
+}
+
+
+/* Launch QEMU Process
+ *
+ * Returns -1 on fatal error,
+ * 0 on success,
+ * 1 when probing QEMU failed
+ */
+static int
+qemuProcessLaunchQmp(virDomainObjPtr dom)
+{
+ const char *machine;
+ int status = 0;
+ int ret = -1;
+
+ qmpDomainObjPrivatePtr priv = QMP_DOMAIN_PRIVATE(dom);
+
+ VIR_DEBUG("dom=%p, emulator=%s, id=%d",
+ dom, NULLSTR(dom->def->emulator), dom->def->id);
+
+ if (priv->forceTCG) {
+ machine = "none,accel=tcg";
+ } else {
+ machine = "none,accel=kvm:tcg";
+ }
+
+ VIR_DEBUG("Try to probe capabilities of '%s' via QMP, machine %s",
+ NULLSTR(dom->def->emulator), machine);
+
+ /* We explicitly need to use -daemonize here, rather than
+ * virCommandDaemonize, because we need to synchronize
+ * with QEMU creating its monitor socket API. Using
+ * daemonize guarantees control won't return to libvirt
+ * until the socket is present.
+ */
+ priv->cmd = virCommandNewArgList(dom->def->emulator,
+ "-S",
+ "-no-user-config",
+ "-nodefaults",
+ "-nographic",
+ "-machine", machine,
+ "-qmp", priv->monarg,
+ "-pidfile", priv->pidfile,
+ "-daemonize",
+ NULL);
+
+ virCommandAddEnvPassCommon(priv->cmd);
+ virCommandClearCaps(priv->cmd);
+ virCommandSetGID(priv->cmd, priv->runGid);
+ virCommandSetUID(priv->cmd, priv->runUid);
+
+ virCommandSetErrorBuffer(priv->cmd, &(priv->qmperr));
+
+ if (virCommandRun(priv->cmd, &status) < 0) {
+ VIR_DEBUG("QEMU %s dom=%p name=%s failed to spawn",
+ dom->def->emulator, dom, dom->def->name);
+ goto cleanup;
+ }
+
+ /* Log, but otherwise ignore, non-zero status. */
+ if (status != 0) {
+ VIR_DEBUG("QEMU %s exited with status %d: %s",
+ dom->def->emulator, status, priv->qmperr);
+ goto ignore;
+ }
+
+ if (virPidFileReadPath(priv->pidfile, &dom->pid) < 0) {
+ VIR_DEBUG("Failed to read pidfile %s", priv->pidfile);
+ goto ignore;
+ }
+
+ ret = 0;
+
+ cleanup:
+ return ret;
+
+ ignore:
+ ret = 1;
+ goto cleanup;
+}
+
+
+/* Connect Monitor to QEMU Process
+ *
+ * Returns -1 on fatal error,
+ * 0 on success,
+ * 1 when probing QEMU failed
+ */
+static int
+qemuConnectMonitorQmp(virDomainObjPtr dom)
+{
+ int ret = 0;
+
+ qemuMonitorPtr mon = NULL;
+ virDomainChrSourceDef monConfig;
+ unsigned long long timeout = 0;
+ bool retry = true;
+ bool enableJson = true;
+ qemuMonitorCallbacksPtr monCallbacks = NULL;
+
+ qmpDomainObjPrivatePtr priv = QMP_DOMAIN_PRIVATE(dom);
+
+ VIR_DEBUG("emulator=%s, dom->pid=%lld",
+ dom->def->emulator, (long long)dom->pid);
+
+ priv->mon = NULL;
+
+ if (!dom->pid)
+ goto ignore;
+
+ monConfig.type = VIR_DOMAIN_CHR_TYPE_UNIX;
+ monConfig.data.nix.path = priv->monpath;
+ monConfig.data.nix.listen = false;
+
+ /* Hold an extra reference because we can't allow dom to be
+ * deleted until the monitor gets its own reference. */
+ virObjectRef(dom);
+ virObjectUnlock(dom);
+
+ mon = qemuMonitorOpen(dom,
+ &monConfig,
+ enableJson,
+ retry,
+ timeout,
+ monCallbacks,
+ NULL);
+
+ virObjectLock(dom);
+ virObjectUnref(dom);
+
+ if (!mon) {
+ VIR_INFO("Failed to connect monitor for %s", dom->def->name);
+ goto ignore;
+ }
+
+ VIR_STEAL_PTR(priv->mon, mon);
+
+ /* Exit capabilities negotiation mode and enter QEMU command mode
+ * by issuing qmp_capabilities command to QEMU */
+ if (qemuMonitorSetCapabilities(priv->mon) < 0) {
+ VIR_DEBUG("Failed to set monitor capabilities %s",
+ virGetLastErrorMessage());
+ goto ignore;
+ }
+
+ ret = 0;
+
+ cleanup:
+ VIR_DEBUG("ret=%i", ret);
+ return ret;
+
+ ignore:
+ ret = 1;
+ goto cleanup;
+}
+
+
+/**
+ * qemuProcessStartQmp:
+ * @dom: Qmp Specific Domain storing Process and Connection State
+ *
+ * Start and connect to QEMU binary so QMP queries can be made.
+ *
+ * Usage:
+ * dom = qmpDomainObjNew(binary, forceTCG, libDir, runUid, runGid);
+ * qemuProcessStartQmp(dom);
+ * mon = QMP_DOMAIN_MONITOR(dom);
+ * ** Send QMP Queries to QEMU using monitor **
+ * qemuProcessStopQmp(dom);
+ * virDomainObjEndAPI(&dom);
+ *
+ * QEMU probing failure is not an error case (negative return value.)
+ * Check monitor before using.
+ *
+ * Resources remain allocated until qemuProcessStopQmp and virDomainObjEndAPI
+ * are called (even in failure cases.) Error string (dom->priv->qmperr)
+ * remains valid until virDomainObjEndAPI(&dom) is called.
+ */
+int
+qemuProcessStartQmp(virDomainObjPtr dom)
+{
+ int ret = -1;
+
+ VIR_DEBUG("dom=%p name=%s emulator=%s",
+ dom, dom->def->name, dom->def->emulator);
+
+ if (qemuProcessInitQmp(dom) < 0)
+ goto stop;
+
+ if (qemuProcessLaunchQmp(dom) < 0)
+ goto stop;
+
+ if (qemuConnectMonitorQmp(dom) < 0)
+ goto stop;
+
+ ret = 0;
+
+ cleanup:
+ VIR_DEBUG("ret=%i", ret);
+ return ret;
+
+ stop:
+ qemuProcessStopQmp(dom);
+ goto cleanup;
+}
+
+/**
+ * qemuProcessStop:
+ * @dom: Qmp Specific Domain storing Process and Connection State
+ *
+ * Stop Monitor Connection and QEMU Process
+ */
+void qemuProcessStopQmp(virDomainObjPtr dom)
+{
+ if (!dom)
+ return;
+
+ qmpDomainObjPrivatePtr priv = QMP_DOMAIN_PRIVATE(dom);
+
+ VIR_DEBUG("Shutting down dom=%p name=%s emulator=%s id->%d pid=%lld",
+ dom, dom->def->name, dom->def->emulator, dom->def->id,
+ (long long)dom->pid);
+
+ if (priv->mon) {
+ virObjectUnlock(priv->mon);
+ qemuMonitorClose(priv->mon);
+ priv->mon = NULL;
+ }
+
+ virCommandAbort(priv->cmd);
+ virCommandFree(priv->cmd);
+ priv->cmd = NULL;
+
+ if (priv->monpath)
+ ignore_value(unlink(priv->monpath));
+
+ if (dom->pid != 0) {
+ char ebuf[1024];
+
+ VIR_DEBUG("Killing QMP caps process %lld", (long long)dom->pid);
+ if (virProcessKill(dom->pid, SIGKILL) < 0 && errno != ESRCH)
+ VIR_ERROR(_("Failed to kill process %lld: %s"),
+ (long long)dom->pid,
+ virStrerror(errno, ebuf, sizeof(ebuf)));
+ }
+
+ if (priv->pidfile)
+ unlink(priv->pidfile);
+
+ dom->pid = 0;
+}
diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
index c2f7c2b5d2..7c5fd32538 100644
--- a/src/qemu/qemu_process.h
+++ b/src/qemu/qemu_process.h
@@ -74,6 +74,36 @@ int qemuProcessBeginJob(virQEMUDriverPtr driver,
void qemuProcessEndJob(virQEMUDriverPtr driver,
virDomainObjPtr vm);
+typedef struct _qmpDomainObjPrivate qmpDomainObjPrivate;
+typedef qmpDomainObjPrivate *qmpDomainObjPrivatePtr;
+struct _qmpDomainObjPrivate {
+ char *libDir;
+ uid_t runUid;
+ gid_t runGid;
+ char *qmperr; /* qemu proc stderr */
+ char *monarg; // "unix:{priv->monpath},server,nowait"
+ char *monpath; // {libDir}/capabilities.monitor.sock
+ char *pidfile;
+ virCommandPtr cmd;
+ qemuMonitorPtr mon;
+ bool forceTCG;
+};
+
+virDomainObjPtr qmpDomainObjNew(const char *binary,
+ bool forceTCG,
+ const char *libDir,
+ uid_t runUid,
+ gid_t runGid);
+
+#define QMP_DOMAIN_PRIVATE(dom) \
+ ((qmpDomainObjPrivatePtr) (dom)->privateData)
+
+#define QMP_DOMAIN_MONITOR(dom) \
+ ((qemuMonitorPtr) (QMP_DOMAIN_PRIVATE(dom) ? QMP_DOMAIN_PRIVATE(dom)->mon : NULL))
+
+#define QMP_DOMAIN_ERROR(dom) \
+ ((char *) (QMP_DOMAIN_PRIVATE(dom) ? QMP_DOMAIN_PRIVATE(dom)->qmperr: NULL))
+
typedef enum {
VIR_QEMU_PROCESS_START_COLD = 1 << 0,
VIR_QEMU_PROCESS_START_PAUSED = 1 << 1,
@@ -97,6 +127,9 @@ int qemuProcessStart(virConnectPtr conn,
virNetDevVPortProfileOp vmop,
unsigned int flags);
+int
+qemuProcessStartQmp(virDomainObjPtr dom);
+
virCommandPtr qemuProcessCreatePretendCmd(virQEMUDriverPtr driver,
virDomainObjPtr vm,
const char *migrateURI,
@@ -155,6 +188,8 @@ void qemuProcessStop(virQEMUDriverPtr driver,
qemuDomainAsyncJob asyncJob,
unsigned int flags);
+void qemuProcessStopQmp(virDomainObjPtr dom);
+
int qemuProcessAttach(virConnectPtr conn,
virQEMUDriverPtr driver,
virDomainObjPtr vm,
diff --git a/tests/qemucapabilitiestest.c b/tests/qemucapabilitiestest.c
index 8fe5a55e1d..ea4671739b 100644
--- a/tests/qemucapabilitiestest.c
+++ b/tests/qemucapabilitiestest.c
@@ -58,6 +58,9 @@ testQemuCaps(const void *opaque)
if (!(mon = qemuMonitorTestNewFromFileFull(repliesFile, &data->driver,
NULL)))
goto cleanup;
+ if (qemuMonitorSetCapabilities(qemuMonitorTestGetMonitor(mon)) < 0)
+ goto cleanup;
+
if (!(capsActual = virQEMUCapsNew()) ||
virQEMUCapsInitQMPMonitor(capsActual,
qemuMonitorTestGetMonitor(mon)) < 0)
@@ -65,6 +68,10 @@ testQemuCaps(const void *opaque)
if (virQEMUCapsGet(capsActual, QEMU_CAPS_KVM)) {
qemuMonitorResetCommandID(qemuMonitorTestGetMonitor(mon));
+
+ if (qemuMonitorSetCapabilities(qemuMonitorTestGetMonitor(mon)) < 0)
+ goto cleanup;
+
if (virQEMUCapsInitQMPMonitorTCG(capsActual,
qemuMonitorTestGetMonitor(mon)) < 0)
goto cleanup;
--
2.17.1
9:14 a.m.
New subject: [libvirt] [PATCH v3 4/6] qemu_process: Use common processes mgmt funcs for all QMP query types
On Thu, Sep 27, 2018 at 16:26:43 -0500, Chris Venteicher wrote:
Generalized QEMU process management functions supporting all forms
of
QMP queries including but no limited to capabilities queries.
QEMU process instances can be maintained and used for multiple different QMP
queries, of the same or different types, as required to complete a
specific libvirt task.
Support concurrent QEMU process instances for QMP queries,
using the same or different qemu binaries.
All process mgmt functions are removed from qemu_capabilities and
re-implemented in qemu_process in a generic, non capabilities specific,
manner consistent with existing qemu_process mgmt functions.
Concept of qmp_query domain is used to contain process state through the
an entire process lifecycle similar to how a VM domains contain process
state in existing process functions.
Monitor callbacks were not used in original process management code for
capabilities and are eliminated in new generalized process code for QMP
queries.
QMP exchange to exit capabilities negotiation mode and enter command mode
is the sole responsibility of the QMP query process code.
Heh, a lot is happening here. Please, separate logical changes rather
than squashing them all in one patch.
I didn't succeed in following what all the changes are doing, but...
...
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 29b0ba1590..c8a34fc37e 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8068,3 +8068,401 @@ qemuProcessReconnectAll(virQEMUDriverPtr driver)
struct qemuProcessReconnectData data = {.driver = driver};
virDomainObjListForEach(driver->domains, qemuProcessReconnectHelper, &data);
}
+
+
+static void *
+qmpDomainObjPrivateAlloc(void *opaque)
+{
+ qmpDomainObjPrivatePtr priv;
+
+ if (VIR_ALLOC(priv) < 0)
+ return NULL;
+
+ VIR_DEBUG("priv=%p opaque=%p", priv, opaque);
+
+ return priv;
+}
... this looks very suspicious. The code does not belong to
qemu_process.c.
+
+
+static void
+qmpDomainObjPrivateFree(void *data)
+{
+ qmpDomainObjPrivatePtr priv = data;
+
+ VIR_DEBUG("priv=%p, priv->mon=%p", priv, (priv ? priv->mon :
NULL));
+
+ if (!priv)
+ return;
+
+ /* This should never be non-NULL if we get here, but just in case... */
+ if (priv->mon) {
+ VIR_ERROR(_("Unexpected QEMU monitor still active during domain
deletion"));
+ qemuMonitorClose(priv->mon);
+ priv->mon = NULL;
+ }
+
+ VIR_FREE(priv->libDir);
+ VIR_FREE(priv->monpath);
+ VIR_FREE(priv->monarg);
+ VIR_FREE(priv->pidfile);
+ VIR_FREE(priv->qmperr);
+ VIR_FREE(priv);
+}
+
+/**
+ * qmpDomainObjNew:
+ * @binary: Qemu binary
+ * @forceTCG: Force TCG mode if true
+ * @libDir: Directory for process and connection artifacts
+ * @runUid: UserId for Qemu Process
+ * @runGid: GroupId for Qemu Process
+ *
+ * Allocate and initialize domain structure encapsulating
+ * QEMU Process state and monitor connection to QEMU
+ * for completing QMP Queries.
+ */
Oh, so you're creating a fake domain object to be able to start QEMU
process for probing? That's both confusing and fragile. Please, don't do
that. You can just rename, move, and extend the existing
virQEMUCapsInitQMPCommand.
If you need to keep some data across several functions, you should
create a new structure for that.
Jirka
4:26 p.m.
New subject: [libvirt] [PATCH v3 5/6] qemu_driver: BaselineHypervisorCPU support for S390
Libvirt can compute baseline hypervisor cpu from list of S390 cpu models
by using QEMU QMP messages to compute baseline within QEMU.
QEMU only baselines one pair of CPU models per query so a sequence of
multiple QMP queries are needed if more than two CPU models are
baselined.
Full expansion of baseline S390 model supported using QEMU QMP messages to
compute expansion within QEMU.
Introduces qemuMonitorCPUModelInfoRemovePropByBoolValue to remove props
by value as required to convert between cpu model property lists that
indicate if property is or isn't include in model with true / false
value and the form of cpu model property lists that only enumerate
properties that are actually included in the model.
Introduces functions for virCPUDef / qemuMonitorCPUModelInfo conversion.
Signed-off-by: Chris Venteicher <cventeic(a)redhat.com>
---
src/qemu/qemu_capabilities.c | 128 +++++++++++++++++++++++++------
src/qemu/qemu_capabilities.h | 4 +
src/qemu/qemu_driver.c | 143 +++++++++++++++++++++++++++++++++--
src/qemu/qemu_monitor.c | 41 ++++++++++
src/qemu/qemu_monitor.h | 10 +++
src/qemu/qemu_monitor_json.c | 60 +++++++++++++++
src/qemu/qemu_monitor_json.h | 7 ++
7 files changed, 365 insertions(+), 28 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 5b1e4f1bbd..c5bf04993b 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -2785,7 +2785,8 @@ virQEMUCapsInitCPUModelS390(virQEMUCapsPtr qemuCaps,
virCPUDefPtr cpu,
bool migratable)
{
- size_t i;
+ virCPUDefPtr tmp = NULL;
+ int ret = -1;
if (!modelInfo) {
if (type == VIR_DOMAIN_VIRT_KVM) {
@@ -2798,32 +2799,18 @@ virQEMUCapsInitCPUModelS390(virQEMUCapsPtr qemuCaps,
return 2;
}
- if (VIR_STRDUP(cpu->model, modelInfo->name) < 0 ||
- VIR_ALLOC_N(cpu->features, modelInfo->nprops) < 0)
- return -1;
-
- cpu->nfeatures_max = modelInfo->nprops;
- cpu->nfeatures = 0;
-
- for (i = 0; i < modelInfo->nprops; i++) {
- virCPUFeatureDefPtr feature = cpu->features + cpu->nfeatures;
- qemuMonitorCPUPropertyPtr prop = modelInfo->props + i;
+ if (!(tmp = virQEMUCapsCPUModelInfoToCPUDef(migratable, modelInfo)))
+ goto cleanup;
- if (prop->type != QEMU_MONITOR_CPU_PROPERTY_BOOLEAN)
- continue;
+ /* Free original then copy over model, vendor, vendor_id and features */
+ virCPUDefStealModel(cpu, tmp, true);
- if (VIR_STRDUP(feature->name, prop->name) < 0)
- return -1;
+ ret = 0;
- if (!prop->value.boolean ||
- (migratable && prop->migratable == VIR_TRISTATE_BOOL_NO))
- feature->policy = VIR_CPU_FEATURE_DISABLE;
- else
- feature->policy = VIR_CPU_FEATURE_REQUIRE;
- cpu->nfeatures++;
- }
+ cleanup:
+ virCPUDefFree(tmp);
- return 0;
+ return ret;
}
@@ -3620,6 +3607,101 @@ virQEMUCapsLoadCache(virArch hostArch,
return ret;
}
+/* virCPUDef model => qemuMonitorCPUModelInfo name
+ * virCPUDef features => qemuMonitorCPUModelInfo boolean properties */
+qemuMonitorCPUModelInfoPtr
+virQEMUCapsCPUModelInfoFromCPUDef(const virCPUDef *cpuDef)
+{
+ size_t i;
+ qemuMonitorCPUModelInfoPtr cpuModel = NULL;
+ qemuMonitorCPUModelInfoPtr ret = NULL;
+
+ if (!cpuDef || (VIR_ALLOC(cpuModel) < 0))
+ goto cleanup;
+
+ VIR_DEBUG("cpuDef->model = %s", NULLSTR(cpuDef->model));
+
+ if (VIR_STRDUP(cpuModel->name, cpuDef->model) < 0 ||
+ VIR_ALLOC_N(cpuModel->props, cpuDef->nfeatures) < 0)
+ goto cleanup;
+
+ cpuModel->nprops = 0;
+
+ for (i = 0; i < cpuDef->nfeatures; i++) {
+ qemuMonitorCPUPropertyPtr prop = &(cpuModel->props[cpuModel->nprops]);
+ virCPUFeatureDefPtr feature = &(cpuDef->features[i]);
+
+ if (!(feature->name) ||
+ VIR_STRDUP(prop->name, feature->name) < 0)
+ goto cleanup;
+
+ prop->type = QEMU_MONITOR_CPU_PROPERTY_BOOLEAN;
+
+ prop->value.boolean = feature->policy == -1 || /* policy undefined */
+ feature->policy == VIR_CPU_FEATURE_FORCE ||
+ feature->policy == VIR_CPU_FEATURE_REQUIRE;
+
+ cpuModel->nprops++;
+ }
+
+ VIR_STEAL_PTR(ret, cpuModel);
+
+ cleanup:
+ qemuMonitorCPUModelInfoFree(cpuModel);
+ return ret;
+}
+
+
+/* qemuMonitorCPUModelInfo name => virCPUDef model
+ * qemuMonitorCPUModelInfo boolean properties => virCPUDef features
+ *
+ * migratable true: mark non-migratable features as disabled
+ * false: allow all features as required
+ */
+virCPUDefPtr
+virQEMUCapsCPUModelInfoToCPUDef(bool migratable, qemuMonitorCPUModelInfoPtr model)
+{
+ virCPUDefPtr cpu = NULL;
+ virCPUDefPtr ret = NULL;
+ size_t i;
+
+ if (!model || VIR_ALLOC(cpu) < 0)
+ goto cleanup;
+
+ VIR_DEBUG("model->name= %s", NULLSTR(model->name));
+
+ if (VIR_STRDUP(cpu->model, model->name) < 0 ||
+ VIR_ALLOC_N(cpu->features, model->nprops) < 0)
+ goto cleanup;
+
+ cpu->nfeatures_max = model->nprops;
+ cpu->nfeatures = 0;
+
+ for (i = 0; i < model->nprops; i++) {
+ virCPUFeatureDefPtr feature = cpu->features + cpu->nfeatures;
+ qemuMonitorCPUPropertyPtr prop = model->props + i;
+
+ if (prop->type != QEMU_MONITOR_CPU_PROPERTY_BOOLEAN)
+ continue;
+
+ if (VIR_STRDUP(feature->name, prop->name) < 0)
+ goto cleanup;
+
+ if (!prop->value.boolean ||
+ (migratable && prop->migratable == VIR_TRISTATE_BOOL_NO))
+ feature->policy = VIR_CPU_FEATURE_DISABLE;
+ else
+ feature->policy = VIR_CPU_FEATURE_REQUIRE;
+
+ cpu->nfeatures++;
+ }
+
+ VIR_STEAL_PTR(ret, cpu);
+
+ cleanup:
+ virCPUDefFree(cpu);
+ return ret;
+}
static void
virQEMUCapsFormatHostCPUModelInfo(virQEMUCapsPtr qemuCaps,
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 934620ed31..98ccec154e 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -571,6 +571,10 @@ int virQEMUCapsGetMachineTypesCaps(virQEMUCapsPtr qemuCaps,
void virQEMUCapsFilterByMachineType(virQEMUCapsPtr qemuCaps,
const char *machineType);
+qemuMonitorCPUModelInfoPtr virQEMUCapsCPUModelInfoFromCPUDef(const virCPUDef *cpuDef);
+virCPUDefPtr virQEMUCapsCPUModelInfoToCPUDef(bool migratable,
+ qemuMonitorCPUModelInfoPtr model);
+
virFileCachePtr virQEMUCapsCacheNew(const char *libDir,
const char *cacheDir,
uid_t uid,
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index b238309852..8db7d01660 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -13406,6 +13406,77 @@ qemuConnectBaselineCPU(virConnectPtr conn ATTRIBUTE_UNUSED,
return cpustr;
}
+/* in:
+ * cpus[0]->model = "z14";
+ * cpus[0]->features[0].name = "xxx";
+ * cpus[0]->features[1].name = "yyy";
+ * ***
+ * cpus[n]->model = "z13";
+ * cpus[n]->features[0].name = "xxx";
+ * cpus[n]->features[1].name = "yyy";
+ *
+ * out:
+ * *baseline->model = "z13-base";
+ * *baseline->features[0].name = "yyy";
+ */
+static int
+qemuConnectBaselineHypervisorCPUViaQEMU(qemuMonitorPtr mon,
+ virCPUDefPtr *cpus,
+ virCPUDefPtr *baseline)
+{
+ qemuMonitorCPUModelInfoPtr model_baseline = NULL;
+ qemuMonitorCPUModelInfoPtr new_model_baseline = NULL;
+ qemuMonitorCPUModelInfoPtr next_model = NULL;
+ bool migratable = true;
+ int ret = -1;
+ size_t i;
+
+ *baseline = NULL;
+
+ if (!cpus || !cpus[0]) {
+ virReportError(VIR_ERR_INVALID_ARG, "%s", _("no cpus"));
+ goto cleanup;
+ }
+
+ for (i = 0; cpus[i]; i++) { /* cpus terminated by NULL element */
+ virCPUDefPtr cpu = cpus[i];
+
+ VIR_DEBUG("cpu[%lu]->model = %s", i, NULLSTR(cpu->model));
+
+ if (!(next_model = virQEMUCapsCPUModelInfoFromCPUDef(cpu)))
+ goto cleanup;
+
+ if (i == 0) {
+ model_baseline = next_model;
+ continue;
+ }
+
+ if (qemuMonitorGetCPUModelBaseline(mon, model_baseline,
+ next_model, &new_model_baseline) < 0)
+ goto cleanup;
+
+ qemuMonitorCPUModelInfoFree(model_baseline);
+ qemuMonitorCPUModelInfoFree(next_model);
+
+ next_model = NULL;
+
+ model_baseline = new_model_baseline;
+ }
+
+ if (!(*baseline = virQEMUCapsCPUModelInfoToCPUDef(migratable, model_baseline)))
+ goto cleanup;
+
+ VIR_DEBUG("baseline->model = %s", (*baseline)->model);
+
+ ret = 0;
+
+ cleanup:
+ qemuMonitorCPUModelInfoFree(model_baseline);
+ qemuMonitorCPUModelInfoFree(next_model);
+
+ return ret;
+}
+
static char *
qemuConnectBaselineHypervisorCPU(virConnectPtr conn,
@@ -13427,6 +13498,11 @@ qemuConnectBaselineHypervisorCPU(virConnectPtr conn,
virCPUDefPtr cpu = NULL;
char *cpustr = NULL;
char **features = NULL;
+ bool forceTCG = false;
+ qemuMonitorCPUModelInfoPtr modelInfo = NULL;
+ qemuMonitorCPUModelInfoPtr expansion = NULL;
+ virDomainObjPtr dom = NULL;
+ qemuMonitorPtr mon = NULL;
virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES |
VIR_CONNECT_BASELINE_CPU_MIGRATABLE, NULL);
@@ -13434,8 +13510,6 @@ qemuConnectBaselineHypervisorCPU(virConnectPtr conn,
if (virConnectBaselineHypervisorCPUEnsureACL(conn) < 0)
goto cleanup;
- migratable = !!(flags & VIR_CONNECT_BASELINE_CPU_MIGRATABLE);
-
if (!(cpus = virCPUDefListParse(xmlCPUs, ncpus, VIR_CPU_TYPE_AUTO)))
goto cleanup;
@@ -13448,6 +13522,20 @@ qemuConnectBaselineHypervisorCPU(virConnectPtr conn,
if (!qemuCaps)
goto cleanup;
+ /* QEMU can enumerate non-migratable cpu model features for some archs like x86
+ * migratable == true: ask for and include only migratable features
+ * migratable == false: ask for and include all features
+ */
+ migratable = !!(flags & VIR_CONNECT_BASELINE_CPU_MIGRATABLE);
+
+ if (ARCH_IS_S390(arch)) {
+ /* QEMU for S390 arch only enumerates migratable features
+ * No reason to explicitly ask QEMU for or include non-migratable
+ * features
+ */
+ migratable = true;
+ }
+
if (!(cpuModels = virQEMUCapsGetCPUDefinitions(qemuCaps, virttype)) ||
cpuModels->nmodels == 0) {
virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
@@ -13472,6 +13560,20 @@ qemuConnectBaselineHypervisorCPU(virConnectPtr conn,
if (!(cpu = virCPUBaseline(arch, cpus, ncpus, cpuModels,
(const char **)features, migratable)))
goto cleanup;
+ } else if (ARCH_IS_S390(arch)) {
+ const char *binary = virQEMUCapsGetBinary(qemuCaps);
+ virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+
+ if (!(dom = qmpDomainObjNew(binary, forceTCG, cfg->libDir, cfg->user,
cfg->group)))
+ goto cleanup;
+
+ if (qemuProcessStartQmp(dom) < 0)
+ goto cleanup;
+
+ mon = QMP_DOMAIN_MONITOR(dom);
+
+ if ((qemuConnectBaselineHypervisorCPUViaQEMU(mon, cpus, &cpu) < 0) ||
!cpu)
+ goto cleanup;
} else {
virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
_("computing baseline hypervisor CPU is not supported "
@@ -13481,9 +13583,36 @@ qemuConnectBaselineHypervisorCPU(virConnectPtr conn,
cpu->fallback = VIR_CPU_FALLBACK_FORBID;
- if ((flags & VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES) &&
- virCPUExpandFeatures(arch, cpu) < 0)
- goto cleanup;
+ if (flags & VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES) {
+ if (ARCH_IS_X86(arch)) {
+ if (virCPUExpandFeatures(arch, cpu) < 0)
+ goto cleanup;
+ } else if (ARCH_IS_S390(arch)) {
+ if (!(modelInfo = virQEMUCapsCPUModelInfoFromCPUDef(cpu)))
+ goto cleanup;
+
+ virCPUDefFree(cpu);
+ cpu = NULL;
+
+ if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("Feature Expansion not supported with this QEMU
binary"));
+ goto cleanup;
+ }
+
+ if (qemuMonitorGetCPUModelExpansion(mon,
+ QEMU_MONITOR_CPU_MODEL_EXPANSION_FULL,
+ migratable, modelInfo, &expansion)
< 0)
+ goto cleanup;
+
+ /* Expansion enumerates all features
+ * Baseline reply enumerates only in-model (true) features */
+ qemuMonitorCPUModelInfoRemovePropByBoolValue(expansion, false);
+
+ if (!(cpu = virQEMUCapsCPUModelInfoToCPUDef(migratable, expansion)))
+ goto cleanup;
+ }
+ }
cpustr = virCPUDefFormat(cpu, NULL);
@@ -13492,6 +13621,10 @@ qemuConnectBaselineHypervisorCPU(virConnectPtr conn,
virCPUDefFree(cpu);
virObjectUnref(qemuCaps);
virStringListFree(features);
+ qemuMonitorCPUModelInfoFree(modelInfo);
+ qemuMonitorCPUModelInfoFree(expansion);
+ qemuProcessStopQmp(dom);
+ virDomainObjEndAPI(&dom);
return cpustr;
}
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 507710d241..7045186c4e 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -3801,6 +3801,47 @@ qemuMonitorCPUModelInfoCopy(const qemuMonitorCPUModelInfo *orig)
}
+/* Squash CPU Model Info property list
+ * removing props of type boolean matching value */
+void
+qemuMonitorCPUModelInfoRemovePropByBoolValue(qemuMonitorCPUModelInfoPtr model,
+ bool value)
+{
+ qemuMonitorCPUPropertyPtr src;
+ qemuMonitorCPUPropertyPtr dst;
+ size_t i;
+ size_t dst_nprops = 0;
+
+ for (i = 0; i < model->nprops; i++) {
+ src = &(model->props[i]);
+ dst = &(model->props[dst_nprops]);
+
+ if (src->type == QEMU_MONITOR_CPU_PROPERTY_BOOLEAN &&
+ src->value.boolean == value)
+ continue;
+
+ *dst = *src;
+
+ dst_nprops++;
+ }
+
+ model->nprops = dst_nprops;
+
+ ignore_value(VIR_REALLOC_N_QUIET(model->props, dst_nprops));
+}
+
+
+int
+qemuMonitorGetCPUModelBaseline(qemuMonitorPtr mon,
+ qemuMonitorCPUModelInfoPtr model_a,
+ qemuMonitorCPUModelInfoPtr model_b,
+ qemuMonitorCPUModelInfoPtr *model_baseline)
+{
+ QEMU_CHECK_MONITOR(mon);
+
+ return qemuMonitorJSONGetCPUModelBaseline(mon, model_a, model_b, model_baseline);
+}
+
int
qemuMonitorCPUModelInfoBoolPropAdd(qemuMonitorCPUModelInfoPtr model,
const char *prop_name,
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index d3efd37099..fad39945d3 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -1053,6 +1053,16 @@ int qemuMonitorCPUModelInfoBoolPropAdd(qemuMonitorCPUModelInfoPtr
model,
bool prop_value)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+void qemuMonitorCPUModelInfoRemovePropByBoolValue(qemuMonitorCPUModelInfoPtr model,
+ bool value)
+ ATTRIBUTE_NONNULL(1);
+
+int qemuMonitorGetCPUModelBaseline(qemuMonitorPtr mon,
+ qemuMonitorCPUModelInfoPtr model_a,
+ qemuMonitorCPUModelInfoPtr model_b,
+ qemuMonitorCPUModelInfoPtr *model_baseline)
+ ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4);
+
int qemuMonitorGetCommands(qemuMonitorPtr mon,
char ***commands);
int qemuMonitorGetEvents(qemuMonitorPtr mon,
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 4e6e220a8c..1cac3d2964 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -5708,6 +5708,66 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon,
}
+int
+qemuMonitorJSONGetCPUModelBaseline(qemuMonitorPtr mon,
+ qemuMonitorCPUModelInfoPtr model_a,
+ qemuMonitorCPUModelInfoPtr model_b,
+ qemuMonitorCPUModelInfoPtr *model_baseline)
+{
+ int ret = -1;
+ virJSONValuePtr cmd = NULL;
+ virJSONValuePtr reply = NULL;
+ virJSONValuePtr data = NULL;
+ virJSONValuePtr modela = NULL;
+ virJSONValuePtr modelb = NULL;
+ virJSONValuePtr cpu_model = NULL;
+
+ *model_baseline = NULL;
+
+ if (!(modela = qemuMonitorJSONBuildCPUModelInfoToJSON(model_a)) ||
+ !(modelb = qemuMonitorJSONBuildCPUModelInfoToJSON(model_b)))
+ goto cleanup;
+
+ if (!(cmd = qemuMonitorJSONMakeCommand("query-cpu-model-baseline",
+ "a:modela", &modela,
+ "a:modelb", &modelb,
+ NULL)))
+ goto cleanup;
+
+ if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
+ goto cleanup;
+
+ if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_OBJECT) < 0) {
+ virReportError(VIR_ERR_INVALID_ARG,
+ _("QEMU doesn't support baseline or recognize model %s or
%s"),
+ model_a->name,
+ model_b->name);
+ goto cleanup;
+ }
+
+ data = virJSONValueObjectGetObject(reply, "return");
+
+ if (!(cpu_model = virJSONValueObjectGetObject(data, "model"))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("query-cpu-model-baseline reply data was missing
'model'"));
+ goto cleanup;
+ }
+
+ if (!(*model_baseline = qemuMonitorJSONBuildCPUModelInfoFromJSON(cpu_model)))
+ goto cleanup;
+
+ ret = 0;
+
+ cleanup:
+ virJSONValueFree(cmd);
+ virJSONValueFree(reply);
+ virJSONValueFree(modela);
+ virJSONValueFree(modelb);
+
+ return ret;
+}
+
+
int qemuMonitorJSONGetCommands(qemuMonitorPtr mon,
char ***commands)
{
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index 2d5679094e..e57f2e3590 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -372,6 +372,13 @@ int qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon,
qemuMonitorCPUModelInfoPtr *expansion)
ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(5);
+int qemuMonitorJSONGetCPUModelBaseline(qemuMonitorPtr mon,
+ qemuMonitorCPUModelInfoPtr model_a,
+ qemuMonitorCPUModelInfoPtr model_b,
+ qemuMonitorCPUModelInfoPtr *model_baseline)
+ ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4);
+
+
int qemuMonitorJSONGetCommands(qemuMonitorPtr mon,
char ***commands)
ATTRIBUTE_NONNULL(2);
--
2.17.1
9:17 a.m.
New subject: [libvirt] [PATCH v3 5/6] qemu_driver: BaselineHypervisorCPU support for S390
On Thu, Sep 27, 2018 at 16:26:44 -0500, Chris Venteicher wrote:
Libvirt can compute baseline hypervisor cpu from list of S390 cpu
models
by using QEMU QMP messages to compute baseline within QEMU.
QEMU only baselines one pair of CPU models per query so a sequence of
multiple QMP queries are needed if more than two CPU models are
baselined.
Full expansion of baseline S390 model supported using QEMU QMP messages to
compute expansion within QEMU.
Introduces qemuMonitorCPUModelInfoRemovePropByBoolValue to remove props
by value as required to convert between cpu model property lists that
indicate if property is or isn't include in model with true / false
value and the form of cpu model property lists that only enumerate
properties that are actually included in the model.
Introduces functions for virCPUDef / qemuMonitorCPUModelInfo conversion.
Again, several logical changes squashed in a single hard to follow
patch.
Jirka
4:26 p.m.
New subject: [libvirt] [PATCH v3 6/6] qemu_monitor: Default props to migratable when expanding cpu model
QEMU only returns migratable props when expanding model unless
explicitly told to also include non-migratable props.
Props will be marked migratable when we are certain QEMU returned only
migratable props resulting in consistent information and expansion output
for s390 that is consistent with x86.
After this change,
immediately default prop->migratable = _YES for all props
when we know QEMU only included migratable props in CPU Model.
Set model->migratability = true when we have set prop->migratable.
Signed-off-by: Chris Venteicher <cventeic(a)redhat.com>
---
src/qemu/qemu_monitor.c | 53 ++++++++++++++-
src/qemu/qemu_monitor.h | 7 +-
.../caps_2.10.0.s390x.xml | 60 ++++++++---------
.../caps_2.11.0.s390x.xml | 58 ++++++++---------
.../caps_2.12.0.s390x.xml | 56 ++++++++--------
.../qemucapabilitiesdata/caps_2.8.0.s390x.xml | 32 +++++-----
.../qemucapabilitiesdata/caps_2.9.0.s390x.xml | 34 +++++-----
.../qemucapabilitiesdata/caps_3.0.0.s390x.xml | 64 +++++++++----------
8 files changed, 210 insertions(+), 154 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 7045186c4e..7a7c175f0a 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -3698,12 +3698,31 @@ qemuMonitorGetCPUModelExpansion(qemuMonitorPtr mon,
qemuMonitorCPUModelInfoPtr *expansion
)
{
+ int ret = -1;
+ qemuMonitorCPUModelInfoPtr tmp;
+
VIR_DEBUG("type=%d model_name=%s migratable=%d",
type, input->name, migratable);
+ *expansion = NULL;
+
QEMU_CHECK_MONITOR(mon);
- return qemuMonitorJSONGetCPUModelExpansion(mon, type, migratable, input, expansion);
+ if ((ret = qemuMonitorJSONGetCPUModelExpansion(mon, type, migratable, input,
&tmp)) < 0)
+ goto cleanup;
+
+ if (migratable) {
+ /* Only migratable props were included in expanded CPU model */
+ *expansion = qemuMonitorCPUModelInfoCopyDefaultMigratable(tmp);
+ } else {
+ VIR_STEAL_PTR(*expansion, tmp);
+ }
+
+ ret = 0;
+
+ cleanup:
+ qemuMonitorCPUModelInfoFree(tmp);
+ return ret;
}
@@ -3801,6 +3820,38 @@ qemuMonitorCPUModelInfoCopy(const qemuMonitorCPUModelInfo *orig)
}
+qemuMonitorCPUModelInfoPtr
+qemuMonitorCPUModelInfoCopyDefaultMigratable(const qemuMonitorCPUModelInfo *orig)
+{
+ qemuMonitorCPUModelInfoPtr ret = NULL;
+ qemuMonitorCPUModelInfoPtr tmp = NULL;
+ qemuMonitorCPUPropertyPtr prop = NULL;
+ size_t i;
+
+ if (!(tmp = qemuMonitorCPUModelInfoCopy(orig)))
+ goto cleanup;
+
+ for (i = 0; i < tmp->nprops; i++) {
+ prop = tmp->props + i;
+
+ /* Default prop thats in cpu model (true) to migratable (_YES)
+ * unless prop already explicitly set not migratable (_NO)
+ */
+ if (prop->type == QEMU_MONITOR_CPU_PROPERTY_BOOLEAN &&
+ prop->value.boolean &&
+ prop->migratable != VIR_TRISTATE_BOOL_NO)
+ prop->migratable = VIR_TRISTATE_BOOL_YES;
+ }
+
+ tmp->migratability = true; /* prop->migratable = YES/NO for all CPU props */
+
+ VIR_STEAL_PTR(ret, tmp);
+
+ cleanup:
+ return ret;
+}
+
+
/* Squash CPU Model Info property list
* removing props of type boolean matching value */
void
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index fad39945d3..7268bd7977 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -1025,7 +1025,7 @@ struct _qemuMonitorCPUModelInfo {
char *name;
size_t nprops;
qemuMonitorCPUPropertyPtr props;
- bool migratability;
+ bool migratability; /* true if prop->migratable is YES/NO for all CPU props */
};
typedef enum {
@@ -1048,6 +1048,11 @@ qemuMonitorCPUModelInfoPtr qemuMonitorCPUModelInfoNew(const char
*name);
qemuMonitorCPUModelInfoPtr
qemuMonitorCPUModelInfoCopy(const qemuMonitorCPUModelInfo *orig);
+qemuMonitorCPUModelInfoPtr
+qemuMonitorCPUModelInfoCopyDefaultMigratable(const qemuMonitorCPUModelInfo *orig)
+ ATTRIBUTE_NONNULL(1);
+
+
int qemuMonitorCPUModelInfoBoolPropAdd(qemuMonitorCPUModelInfoPtr model,
const char *prop_name,
bool prop_value)
diff --git a/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml
b/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml
index e000aac384..391bee4f06 100644
--- a/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml
@@ -118,36 +118,36 @@
<microcodeVersion>306247</microcodeVersion>
<package></package>
<arch>s390x</arch>
- <hostCPU type='kvm' model='z14-base' migratability='no'>
- <property name='aen' type='boolean' value='true'/>
- <property name='cmmnt' type='boolean' value='true'/>
- <property name='aefsi' type='boolean' value='true'/>
- <property name='mepoch' type='boolean' value='true'/>
- <property name='msa8' type='boolean' value='true'/>
- <property name='msa7' type='boolean' value='true'/>
- <property name='msa6' type='boolean' value='true'/>
- <property name='msa5' type='boolean' value='true'/>
- <property name='msa4' type='boolean' value='true'/>
- <property name='msa3' type='boolean' value='true'/>
- <property name='msa2' type='boolean' value='true'/>
- <property name='msa1' type='boolean' value='true'/>
- <property name='sthyi' type='boolean' value='true'/>
- <property name='edat' type='boolean' value='true'/>
- <property name='ri' type='boolean' value='true'/>
- <property name='edat2' type='boolean' value='true'/>
- <property name='vx' type='boolean' value='true'/>
- <property name='ipter' type='boolean' value='true'/>
- <property name='vxeh' type='boolean' value='true'/>
- <property name='vxpd' type='boolean' value='true'/>
- <property name='esop' type='boolean' value='true'/>
- <property name='iep' type='boolean' value='true'/>
- <property name='cte' type='boolean' value='true'/>
- <property name='ais' type='boolean' value='true'/>
- <property name='gs' type='boolean' value='true'/>
- <property name='zpci' type='boolean' value='true'/>
- <property name='sea_esop2' type='boolean'
value='true'/>
- <property name='te' type='boolean' value='true'/>
- <property name='cmm' type='boolean' value='true'/>
+ <hostCPU type='kvm' model='z14-base'
migratability='yes'>
+ <property name='aen' type='boolean' value='true'
migratable='yes'/>
+ <property name='cmmnt' type='boolean' value='true'
migratable='yes'/>
+ <property name='aefsi' type='boolean' value='true'
migratable='yes'/>
+ <property name='mepoch' type='boolean' value='true'
migratable='yes'/>
+ <property name='msa8' type='boolean' value='true'
migratable='yes'/>
+ <property name='msa7' type='boolean' value='true'
migratable='yes'/>
+ <property name='msa6' type='boolean' value='true'
migratable='yes'/>
+ <property name='msa5' type='boolean' value='true'
migratable='yes'/>
+ <property name='msa4' type='boolean' value='true'
migratable='yes'/>
+ <property name='msa3' type='boolean' value='true'
migratable='yes'/>
+ <property name='msa2' type='boolean' value='true'
migratable='yes'/>
+ <property name='msa1' type='boolean' value='true'
migratable='yes'/>
+ <property name='sthyi' type='boolean' value='true'
migratable='yes'/>
+ <property name='edat' type='boolean' value='true'
migratable='yes'/>
+ <property name='ri' type='boolean' value='true'
migratable='yes'/>
+ <property name='edat2' type='boolean' value='true'
migratable='yes'/>
+ <property name='vx' type='boolean' value='true'
migratable='yes'/>
+ <property name='ipter' type='boolean' value='true'
migratable='yes'/>
+ <property name='vxeh' type='boolean' value='true'
migratable='yes'/>
+ <property name='vxpd' type='boolean' value='true'
migratable='yes'/>
+ <property name='esop' type='boolean' value='true'
migratable='yes'/>
+ <property name='iep' type='boolean' value='true'
migratable='yes'/>
+ <property name='cte' type='boolean' value='true'
migratable='yes'/>
+ <property name='ais' type='boolean' value='true'
migratable='yes'/>
+ <property name='gs' type='boolean' value='true'
migratable='yes'/>
+ <property name='zpci' type='boolean' value='true'
migratable='yes'/>
+ <property name='sea_esop2' type='boolean' value='true'
migratable='yes'/>
+ <property name='te' type='boolean' value='true'
migratable='yes'/>
+ <property name='cmm' type='boolean' value='true'
migratable='yes'/>
</hostCPU>
<cpu type='kvm' name='z10EC-base' usable='yes'/>
<cpu type='kvm' name='z9EC-base' usable='yes'/>
diff --git a/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml
b/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml
index 4eb8a39d94..d63f56be7d 100644
--- a/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml
@@ -125,35 +125,35 @@
<microcodeVersion>345099</microcodeVersion>
<package></package>
<arch>s390x</arch>
- <hostCPU type='kvm' model='z14-base' migratability='no'>
- <property name='aen' type='boolean' value='true'/>
- <property name='cmmnt' type='boolean' value='true'/>
- <property name='aefsi' type='boolean' value='true'/>
- <property name='mepoch' type='boolean' value='true'/>
- <property name='msa8' type='boolean' value='true'/>
- <property name='msa7' type='boolean' value='true'/>
- <property name='msa6' type='boolean' value='true'/>
- <property name='msa5' type='boolean' value='true'/>
- <property name='msa4' type='boolean' value='true'/>
- <property name='msa3' type='boolean' value='true'/>
- <property name='msa2' type='boolean' value='true'/>
- <property name='msa1' type='boolean' value='true'/>
- <property name='sthyi' type='boolean' value='true'/>
- <property name='edat' type='boolean' value='true'/>
- <property name='ri' type='boolean' value='true'/>
- <property name='edat2' type='boolean' value='true'/>
- <property name='vx' type='boolean' value='true'/>
- <property name='ipter' type='boolean' value='true'/>
- <property name='vxeh' type='boolean' value='true'/>
- <property name='vxpd' type='boolean' value='true'/>
- <property name='esop' type='boolean' value='true'/>
- <property name='iep' type='boolean' value='true'/>
- <property name='cte' type='boolean' value='true'/>
- <property name='gs' type='boolean' value='true'/>
- <property name='zpci' type='boolean' value='true'/>
- <property name='sea_esop2' type='boolean'
value='true'/>
- <property name='te' type='boolean' value='true'/>
- <property name='cmm' type='boolean' value='true'/>
+ <hostCPU type='kvm' model='z14-base'
migratability='yes'>
+ <property name='aen' type='boolean' value='true'
migratable='yes'/>
+ <property name='cmmnt' type='boolean' value='true'
migratable='yes'/>
+ <property name='aefsi' type='boolean' value='true'
migratable='yes'/>
+ <property name='mepoch' type='boolean' value='true'
migratable='yes'/>
+ <property name='msa8' type='boolean' value='true'
migratable='yes'/>
+ <property name='msa7' type='boolean' value='true'
migratable='yes'/>
+ <property name='msa6' type='boolean' value='true'
migratable='yes'/>
+ <property name='msa5' type='boolean' value='true'
migratable='yes'/>
+ <property name='msa4' type='boolean' value='true'
migratable='yes'/>
+ <property name='msa3' type='boolean' value='true'
migratable='yes'/>
+ <property name='msa2' type='boolean' value='true'
migratable='yes'/>
+ <property name='msa1' type='boolean' value='true'
migratable='yes'/>
+ <property name='sthyi' type='boolean' value='true'
migratable='yes'/>
+ <property name='edat' type='boolean' value='true'
migratable='yes'/>
+ <property name='ri' type='boolean' value='true'
migratable='yes'/>
+ <property name='edat2' type='boolean' value='true'
migratable='yes'/>
+ <property name='vx' type='boolean' value='true'
migratable='yes'/>
+ <property name='ipter' type='boolean' value='true'
migratable='yes'/>
+ <property name='vxeh' type='boolean' value='true'
migratable='yes'/>
+ <property name='vxpd' type='boolean' value='true'
migratable='yes'/>
+ <property name='esop' type='boolean' value='true'
migratable='yes'/>
+ <property name='iep' type='boolean' value='true'
migratable='yes'/>
+ <property name='cte' type='boolean' value='true'
migratable='yes'/>
+ <property name='gs' type='boolean' value='true'
migratable='yes'/>
+ <property name='zpci' type='boolean' value='true'
migratable='yes'/>
+ <property name='sea_esop2' type='boolean' value='true'
migratable='yes'/>
+ <property name='te' type='boolean' value='true'
migratable='yes'/>
+ <property name='cmm' type='boolean' value='true'
migratable='yes'/>
</hostCPU>
<cpu type='kvm' name='z890.2' usable='yes'/>
<cpu type='kvm' name='z990.4' usable='yes'/>
diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml
b/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml
index 79320d5229..ea3bce232e 100644
--- a/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml
@@ -133,34 +133,34 @@
<microcodeVersion>374287</microcodeVersion>
<package></package>
<arch>s390x</arch>
- <hostCPU type='kvm' model='z14-base' migratability='no'>
- <property name='aen' type='boolean' value='true'/>
- <property name='aefsi' type='boolean' value='true'/>
- <property name='msa8' type='boolean' value='true'/>
- <property name='msa7' type='boolean' value='true'/>
- <property name='msa6' type='boolean' value='true'/>
- <property name='msa5' type='boolean' value='true'/>
- <property name='msa4' type='boolean' value='true'/>
- <property name='msa3' type='boolean' value='true'/>
- <property name='msa2' type='boolean' value='true'/>
- <property name='msa1' type='boolean' value='true'/>
- <property name='sthyi' type='boolean' value='true'/>
- <property name='edat' type='boolean' value='true'/>
- <property name='ri' type='boolean' value='true'/>
- <property name='edat2' type='boolean' value='true'/>
- <property name='vx' type='boolean' value='true'/>
- <property name='ipter' type='boolean' value='true'/>
- <property name='vxeh' type='boolean' value='true'/>
- <property name='vxpd' type='boolean' value='true'/>
- <property name='esop' type='boolean' value='true'/>
- <property name='iep' type='boolean' value='true'/>
- <property name='cte' type='boolean' value='true'/>
- <property name='gs' type='boolean' value='true'/>
- <property name='ppa15' type='boolean' value='true'/>
- <property name='zpci' type='boolean' value='true'/>
- <property name='sea_esop2' type='boolean'
value='true'/>
- <property name='te' type='boolean' value='true'/>
- <property name='cmm' type='boolean' value='true'/>
+ <hostCPU type='kvm' model='z14-base'
migratability='yes'>
+ <property name='aen' type='boolean' value='true'
migratable='yes'/>
+ <property name='aefsi' type='boolean' value='true'
migratable='yes'/>
+ <property name='msa8' type='boolean' value='true'
migratable='yes'/>
+ <property name='msa7' type='boolean' value='true'
migratable='yes'/>
+ <property name='msa6' type='boolean' value='true'
migratable='yes'/>
+ <property name='msa5' type='boolean' value='true'
migratable='yes'/>
+ <property name='msa4' type='boolean' value='true'
migratable='yes'/>
+ <property name='msa3' type='boolean' value='true'
migratable='yes'/>
+ <property name='msa2' type='boolean' value='true'
migratable='yes'/>
+ <property name='msa1' type='boolean' value='true'
migratable='yes'/>
+ <property name='sthyi' type='boolean' value='true'
migratable='yes'/>
+ <property name='edat' type='boolean' value='true'
migratable='yes'/>
+ <property name='ri' type='boolean' value='true'
migratable='yes'/>
+ <property name='edat2' type='boolean' value='true'
migratable='yes'/>
+ <property name='vx' type='boolean' value='true'
migratable='yes'/>
+ <property name='ipter' type='boolean' value='true'
migratable='yes'/>
+ <property name='vxeh' type='boolean' value='true'
migratable='yes'/>
+ <property name='vxpd' type='boolean' value='true'
migratable='yes'/>
+ <property name='esop' type='boolean' value='true'
migratable='yes'/>
+ <property name='iep' type='boolean' value='true'
migratable='yes'/>
+ <property name='cte' type='boolean' value='true'
migratable='yes'/>
+ <property name='gs' type='boolean' value='true'
migratable='yes'/>
+ <property name='ppa15' type='boolean' value='true'
migratable='yes'/>
+ <property name='zpci' type='boolean' value='true'
migratable='yes'/>
+ <property name='sea_esop2' type='boolean' value='true'
migratable='yes'/>
+ <property name='te' type='boolean' value='true'
migratable='yes'/>
+ <property name='cmm' type='boolean' value='true'
migratable='yes'/>
</hostCPU>
<cpu type='kvm' name='z890.2' usable='yes'/>
<cpu type='kvm' name='z990.4' usable='yes'/>
diff --git a/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml
b/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml
index b010f731a5..858bc49918 100644
--- a/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml
@@ -108,22 +108,22 @@
<microcodeVersion>244554</microcodeVersion>
<package></package>
<arch>s390x</arch>
- <hostCPU type='kvm' model='zEC12.2-base'
migratability='no'>
- <property name='aefsi' type='boolean' value='true'/>
- <property name='msa5' type='boolean' value='true'/>
- <property name='msa4' type='boolean' value='true'/>
- <property name='msa3' type='boolean' value='true'/>
- <property name='msa2' type='boolean' value='true'/>
- <property name='msa1' type='boolean' value='true'/>
- <property name='sthyi' type='boolean' value='true'/>
- <property name='edat' type='boolean' value='true'/>
- <property name='ri' type='boolean' value='true'/>
- <property name='edat2' type='boolean' value='true'/>
- <property name='ipter' type='boolean' value='true'/>
- <property name='esop' type='boolean' value='true'/>
- <property name='cte' type='boolean' value='true'/>
- <property name='te' type='boolean' value='true'/>
- <property name='cmm' type='boolean' value='true'/>
+ <hostCPU type='kvm' model='zEC12.2-base'
migratability='yes'>
+ <property name='aefsi' type='boolean' value='true'
migratable='yes'/>
+ <property name='msa5' type='boolean' value='true'
migratable='yes'/>
+ <property name='msa4' type='boolean' value='true'
migratable='yes'/>
+ <property name='msa3' type='boolean' value='true'
migratable='yes'/>
+ <property name='msa2' type='boolean' value='true'
migratable='yes'/>
+ <property name='msa1' type='boolean' value='true'
migratable='yes'/>
+ <property name='sthyi' type='boolean' value='true'
migratable='yes'/>
+ <property name='edat' type='boolean' value='true'
migratable='yes'/>
+ <property name='ri' type='boolean' value='true'
migratable='yes'/>
+ <property name='edat2' type='boolean' value='true'
migratable='yes'/>
+ <property name='ipter' type='boolean' value='true'
migratable='yes'/>
+ <property name='esop' type='boolean' value='true'
migratable='yes'/>
+ <property name='cte' type='boolean' value='true'
migratable='yes'/>
+ <property name='te' type='boolean' value='true'
migratable='yes'/>
+ <property name='cmm' type='boolean' value='true'
migratable='yes'/>
</hostCPU>
<cpu type='kvm' name='z10EC-base'/>
<cpu type='kvm' name='z9EC-base'/>
diff --git a/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml
b/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml
index 5a4371ab83..621036c914 100644
--- a/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml
@@ -112,23 +112,23 @@
<microcodeVersion>267973</microcodeVersion>
<package></package>
<arch>s390x</arch>
- <hostCPU type='kvm' model='z13.2-base'
migratability='no'>
- <property name='aefsi' type='boolean' value='true'/>
- <property name='msa5' type='boolean' value='true'/>
- <property name='msa4' type='boolean' value='true'/>
- <property name='msa3' type='boolean' value='true'/>
- <property name='msa2' type='boolean' value='true'/>
- <property name='msa1' type='boolean' value='true'/>
- <property name='sthyi' type='boolean' value='true'/>
- <property name='edat' type='boolean' value='true'/>
- <property name='ri' type='boolean' value='true'/>
- <property name='edat2' type='boolean' value='true'/>
- <property name='vx' type='boolean' value='true'/>
- <property name='ipter' type='boolean' value='true'/>
- <property name='esop' type='boolean' value='true'/>
- <property name='cte' type='boolean' value='true'/>
- <property name='te' type='boolean' value='true'/>
- <property name='cmm' type='boolean' value='true'/>
+ <hostCPU type='kvm' model='z13.2-base'
migratability='yes'>
+ <property name='aefsi' type='boolean' value='true'
migratable='yes'/>
+ <property name='msa5' type='boolean' value='true'
migratable='yes'/>
+ <property name='msa4' type='boolean' value='true'
migratable='yes'/>
+ <property name='msa3' type='boolean' value='true'
migratable='yes'/>
+ <property name='msa2' type='boolean' value='true'
migratable='yes'/>
+ <property name='msa1' type='boolean' value='true'
migratable='yes'/>
+ <property name='sthyi' type='boolean' value='true'
migratable='yes'/>
+ <property name='edat' type='boolean' value='true'
migratable='yes'/>
+ <property name='ri' type='boolean' value='true'
migratable='yes'/>
+ <property name='edat2' type='boolean' value='true'
migratable='yes'/>
+ <property name='vx' type='boolean' value='true'
migratable='yes'/>
+ <property name='ipter' type='boolean' value='true'
migratable='yes'/>
+ <property name='esop' type='boolean' value='true'
migratable='yes'/>
+ <property name='cte' type='boolean' value='true'
migratable='yes'/>
+ <property name='te' type='boolean' value='true'
migratable='yes'/>
+ <property name='cmm' type='boolean' value='true'
migratable='yes'/>
</hostCPU>
<cpu type='kvm' name='z10EC-base'/>
<cpu type='kvm' name='z9EC-base'/>
diff --git a/tests/qemucapabilitiesdata/caps_3.0.0.s390x.xml
b/tests/qemucapabilitiesdata/caps_3.0.0.s390x.xml
index 3b5f9818a5..7bb42d211c 100644
--- a/tests/qemucapabilitiesdata/caps_3.0.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_3.0.0.s390x.xml
@@ -135,38 +135,38 @@
<microcodeVersion>387601</microcodeVersion>
<package></package>
<arch>s390x</arch>
- <hostCPU type='kvm' model='z14-base' migratability='no'>
- <property name='aen' type='boolean' value='true'/>
- <property name='cmmnt' type='boolean' value='true'/>
- <property name='aefsi' type='boolean' value='true'/>
- <property name='mepoch' type='boolean' value='true'/>
- <property name='msa8' type='boolean' value='true'/>
- <property name='msa7' type='boolean' value='true'/>
- <property name='msa6' type='boolean' value='true'/>
- <property name='msa5' type='boolean' value='true'/>
- <property name='msa4' type='boolean' value='true'/>
- <property name='msa3' type='boolean' value='true'/>
- <property name='msa2' type='boolean' value='true'/>
- <property name='msa1' type='boolean' value='true'/>
- <property name='sthyi' type='boolean' value='true'/>
- <property name='edat' type='boolean' value='true'/>
- <property name='ri' type='boolean' value='true'/>
- <property name='edat2' type='boolean' value='true'/>
- <property name='vx' type='boolean' value='true'/>
- <property name='ipter' type='boolean' value='true'/>
- <property name='mepochptff' type='boolean'
value='true'/>
- <property name='vxeh' type='boolean' value='true'/>
- <property name='vxpd' type='boolean' value='true'/>
- <property name='esop' type='boolean' value='true'/>
- <property name='iep' type='boolean' value='true'/>
- <property name='cte' type='boolean' value='true'/>
- <property name='bpb' type='boolean' value='true'/>
- <property name='gs' type='boolean' value='true'/>
- <property name='ppa15' type='boolean' value='true'/>
- <property name='zpci' type='boolean' value='true'/>
- <property name='sea_esop2' type='boolean'
value='true'/>
- <property name='te' type='boolean' value='true'/>
- <property name='cmm' type='boolean' value='true'/>
+ <hostCPU type='kvm' model='z14-base'
migratability='yes'>
+ <property name='aen' type='boolean' value='true'
migratable='yes'/>
+ <property name='cmmnt' type='boolean' value='true'
migratable='yes'/>
+ <property name='aefsi' type='boolean' value='true'
migratable='yes'/>
+ <property name='mepoch' type='boolean' value='true'
migratable='yes'/>
+ <property name='msa8' type='boolean' value='true'
migratable='yes'/>
+ <property name='msa7' type='boolean' value='true'
migratable='yes'/>
+ <property name='msa6' type='boolean' value='true'
migratable='yes'/>
+ <property name='msa5' type='boolean' value='true'
migratable='yes'/>
+ <property name='msa4' type='boolean' value='true'
migratable='yes'/>
+ <property name='msa3' type='boolean' value='true'
migratable='yes'/>
+ <property name='msa2' type='boolean' value='true'
migratable='yes'/>
+ <property name='msa1' type='boolean' value='true'
migratable='yes'/>
+ <property name='sthyi' type='boolean' value='true'
migratable='yes'/>
+ <property name='edat' type='boolean' value='true'
migratable='yes'/>
+ <property name='ri' type='boolean' value='true'
migratable='yes'/>
+ <property name='edat2' type='boolean' value='true'
migratable='yes'/>
+ <property name='vx' type='boolean' value='true'
migratable='yes'/>
+ <property name='ipter' type='boolean' value='true'
migratable='yes'/>
+ <property name='mepochptff' type='boolean' value='true'
migratable='yes'/>
+ <property name='vxeh' type='boolean' value='true'
migratable='yes'/>
+ <property name='vxpd' type='boolean' value='true'
migratable='yes'/>
+ <property name='esop' type='boolean' value='true'
migratable='yes'/>
+ <property name='iep' type='boolean' value='true'
migratable='yes'/>
+ <property name='cte' type='boolean' value='true'
migratable='yes'/>
+ <property name='bpb' type='boolean' value='true'
migratable='yes'/>
+ <property name='gs' type='boolean' value='true'
migratable='yes'/>
+ <property name='ppa15' type='boolean' value='true'
migratable='yes'/>
+ <property name='zpci' type='boolean' value='true'
migratable='yes'/>
+ <property name='sea_esop2' type='boolean' value='true'
migratable='yes'/>
+ <property name='te' type='boolean' value='true'
migratable='yes'/>
+ <property name='cmm' type='boolean' value='true'
migratable='yes'/>
</hostCPU>
<cpu type='kvm' name='z890.2' usable='yes'/>
<cpu type='kvm' name='z990.4' usable='yes'/>
--
2.17.1
9:36 a.m.
Hmm, I guess I could have checked the cover letter before looking at the
individual patches. That would safe me some time and thinking :-)
On Thu, Sep 27, 2018 at 16:26:39 -0500, Chris Venteicher wrote:
Some architectures (S390) depend on QEMU to compute baseline CPU
model and
expand a models feature set.
Interacting with QEMU requires starting the QEMU process and completing one or
more query-cpu-model-baseline QMP exchanges with QEMU in addition to a
query-cpu-model-expansion QMP exchange to expand all features in the model.
See "s390x CPU models: exposing features" patch set on Qemu-devel for
discussion
of QEMU aspects.
This is part of resolution of: https://bugzilla.redhat.com/show_bug.cgi?id=1511999
Version 3 attempts to address all issues from V1 and V2 including making the
QEMU process activation for QMP Queries generic (not specific to capabilities)
and moving that code from qemu_capabilities to qemu_process.
So far so good.
I attempted to make the new qemu_process functions consistent with
the functions
but mostly ended up reusing the guts of the previous functions from qemu_capabilities.
I guess you wanted to say "... consistent with the existing functions in
qemu_process.c" or something similar, right? It's fine to just move and
generalize the functions from qemu_capabilities (and perhaps make the
original functions just wrappers around the new ones if needed).
Of interest may be the choice to reuse a domain structure to hold the
Qmp Query
process state and connection information.
This sounds like a bad idea to me.
Also, pay attention to the use of a large random number to uniquely
identify decoupled processes in terms of sockets and file system
footprint. If you believe it's worth the effort I think there are
some pre-existing library functions to establish files with unique
identifiers in a thread safe way.
We already have src/util/virrandom.c for random numbers, but it doesn't
really matter anyway since we don't need or either want to use them
here. Just use mkdtemp() and store the socket, pid, whatever you need in
there.
The last patch may also be interesting and is based on past
discussion of QEMU cpu
expansion only returning migratable features except for one x86 case where
non-migratable features are explicitly requested. The patch records that features
are migratable based on QEMU only returning migratable features. The main result
is the CPU xml for S390 CPU's contains the same migratability info the X86 currently
does. The testcases were updated to reflect the change. Is this ok?
Yeah, looks OK, although I didn't look too closely at it.
Unlike the previous versions every patch should compile independently
if applied
in sequence.
Good, each series have to make sure the code can be compiled after every
single patch in the series (so that we can easily use git bisect). But
please, don't achieve the goal by squashing patches until the result can
be compiled.
Jirka
4:16 p.m.
Quoting Jiri Denemark (2018-10-03 09:36:34)
Hmm, I guess I could have checked the cover letter before looking at
the
individual patches. That would safe me some time and thinking :-)
On Thu, Sep 27, 2018 at 16:26:39 -0500, Chris Venteicher wrote:
> Some architectures (S390) depend on QEMU to compute baseline CPU model and
> expand a models feature set.
>
> Interacting with QEMU requires starting the QEMU process and completing one or
> more query-cpu-model-baseline QMP exchanges with QEMU in addition to a
> query-cpu-model-expansion QMP exchange to expand all features in the model.
>
> See "s390x CPU models: exposing features" patch set on Qemu-devel for
discussion
> of QEMU aspects.
>
> This is part of resolution of: https://bugzilla.redhat.com/show_bug.cgi?id=1511999
>
> Version 3 attempts to address all issues from V1 and V2 including making the
> QEMU process activation for QMP Queries generic (not specific to capabilities)
> and moving that code from qemu_capabilities to qemu_process.
So far so good.
> I attempted to make the new qemu_process functions consistent with the functions
> but mostly ended up reusing the guts of the previous functions from
qemu_capabilities.
I guess you wanted to say "... consistent with the existing functions in
qemu_process.c" or something similar, right? It's fine to just move and
generalize the functions from qemu_capabilities (and perhaps make the
original functions just wrappers around the new ones if needed).
> Of interest may be the choice to reuse a domain structure to hold the Qmp Query
> process state and connection information.
This sounds like a bad idea to me.
> Also, pay attention to the use of a large random number to uniquely
> identify decoupled processes in terms of sockets and file system
> footprint. If you believe it's worth the effort I think there are
> some pre-existing library functions to establish files with unique
> identifiers in a thread safe way.
We already have src/util/virrandom.c for random numbers, but it doesn't
really matter anyway since we don't need or either want to use them
here. Just use mkdtemp() and store the socket, pid, whatever you need in
there.
> The last patch may also be interesting and is based on past discussion of QEMU cpu
> expansion only returning migratable features except for one x86 case where
> non-migratable features are explicitly requested. The patch records that features
> are migratable based on QEMU only returning migratable features. The main result
> is the CPU xml for S390 CPU's contains the same migratability info the X86
currently
> does. The testcases were updated to reflect the change. Is this ok?
Yeah, looks OK, although I didn't look too closely at it.
> Unlike the previous versions every patch should compile independently if applied
> in sequence.
Good, each series have to make sure the code can be compiled after every
single patch in the series (so that we can easily use git bisect). But
please, don't achieve the goal by squashing patches until the result can
be compiled.
Hi Jiri, At first look I am getting what your saying on everything but
am concerned I am not understanding something about the rules on
splitting out patches.
Thanks for pointing out that I could split out virQEMUCapsMigratablePropsDiff.
I missed that one.
I am fully on board with making the patches as small and easy to review
as possible.
But I think I got pretty strong feedback in the last review that
there should be no floating (unused) functions in a patch and each patch should
compile and pass test cases and function without crashing.
I tried pretty hard to create / retain as many distinct patches as I
could but those rules, as I understand them, didn't seem to leave me
many options.
Should I be creating unit tests or something for what would otherwise
be unused functions to get patches with working compiles? Am I
misunderstanding the rules? Some other trick I am missing that would
enable me to get to smaller patches?
Thanks for any feedback you can give on this.
Chris
Jirka
4:34 a.m.
On Wed, Oct 03, 2018 at 16:16:50 -0500, Chris Venteicher wrote:
Quoting Jiri Denemark (2018-10-03 09:36:34)
> Hmm, I guess I could have checked the cover letter before looking at the
> individual patches. That would safe me some time and thinking :-)
>
> On Thu, Sep 27, 2018 at 16:26:39 -0500, Chris Venteicher wrote:
> > Some architectures (S390) depend on QEMU to compute baseline CPU model and
> > expand a models feature set.
> >
> > Interacting with QEMU requires starting the QEMU process and completing one or
> > more query-cpu-model-baseline QMP exchanges with QEMU in addition to a
> > query-cpu-model-expansion QMP exchange to expand all features in the model.
> >
> > See "s390x CPU models: exposing features" patch set on Qemu-devel for
discussion
> > of QEMU aspects.
> >
> > This is part of resolution of:
https://bugzilla.redhat.com/show_bug.cgi?id=1511999
> >
> > Version 3 attempts to address all issues from V1 and V2 including making the
> > QEMU process activation for QMP Queries generic (not specific to capabilities)
> > and moving that code from qemu_capabilities to qemu_process.
>
> So far so good.
>
> > I attempted to make the new qemu_process functions consistent with the
functions
> > but mostly ended up reusing the guts of the previous functions from
qemu_capabilities.
>
> I guess you wanted to say "... consistent with the existing functions in
> qemu_process.c" or something similar, right? It's fine to just move and
> generalize the functions from qemu_capabilities (and perhaps make the
> original functions just wrappers around the new ones if needed).
>
> > Of interest may be the choice to reuse a domain structure to hold the Qmp
Query
> > process state and connection information.
>
> This sounds like a bad idea to me.
>
> > Also, pay attention to the use of a large random number to uniquely
> > identify decoupled processes in terms of sockets and file system
> > footprint. If you believe it's worth the effort I think there are
> > some pre-existing library functions to establish files with unique
> > identifiers in a thread safe way.
>
> We already have src/util/virrandom.c for random numbers, but it doesn't
> really matter anyway since we don't need or either want to use them
> here. Just use mkdtemp() and store the socket, pid, whatever you need in
> there.
>
> > The last patch may also be interesting and is based on past discussion of QEMU
cpu
> > expansion only returning migratable features except for one x86 case where
> > non-migratable features are explicitly requested. The patch records that
features
> > are migratable based on QEMU only returning migratable features. The main
result
> > is the CPU xml for S390 CPU's contains the same migratability info the X86
currently
> > does. The testcases were updated to reflect the change. Is this ok?
>
> Yeah, looks OK, although I didn't look too closely at it.
>
> > Unlike the previous versions every patch should compile independently if
applied
> > in sequence.
>
> Good, each series have to make sure the code can be compiled after every
> single patch in the series (so that we can easily use git bisect). But
> please, don't achieve the goal by squashing patches until the result can
> be compiled.
Hi Jiri, At first look I am getting what your saying on everything but
am concerned I am not understanding something about the rules on
splitting out patches.
Thanks for pointing out that I could split out virQEMUCapsMigratablePropsDiff.
I missed that one.
I am fully on board with making the patches as small and easy to review
as possible.
But I think I got pretty strong feedback in the last review that
there should be no floating (unused) functions in a patch and each patch should
compile and pass test cases and function without crashing.
I don't think I would strongly oppose to unused functions. It always
depends on what a patch is doing. Of course, you can't do unused static
functions, because the could would fail to compile. Sometimes unused
functions are OK, but it's better to avoid them.
You can move and rename existing functions in one patch, followed by
other patches that would incrementally change the functions and their
callers. Or you can keep the old functions as wrappers around the new
ones (which will become more general) so that you don't have to fix all
the callers while changing the new functions. It's hard to give more
specific tips since the solution often depends on the code.
What you can always do is to refactor functions and update callers
(ideally in several patches if possible, especially when moving code
between files) and follow up with patches which add a new code using the
better functions.
I know some of the changes are hard to split once done, but it's still
doable and the result is much better and easier to review, comment, and
spot possible issues.
Jirka
2398
days inactive
2405
days old
12 comments
2 participants
participants (2)
-
Chris Venteicher -
Jiri Denemark