Re: [libvirt] [PATCH 4/8] process: Translate "unlimited" correctly
On Fri, 2017-03-24 at 13:47 -0400, Luiz Capitulino wrote:
> @@ -747,7 +747,15 @@ virProcessSetMaxMemLock(pid_t pid, unsigned
long long bytes)
> if (bytes == 0)
> return 0;
>
> - rlim.rlim_cur = rlim.rlim_max = bytes;
> + /* We use VIR_DOMAIN_MEMORY_PARAM_UNLIMITED internally to represent
> + * unlimited memory amounts, but setrlimit() and prlimit() use
> + * RLIM_INFINITY for the same purpose, so we need to translate between
> + * the two conventions */
> + if (virMemoryLimitIsSet(bytes))
> + rlim.rlim_cur = rlim.rlim_max = bytes;
> + else
> + rlim.rlim_cur = rlim.rlim_max = RLIM_INFINITY;
I know I'm not very smart, but I had trouble parsing this. What
about:
if (virMemoryLimitIsInfinity(bytes))
rlim.rlim_cur = rlim.rlim_max = RLIM_INFINITY;
...
This reads better, and avoids using virMemoryLimitIsSet() which
seems very error-prone. It doesn't check for zero and it's strange
that "limit < infinity" means "limit is set".
I would love to have something like that, and in fact I
spent a significant amount of time trying to clean up the
way libvirt stores and represents memory limits. The short
version is: not gonna happen ;)
Moreover, while the current API looks poorly named in this
context, it's also used for other memory limits and the
names make more sense there, so it's not a terrible API
when you look at the big picture.
The case where bytes is zero is accounted for, though. You
can see it right at the beginning of the hunk.
--
Andrea Bolognani / Red Hat / Virtualization