[libvirt] [PATCH] conf: resctrl object is not properly handled

[Daniel Henrique Barboza]

On 8/20/19 7:06 AM, Wang Huaqiang wrote:
> resctrl object stored in def->resctrls is shared by cachetune and
> memorytune. The domain xml configuration is parsed firstly for
> cachetune then memorytune, and the resctrl object will not be created
> in parsing settings for memorytune once it found sharing exists.
>
> But resctrl is improperly freed when sharing happens.
>
> Signed-off-by: Wang Huaqiang <huaqiang.wang at intel.com>
> ---
>   src/conf/domain_conf.c                    | 12 +++++-------
>   tests/genericxml2xmlindata/memorytune.xml |  4 ++++
>   2 files changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 617ccac..604e006 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -19590,7 +19590,6 @@ virDomainMemorytuneDefParse(virDomainDefPtr def,
>       VIR_AUTOUNREF(virResctrlAllocPtr) alloc = NULL;
>       ssize_t i = 0;
>       int n;
> -    int ret = -1;
>   
>       ctxt->node = node;
>   
> @@ -19632,14 +19631,13 @@ virDomainMemorytuneDefParse(virDomainDefPtr def,
>           if (!(resctrl = virDomainResctrlNew(node, alloc, vcpus, flags)))
>               return -1;
>   
> -        if (VIR_APPEND_ELEMENT(def->resctrls, def->nresctrls, resctrl) < 0)
> -            goto cleanup;
> +        if (VIR_APPEND_ELEMENT(def->resctrls, def->nresctrls, resctrl) < 0) {
> +            virDomainResctrlDefFree(resctrl);
> +            return -1;
> +        }
>       }
>   
> -    ret = 0;
> - cleanup:
> -    virDomainResctrlDefFree(resctrl);
> -    return ret;
> +    return 0;
>   }

This could also be fixed by putting the free into a conditional:

if (resctrl)
     virDomainRescrtlDefFree(resctrl);


But I appreciate one less 'goto' jump to worry about.


Reviewed-by: Daniel Henrique Barboza <danielhb413 at gmail.com>

>   
>   
> diff --git a/tests/genericxml2xmlindata/memorytune.xml b/tests/genericxml2xmlindata/memorytune.xml
> index ea03e22..7486b54 100644
> --- a/tests/genericxml2xmlindata/memorytune.xml
> +++ b/tests/genericxml2xmlindata/memorytune.xml
> @@ -5,6 +5,10 @@
>     <currentMemory unit='KiB'>219136</currentMemory>
>     <vcpu placement='static'>4</vcpu>
>     <cputune>
> +    <cachetune vcpus='0-1'>
> +      <cache id='0' level='3' type='both' size='768' unit='KiB'/>
> +      <cache id='1' level='3' type='both' size='768' unit='KiB'/>
> +    </cachetune>
>       <memorytune vcpus='0-1'>
>         <node id='0' bandwidth='20'/>
>         <node id='1' bandwidth='30'/>