[libvirt] [PATCH 0/3] Expose distance between host NUMA nodes

*** BLURB HERE *** Michal Privoznik (3): virnuma.c: Fix some comments virnuma: Introduce virNumaGetDistances virCaps: Expose distance between host NUMA nodes docs/schemas/capability.rng | 11 ++++++++ src/conf/capabilities.c | 13 +++++++++- src/conf/capabilities.h | 13 +++++++++- src/libvirt_private.syms | 1 + src/libxl/libxl_conf.c | 4 +-- src/nodeinfo.c | 61 ++++++++++++++++++++++++++++++++++++++++++--- src/test/test_driver.c | 2 +- src/util/virnuma.c | 54 +++++++++++++++++++++++++++++++++++++-- src/util/virnuma.h | 3 +++ src/xen/xend_internal.c | 3 ++- tests/vircapstest.c | 4 +-- 11 files changed, 156 insertions(+), 13 deletions(-) -- 2.0.0

In 9dd02965 the virNumaGetNodeMemory was introduced, however the comment describing the function mentions virNumaGetNodeMemorySize. And there's one typo in virNumaIsAvailable() description. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/util/virnuma.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util/virnuma.c b/src/util/virnuma.c index bf3b9e6..1e099eb 100644 --- a/src/util/virnuma.c +++ b/src/util/virnuma.c @@ -193,7 +193,7 @@ virNumaIsAvailable(void) * Get the highest node number available on the current system. * (See the node numbers in /sys/devices/system/node/ ). * - * Returns the highes NUMA node id on success, -1 on error. + * Returns the highest NUMA node id on success, -1 on error. */ int virNumaGetMaxNode(void) @@ -217,7 +217,7 @@ virNumaGetMaxNode(void) /** - * virNumaGetNodeMemorySize: + * virNumaGetNodeMemory: * @node: identifier of the requested NUMA node * @memsize: returns the total size of memory in the NUMA node * @memfree: returns the total free memory in a NUMA node -- 2.0.0

