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 | 55 ++++++++++++++++++++++++++++++++++++++++++++++++
src/util/virresctrl.h | 5 ++++-
3 files changed, 60 insertions(+), 1 deletion(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index d2573c5..5235046 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2680,6 +2680,7 @@ virResctrlInfoGetCache;
virResctrlInfoGetMonitorPrefix;
virResctrlInfoMonFree;
virResctrlInfoNew;
+virResctrlMonitorDeterminePath;
virResctrlMonitorNew;
diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c
index 956aca8..1d0eb01 100644
--- a/src/util/virresctrl.c
+++ b/src/util/virresctrl.c
@@ -2462,3 +2462,58 @@ virResctrlMonitorNew(void)
return virObjectNew(virResctrlMonitorClass);
}
+
+
+/*
+ * virResctrlMonitorDeterminePath
+ *
+ * @monitor: Pointer to a resctrl monitor
+ * @machinename: Name string of the VM
+ *
+ * Determines the directory path that the underlying resctrl group will be
+ * created with.
+ *
+ * A monitor represents a directory under resource control file system,
+ * its directory path could be the same path as @monitor->alloc, could be a
+ * path of directory under 'mon_groups' of @monitor->alloc, or a path of
+ * directory under '/sys/fs/resctrl/mon_groups' if @monitor->alloc is NULL.
+ *
+ * Returns 0 on success, -1 on error.
+ */
+int
+virResctrlMonitorDeterminePath(virResctrlMonitorPtr monitor,
+ const char *machinename)
+{
+ VIR_AUTOFREE(char *) parentpath = NULL;
+
+ if (!monitor) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Invalid resctrl monitor"));
+ return -1;
+ }
+
+ if (monitor->path) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Resctrl monitor path is already set to '%s'"),
+ monitor->path);
+ return -1;
+ }
+
+ if (monitor->id && monitor->alloc && monitor->alloc->id) {
+ if (STREQ(monitor->id, monitor->alloc->id)) {
+ if (VIR_STRDUP(monitor->path, monitor->alloc->path) < 0)
+ return -1;
+ return 0;
+ }
+ }
+
+ if (virAsprintf(&parentpath, "%s/mon_groups", monitor->alloc->path) < 0)
+ return -1;
+
+ monitor->path = virResctrlDeterminePath(parentpath, machinename,
+ monitor->id);
+ if (!monitor->path)
+ return -1;
+
+ return 0;
+}
diff --git a/src/util/virresctrl.h b/src/util/virresctrl.h
index eaa077d..baae554 100644
--- a/src/util/virresctrl.h
+++ b/src/util/virresctrl.h
@@ -191,7 +191,10 @@ virResctrlInfoGetMonitorPrefix(virResctrlInfoPtr resctrl,
typedef struct _virResctrlMonitor virResctrlMonitor;
typedef virResctrlMonitor *virResctrlMonitorPtr;
-
virResctrlMonitorPtr
virResctrlMonitorNew(void);
+
+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/22/18 4:01 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 | 55 ++++++++++++++++++++++++++++++++++++++++++++++++ > src/util/virresctrl.h | 5 ++++- > 3 files changed, 60 insertions(+), 1 deletion(-) > Reviewed-by: John Ferlan <jferlan@redhat.com> John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 2018年11月05日 23:02, John Ferlan wrote: > > On 10/22/18 4:01 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 | 55 ++++++++++++++++++++++++++++++++++++++++++++++++ >> src/util/virresctrl.h | 5 ++++- >> 3 files changed, 60 insertions(+), 1 deletion(-) >> > Reviewed-by: John Ferlan <jferlan@redhat.com> > > John Thanks for the review. Huaqiang -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 10/22/18 4:01 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 | 55 ++++++++++++++++++++++++++++++++++++++++++++++++
> src/util/virresctrl.h | 5 ++++-
> 3 files changed, 60 insertions(+), 1 deletion(-)
>
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index d2573c5..5235046 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -2680,6 +2680,7 @@ virResctrlInfoGetCache;
> virResctrlInfoGetMonitorPrefix;
> virResctrlInfoMonFree;
> virResctrlInfoNew;
> +virResctrlMonitorDeterminePath;
> virResctrlMonitorNew;
>
>
> diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c
> index 956aca8..1d0eb01 100644
> --- a/src/util/virresctrl.c
> +++ b/src/util/virresctrl.c
> @@ -2462,3 +2462,58 @@ virResctrlMonitorNew(void)
>
> return virObjectNew(virResctrlMonitorClass);
> }
> +
> +
> +/*
> + * virResctrlMonitorDeterminePath
> + *
> + * @monitor: Pointer to a resctrl monitor
> + * @machinename: Name string of the VM
> + *
> + * Determines the directory path that the underlying resctrl group will be
> + * created with.
> + *
> + * A monitor represents a directory under resource control file system,
> + * its directory path could be the same path as @monitor->alloc, could be a
> + * path of directory under 'mon_groups' of @monitor->alloc, or a path of
> + * directory under '/sys/fs/resctrl/mon_groups' if @monitor->alloc is NULL.
> + *
> + * Returns 0 on success, -1 on error.
> + */
> +int
> +virResctrlMonitorDeterminePath(virResctrlMonitorPtr monitor,
> + const char *machinename)
> +{
> + VIR_AUTOFREE(char *) parentpath = NULL;
> +
> + if (!monitor) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Invalid resctrl monitor"));
> + return -1;
> + }
> +
> + if (monitor->path) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Resctrl monitor path is already set to '%s'"),
> + monitor->path);
> + return -1;
> + }
> +
> + if (monitor->id && monitor->alloc && monitor->alloc->id) {
> + if (STREQ(monitor->id, monitor->alloc->id)) {
> + if (VIR_STRDUP(monitor->path, monitor->alloc->path) < 0)
> + return -1;
> + return 0;
> + }
> + }
> +
> + if (virAsprintf(&parentpath, "%s/mon_groups", monitor->alloc->path) < 0)
Just ran the changes through Coverity - because of the above
"monitor->alloc" check, Coverity notes right here that monitor->alloc
could be NULL, so I think a check prior to here would be in order, such
as either before or after the monitor->path check:
if (!monitor->alloc) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Missing resctrl monitor allocation"));
return -1;
}
Then the monitor->alloc check wouldn't be necessary. Thus the above
STRDUP becomes:
if (STREQ_NULLABLE(monitor->id, monitor->alloc->id)) {
if (VIR_STRDUP(monitor->path, monitor->alloc->path) < 0)
return -1;
return 0;
}
Let me know where you want it.
John
> + return -1;
> +
> + monitor->path = virResctrlDeterminePath(parentpath, machinename,
> + monitor->id);
> + if (!monitor->path)
> + return -1;
> +
> + return 0;
> +}
> diff --git a/src/util/virresctrl.h b/src/util/virresctrl.h
> index eaa077d..baae554 100644
> --- a/src/util/virresctrl.h
> +++ b/src/util/virresctrl.h
> @@ -191,7 +191,10 @@ virResctrlInfoGetMonitorPrefix(virResctrlInfoPtr resctrl,
> typedef struct _virResctrlMonitor virResctrlMonitor;
> typedef virResctrlMonitor *virResctrlMonitorPtr;
>
> -
> virResctrlMonitorPtr
> virResctrlMonitorNew(void);
> +
> +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 2018年11月06日 01:19, John Ferlan wrote:
>
> On 10/22/18 4:01 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 | 55 ++++++++++++++++++++++++++++++++++++++++++++++++
>> src/util/virresctrl.h | 5 ++++-
>> 3 files changed, 60 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
>> index d2573c5..5235046 100644
>> --- a/src/libvirt_private.syms
>> +++ b/src/libvirt_private.syms
>> @@ -2680,6 +2680,7 @@ virResctrlInfoGetCache;
>> virResctrlInfoGetMonitorPrefix;
>> virResctrlInfoMonFree;
>> virResctrlInfoNew;
>> +virResctrlMonitorDeterminePath;
>> virResctrlMonitorNew;
>>
>>
>> diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c
>> index 956aca8..1d0eb01 100644
>> --- a/src/util/virresctrl.c
>> +++ b/src/util/virresctrl.c
>> @@ -2462,3 +2462,58 @@ virResctrlMonitorNew(void)
>>
>> return virObjectNew(virResctrlMonitorClass);
>> }
>> +
>> +
>> +/*
>> + * virResctrlMonitorDeterminePath
>> + *
>> + * @monitor: Pointer to a resctrl monitor
>> + * @machinename: Name string of the VM
>> + *
>> + * Determines the directory path that the underlying resctrl group will be
>> + * created with.
>> + *
>> + * A monitor represents a directory under resource control file system,
>> + * its directory path could be the same path as @monitor->alloc, could be a
>> + * path of directory under 'mon_groups' of @monitor->alloc, or a path of
>> + * directory under '/sys/fs/resctrl/mon_groups' if @monitor->alloc is NULL.
>> + *
>> + * Returns 0 on success, -1 on error.
>> + */
>> +int
>> +virResctrlMonitorDeterminePath(virResctrlMonitorPtr monitor,
>> + const char *machinename)
>> +{
>> + VIR_AUTOFREE(char *) parentpath = NULL;
>> +
>> + if (!monitor) {
>> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> + _("Invalid resctrl monitor"));
>> + return -1;
>> + }
>> +
>> + if (monitor->path) {
>> + virReportError(VIR_ERR_INTERNAL_ERROR,
>> + _("Resctrl monitor path is already set to '%s'"),
>> + monitor->path);
>> + return -1;
>> + }
>> +
>> + if (monitor->id && monitor->alloc && monitor->alloc->id) {
>> + if (STREQ(monitor->id, monitor->alloc->id)) {
>> + if (VIR_STRDUP(monitor->path, monitor->alloc->path) < 0)
>> + return -1;
>> + return 0;
>> + }
>> + }
>> +
>> + if (virAsprintf(&parentpath, "%s/mon_groups", monitor->alloc->path) < 0)
> Just ran the changes through Coverity - because of the above
> "monitor->alloc" check, Coverity notes right here that monitor->alloc
> could be NULL, so I think a check prior to here would be in order,
Yes. Here exists a risk that this line you mentioned could be executed at
the condition that @monitor->alloc is empty pointer, this will cause a
crash.
> such
> as either before or after the monitor->path check:
>
> if (!monitor->alloc) {
> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> _("Missing resctrl monitor allocation"));
> return -1;
> }
Putting these lines before and after the monitor->path check should be
OK for me.
(I don't find the influence made by the order. )
>
> Then the monitor->alloc check wouldn't be necessary. Thus the above
> STRDUP becomes:
>
> if (STREQ_NULLABLE(monitor->id, monitor->alloc->id)) {
> if (VIR_STRDUP(monitor->path, monitor->alloc->path) < 0)
> return -1;
> return 0;
> }
Agree.
Thanks.
>
> Let me know where you want it.
>
> John
Thanks for review.
Huaqiang
>
>> + return -1;
>> +
>> + monitor->path = virResctrlDeterminePath(parentpath, machinename,
>> + monitor->id);
>> + if (!monitor->path)
>> + return -1;
>> +
>> + return 0;
>> +}
>> diff --git a/src/util/virresctrl.h b/src/util/virresctrl.h
>> index eaa077d..baae554 100644
>> --- a/src/util/virresctrl.h
>> +++ b/src/util/virresctrl.h
>> @@ -191,7 +191,10 @@ virResctrlInfoGetMonitorPrefix(virResctrlInfoPtr resctrl,
>> typedef struct _virResctrlMonitor virResctrlMonitor;
>> typedef virResctrlMonitor *virResctrlMonitorPtr;
>>
>> -
>> virResctrlMonitorPtr
>> virResctrlMonitorNew(void);
>> +
>> +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.