virsh: blockcopy: Support --bytes and scaled integers

Use vshBlockJobOptionBandwidth to parse the bandwidth value which will
allow users to specify bandwidth in bytes or as a scaled integer.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1288000

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index b18ac32b4773d94412b5a8c528c16c97ecd51772..510f68128b18257c43b5efa8b422d61c440e473e 100644 (file)
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -2245,6 +2245,10 @@ static const vshCmdOptDef opts_block_copy[] = {
      .type = VSH_OT_INT,
      .help = N_("maximum amount of in-flight data during the copy")
     },
+    {.name = "bytes",
+     .type = VSH_OT_BOOL,
+     .help = N_("the bandwidth limit is in bytes/s rather than MiB/s")
+    },
     {.name = NULL}
 };
 
@@ -2265,6 +2269,7 @@ cmdBlockCopy(vshControl *ctl, const vshCmd *cmd)
     bool blockdev = vshCommandOptBool(cmd, "blockdev");
     bool blocking = vshCommandOptBool(cmd, "wait") || finish || pivot;
     bool async = vshCommandOptBool(cmd, "async");
+    bool bytes = vshCommandOptBool(cmd, "bytes");
     int timeout = 0;
     const char *path = NULL;
     int abort_flags = 0;
@@ -2282,11 +2287,7 @@ cmdBlockCopy(vshControl *ctl, const vshCmd *cmd)
         return false;
     if (vshCommandOptStringReq(ctl, cmd, "format", &format) < 0)
         return false;
-    /* XXX: Parse bandwidth as scaled input, rather than forcing
-     * MiB/s, and either reject negative input or treat it as 0 rather
-     * than trying to guess which value will work well across both
-     * APIs with their different sizes and scales.  */
-    if (vshCommandOptULWrap(ctl, cmd, "bandwidth", &bandwidth) < 0)
+    if (vshBlockJobOptionBandwidth(ctl, cmd, bytes, &bandwidth) < 0)
         return false;
     if (vshCommandOptUInt(ctl, cmd, "granularity", &granularity) < 0)
         return false;
@@ -2351,17 +2352,21 @@ cmdBlockCopy(vshControl *ctl, const vshCmd *cmd)
         if (bandwidth || granularity || buf_size) {
             params = vshCalloc(ctl, 3, sizeof(*params));
             if (bandwidth) {
-                /* bandwidth is ulong MiB/s, but the typed parameter is
-                 * ullong bytes/s; make sure we don't overflow */
-                unsigned long long limit = MIN(ULONG_MAX, ULLONG_MAX >> 20);
-                if (bandwidth > limit) {
-                    vshError(ctl, _("bandwidth must be less than %llu"), limit);
-                    goto cleanup;
+                if (!bytes) {
+                    /* bandwidth is ulong MiB/s, but the typed parameter is
+                     * ullong bytes/s; make sure we don't overflow */
+                    unsigned long long limit = MIN(ULONG_MAX, ULLONG_MAX >> 20);
+                    if (bandwidth > limit) {
+                        vshError(ctl, _("bandwidth must be less than %llu"), limit);
+                        goto cleanup;
+                    }
+
+                    bandwidth <<= 20ULL;
                 }
                 if (virTypedParameterAssign(&params[nparams++],
                                             VIR_DOMAIN_BLOCK_COPY_BANDWIDTH,
                                             VIR_TYPED_PARAM_ULLONG,
-                                            bandwidth << 20ULL) < 0)
+                                            bandwidth) < 0)
                     goto cleanup;
             }
             if (granularity &&
@@ -2402,6 +2407,8 @@ cmdBlockCopy(vshControl *ctl, const vshCmd *cmd)
             flags |= VIR_DOMAIN_BLOCK_REBASE_COPY_DEV;
         if (STREQ_NULLABLE(format, "raw"))
             flags |= VIR_DOMAIN_BLOCK_REBASE_COPY_RAW;
+        if (bytes)
+            flags |= VIR_DOMAIN_BLOCK_REBASE_BANDWIDTH_BYTES;
 
         if (virDomainBlockRebase(dom, path, dest, bandwidth, flags) < 0)
             goto cleanup;

diff --git a/tools/virsh.pod b/tools/virsh.pod
index a1808acf433f424236e5e3618b875321d569939a..b1d859416f23d3efa8acdc31a6a6c8b6810a6a6a 100644 (file)
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -994,7 +994,7 @@ command.
 =item B<blockcopy> I<domain> I<path> { I<dest> [I<format>] [I<--blockdev>]
 | I<--xml> B<file> } [I<--shallow>] [I<--reuse-external>] [I<bandwidth>]
 [I<--wait> [I<--async>] [I<--verbose>]] [{I<--pivot> | I<--finish>}]
-[I<--timeout> B<seconds>] [I<granularity>] [I<buf-size>]
+[I<--timeout> B<seconds>] [I<granularity>] [I<buf-size>] [I<--bytes>]
 
 Copy a disk backing image chain to a destination.  Either I<dest> as
 the destination file name, or I<--xml> with the name of an XML file containing
@@ -1039,8 +1039,10 @@ I<path> specifies fully-qualified path of the disk.
 I<bandwidth> specifies copying bandwidth limit in MiB/s. Specifying a negative
 value is interpreted as an unsigned long long value that might be essentially
 unlimited, but more likely would overflow; it is safer to use 0 for that
-purpose.  Specifying I<granularity> allows fine-tuning of the granularity that
-will be copied when a dirty region is detected; larger values trigger less
+purpose. For further information on the I<bandwidth> argument see the
+corresponding section for the B<blockjob> command.
+Specifying I<granularity> allows fine-tuning of the granularity that will be
+copied when a dirty region is detected; larger values trigger less
 I/O overhead but may end up copying more data overall (the default value is
 usually correct); hypervisors may restrict this to be a power of two or fall
 within a certain range. Specifying I<buf-size> will control how much data can