Re: [libvirt] [RFC PATCH 2/2] conf: Extend cputune/cachetune to support memory bandwidth allocation

On Tue, May 29, 2018 at 06:58:03PM +0800, bing.niu(a)intel.com wrote: > From: Bing Niu <bing.niu(a)intel.com&gt; > > Extend current cachetune section to support memory bandwidth allocation. > Add a new cachetune element llc for memory allocation. As the example > below: > >        <cachetune vcpus='0'> >                <llc id='0' bandwidth='30'/> >        </cachetune> > > id        --- on which last level cache memory bandwidth to be set > bandwidth --- the memory bandwidth percent to set. > > Signed-off-by: Bing Niu <bing.niu(a)intel.com&gt; > --- > src/conf/domain_conf.c | 86 > ++++++++++++++++++++++++++++++++++++++++++++++---- > src/util/virresctrl.c  |  7 ++++ > 2 files changed, 86 insertions(+), 7 deletions(-) > > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index d6ac47c..aba998d 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -18578,6 +18578,55 @@ virDomainDefParseBootOptions(virDomainDefPtr > def, >     return ret; > } > > static int > virDomainCachetuneDefParseCache(xmlXPathContextPtr ctxt, > @@ -18670,6 +18719,7 @@ virDomainCachetuneDefParse(virDomainDefPtr def, >     ssize_t i = 0; >     int n; >     int ret = -1; > +    bool mba_available = false; > >     ctxt->node = node; > > @@ -18701,12 +18751,26 @@ virDomainCachetuneDefParse(virDomainDefPtr def, >         goto cleanup; >     } > > +    if ((n = virXPathNodeSet("./llc", ctxt, &nodes)) < 0) { > +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > +                       _("Cannot extract Memory Bandwidth  nodes under " > +                         "cachetune. try cache allocation")); > +    } > + > +    for (i = 0; i < n; i++) { > +        if (virDomainCachetuneDefParseMemoryBandwidth(ctxt, nodes[i], > alloc) < 0) > +            goto cleanup; > +    } > + > +    if (n) > +        mba_available = true; > + >     if ((n = virXPathNodeSet("./cache", ctxt, &nodes)) < 0) { >         virReportError(VIR_ERR_INTERNAL_ERROR, "%s", >                        _("Cannot extract cache nodes under cachetune")); > -        goto cleanup; > +        if (!mba_available) > +            goto cleanup; Is it okay to skip cache parsing in that case?

>     } > - >     for (i = 0; i < n; i++) { >         if (virDomainCachetuneDefParseCache(ctxt, nodes[i], alloc) < 0) >             goto cleanup; > @@ -26394,11 +26458,19 @@ virDomainCachetuneDefFormatHelper(unsigned > int level, >     virBufferPtr buf = opaque; >     unsigned long long short_size = virFormatIntPretty(size, &unit); > > -    virBufferAsprintf(buf, > -                      "<cache id='%u' level='%u' type='%s' " > -                      "size='%llu' unit='%s'/>\n", > -                      cache, level, virCacheTypeToString(type), > -                      short_size, unit); > +    /* If type is VIR_CACHE_TYPE_LAST, this means it's a memory > +     * bandwidth allocation formatting request */ _TYPE_LAST values are sentinels used to iterate over all enum values and should never be given any other meaning.

Jano > +    if (type == VIR_CACHE_TYPE_LAST) > +        virBufferAsprintf(buf, > +                          "<llc id='%u' bandwidth='%llu'/>\n", > +                          cache, size); > +