[libvirt] [PATCHv7 04/18] util: Add interface to determine monitor path

[John Ferlan]

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 at 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__ */
>