Re: [libvirt] [PATCHv2 14/15] virsh: improve storage unit parsing
On 03/06/2012 01:34 AM, Eric Blake wrote:
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.
* tools/virsh.c (vshCommandOptScaledInt): New function.
(cmdVolResize): Don't pass negative size.
(cmdVolSize): Use new helper routine.
(cmdBlockResize): Likewise; also support bytes.
* tools/virsh.pod (NOTES): Document suffixes.
(blockresize, vol-create-as, vol-resize): Point to notes.
---
static bool
@@ -11754,7 +11743,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")},
@@ -11792,16 +11781,12 @@ 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);
- goto cleanup;
- }
+ capacityStr++;
This shift in the string discards the minus sign in
error/success
messages, but their meaning remains correct.
+ flags |= VIR_STORAGE_VOL_RESIZE_SHRINK;
This change in code ...
+ }
+ if (cmdVolSize(capacityStr,&capacity)< 0) {
+ vshError(ctl, _("Malformed size %s"), capacityStr);
+ goto cleanup;
}
if (virStorageVolResize(vol, capacity, flags) == 0) {
@@ -2146,7 +2168,10 @@ 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.<capacity> is
... contradicts with this statment in the virsh man page. With your
change, if the delta is negative the shrink flag for the api call is set
automaticaly.
This was probably added as a safety measure and I'd prefer if we would
require the --shrink flag to mark that the user is sure of what he's
doing, although it should be obvious enough from the minus sign. (I
honestly would prefer a docs change but I'm afraid someone could get mad
at us if he "accidentaly" corrupts his images.)
+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
ACK, if you add the check for --shrink.
Peter