[libvirt PATCH 04/17] util: Introduce virProcess{Get,Set}Limit()

[Michal Privoznik]

On 3/5/21 8:13 PM, Andrea Bolognani wrote:
> These functions abstract part of the existing logic, which is
> the same in all virProcessSetMax*() functions, and changes it
> so that which underlying syscall is used depends on their
> availability rather than on the context in which they are
> called: since prlimit() and {g,s}etrlimit() have slightly
> different requirements, using the same one every single time
> should make for a more consistent experience.
> 
> As part of the change, we also remove the special case for
> passing zero to virProcessSetMax*() functions: we have removed
> all callers that depended on that functionality in the previous
> commit, so this is now safe to do and makes the semantics
> simpler.
> 
> This commit is better viewed with 'git show -w'.
> 
> Signed-off-by: Andrea Bolognani <abologna at redhat.com>
> ---
>   src/util/virprocess.c | 175 +++++++++++++++++++++++-------------------
>   1 file changed, 95 insertions(+), 80 deletions(-)
> 
> diff --git a/src/util/virprocess.c b/src/util/virprocess.c
> index e01ff25540..38b248217e 100644
> --- a/src/util/virprocess.c
> +++ b/src/util/virprocess.c
> @@ -738,10 +738,66 @@ virProcessPrLimit(pid_t pid G_GNUC_UNUSED,
>   }
>   #endif
>   
> +#if WITH_GETRLIMIT
> +static int
> +virProcessGetRLimit(int resource,
> +                    struct rlimit *old_limit)
> +{
> +    return getrlimit(resource, old_limit);
> +}
> +#endif /* WITH_GETRLIMIT */
> +
> +#if WITH_SETRLIMIT
> +static int
> +virProcessSetRLimit(int resource,
> +                    const struct rlimit *new_limit)
> +{
> +    return setrlimit(resource, new_limit);
> +}
> +#endif /* WITH_SETRLIMIT */
> +
> +#if WITH_GETRLIMIT

Question of preference. I'd rather have these two WITH_GETRLIMIT 
sections joined together.... [1]

> +static int
> +virProcessGetLimit(pid_t pid,
> +                   int resource,
> +                   struct rlimit *old_limit)
> +{
> +    pid_t current_pid = getpid();
> +    bool same_process = (pid == current_pid);
> +
> +    if (virProcessPrLimit(pid, resource, NULL, old_limit) == 0)
> +        return 0;
> +
> +    if (same_process && virProcessGetRLimit(resource, old_limit) == 0)
> +        return 0;
> +
> +    return -1;
> +}
> +#endif /* WITH_GETRLIMIT */
> +
> +#if WITH_SETRLIMIT

1: ... just like these two WITH_SETRLIMIT. But it's matter of taste, I 
agree.

> +static int
> +virProcessSetLimit(pid_t pid,
> +                   int resource,
> +                   const struct rlimit *new_limit)
> +{
> +    pid_t current_pid = getpid();
> +    bool same_process = (pid == current_pid);
> +
> +    if (virProcessPrLimit(pid, resource, new_limit, NULL) == 0)
> +        return 0;
> +
> +    if (same_process && virProcessSetRLimit(resource, new_limit) == 0)
> +        return 0;
> +
> +    return -1;
> +}
> +#endif /* WITH_SETRLIMIT */
> +

Michal