[PATCH] libxl: Improve reporting of die_id in capabilities

Jim Fehlig posted 1 patch 2 years, 6 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20210913220107.27104-1-jfehlig@suse.com
src/libxl/libxl_capabilities.c | 2 ++
1 file changed, 2 insertions(+)
[PATCH] libxl: Improve reporting of die_id in capabilities
Posted by Jim Fehlig 2 years, 6 months ago
On Xen, libvirt runs in a VM (typically dom0) and does not have an accurate
picture of numa and cpu topology of the underlying physical machine using
the "usual" mechanisms. numa info and cpu toplogy are retrieved from libxl
and used to populate the libvirt conterparts. Commit 7b79ee2f78b introduced
support for reporting die_id in capabilities, but did not account for
special handling of numa and cpu topology in libxl.

Currently, Xen does not report die_id in the libxl_cputopology structure.
In the meantime, set die_id to 0, which was suggested by the Xen developers
and is slightly better than random garbage such as

<cpu id='1' socket_id='0' die_id='-1073069552' core_id='0' siblings='0-1'/>

Signed-off-by: Jim Fehlig <jfehlig@suse.com>
---
 src/libxl/libxl_capabilities.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/libxl/libxl_capabilities.c b/src/libxl/libxl_capabilities.c
index b4bd1d7e62..4afed71436 100644
--- a/src/libxl/libxl_capabilities.c
+++ b/src/libxl/libxl_capabilities.c
@@ -292,6 +292,8 @@ libxlCapsInitNuma(libxl_ctx *ctx, virCaps *caps)
         cpus[node][nr_cpus_node[node]-1].id = i;
         cpus[node][nr_cpus_node[node]-1].socket_id = cpu_topo[i].socket;
         cpus[node][nr_cpus_node[node]-1].core_id = cpu_topo[i].core;
+        /* Until Xen reports die_id, 0 is better than random garbage */
+        cpus[node][nr_cpus_node[node]-1].die_id = 0;
         /* Allocate the siblings maps. We will be filling them later */
         cpus[node][nr_cpus_node[node]-1].siblings = virBitmapNew(nr_cpus);
     }
-- 
2.33.0


Re: [PATCH] libxl: Improve reporting of die_id in capabilities
Posted by Michal Prívozník 2 years, 6 months ago
On 9/14/21 12:01 AM, Jim Fehlig wrote:
> On Xen, libvirt runs in a VM (typically dom0) and does not have an accurate
> picture of numa and cpu topology of the underlying physical machine using
> the "usual" mechanisms. numa info and cpu toplogy are retrieved from libxl
> and used to populate the libvirt conterparts. Commit 7b79ee2f78b introduced
> support for reporting die_id in capabilities, but did not account for
> special handling of numa and cpu topology in libxl.
> 
> Currently, Xen does not report die_id in the libxl_cputopology structure.
> In the meantime, set die_id to 0, which was suggested by the Xen developers
> and is slightly better than random garbage such as
> 
> <cpu id='1' socket_id='0' die_id='-1073069552' core_id='0' siblings='0-1'/>
> 
> Signed-off-by: Jim Fehlig <jfehlig@suse.com>
> ---
>  src/libxl/libxl_capabilities.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/src/libxl/libxl_capabilities.c b/src/libxl/libxl_capabilities.c
> index b4bd1d7e62..4afed71436 100644
> --- a/src/libxl/libxl_capabilities.c
> +++ b/src/libxl/libxl_capabilities.c
> @@ -292,6 +292,8 @@ libxlCapsInitNuma(libxl_ctx *ctx, virCaps *caps)
>          cpus[node][nr_cpus_node[node]-1].id = i;
>          cpus[node][nr_cpus_node[node]-1].socket_id = cpu_topo[i].socket;
>          cpus[node][nr_cpus_node[node]-1].core_id = cpu_topo[i].core;
> +        /* Until Xen reports die_id, 0 is better than random garbage */
> +        cpus[node][nr_cpus_node[node]-1].die_id = 0;
>          /* Allocate the siblings maps. We will be filling them later */
>          cpus[node][nr_cpus_node[node]-1].siblings = virBitmapNew(nr_cpus);
>      }
> 

Oh right. It took me a while to realize where the garbage value might
come from. All I saw was g_new0() but then I came across VIR_REALLOC_N()
a few lines above this hunk. Therefore, the die_id can indeed be
uninitialized.

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

Michal