Add interface for resctrl monitor to determine the path.
Signed-off-by: Wang Huaqiang <huaqiang.wang@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;
+ char *parentpath = NULL;
+
+ if (!monitor) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Invalid resctrl monitor"));
+ return -1;
+ }
+
+ if (monitor->alloc)
+ alloc_path = monitor->alloc->path;
+ else
+ alloc_path = (char *)SYSFS_RESCTRL_PATH;
+
+ if (virAsprintf(&parentpath, "%s/mon_groups", alloc_path) < 0)
+ return -1;
+
+ monitor->path = virResctrlDeterminePath(parentpath, machinename,
+ monitor->id);
+
+ VIR_FREE(parentpath);
+
+ 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__ */
--
2.7.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
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@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
> + char *parentpath = NULL;
VIR_AUTOFREE(char *) parentpath = NULL;
(thus the VIR_FREE later isn't necessary)
> +
> + 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?
> +
> + if (monitor->alloc)
> + alloc_path = monitor->alloc->path;
> + else
> + alloc_path = (char *)SYSFS_RESCTRL_PATH;
s/(char *)//
> +
> + if (virAsprintf(&parentpath, "%s/mon_groups", alloc_path) < 0)
> + return -1;
> +
> + monitor->path = virResctrlDeterminePath(parentpath, machinename,
> + monitor->id);
> +
> + VIR_FREE(parentpath);
see above...
John
> +
> + 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@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
> -----Original Message-----
> From: John Ferlan [mailto:jferlan@redhat.com]
> Sent: Wednesday, October 10, 2018 7:16 AM
> To: Wang, Huaqiang <huaqiang.wang@intel.com>; libvir-list@redhat.com
> Cc: Feng, Shaohe <shaohe.feng@intel.com>; Niu, Bing <bing.niu@intel.com>;
> Ding, Jian-feng <jian-feng.ding@intel.com>; Zang, Rui <rui.zang@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@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__ */
> >
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 10/10/18 9:56 AM, Wang, Huaqiang wrote:
>
>> -----Original Message-----
>> From: John Ferlan [mailto:jferlan@redhat.com]
>> Sent: Wednesday, October 10, 2018 7:16 AM
>> To: Wang, Huaqiang <huaqiang.wang@intel.com>; libvir-list@redhat.com
>> Cc: Feng, Shaohe <shaohe.feng@intel.com>; Niu, Bing <bing.niu@intel.com>;
>> Ding, Jian-feng <jian-feng.ding@intel.com>; Zang, Rui <rui.zang@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@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.
>
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
>>
>>> +
>>> + 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@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
> -----Original Message-----
> From: John Ferlan [mailto:jferlan@redhat.com]
> Sent: Thursday, October 11, 2018 5:43 AM
> To: Wang, Huaqiang <huaqiang.wang@intel.com>; libvir-list@redhat.com
> Cc: Feng, Shaohe <shaohe.feng@intel.com>; Niu, Bing <bing.niu@intel.com>;
> Ding, Jian-feng <jian-feng.ding@intel.com>; Zang, Rui <rui.zang@intel.com>
> Subject: Re: [libvirt] [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@redhat.com]
> >> Sent: Wednesday, October 10, 2018 7:16 AM
> >> To: Wang, Huaqiang <huaqiang.wang@intel.com>; libvir-list@redhat.com
> >> Cc: Feng, Shaohe <shaohe.feng@intel.com>; Niu, Bing
> >> <bing.niu@intel.com>; Ding, Jian-feng <jian-feng.ding@intel.com>;
> >> Zang, Rui <rui.zang@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@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.
> >
>
> 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@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2026 Red Hat, Inc.