[libvirt] [PATCHv5 06/19] util: Add monitor interface to determine path
Wang, Huaqiang
huaqiang.wang at intel.com
Wed Oct 10 13:56:18 UTC 2018
> -----Original Message-----
> From: John Ferlan [mailto:jferlan at redhat.com]
> Sent: Wednesday, October 10, 2018 7:16 AM
> To: Wang, Huaqiang <huaqiang.wang at intel.com>; libvir-list at redhat.com
> Cc: Feng, Shaohe <shaohe.feng at intel.com>; Niu, Bing <bing.niu at intel.com>;
> Ding, Jian-feng <jian-feng.ding at intel.com>; Zang, Rui <rui.zang at intel.com>
> Subject: Re: [libvirt] [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 at intel.com>
> > ---
> > 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__ */
> >
More information about the libvir-list
mailing list