From 210ed0e871c628cc696bcf7cc0531b8b8dfcd533 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Mon, 5 Mar 2012 17:06:57 -0700 Subject: [PATCH] virsh: improve memory unit parsing 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. (vshMemtuneGetSize): New helper. (cmdMemtune): Use it. * tools/virsh.pod (setmem, setmaxmem, memtune): Document this. --- tools/virsh.c | 110 +++++++++++++++++++++++++++++------------------- tools/virsh.pod | 50 +++++++++++----------- 2 files changed, 91 insertions(+), 69 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index c966904a5f..ee8ff310da 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -5822,7 +5822,7 @@ cleanup: } /* - * "setmemory" command + * "setmem" command */ static const vshCmdInfo info_setmem[] = { {"help", N_("change memory allocation")}, @@ -5832,7 +5832,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")}, @@ -5843,8 +5845,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"); @@ -5873,36 +5876,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; } } @@ -5922,7 +5914,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")}, @@ -5933,7 +5927,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"); @@ -5961,24 +5957,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; } @@ -6151,21 +6149,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 +vshMemtuneGetSize(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; @@ -6198,10 +6220,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 (vshMemtuneGetSize(cmd, "hard-limit", &hard_limit) < 0 || + vshMemtuneGetSize(cmd, "soft-limit", &soft_limit) < 0 || + vshMemtuneGetSize(cmd, "swap-hard-limit", &swap_hard_limit) < 0 || + vshMemtuneGetSize(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 2149d22655..b4b8e85c66 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1170,7 +1170,7 @@ B # send a tab, held for 1 second virsh send-key --holdtime 1000 0xf -=item B I B [[I<--config>] [I<--live>] | +=item B I B [[I<--config>] [I<--live>] | [I<--current>]] Change the memory allocation for a guest domain. @@ -1181,15 +1181,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 -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). +I is a scaled integer (see B above); it defaults to kibibytes +(blocks of 1024 bytes) 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). 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 I B [[I<--config>] [I<--live>] | +=item B I B [[I<--config>] [I<--live>] | [I<--current>]] Change the maximum memory allocation limit for a guest domain. @@ -1202,22 +1204,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 is a scaled integer (see B above); it defaults to kibibytes +(blocks of 1024 bytes) 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 I [I<--hard-limit> B] -[I<--soft-limit> B] [I<--swap-hard-limit> B] -[I<--min-guarantee> B] [[I<--config>] [I<--live>] | [I<--current>]] +=item B I [I<--hard-limit> B] +[I<--soft-limit> B] [I<--swap-hard-limit> B] +[I<--min-guarantee> B] [[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 above), with a default of +kibibytes (blocks of 1024 bytes) 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. @@ -1235,24 +1239,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