[PATCH] virDomainNumaFillCPUsInNode: Skip over NUMA nodes without vCPUs

Michal Privoznik posted 1 patch 3 years, 6 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/71f8732774b135c67496a4d2baf8441813bfd03c.1601459673.git.mprivozn@redhat.com
src/conf/numa_conf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] virDomainNumaFillCPUsInNode: Skip over NUMA nodes without vCPUs
Posted by Michal Privoznik 3 years, 6 months ago
After v6.5.0-rc1~148 we started to rectify vCPU to guest NUMA
assignment - if there is a vCPU not assigned to any guest NUMA
node it is automatically assigned to node #0.

A month later I've made it possible to define guest NUMA nodes
without vCPUs (v6.6.0-rc1~250) - this is needed because of HMAT.
As a part of that I fixed all callers of
virDomainNumaGetNodeCpumask() (which returns a bitmap of vCPUs for
given node) to handle case when NULL is returned (i.e. no vCPUs
assigned to given node). But of course my patch was written
before aforementioned vCPU rectify patch but merged afterwards
and hence I missed the virDomainNumaFillCPUsInNode() caller.

And because we are dealing with a NULL pointer, of course this
leads to a crash. Just try to define a domain with at least two
NUMA nodes and no vCPU assignment to any of the nodes.

Fixes: a26f61ee0cffa421b87ef568002b684dd8025432
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1880289
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/conf/numa_conf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c
index bfa312215c..cc6c77f105 100644
--- a/src/conf/numa_conf.c
+++ b/src/conf/numa_conf.c
@@ -1816,7 +1816,7 @@ virDomainNumaFillCPUsInNode(virDomainNumaPtr numa,
     for (i = 0; i < numa->nmem_nodes; i++) {
         virBitmapPtr nodeCpus = virDomainNumaGetNodeCpumask(numa, i);
 
-        if (i == node)
+        if (i == node || !nodeCpus)
             continue;
 
         virBitmapSubtract(maxCPUsBitmap, nodeCpus);
-- 
2.26.2

Re: [PATCH] virDomainNumaFillCPUsInNode: Skip over NUMA nodes without vCPUs
Posted by Daniel Henrique Barboza 3 years, 6 months ago

On 9/30/20 6:54 AM, Michal Privoznik wrote:
> After v6.5.0-rc1~148 we started to rectify vCPU to guest NUMA
> assignment - if there is a vCPU not assigned to any guest NUMA
> node it is automatically assigned to node #0.
> 
> A month later I've made it possible to define guest NUMA nodes
> without vCPUs (v6.6.0-rc1~250) - this is needed because of HMAT.
> As a part of that I fixed all callers of
> virDomainNumaGetNodeCpumask() (which returns a bitmap of vCPUs for
> given node) to handle case when NULL is returned (i.e. no vCPUs
> assigned to given node). But of course my patch was written
> before aforementioned vCPU rectify patch but merged afterwards
> and hence I missed the virDomainNumaFillCPUsInNode() caller.
> 
> And because we are dealing with a NULL pointer, of course this
> leads to a crash. Just try to define a domain with at least two
> NUMA nodes and no vCPU assignment to any of the nodes.


Well, I wrote v6.5.0-rc1~148 and reviewed v6.6.0-rc1~250, and didn't see
this coming either. Hopefully we'll redeem ourselves now :)


> 
> Fixes: a26f61ee0cffa421b87ef568002b684dd8025432
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1880289
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---


Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>




>   src/conf/numa_conf.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c
> index bfa312215c..cc6c77f105 100644
> --- a/src/conf/numa_conf.c
> +++ b/src/conf/numa_conf.c
> @@ -1816,7 +1816,7 @@ virDomainNumaFillCPUsInNode(virDomainNumaPtr numa,
>       for (i = 0; i < numa->nmem_nodes; i++) {
>           virBitmapPtr nodeCpus = virDomainNumaGetNodeCpumask(numa, i);
>   
> -        if (i == node)
> +        if (i == node || !nodeCpus)
>               continue;
>   
>           virBitmapSubtract(maxCPUsBitmap, nodeCpus);
>