On Fri, Aug 21, 2020 at 05:12:08PM +0200, Michal Privoznik wrote: > In a very distant past, we came around machines that has not > continuous node IDs. This made us error out when constructing > capabilities XML. We resolved it by utilizing strange behaviour > of numa_node_to_cpus() in which it returned a mask with all bits > set for a non-existent node. However, this is not the only case > when it returns all ones mask - if the node exists and has enough > CPUs to fill the mask up (e.g. 128 CPUs). > > The fix consists of using nodemask_isset(&numa_all_nodes, ..) > prior to calling numa_node_to_cpus() to determine if the node > exists. > > Fixes: 628c93574758abb59e71160042524d321a33543f > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1860231 > Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx> > --- > src/util/virnuma.c | 18 +++++------------- > 1 file changed, 5 insertions(+), 13 deletions(-) Reviewed-by: Daniel P. Berrangé <berrange@xxxxxxxxxx> > > diff --git a/src/util/virnuma.c b/src/util/virnuma.c > index eeca438f25..75d5628cff 100644 > --- a/src/util/virnuma.c > +++ b/src/util/virnuma.c > @@ -256,31 +256,23 @@ virNumaGetNodeCPUs(int node, > int mask_n_bytes = max_n_cpus / 8; > size_t i; > g_autofree unsigned long *mask = NULL; > - g_autofree unsigned long *allonesmask = NULL; > g_autoptr(virBitmap) cpumap = NULL; > > *cpus = NULL; > > + if (!nodemask_isset(&numa_all_nodes, node)) { > + VIR_DEBUG("NUMA topology for cell %d is not available, ignoring", node); > + return -2; > + } > + > if (VIR_ALLOC_N(mask, mask_n_bytes / sizeof(*mask)) < 0) > return -1; > > - if (VIR_ALLOC_N(allonesmask, mask_n_bytes / sizeof(*mask)) < 0) > - return -1; > - > - memset(allonesmask, 0xff, mask_n_bytes); > - > - /* The first time this returns -1, ENOENT if node doesn't exist... */ > if (numa_node_to_cpus(node, mask, mask_n_bytes) < 0) { > VIR_WARN("NUMA topology for cell %d is not available, ignoring", node); > return -2; > } > > - /* second, third... times it returns an all-1's mask */ > - if (memcmp(mask, allonesmask, mask_n_bytes) == 0) { > - VIR_DEBUG("NUMA topology for cell %d is invalid, ignoring", node); > - return -2; > - } > - > if (!(cpumap = virBitmapNew(max_n_cpus))) > return -1; Nice, that's simpler than I expected the fix would be. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