On Mon, Jun 02, 2014 at 02:15:58PM +0200, Michal Privoznik wrote:
In 9dd02965 the virNumaGetNodeMemory was introduced, however the comment describing the function mentions virNumaGetNodeMemorySize. And there's one typo in virNumaIsAvailable() description.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/util/virnuma.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/util/virnuma.c b/src/util/virnuma.c index bf3b9e6..1e099eb 100644 --- a/src/util/virnuma.c +++ b/src/util/virnuma.c @@ -193,7 +193,7 @@ virNumaIsAvailable(void) * Get the highest node number available on the current system. * (See the node numbers in /sys/devices/system/node/ ). * - * Returns the highes NUMA node id on success, -1 on error. + * Returns the highest NUMA node id on success, -1 on error. */ int virNumaGetMaxNode(void) @@ -217,7 +217,7 @@ virNumaGetMaxNode(void)
/** - * virNumaGetNodeMemorySize: + * virNumaGetNodeMemory: * @node: identifier of the requested NUMA node * @memsize: returns the total size of memory in the NUMA node * @memfree: returns the total free memory in a NUMA node
ACK Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

The API gets a NUMA node and find distances to other nodes. The distances are returned in an array. If an item X within the array equals to value of zero, then there's no such node as X. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virnuma.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++ src/util/virnuma.h | 3 +++ 3 files changed, 54 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index cb635cd..6168f76 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1652,6 +1652,7 @@ virNodeSuspendGetTargetMask; virDomainNumatuneMemModeTypeFromString; virDomainNumatuneMemModeTypeToString; virNumaGetAutoPlacementAdvice; +virNumaGetDistances; virNumaGetMaxNode; virNumaGetNodeMemory; virNumaIsAvailable; diff --git a/src/util/virnuma.c b/src/util/virnuma.c index 1e099eb..68b2698 100644 --- a/src/util/virnuma.c +++ b/src/util/virnuma.c @@ -217,6 +217,56 @@ virNumaGetMaxNode(void) /** + * virNumaGetDistances: + * @node: identifier of the requested NUMA node + * @distances: array of distances to sibling nodes + * @ndistances: size of @distances + * + * Get array of distances to sibling nodes from @node. If a + * distances[x] equals to zero, the node x is not enabled or + * doesn't exist. As a special case, if @node itself refers to + * disabled or nonexistent NUMA node, then @distances and + * @ndistances are set to NULL and zero respectively. + * + * Returns 0 on success, -1 otherwise. + */ +int +virNumaGetDistances(int node, + int **distances, + int *ndistances) +{ + int ret = -1; + int max_node; + size_t i; + + if (!numa_bitmask_isbitset(numa_nodes_ptr, node)) { + VIR_DEBUG("Node %d does not exist", node); + *distances = NULL; + *ndistances = 0; + return 0; + } + + if ((max_node = virNumaGetMaxNode()) < 0) + goto cleanup; + + if (VIR_ALLOC_N(*distances, max_node) < 0) + goto cleanup; + + *ndistances = max_node + 1; + + for (i = 0; i<= max_node; i++) { + if (!numa_bitmask_isbitset(numa_nodes_ptr, i)) + continue; + + (*distances)[i] = numa_distance(node, i); + } + + ret = 0; + cleanup: + return ret; +} + +/** * virNumaGetNodeMemory: * @node: identifier of the requested NUMA node * @memsize: returns the total size of memory in the NUMA node diff --git a/src/util/virnuma.h b/src/util/virnuma.h index 8464b19..fe1e966 100644 --- a/src/util/virnuma.h +++ b/src/util/virnuma.h @@ -58,6 +58,9 @@ int virNumaSetupMemoryPolicy(virNumaTuneDef numatune, bool virNumaIsAvailable(void); int virNumaGetMaxNode(void); +int virNumaGetDistances(int node, + int **distances, + int *ndistances); int virNumaGetNodeMemory(int node, unsigned long long *memsize, unsigned long long *memfree); -- 2.0.0

On Mon, Jun 02, 2014 at 02:15:59PM +0200, Michal Privoznik wrote:
The API gets a NUMA node and find distances to other nodes. The distances are returned in an array. If an item X within the array equals to value of zero, then there's no such node as X.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virnuma.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++ src/util/virnuma.h | 3 +++ 3 files changed, 54 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index cb635cd..6168f76 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1652,6 +1652,7 @@ virNodeSuspendGetTargetMask; virDomainNumatuneMemModeTypeFromString; virDomainNumatuneMemModeTypeToString; virNumaGetAutoPlacementAdvice; +virNumaGetDistances; virNumaGetMaxNode; virNumaGetNodeMemory; virNumaIsAvailable; diff --git a/src/util/virnuma.c b/src/util/virnuma.c index 1e099eb..68b2698 100644 --- a/src/util/virnuma.c +++ b/src/util/virnuma.c @@ -217,6 +217,56 @@ virNumaGetMaxNode(void)
/** + * virNumaGetDistances: + * @node: identifier of the requested NUMA node + * @distances: array of distances to sibling nodes + * @ndistances: size of @distances + * + * Get array of distances to sibling nodes from @node. If a + * distances[x] equals to zero, the node x is not enabled or + * doesn't exist. As a special case, if @node itself refers to + * disabled or nonexistent NUMA node, then @distances and + * @ndistances are set to NULL and zero respectively.
I think it'd be worth stating what the distance is between a node and itself, since that's another special case. I'd assumed that the distince between a ndoe and itself would be zero, but your next patch shows that it would be 10 which I find a bit bizarre. Presumably that's just what numactl, or perhaps the kernel, reports ? If we are relying on numactl's value ranges, then we should be clear about this to help people porting to non-Linux platforms. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On Mon, Jun 02, 2014 at 04:57:42PM +0100, Daniel P. Berrange wrote:
On Mon, Jun 02, 2014 at 02:15:59PM +0200, Michal Privoznik wrote:
The API gets a NUMA node and find distances to other nodes. The distances are returned in an array. If an item X within the array equals to value of zero, then there's no such node as X.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virnuma.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++ src/util/virnuma.h | 3 +++ 3 files changed, 54 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index cb635cd..6168f76 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1652,6 +1652,7 @@ virNodeSuspendGetTargetMask; virDomainNumatuneMemModeTypeFromString; virDomainNumatuneMemModeTypeToString; virNumaGetAutoPlacementAdvice; +virNumaGetDistances; virNumaGetMaxNode; virNumaGetNodeMemory; virNumaIsAvailable; diff --git a/src/util/virnuma.c b/src/util/virnuma.c index 1e099eb..68b2698 100644 --- a/src/util/virnuma.c +++ b/src/util/virnuma.c @@ -217,6 +217,56 @@ virNumaGetMaxNode(void)
/** + * virNumaGetDistances: + * @node: identifier of the requested NUMA node + * @distances: array of distances to sibling nodes + * @ndistances: size of @distances + * + * Get array of distances to sibling nodes from @node. If a + * distances[x] equals to zero, the node x is not enabled or + * doesn't exist. As a special case, if @node itself refers to + * disabled or nonexistent NUMA node, then @distances and + * @ndistances are set to NULL and zero respectively.
I think it'd be worth stating what the distance is between a node and itself, since that's another special case. I'd assumed that the distince between a ndoe and itself would be zero, but your next patch shows that it would be 10 which I find a bit bizarre. Presumably that's just what numactl, or perhaps the kernel, reports ?
If we are relying on numactl's value ranges, then we should be clear about this to help people porting to non-Linux platforms.
Answering my own question.... these values come from the kernel which in turn gets them from the BIOS SLIT tables. '10' is the magic value for "local distance" and everything large is a rough indication of memory access time penalty. So '20' means the remote node memory access is x2 slower. Since it is BIOS defined, we should be fine using these values as-is in the libvirt XML. Lets just document '10' as being the local node value and that other values are a scale factor relative to this. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

If user or management application wants to create a guest, it may be useful to know the cost of internode latencies before the guest resources are pinned. For example: <capabilities> <host> ... <topology> <cells num='2'> <cell id='0'> <memory unit='KiB'>4004132</memory> <cell id='0' distance='10'/> <cell id='1' distance='20'/> <cpus num='2'> <cpu id='0' socket_id='0' core_id='0' siblings='0'/> <cpu id='2' socket_id='0' core_id='2' siblings='2'/> </cpus> </cell> <cell id='1'> <memory unit='KiB'>4030064</memory> <cell id='0' distance='20'/> <cell id='1' distance='10'/> <cpus num='2'> <cpu id='1' socket_id='0' core_id='0' siblings='1'/> <cpu id='3' socket_id='0' core_id='2' siblings='3'/> </cpus> </cell> </cells> </topology> ... </host> ... </capabilities> we can see the distance from node1 to node0 is 20 and within nodes 10. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- docs/schemas/capability.rng | 11 ++++++++ src/conf/capabilities.c | 13 +++++++++- src/conf/capabilities.h | 13 +++++++++- src/libxl/libxl_conf.c | 4 +-- src/nodeinfo.c | 61 ++++++++++++++++++++++++++++++++++++++++++--- src/test/test_driver.c | 2 +- src/xen/xend_internal.c | 3 ++- tests/vircapstest.c | 4 +-- 8 files changed, 100 insertions(+), 11 deletions(-) diff --git a/docs/schemas/capability.rng b/docs/schemas/capability.rng index d2d9776..63b53b9 100644 --- a/docs/schemas/capability.rng +++ b/docs/schemas/capability.rng @@ -188,6 +188,17 @@ <ref name='memory'/> </optional> + <zeroOrMore> + <element name='cell'> + <attribute name='id'> + <ref name='unsignedInt'/> + </attribute> + <attribute name='distance'> + <ref name='unsignedInt'/> + </attribute> + </element> + </zeroOrMore> + <optional> <element name='cpus'> <attribute name='num'> diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c index cf474d7..e66abb2 100644 --- a/src/conf/capabilities.c +++ b/src/conf/capabilities.c @@ -107,6 +107,7 @@ virCapabilitiesFreeHostNUMACell(virCapsHostNUMACellPtr cell) virCapabilitiesClearHostNUMACellCPUTopology(cell->cpus, cell->ncpus); VIR_FREE(cell->cpus); + VIR_FREE(cell->siblings); VIR_FREE(cell); } @@ -286,8 +287,10 @@ int virCapabilitiesAddHostNUMACell(virCapsPtr caps, int num, int ncpus, + int nsiblings, unsigned long long mem, - virCapsHostNUMACellCPUPtr cpus) + virCapsHostNUMACellCPUPtr cpus, + virCapsHostNUMACellSiblingInfoPtr siblings) { virCapsHostNUMACellPtr cell; @@ -302,6 +305,8 @@ virCapabilitiesAddHostNUMACell(virCapsPtr caps, cell->num = num; cell->mem = mem; cell->cpus = cpus; + cell->siblings = siblings; + cell->nsiblings = nsiblings; caps->host.numaCell[caps->host.nnumaCell++] = cell; @@ -766,6 +771,12 @@ virCapabilitiesFormatNUMATopology(virBufferPtr buf, virBufferAsprintf(buf, "<memory unit='KiB'>%llu</memory>\n", cells[i]->mem); + for (j = 0; j < cells[i]->nsiblings; j++) { + virBufferAsprintf(buf, "<cell id='%d' distance='%d'/>\n", + cells[i]->siblings[j].node, + cells[i]->siblings[j].distance); + } + virBufferAsprintf(buf, "<cpus num='%d'>\n", cells[i]->ncpus); virBufferAdjustIndent(buf, 2); for (j = 0; j < cells[i]->ncpus; j++) { diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h index ba99e1a..5da31a8 100644 --- a/src/conf/capabilities.h +++ b/src/conf/capabilities.h @@ -95,6 +95,13 @@ struct _virCapsHostNUMACellCPU { virBitmapPtr siblings; }; +typedef struct _virCapsHostNUMACellSiblingInfo virCapsHostNUMACellSiblingInfo; +typedef virCapsHostNUMACellSiblingInfo *virCapsHostNUMACellSiblingInfoPtr; +struct _virCapsHostNUMACellSiblingInfo { + int node; /* foreign NUMA node */ + unsigned int distance; /* distance to the node */ +}; + typedef struct _virCapsHostNUMACell virCapsHostNUMACell; typedef virCapsHostNUMACell *virCapsHostNUMACellPtr; struct _virCapsHostNUMACell { @@ -102,6 +109,8 @@ struct _virCapsHostNUMACell { int ncpus; unsigned long long mem; /* in kibibytes */ virCapsHostNUMACellCPUPtr cpus; + int nsiblings; + virCapsHostNUMACellSiblingInfoPtr siblings; }; typedef struct _virCapsHostSecModelLabel virCapsHostSecModelLabel; @@ -194,8 +203,10 @@ extern int virCapabilitiesAddHostNUMACell(virCapsPtr caps, int num, int ncpus, + int nsiblings, unsigned long long mem, - virCapsHostNUMACellCPUPtr cpus); + virCapsHostNUMACellCPUPtr cpus, + virCapsHostNUMACellSiblingInfoPtr siblings); extern int diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index b7fed7f..1db570d 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -208,8 +208,8 @@ libxlCapsInitNuma(libxl_ctx *ctx, virCapsPtr caps) continue; if (virCapabilitiesAddHostNUMACell(caps, i, nr_cpus_node[i], - numa_info[i].size / 1024, - cpus[i]) < 0) { + 0, numa_info[i].size / 1024, + cpus[i], NULL) < 0) { virCapabilitiesClearHostNUMACellCPUTopology(cpus[i], nr_cpus_node[i]); goto cleanup; diff --git a/src/nodeinfo.c b/src/nodeinfo.c index 56f2b02..f76a91e 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -1644,9 +1644,9 @@ nodeCapsInitNUMAFake(virCapsPtr caps ATTRIBUTE_UNUSED) } if (virCapabilitiesAddHostNUMACell(caps, 0, - ncpus, + ncpus, 0, nodeinfo.memory, - cpus) < 0) + cpus, NULL) < 0) goto error; return 0; @@ -1748,6 +1748,53 @@ virNodeCapsFillCPUInfo(int cpu_id ATTRIBUTE_UNUSED, #endif } +static int +virNodeCapsGetSiblingInfo(int node, + virCapsHostNUMACellSiblingInfoPtr *siblings, + int *nsiblings) +{ + virCapsHostNUMACellSiblingInfoPtr tmp = NULL; + int tmp_size = 0; + int ret = -1; + int *distances = NULL; + int ndistances = 0; + size_t i; + + if (virNumaGetDistances(node, &distances, &ndistances) < 0) + goto cleanup; + + if (!distances) { + *siblings = NULL; + *nsiblings = 0; + return 0; + } + + if (VIR_ALLOC_N(tmp, ndistances) < 0) + goto cleanup; + + for (i = 0; i < ndistances; i++) { + if (!distances[i]) + continue; + + tmp[tmp_size].node = i; + tmp[tmp_size].distance = distances[i]; + tmp_size++; + } + + if (VIR_REALLOC_N(tmp, tmp_size) < 0) + goto cleanup; + + *siblings = tmp; + *nsiblings = tmp_size; + tmp = NULL; + tmp_size = 0; + ret = 0; + cleanup: + VIR_FREE(distances); + VIR_FREE(tmp); + return ret; +} + int nodeCapsInitNUMA(virCapsPtr caps) { @@ -1755,6 +1802,8 @@ nodeCapsInitNUMA(virCapsPtr caps) unsigned long long memory; virCapsHostNUMACellCPUPtr cpus = NULL; virBitmapPtr cpumap = NULL; + virCapsHostNUMACellSiblingInfoPtr siblings = NULL; + int nsiblings; int ret = -1; int ncpus = 0; int cpu; @@ -1794,14 +1843,19 @@ nodeCapsInitNUMA(virCapsPtr caps) } } + if (virNodeCapsGetSiblingInfo(n, &siblings, &nsiblings) < 0) + goto cleanup; + /* Detect the amount of memory in the numa cell in KiB */ virNumaGetNodeMemory(n, &memory, NULL); memory >>= 10; - if (virCapabilitiesAddHostNUMACell(caps, n, ncpus, memory, cpus) < 0) + if (virCapabilitiesAddHostNUMACell(caps, n, ncpus, nsiblings, + memory, cpus, siblings) < 0) goto cleanup; cpus = NULL; + siblings = NULL; } ret = 0; @@ -1812,6 +1866,7 @@ nodeCapsInitNUMA(virCapsPtr caps) virBitmapFree(cpumap); VIR_FREE(cpus); + VIR_FREE(siblings); if (ret < 0) VIR_FREE(cpus); diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 37756e7..b204e7d 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -337,7 +337,7 @@ testBuildCapabilities(virConnectPtr conn) if (virCapabilitiesAddHostNUMACell(caps, i, privconn->cells[i].numCpus, - 0, cpu_cells) < 0) + 0, 0, cpu_cells, NULL) < 0) goto error; } diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index c30b136..f4c824e 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -1100,7 +1100,8 @@ sexpr_to_xend_topology(const struct sexpr *root, virCapsPtr caps) } virBitmapFree(cpuset); - if (virCapabilitiesAddHostNUMACell(caps, cell, nb_cpus, 0, cpuInfo) < 0) + if (virCapabilitiesAddHostNUMACell(caps, cell, nb_cpus, + 0, 0, cpuInfo, NULL) < 0) goto error; cpuInfo = NULL; } diff --git a/tests/vircapstest.c b/tests/vircapstest.c index 821a92b..9df074a 100644 --- a/tests/vircapstest.c +++ b/tests/vircapstest.c @@ -64,9 +64,9 @@ buildNUMATopology(int seq) id++; if (virCapabilitiesAddHostNUMACell(caps, cell_id + seq, - MAX_CPUS_IN_CELL, + MAX_CPUS_IN_CELL, 0, MAX_MEM_IN_CELL, - cell_cpus) < 0) + cell_cpus, NULL) < 0) goto error; cell_cpus = NULL; -- 2.0.0

On Mon, Jun 02, 2014 at 02:16:00PM +0200, Michal Privoznik wrote:
If user or management application wants to create a guest, it may be useful to know the cost of internode latencies before the guest resources are pinned. For example:
<capabilities>
<host> ... <topology> <cells num='2'> <cell id='0'> <memory unit='KiB'>4004132</memory> <cell id='0' distance='10'/> <cell id='1' distance='20'/>
I'd be a little more comfortable if we didn't use a <cell> within a <cell>. Perhaps lets use 'sibling' as the name instead and group the elements. eg could we do <distances> <sibling id="0" value="10"/> <sibling id="1" value="20'/> </distance>
</topology> ... </host> ... </capabilities>
we can see the distance from node1 to node0 is 20 and within nodes 10.
One thing with having the data under each <cell> is that we're actually reporting twice as much as we need to. ie the distance between cell N and M is reported under both N and M. A different option would be todo reporting at the toplevel within <topology> eg <distances> <siblings distance="10"> <cell id="0"/> <cell id="1"/> </siblings> </distance> I'm not sure whether doing this is worth while or not though ? Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On 02.06.2014 17:55, Daniel P. Berrange wrote:
On Mon, Jun 02, 2014 at 02:16:00PM +0200, Michal Privoznik wrote:
If user or management application wants to create a guest, it may be useful to know the cost of internode latencies before the guest resources are pinned. For example:
<capabilities>
<host> ... <topology> <cells num='2'> <cell id='0'> <memory unit='KiB'>4004132</memory> <cell id='0' distance='10'/> <cell id='1' distance='20'/>
I'd be a little more comfortable if we didn't use a <cell> within a <cell>. Perhaps lets use 'sibling' as the name instead and group the elements. eg could we do
<distances> <sibling id="0" value="10"/> <sibling id="1" value="20'/> </distance>
</topology> ... </host> ... </capabilities>
we can see the distance from node1 to node0 is 20 and within nodes 10.
One thing with having the data under each <cell> is that we're actually reporting twice as much as we need to. ie the distance between cell N and M is reported under both N and M. A different option would be todo reporting at the toplevel within <topology> eg
<distances> <siblings distance="10"> <cell id="0"/> <cell id="1"/> </siblings> </distance>
I'm not sure whether doing this is worth while or not though ?
I'm not sure this is the correct approach. The XML snippet suggest that from node0 to node1 the distance is only 10 (which can't be correct, since it's not the same node). I know you made the numbers up, but still. It's unclear from the XML what the groups are. Consider this NUMA topology: 0 1 2 0 10 20 20 1 20 10 20 2 20 20 10 which is pretty common topology if you have 3 NUMA nodes. Now, what would the <distance/> XML look like? <distance> <siblings distance="10"> <cell id="0"/> </siblings> <siblings distance="10"> <cell id="1"/> </siblings> <siblings distance="10"> <cell id="2"/> </siblings> <siblings distance="20"> <cell id="0"/> <cell id="1"/> <cell id="2"/> </siblings> </distance> The XML reflects that distance within a node itself the is 10, while it's 20 in case of any two nodes different to each other. But it's not visible at first glance why there are multiple <cell/> elements with the same @id. Moreover, we can't join the first three <siblings/> into a single element because it would have a different meaning then that the last one. So I'll go with your first suggestion. Michal
participants (2)
-
Daniel P. Berrange
-
Michal Privoznik