[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



On Mon, 07 Mar 2011 15:11:12 +0530
"Nikunj A. Dadhania" <nikunj linux vnet ibm com> wrote:

> Argh, thats me... :(
> Thanks for the patch Kame.
> 
> Reviewed-by: "Nikunj A. Dadhania" <nikunj linux vnet ibm com>
> 
Thanks, rebased onto the latest git tree.
==
>From 421a27337458648b808f0e45cdfd83192313d7cc Mon Sep 17 00:00:00 2001
From: KAMEZAWA Hiroyuki <kamezawa hiroyu jp fujitsu com>
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 jp fujitsu com>
Reviewed-by: "Nikunj A. Dadhania" <nikunj linux vnet ibm com>
---
 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



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]