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

On 07/18/2018 03:57 AM, bing.niu(a)intel.com wrote: > From: Bing Niu <bing.niu(a)intel.com&gt; > > 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(a)intel.com&gt; > --- > 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.

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.

This was the only issue I saw in my quick glance - rest seemed OK. John