Re: [libvirt] [PATCHv7 01/18] docs, util: Refactor schemas and virresctrl to support optional cache

On 2018年11月05日 23:01, John Ferlan wrote: > > On 10/22/18 4:01 AM, Wang Huaqiang wrote: >> Refactor schemas and virresctrl to support optional <cache> element >> in <cachetune>. >> >> Later, the monitor entry will be introduced and to be placed >> under <cachetune>. Either cache entry or monitor entry is >> an optional element of <cachetune>. >> >> An cachetune has no <cache> element is taking the default resource >> allocating policy defined in '/sys/fs/resctrl/schemata'. >> >> Signed-off-by: Wang Huaqiang <huaqiang.wang(a)intel.com&gt; >> --- >>   docs/formatdomain.html.in     |  4 ++-- >>   docs/schemas/domaincommon.rng |  4 ++-- >>   src/util/virresctrl.c         | 28 ++++++++++++++++++++++++++++ >>   3 files changed, 32 insertions(+), 4 deletions(-) >> > [...] > >> +    /* If the allocation is empty, then the path will be >> SYSFS_RESCTRL_PATH */ >> +    if (virResctrlAllocIsEmpty(alloc)) { >> +        if (!alloc->path && >> +            VIR_STRDUP(alloc->path, SYSFS_RESCTRL_PATH) < 0) >> +            return -1; >> + >> +        return 0; >> +    } >> + > Because of ... > >>       if (!alloc->path && >>           virAsprintf(&alloc->path, "%s/%s-%s", >>                       SYSFS_RESCTRL_PATH, machinename, alloc->id) < 0) > [...] > >> @@ -2334,6 +2358,10 @@ virResctrlAllocRemove(virResctrlAllocPtr alloc) >>   { >>       int ret = 0; >>   +    /* No directory have ever been created. Just return */ >> +    if (virResctrlAllocIsEmpty(alloc)) >> +        return 0; > ... the change to virResctrlAllocDeterminePath to fill in alloc->path > when virResctrlAllocIsEmpty to be a default path, this should be: > >      if (STREQ_NULLABLE(alloc->path, SYSFS_RESCTRL_PATH)) >         return 0; > > or moved after the next check and the _NULLABLE removed. > > Whether the AllocIsEmpty is true or not shouldn't be the bearing on > whether the directory created because of that Agree with the changes. "No need to create a directory that has already been created by system." (SYSFS_RESCTRL_PATH is the created directory).

>> + >>       if (!alloc->path) >>           return 0; >> > I can adjust for you, let me know; otherwise, things are fine for the > > Reviewed-by: John Ferlan <jferlan(a)redhat.com&gt; > > John Thanks for the review. Huaqiang