On Thu, 03 Mar 2011 14:47:36 +0800 Osier Yang <jyang(a)redhat.com&gt; wrote: A patch (full change version) is here. maybe good input for discussion. == From: KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com&gt; Date: Fri, 4 Mar 2011 14:46:08 +0900 Subject: [PATCH] Rename swap_hard_limit as memswap_hard_limit This patch affects qemu and lxc, using memory cgroup. Linux's memory cgroup's memory.memsw.limit_in_bytes is a limit for memory+swap, not for swap. (This behavior is for avoiding bad influence for global vmscan and never disturb kswapd behavior by user's setting.) libvirt's support for memsw.limit_in_bytes is named as swap_hard_limit and documenteda as "hard limit for swap usage". This is misleading. This patch renames all swap_hard_limit things as memswap_hard_limit. After this, domain's XML definition <swap_hard_limit> ... </swap_hard_limit> will be <memswap_hard_limit> ...</memswap_hard_limit> And macro VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT should be VIR_DOMAIN_MEMORY_MEMSWAP_HARD_LIMIT virsh commands' --swap-hard-limit will be memswap-hard-limit virCgroupGetSwapHardLimit will be virCgroupGetMemSwapHardLimit will be virCgroupSetSwapHardLimit will be virCgroupSetMemSwapHardLimit This patch keeps backwoard compatibility in XML parser and virsh memtune commandline argument. But it shows warning. Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com&gt; --- docs/formatdomain.html.in | 9 ++++--- docs/schemas/domain.rng | 2 +- include/libvirt/libvirt.h.in | 16 +++++++++--- src/conf/domain_conf.c | 24 +++++++++++++------ src/conf/domain_conf.h | 2 +- src/libvirt_private.syms | 4 +- src/lxc/lxc_controller.c | 8 +++--- src/lxc/lxc_driver.c | 12 +++++----- src/qemu/qemu_cgroup.c | 5 ++- src/qemu/qemu_driver.c | 16 ++++++------ src/util/cgroup.c | 10 ++++---- src/util/cgroup.h | 4 +- tests/qemuxml2argvdata/qemuxml2argv-memtune.xml | 2 +- tools/virsh.c | 28 ++++++++++++++-------- 14 files changed, 84 insertions(+), 58 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 84b1cab..60cf572 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -284,7 +284,7 @@ &lt;memtune&gt; &lt;hard_limit&gt;1048576&lt;/hard_limit&gt; &lt;soft_limit&gt;131072&lt;/soft_limit&gt; - &lt;swap_hard_limit&gt;2097152&lt;/swap_hard_limit&gt; + &lt;memswap_hard_limit&gt;2097152&lt;/memswap_hard_limit&gt; &lt;min_guarantee&gt;65536&lt;/min_guarantee&gt; &lt;/memtune&gt; &lt;vcpu cpuset="1-4,^3,6" current="1"&gt;2&lt;/vcpu&gt; @@ -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 (i.e. blocks of 1024 bytes)</dd> + </span> <dt><code>min_guarantee</code></dt> <dd> The optional <code>min_guarantee</code> element is the guaranteed minimum memory allocation for the guest. The units for this value are diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng index 8b215f3..2ac50a4 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -341,7 +341,7 @@ </optional> <!-- Maximum swap area the VM can use --> <optional> - <element name="swap_hard_limit"> + <element name="memswap_hard_limit"> <ref name="memoryKB"/> </element> </optional> diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 5dfb752..0b397b8 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -735,13 +735,21 @@ typedef enum { #define VIR_DOMAIN_MEMORY_MIN_GUARANTEE "min_guarantee" /** - * 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. */ -#define VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT "swap_hard_limit" +#define VIR_DOMAIN_MEMORY_MEMSWAP_HARD_LIMIT "memswap_hard_limit" + +/** + * VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT + * + * macro for comaptibility with old version. please use + * VIR_DOMAIN_MEMORY_MEMSWAP_HARD_LIMIT + */ +#define VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT "memswap_hard_limit" /** * virDomainMemoryParameter: diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index c8350c6..555e90e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5188,9 +5188,17 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, &def->mem.min_guarantee) < 0) def->mem.min_guarantee = 0; - if (virXPathULong("string(./memtune/swap_hard_limit[1])", ctxt, - &def->mem.swap_hard_limit) < 0) - def->mem.swap_hard_limit = 0; + if (virXPathULong("string(./memtune/memswap_hard_limit[1])", ctxt, + &def->mem.memswap_hard_limit) < 0) + def->mem.memswap_hard_limit = 0; + /* internal support for old style */ + if (def->mem.memswap_hard_limit == 0 && + virXPathULong("string(./memtune/swap_hard_limit[1])", ctxt, + &def->mem.memswap_hard_limit) < 0) { + def->mem.memswap_hard_limit = 0; + VIR_WARN("%s", _("<swap_hard_limit> is obsolete, please use" + "<memswap_hard_limit>")); + } n = virXPathULong("string(./vcpu[1])", ctxt, &count); if (n == -2) { @@ -7681,7 +7689,7 @@ char *virDomainDefFormat(virDomainDefPtr def, /* add memtune only if there are any */ if (def->mem.hard_limit || def->mem.soft_limit || def->mem.min_guarantee || - def->mem.swap_hard_limit) + def->mem.memswap_hard_limit) virBufferVSprintf(&buf, " <memtune>\n"); if (def->mem.hard_limit) { virBufferVSprintf(&buf, " <hard_limit>%lu</hard_limit>\n", @@ -7695,12 +7703,12 @@ char *virDomainDefFormat(virDomainDefPtr def, virBufferVSprintf(&buf, " <min_guarantee>%lu</min_guarantee>\n", def->mem.min_guarantee); } - if (def->mem.swap_hard_limit) { - virBufferVSprintf(&buf, " <swap_hard_limit>%lu</swap_hard_limit>\n", - def->mem.swap_hard_limit); + if (def->mem.memswap_hard_limit) { + virBufferVSprintf(&buf, " <memswap_hard_limit>%lu</memswap_hard_limit>\n", + def->mem.memswap_hard_limit); } if (def->mem.hard_limit || def->mem.soft_limit || def->mem.min_guarantee || - def->mem.swap_hard_limit) + def->mem.memswap_hard_limit) virBufferVSprintf(&buf, " </memtune>\n"); if (def->mem.hugepage_backed) { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 30aeccc..f71321c 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1053,7 +1053,7 @@ struct _virDomainDef { unsigned long hard_limit; unsigned long soft_limit; unsigned long min_guarantee; - unsigned long swap_hard_limit; + unsigned long memswap_hard_limit; } mem; unsigned short vcpus; unsigned short maxvcpus; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 5e63a12..fce0fe5 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -74,7 +74,7 @@ virCgroupGetFreezerState; virCgroupGetMemoryHardLimit; virCgroupGetMemorySoftLimit; virCgroupGetMemoryUsage; -virCgroupGetSwapHardLimit; +virCgroupGetMemSwapHardLimit; virCgroupKill; virCgroupKillRecursive; virCgroupKillPainfully; @@ -86,7 +86,7 @@ virCgroupSetFreezerState; virCgroupSetMemory; virCgroupSetMemoryHardLimit; virCgroupSetMemorySoftLimit; -virCgroupSetSwapHardLimit; +virCgroupSetMemSwapHardLimit; # command.h diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index d2b113c..8a62eaa 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -145,12 +145,12 @@ static int lxcSetContainerResources(virDomainDefPtr def) } } - if(def->mem.swap_hard_limit) { - rc = virCgroupSetSwapHardLimit(cgroup, def->mem.swap_hard_limit); + if(def->mem.memswap_hard_limit) { + rc = virCgroupSetMemSwapHardLimit(cgroup, def->mem.memswap_hard_limit); if (rc != 0) { virReportSystemError(-rc, - _("Unable to set swap hard limit for domain %s"), - def->name); + _("Unable to set memory+swap hard limit for domain %s"), + def->name); goto cleanup; } } diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 5b6f784..1e3a05a 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -770,16 +770,16 @@ static int lxcDomainSetMemoryParameters(virDomainPtr dom, _("unable to set memory soft_limit tunable")); ret = -1; } - } else if (STREQ(param->field, VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT)) { + } else if (STREQ(param->field, VIR_DOMAIN_MEMORY_MEMSWAP_HARD_LIMIT)) { int rc; if (param->type != VIR_DOMAIN_MEMORY_PARAM_ULLONG) { lxcError(VIR_ERR_INVALID_ARG, "%s", - _("invalid type for swap_hard_limit tunable, expected a 'ullong'")); + _("invalid type for memswap_hard_limit tunable, expected a 'ullong'")); ret = -1; continue; } - rc = virCgroupSetSwapHardLimit(cgroup, params[i].value.ul); + rc = virCgroupSetMemSwapHardLimit(cgroup, params[i].value.ul); if (rc != 0) { virReportSystemError(-rc, "%s", _("unable to set swap_hard_limit tunable")); @@ -885,15 +885,15 @@ static int lxcDomainGetMemoryParameters(virDomainPtr dom, break; case 2: /* fill swap hard limit here */ - rc = virCgroupGetSwapHardLimit(cgroup, &val); + rc = virCgroupGetMemSwapHardLimit(cgroup, &val); if (rc != 0) { virReportSystemError(-rc, "%s", _("unable to get swap hard limit")); goto cleanup; } - 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")); goto cleanup; } param->value.ul = val; diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index e71d3fa..0bac1be 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -330,8 +330,9 @@ int qemuSetupCgroup(struct qemud_driver *driver, } } - if (vm->def->mem.swap_hard_limit != 0) { - rc = virCgroupSetSwapHardLimit(cgroup, vm->def->mem.swap_hard_limit); + if (vm->def->mem.memswap_hard_limit != 0) { + rc = virCgroupSetMemSwapHardLimit(cgroup, + vm->def->mem.memswap_hard_limit); if (rc != 0) { virReportSystemError(-rc, _("Unable to set swap hard limit for domain %s"), diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c9095bb..9f55ca8 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4521,19 +4521,19 @@ static int qemuDomainSetMemoryParameters(virDomainPtr dom, _("unable to set memory soft_limit tunable")); ret = -1; } - } else if (STREQ(param->field, VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT)) { + } else if (STREQ(param->field, VIR_DOMAIN_MEMORY_MEMSWAP_HARD_LIMIT)) { int rc; if (param->type != VIR_DOMAIN_MEMORY_PARAM_ULLONG) { qemuReportError(VIR_ERR_INVALID_ARG, "%s", - _("invalid type for swap_hard_limit tunable, expected a 'ullong'")); + _("invalid type for memswap_hard_limit tunable, expected a 'ullong'")); ret = -1; continue; } - rc = virCgroupSetSwapHardLimit(group, params[i].value.ul); + rc = virCgroupSetMemSwapHardLimit(group, params[i].value.ul); if (rc != 0) { virReportSystemError(-rc, "%s", - _("unable to set swap_hard_limit tunable")); + _("unable to set memswap_hard_limit tunable")); ret = -1; } } else if (STREQ(param->field, VIR_DOMAIN_MEMORY_MIN_GUARANTEE)) { @@ -4641,15 +4641,15 @@ static int qemuDomainGetMemoryParameters(virDomainPtr dom, break; case 2: /* fill swap hard limit here */ - rc = virCgroupGetSwapHardLimit(group, &val); + rc = virCgroupGetMemSwapHardLimit(group, &val); if (rc != 0) { virReportSystemError(-rc, "%s", - _("unable to get swap hard limit")); + _("unable to get memory+swap hard limit")); goto cleanup; } - if (virStrcpyStatic(param->field, VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT) == NULL) { + if (virStrcpyStatic(param->field, VIR_DOMAIN_MEMORY_MEMSWAP_HARD_LIMIT) == NULL) { qemuReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("Field swap hard limit too long for destination")); + "%s", _("Field memory+swap hard limit too long for destination")); goto cleanup; } param->value.ul = val; diff --git a/src/util/cgroup.c b/src/util/cgroup.c index c5b8cdd..a2de923 100644 --- a/src/util/cgroup.c +++ b/src/util/cgroup.c @@ -1033,14 +1033,14 @@ int virCgroupGetMemorySoftLimit(virCgroupPtr group, unsigned long long *kb) } /** - * virCgroupSetSwapHardLimit: + * virCgroupSetMemSwapHardLimit: * - * @group: The cgroup to change swap hard limit for + * @group: The cgroup to change memory+swap hard limit for * @kb: The swap amount in kilobytes * * Returns: 0 on success */ -int virCgroupSetSwapHardLimit(virCgroupPtr group, unsigned long long kb) +int virCgroupSetMemSwapHardLimit(virCgroupPtr group, unsigned long long kb) { unsigned long long maxkb = VIR_DOMAIN_MEMORY_PARAM_UNLIMITED; @@ -1061,12 +1061,12 @@ int virCgroupSetSwapHardLimit(virCgroupPtr group, unsigned long long kb) /** * virCgroupGetSwapHardLimit: * - * @group: The cgroup to get swap hard limit for + * @group: The cgroup to get memory+swap hard limit for * @kb: The swap amount in kilobytes * * Returns: 0 on success */ -int virCgroupGetSwapHardLimit(virCgroupPtr group, unsigned long long *kb) +int virCgroupGetMemSwapHardLimit(virCgroupPtr group, unsigned long long *kb) { long long unsigned int limit_in_bytes; int ret; diff --git a/src/util/cgroup.h b/src/util/cgroup.h index d468cb3..fd16a40 100644 --- a/src/util/cgroup.h +++ b/src/util/cgroup.h @@ -52,8 +52,8 @@ int virCgroupSetMemoryHardLimit(virCgroupPtr group, unsigned long long kb); int virCgroupGetMemoryHardLimit(virCgroupPtr group, unsigned long long *kb); int virCgroupSetMemorySoftLimit(virCgroupPtr group, unsigned long long kb); int virCgroupGetMemorySoftLimit(virCgroupPtr group, unsigned long long *kb); -int virCgroupSetSwapHardLimit(virCgroupPtr group, unsigned long long kb); -int virCgroupGetSwapHardLimit(virCgroupPtr group, unsigned long long *kb); +int virCgroupSetMemSwapHardLimit(virCgroupPtr group, unsigned long long kb); +int virCgroupGetMemSwapHardLimit(virCgroupPtr group, unsigned long long *kb); int virCgroupDenyAllDevices(virCgroupPtr group); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-memtune.xml b/tests/qemuxml2argvdata/qemuxml2argv-memtune.xml index 37b5c88..e0a1825 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-memtune.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-memtune.xml @@ -6,7 +6,7 @@ <memtune> <hard_limit>512000</hard_limit> <soft_limit>128000</soft_limit> - <swap_hard_limit>1024000</swap_hard_limit> + <memswap_hard_limit>1024000</memswap_hard_limit> </memtune> <vcpu>1</vcpu> <os> diff --git a/tools/virsh.c b/tools/virsh.c index 62fca17..62df3b9 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -3032,8 +3032,8 @@ static const vshCmdOptDef opts_memtune[] = { N_("Max memory in kilobytes")}, {"soft-limit", VSH_OT_INT, VSH_OFLAG_NONE, N_("Memory during contention in kilobytes")}, - {"swap-hard-limit", VSH_OT_INT, VSH_OFLAG_NONE, - N_("Max swap in kilobytes")}, + {"memswap-hard-limit", VSH_OT_INT, VSH_OFLAG_NONE, + N_("Max memory+swap in kilobytes")}, {"min-guarantee", VSH_OT_INT, VSH_OFLAG_NONE, N_("Min guaranteed memory in kilobytes")}, {NULL, 0, 0, NULL} @@ -3043,7 +3043,7 @@ static int cmdMemtune(vshControl * ctl, const vshCmd * cmd) { virDomainPtr dom; - long long hard_limit, soft_limit, swap_hard_limit, min_guarantee; + long long hard_limit, soft_limit, memswap_hard_limit, min_guarantee; int nparams = 0; unsigned int i = 0; virMemoryParameterPtr params = NULL, temp = NULL; @@ -3065,10 +3065,18 @@ cmdMemtune(vshControl * ctl, const vshCmd * cmd) if (soft_limit) nparams++; - swap_hard_limit = - vshCommandOptLongLong(cmd, "swap-hard-limit", NULL); - if (swap_hard_limit) + memswap_hard_limit = + vshCommandOptLongLong(cmd, "memswap-hard-limit", NULL); + if (memswap_hard_limit) { nparams++; + } else { /* for backward compatibility */ + memswap_hard_limit = + vshCommandOptLongLong(cmd, "swap-hard-limit", NULL); + if (memswap_hard_limit) + vshError(ctl, "%s", + _("Warning swap_hard_limit is obsolete,use memswap_hard_limit")); + goto cleanup; + } min_guarantee = vshCommandOptLongLong(cmd, "min-guarantee", NULL); @@ -3155,11 +3163,11 @@ cmdMemtune(vshControl * ctl, const vshCmd * cmd) strncpy(temp->field, VIR_DOMAIN_MEMORY_HARD_LIMIT, sizeof(temp->field)); hard_limit = 0; - } else if (swap_hard_limit) { - temp->value.ul = swap_hard_limit; - strncpy(temp->field, VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT, + } else if (memswap_hard_limit) { + temp->value.ul = memswap_hard_limit; + strncpy(temp->field, VIR_DOMAIN_MEMORY_MEMSWAP_HARD_LIMIT, sizeof(temp->field)); - swap_hard_limit = 0; + memswap_hard_limit = 0; } else if (min_guarantee) { temp->value.ul = min_guarantee; strncpy(temp->field, VIR_DOMAIN_MEMORY_MIN_GUARANTEE, -- 1.7.4.1

On Mon, 07 Mar 2011 15:11:12 +0530 "Nikunj A. Dadhania" <nikunj(a)linux.vnet.ibm.com&gt; wrote: Thanks, rebased onto the latest git tree. == From: KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com&gt; Date: Wed, 9 Mar 2011 17:18:41 +0900 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: If other hypervisors than Linux support this and meaning is not same as memory+swap, the name swap_hard_limit will have confusion. Changelog: - rebased onto the latest libvirt-git. Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com&gt; Reviewed-by: "Nikunj A. Dadhania" <nikunj(a)linux.vnet.ibm.com&gt; --- docs/formatdomain.html.in | 9 ++++--- docs/schemas/domain.rng | 2 +- include/libvirt/libvirt.h.in | 16 +++++++++++--- src/conf/domain_conf.c | 24 +++++++++++++++------- src/conf/domain_conf.h | 2 +- src/libvirt_private.syms | 4 +- src/lxc/lxc_controller.c | 8 +++--- src/lxc/lxc_driver.c | 12 +++++----- src/qemu/qemu_cgroup.c | 5 ++- src/qemu/qemu_driver.c | 16 +++++++------- src/util/cgroup.c | 10 ++++---- src/util/cgroup.h | 4 +- tests/qemuxml2argvdata/qemuxml2argv-memtune.xml | 2 +- tools/virsh.c | 18 ++++++++-------- 14 files changed, 75 insertions(+), 57 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 0c7c965..f764d09 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -284,7 +284,7 @@ &lt;memtune&gt; &lt;hard_limit&gt;1048576&lt;/hard_limit&gt; &lt;soft_limit&gt;131072&lt;/soft_limit&gt; - &lt;swap_hard_limit&gt;2097152&lt;/swap_hard_limit&gt; + &lt;memswap_hard_limit&gt;2097152&lt;/memswap_hard_limit&gt; &lt;min_guarantee&gt;65536&lt;/min_guarantee&gt; &lt;/memtune&gt; &lt;vcpu cpuset="1-4,^3,6" current="1"&gt;2&lt;/vcpu&gt; @@ -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 (i.e. blocks of 1024 bytes)</dd> + </span> <dt><code>min_guarantee</code></dt> <dd> The optional <code>min_guarantee</code> element is the guaranteed minimum memory allocation for the guest. The units for this value are diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng index 8b215f3..2ac50a4 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -341,7 +341,7 @@ </optional> <!-- Maximum swap area the VM can use --> <optional> - <element name="swap_hard_limit"> + <element name="memswap_hard_limit"> <ref name="memoryKB"/> </element> </optional> diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 618b350..d2600fa 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -735,13 +735,21 @@ typedef enum { #define VIR_DOMAIN_MEMORY_MIN_GUARANTEE "min_guarantee" /** - * 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. */ -#define VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT "swap_hard_limit" +#define VIR_DOMAIN_MEMORY_MEMSWAP_HARD_LIMIT "memswap_hard_limit" + +/** + * VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT + * + * macro for comaptibility with old version. please use + * VIR_DOMAIN_MEMORY_MEMSWAP_HARD_LIMIT + */ +#define VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT "memswap_hard_limit" /** * virDomainMemoryParameter: diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 16e1291..fdf37e1 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5192,9 +5192,17 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, &def->mem.min_guarantee) < 0) def->mem.min_guarantee = 0; - if (virXPathULong("string(./memtune/swap_hard_limit[1])", ctxt, - &def->mem.swap_hard_limit) < 0) - def->mem.swap_hard_limit = 0; + if (virXPathULong("string(./memtune/memswap_hard_limit[1])", ctxt, + &def->mem.memswap_hard_limit) < 0) + def->mem.memswap_hard_limit = 0; + /* internal support for old style */ + if (def->mem.memswap_hard_limit == 0 && + virXPathULong("string(./memtune/swap_hard_limit[1])", ctxt, + &def->mem.memswap_hard_limit) < 0) { + def->mem.memswap_hard_limit = 0; + VIR_WARN("%s", _("<swap_hard_limit> is obsolete, please use" + "<memswap_hard_limit>")); + } n = virXPathULong("string(./vcpu[1])", ctxt, &count); if (n == -2) { @@ -7685,7 +7693,7 @@ char *virDomainDefFormat(virDomainDefPtr def, /* add memtune only if there are any */ if (def->mem.hard_limit || def->mem.soft_limit || def->mem.min_guarantee || - def->mem.swap_hard_limit) + def->mem.memswap_hard_limit) virBufferVSprintf(&buf, " <memtune>\n"); if (def->mem.hard_limit) { virBufferVSprintf(&buf, " <hard_limit>%lu</hard_limit>\n", @@ -7699,12 +7707,12 @@ char *virDomainDefFormat(virDomainDefPtr def, virBufferVSprintf(&buf, " <min_guarantee>%lu</min_guarantee>\n", def->mem.min_guarantee); } - if (def->mem.swap_hard_limit) { - virBufferVSprintf(&buf, " <swap_hard_limit>%lu</swap_hard_limit>\n", - def->mem.swap_hard_limit); + if (def->mem.memswap_hard_limit) { + virBufferVSprintf(&buf, " <memswap_hard_limit>%lu</memswap_hard_limit>\n", + def->mem.memswap_hard_limit); } if (def->mem.hard_limit || def->mem.soft_limit || def->mem.min_guarantee || - def->mem.swap_hard_limit) + def->mem.memswap_hard_limit) virBufferVSprintf(&buf, " </memtune>\n"); if (def->mem.hugepage_backed) { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 30aeccc..f71321c 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1053,7 +1053,7 @@ struct _virDomainDef { unsigned long hard_limit; unsigned long soft_limit; unsigned long min_guarantee; - unsigned long swap_hard_limit; + unsigned long memswap_hard_limit; } mem; unsigned short vcpus; unsigned short maxvcpus; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index efcf3c5..f7a308e 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -74,7 +74,7 @@ virCgroupGetFreezerState; virCgroupGetMemoryHardLimit; virCgroupGetMemorySoftLimit; virCgroupGetMemoryUsage; -virCgroupGetSwapHardLimit; +virCgroupGetMemSwapHardLimit; virCgroupKill; virCgroupKillRecursive; virCgroupKillPainfully; @@ -86,7 +86,7 @@ virCgroupSetFreezerState; virCgroupSetMemory; virCgroupSetMemoryHardLimit; virCgroupSetMemorySoftLimit; -virCgroupSetSwapHardLimit; +virCgroupSetMemSwapHardLimit; # command.h diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index d2b113c..8a62eaa 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -145,12 +145,12 @@ static int lxcSetContainerResources(virDomainDefPtr def) } } - if(def->mem.swap_hard_limit) { - rc = virCgroupSetSwapHardLimit(cgroup, def->mem.swap_hard_limit); + if(def->mem.memswap_hard_limit) { + rc = virCgroupSetMemSwapHardLimit(cgroup, def->mem.memswap_hard_limit); if (rc != 0) { virReportSystemError(-rc, - _("Unable to set swap hard limit for domain %s"), - def->name); + _("Unable to set memory+swap hard limit for domain %s"), + def->name); goto cleanup; } } diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 5b6f784..1e3a05a 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -770,16 +770,16 @@ static int lxcDomainSetMemoryParameters(virDomainPtr dom, _("unable to set memory soft_limit tunable")); ret = -1; } - } else if (STREQ(param->field, VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT)) { + } else if (STREQ(param->field, VIR_DOMAIN_MEMORY_MEMSWAP_HARD_LIMIT)) { int rc; if (param->type != VIR_DOMAIN_MEMORY_PARAM_ULLONG) { lxcError(VIR_ERR_INVALID_ARG, "%s", - _("invalid type for swap_hard_limit tunable, expected a 'ullong'")); + _("invalid type for memswap_hard_limit tunable, expected a 'ullong'")); ret = -1; continue; } - rc = virCgroupSetSwapHardLimit(cgroup, params[i].value.ul); + rc = virCgroupSetMemSwapHardLimit(cgroup, params[i].value.ul); if (rc != 0) { virReportSystemError(-rc, "%s", _("unable to set swap_hard_limit tunable")); @@ -885,15 +885,15 @@ static int lxcDomainGetMemoryParameters(virDomainPtr dom, break; case 2: /* fill swap hard limit here */ - rc = virCgroupGetSwapHardLimit(cgroup, &val); + rc = virCgroupGetMemSwapHardLimit(cgroup, &val); if (rc != 0) { virReportSystemError(-rc, "%s", _("unable to get swap hard limit")); goto cleanup; } - 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")); goto cleanup; } param->value.ul = val; diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index e71d3fa..0bac1be 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -330,8 +330,9 @@ int qemuSetupCgroup(struct qemud_driver *driver, } } - if (vm->def->mem.swap_hard_limit != 0) { - rc = virCgroupSetSwapHardLimit(cgroup, vm->def->mem.swap_hard_limit); + if (vm->def->mem.memswap_hard_limit != 0) { + rc = virCgroupSetMemSwapHardLimit(cgroup, + vm->def->mem.memswap_hard_limit); if (rc != 0) { virReportSystemError(-rc, _("Unable to set swap hard limit for domain %s"), diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 0f7cbad..3897447 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4522,19 +4522,19 @@ static int qemuDomainSetMemoryParameters(virDomainPtr dom, _("unable to set memory soft_limit tunable")); ret = -1; } - } else if (STREQ(param->field, VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT)) { + } else if (STREQ(param->field, VIR_DOMAIN_MEMORY_MEMSWAP_HARD_LIMIT)) { int rc; if (param->type != VIR_DOMAIN_MEMORY_PARAM_ULLONG) { qemuReportError(VIR_ERR_INVALID_ARG, "%s", - _("invalid type for swap_hard_limit tunable, expected a 'ullong'")); + _("invalid type for memswap_hard_limit tunable, expected a 'ullong'")); ret = -1; continue; } - rc = virCgroupSetSwapHardLimit(group, params[i].value.ul); + rc = virCgroupSetMemSwapHardLimit(group, params[i].value.ul); if (rc != 0) { virReportSystemError(-rc, "%s", - _("unable to set swap_hard_limit tunable")); + _("unable to set memswap_hard_limit tunable")); ret = -1; } } else if (STREQ(param->field, VIR_DOMAIN_MEMORY_MIN_GUARANTEE)) { @@ -4642,15 +4642,15 @@ static int qemuDomainGetMemoryParameters(virDomainPtr dom, break; case 2: /* fill swap hard limit here */ - rc = virCgroupGetSwapHardLimit(group, &val); + rc = virCgroupGetMemSwapHardLimit(group, &val); if (rc != 0) { virReportSystemError(-rc, "%s", - _("unable to get swap hard limit")); + _("unable to get memory+swap hard limit")); goto cleanup; } - if (virStrcpyStatic(param->field, VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT) == NULL) { + if (virStrcpyStatic(param->field, VIR_DOMAIN_MEMORY_MEMSWAP_HARD_LIMIT) == NULL) { qemuReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("Field swap hard limit too long for destination")); + "%s", _("Field memory+swap hard limit too long for destination")); goto cleanup; } param->value.ul = val; diff --git a/src/util/cgroup.c b/src/util/cgroup.c index 8551acd..6e38ef5 100644 --- a/src/util/cgroup.c +++ b/src/util/cgroup.c @@ -1033,14 +1033,14 @@ int virCgroupGetMemorySoftLimit(virCgroupPtr group, unsigned long long *kb) } /** - * virCgroupSetSwapHardLimit: + * virCgroupSetMemSwapHardLimit: * - * @group: The cgroup to change swap hard limit for + * @group: The cgroup to change memory+swap hard limit for * @kb: The swap amount in kilobytes * * Returns: 0 on success */ -int virCgroupSetSwapHardLimit(virCgroupPtr group, unsigned long long kb) +int virCgroupSetMemSwapHardLimit(virCgroupPtr group, unsigned long long kb) { unsigned long long maxkb = VIR_DOMAIN_MEMORY_PARAM_UNLIMITED; @@ -1061,12 +1061,12 @@ int virCgroupSetSwapHardLimit(virCgroupPtr group, unsigned long long kb) /** * virCgroupGetSwapHardLimit: * - * @group: The cgroup to get swap hard limit for + * @group: The cgroup to get memory+swap hard limit for * @kb: The swap amount in kilobytes * * Returns: 0 on success */ -int virCgroupGetSwapHardLimit(virCgroupPtr group, unsigned long long *kb) +int virCgroupGetMemSwapHardLimit(virCgroupPtr group, unsigned long long *kb) { long long unsigned int limit_in_bytes; int ret; diff --git a/src/util/cgroup.h b/src/util/cgroup.h index d468cb3..fd16a40 100644 --- a/src/util/cgroup.h +++ b/src/util/cgroup.h @@ -52,8 +52,8 @@ int virCgroupSetMemoryHardLimit(virCgroupPtr group, unsigned long long kb); int virCgroupGetMemoryHardLimit(virCgroupPtr group, unsigned long long *kb); int virCgroupSetMemorySoftLimit(virCgroupPtr group, unsigned long long kb); int virCgroupGetMemorySoftLimit(virCgroupPtr group, unsigned long long *kb); -int virCgroupSetSwapHardLimit(virCgroupPtr group, unsigned long long kb); -int virCgroupGetSwapHardLimit(virCgroupPtr group, unsigned long long *kb); +int virCgroupSetMemSwapHardLimit(virCgroupPtr group, unsigned long long kb); +int virCgroupGetMemSwapHardLimit(virCgroupPtr group, unsigned long long *kb); int virCgroupDenyAllDevices(virCgroupPtr group); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-memtune.xml b/tests/qemuxml2argvdata/qemuxml2argv-memtune.xml index 37b5c88..e0a1825 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-memtune.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-memtune.xml @@ -6,7 +6,7 @@ <memtune> <hard_limit>512000</hard_limit> <soft_limit>128000</soft_limit> - <swap_hard_limit>1024000</swap_hard_limit> + <memswap_hard_limit>1024000</memswap_hard_limit> </memtune> <vcpu>1</vcpu> <os> diff --git a/tools/virsh.c b/tools/virsh.c index 14c6d6b..b790248 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -3034,8 +3034,8 @@ static const vshCmdOptDef opts_memtune[] = { N_("Max memory in kilobytes")}, {"soft-limit", VSH_OT_INT, VSH_OFLAG_NONE, N_("Memory during contention in kilobytes")}, - {"swap-hard-limit", VSH_OT_INT, VSH_OFLAG_NONE, - N_("Max swap in kilobytes")}, + {"memswap-hard-limit", VSH_OT_INT, VSH_OFLAG_NONE, + N_("Max memory+swap in kilobytes")}, {"min-guarantee", VSH_OT_INT, VSH_OFLAG_NONE, N_("Min guaranteed memory in kilobytes")}, {NULL, 0, 0, NULL} @@ -3045,7 +3045,7 @@ static int cmdMemtune(vshControl * ctl, const vshCmd * cmd) { virDomainPtr dom; - long long hard_limit = 0, soft_limit = 0, swap_hard_limit = 0; + long long hard_limit = 0, soft_limit = 0, memswap_hard_limit = 0; long long min_guarantee = 0; int nparams = 0; unsigned int i = 0; @@ -3060,7 +3060,7 @@ cmdMemtune(vshControl * ctl, const vshCmd * cmd) if (vshCommandOptLongLong(cmd, "hard-limit", &hard_limit) < 0 || vshCommandOptLongLong(cmd, "soft-limit", &soft_limit) < 0 || - vshCommandOptLongLong(cmd, "swap-hard-limit", &swap_hard_limit) < 0 || + vshCommandOptLongLong(cmd, "swap-hard-limit", &memswap_hard_limit) < 0 || vshCommandOptLongLong(cmd, "min-guarantee", &min_guarantee) < 0) { vshError(ctl, "%s", _("Unable to parse integer parameter")); @@ -3073,7 +3073,7 @@ cmdMemtune(vshControl * ctl, const vshCmd * cmd) if (soft_limit) nparams++; - if (swap_hard_limit) + if (memswap_hard_limit) nparams++; if (min_guarantee) @@ -3159,11 +3159,11 @@ cmdMemtune(vshControl * ctl, const vshCmd * cmd) strncpy(temp->field, VIR_DOMAIN_MEMORY_HARD_LIMIT, sizeof(temp->field)); hard_limit = 0; - } else if (swap_hard_limit) { - temp->value.ul = swap_hard_limit; - strncpy(temp->field, VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT, + } else if (memswap_hard_limit) { + temp->value.ul = memswap_hard_limit; + strncpy(temp->field, VIR_DOMAIN_MEMORY_MEMSWAP_HARD_LIMIT, sizeof(temp->field)); - swap_hard_limit = 0; + memswap_hard_limit = 0; } else if (min_guarantee) { temp->value.ul = min_guarantee; strncpy(temp->field, VIR_DOMAIN_MEMORY_MIN_GUARANTEE, -- 1.7.4.1

On Mon, Mar 14, 2011 at 09:24:58PM -0600, Eric Blake wrote: Adding back-compat hacks are only reasonable if we have already had an accident & mistakenly changed public facing XML/API. Given, that we're not in that situation, we should simply not make the proposed changes to the XML/API, rather than changing it & adding compat hacks. Agreed, we shouldn't change this either. Agreed, we should simply improve docs for the existing names to clarify their meaning. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On Wed, Mar 09, 2011 at 05:13:47PM +0900, KAMEZAWA Hiroyuki wrote: NACK to both these changes. The XML and public API must *never* be changed once included in a release. The current names may not be the perfect choices, but we can't change them now I'm afraid. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On Wed, 16 Mar 2011 09:37:32 +0900, KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com&gt; wrote: Ok, here is the patch with doc changes and cgroup api rename == From: Nikunj A. Dadhania <nikunj(a)linux.vnet.ibm.com&gt; * Correct the documentation for cgroup: the swap_hard_limit indicates mem+swap_hard_limit. * Change cgroup private apis to: virCgroupGet/SetMemSwapHardLimit Signed-off-by: Nikunj A. Dadhania <nikunj(a)linux.vnet.ibm.com&gt; --- docs/formatdomain.html.in | 5 +++-- include/libvirt/libvirt.h.in | 3 ++- src/libvirt_private.syms | 4 ++-- src/lxc/lxc_controller.c | 2 +- src/lxc/lxc_driver.c | 4 ++-- src/qemu/qemu_cgroup.c | 2 +- src/qemu/qemu_driver.c | 6 +++--- src/util/cgroup.c | 16 ++++++++-------- src/util/cgroup.h | 4 ++-- 9 files changed, 24 insertions(+), 22 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index dad268d..52bd4e6 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -231,8 +231,9 @@ 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 - (i.e. blocks of 1024 bytes)</dd> + memory plus swap the guest can use. The units for this value are + kilobytes (i.e. blocks of 1024 bytes). This has to be more than + hard_limit value provided</dd> <dt><code>min_guarantee</code></dt> <dd> The optional <code>min_guarantee</code> element is the guaranteed minimum memory allocation for the guest. The units for this value are diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 055eb2e..87bb9c5 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -729,7 +729,8 @@ typedef enum { * VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT: * * Macro for the swap tunable swap_hard_limit: it represents the maximum swap - * the guest can use. + * plus memory the guest can use. This limit has to be more than + * VIR_DOMAIN_MEMORY_HARD_LIMIT. */ #define VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT "swap_hard_limit" diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 2ce4bed..025988e 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -72,7 +72,7 @@ virCgroupGetFreezerState; virCgroupGetMemoryHardLimit; virCgroupGetMemorySoftLimit; virCgroupGetMemoryUsage; -virCgroupGetSwapHardLimit; +virCgroupGetMemSwapHardLimit; virCgroupMounted; virCgroupRemove; virCgroupSetCpuShares; @@ -80,7 +80,7 @@ virCgroupSetFreezerState; virCgroupSetMemory; virCgroupSetMemoryHardLimit; virCgroupSetMemorySoftLimit; -virCgroupSetSwapHardLimit; +virCgroupSetMemSwapHardLimit; # command.h diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index af0b70c..cfb0356 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -135,7 +135,7 @@ static int lxcSetContainerResources(virDomainDefPtr def) } if(def->mem.swap_hard_limit) { - rc = virCgroupSetSwapHardLimit(cgroup, def->mem.swap_hard_limit); + rc = virCgroupSetMemSwapHardLimit(cgroup, def->mem.swap_hard_limit); if (rc != 0) { virReportSystemError(-rc, _("Unable to set swap hard limit for domain %s"), diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index c4fe936..0eca48e 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -780,7 +780,7 @@ static int lxcDomainSetMemoryParameters(virDomainPtr dom, continue; } - rc = virCgroupSetSwapHardLimit(cgroup, params[i].value.ul); + rc = virCgroupSetMemSwapHardLimit(cgroup, params[i].value.ul); if (rc != 0) { virReportSystemError(-rc, "%s", _("unable to set swap_hard_limit tunable")); @@ -886,7 +886,7 @@ static int lxcDomainGetMemoryParameters(virDomainPtr dom, break; case 2: /* fill swap hard limit here */ - rc = virCgroupGetSwapHardLimit(cgroup, &val); + rc = virCgroupGetMemSwapHardLimit(cgroup, &val); if (rc != 0) { virReportSystemError(-rc, "%s", _("unable to get swap hard limit")); diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index e5536c0..a25b486 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -291,7 +291,7 @@ int qemuSetupCgroup(struct qemud_driver *driver, } if (vm->def->mem.swap_hard_limit != 0) { - rc = virCgroupSetSwapHardLimit(cgroup, vm->def->mem.swap_hard_limit); + rc = virCgroupSetMemSwapHardLimit(cgroup, vm->def->mem.swap_hard_limit); if (rc != 0) { virReportSystemError(-rc, _("Unable to set swap hard limit for domain %s"), diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 395f72f..9b5fdec 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7002,7 +7002,7 @@ static int qemuDomainSetMemoryParameters(virDomainPtr dom, continue; } - rc = virCgroupSetSwapHardLimit(group, params[i].value.ul); + rc = virCgroupSetMemSwapHardLimit(group, params[i].value.ul); if (rc != 0) { virReportSystemError(-rc, "%s", _("unable to set swap_hard_limit tunable")); @@ -7112,8 +7112,8 @@ static int qemuDomainGetMemoryParameters(virDomainPtr dom, param->value.ul = val; break; - case 2: /* fill swap hard limit here */ - rc = virCgroupGetSwapHardLimit(group, &val); + case 2: /* fill mem+swap hard limit here */ + rc = virCgroupGetMemSwapHardLimit(group, &val); if (rc != 0) { virReportSystemError(-rc, "%s", _("unable to get swap hard limit")); diff --git a/src/util/cgroup.c b/src/util/cgroup.c index cd9caba..71c928e 100644 --- a/src/util/cgroup.c +++ b/src/util/cgroup.c @@ -977,14 +977,14 @@ int virCgroupGetMemorySoftLimit(virCgroupPtr group, unsigned long long *kb) } /** - * virCgroupSetSwapHardLimit: + * virCgroupSetMemSwapHardLimit: * - * @group: The cgroup to change swap hard limit for - * @kb: The swap amount in kilobytes + * @group: The cgroup to change mem+swap hard limit for + * @kb: The mem+swap amount in kilobytes * * Returns: 0 on success */ -int virCgroupSetSwapHardLimit(virCgroupPtr group, unsigned long long kb) +int virCgroupSetMemSwapHardLimit(virCgroupPtr group, unsigned long long kb) { unsigned long long maxkb = VIR_DOMAIN_MEMORY_PARAM_UNLIMITED; @@ -1003,14 +1003,14 @@ int virCgroupSetSwapHardLimit(virCgroupPtr group, unsigned long long kb) } /** - * virCgroupGetSwapHardLimit: + * virCgroupGetMemSwapHardLimit: * - * @group: The cgroup to get swap hard limit for - * @kb: The swap amount in kilobytes + * @group: The cgroup to get mem+swap hard limit for + * @kb: The mem+swap amount in kilobytes * * Returns: 0 on success */ -int virCgroupGetSwapHardLimit(virCgroupPtr group, unsigned long long *kb) +int virCgroupGetMemSwapHardLimit(virCgroupPtr group, unsigned long long *kb) { long long unsigned int limit_in_bytes; int ret; diff --git a/src/util/cgroup.h b/src/util/cgroup.h index 964da7a..8333b1b 100644 --- a/src/util/cgroup.h +++ b/src/util/cgroup.h @@ -47,8 +47,8 @@ int virCgroupSetMemoryHardLimit(virCgroupPtr group, unsigned long long kb); int virCgroupGetMemoryHardLimit(virCgroupPtr group, unsigned long long *kb); int virCgroupSetMemorySoftLimit(virCgroupPtr group, unsigned long long kb); int virCgroupGetMemorySoftLimit(virCgroupPtr group, unsigned long long *kb); -int virCgroupSetSwapHardLimit(virCgroupPtr group, unsigned long long kb); -int virCgroupGetSwapHardLimit(virCgroupPtr group, unsigned long long *kb); +int virCgroupSetMemSwapHardLimit(virCgroupPtr group, unsigned long long kb); +int virCgroupGetMemSwapHardLimit(virCgroupPtr group, unsigned long long *kb); int virCgroupDenyAllDevices(virCgroupPtr group);

On 03/09/2011 01:13 AM, KAMEZAWA Hiroyuki wrote: For the record - patching this sentence to mention that <code>swap_hard_limit</code> covers memory plus swap is reasonable, as is updating this documentation sentence (but not renaming the macro). I'd rather see 'memory plus swap' than 'memory+swap' in documentation. 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). 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(a)redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org