On 5/21/21 9:58 AM, Peter Krempa wrote:
On Thu, May 20, 2021 at 17:24:56 +0200, Michal Privoznik wrote:
After previous patches we have two structures: virCapsHostNUMACellDistance and virNumaDistance which express the same thing. And have the exact same members (modulo their names). Drop the former in favor of the latter.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/conf/capabilities.c | 26 ++++++++------------------ src/conf/capabilities.h | 11 +++-------- src/conf/virconftypes.h | 2 -- src/libxl/libxl_capabilities.c | 8 ++++---- 4 files changed, 15 insertions(+), 32 deletions(-)
diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c index 926ecb5a24..1290c9c15d 100644 --- a/src/conf/capabilities.c +++ b/src/conf/capabilities.c
[...]
@@ -833,17 +833,7 @@ virCapabilitiesHostNUMAFormat(virBuffer *buf, cell->pageinfo[j].avail); }
- if (cell->ndistances) { - virBufferAddLit(buf, "<distances>\n"); - virBufferAdjustIndent(buf, 2); - for (j = 0; j < cell->ndistances; j++) {
This code didn't skip printing the sibling if 'value' is 0 ...
- virBufferAsprintf(buf, "<sibling id='%d' value='%d'/>\n", - cell->distances[j].node, - cell->distances[j].distance); - } - virBufferAdjustIndent(buf, -2); - virBufferAddLit(buf, "</distances>\n"); - } + virNumaDistanceFormat(buf, cell->distances, cell->ndistances);
... but this new implementation does that. I didn't check whether that's justified or not, but the commit message doesn't try to justify it either.
Was that an expected change?
Yes, I will adjust commit message. The point is that in domain XML we allow partial specification of distances. Each guest NUMA node will have an array of virDomainNumaDistance structs (#nodes long) and for each the .value member will either be in [10, 255] range or 0 (if not specified in XML). For capabilities - we query numa_distance() and store its output into an array. The numa_distance() comes from numactl package, and returns 0 to indicate an error (if either parsing distances from sysfs failed or we passed a non-existent node in): https://github.com/numactl/numactl/blob/master/distance.c#L110 Therefore, I think it's safe to ignore 0 - it's not valid distance anyway. However, what I forgot to squash in was: diff --git i/docs/schemas/capability.rng w/docs/schemas/capability.rng index c4cafc47ee..fb8203ad6d 100644 --- i/docs/schemas/capability.rng +++ w/docs/schemas/capability.rng @@ -157,16 +157,9 @@ <optional> <element name="distances"> - <zeroOrMore> - <element name="sibling"> - <attribute name="id"> - <ref name="unsignedInt"/> - </attribute> - <attribute name="value"> - <ref name="unsignedInt"/> - </attribute> - </element> - </zeroOrMore> + <oneOrMore> + <ref name="numaDistance"/> + </oneOrMore> </element> </optional> So let me resend v2. Thanks! Michal