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@xxxxxxxxxx>
>> ---
>> 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
