5:51 a.m.
On Sun, Dec 18, 2016 at 14:22:27 -0500, Jason J. Herne wrote:
When qmp query-cpu-model-expansion is available probe Qemu for its
view of the
host model. In kvm environments this can provide a more complete view of the
host model because features supported by Qemu and Kvm can be considered.
Signed-off-by: Collin L. Walling <walling(a)linux.vnet.ibm.com>
Signed-off-by: Jason J. Herne <jjherne(a)linux.vnet.ibm.com>
# Conflicts:
# tests/qemucapabilitiesdata/caps_2.8.0.s390x.replies
Signed-off-by: Jason J. Herne <jjherne(a)linux.vnet.ibm.com>
---
src/qemu/qemu_capabilities.c | 185 ++++++++++++++++++++-
tests/domaincapsschemadata/qemu_2.7.0.s390x.xml | 4 +-
tests/domaincapsschemadata/qemu_2.8.0.s390x.xml | 17 +-
.../qemucapabilitiesdata/caps_2.8.0.s390x.replies | 26 +++
tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml | 17 ++
5 files changed, 239 insertions(+), 10 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index bff30ed..7d33b19 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
...
@@ -3055,6 +3117,98 @@ virQEMUCapsInitHostCPUModel(virQEMUCapsPtr
qemuCaps,
virResetLastError();
}
One more empty line here, please.
+void
+virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps,
+ virCapsPtr caps)
+{
+ if (!caps || !virQEMUCapsGuestIsNative(caps->host.arch, qemuCaps->arch))
+ return;
+
+ switch (qemuCaps->arch) {
+ case VIR_ARCH_S390:
+ case VIR_ARCH_S390X:
+ virQEMUCapsCopyCPUModelFromQEMU(qemuCaps);
+ break;
+ default: virQEMUCapsCopyCPUModelFromHost(qemuCaps, caps);
default:
virQEMUCapsCopyCPUModelFromHost(qemuCaps, caps);
+ }
+}
+
+
+static int
+virQEMUCapsLoadHostCPUModelInfo(virQEMUCapsPtr qemuCaps,
+ xmlXPathContextPtr ctxt)
+{
+ char *str = NULL;
+ xmlNodePtr hostCPUNode;
+ xmlNodePtr *featureNodes = NULL;
+ xmlNodePtr oldnode = ctxt->node;
+ qemuMonitorCPUModelInfoPtr hostCPU = NULL;
+ int ret = -1;
+ size_t i;
+ int n;
+
+ if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION)) {
+ ret = 0;
+ goto cleanup;
+ }
This is not really necessary, properly checking for <hostCPU> element is
enough.
+
+ if (!(hostCPUNode = virXPathNode("./hostCPU", ctxt)) ||
+ VIR_ALLOC(hostCPU) < 0)
+ goto cleanup;
When QEMU is probed in TCG mode, QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION
might be set but ./hostCPU will not be present in the capabilities XML.
The code should not fail in such a case. In other words:
if (!(hostCPUNode = virXPathNode("./hostCPU", ctxt))) {
ret = 0;
goto cleanup;
}
if (VIR_ALLOC(hostCPU) < 0)
goto cleanup;
+
+ if ((str = virXMLPropString(hostCPUNode, "model"))) {
Don't confuse virXMLPropString and virXPathString. The latter does not
make a copy of the string while the former does make the copy.
+ if (VIR_STRDUP(hostCPU->name, str) < 0)
+ goto cleanup;
+ }
Anyway, it looks like the name attribute should always be present, so
if (!(hostCPU->name = virXMLPropString(hostCPUNode, "model"))) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("missing host CPU model name in QEMU "
"capabilities cache"));
goto cleanup;
}
should be enough.
+
+ ctxt->node = hostCPUNode;
+
+ if ((n = virXPathNodeSet("./feature", ctxt, &featureNodes)) > 0) {
+ if (VIR_ALLOC_N(hostCPU->props, n) < 0)
+ goto cleanup;
+
+ hostCPU->nprops = n;
+
+ for (i = 0; i < n; i++) {
+ if (!(str = virXMLPropString(featureNodes[i], "name"))) {
You can directly assign it to hostCPU->props[i].name.
+ virReportError(VIR_ERR_INTERNAL_ERROR,
"%s",
+ _("missing 'name' element for a host CPU
model"
'name' is not an element, it's an attribute.
+ " feature in QEMU capabilities
cache"));
+ goto cleanup;
+ }
+ if (VIR_STRDUP(hostCPU->props[i].name, str) < 0)
+ goto cleanup;
Let's separate the two attributes by an empty line here.
+ if (!(str = virXMLPropString(featureNodes[i],
"supported"))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("missing 'supported' element for a host
CPU"
s/element/attribute/
+ " model feature in QEMU
capabilities cache"));
+ goto cleanup;
+ }
+ if (STREQ(str, "yes")) {
+ hostCPU->props[i].supported = true;
+ } else if (STREQ(str, "no")) {
+ hostCPU->props[i].supported = false;
+ } else {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("malformed supported value: expected
'yes'"
+ " or 'no'"));
It's an internal XML, not something a user is supposed to create so
printing the incorrect value should be enough:
..., _("invalid supported value: '%s'"), str);
+ goto cleanup;
+ }
You'd leak str here if n > 1.
+ }
+ }
+
+ qemuCaps->hostCPUModelInfo = hostCPU;
+ hostCPU = NULL;
+ ret = 0;
+
+ cleanup:
+ ctxt->node = oldnode;
+ VIR_FREE(str);
+ VIR_FREE(featureNodes);
+ qemuMonitorCPUModelInfoFree(hostCPU);
+ return ret;
+}
+
static int
virQEMUCapsLoadCPUModels(virQEMUCapsPtr qemuCaps,
...
diff --git a/tests/domaincapsschemadata/qemu_2.7.0.s390x.xml
b/tests/domaincapsschemadata/qemu_2.7.0.s390x.xml
index 9f181d3..999e279 100644
--- a/tests/domaincapsschemadata/qemu_2.7.0.s390x.xml
+++ b/tests/domaincapsschemadata/qemu_2.7.0.s390x.xml
@@ -20,9 +20,7 @@
</os>
<cpu>
<mode name='host-passthrough' supported='yes'/>
- <mode name='host-model' supported='yes'>
- <model fallback='allow'></model>
- </mode>
+ <mode name='host-model' supported='no'/>
<mode name='custom' supported='no'/>
</cpu>
<devices>
This hunk should be part of 4/9.
ACK with the attached diff squashed in...
Jirka
diff --git i/src/qemu/qemu_capabilities.c w/src/qemu/qemu_capabilities.c
index a3f97d209..2512e4838 100644
--- i/src/qemu/qemu_capabilities.c
+++ w/src/qemu/qemu_capabilities.c
@@ -3117,6 +3117,7 @@ virQEMUCapsCopyCPUModelFromHost(virQEMUCapsPtr qemuCaps,
virResetLastError();
}
+
void
virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps,
virCapsPtr caps)
@@ -3129,7 +3130,9 @@ virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps,
case VIR_ARCH_S390X:
virQEMUCapsCopyCPUModelFromQEMU(qemuCaps);
break;
- default: virQEMUCapsCopyCPUModelFromHost(qemuCaps, caps);
+
+ default:
+ virQEMUCapsCopyCPUModelFromHost(qemuCaps, caps);
}
}
@@ -3147,18 +3150,19 @@ virQEMUCapsLoadHostCPUModelInfo(virQEMUCapsPtr qemuCaps,
size_t i;
int n;
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION)) {
+ if (!(hostCPUNode = virXPathNode("./hostCPU", ctxt))) {
ret = 0;
goto cleanup;
}
- if (!(hostCPUNode = virXPathNode("./hostCPU", ctxt)) ||
- VIR_ALLOC(hostCPU) < 0)
+ if (VIR_ALLOC(hostCPU) < 0)
goto cleanup;
- if ((str = virXMLPropString(hostCPUNode, "model"))) {
- if (VIR_STRDUP(hostCPU->name, str) < 0)
- goto cleanup;
+ if (!(hostCPU->name = virXMLPropString(hostCPUNode, "model"))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("missing host CPU model name in QEMU "
+ "capabilities cache"));
+ goto cleanup;
}
ctxt->node = hostCPUNode;
@@ -3170,17 +3174,17 @@ virQEMUCapsLoadHostCPUModelInfo(virQEMUCapsPtr qemuCaps,
hostCPU->nprops = n;
for (i = 0; i < n; i++) {
- if (!(str = virXMLPropString(featureNodes[i], "name"))) {
+ hostCPU->props[i].name = virXMLPropString(featureNodes[i],
"name");
+ if (!hostCPU->props[i].name) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("missing 'name' element for a host CPU
model"
- " feature in QEMU capabilities cache"));
+ _("missing 'name' attribute for a host
CPU"
+ " model feature in QEMU capabilities
cache"));
goto cleanup;
}
- if (VIR_STRDUP(hostCPU->props[i].name, str) < 0)
- goto cleanup;
+
if (!(str = virXMLPropString(featureNodes[i], "supported"))) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("missing 'supported' element for a host
CPU"
+ _("missing 'supported' attribute for a host
CPU"
" model feature in QEMU capabilities
cache"));
goto cleanup;
}
@@ -3189,11 +3193,11 @@ virQEMUCapsLoadHostCPUModelInfo(virQEMUCapsPtr qemuCaps,
} else if (STREQ(str, "no")) {
hostCPU->props[i].supported = false;
} else {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("malformed supported value: expected
'yes'"
- " or 'no'"));
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("invalid supported value: '%s'"),
str);
goto cleanup;
}
+ VIR_FREE(str);
}
}