From ab95da40587665063b0c8eaeae22c91003f0da1c Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Wed, 7 Mar 2012 18:10:30 -0700 Subject: [PATCH] virsh: improve storage unit parsing Now can now do: virsh vol-resize $vol 10M virsh blockresize $dom $vol 10M to get both interfaces to resize to 10MiB. The remaining wart is that vol-resize defaults to bytes, but blockresize defaults to KiB, but we can't break existing scripts; oh well, it's no worse than the same wart of the underlying virDomainBlockResize. The API for virStorageVolResize states that capacity must always be positive, and that the presence of shrink and delta flags is what implies a negative change. * tools/virsh.c (vshCommandOptScaledInt): New function. (cmdVolResize): Don't pass negative size. (cmdVolSize): Rename... (vshVolSize): ...and use new helper routine. (cmdBlockResize): Use new helper routine, and support new bytes flag. * tools/virsh.pod (NOTES): Document suffixes. (blockresize, vol-create-as, vol-resize): Point to notes. --- tools/virsh.c | 123 ++++++++++++++++++++++++++++-------------------- tools/virsh.pod | 45 ++++++++++++++---- 2 files changed, 109 insertions(+), 59 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index eb0eff489c..c966904a5f 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -313,6 +313,10 @@ static int vshCommandOptLongLong(const vshCmd *cmd, const char *name, static int vshCommandOptULongLong(const vshCmd *cmd, const char *name, unsigned long long *value) ATTRIBUTE_NONNULL(3) ATTRIBUTE_RETURN_CHECK; +static int vshCommandOptScaledInt(const vshCmd *cmd, const char *name, + unsigned long long *value, int scale, + unsigned long long max) + ATTRIBUTE_NONNULL(3) ATTRIBUTE_RETURN_CHECK; static bool vshCommandOptBool(const vshCmd *cmd, const char *name); static const vshCmdOpt *vshCommandOptArgv(const vshCmd *cmd, const vshCmdOpt *opt); @@ -7625,9 +7629,10 @@ static const vshCmdInfo info_block_resize[] = { static const vshCmdOptDef opts_block_resize[] = { {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, - {"path", VSH_OT_DATA, VSH_OFLAG_REQ, N_("Fully-qualified path of block device")}, - {"size", VSH_OT_INT, VSH_OFLAG_REQ, N_("New size of the block device in kilobytes, " - "the size must be integer")}, + {"path", VSH_OT_DATA, VSH_OFLAG_REQ, + N_("Fully-qualified path of block device")}, + {"size", VSH_OT_INT, VSH_OFLAG_REQ, + N_("New size of the block device, as scaled integer (default KiB)")}, {NULL, 0, 0, NULL} }; @@ -7648,15 +7653,16 @@ cmdBlockResize(vshControl *ctl, const vshCmd *cmd) return false; } - if (vshCommandOptULongLong(cmd, "size", &size) < 0) { + if (vshCommandOptScaledInt(cmd, "size", &size, 1024, ULLONG_MAX) < 0) { vshError(ctl, "%s", _("Unable to parse integer")); return false; } - if (size > ULLONG_MAX / 1024) { - vshError(ctl, _("Size must be less than %llu"), ULLONG_MAX / 1024); - return false; - } + /* Prefer the older interface of KiB. */ + if (size % 1024 == 0) + size /= 1024; + else + flags |= VIR_DOMAIN_BLOCK_RESIZE_BYTES; if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) return false; @@ -11046,43 +11052,26 @@ static const vshCmdInfo info_vol_create_as[] = { static const vshCmdOptDef opts_vol_create_as[] = { {"pool", VSH_OT_DATA, VSH_OFLAG_REQ, N_("pool name")}, {"name", VSH_OT_DATA, VSH_OFLAG_REQ, N_("name of the volume")}, - {"capacity", VSH_OT_DATA, VSH_OFLAG_REQ, N_("size of the vol with optional k,M,G,T suffix")}, - {"allocation", VSH_OT_STRING, 0, N_("initial allocation size with optional k,M,G,T suffix")}, - {"format", VSH_OT_STRING, 0, N_("file format type raw,bochs,qcow,qcow2,vmdk")}, - {"backing-vol", VSH_OT_STRING, 0, N_("the backing volume if taking a snapshot")}, - {"backing-vol-format", VSH_OT_STRING, 0, N_("format of backing volume if taking a snapshot")}, + {"capacity", VSH_OT_DATA, VSH_OFLAG_REQ, + N_("size of the vol, as scaled integer (default bytes)")}, + {"allocation", VSH_OT_STRING, 0, + N_("initial allocation size, as scaled integer (default bytes)")}, + {"format", VSH_OT_STRING, 0, + N_("file format type raw,bochs,qcow,qcow2,vmdk")}, + {"backing-vol", VSH_OT_STRING, 0, + N_("the backing volume if taking a snapshot")}, + {"backing-vol-format", VSH_OT_STRING, 0, + N_("format of backing volume if taking a snapshot")}, {NULL, 0, 0, NULL} }; -static int cmdVolSize(const char *data, unsigned long long *val) +static int +vshVolSize(const char *data, unsigned long long *val) { char *end; if (virStrToLong_ull(data, &end, 10, val) < 0) return -1; - - if (end && *end) { - /* Deliberate fallthrough cases here :-) */ - switch (*end) { - case 'T': - *val *= 1024; - /* fallthrough */ - case 'G': - *val *= 1024; - /* fallthrough */ - case 'M': - *val *= 1024; - /* fallthrough */ - case 'k': - *val *= 1024; - break; - default: - return -1; - } - end++; - if (*end) - return -1; - } - return 0; + return virScaleInteger(val, end, 1, ULLONG_MAX); } static bool @@ -11108,11 +11097,11 @@ cmdVolCreateAs(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptString(cmd, "capacity", &capacityStr) <= 0) goto cleanup; - if (cmdVolSize(capacityStr, &capacity) < 0) + if (vshVolSize(capacityStr, &capacity) < 0) vshError(ctl, _("Malformed size %s"), capacityStr); if ((vshCommandOptString(cmd, "allocation", &allocationStr) > 0) && - (cmdVolSize(allocationStr, &allocation) < 0)) + (vshVolSize(allocationStr, &allocation) < 0)) vshError(ctl, _("Malformed size %s"), allocationStr); if (vshCommandOptString(cmd, "format", &format) < 0 || @@ -11908,7 +11897,7 @@ static const vshCmdInfo info_vol_resize[] = { static const vshCmdOptDef opts_vol_resize[] = { {"vol", VSH_OT_DATA, VSH_OFLAG_REQ, N_("vol name, key or path")}, {"capacity", VSH_OT_DATA, VSH_OFLAG_REQ, - N_("new capacity for the vol with optional k,M,G,T suffix")}, + N_("new capacity for the vol, as scaled integer (default bytes)")}, {"pool", VSH_OT_STRING, 0, N_("pool name or uuid")}, {"allocate", VSH_OT_BOOL, 0, N_("allocate the new capacity, rather than leaving it sparse")}, @@ -11945,18 +11934,22 @@ cmdVolResize(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptString(cmd, "capacity", &capacityStr) <= 0) goto cleanup; - if (delta && *capacityStr == '-') { - if (cmdVolSize(capacityStr + 1, &capacity) < 0) { - vshError(ctl, _("Malformed size %s"), capacityStr); - goto cleanup; - } - capacity = -capacity; - } else { - if (cmdVolSize(capacityStr, &capacity) < 0) { - vshError(ctl, _("Malformed size %s"), capacityStr); + virSkipSpaces(&capacityStr); + if (*capacityStr == '-') { + /* The API always requires a positive value; but we allow a + * negative value for convenience. */ + if (delta && vshCommandOptBool(cmd, "shrink")){ + capacityStr++; + } else { + vshError(ctl, "%s", + _("negative size requires --delta and --shrink")); goto cleanup; } } + if (vshVolSize(capacityStr, &capacity) < 0) { + vshError(ctl, _("Malformed size %s"), capacityStr); + goto cleanup; + } if (virStorageVolResize(vol, capacity, flags) == 0) { vshPrint(ctl, @@ -18005,6 +17998,36 @@ vshCommandOptULongLong(const vshCmd *cmd, const char *name, } +/** + * vshCommandOptScaledInt: + * @cmd command reference + * @name option name + * @value result + * @scale default of 1 or 1024, if no suffix is present + * @max maximum value permitted + * + * Returns option as long long, scaled according to suffix + * See vshCommandOptInt() + */ +static int +vshCommandOptScaledInt(const vshCmd *cmd, const char *name, + unsigned long long *value, int scale, + unsigned long long max) +{ + const char *str; + int ret; + char *end; + + ret = vshCommandOptString(cmd, name, &str); + if (ret <= 0) + return ret; + if (virStrToLong_ull(str, &end, 10, value) < 0 || + virScaleInteger(value, end, scale, max) < 0) + return -1; + return 1; +} + + /** * vshCommandOptBool: * @cmd command reference diff --git a/tools/virsh.pod b/tools/virsh.pod index ea7db5f775..2149d22655 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -121,6 +121,25 @@ version of B supported an alternate spelling of a command or option (such as I<--tunnelled> instead of I<--tunneled>), then scripts using that older spelling will continue to work. +Several B commands take an optionally scaled integer; if no +scale is provided, then the default is listed in the command (for +historical reasons, some commands default to bytes, while other +commands default to kibibytes). The following case-insensitive +suffixes can be used to select a specfic scale: + b, byte byte 1 + KB kilobyte 1,000 + k, KiB kibibyte 1,024 + MB megabyte 1,000,000 + M, MiB mebibyte 1,048,576 + GB gigabyte 1,000,000,000 + G, GiB gibibyte 1,073,741,824 + TB terabyte 1,000,000,000,000 + T, TiB tebibyte 1,099,511,627,776 + PB petabyte 1,000,000,000,000,000 + P, PiB pebibyte 1,125,899,906,842,624 + EB exabyte 1,000,000,000,000,000,000 + E, EiB exbibyte 1,152,921,504,606,846,976 + =head1 GENERIC COMMANDS The following commands are generic i.e. not specific to a domain. @@ -663,11 +682,14 @@ If I<--info> is specified, the active job information on the specified disk will be printed. I can be used to set bandwidth limit for the active job. -=item B I I<--path> I<--size> +=item B I I I -Resize a block device of domain while the domain is running, I<--path> -specifies the absolute path of the block device, I<--size> specifies the -new size in kilobytes +Resize a block device of domain while the domain is running, I +specifies the absolute path of the block device, I is a scaled +integer (see B above) which defaults to KiB (blocks of 1024 bytes) +if there is no suffix. You must use a suffix of "B" to get bytes (note +that for historical reasons, this differs from B which +defaults to bytes without a suffix). =item B I @@ -2024,10 +2046,10 @@ Create a volume from a set of arguments. I is the name or UUID of the storage pool to create the volume in. I is the name of the new volume. -I is the size of the volume to be created, with optional k, M, G, or -T suffix. -I<--allocation> I is the initial size to be allocated in the volume, with -optional k, M, G, or T suffix. +I is the size of the volume to be created, as a scaled integer +(see B above), defaulting to bytes if there is no suffix. +I<--allocation> I is the initial size to be allocated in the volume, +also as a scaled integer defaulting to bytes. I<--format> I is used in file based storage pools to specify the volume file format to use; raw, bochs, qcow, qcow2, vmdk. I<--backing-vol> I is the source backing @@ -2163,7 +2185,12 @@ is in. I is the name or key or path of the volume to resize. The new capacity might be sparse unless I<--allocate> is specified. Normally, I is the new size, but if I<--delta> is present, then it is added to the existing size. Attempts to shrink -the volume will fail unless I<--shrink> is present. +the volume will fail unless I<--shrink> is present; I cannot +be negative unless I<--shrink> is provided, but a negative sign is not +necessary. I is a scaled integer (see B above), which +defaults to bytes if there is no suffix. This command is only safe +for storage volumes not in use by an active guest; see also +B for live resizing. =back