> -----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. > > > + > > + 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