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@xxxxxxxxxx> > --- > 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.
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
