[libvirt] [PATCHv5 11/19] conf: Refactor code for matching existing resctrls

[Wang, Huaqiang]

> -----Original Message-----
> From: John Ferlan [mailto:jferlan at redhat.com]
> Sent: Thursday, October 11, 2018 5:58 AM
> To: Wang, Huaqiang <huaqiang.wang at intel.com>; libvir-list at redhat.com
> Cc: Feng, Shaohe <shaohe.feng at intel.com>; Niu, Bing <bing.niu at intel.com>;
> Ding, Jian-feng <jian-feng.ding at intel.com>; Zang, Rui <rui.zang at intel.com>
> Subject: Re: [libvirt] [PATCHv5 11/19] conf: Refactor code for matching existing
> resctrls
> 
> 
> 
> On 10/9/18 6:30 AM, Wang Huaqiang wrote:
> > Refactoring the code of matching the new resctrl with existing resctrl
> > groups. Add the virObjectRef action into function
> > virDomainResctrlVcpuMatch.
> >
> > Signed-off-by: Wang Huaqiang <huaqiang.wang at intel.com>
> > ---
> >  src/conf/domain_conf.c | 4 +---
> >  1 file changed, 1 insertion(+), 3 deletions(-)
> >
> 
> Extra question here... Another caller to virDomainResctrlVcpuMatch is
> virDomainCachetuneDefParse...
> 
> Prior to this change, if @alloc was returned we'd go to Unref(alloc) which I think
> is a bug, right?  All things considered.

Yes.

> 
> At least with this change, the Unref wouldn't be for the only Ref ever done on
> @alloc.
> 
> I can push this separately, but the answer perhaps changes the commit message
> a bit...

I will make this patch as a standalone patch for pushing and change commit
message accordingly.

> 
> John
> 

Thanks for review.
Huaqiang

> > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index
> > b77680e..e2b4701 100644
> > --- a/src/conf/domain_conf.c
> > +++ b/src/conf/domain_conf.c
> > @@ -18833,7 +18833,7 @@ virDomainResctrlVcpuMatch(virDomainDefPtr
> def,
> >           * Just updating memory allocation information of that group
> >           */
> >          if (virBitmapEqual(def->resctrls[i]->vcpus, vcpus)) {
> > -            *alloc = def->resctrls[i]->alloc;
> > +            *alloc = virObjectRef(def->resctrls[i]->alloc);
> >              break;
> >          }
> >          if (virBitmapOverlaps(def->resctrls[i]->vcpus, vcpus)) { @@
> > -19225,8 +19225,6 @@ virDomainMemorytuneDefParse(virDomainDefPtr def,
> >          if (!alloc)
> >              goto cleanup;
> >          new_alloc = true;
> > -    } else {
> > -        alloc = virObjectRef(alloc);
> >      }
> >
> >      for (i = 0; i < n; i++) {
> >