[libvirt] [PATCH 2/6] process: Add virProcessGetMaxMemLock()

[John Ferlan]

On 11/24/2015 08:56 AM, Andrea Bolognani wrote:
> This function can be used to retrieve the current locked memory
> limit for a process, so that the setting can be later restored.
> 
> Add a configure check for getrlimit(), which we now use.
> ---
>  configure.ac             |  2 +-
>  src/libvirt_private.syms |  1 +
>  src/util/virprocess.c    | 42 ++++++++++++++++++++++++++++++++++++++++++
>  src/util/virprocess.h    |  2 ++
>  4 files changed, 46 insertions(+), 1 deletion(-)
> 
> diff --git a/configure.ac b/configure.ac
> index f481c50..c2a567f 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -279,7 +279,7 @@ AC_CHECK_SIZEOF([long])
>  dnl Availability of various common functions (non-fatal if missing),
>  dnl and various less common threadsafe functions
>  AC_CHECK_FUNCS_ONCE([cfmakeraw fallocate geteuid getgid getgrnam_r \
> -  getmntent_r getpwuid_r getuid kill mmap newlocale posix_fallocate \
> +  getmntent_r getpwuid_r getrlimit getuid kill mmap newlocale posix_fallocate \
>    posix_memalign prlimit regexec sched_getaffinity setgroups setns \
>    setrlimit symlink sysctlbyname getifaddrs sched_setscheduler])
>  
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 7e60d87..e330a16 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -2022,6 +2022,7 @@ virPortAllocatorSetUsed;
>  virProcessAbort;
>  virProcessExitWithStatus;
>  virProcessGetAffinity;
> +virProcessGetMaxMemLock;
>  virProcessGetNamespaces;
>  virProcessGetPids;
>  virProcessGetStartTime;
> diff --git a/src/util/virprocess.c b/src/util/virprocess.c
> index 9b38834..b14164a 100644
> --- a/src/util/virprocess.c
> +++ b/src/util/virprocess.c
> @@ -788,6 +788,48 @@ virProcessSetMaxMemLock(pid_t pid ATTRIBUTE_UNUSED, unsigned long long bytes)
>  }
>  #endif /* ! (HAVE_SETRLIMIT && defined(RLIMIT_MEMLOCK)) */
>  
> +#if HAVE_GETRLIMIT && defined(RLIMIT_MEMLOCK)
> +int
> +virProcessGetMaxMemLock(pid_t pid,
> +                        unsigned long long *bytes)

Another option would be to return bytes...

where at least as I read patch 3 bytes == 0 is no different than
original_memlock == 0 especially if arg2 is NULL

Of course, does calling getrlimit or virProcessPrLimit() potentially
multiple times make sense? Does blasting those error messages each time
to the log, but continuing on cause confusion?

I guess what I'm thinking about is how it's eventually used in patch 3
and the concept of failure because something in here fails or perhaps
the getrlimit *or* prlimit isn't supported on the platform...

> +{
> +    struct rlimit rlim;
> +
> +    if (!bytes)
> +        return 0;

Since you return 0 here if passed a NULL bytes, then I think you'd have
to do so in the other API

> +
> +    if (pid == 0) {
> +        if (getrlimit(RLIMIT_MEMLOCK, &rlim) < 0) {
> +            virReportSystemError(errno,
> +                                 "%s",

This one could go on previous line - no big deal other than extra line

> +                                 _("cannot get locked memory limit"));
> +            return -1;
> +        }
> +    } else {
> +        if (virProcessPrLimit(pid, RLIMIT_MEMLOCK, NULL, &rlim) < 0) {
> +            virReportSystemError(errno,
> +                                 _("cannot get locked memory limit "
> +                                   "of process %lld"),
> +                                 (long long int) pid);
> +            return -1;
> +        }
> +    }
> +
> +    /* virProcessSetMaxMemLock() sets both rlim_cur and rlim_max to the
> +     * same value, so we can retrieve just rlim_max here */
> +    *bytes = rlim.rlim_max;

One oddball thought... what if rlim.rlim_max == 0?  Is that possible? I
suppose only if rlim_cur == 0 too, right?  Not sure it makes sense and I
didn't chase the syscall. It does say rlim_cur can be 0 and that
rlim_max must be equal or higher...

I'm just trying to logically follow through on the thought of how 0 is
used in patch 3.

> +
> +    return 0;
> +}
> +#else /* ! (HAVE_GETRLIMIT && defined(RLIMIT_MEMLOCK)) */
> +int
> +virProcessGetMaxMemLock(pid_t pid ATTRIBUTE_UNUSED,
> +                        unsigned long long *bytes)  

Would technically be unused if kept as is... However since the other API
returns 0 when value is passed as NULL, this could too.


John
> +{
> +    virReportSystemError(ENOSYS, "%s", _("Not supported on this platform"));
> +    return -1;
> +}
> +#endif /* ! (HAVE_GETRLIMIT && defined(RLIMIT_MEMLOCK)) */
>  
>  #if HAVE_SETRLIMIT && defined(RLIMIT_NPROC)
>  int
> diff --git a/src/util/virprocess.h b/src/util/virprocess.h
> index 1768009..a7a1fe9 100644
> --- a/src/util/virprocess.h
> +++ b/src/util/virprocess.h
> @@ -76,6 +76,8 @@ int virProcessSetMaxMemLock(pid_t pid, unsigned long long bytes);
>  int virProcessSetMaxProcesses(pid_t pid, unsigned int procs);
>  int virProcessSetMaxFiles(pid_t pid, unsigned int files);
>  
> +int virProcessGetMaxMemLock(pid_t pid, unsigned long long *bytes);
> +
>  /* Callback to run code within the mount namespace tied to the given
>   * pid.  This function must use only async-signal-safe functions, as
>   * it gets run after a fork of a multi-threaded process.  The return
>