[PATCHv2 15/15] virsh: improve memory unit parsing

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

 



The last vestige of the inaccurate 'kilobytes' when we meant 1024 is
now gone.  And virsh is now useful for setting memory in units other
than KiB.

* tools/virsh.c (cmdSetmem, cmdSetmaxmem): Use new helper routine,
allow passing bogus arguments on to hypervisor to test driver
sanity checking, and fix leak on parse error.
(cmdMemtuneGetSize): New helper.
(cmdMemtune): Use it.
* tools/virsh.pod (setmem, setmaxmem, memtune): Document this.
---

v2: new

 tools/virsh.c   |  110 +++++++++++++++++++++++++++++++++----------------------
 tools/virsh.pod |   48 ++++++++++++------------
 2 files changed, 90 insertions(+), 68 deletions(-)

diff --git a/tools/virsh.c b/tools/virsh.c
index d5cc46b..b93706f 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -5668,7 +5668,7 @@ cleanup:
 }

 /*
- * "setmemory" command
+ * "setmem" command
  */
 static const vshCmdInfo info_setmem[] = {
     {"help", N_("change memory allocation")},
@@ -5678,7 +5678,9 @@ static const vshCmdInfo info_setmem[] = {

 static const vshCmdOptDef opts_setmem[] = {
     {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")},
-    {"kilobytes", VSH_OT_INT, VSH_OFLAG_REQ, N_("number of kilobytes of memory")},
+    {"kilobytes", VSH_OT_ALIAS, 0, "size"},
+    {"size", VSH_OT_INT, VSH_OFLAG_REQ,
+     N_("new memory size, as scaled integer (default KiB)")},
     {"config", VSH_OT_BOOL, 0, N_("affect next boot")},
     {"live", VSH_OT_BOOL, 0, N_("affect running domain")},
     {"current", VSH_OT_BOOL, 0, N_("affect current domain")},
@@ -5689,8 +5691,9 @@ static bool
 cmdSetmem(vshControl *ctl, const vshCmd *cmd)
 {
     virDomainPtr dom;
-    virDomainInfo info;
-    unsigned long kilobytes = 0;
+    unsigned long long bytes = 0;
+    unsigned long long max;
+    unsigned long kibibytes = 0;
     bool ret = true;
     int config = vshCommandOptBool(cmd, "config");
     int live = vshCommandOptBool(cmd, "live");
@@ -5719,36 +5722,25 @@ cmdSetmem(vshControl *ctl, const vshCmd *cmd)
     if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
         return false;

-    if (vshCommandOptUL(cmd, "kilobytes", &kilobytes) < 0) {
+    /* The API expects 'unsigned long' KiB, so depending on whether we
+     * are 32-bit or 64-bit determines the maximum we can use.  */
+    if (sizeof(kibibytes) < sizeof(max))
+        max = 1024ull * ULONG_MAX;
+    else
+        max = ULONG_MAX;
+    if (vshCommandOptScaledInt(cmd, "size", &bytes, 1024, max) < 0) {
         vshError(ctl, "%s", _("memory size has to be a number"));
-        return false;
-    }
-
-    if (kilobytes <= 0) {
-        virDomainFree(dom);
-        vshError(ctl, _("Invalid value of %lu for memory size"), kilobytes);
-        return false;
-    }
-
-    if (virDomainGetInfo(dom, &info) != 0) {
         virDomainFree(dom);
-        vshError(ctl, "%s", _("Unable to verify MaxMemorySize"));
-        return false;
-    }
-
-    if (kilobytes > info.maxMem) {
-        virDomainFree(dom);
-        vshError(ctl, _("Requested memory size %lu kb is larger than maximum of %lu kb"),
-                 kilobytes, info.maxMem);
         return false;
     }
+    kibibytes = VIR_DIV_UP(bytes, 1024);

     if (flags == -1) {
-        if (virDomainSetMemory(dom, kilobytes) != 0) {
+        if (virDomainSetMemory(dom, kibibytes) != 0) {
             ret = false;
         }
     } else {
-        if (virDomainSetMemoryFlags(dom, kilobytes, flags) < 0) {
+        if (virDomainSetMemoryFlags(dom, kibibytes, flags) < 0) {
             ret = false;
         }
     }
@@ -5768,7 +5760,9 @@ static const vshCmdInfo info_setmaxmem[] = {

 static const vshCmdOptDef opts_setmaxmem[] = {
     {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")},
-    {"kilobytes", VSH_OT_INT, VSH_OFLAG_REQ, N_("maximum memory limit in kilobytes")},
+    {"kilobytes", VSH_OT_ALIAS, 0, "size"},
+    {"size", VSH_OT_INT, VSH_OFLAG_REQ,
+     N_("new maximum memory size, as scaled integer (default KiB)")},
     {"config", VSH_OT_BOOL, 0, N_("affect next boot")},
     {"live", VSH_OT_BOOL, 0, N_("affect running domain")},
     {"current", VSH_OT_BOOL, 0, N_("affect current domain")},
@@ -5779,7 +5773,9 @@ static bool
 cmdSetmaxmem(vshControl *ctl, const vshCmd *cmd)
 {
     virDomainPtr dom;
-    int kilobytes = 0;
+    unsigned long long bytes = 0;
+    unsigned long long max;
+    unsigned long kibibytes = 0;
     bool ret = true;
     int config = vshCommandOptBool(cmd, "config");
     int live = vshCommandOptBool(cmd, "live");
@@ -5807,24 +5803,26 @@ cmdSetmaxmem(vshControl *ctl, const vshCmd *cmd)
     if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
         return false;

-    if (vshCommandOptInt(cmd, "kilobytes", &kilobytes) < 0) {
+    /* The API expects 'unsigned long' KiB, so depending on whether we
+     * are 32-bit or 64-bit determines the maximum we can use.  */
+    if (sizeof(kibibytes) < sizeof(max))
+        max = 1024ull * ULONG_MAX;
+    else
+        max = ULONG_MAX;
+    if (vshCommandOptScaledInt(cmd, "size", &bytes, 1024, max) < 0) {
         vshError(ctl, "%s", _("memory size has to be a number"));
-        return false;
-    }
-
-    if (kilobytes <= 0) {
         virDomainFree(dom);
-        vshError(ctl, _("Invalid value of %d for memory size"), kilobytes);
         return false;
     }
+    kibibytes = VIR_DIV_UP(bytes, 1024);

     if (flags == -1) {
-        if (virDomainSetMaxMemory(dom, kilobytes) != 0) {
+        if (virDomainSetMaxMemory(dom, kibibytes) != 0) {
             vshError(ctl, "%s", _("Unable to change MaxMemorySize"));
             ret = false;
         }
     } else {
-        if (virDomainSetMemoryFlags(dom, kilobytes, flags) < 0) {
+        if (virDomainSetMemoryFlags(dom, kibibytes, flags) < 0) {
             vshError(ctl, "%s", _("Unable to change MaxMemorySize"));
             ret = false;
         }
@@ -5997,21 +5995,45 @@ static const vshCmdInfo info_memtune[] = {
 static const vshCmdOptDef opts_memtune[] = {
     {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")},
     {"hard-limit", VSH_OT_INT, VSH_OFLAG_NONE,
-     N_("Max memory in kilobytes")},
+     N_("Max memory, as scaled integer (default KiB)")},
     {"soft-limit", VSH_OT_INT, VSH_OFLAG_NONE,
-     N_("Memory during contention in kilobytes")},
+     N_("Memory during contention, as scaled integer (default KiB)")},
     {"swap-hard-limit", VSH_OT_INT, VSH_OFLAG_NONE,
-     N_("Max memory plus swap in kilobytes")},
+     N_("Max memory plus swap, as scaled integer (default KiB)")},
     {"min-guarantee", VSH_OT_INT, VSH_OFLAG_NONE,
-     N_("Min guaranteed memory in kilobytes")},
+     N_("Min guaranteed memory, as scaled integer (default KiB)")},
     {"config", VSH_OT_BOOL, 0, N_("affect next boot")},
     {"live", VSH_OT_BOOL, 0, N_("affect running domain")},
     {"current", VSH_OT_BOOL, 0, N_("affect current domain")},
     {NULL, 0, 0, NULL}
 };

+static int
+cmdMemtuneGetSize(const vshCmd *cmd, const char *name, long long *value)
+{
+    int ret;
+    unsigned long long tmp;
+    const char *str;
+    char *end;
+
+    ret = vshCommandOptString(cmd, name, &str);
+    if (ret <= 0)
+        return ret;
+    if (virStrToLong_ll(str, &end, 10, value) < 0)
+        return -1;
+    if (*value < 0) {
+        *value = VIR_DOMAIN_MEMORY_PARAM_UNLIMITED;
+        return 1;
+    }
+    tmp = *value;
+    if (virScaleInteger(&tmp, end, 1024, LLONG_MAX) < 0)
+        return -1;
+    *value = VIR_DIV_UP(tmp, 1024);
+    return 0;
+}
+
 static bool
-cmdMemtune(vshControl * ctl, const vshCmd * cmd)
+cmdMemtune(vshControl *ctl, const vshCmd *cmd)
 {
     virDomainPtr dom;
     long long hard_limit = 0, soft_limit = 0, swap_hard_limit = 0;
@@ -6044,10 +6066,10 @@ cmdMemtune(vshControl * ctl, const vshCmd * cmd)
     if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
         return false;

-    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, "min-guarantee", &min_guarantee) < 0) {
+    if (cmdMemtuneGetSize(cmd, "hard-limit", &hard_limit) < 0 ||
+        cmdMemtuneGetSize(cmd, "soft-limit", &soft_limit) < 0 ||
+        cmdMemtuneGetSize(cmd, "swap-hard-limit", &swap_hard_limit) < 0 ||
+        cmdMemtuneGetSize(cmd, "min-guarantee", &min_guarantee) < 0) {
         vshError(ctl, "%s",
                  _("Unable to parse integer parameter"));
         goto cleanup;
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 74d3ff5..a13a3b2 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -1153,7 +1153,7 @@ B<Examples>
   # send a tab, held for 1 second
   virsh send-key --holdtime 1000 0xf

-=item B<setmem> I<domain-id> B<kilobytes> [[I<--config>] [I<--live>] |
+=item B<setmem> I<domain-id> B<size> [[I<--config>] [I<--live>] |
 [I<--current>]]

 Change the memory allocation for a guest domain.
@@ -1164,15 +1164,17 @@ Both I<--live> and I<--config> flags may be given, but I<--current> is
 exclusive. If no flag is specified, behavior is different depending
 on hypervisor.

-Some hypervisors require a larger granularity than kilobytes, and requests
+I<size> is a scaled integer (see B<NOTES> above); it defaults to kibibytes
+(1024) unless you provide a suffix (and the older option name I<--kilobytes>
+is available as a deprecated synonym) .  Libvirt rounds up to the nearest
+kibibyte.  Some hypervisors require a larger granularity than KiB, and requests
 that are not an even multiple will be rounded up.  For example, vSphere/ESX
-rounds the parameter up unless the kB argument is evenly divisible by 1024
-(that is, the kB argument happens to represent megabytes).
+rounds the parameter up to mebibytes (1024 kibibytes).

 For Xen, you can only adjust the memory of a running domain if the domain is
 paravirtualized or running the PV balloon driver.

-=item B<setmaxmem> I<domain-id> B<kilobytes> [[I<--config>] [I<--live>] |
+=item B<setmaxmem> I<domain-id> B<size> [[I<--config>] [I<--live>] |
 [I<--current>]]

 Change the maximum memory allocation limit for a guest domain.
@@ -1185,22 +1187,24 @@ on hypervisor.

 This command works for at least the Xen, QEMU/KVM and vSphere/ESX hypervisors.

-Some hypervisors require a larger granularity than kilobytes, rounding up
-requests that are not an even multiple of the desired amount.  vSphere/ESX
-is one of these, requiring the parameter to be evenly divisible by 4MB.  For
-vSphere/ESX, 263168 (257MB) would be rounded up because it's not a multiple
-of 4MB, while 266240 (260MB) is valid without rounding.
-
+I<size> is a scaled integer (see B<NOTES> above); it defaults to kibibytes
+(1024) unless you provide a suffix (and the older option name I<--kilobytes>
+is available as a deprecated synonym) .  Libvirt rounds up to the nearest
+kibibyte.  Some hypervisors require a larger granularity than KiB, and requests
+that are not an even multiple will be rounded up.  For example, vSphere/ESX
+rounds the parameter up to mebibytes (1024 kibibytes).

-=item B<memtune> I<domain-id> [I<--hard-limit> B<kilobytes>]
-[I<--soft-limit> B<kilobytes>] [I<--swap-hard-limit> B<kilobytes>]
-[I<--min-guarantee> B<kilobytes>] [[I<--config>] [I<--live>] | [I<--current>]]
+=item B<memtune> I<domain-id> [I<--hard-limit> B<size>]
+[I<--soft-limit> B<size>] [I<--swap-hard-limit> B<size>]
+[I<--min-guarantee> B<size>] [[I<--config>] [I<--live>] | [I<--current>]]

 Allows you to display or set the domain memory parameters. Without
 flags, the current settings are displayed; with a flag, the
 appropriate limit is adjusted if supported by the hypervisor.  LXC and
 QEMU/KVM support I<--hard-limit>, I<--soft-limit>, and I<--swap-hard-limit>.
-I<--min-guarantee> is supported only by ESX hypervisor.
+I<--min-guarantee> is supported only by ESX hypervisor.  Each of these
+limits are scaled integers (see B<NOTES> above), with a default of
+kibibytes (blocks of 1024) if no suffix is present.

 If I<--live> is specified, affect a running guest.
 If I<--config> is specified, affect the next boot of a persistent guest.
@@ -1218,24 +1222,20 @@ one needs guess and try.

 =item I<--hard-limit>

-The maximum memory the guest can use.  The units for this value are kilobytes
-(i.e. blocks of 1024 bytes).
+The maximum memory the guest can use.

 =item I<--soft-limit>

-The memory limit to enforce during memory contention.  The units for this
-value are kilobytes (i.e. blocks of 1024 bytes).
+The memory limit to enforce during memory contention.

 =item I<--swap-hard-limit>

-The maximum 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.
+The maximum memory plus swap the guest can use.  This has to be more
+than hard-limit value provided.

 =item I<--min-guarantee>

-The guaranteed minimum memory allocation for the guest.  The units for this
-value are kilobytes (i.e. blocks of 1024 bytes).
+The guaranteed minimum memory allocation for the guest.

 =back

-- 
1.7.7.6

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]