Re: [libvirt] [PATCH] virsh: fix memtune's help message for swap_hard_limit

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