9:32 a.m.
Hi Jim, Everyone,
First patch series, so maybe a small introduction is required: I'm Dario and I
work for Citrix on improving the NUMA support of Xen.
This patch series implements some of the missing bits and pieces, in the libxl
driver, regarding obtaining per-host and per-domain NUMA related information.
It's not the full libvirt NUMA interface, since we don't have all we would need
for that in Xen yet (we will in the next version, 4.4), but it's certainly
better than having nothing! :-)
Basically, I'm enhancing capability reporting, to cover NUMA topology (patch
01), and I'm implementing nodeGetCellsFreeMemory (patch 02) and
virDomainGetNumaParameters (patch 04) for the libxl driver. This last one
requires at least Xen 4.3, so I put the implementation within the proper
#ifdef-ery.
What I'm really not sure about is patch 03, which is something I need if I want
patch 04 to function properly. Basically it is about advertising that the libxl
driver supports VIR_TYPED_PARAM_STRING. I looked at how that is done in the
qemu driver, but I'm not entirely sure I completely understood the logic behind
it, so, please, tell me if I missed or misinterpreted anything! In particular,
should I have added more of those "flags &= ~VIR_TYPED_PARAM_STRING_OKAY;"
statements, as it happens in the qemu driver? If yes, in which functions?
Finally, allow me to say that it was a while that I wanted start hacking a bit
on libvirt. I'm really glad I've eventually been able to do so, and I
definitely plan to continue (with particular focus on NUMA related stuff).
Comments are of course more than welcome. :-)
Thanks and Regards,
Dario
---
Dario Faggioli (4):
libxl: implement NUMA capabilities reporting
libxl: implement per NUMA node free memory reporting
libxl: advertise the support for VIR_TYPED_PARAM_STRING
libxl: implement virDomainGetNumaParameters
src/libxl/libxl_conf.c | 128 +++++++++++++++++++++++++++++-
src/libxl/libxl_driver.c | 199 ++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 326 insertions(+), 1 deletion(-)
--
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
9:32 a.m.
New subject: [libvirt] [PATCH 1/4] libxl: implement NUMA capabilities reporting
Starting from Xen 4.2, libxl has all the bits and pieces are in
place for retrieving an adequate amount of information about the
host NUMA topology. It is therefore possible, after a bit of
shuffling, to arrange those information in the way libvirt wants
to present them to the outside world.
Therefore, with this patch, the <topology> section of the host
capabilities is properly populated, when running on Xen, so that
we can figure out whether or not we're running on a NUMA host,
and what its characteristics are.
[raistlin@Zhaman ~]$ sudo virsh --connect xen:/// capabilities
<capabilities>
<host>
<cpu>
....
<topology>
<cells num='2'>
<cell id='0'>
<memory unit='KiB'>6291456</memory>
<cpus num='8'>
<cpu id='0' socket_id='1' core_id='0'
siblings='0-1'/>
<cpu id='1' socket_id='1' core_id='0'
siblings='0-1'/>
<cpu id='2' socket_id='1' core_id='1'
siblings='2-3'/>
<cpu id='3' socket_id='1' core_id='1'
siblings='2-3'/>
<cpu id='4' socket_id='1' core_id='9'
siblings='4-5'/>
<cpu id='5' socket_id='1' core_id='9'
siblings='4-5'/>
<cpu id='6' socket_id='1' core_id='10'
siblings='6-7'/>
<cpu id='7' socket_id='1' core_id='10'
siblings='6-7'/>
</cpus>
</cell>
<cell id='1'>
<memory unit='KiB'>6881280</memory>
<cpus num='8'>
<cpu id='8' socket_id='0' core_id='0'
siblings='8-9'/>
<cpu id='9' socket_id='0' core_id='0'
siblings='8-9'/>
<cpu id='10' socket_id='0' core_id='1'
siblings='10-11'/>
<cpu id='11' socket_id='0' core_id='1'
siblings='10-11'/>
<cpu id='12' socket_id='0' core_id='9'
siblings='12-13'/>
<cpu id='13' socket_id='0' core_id='9'
siblings='12-13'/>
<cpu id='14' socket_id='0' core_id='10'
siblings='14-15'/>
<cpu id='15' socket_id='0' core_id='10'
siblings='14-15'/>
</cpus>
</cell>
</cells>
</topology>
</host>
....
Signed-off-by: Dario Faggioli <dario.faggioli(a)citrix.com>
---
src/libxl/libxl_conf.c | 128 ++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 127 insertions(+), 1 deletion(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index e170357..2a9bcf0 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -163,6 +163,8 @@ libxlBuildCapabilities(virArch hostarch,
static virCapsPtr
libxlMakeCapabilitiesInternal(virArch hostarch,
libxl_physinfo *phy_info,
+ libxl_numainfo *numa_info, int nr_nodes,
+ libxl_cputopology *cpu_topo, int nr_cpus,
char *capabilities)
{
char *str, *token;
@@ -173,6 +175,12 @@ libxlMakeCapabilitiesInternal(virArch hostarch,
int host_pae = 0;
struct guest_arch guest_archs[32];
int nr_guest_archs = 0;
+
+ /* For building NUMA capabilities */
+ virCapsHostNUMACellCPUPtr *cpus = NULL;
+ int *nr_cpus_node = NULL;
+ bool numa_failed = false;
+
virCapsPtr caps = NULL;
memset(guest_archs, 0, sizeof(guest_archs));
@@ -280,6 +288,100 @@ libxlMakeCapabilitiesInternal(virArch hostarch,
nr_guest_archs)) == NULL)
goto no_memory;
+ /* What about NUMA? */
+ if (!numa_info || !cpu_topo)
+ return caps;
+
+ if (VIR_ALLOC_N(cpus, nr_nodes))
+ goto no_memory;
+ memset(cpus, 0, sizeof(cpus) * nr_nodes);
+
+ if (VIR_ALLOC_N(nr_cpus_node, nr_nodes)) {
+ VIR_FREE(cpus);
+ goto no_memory;
+ }
+ memset(nr_cpus_node, 0, sizeof(nr_cpus_node) * nr_nodes);
+
+ /* For each node, prepare a list of CPUs belonging to that node */
+ for (i = 0; i < nr_cpus; i++) {
+ int node = cpu_topo[i].node;
+
+ if (cpu_topo[i].core == LIBXL_CPUTOPOLOGY_INVALID_ENTRY)
+ continue;
+
+ nr_cpus_node[node]++;
+
+ if (nr_cpus_node[node] == 1) {
+ if (VIR_ALLOC(cpus[node]) < 0) {
+ numa_failed = true;
+ goto cleanup;
+ }
+ }
+ else {
+ if (VIR_REALLOC_N(cpus[node], nr_cpus_node[node]) < 0) {
+ numa_failed = true;
+ goto cleanup;
+ }
+ }
+
+ /* Mapping between what libxl tells and what libvirt wants */
+ 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;
+ cpus[node][nr_cpus_node[node]-1].siblings = virBitmapNew(nr_cpus);
+
+ if (!cpus[node][nr_cpus_node[node]-1].siblings) {
+ virReportOOMError();
+ numa_failed = true;
+ goto cleanup;
+ }
+ }
+
+ /* Let's now populate the siblings bitmaps */
+ for (i = 0; i < nr_cpus; i++) {
+ int j, node = cpu_topo[i].node;
+
+ if (cpu_topo[i].core == LIBXL_CPUTOPOLOGY_INVALID_ENTRY)
+ continue;
+
+ for (j = 0; j < nr_cpus_node[node]; j++) {
+ if (cpus[node][j].core_id == cpu_topo[i].core)
+ ignore_value(virBitmapSetBit(cpus[node][j].siblings, i));
+ }
+ }
+
+ for (i = 0; i < nr_nodes; i++) {
+ if (numa_info[i].size == LIBXL_NUMAINFO_INVALID_ENTRY)
+ continue;
+
+ if (virCapabilitiesAddHostNUMACell(caps, i, nr_cpus_node[i],
+ numa_info[i].size / 1024,
+ cpus[i]) < 0) {
+ virCapabilitiesClearHostNUMACellCPUTopology(cpus[i],
+ nr_cpus_node[i]);
+ numa_failed = true;
+ goto cleanup;
+ }
+
+ /* This is safe, as the CPU list is now stored in the NUMA cell */
+ cpus[i] = NULL;
+ }
+
+cleanup:
+ if (numa_failed) {
+ /* Looks like something went wrong. Well, that's bad, but probably
+ * not enough to break the whole driver, so we log and carry on */
+ for (i = 0; i < nr_nodes; i++) {
+ VIR_FREE(cpus[i]);
+ }
+ VIR_WARN("Failed to retrieve and build host NUMA topology properly,\n"
+ "disabling NUMA capabilities");
+ virCapabilitiesFreeNUMAInfo(caps);
+ }
+
+ VIR_FREE(cpus);
+ VIR_FREE(nr_cpus_node);
+
return caps;
no_memory:
@@ -772,7 +874,11 @@ libxlMakeCapabilities(libxl_ctx *ctx)
{
int err;
libxl_physinfo phy_info;
+ libxl_numainfo *numa_info = NULL;
+ libxl_cputopology *cpu_topo = NULL;
const libxl_version_info *ver_info;
+ int nr_nodes = 0, nr_cpus = 0;
+ virCapsPtr caps;
err = regcomp(&xen_cap_rec, xen_cap_re, REG_EXTENDED);
if (err != 0) {
@@ -796,9 +902,29 @@ libxlMakeCapabilities(libxl_ctx *ctx)
return NULL;
}
- return libxlMakeCapabilitiesInternal(virArchFromHost(),
+ /* Let's try to fetch NUMA info, but it is not critical if we fail */
+ numa_info = libxl_get_numainfo(ctx, &nr_nodes);
+ if (numa_info == NULL)
+ VIR_WARN("libxl_get_numainfo failed to retrieve NUMA data");
+ else {
+ /* If the above failed, we'd have no NUMa caps anyway! */
+ cpu_topo = libxl_get_cpu_topology(ctx, &nr_cpus);
+ if (cpu_topo == NULL) {
+ VIR_WARN("libxl_get_cpu_topology failed to retrieve topology");
+ libxl_numainfo_list_free(numa_info, nr_nodes);
+ }
+ }
+
+ caps = libxlMakeCapabilitiesInternal(virArchFromHost(),
&phy_info,
+ numa_info, nr_nodes,
+ cpu_topo, nr_cpus,
ver_info->capabilities);
+
+ libxl_cputopology_list_free(cpu_topo, nr_cpus);
+ libxl_numainfo_list_free(numa_info, nr_nodes);
+
+ return caps;
}
int
5:47 p.m.
New subject: [libvirt] [PATCH 1/4] libxl: implement NUMA capabilities reporting
On 06/28/2013 08:32 AM, Dario Faggioli wrote:
Starting from Xen 4.2, libxl has all the bits and pieces are in
s/are in/in/
place for retrieving an adequate amount of information about the
host NUMA topology. It is therefore possible, after a bit of
shuffling, to arrange those information in the way libvirt wants
to present them to the outside world.
Therefore, with this patch, the <topology> section of the host
capabilities is properly populated, when running on Xen, so that
we can figure out whether or not we're running on a NUMA host,
and what its characteristics are.
[raistlin@Zhaman ~]$ sudo virsh --connect xen:/// capabilities
<capabilities>
<host>
<cpu>
....
<topology>
<cells num='2'>
<cell id='0'>
<memory unit='KiB'>6291456</memory>
<cpus num='8'>
<cpu id='0' socket_id='1' core_id='0'
siblings='0-1'/>
<cpu id='1' socket_id='1' core_id='0'
siblings='0-1'/>
<cpu id='2' socket_id='1' core_id='1'
siblings='2-3'/>
<cpu id='3' socket_id='1' core_id='1'
siblings='2-3'/>
<cpu id='4' socket_id='1' core_id='9'
siblings='4-5'/>
<cpu id='5' socket_id='1' core_id='9'
siblings='4-5'/>
<cpu id='6' socket_id='1' core_id='10'
siblings='6-7'/>
<cpu id='7' socket_id='1' core_id='10'
siblings='6-7'/>
</cpus>
</cell>
<cell id='1'>
<memory unit='KiB'>6881280</memory>
<cpus num='8'>
<cpu id='8' socket_id='0' core_id='0'
siblings='8-9'/>
<cpu id='9' socket_id='0' core_id='0'
siblings='8-9'/>
<cpu id='10' socket_id='0' core_id='1'
siblings='10-11'/>
<cpu id='11' socket_id='0' core_id='1'
siblings='10-11'/>
<cpu id='12' socket_id='0' core_id='9'
siblings='12-13'/>
<cpu id='13' socket_id='0' core_id='9'
siblings='12-13'/>
<cpu id='14' socket_id='0' core_id='10'
siblings='14-15'/>
<cpu id='15' socket_id='0' core_id='10'
siblings='14-15'/>
</cpus>
</cell>
</cells>
</topology>
</host>
....
Signed-off-by: Dario Faggioli <dario.faggioli(a)citrix.com>
---
src/libxl/libxl_conf.c | 128 ++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 127 insertions(+), 1 deletion(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index e170357..2a9bcf0 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -163,6 +163,8 @@ libxlBuildCapabilities(virArch hostarch,
static virCapsPtr
libxlMakeCapabilitiesInternal(virArch hostarch,
libxl_physinfo *phy_info,
+ libxl_numainfo *numa_info, int nr_nodes,
+ libxl_cputopology *cpu_topo, int nr_cpus,
The most prominent pattern in libvirt is one param per line after the first, if
they all don't fit in 80 columns. E.g.
libxlMakeCapabilitiesInternal(virArch hostarch,
libxl_physinfo *phy_info,
libxl_numainfo *numa_info,
int nr_nodes,
libxl_cputopology *cpu_topo,
int nr_cpus,
...)
char *capabilities)
{
char *str, *token;
@@ -173,6 +175,12 @@ libxlMakeCapabilitiesInternal(virArch hostarch,
int host_pae = 0;
struct guest_arch guest_archs[32];
int nr_guest_archs = 0;
+
+ /* For building NUMA capabilities */
+ virCapsHostNUMACellCPUPtr *cpus = NULL;
+ int *nr_cpus_node = NULL;
+ bool numa_failed = false;
+
No need for the extra whitespace between these local variables.
virCapsPtr caps = NULL;
memset(guest_archs, 0, sizeof(guest_archs));
@@ -280,6 +288,100 @@ libxlMakeCapabilitiesInternal(virArch hostarch,
nr_guest_archs)) == NULL)
goto no_memory;
+ /* What about NUMA? */
What about it? I think the comment should just say "Include NUMA information if
available" or similar :).
+ if (!numa_info || !cpu_topo)
+ return caps;
+
+ if (VIR_ALLOC_N(cpus, nr_nodes))
+ goto no_memory;
+ memset(cpus, 0, sizeof(cpus) * nr_nodes);
VIR_ALLOC_N will already zero-fill the memory. From its' documentation in
viralloc.h
* Allocate an array of 'count' elements, each sizeof(*ptr)
* bytes long and store the address of allocated memory in
* 'ptr'. Fill the newly allocated memory with zeros.
+
+ if (VIR_ALLOC_N(nr_cpus_node, nr_nodes)) {
+ VIR_FREE(cpus);
+ goto no_memory;
+ }
+ memset(nr_cpus_node, 0, sizeof(nr_cpus_node) * nr_nodes);
Same here.
+
+ /* For each node, prepare a list of CPUs belonging to that node */
+ for (i = 0; i < nr_cpus; i++) {
+ int node = cpu_topo[i].node;
+
+ if (cpu_topo[i].core == LIBXL_CPUTOPOLOGY_INVALID_ENTRY)
+ continue;
+
+ nr_cpus_node[node]++;
+
+ if (nr_cpus_node[node] == 1) {
+ if (VIR_ALLOC(cpus[node]) < 0) {
+ numa_failed = true;
+ goto cleanup;
+ }
+ }
+ else {
+ if (VIR_REALLOC_N(cpus[node], nr_cpus_node[node]) < 0) {
virReportOOMError() ?
+ numa_failed = true;
+ goto cleanup;
+ }
+ }
+
+ /* Mapping between what libxl tells and what libvirt wants */
+ 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;
+ cpus[node][nr_cpus_node[node]-1].siblings = virBitmapNew(nr_cpus);
+
+ if (!cpus[node][nr_cpus_node[node]-1].siblings) {
+ virReportOOMError();
+ numa_failed = true;
+ goto cleanup;
+ }
+ }
+
+ /* Let's now populate the siblings bitmaps */
+ for (i = 0; i < nr_cpus; i++) {
+ int j, node = cpu_topo[i].node;
+
+ if (cpu_topo[i].core == LIBXL_CPUTOPOLOGY_INVALID_ENTRY)
+ continue;
+
+ for (j = 0; j < nr_cpus_node[node]; j++) {
+ if (cpus[node][j].core_id == cpu_topo[i].core)
+ ignore_value(virBitmapSetBit(cpus[node][j].siblings, i));
+ }
+ }
+
+ for (i = 0; i < nr_nodes; i++) {
+ if (numa_info[i].size == LIBXL_NUMAINFO_INVALID_ENTRY)
+ continue;
+
+ if (virCapabilitiesAddHostNUMACell(caps, i, nr_cpus_node[i],
+ numa_info[i].size / 1024,
+ cpus[i]) < 0) {
On my non-NUMA test machine I have the cell memory reported as
<memory unit='KiB'>9175040</memory>
The machine has 8G of memory, running xen 4.3 rc6, with dom0_mem=1024M. 'xl
info --numa' says
total_memory : 8190
...
numa_info :
node: memsize memfree distances
0: 8960 7116 10
Why is the node memsize > total_memory?
+ virCapabilitiesClearHostNUMACellCPUTopology(cpus[i],
+ nr_cpus_node[i]);
+ numa_failed = true;
+ goto cleanup;
+ }
+
+ /* This is safe, as the CPU list is now stored in the NUMA cell */
+ cpus[i] = NULL;
+ }
+
+cleanup:
+ if (numa_failed) {
+ /* Looks like something went wrong. Well, that's bad, but probably
+ * not enough to break the whole driver, so we log and carry on */
+ for (i = 0; i < nr_nodes; i++) {
+ VIR_FREE(cpus[i]);
+ }
+ VIR_WARN("Failed to retrieve and build host NUMA topology
properly,\n"
+ "disabling NUMA capabilities");
+ virCapabilitiesFreeNUMAInfo(caps);
+ }
+
+ VIR_FREE(cpus);
+ VIR_FREE(nr_cpus_node);
Hmm, I'm beginning to think the numa additions to
libxlMakeCapabilitiesInternal() should be in a helper function, e.g.
libxlMakeNumaCapabilities(), and called when numa_info and cpu_topo are provided.
Regards,
Jim
+
return caps;
no_memory:
@@ -772,7 +874,11 @@ libxlMakeCapabilities(libxl_ctx *ctx)
{
int err;
libxl_physinfo phy_info;
+ libxl_numainfo *numa_info = NULL;
+ libxl_cputopology *cpu_topo = NULL;
const libxl_version_info *ver_info;
+ int nr_nodes = 0, nr_cpus = 0;
+ virCapsPtr caps;
err = regcomp(&xen_cap_rec, xen_cap_re, REG_EXTENDED);
if (err != 0) {
@@ -796,9 +902,29 @@ libxlMakeCapabilities(libxl_ctx *ctx)
return NULL;
}
- return libxlMakeCapabilitiesInternal(virArchFromHost(),
+ /* Let's try to fetch NUMA info, but it is not critical if we fail */
+ numa_info = libxl_get_numainfo(ctx, &nr_nodes);
+ if (numa_info == NULL)
+ VIR_WARN("libxl_get_numainfo failed to retrieve NUMA data");
+ else {
+ /* If the above failed, we'd have no NUMa caps anyway! */
+ cpu_topo = libxl_get_cpu_topology(ctx, &nr_cpus);
+ if (cpu_topo == NULL) {
+ VIR_WARN("libxl_get_cpu_topology failed to retrieve topology");
+ libxl_numainfo_list_free(numa_info, nr_nodes);
+ }
+ }
+
+ caps = libxlMakeCapabilitiesInternal(virArchFromHost(),
&phy_info,
+ numa_info, nr_nodes,
+ cpu_topo, nr_cpus,
ver_info->capabilities);
+
+ libxl_cputopology_list_free(cpu_topo, nr_cpus);
+ libxl_numainfo_list_free(numa_info, nr_nodes);
+
+ return caps;
}
int
10:49 a.m.
New subject: [libvirt] [PATCH 1/4] libxl: implement NUMA capabilities reporting
On lun, 2013-07-01 at 16:47 -0600, Jim Fehlig wrote:
On 06/28/2013 08:32 AM, Dario Faggioli wrote:
> Starting from Xen 4.2, libxl has all the bits and pieces are in
s/are in/in/
Uups! Will fix, thanks.
> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
> index e170357..2a9bcf0 100644
>
> static virCapsPtr
> libxlMakeCapabilitiesInternal(virArch hostarch,
> libxl_physinfo *phy_info,
> + libxl_numainfo *numa_info, int nr_nodes,
> + libxl_cputopology *cpu_topo, int nr_cpus,
The most prominent pattern in libvirt is one param per line after the first, if
they all don't fit in 80 columns. E.g.
libxlMakeCapabilitiesInternal(virArch hostarch,
libxl_physinfo *phy_info,
libxl_numainfo *numa_info,
int nr_nodes,
libxl_cputopology *cpu_topo,
int nr_cpus,
...)
Ok.
> char *capabilities)
> {
> char *str, *token;
> @@ -173,6 +175,12 @@ libxlMakeCapabilitiesInternal(virArch hostarch,
> int host_pae = 0;
> struct guest_arch guest_archs[32];
> int nr_guest_archs = 0;
> +
> + /* For building NUMA capabilities */
> + virCapsHostNUMACellCPUPtr *cpus = NULL;
> + int *nr_cpus_node = NULL;
> + bool numa_failed = false;
> +
No need for the extra whitespace between these local variables.
Killed.
> virCapsPtr caps = NULL;
>
> memset(guest_archs, 0, sizeof(guest_archs));
> @@ -280,6 +288,100 @@ libxlMakeCapabilitiesInternal(virArch hostarch,
> nr_guest_archs)) == NULL)
> goto no_memory;
>
> + /* What about NUMA? */
What about it? I think the comment should just say "Include NUMA information if
available" or similar :).
Agreed.
> + if (!numa_info || !cpu_topo)
> + return caps;
> +
> + if (VIR_ALLOC_N(cpus, nr_nodes))
> + goto no_memory;
> + memset(cpus, 0, sizeof(cpus) * nr_nodes);
VIR_ALLOC_N will already zero-fill the memory. From its' documentation in
viralloc.h
Right. I even looked it up (or so I remember), but then I wasn't sure it
was doing that. Anyway, thanks, I'll get rid of the explicit
zero-filling part.
> + if (nr_cpus_node[node] == 1) {
> + if (VIR_ALLOC(cpus[node]) < 0) {
> + numa_failed = true;
> + goto cleanup;
> + }
> + }
> + else {
> + if (VIR_REALLOC_N(cpus[node], nr_cpus_node[node]) < 0) {
virReportOOMError() ?
Sounds reasonable. Will do.
> +cleanup:
> + if (numa_failed) {
> + /* Looks like something went wrong. Well, that's bad, but probably
> + * not enough to break the whole driver, so we log and carry on */
> + for (i = 0; i < nr_nodes; i++) {
> + VIR_FREE(cpus[i]);
> + }
> + VIR_WARN("Failed to retrieve and build host NUMA topology
properly,\n"
> + "disabling NUMA capabilities");
> + virCapabilitiesFreeNUMAInfo(caps);
> + }
> +
> + VIR_FREE(cpus);
> + VIR_FREE(nr_cpus_node);
Hmm, I'm beginning to think the numa additions to
libxlMakeCapabilitiesInternal() should be in a helper function, e.g.
libxlMakeNumaCapabilities(), and called when numa_info and cpu_topo are provided.
Yeah, it's a bit long, isn't it. I agree with the above and will make it
an helper for v2.
Thanks and Regards,
Dario
11:53 a.m.
New subject: [libvirt] [PATCH 1/4] libxl: implement NUMA capabilities reporting
[Moving the conversation on @xen-devel and adding Jan, as that seems
more appropriate]
[Jan, this came up as I'm implementing some NUMA bits in libvirt but, as
you see, the core of Jim's question is purely about Xen]
On lun, 2013-07-01 at 16:47 -0600, Jim Fehlig wrote:
On my non-NUMA test machine I have the cell memory reported as
<memory unit='KiB'>9175040</memory>
Which is 8960, if divided by 1024, so at least it's consistent.
However...
The machine has 8G of memory, running xen 4.3 rc6, with
dom0_mem=1024M. 'xl
info --numa' says
total_memory : 8190
...
numa_info :
node: memsize memfree distances
0: 8960 7116 10
Why is the node memsize > total_memory?
Mmm... Interesting question. I really never paid attention to this...
Jan (or anyone else), is that something known and/or expected?
I went checking this down in Xen, and here's what I found.
total_memory is: info.total_pages/((1 << 20) / vinfo->pagesize)
where 'info' is what libxl_get_physinfo() provides. On its turn,
libxl_get_physinfo() is xc_physinfo(), which is XEN_SYSCTL_physinfo,
which uses total_pages, which is assigned the number of pages, down in
__start_xen(), as it results from parsing the E820 map (looking for RAM
blocks).
OTOH, memsize comes from libxl_get_numainfo(), which is xc_numainfo(),
which is XEN_SYSCTL_numainfo, which puts in memsize what
node_spanned_pages(<node_id>) says.
That seems to come, on a NUMA box, from the parsing of SRAT, and on a
non-NUMA box, from just (start_pfn-end_pfn) (in pages, of course).
Anyway, on my NUMA box, I see something similar to what Jim sees on a
non-NUMA one:
# xl info -n
...
total_memory : 12285
...
numa_info :
node: memsize memfree distances
0: 6144 23 10,20
1: 6720 104 20,10
Where 6144+6720=12864 > 12285
Looking at what Xen says during boot, I see this (the [*], [+], [=] and
[|] are mine):
(XEN) Xen-e820 RAM map:
(XEN) 0000000000000000 - 0000000000096000 (usable)
(XEN) 00000000000f0000 - 0000000000100000 (reserved) [*]
(XEN) 0000000000100000 - 00000000dbdf9c00 (usable)
(XEN) 00000000dbdf9c00 - 00000000dbe4bc00 (ACPI NVS) [+]
(XEN) 00000000dbe4bc00 - 00000000dbe4dc00 (ACPI data) [=]
(XEN) 00000000dbe4dc00 - 00000000dc000000 (reserved) [|]
(XEN) 00000000f8000000 - 00000000fd000000 (reserved)
(XEN) 00000000fe000000 - 00000000fed00400 (reserved)
(XEN) 00000000fee00000 - 00000000fef00000 (reserved)
(XEN) 00000000ffb00000 - 0000000100000000 (reserved)
(XEN) 0000000100000000 - 0000000324000000 (usable)
...
(XEN) System RAM: 12285MB (12580412kB)
And my math says that 12285MB is the sum of the areas marked as
(usable), i.e., I guess, what during parsing is recognised as
E820_RAM... which makes total sense.
A bit below that I have this:
(XEN) SRAT: Node 1 PXM 1 0-dc000000
(XEN) SRAT: Node 1 PXM 1 100000000-1a4000000
(XEN) SRAT: Node 0 PXM 0 1a4000000-324000000
Which, after the needed calculations, gives exactly the same results
than memsize-s in `xl info -n'.
Now, if I add up the [*], [+], [=] and [|] regions above, and then
subtract that from node 1's PXMs, I see that node 1 has only ~6141MB of
usable RAM, instead of 6720MB.
And in fact, 6720-6141=579, just as much as 12864-12285=579.
So, if I haven't messed up with the calculations, it looks like that
Xen, when reporting to the upper layers the amount of memory it has
available, does filter out the non-RAM regions, if this happens via
XEN_SYSCTL_physinfo (i.e., by parsing E820), while it does not do that,
if this happens via XEN_SYSCTL_numainfo (i.e., by parsing ACPI SRAT).
What I'm not sure about is whether or not that was something
known/intended and whether or not it is something we should be concerned
about.
Thanks and Regards,
Dario
--
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
9:32 a.m.
New subject: [libvirt] [PATCH 2/4] libxl: implement per NUMA node free memory reporting
By providing the implementation of nodeGetCellsFreeMemory for
the driver. This is all just a matter of properly formatting, in
a way that libvirt like, what Xen provides via libxl_get_numainfo().
[raistlin@Zhaman ~]$ sudo virsh --connect xen:/// freecell --all
0: 25004 KiB
1: 105848 KiB
--------------------
Total: 130852 KiB
Signed-off-by: Dario Faggioli <dario.faggioli(a)citrix.com>
---
src/libxl/libxl_driver.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 46 insertions(+)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 9f52394..a3a9171 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -4098,6 +4098,51 @@ libxlNodeGetFreeMemory(virConnectPtr conn)
}
static int
+libxlNodeGetCellsFreeMemory(virConnectPtr conn,
+ unsigned long long *freeMems,
+ int startCell,
+ int maxCells)
+{
+ int n, lastCell, numCells;
+ int ret = -1, nr_nodes = 0;
+ libxl_numainfo *numa_info = NULL;
+ libxlDriverPrivatePtr driver = conn->privateData;
+
+ if (virNodeGetCellsFreeMemoryEnsureACL(conn) < 0)
+ return -1;
+
+ /* Early failure is probably worth just a warning */
+ numa_info = libxl_get_numainfo(driver->ctx, &nr_nodes);
+ if (numa_info == NULL || nr_nodes == 0) {
+ VIR_WARN("libxl_get_numainfo failed to retrieve NUMA data");
+ return 0;
+ }
+
+ /* Check/sanitize the cell range */
+ if (startCell > nr_nodes) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("start cell %d out of range (0-%d)"),
+ startCell, nr_nodes);
+ goto cleanup;
+ }
+ lastCell = startCell + maxCells - 1;
+ if (lastCell > nr_nodes)
+ lastCell = nr_nodes;
+
+ for (numCells = 0, n = startCell; n <= lastCell; n++) {
+ if (numa_info[n].size == LIBXL_NUMAINFO_INVALID_ENTRY)
+ freeMems[numCells++] = 0;
+ else
+ freeMems[numCells++] = numa_info[n].free;
+ }
+ ret = numCells;
+
+cleanup:
+ libxl_numainfo_list_free(numa_info, nr_nodes);
+ return ret;
+}
+
+static int
libxlConnectDomainEventRegister(virConnectPtr conn,
virConnectDomainEventCallback callback, void *opaque,
virFreeCallback freecb)
@@ -4683,6 +4728,7 @@ static virDriver libxlDriver = {
.domainSetSchedulerParameters = libxlDomainSetSchedulerParameters, /* 0.9.0 */
.domainSetSchedulerParametersFlags = libxlDomainSetSchedulerParametersFlags, /* 0.9.2
*/
.nodeGetFreeMemory = libxlNodeGetFreeMemory, /* 0.9.0 */
+ .nodeGetCellsFreeMemory = libxlNodeGetCellsFreeMemory, /* 1.1.1 */
.connectDomainEventRegister = libxlConnectDomainEventRegister, /* 0.9.0 */
.connectDomainEventDeregister = libxlConnectDomainEventDeregister, /* 0.9.0 */
.domainManagedSave = libxlDomainManagedSave, /* 0.9.2 */
6:27 p.m.
New subject: [libvirt] [PATCH 2/4] libxl: implement per NUMA node free memory reporting
On 06/28/2013 08:32 AM, Dario Faggioli wrote:
By providing the implementation of nodeGetCellsFreeMemory for
the driver. This is all just a matter of properly formatting, in
a way that libvirt like, what Xen provides via libxl_get_numainfo().
[raistlin@Zhaman ~]$ sudo virsh --connect xen:/// freecell --all
0: 25004 KiB
1: 105848 KiB
--------------------
Total: 130852 KiB
Yeah, rather straight forward patch, which looks good on my non-NUMA machine as well
# virsh freecell --all
0: 7287216 KiB
--------------------
Total: 7287216 KiB
# virsh freecell --cellno 0
0: 7287216 KiB
ACK and pushed.
Regards,
Jim
Signed-off-by: Dario Faggioli <dario.faggioli(a)citrix.com>
---
src/libxl/libxl_driver.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 46 insertions(+)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 9f52394..a3a9171 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -4098,6 +4098,51 @@ libxlNodeGetFreeMemory(virConnectPtr conn)
}
static int
+libxlNodeGetCellsFreeMemory(virConnectPtr conn,
+ unsigned long long *freeMems,
+ int startCell,
+ int maxCells)
+{
+ int n, lastCell, numCells;
+ int ret = -1, nr_nodes = 0;
+ libxl_numainfo *numa_info = NULL;
+ libxlDriverPrivatePtr driver = conn->privateData;
+
+ if (virNodeGetCellsFreeMemoryEnsureACL(conn) < 0)
+ return -1;
+
+ /* Early failure is probably worth just a warning */
+ numa_info = libxl_get_numainfo(driver->ctx, &nr_nodes);
+ if (numa_info == NULL || nr_nodes == 0) {
+ VIR_WARN("libxl_get_numainfo failed to retrieve NUMA data");
+ return 0;
+ }
+
+ /* Check/sanitize the cell range */
+ if (startCell > nr_nodes) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("start cell %d out of range (0-%d)"),
+ startCell, nr_nodes);
+ goto cleanup;
+ }
+ lastCell = startCell + maxCells - 1;
+ if (lastCell > nr_nodes)
+ lastCell = nr_nodes;
+
+ for (numCells = 0, n = startCell; n <= lastCell; n++) {
+ if (numa_info[n].size == LIBXL_NUMAINFO_INVALID_ENTRY)
+ freeMems[numCells++] = 0;
+ else
+ freeMems[numCells++] = numa_info[n].free;
+ }
+ ret = numCells;
+
+cleanup:
+ libxl_numainfo_list_free(numa_info, nr_nodes);
+ return ret;
+}
+
+static int
libxlConnectDomainEventRegister(virConnectPtr conn,
virConnectDomainEventCallback callback, void *opaque,
virFreeCallback freecb)
@@ -4683,6 +4728,7 @@ static virDriver libxlDriver = {
.domainSetSchedulerParameters = libxlDomainSetSchedulerParameters, /* 0.9.0 */
.domainSetSchedulerParametersFlags = libxlDomainSetSchedulerParametersFlags, /*
0.9.2 */
.nodeGetFreeMemory = libxlNodeGetFreeMemory, /* 0.9.0 */
+ .nodeGetCellsFreeMemory = libxlNodeGetCellsFreeMemory, /* 1.1.1 */
.connectDomainEventRegister = libxlConnectDomainEventRegister, /* 0.9.0 */
.connectDomainEventDeregister = libxlConnectDomainEventDeregister, /* 0.9.0 */
.domainManagedSave = libxlDomainManagedSave, /* 0.9.2 */
11:57 a.m.
New subject: [libvirt] [PATCH 2/4] libxl: implement per NUMA node free memory reporting
On lun, 2013-07-01 at 17:27 -0600, Jim Fehlig wrote:
Yeah, rather straight forward patch, which looks good on my non-NUMA
machine as well
# virsh freecell --all
0: 7287216 KiB
--------------------
Total: 7287216 KiB
# virsh freecell --cellno 0
0: 7287216 KiB
ACK and pushed.
Cool, thanks! I will take this out of my queue.
Regards,
Dario
--
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
9:32 a.m.
New subject: [libvirt] [PATCH 3/4] libxl: advertise the support for VIR_TYPED_PARAM_STRING
domainGetNumaParameters has a string typed parameter, hence it
is necessary for the libxl driver to support this.
This change implements the connectSupportsFeature hook for the
libxl driver, advertising that VIR_DRV_FEATURE_TYPED_PARAM_STRING
is something good to go.
Signed-off-by: Dario Faggioli <dario.faggioli(a)citrix.com>
Cc: Eric Blake <eblake(a)redhat.com>
---
src/libxl/libxl_driver.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index a3a9171..53af609 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -4660,6 +4660,20 @@ libxlConnectListAllDomains(virConnectPtr conn,
return ret;
}
+/* Which features are supported by this driver? */
+static int
+libxlConnectSupportsFeature(virConnectPtr conn, int feature)
+{
+ if (virConnectSupportsFeatureEnsureACL(conn) < 0)
+ return -1;
+
+ switch (feature) {
+ case VIR_DRV_FEATURE_TYPED_PARAM_STRING:
+ return 1;
+ default:
+ return 0;
+ }
+}
static virDriver libxlDriver = {
@@ -4740,6 +4754,7 @@ static virDriver libxlDriver = {
.connectDomainEventRegisterAny = libxlConnectDomainEventRegisterAny, /* 0.9.0 */
.connectDomainEventDeregisterAny = libxlConnectDomainEventDeregisterAny, /* 0.9.0 */
.connectIsAlive = libxlConnectIsAlive, /* 0.9.8 */
+ .connectSupportsFeature = libxlConnectSupportsFeature, /* 1.1.1 */
};
static virStateDriver libxlStateDriver = {
6:23 p.m.
New subject: [libvirt] [PATCH 3/4] libxl: advertise the support for VIR_TYPED_PARAM_STRING
On 06/28/2013 08:32 AM, Dario Faggioli wrote:
domainGetNumaParameters has a string typed parameter, hence it
is necessary for the libxl driver to support this.
This change implements the connectSupportsFeature hook for the
libxl driver, advertising that VIR_DRV_FEATURE_TYPED_PARAM_STRING
is something good to go.
s/something good to go/supported/
Signed-off-by: Dario Faggioli <dario.faggioli(a)citrix.com>
Cc: Eric Blake <eblake(a)redhat.com>
---
src/libxl/libxl_driver.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index a3a9171..53af609 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -4660,6 +4660,20 @@ libxlConnectListAllDomains(virConnectPtr conn,
return ret;
}
+/* Which features are supported by this driver? */
+static int
+libxlConnectSupportsFeature(virConnectPtr conn, int feature)
+{
+ if (virConnectSupportsFeatureEnsureACL(conn) < 0)
+ return -1;
+
+ switch (feature) {
+ case VIR_DRV_FEATURE_TYPED_PARAM_STRING:
+ return 1;
+ default:
+ return 0;
+ }
+}
static virDriver libxlDriver = {
@@ -4740,6 +4754,7 @@ static virDriver libxlDriver = {
.connectDomainEventRegisterAny = libxlConnectDomainEventRegisterAny, /* 0.9.0 */
.connectDomainEventDeregisterAny = libxlConnectDomainEventDeregisterAny, /* 0.9.0
*/
.connectIsAlive = libxlConnectIsAlive, /* 0.9.8 */
+ .connectSupportsFeature = libxlConnectSupportsFeature, /* 1.1.1 */
};
static virStateDriver libxlStateDriver = {
As was mentioned in the reply to 0/4, libxlDomainGetSchedulerParametersFlags()
will have to accommodate this change.
Regards,
Jim
9:33 a.m.
New subject: [libvirt] [PATCH 4/4] libxl: implement virDomainGetNumaParameters
Although, having it depending on Xen >= 4.3 (by using the proper
libxl feature flag).
Xen currently implements a NUMA placement policy which is basically
the same as the 'interleaved' policy of `numactl', although it can
be applied on a subset of the available nodes. We therefore hardcode
"interleave" as 'numa_mode', and we use the newly introduced libxl
interface to figure out what nodes a domain spans ('numa_nodeset').
With this change, it is now possible to query the NUMA node
affinity of a running domain:
[raistlin@Zhaman ~]$ sudo virsh --connect xen:/// list
Id Name State
----------------------------------------------------
23 F18_x64 running
[raistlin@Zhaman ~]$ sudo virsh --connect xen:/// numatune 23
numa_mode : interleave
numa_nodeset : 1
Signed-off-by: Dario Faggioli <dario.faggioli(a)citrix.com>
---
src/libxl/libxl_driver.c | 138 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 138 insertions(+)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 53af609..9bd6d99 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -28,6 +28,7 @@
#include <math.h>
#include <libxl.h>
+#include <libxl_utils.h>
#include <fcntl.h>
#include <regex.h>
@@ -4514,6 +4515,140 @@ libxlDomainSetSchedulerParameters(virDomainPtr dom,
virTypedParameterPtr params,
return libxlDomainSetSchedulerParametersFlags(dom, params, nparams, 0);
}
+/* NUMA node affinity information is available through libxl
+ * starting from Xen 4.3. */
+#ifdef LIBXL_HAVE_DOMAIN_NODEAFFINITY
+
+/* Number of Xen NUMA parameters */
+#define LIBXL_NUMA_NPARAM 2
+
+static int
+libxlDomainGetNumaParameters(virDomainPtr dom,
+ virTypedParameterPtr params,
+ int *nparams,
+ unsigned int flags)
+{
+ libxlDriverPrivatePtr driver = dom->conn->privateData;
+ libxlDomainObjPrivatePtr priv;
+ virDomainObjPtr vm;
+
+ libxl_bitmap nodemap;
+ virBitmapPtr nodes = NULL;
+ char *nodeset = NULL;
+
+ int rc, ret = -1;
+ int i, j;
+
+ /* In Xen 4.3, it is possible to query the NUMA node affinity of a domain
+ * via libxl, but not to change it. We therefore only allow AFFECT_LIVE. */
+ virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
+ VIR_TYPED_PARAM_STRING_OKAY, -1);
+
+ libxlDriverLock(driver);
+ vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
+ libxlDriverUnlock(driver);
+
+ if (!vm) {
+ virReportError(VIR_ERR_NO_DOMAIN, "%s",
+ _("no domain with matching uuid"));
+ goto cleanup;
+ }
+
+ if (virDomainGetNumaParametersEnsureACL(dom->conn, vm->def) < 0)
+ goto cleanup;
+
+ if (!virDomainObjIsActive(vm)) {
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("Domain is not running"));
+ goto cleanup;
+ }
+
+ priv = vm->privateData;
+
+ libxl_bitmap_init(&nodemap);
+
+ if ((*nparams) == 0) {
+ *nparams = LIBXL_NUMA_NPARAM;
+ ret = 0;
+ goto cleanup;
+ }
+
+ for (i = 0; i < LIBXL_NUMA_NPARAM && i < *nparams; i++) {
+ virMemoryParameterPtr param = ¶ms[i];
+
+ switch (i) {
+ case 0:
+ /* NUMA mode */
+
+ /* Xen implements something that is is really close to numactl's
+ * 'interleave' policy (see `man 8 numactl' for details). */
+ if (virTypedParameterAssign(param, VIR_DOMAIN_NUMA_MODE,
+ VIR_TYPED_PARAM_INT,
+ VIR_DOMAIN_NUMATUNE_MEM_INTERLEAVE) < 0)
+ goto cleanup;
+
+ break;
+
+ case 1:
+ /* Node affinity */
+
+ /* Let's allocate both libxl and libvirt bitmaps */
+ if (libxl_node_bitmap_alloc(priv->ctx, &nodemap, 0) ||
+ !(nodes = virBitmapNew(libxl_get_max_nodes(priv->ctx)))) {
+ virReportOOMError();
+ goto cleanup;
+ }
+
+ rc = libxl_domain_get_nodeaffinity(priv->ctx, vm->def->id,
+ &nodemap);
+ if (rc != 0) {
+ virReportSystemError(-rc, "%s",
+ _("unable to get numa affinity"));
+ goto cleanup;
+ }
+
+ /* First, we convert libxl_bitmap into virBitmap. After that,
+ * we format virBitmap as a string that can be returned. */
+ virBitmapClearAll(nodes);
+ libxl_for_each_set_bit(j, nodemap) {
+ if (virBitmapSetBit(nodes, j)) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Node %d out of range"), j);
+ goto cleanup;
+ }
+ }
+
+ nodeset = virBitmapFormat(nodes);
+ if (!nodeset && VIR_STRDUP(nodeset, "") < 0)
+ goto cleanup;
+
+ if (virTypedParameterAssign(param, VIR_DOMAIN_NUMA_NODESET,
+ VIR_TYPED_PARAM_STRING, nodeset) < 0)
+ goto cleanup;
+
+ nodeset = NULL;
+
+ break;
+
+ default:
+ break;
+ }
+ }
+
+ if (*nparams > LIBXL_NUMA_NPARAM)
+ *nparams = LIBXL_NUMA_NPARAM;
+ ret = 0;
+
+cleanup:
+ VIR_FREE(nodeset);
+ virBitmapFree(nodes);
+ libxl_bitmap_dispose(&nodemap);
+ if (vm)
+ virObjectUnlock(vm);
+ return ret;
+}
+#endif
+
static int
libxlDomainIsActive(virDomainPtr dom)
{
@@ -4741,6 +4876,9 @@ static virDriver libxlDriver = {
.domainGetSchedulerParametersFlags = libxlDomainGetSchedulerParametersFlags, /* 0.9.2
*/
.domainSetSchedulerParameters = libxlDomainSetSchedulerParameters, /* 0.9.0 */
.domainSetSchedulerParametersFlags = libxlDomainSetSchedulerParametersFlags, /* 0.9.2
*/
+#ifdef LIBXL_HAVE_DOMAIN_NODEAFFINITY
+ .domainGetNumaParameters = libxlDomainGetNumaParameters, /* 1.1.1 */
+#endif
.nodeGetFreeMemory = libxlNodeGetFreeMemory, /* 0.9.0 */
.nodeGetCellsFreeMemory = libxlNodeGetCellsFreeMemory, /* 1.1.1 */
.connectDomainEventRegister = libxlConnectDomainEventRegister, /* 0.9.0 */
3:22 p.m.
New subject: [libvirt] [PATCH 4/4] libxl: implement virDomainGetNumaParameters
On 06/28/2013 08:33 AM, Dario Faggioli wrote:
Although, having it depending on Xen >= 4.3 (by using the proper
libxl feature flag).
Xen currently implements a NUMA placement policy which is basically
the same as the 'interleaved' policy of `numactl', although it can
be applied on a subset of the available nodes. We therefore hardcode
"interleave" as 'numa_mode', and we use the newly introduced libxl
interface to figure out what nodes a domain spans ('numa_nodeset').
With this change, it is now possible to query the NUMA node
affinity of a running domain:
[raistlin@Zhaman ~]$ sudo virsh --connect xen:/// list
Id Name State
----------------------------------------------------
23 F18_x64 running
[raistlin@Zhaman ~]$ sudo virsh --connect xen:/// numatune 23
numa_mode : interleave
numa_nodeset : 1
Signed-off-by: Dario Faggioli <dario.faggioli(a)citrix.com>
---
src/libxl/libxl_driver.c | 138 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 138 insertions(+)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 53af609..9bd6d99 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -28,6 +28,7 @@
#include <math.h>
#include <libxl.h>
+#include <libxl_utils.h>
#include <fcntl.h>
#include <regex.h>
@@ -4514,6 +4515,140 @@ libxlDomainSetSchedulerParameters(virDomainPtr dom,
virTypedParameterPtr params,
return libxlDomainSetSchedulerParametersFlags(dom, params, nparams, 0);
}
+/* NUMA node affinity information is available through libxl
+ * starting from Xen 4.3. */
+#ifdef LIBXL_HAVE_DOMAIN_NODEAFFINITY
+
+/* Number of Xen NUMA parameters */
+#define LIBXL_NUMA_NPARAM 2
Is there a similar definition in Xen? E.g. would future changes to libxl adding
more parameters, but neglecting to update here, cause problems?
+
+static int
+libxlDomainGetNumaParameters(virDomainPtr dom,
+ virTypedParameterPtr params,
+ int *nparams,
+ unsigned int flags)
+{
+ libxlDriverPrivatePtr driver = dom->conn->privateData;
+ libxlDomainObjPrivatePtr priv;
+ virDomainObjPtr vm;
+
+ libxl_bitmap nodemap;
+ virBitmapPtr nodes = NULL;
+ char *nodeset = NULL;
+
+ int rc, ret = -1;
+ int i, j;
No need for the extra whitespace between these local variables.
+
+ /* In Xen 4.3, it is possible to query the NUMA node affinity of a domain
+ * via libxl, but not to change it. We therefore only allow AFFECT_LIVE. */
+ virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
+ VIR_TYPED_PARAM_STRING_OKAY, -1);
+
+ libxlDriverLock(driver);
+ vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
+ libxlDriverUnlock(driver);
+
+ if (!vm) {
+ virReportError(VIR_ERR_NO_DOMAIN, "%s",
+ _("no domain with matching uuid"));
+ goto cleanup;
+ }
+
+ if (virDomainGetNumaParametersEnsureACL(dom->conn, vm->def) < 0)
+ goto cleanup;
+
+ if (!virDomainObjIsActive(vm)) {
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("Domain is not running"));
+ goto cleanup;
+ }
+
+ priv = vm->privateData;
+
+ libxl_bitmap_init(&nodemap);
+
+ if ((*nparams) == 0) {
+ *nparams = LIBXL_NUMA_NPARAM;
+ ret = 0;
+ goto cleanup;
+ }
+
+ for (i = 0; i < LIBXL_NUMA_NPARAM && i < *nparams; i++) {
+ virMemoryParameterPtr param = ¶ms[i];
+
+ switch (i) {
+ case 0:
+ /* NUMA mode */
+
+ /* Xen implements something that is is really close to numactl's
+ * 'interleave' policy (see `man 8 numactl' for details). */
+ if (virTypedParameterAssign(param, VIR_DOMAIN_NUMA_MODE,
+ VIR_TYPED_PARAM_INT,
+ VIR_DOMAIN_NUMATUNE_MEM_INTERLEAVE) < 0)
+ goto cleanup;
+
+ break;
+
+ case 1:
+ /* Node affinity */
+
+ /* Let's allocate both libxl and libvirt bitmaps */
+ if (libxl_node_bitmap_alloc(priv->ctx, &nodemap, 0) ||
+ !(nodes = virBitmapNew(libxl_get_max_nodes(priv->ctx)))) {
+ virReportOOMError();
+ goto cleanup;
+ }
+
+ rc = libxl_domain_get_nodeaffinity(priv->ctx, vm->def->id,
+ &nodemap);
Fits on one line, albeit with nothing to spare :).
+ if (rc != 0) {
+ virReportSystemError(-rc, "%s",
+ _("unable to get numa affinity"));
+ goto cleanup;
+ }
+
+ /* First, we convert libxl_bitmap into virBitmap. After that,
+ * we format virBitmap as a string that can be returned. */
+ virBitmapClearAll(nodes);
+ libxl_for_each_set_bit(j, nodemap) {
+ if (virBitmapSetBit(nodes, j)) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Node %d out of range"), j);
+ goto cleanup;
+ }
+ }
+
+ nodeset = virBitmapFormat(nodes);
+ if (!nodeset && VIR_STRDUP(nodeset, "") < 0)
+ goto cleanup;
+
+ if (virTypedParameterAssign(param, VIR_DOMAIN_NUMA_NODESET,
+ VIR_TYPED_PARAM_STRING, nodeset) < 0)
+ goto cleanup;
+
+ nodeset = NULL;
+
+ break;
+
+ default:
+ break;
+ }
+ }
+
+ if (*nparams > LIBXL_NUMA_NPARAM)
+ *nparams = LIBXL_NUMA_NPARAM;
+ ret = 0;
+
+cleanup:
+ VIR_FREE(nodeset);
+ virBitmapFree(nodes);
+ libxl_bitmap_dispose(&nodemap);
+ if (vm)
+ virObjectUnlock(vm);
+ return ret;
+}
+#endif
+
static int
libxlDomainIsActive(virDomainPtr dom)
{
@@ -4741,6 +4876,9 @@ static virDriver libxlDriver = {
.domainGetSchedulerParametersFlags = libxlDomainGetSchedulerParametersFlags, /*
0.9.2 */
.domainSetSchedulerParameters = libxlDomainSetSchedulerParameters, /* 0.9.0 */
.domainSetSchedulerParametersFlags = libxlDomainSetSchedulerParametersFlags, /*
0.9.2 */
+#ifdef LIBXL_HAVE_DOMAIN_NODEAFFINITY
+ .domainGetNumaParameters = libxlDomainGetNumaParameters, /* 1.1.1 */
+#endif
.nodeGetFreeMemory = libxlNodeGetFreeMemory, /* 0.9.0 */
.nodeGetCellsFreeMemory = libxlNodeGetCellsFreeMemory, /* 1.1.1 */
.connectDomainEventRegister = libxlConnectDomainEventRegister, /* 0.9.0 */
Looks good with the exception of these minor nits, but needs to go along with an
updated 3/4.
Regards,
Jim
3:35 p.m.
New subject: [libvirt] [PATCH 4/4] libxl: implement virDomainGetNumaParameters
On 07/02/2013 02:22 PM, Jim Fehlig wrote:
On 06/28/2013 08:33 AM, Dario Faggioli wrote:
> Although, having it depending on Xen >= 4.3 (by using the proper
> libxl feature flag).
>
> Xen currently implements a NUMA placement policy which is basically
> the same as the 'interleaved' policy of `numactl', although it can
> be applied on a subset of the available nodes. We therefore hardcode
> "interleave" as 'numa_mode', and we use the newly introduced libxl
> interface to figure out what nodes a domain spans ('numa_nodeset').
>
> With this change, it is now possible to query the NUMA node
> affinity of a running domain:
>
> [raistlin@Zhaman ~]$ sudo virsh --connect xen:/// list
> Id Name State
> ----------------------------------------------------
> 23 F18_x64 running
>
> [raistlin@Zhaman ~]$ sudo virsh --connect xen:/// numatune 23
> numa_mode : interleave
> numa_nodeset : 1
>
> Signed-off-by: Dario Faggioli <dario.faggioli(a)citrix.com>
> ---
> src/libxl/libxl_driver.c | 138 ++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 138 insertions(+)
>
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index 53af609..9bd6d99 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -28,6 +28,7 @@
> #include <math.h>
> #include <libxl.h>
> +#include <libxl_utils.h>
> #include <fcntl.h>
> #include <regex.h>
> @@ -4514,6 +4515,140 @@ libxlDomainSetSchedulerParameters(virDomainPtr dom,
> virTypedParameterPtr params,
> return libxlDomainSetSchedulerParametersFlags(dom, params, nparams, 0);
> }
> +/* NUMA node affinity information is available through libxl
> + * starting from Xen 4.3. */
> +#ifdef LIBXL_HAVE_DOMAIN_NODEAFFINITY
> +
> +/* Number of Xen NUMA parameters */
> +#define LIBXL_NUMA_NPARAM 2
Is there a similar definition in Xen? E.g. would future changes to libxl
adding more parameters, but neglecting to update here, cause problems?
BTW, with cppi installed this fails 'make syntax-check'
preprocessor_indentation
cppi: src/libxl/libxl_driver.c: line 4523: not properly indented
maint.mk: incorrect preprocessor indentation
make: *** [sc_preprocessor_indentation] Error 1
Preprocessor nesting is indented as follows
#if ...
# define ...
# if ...
# define ...
# endif
#endif
Regards,
Jim
9:43 a.m.
New subject: [libvirt] [PATCH 4/4] libxl: implement virDomainGetNumaParameters
On mar, 2013-07-02 at 14:35 -0600, Jim Fehlig wrote:
BTW, with cppi installed this fails 'make syntax-check'
preprocessor_indentation
cppi: src/libxl/libxl_driver.c: line 4523: not properly indented
maint.mk: incorrect preprocessor indentation
make: *** [sc_preprocessor_indentation] Error 1
Preprocessor nesting is indented as follows
#if ...
# define ...
# if ...
# define ...
# endif
#endif
Oh, I see... I indeed tried to install cppi, but `yum search' found
nothing, probably because of some typo, since I'm finding (and I have
installed it) now! :-)
Will fix this.
Thanks and Regards,
Dario
--
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
12:21 p.m.
New subject: [libvirt] [PATCH 4/4] libxl: implement virDomainGetNumaParameters
On mar, 2013-07-02 at 14:22 -0600, Jim Fehlig wrote:
On 06/28/2013 08:33 AM, Dario Faggioli wrote:
>
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index 53af609..9bd6d99 100644
>
> +/* NUMA node affinity information is available through libxl
> + * starting from Xen 4.3. */
> +#ifdef LIBXL_HAVE_DOMAIN_NODEAFFINITY
> +
> +/* Number of Xen NUMA parameters */
> +#define LIBXL_NUMA_NPARAM 2
Is there a similar definition in Xen? E.g. would future changes to libxl adding
more parameters, but neglecting to update here, cause problems?
There is nothing like this in Xen. I went for this #define based
approach because that is right what the QEMU driver does for NUMA
parameters and, also, what libxl driver does for scheduler parameters.
I therefore think that this could be considered safe, from that point of
view. What could happen is that more NUMA (and, perhaps, even more
scheduler) parameters can pop up in the future. However, given libxl
commitment for API stability, this will be advertised with something
like LIBXL_HAVE_THIS_NEW_NUMA_PARAM symbol.
That means we, as soon as we wan to support it, will end up with
something like:
#ifdef LIBXL_HAVE_THIS_NEW_NUMA_PARAM
# define LIBXL_NUMA_NPARAM 3
#else
# define LIBXL_NUMA_NPARAM 2
#endif
Is that reasonable?
> +
> +static int
> +libxlDomainGetNumaParameters(virDomainPtr dom,
> + virTypedParameterPtr params,
> + int *nparams,
> + unsigned int flags)
> +{
> + libxlDriverPrivatePtr driver = dom->conn->privateData;
> + libxlDomainObjPrivatePtr priv;
> + virDomainObjPtr vm;
> +
> + libxl_bitmap nodemap;
> + virBitmapPtr nodes = NULL;
> + char *nodeset = NULL;
> +
> + int rc, ret = -1;
> + int i, j;
No need for the extra whitespace between these local variables.
Ok, done.
> + case 1:
> + /* Node affinity */
> +
> + /* Let's allocate both libxl and libvirt bitmaps */
> + if (libxl_node_bitmap_alloc(priv->ctx, &nodemap, 0) ||
> + !(nodes = virBitmapNew(libxl_get_max_nodes(priv->ctx)))) {
> + virReportOOMError();
> + goto cleanup;
> + }
> +
> + rc = libxl_domain_get_nodeaffinity(priv->ctx, vm->def->id,
> + &nodemap);
Fits on one line, albeit with nothing to spare :).
Does it? According to my editor here, if I put it in just one line that
ends at 82. :-(
I'm therefore changing it in
rc = libxl_domain_get_nodeaffinity(priv->ctx,
vm->def->id,
&nodemap);
As you mentioned in another e-mail.
> @@ -4741,6 +4876,9 @@ static virDriver libxlDriver = {
> .domainGetSchedulerParametersFlags = libxlDomainGetSchedulerParametersFlags,
/* 0.9.2 */
> .domainSetSchedulerParameters = libxlDomainSetSchedulerParameters, /* 0.9.0
*/
> .domainSetSchedulerParametersFlags = libxlDomainSetSchedulerParametersFlags,
/* 0.9.2 */
> +#ifdef LIBXL_HAVE_DOMAIN_NODEAFFINITY
> + .domainGetNumaParameters = libxlDomainGetNumaParameters, /* 1.1.1 */
> +#endif
> .nodeGetFreeMemory = libxlNodeGetFreeMemory, /* 0.9.0 */
> .nodeGetCellsFreeMemory = libxlNodeGetCellsFreeMemory, /* 1.1.1 */
> .connectDomainEventRegister = libxlConnectDomainEventRegister, /* 0.9.0 */
Looks good with the exception of these minor nits, but needs to go along with an
updated 3/4.
Sure.
Thanks and Regards,
Dario
--
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
4:20 p.m.
On 06/28/2013 08:32 AM, Dario Faggioli wrote:
Hi Jim, Everyone,
First patch series, so maybe a small introduction is required: I'm Dario and I
work for Citrix on improving the NUMA support of Xen.
Welcome Dario!
This patch series implements some of the missing bits and pieces, in
the libxl
driver, regarding obtaining per-host and per-domain NUMA related information.
It's not the full libvirt NUMA interface, since we don't have all we would need
for that in Xen yet (we will in the next version, 4.4), but it's certainly
better than having nothing! :-)
Yep, incremental improvements are fine.
Basically, I'm enhancing capability reporting, to cover NUMA
topology (patch
01), and I'm implementing nodeGetCellsFreeMemory (patch 02) and
virDomainGetNumaParameters (patch 04) for the libxl driver. This last one
requires at least Xen 4.3, so I put the implementation within the proper
#ifdef-ery.
Ok, I'll comment in the individual patches too.
What I'm really not sure about is patch 03, which is something I
need if I want
patch 04 to function properly. Basically it is about advertising that the libxl
driver supports VIR_TYPED_PARAM_STRING. I looked at how that is done in the
qemu driver, but I'm not entirely sure I completely understood the logic behind
it, so, please, tell me if I missed or misinterpreted anything!
I think we'll need another libvirt dev more familiar with this to verify, but
afaict advertising that the driver supports VIR_TYPED_PARAM_STRING is required
when returning a string in virTypedParam, for compatibility with older libvirt
clients. Without it, strings wouldn't be returned to newer clients that support
VIR_TYPED_PARAM_STRING.
In particular,
should I have added more of those "flags &= ~VIR_TYPED_PARAM_STRING_OKAY;"
statements, as it happens in the qemu driver? If yes, in which functions?
Once the driver advertises VIR_TYPED_PARAM_STRING, I think the functions taking
virTypedParamPtr as a parameter must accommodate that, at least the getters.
E.g. qemuDomainGetSchedulerParametersFlags() in the qemu driver has
virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
VIR_DOMAIN_AFFECT_CONFIG |
VIR_TYPED_PARAM_STRING_OKAY, -1);
/* We don't return strings, and thus trivially support this flag. */
flags &= ~VIR_TYPED_PARAM_STRING_OKAY;
I think the same applies for libxlDomainGetSchedulerParametersFlags(), which
looks to be the only getter function in the libxl driver taking virTypedParam.
Finally, allow me to say that it was a while that I wanted start
hacking a bit
on libvirt. I'm really glad I've eventually been able to do so, and I
definitely plan to continue (with particular focus on NUMA related stuff).
Great to hear and looking forward to your work!
Regards,
Jim
4:27 p.m.
On 07/01/2013 03:20 PM, Jim Fehlig wrote:
> What I'm really not sure about is patch 03, which is
something I need
> if I want
> patch 04 to function properly. Basically it is about advertising that
> the libxl
> driver supports VIR_TYPED_PARAM_STRING. I looked at how that is done
> in the
> qemu driver, but I'm not entirely sure I completely understood the
> logic behind
> it, so, please, tell me if I missed or misinterpreted anything!
I think we'll need another libvirt dev more familiar with this to
verify, but afaict advertising that the driver supports
VIR_TYPED_PARAM_STRING is required when returning a string in
virTypedParam, for compatibility with older libvirt clients. Without it,
strings wouldn't be returned to newer clients that support
VIR_TYPED_PARAM_STRING.
Any new API added after VIR_TYPED_PARAM_STRING was added can
unconditionally assume that all callers are prepared to handle the
string. But for APIs that were created before string support was added,
even if your driver doesn't implement the API until now, you are correct
that you must advertise support for strings before returning any
strings. About the only API I know of off-hand that uses typed
parameters but was added after string support is the latest incantation
of migration via parameters.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
12:01 p.m.
On lun, 2013-07-01 at 15:27 -0600, Eric Blake wrote:
On 07/01/2013 03:20 PM, Jim Fehlig wrote:
>> What I'm really not sure about is patch 03, which is something I need
>> if I want
>> patch 04 to function properly. Basically it is about advertising that
>> the libxl
>> driver supports VIR_TYPED_PARAM_STRING. I looked at how that is done
>> in the
>> qemu driver, but I'm not entirely sure I completely understood the
>> logic behind
>> it, so, please, tell me if I missed or misinterpreted anything!
>
> I think we'll need another libvirt dev more familiar with this to
> verify, but afaict advertising that the driver supports
> VIR_TYPED_PARAM_STRING is required when returning a string in
> virTypedParam, for compatibility with older libvirt clients. Without it,
> strings wouldn't be returned to newer clients that support
> VIR_TYPED_PARAM_STRING.
Any new API added after VIR_TYPED_PARAM_STRING was added can
unconditionally assume that all callers are prepared to handle the
string. But for APIs that were created before string support was added,
even if your driver doesn't implement the API until now, you are correct
that you must advertise support for strings before returning any
strings.
Ok, I think I've understood this better now... I'll do as Jim suggest,
and add the proper bits to the functions, in the libxl driver, that need
them.
Thanks and Regards,
Dario
--
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
8:26 p.m.
On lun, 2013-07-01 at 15:20 -0600, Jim Fehlig wrote:
On 06/28/2013 08:32 AM, Dario Faggioli wrote:
> Hi Jim, Everyone,
>
> First patch series, so maybe a small introduction is required: I'm Dario and I
> work for Citrix on improving the NUMA support of Xen.
Welcome Dario!
Hey! :-)
> Basically, I'm enhancing capability reporting, to cover NUMA
topology (patch
> 01), and I'm implementing nodeGetCellsFreeMemory (patch 02) and
> virDomainGetNumaParameters (patch 04) for the libxl driver. This last one
> requires at least Xen 4.3, so I put the implementation within the proper
> #ifdef-ery.
Ok, I'll comment in the individual patches too.
So, I just released v2, in which I think I addressed all the comments
raised in this round... Let me know what you think.
Thanks again and Regards,
Dario
--
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
4159
days inactive
4165
days old
18 comments
3 participants
participants (3)
-
Dario Faggioli -
Eric Blake -
Jim Fehlig