On Thu, Apr 12, 2018 at 08:47:58 +0200, Andrea Bolognani wrote:
The current private XML parsing code relies on the assumption
that NUMA node IDs start from 0 and are densely allocated,
neither of which is necessarily the case.
Change it so that the bitmap size is dynamically calculated by
looking at NUMA node IDs instead, which ensures all nodes will
be able to fit and thus the bitmap will be parsed successfully.
Update one of the test cases so that it would fail with the
previous approach, but passes with the new one.
Resolves:
https://bugzilla.redhat.com/show_bug.cgi?id=1490158
While the patch below will fix this case, I'd also like to see that the
parsing of the bitmaps is made non-fatal. If the nodesets are missing
some of the reported data will be wrong, but not having this is
certainly not a deal-breaker so that we should not reconnect to qemu.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/qemu/qemu_domain.c | 11 ++++++++++-
tests/qemustatusxml2xmldata/modern-in.xml | 2 +-
2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 100304fd05..b126c69490 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2248,6 +2248,8 @@ qemuDomainObjPrivateXMLParseAutomaticPlacement(xmlXPathContextPtr
ctxt,
virCapsPtr caps = NULL;
char *nodeset;
char *cpuset;
+ int nodesetSize = 0;
+ int i;
int ret = -1;
nodeset = virXPathString("string(./numad/@nodeset)", ctxt);
@@ -2259,8 +2261,15 @@ qemuDomainObjPrivateXMLParseAutomaticPlacement(xmlXPathContextPtr
ctxt,
if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
goto cleanup;
+ /* Figure out how big the nodeset bitmap needs to be.
+ * This is necessary because NUMA node IDs are not guaranteed to
+ * start from 0 or be densely allocated */
+ for (i = 0; i < caps->host.nnumaCell; i++) {
+ nodesetSize = MAX(nodesetSize, caps->host.numaCell[i]->num + 1);
+ }
Syntax-check is moaning about this.
+
if (nodeset &&
- virBitmapParse(nodeset, &priv->autoNodeset, caps->host.nnumaCell_max)
< 0)
+ virBitmapParse(nodeset, &priv->autoNodeset, nodesetSize) < 0)
goto cleanup;
if (cpuset) {
ACK if you actually run syntax-check.