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

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(a)intel.com&gt; > --- >   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);