Re: [libvirt] [PATCHv5 05/19] util: Refactor code for determining allocation path

-----Original Message----- From: John Ferlan [mailto:jferlan@redhat.com] Sent: Wednesday, October 10, 2018 7:09 AM To: Wang, Huaqiang <huaqiang.wang(a)intel.com>; libvir-list(a)redhat.com Cc: Feng, Shaohe <shaohe.feng(a)intel.com>; Niu, Bing <bing.niu(a)intel.com>; Ding, Jian-feng <jian-feng.ding(a)intel.com>; Zang, Rui <rui.zang(a)intel.com&gt; Subject: Re: [libvirt] [PATCHv5 05/19] util: Refactor code for determining allocation path On 10/9/18 6:30 AM, Wang Huaqiang wrote: > The code for determining resctrl allocation path could be reused for > monitor. Refactor it for reusing. > > Signed-off-by: Wang Huaqiang <huaqiang.wang(a)intel.com&gt; > --- > src/util/virresctrl.c | 33 +++++++++++++++++++++++++++------ > 1 file changed, 27 insertions(+), 6 deletions(-) > > diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c index > c9a79f7..03001cc 100644 > --- a/src/util/virresctrl.c > +++ b/src/util/virresctrl.c > @@ -2267,6 +2267,26 @@ virResctrlAllocAssign(virResctrlInfoPtr > resctrl, } > > > +static char * > +virResctrlDeterminePath(const char *pathparent, s/pathparent/parentpath

> + const char *prefix, > + const char *id) { > + char *path = NULL; > + > + if (!id) { > + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > + _("Resctrl resource ID must be set before > + creation")); s/Resctrl resource ID/'%s' resctrl ID/ where %s is @parentpath Working in the @parentpath, then we'd know which it was Alloc or Monitor

> + return NULL; > + } > + > + if (virAsprintf(&path, "%s/%s-%s", pathparent, prefix, id) < 0) parentpath

> + return NULL; > + > + return path; > +} > + > + > int > virResctrlAllocDeterminePath(virResctrlAllocPtr alloc, > const char *machinename) @@ -2274,15 > +2294,16 @@ virResctrlAllocDeterminePath(virResctrlAllocPtr alloc, > if (!alloc) > return 0; > > - if (!alloc->id) { > - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > - _("Resctrl Allocation ID must be set before creation")); > + if (alloc->path) { > + virReportError(VIR_ERR_INVALID_ARG, "%s", > + _("Resctrl group path is expected to be > + NULL")); Resctrl alloc group path is already set '%s' I think this is an internal error not an invalid arg, too

John

> return -1; > } > > - if (!alloc->path && > - virAsprintf(&alloc->path, "%s/%s-%s", > - SYSFS_RESCTRL_PATH, machinename, alloc->id) < 0) > + alloc->path = virResctrlDeterminePath(SYSFS_RESCTRL_PATH, > + machinename, > + alloc->id); > + if (!alloc->path) > return -1; > > return 0; >