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

@@ -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

tools/virsh.pod

@@ -121,6 +121,25 @@ version of B<virsh> 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<virsh> 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<bandwidth> can be used to set bandwidth limit for the active job.
 
-=item B<blockresize> I<domain> I<--path> I<--size>
+=item B<blockresize> I<domain> I<path> I<size>
 
-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<path>
+specifies the absolute path of the block device, I<size> is a scaled
+integer (see B<NOTES> 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<vol-resize> which
+defaults to bytes without a suffix).
 
 =item B<dominfo> I<domain-id>
 
@@ -2024,10 +2046,10 @@ Create a volume from a set of arguments.
 I<pool-or-uuid> is the name or UUID of the storage pool to create the volume
 in.
 I<name> is the name of the new volume.
-I<capacity> is the size of the volume to be created, with optional k, M, G, or
-T suffix.
-I<--allocation> I<size> is the initial size to be allocated in the volume, with
-optional k, M, G, or T suffix.
+I<capacity> is the size of the volume to be created, as a scaled integer
+(see B<NOTES> above), defaulting to bytes if there is no suffix.
+I<--allocation> I<size> is the initial size to be allocated in the volume,
+also as a scaled integer defaulting to bytes.
 I<--format> I<string> is used in file based storage pools to specify the volume
 file format to use; raw, bochs, qcow, qcow2, vmdk.
 I<--backing-vol> I<vol-name-or-key-or-path> is the source backing
@@ -2163,7 +2185,12 @@ is in. I<vol-name-or-key-or-path> 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<capacity> 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<capacity> cannot
+be negative unless I<--shrink> is provided, but a negative sign is not
+necessary. I<capacity> is a scaled integer (see B<NOTES> 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<blockresize> for live resizing.
 
 =back