> -----Original Message----- > From: John Ferlan [mailto:jferlan@xxxxxxxxxx] > Sent: Thursday, October 11, 2018 5:43 AM > To: Wang, Huaqiang <huaqiang.wang@xxxxxxxxx>; libvir-list@xxxxxxxxxx > Cc: Feng, Shaohe <shaohe.feng@xxxxxxxxx>; Niu, Bing <bing.niu@xxxxxxxxx>; > Ding, Jian-feng <jian-feng.ding@xxxxxxxxx>; Zang, Rui <rui.zang@xxxxxxxxx> > Subject: Re: [PATCHv5 06/19] util: Add monitor interface to determine > path > > > > On 10/10/18 9:56 AM, Wang, Huaqiang wrote: > > > >> -----Original Message----- > >> From: John Ferlan [mailto:jferlan@xxxxxxxxxx] > >> Sent: Wednesday, October 10, 2018 7:16 AM > >> To: Wang, Huaqiang <huaqiang.wang@xxxxxxxxx>; libvir-list@xxxxxxxxxx > >> Cc: Feng, Shaohe <shaohe.feng@xxxxxxxxx>; Niu, Bing > >> <bing.niu@xxxxxxxxx>; Ding, Jian-feng <jian-feng.ding@xxxxxxxxx>; > >> Zang, Rui <rui.zang@xxxxxxxxx> > >> Subject: Re: [PATCHv5 06/19] util: Add monitor interface to > >> determine path > >> > >> > >> > >> On 10/9/18 6:30 AM, Wang Huaqiang wrote: > >>> Add interface for resctrl monitor to determine the path. > >>> > >>> Signed-off-by: Wang Huaqiang <huaqiang.wang@xxxxxxxxx> > >>> --- > >>> src/libvirt_private.syms | 1 + > >>> src/util/virresctrl.c | 32 ++++++++++++++++++++++++++++++++ > >>> src/util/virresctrl.h | 3 +++ > >>> 3 files changed, 36 insertions(+) > >>> > >>> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms > >>> index 4a52a86..e175c8b 100644 > >>> --- a/src/libvirt_private.syms > >>> +++ b/src/libvirt_private.syms > >>> @@ -2681,6 +2681,7 @@ virResctrlInfoGetMonitorPrefix; > >>> virResctrlInfoMonFree; virResctrlInfoNew; virResctrlMonitorAddPID; > >>> +virResctrlMonitorDeterminePath; > >>> virResctrlMonitorNew; > >>> > >>> > >>> diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c index > >>> 03001cc..1a5578e 100644 > >>> --- a/src/util/virresctrl.c > >>> +++ b/src/util/virresctrl.c > >>> @@ -2465,3 +2465,35 @@ virResctrlMonitorAddPID(virResctrlMonitorPtr > >>> monitor, { > >>> return virResctrlAddPID(monitor->path, pid); } > >>> + > >>> +int > >>> +virResctrlMonitorDeterminePath(virResctrlMonitorPtr monitor, > >>> + const char *machinename) { > >>> + char *alloc_path = NULL; > >> > >> const char > > > > OK, a pointer to const char will be better. > > > >> > >>> + char *parentpath = NULL; > >> > >> VIR_AUTOFREE(char *) parentpath = NULL; > >> > >> (thus the VIR_FREE later isn't necessary) > > > > I haven't realized the existence of such kind of variable 'decorator'. > > VIR_AUTOFREE will be used in next update. > > Thanks. > > > > Yeah it's "newer" stuff, but it hasn't always gotten into my review cadence so > sometimes I remember, sometimes I don't. We had a Google summer of code > student working through those changes, but there's still many more places to > change. > > John > Thanks! Huaqiang > >> > >>> + > >>> + if (!monitor) { > >>> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > >>> + _("Invalid resctrl monitor")); > >>> + return -1; > >>> + } > >> > >> Shouldn't there be a monitor->path check here like there is in > >> virResctrlAllocDeterminePath? > > > > Since virResctrlAllocDeterminePath has such kind of safety check. > > Let's add the similar check here. > > will be added. > > > >> > >>> + > >>> + if (monitor->alloc) > >>> + alloc_path = monitor->alloc->path; > >>> + else > >>> + alloc_path = (char *)SYSFS_RESCTRL_PATH; > >> > >> s/(char *)// > > > > Will be removed. > > > >> > >>> + > >>> + if (virAsprintf(&parentpath, "%s/mon_groups", alloc_path) < 0) > >>> + return -1; > >>> + > >>> + monitor->path = virResctrlDeterminePath(parentpath, machinename, > >>> + monitor->id); > >>> + > >>> + VIR_FREE(parentpath); > >> > >> see above... > > > > Line will be removed. > > > >> > >> John > > > > Thanks for review. > > Huaqiang > > > >> > >>> + > >>> + if (!monitor->path) > >>> + return -1; > >>> + > >>> + return 0; > >>> +} > >>> diff --git a/src/util/virresctrl.h b/src/util/virresctrl.h index > >>> cb9bfae..69b6b1d 100644 > >>> --- a/src/util/virresctrl.h > >>> +++ b/src/util/virresctrl.h > >>> @@ -196,4 +196,7 @@ virResctrlMonitorNew(void); int > >>> virResctrlMonitorAddPID(virResctrlMonitorPtr monitor, > >>> pid_t pid); > >>> +int > >>> +virResctrlMonitorDeterminePath(virResctrlMonitorPtr monitor, > >>> + const char *machinename); > >>> #endif /* __VIR_RESCTRL_H__ */ > >>> > > -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
