On 03/09/2011 01:13 AM, KAMEZAWA Hiroyuki wrote:
> Subject: [PATCH] rename swap_limit as memswap_limit.
>
> cgroup's /cgroup/memory/memory.memsw.limit_in_bytes is not
> for limitinit 'swap' but for 'memory+swap' (then, it's memsw)
> (So, this number cannot be smaller than memory.limit_in_bytes)
>
> Note:
> @@ -323,10 +323,11 @@
> <dd> The optional <code>soft_limit</code> element is the memory limit to
> enforce during memory contention. The units for this value are
> kilobytes (i.e. blocks of 1024 bytes)</dd>
> - <dt><code>swap_hard_limit</code></dt>
> - <dd> The optional <code>swap_hard_limit</code> element is the maximum
> - swap the guest can use. The units for this value are kilobytes
> + <dt><code>memswap_hard_limit</code></dt>
> + <dd> The optional <code>memswap_hard_limit</code> element is the maximum
> + memory+swap the guest can use. The units for this value are kilobytes
For the record - patching this sentence to mention that
<code>swap_hard_limit</code> covers memory plus swap is reasonable,
> /**
> - * VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT:
> + * VIR_DOMAIN_MEMORY_MEMSWAP_HARD_LIMIT:
> *
> - * Macro for the swap tunable swap_hard_limit: it represents the maximum swap
> - * the guest can use.
> + * Macro for the swap tunable memswap_hard_limit: it represents the maximum
> + * memory+swap the guest can use.
as is updating this documentation sentence (but not renaming the macro).
I'd rather see 'memory plus swap' than 'memory+swap' in documentation.
> -virCgroupGetSwapHardLimit;
> +virCgroupGetMemSwapHardLimit;
Renaming this internal virCgroup helper function is doable; I'm not sure
whether we need that rename, but it doesn't hurt (anything in
libvirt_private.syms can be renamed or API altered at will).
> - if (virStrcpyStatic(param->field, VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT) == NULL) {
> + if (virStrcpyStatic(param->field, VIR_DOMAIN_MEMORY_MEMSWAP_HARD_LIMIT) == NULL) {
> lxcError(VIR_ERR_INTERNAL_ERROR,
> - "%s", _("Field swap hard limit too long for destination"));
> + "%s", _("Field memory+swap hard limit too long for destination"));
Tweaking the error message is okay.
Not very many places left in this patch that are worth keeping, but at
least you've already identified where to document things.
One alternative idea is leaving the field with the name swap_hard_limit,
and manually computing that limit that we write to the cgroup file by
adding in the memory hard limit, and when reading back from a cgroup
file, doing two reads and the subtraction so that the user is just
presented with a swap limit. That is, change the API to do what the
name implies, instead of changing the name to match what the underlying
implementation expects, by making libvirt be the glue that does the math
for you. However, that's potentially confusing in its own right, since
swap limit isn't a notion directly visible in the cgroup controller, and
also not quite backwards compatible (it doesn't break API, but older
guests might not be expecting the newer semantics and use an incorrect
size for a limit when expecting the cgroup semantics).
--
Eric Blake eblake@xxxxxxxxxx +1-801-349-2682
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list