[libvirt] [PATCH 9/9] conf: Add memory bandwidth allocation capability of host

[bing.niu]

On 2018年07月27日 05:04, John Ferlan wrote:
> 
> 
> On 07/18/2018 03:57 AM, bing.niu at intel.com wrote:
>> From: Bing Niu <bing.niu at intel.com>
>>
>> Add new XML section to report host's memory bandwidth allocation
>> capability. The format as below example:
>>
>>   <host>
>>   .....
>>     <memory_bandwidth>
>>       <node id='0' cpus='0-19'>
>>         <control granularity='10' min ='10' maxAllocs='8'/>
>>       </node>
>>     </memory_bandwidth>
>> </host>
>>
>> granularity   ---- granularity of memory bandwidth, unit percentage.
>> min           ---- minimum memory bandwidth allowed, unit percentage.
>> maxAllocs     ---- maximum memory bandwidth allocation group supported.
>>
>> Signed-off-by: Bing Niu <bing.niu at intel.com>
>> ---
>>   docs/schemas/capability.rng                        |  33 +++++++
>>   src/conf/capabilities.c                            | 108 +++++++++++++++++++++
>>   src/conf/capabilities.h                            |  11 +++
>>   src/util/virresctrl.c                              |  20 ++++
>>   src/util/virresctrl.h                              |  15 +++
>>   .../linux-resctrl/resctrl/info/MB/bandwidth_gran   |   1 +
>>   .../linux-resctrl/resctrl/info/MB/min_bandwidth    |   1 +
>>   .../linux-resctrl/resctrl/info/MB/num_closids      |   1 +
>>   tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml   |   8 ++
>>   tests/virresctrldata/resctrl.schemata              |   1 +
>>   10 files changed, 199 insertions(+)
>>   create mode 100644 tests/vircaps2xmldata/linux-resctrl/resctrl/info/MB/bandwidth_gran
>>   create mode 100644 tests/vircaps2xmldata/linux-resctrl/resctrl/info/MB/min_bandwidth
>>   create mode 100644 tests/vircaps2xmldata/linux-resctrl/resctrl/info/MB/num_closids
>>
> 
> What about virsh available views?  And similar to the RDT series what
> about domstats?  I think you can get some good ideas from the RDT CMT
> RFC that's posted.  Not even sure if it's already done internally - but
> pointing it out...  It doesn't have to be done as part of the series,
> but eventually it may be nice.

Yes. RDT CMT will follow this memory_bandwidth host capability XML. 
Huaqiang will handle that.
> 
> I'll give the following a cursory look as I have other tasks needing
> some attention. I'll leave it in the back of my mind that I have to be
> more thorough on the next pass once I get here.
> 
>> diff --git a/docs/schemas/capability.rng b/docs/schemas/capability.rng
>> index 52164d5..d61515c 100644
>> --- a/docs/schemas/capability.rng
>> +++ b/docs/schemas/capability.rng
>> @@ -51,6 +51,9 @@
>>         <optional>
>>           <ref name='cache'/>
>>         </optional>
>> +      <optional>
>> +        <ref name='memory_bandwidth'/>
>> +      </optional>
>>         <zeroOrMore>
>>           <ref name='secmodel'/>
>>         </zeroOrMore>
>> @@ -326,6 +329,36 @@
>>       </attribute>
>>     </define>
>>   
>> +  <define name='memory_bandwidth'>
>> +    <element name='memory_bandwidth'>
>> +      <oneOrMore>
>> +        <element name='node'>
>> +          <attribute name='id'>
>> +            <ref name='unsignedInt'/>
>> +          </attribute>
>> +          <attribute name='cpus'>
>> +            <ref name='cpuset'/>
>> +          </attribute>
>> +          <zeroOrMore>
>> +            <element name='control'>
>> +              <attribute name='granularity'>
>> +                <ref name='unsignedInt'/>
>> +              </attribute>
>> +            <optional>
>> +              <attribute name='min'>
>> +                <ref name='unsignedInt'/>
>> +            </attribute>
>> +            </optional>
>> +              <attribute name='maxAllocs'>
>> +                <ref name='unsignedInt'/>
>> +              </attribute>
>> +            </element>
>> +          </zeroOrMore>
>> +        </element>
>> +      </oneOrMore>
>> +    </element>
>> +  </define>
>> +
>>     <define name='guestcaps'>
>>       <element name='guest'>
>>         <ref name='ostype'/>
>> diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
>> index 7a810ef..3f52296 100644
>> --- a/src/conf/capabilities.c
>> +++ b/src/conf/capabilities.c
>> @@ -198,6 +198,16 @@ virCapabilitiesFreeNUMAInfo(virCapsPtr caps)
>>   }
>>   
>>   static void
>> +virCapsHostMemBWNodeFree(virCapsHostMemBWNodePtr ptr)
>> +{
>> +    if (!ptr)
>> +        return;
>> +
>> +    virBitmapFree(ptr->cpus);
>> +    VIR_FREE(ptr);
>> +}
>> +
>> +static void
>>   virCapabilitiesClearSecModel(virCapsHostSecModelPtr secmodel)
>>   {
>>       size_t i;
>> @@ -239,6 +249,11 @@ virCapsDispose(void *object)
>>           virCapsHostCacheBankFree(caps->host.caches[i]);
>>       VIR_FREE(caps->host.caches);
>>   
>> +    for (i = 0; i < caps->host.nnodes; i++)
>> +        virCapsHostMemBWNodeFree(caps->host.nodes[i]);
>> +    VIR_FREE(caps->host.nodes);
>> +
>> +
> 
> Remove one of the blank lines.

OK.

Thanks a lot~
> 
> This was the only issue I saw in my quick glance - rest seemed OK.
> 
> John
> 
[.....]