[libvirt] [PATCHv5 08/19] util: Add interface for creating monitor group

[John Ferlan]

On 10/9/18 6:30 AM, Wang Huaqiang wrote:
> Add interface for creating the resource monitoring group according
> to '@virResctrlMonitor->path'.
> 
> Signed-off-by: Wang Huaqiang <huaqiang.wang at intel.com>
> ---
>  src/libvirt_private.syms |  1 +
>  src/util/virresctrl.c    | 28 ++++++++++++++++++++++++++++
>  src/util/virresctrl.h    |  6 ++++++
>  3 files changed, 35 insertions(+)
> 
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index e175c8b..a878083 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -2681,6 +2681,7 @@ virResctrlInfoGetMonitorPrefix;
>  virResctrlInfoMonFree;
>  virResctrlInfoNew;
>  virResctrlMonitorAddPID;
> +virResctrlMonitorCreate;
>  virResctrlMonitorDeterminePath;
>  virResctrlMonitorNew;
>  
> diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c
> index 8b617a6..b3d20cc 100644
> --- a/src/util/virresctrl.c
> +++ b/src/util/virresctrl.c
> @@ -2475,6 +2475,7 @@ virResctrlMonitorAddPID(virResctrlMonitorPtr monitor,
>      return virResctrlAddPID(monitor->path, pid);
>  }
>  
> +

This should have been squashed into patch6

>  int
>  virResctrlMonitorDeterminePath(virResctrlMonitorPtr monitor,
>                                 const char *machinename)
> @@ -2506,3 +2507,30 @@ virResctrlMonitorDeterminePath(virResctrlMonitorPtr monitor,
>  
>      return 0;
>  }
> +
> +
> +int
> +virResctrlMonitorCreate(virResctrlAllocPtr alloc,
> +                        virResctrlMonitorPtr monitor,
> +                        const char *machinename)
> +{
> +    int lockfd = -1;
> +    int ret = -1;
> +
> +    if (!monitor)
> +        return 0;
> +
> +    monitor->alloc = virObjectRef(alloc);

Can @alloc be NULL here? I see that the eventual caller from
qemuProcessResctrlCreate would pass vm->def->resctrls[i]->alloc after a
virResctrlAllocCreate, but that API can return 0 immediately "if
(!alloc)"?

Furthermore, if we Ref it here, but return -1 in the subsequent steps
are we sure that the Unref gets done.

The one "thing" about the order of patches here is that it forces me to
look forward to ensure decisions made in previous patches will be
handled in the future.

> +
> +    if (virResctrlMonitorDeterminePath(monitor, machinename) < 0)
> +        return -1;

At least for now virResctrlMonitorDeterminePath can handle a NULL
monitor->alloc...

John

> +
> +    lockfd = virResctrlLockWrite();
> +    if (lockfd < 0)
> +        return -1;
> +
> +    ret = virResctrlCreateGroupPath(monitor->path);
> +
> +    virResctrlUnlock(lockfd);
> +    return ret;
> +}
> diff --git a/src/util/virresctrl.h b/src/util/virresctrl.h
> index 69b6b1d..1efe394 100644
> --- a/src/util/virresctrl.h
> +++ b/src/util/virresctrl.h
> @@ -196,7 +196,13 @@ virResctrlMonitorNew(void);
>  int
>  virResctrlMonitorAddPID(virResctrlMonitorPtr monitor,
>                          pid_t pid);
> +
>  int
>  virResctrlMonitorDeterminePath(virResctrlMonitorPtr monitor,
>                                 const char *machinename);
> +
> +int
> +virResctrlMonitorCreate(virResctrlAllocPtr alloc,
> +                        virResctrlMonitorPtr monitor,
> +                        const char *machinename);
>  #endif /*  __VIR_RESCTRL_H__ */
>