[PATCH 4/4] conf: Deduplicate NUMA distance code

[Michal Prívozník]

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 at 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