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

Wang Huaqiang posted 19 patches 6 years, 7 months ago
There is a newer version of this series
[libvirt] [PATCHv5 11/19] conf: Refactor code for matching existing resctrls
Posted by Wang Huaqiang 6 years, 7 months ago
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@intel.com>
---
 src/conf/domain_conf.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

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++) {
-- 
2.7.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv5 11/19] conf: Refactor code for matching existing resctrls
Posted by John Ferlan 6 years, 7 months ago

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@intel.com>
> ---
>  src/conf/domain_conf.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 

Reviewed-by: John Ferlan <jferlan@redhat.com>

John

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv5 11/19] conf: Refactor code for matching existing resctrls
Posted by Wang, Huaqiang 6 years, 7 months ago
On 10/11/2018 3:15 AM, John Ferlan wrote:
>
>
> 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@intel.com>
>> ---
>>  src/conf/domain_conf.c | 4 +---
>>  1 file changed, 1 insertion(+), 3 deletions(-)
>>
>
> Reviewed-by: John Ferlan <jferlan@redhat.com>
>
> John

Thanks for review.
Huaqiang

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv5 11/19] conf: Refactor code for matching existing resctrls
Posted by John Ferlan 6 years, 7 months ago

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@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.

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...

John

> 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++) {
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv5 11/19] conf: Refactor code for matching existing resctrls
Posted by Wang, Huaqiang 6 years, 7 months ago

> -----Original Message-----
> From: John Ferlan [mailto:jferlan@redhat.com]
> Sent: Thursday, October 11, 2018 5:58 AM
> To: Wang, Huaqiang <huaqiang.wang@intel.com>; libvir-list@redhat.com
> Cc: Feng, Shaohe <shaohe.feng@intel.com>; Niu, Bing <bing.niu@intel.com>;
> Ding, Jian-feng <jian-feng.ding@intel.com>; Zang, Rui <rui.zang@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@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++) {
> >


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list