[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 2/5] numa: Introduce virDomainNumaNodeDistanceSpecified



On 11/22/2017 12:38 AM, John Ferlan wrote:
> 
> 
> On 11/14/2017 09:47 AM, Michal Privoznik wrote:
>> The function returns true/false depending on distance
>> configuration being present in the domain XML.
>>
>> Signed-off-by: Michal Privoznik <mprivozn redhat com>
>> ---
>>  src/conf/numa_conf.c     | 13 +++++++++++++
>>  src/conf/numa_conf.h     |  4 ++++
>>  src/libvirt_private.syms |  1 +
>>  3 files changed, 18 insertions(+)
>>
>> diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c
>> index 5f0b3f9ed..6a42777e2 100644
>> --- a/src/conf/numa_conf.c
>> +++ b/src/conf/numa_conf.c
>> @@ -1137,6 +1137,19 @@ virDomainNumaSetNodeCount(virDomainNumaPtr numa, size_t nmem_nodes)
>>      return numa->nmem_nodes;
>>  }
>>  
> 
> Two blank lines here too.
> 
>> +bool
>> +virDomainNumaNodeDistanceSpecified(virDomainNumaPtr numa,
>> +                                   size_t node,
>> +                                   size_t sibling)
>> +{
>> +    return node < numa->nmem_nodes &&
>> +        sibling < numa->nmem_nodes &&
>> +        numa->mem_nodes[node].distances &&
>> +        numa->mem_nodes[node].distances[sibling].value != LOCAL_DISTANCE &&
>> +        numa->mem_nodes[node].distances[sibling].value != REMOTE_DISTANCE;
>> +}
>> +
>> +
> 
> According to how I read commit message '74119a03' it *is* possible to
> set the @value to the same value as LOCAL_DISTANCE(10) or
> REMOTE_DISTANCE(20) - so using that as a comparison for whether it was
> specified would seem to be wrong.

Sure it is possible. But see my reply to 4/5 why I need this function.

> 
> Still if a distance *is* provided, then it seems that 'id' and 'value'
> are also required to be provided or defaulted.  That means what seems to
> matter regarding whether something was provided is if the *.value and/or
> "*.cellid are zero
> 
> At least that's how I'm reading virDomainNumaDefNodeDistanceParseXML

Okay, so the name is confusing. Would you be okay if I rename this to
virDomainNumaNodeDistanceSpecifiedAndNotDefault()? Ugrh, long name. Any
suggestion is welcome.

Alternatively, I can move LOCAL/REMOTE_DISTANCE macros to numa_conf.h,
give them proper prefix and call virDomainNumaGetNodeDistance() from
qemu and check if returned value is LOCAL/REMOTE.

Michal


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]