[libvirt] [PATCH 0/6] virsh: Add support for byte granularity and scaled integers for virsh block APIs
Quite some time ago we've added support for byte granularity for block job bandwidth. Make it work in virsh and add support for scaled integers too. Peter Krempa (6): vsh: Tweak error message for scaled integers vsh: Refactor vshCommandOptScaledInt virsh: blockjob: Support --bytes and scaled integers as bandwidth virsh: blockcommit: Support --bytes and scaled integers virsh: blockcopy: Support --bytes and scaled integers virsh: blockpull: Support --bytes and scaled integers tests/virsh-optparse | 6 ++--- tools/virsh-domain.c | 73 ++++++++++++++++++++++++++++++++++++---------------- tools/virsh.pod | 37 ++++++++++++++------------ tools/vsh.c | 66 +++++++++++++++++++++++++++++++++++++++++------ tools/vsh.h | 4 +++ 5 files changed, 137 insertions(+), 49 deletions(-) -- 2.7.3
It was too similar to the non-scaled alternative. before: error: Numeric value 'abc' for <size> option is malformed or out of range after: error: Scaled numeric value 'abc' for <size> option is malformed or out of range --- tests/virsh-optparse | 6 +++--- tools/vsh.c | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/virsh-optparse b/tests/virsh-optparse index e032094..0fc261d 100755 --- a/tests/virsh-optparse +++ b/tests/virsh-optparse @@ -205,7 +205,7 @@ compare exp-err err || fail=1 # Non-numeric value cat <<\EOF > exp-err || framework_failure -error: Numeric value 'abc' for <size> option is malformed or out of range +error: Scaled numeric value 'abc' for <size> option is malformed or out of range EOF virsh -q -c $test_url setmaxmem test abc >out 2>err && fail=1 test -s out && fail=1 @@ -213,7 +213,7 @@ compare exp-err err || fail=1 # Numeric value with invalid suffix cat <<\EOF > exp-err || framework_failure -error: Numeric value '42WB' for <size> option is malformed or out of range +error: Scaled numeric value '42WB' for <size> option is malformed or out of range EOF virsh -q -c $test_url setmaxmem test 42WB >out 2>err && fail=1 test -s out && fail=1 @@ -232,7 +232,7 @@ test -s err && fail=1 # Negative numeric value cat <<\EOF > exp-err || framework_failure -error: Numeric value '-1' for <size> option is malformed or out of range +error: Scaled numeric value '-1' for <size> option is malformed or out of range EOF virsh -q -c $test_url setmaxmem test -1 >out 2>err && fail=1 test -s out && fail=1 diff --git a/tools/vsh.c b/tools/vsh.c index 4ce1727..047a0cc 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -1147,8 +1147,8 @@ vshCommandOptScaledInt(vshControl *ctl, const vshCmd *cmd, virScaleInteger(value, end, scale, max) < 0) { vshError(ctl, - _("Numeric value '%s' for <%s> option is malformed or out of range"), - arg->data, name); + _("Scaled numeric value '%s' for <%s> option is malformed or " + "out of range"), arg->data, name); ret = -1; } else { ret = 1; -- 2.7.3
Fix control flow and spacing issues. --- tools/vsh.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/tools/vsh.c b/tools/vsh.c index 047a0cc..cbe8189 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -1143,18 +1143,16 @@ vshCommandOptScaledInt(vshControl *ctl, const vshCmd *cmd, if ((ret = vshCommandOpt(cmd, name, &arg, true)) <= 0) return ret; + if (virStrToLong_ullp(arg->data, &end, 10, value) < 0 || - virScaleInteger(value, end, scale, max) < 0) - { + virScaleInteger(value, end, scale, max) < 0) { vshError(ctl, _("Scaled numeric value '%s' for <%s> option is malformed or " "out of range"), arg->data, name); - ret = -1; - } else { - ret = 1; + return -1; } - return ret; + return 1; } -- 2.7.3
Allow specifying sizes in bytes or as scaled integers for user convenience. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1288000 --- tools/virsh-domain.c | 17 ++++++++++------- tools/virsh.pod | 11 ++++++++--- tools/vsh.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++ tools/vsh.h | 4 ++++ 4 files changed, 74 insertions(+), 10 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 854823e..5ea39f7 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -2492,7 +2492,7 @@ static const vshCmdOptDef opts_block_job[] = { }, {.name = "bytes", .type = VSH_OT_BOOL, - .help = N_("with --info, get bandwidth in bytes rather than MiB/s") + .help = N_("get/set bandwidth in bytes rather than MiB/s") }, {.name = "raw", .type = VSH_OT_BOOL, @@ -2611,14 +2611,19 @@ static bool virshBlockJobSetSpeed(vshControl *ctl, const vshCmd *cmd, virDomainPtr dom, - const char *path) + const char *path, + bool bytes) { unsigned long bandwidth; + unsigned int flags = 0; - if (vshCommandOptULWrap(ctl, cmd, "bandwidth", &bandwidth) < 0) + if (bytes) + flags |= VIR_DOMAIN_BLOCK_JOB_SPEED_BANDWIDTH_BYTES; + + if (vshBlockJobOptionBandwidth(ctl, cmd, bytes, &bandwidth) < 0) return false; - if (virDomainBlockJobSetSpeed(dom, path, bandwidth, 0) < 0) + if (virDomainBlockJobSetSpeed(dom, path, bandwidth, flags) < 0) return false; return true; @@ -2672,8 +2677,6 @@ cmdBlockJob(vshControl *ctl, const vshCmd *cmd) VSH_EXCLUSIVE_OPTIONS("bytes", "abort"); VSH_EXCLUSIVE_OPTIONS_VAR(bytes, pivot); VSH_EXCLUSIVE_OPTIONS_VAR(bytes, async); - /* XXX also support --bytes with bandwidth mode */ - VSH_EXCLUSIVE_OPTIONS_VAR(bytes, bandwidth); if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) goto cleanup; @@ -2683,7 +2686,7 @@ cmdBlockJob(vshControl *ctl, const vshCmd *cmd) goto cleanup; if (bandwidth) - ret = virshBlockJobSetSpeed(ctl, cmd, dom, path); + ret = virshBlockJobSetSpeed(ctl, cmd, dom, path, bytes); else if (abortMode || pivot || async) ret = virshBlockJobAbort(dom, path, pivot, async); else diff --git a/tools/virsh.pod b/tools/virsh.pod index 1e2c6a6..edb9ce5 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1152,10 +1152,15 @@ not supply bytes/s resolution; when omitting the flag, raw output is listed in MiB/s and human-readable output automatically selects the best resolution supported by the server. -I<bandwidth> can be used to set bandwidth limit for the active job. -Specifying a negative value is interpreted as an unsigned long long +I<bandwidth> can be used to set bandwidth limit for the active job in MiB/s. +If I<--bytes> is specified then the bandwidth value is interpreted in +bytes/s. Specifying a negative value is interpreted as an unsigned long value or essentially unlimited. The hypervisor can choose whether to -reject the value or convert it to the maximum value allowed. +reject the value or convert it to the maximum value allowed. Optionally a +scaled positive number may be used as bandwidth (see B<NOTES> above). Using +I<--bytes> with a scaled value allows to use finer granularity. A scaled value +used without I<--bytes> will be rounded down to MiB/s. Note that the +I<--bytes> may be unsupported by the hypervisor. =item B<blockresize> I<domain> I<path> I<size> diff --git a/tools/vsh.c b/tools/vsh.c index cbe8189..f90619d 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -1201,6 +1201,58 @@ vshCommandOptArgv(vshControl *ctl ATTRIBUTE_UNUSED, const vshCmd *cmd, } +/** + * vshBlockJobOptionBandwidth + * @ctl: virsh control data + * @cmd: virsh command description + * @bytes: return bandwidth in bytes/s instead of MiB/s + * @bandwidth: return value + * + * Extracts the value of --bandwidth either as a wrappable number without scale + * or as a scaled integer. The returned value is checked to fit into a unsigned + * long data type. This is a legacy compatibility function and it should not + * be used for things other the block job APIs. + * + * Returns 0 on success, -1 on error. + */ +int +vshBlockJobOptionBandwidth(vshControl *ctl, + const vshCmd *cmd, + bool bytes, + unsigned long *bandwidth) +{ + vshCmdOpt *arg; + char *end; + unsigned long long bw; + int ret; + + if ((ret = vshCommandOpt(cmd, "bandwidth", &arg, true)) <= 0) + return ret; + + /* due to historical reasons we declare to parse negative numbers and wrap + * them to the unsigned data type. */ + if (virStrToLong_ul(arg->data, NULL, 10, bandwidth) < 0) { + /* try to parse the number as scaled size in this cas we don't accept + * wrapping since it would be ridiculous. In case of a 32 bit host, + * limit the value to ULONG_MAX */ + if (virStrToLong_ullp(arg->data, &end, 10, &bw) < 0 || + virScaleInteger(&bw, end, 1, ULONG_MAX) < 0) { + vshError(ctl, + _("Scaled numeric value '%s' for <--bandwidth> option is " + "malformed or out of range"), arg->data); + return -1; + } + + if (!bytes) + bw >>= 20; + + *bandwidth = bw; + } + + return 0; +} + + /* * Executes command(s) and returns return code from last command */ diff --git a/tools/vsh.h b/tools/vsh.h index f6e40e6..f738a6f 100644 --- a/tools/vsh.h +++ b/tools/vsh.h @@ -291,6 +291,10 @@ int vshCommandOptScaledInt(vshControl *ctl, const vshCmd *cmd, const char *name, unsigned long long *value, int scale, unsigned long long max) ATTRIBUTE_NONNULL(4) ATTRIBUTE_RETURN_CHECK; +int vshBlockJobOptionBandwidth(vshControl *ctl, + const vshCmd *cmd, + bool bytes, + unsigned long *bandwidth); bool vshCommandOptBool(const vshCmd *cmd, const char *name); bool vshCommandRun(vshControl *ctl, const vshCmd *cmd); bool vshCommandStringParse(vshControl *ctl, char *cmdstr); -- 2.7.3
On 03/18/2016 04:57 AM, Peter Krempa wrote:
Allow specifying sizes in bytes or as scaled integers for user convenience.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1288000 --- tools/virsh-domain.c | 17 ++++++++++------- tools/virsh.pod | 11 ++++++++--- tools/vsh.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++ tools/vsh.h | 4 ++++ 4 files changed, 74 insertions(+), 10 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
[...]
diff --git a/tools/vsh.c b/tools/vsh.c index cbe8189..f90619d 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -1201,6 +1201,58 @@ vshCommandOptArgv(vshControl *ctl ATTRIBUTE_UNUSED, const vshCmd *cmd, }
+/** + * vshBlockJobOptionBandwidth + * @ctl: virsh control data + * @cmd: virsh command description + * @bytes: return bandwidth in bytes/s instead of MiB/s + * @bandwidth: return value + * + * Extracts the value of --bandwidth either as a wrappable number without scale
wrappable is not a word... but wrap-able works
+ * or as a scaled integer. The returned value is checked to fit into a unsigned + * long data type. This is a legacy compatibility function and it should not + * be used for things other the block job APIs. + * + * Returns 0 on success, -1 on error. + */ +int +vshBlockJobOptionBandwidth(vshControl *ctl, + const vshCmd *cmd, + bool bytes, + unsigned long *bandwidth) +{ + vshCmdOpt *arg; + char *end; + unsigned long long bw; + int ret; + + if ((ret = vshCommandOpt(cmd, "bandwidth", &arg, true)) <= 0) + return ret; + + /* due to historical reasons we declare to parse negative numbers and wrap + * them to the unsigned data type. */ + if (virStrToLong_ul(arg->data, NULL, 10, bandwidth) < 0) { + /* try to parse the number as scaled size in this cas we don't accept
s/this cas/this case/
+ * wrapping since it would be ridiculous. In case of a 32 bit host, + * limit the value to ULONG_MAX */ + if (virStrToLong_ullp(arg->data, &end, 10, &bw) < 0 || + virScaleInteger(&bw, end, 1, ULONG_MAX) < 0) { + vshError(ctl, + _("Scaled numeric value '%s' for <--bandwidth> option is " + "malformed or out of range"), arg->data); + return -1; + } + + if (!bytes) + bw >>= 20; + + *bandwidth = bw; + } + + return 0; +} + + /* * Executes command(s) and returns return code from last command */ [...]
Reuse the approach and helper from the last patch. --- tools/virsh-domain.c | 10 +++++++++- tools/virsh.pod | 9 ++++----- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 5ea39f7..cd616a8 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -2010,6 +2010,10 @@ static const vshCmdOptDef opts_block_commit[] = { .type = VSH_OT_BOOL, .help = N_("keep the backing chain relatively referenced") }, + {.name = "bytes", + .type = VSH_OT_BOOL, + .help = N_("the bandwidth limit is in bytes/s rather than MiB/s") + }, {.name = NULL} }; @@ -2024,6 +2028,7 @@ cmdBlockCommit(vshControl *ctl, const vshCmd *cmd) bool active = vshCommandOptBool(cmd, "active") || pivot || finish; bool blocking = vshCommandOptBool(cmd, "wait") || pivot || finish; bool async = vshCommandOptBool(cmd, "async"); + bool bytes = vshCommandOptBool(cmd, "bytes"); int timeout = 0; const char *path = NULL; const char *base = NULL; @@ -2044,9 +2049,12 @@ cmdBlockCommit(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptStringReq(ctl, cmd, "top", &top) < 0) return false; - if (vshCommandOptULWrap(ctl, cmd, "bandwidth", &bandwidth) < 0) + if (vshBlockJobOptionBandwidth(ctl, cmd, bytes, &bandwidth) < 0) return false; + if (bytes) + flags |= VIR_DOMAIN_BLOCK_COMMIT_BANDWIDTH_BYTES; + if (vshCommandOptBool(cmd, "shallow")) flags |= VIR_DOMAIN_BLOCK_COMMIT_SHALLOW; diff --git a/tools/virsh.pod b/tools/virsh.pod index edb9ce5..7d61f22 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -938,7 +938,7 @@ currently in use by a running domain. Other contexts that require a MAC address of virtual interface (such as I<detach-interface> or I<domif-setlink>) will accept the MAC address printed by this command. -=item B<blockcommit> I<domain> I<path> [I<bandwidth>] +=item B<blockcommit> I<domain> I<path> [I<bandwidth>] [I<--bytes>] [I<base>] [I<--shallow>] [I<top>] [I<--delete>] [I<--keep-relative>] [I<--wait> [I<--async>] [I<--verbose>]] [I<--timeout> B<seconds>] [I<--active>] [{I<--pivot> | I<--keep-overlay>}] @@ -984,10 +984,9 @@ to a unique target name (<target dev='name'/>) or source file (<source file='name'/>) for one of the disk devices attached to I<domain> (see also B<domblklist> for listing these names). I<bandwidth> specifies copying bandwidth limit in MiB/s, although for -qemu, it may be non-zero only for an online domain. Specifying a negative -value is interpreted as an unsigned long long value or essentially -unlimited. The hypervisor can choose whether to reject the value or -convert it to the maximum value allowed. +qemu, it may be non-zero only for an online domain. For further information +on the I<bandwidth> argument see the corresponding section for the B<blockjob> +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>] -- 2.7.3
Reuse the approach and helper from the last patch. --- tools/virsh-domain.c | 31 +++++++++++++++++++------------ tools/virsh.pod | 8 +++++--- 2 files changed, 24 insertions(+), 15 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index cd616a8..4053c75 100644 --- 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(¶ms[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 7d61f22..26cfe35 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -991,7 +991,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 @@ -1036,8 +1036,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 -- 2.7.3
Reuse the approach and helper from the last patch. --- tools/virsh-domain.c | 15 +++++++++++++-- tools/virsh.pod | 9 ++++----- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 4053c75..f808c73 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -2761,6 +2761,10 @@ static const vshCmdOptDef opts_block_pull[] = { .type = VSH_OT_BOOL, .help = N_("keep the backing chain relatively referenced") }, + {.name = "bytes", + .type = VSH_OT_BOOL, + .help = N_("the bandwidth limit is in bytes/s rather than MiB/s") + }, {.name = NULL} }; @@ -2772,6 +2776,7 @@ cmdBlockPull(vshControl *ctl, const vshCmd *cmd) bool blocking = vshCommandOptBool(cmd, "wait"); bool verbose = vshCommandOptBool(cmd, "verbose"); bool async = vshCommandOptBool(cmd, "async"); + bool bytes = vshCommandOptBool(cmd, "bytes"); int timeout = 0; const char *path = NULL; const char *base = NULL; @@ -2788,7 +2793,7 @@ cmdBlockPull(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptStringReq(ctl, cmd, "base", &base) < 0) return false; - if (vshCommandOptULWrap(ctl, cmd, "bandwidth", &bandwidth) < 0) + if (vshBlockJobOptionBandwidth(ctl, cmd, bytes, &bandwidth) < 0) return false; if (vshCommandOptTimeoutToMs(ctl, cmd, &timeout) < 0) @@ -2806,10 +2811,16 @@ cmdBlockPull(vshControl *ctl, const vshCmd *cmd) goto cleanup; if (base || flags) { + if (bytes) + flags |= VIR_DOMAIN_BLOCK_REBASE_BANDWIDTH_BYTES; + if (virDomainBlockRebase(dom, path, base, bandwidth, flags) < 0) goto cleanup; } else { - if (virDomainBlockPull(dom, path, bandwidth, 0) < 0) + if (bytes) + flags |= VIR_DOMAIN_BLOCK_PULL_BANDWIDTH_BYTES; + + if (virDomainBlockPull(dom, path, bandwidth, flags) < 0) goto cleanup; } diff --git a/tools/virsh.pod b/tools/virsh.pod index 26cfe35..35bc101 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1046,7 +1046,7 @@ within a certain range. Specifying I<buf-size> will control how much data can be simultaneously in-flight during the copy; larger values use more memory but may allow faster completion (the default value is usually correct). -=item B<blockpull> I<domain> I<path> [I<bandwidth>] [I<base>] +=item B<blockpull> I<domain> I<path> [I<bandwidth>] [I<--bytes>] [I<base>] [I<--wait> [I<--verbose>] [I<--timeout> B<seconds>] [I<--async>]] [I<--keep-relative>] @@ -1075,10 +1075,9 @@ I<path> specifies fully-qualified path of the disk; it corresponds to a unique target name (<target dev='name'/>) or source file (<source file='name'/>) for one of the disk devices attached to I<domain> (see also B<domblklist> for listing these names). -I<bandwidth> specifies copying bandwidth limit in MiB/s. Specifying a negative -value is interpreted as an unsigned long long value or essentially -unlimited. The hypervisor can choose whether to reject the value or -convert it to the maximum value allowed. +I<bandwidth> specifies copying bandwidth limit in MiB/s. For further information +on the I<bandwidth> argument see the corresponding section for the B<blockjob> +command. =item B<blkdeviotune> I<domain> I<device> [[I<--config>] [I<--live>] | [I<--current>]] -- 2.7.3
On 03/18/2016 04:56 AM, Peter Krempa wrote:
Quite some time ago we've added support for byte granularity for block job bandwidth. Make it work in virsh and add support for scaled integers too.
Peter Krempa (6): vsh: Tweak error message for scaled integers vsh: Refactor vshCommandOptScaledInt virsh: blockjob: Support --bytes and scaled integers as bandwidth virsh: blockcommit: Support --bytes and scaled integers virsh: blockcopy: Support --bytes and scaled integers virsh: blockpull: Support --bytes and scaled integers
tests/virsh-optparse | 6 ++--- tools/virsh-domain.c | 73 ++++++++++++++++++++++++++++++++++++---------------- tools/virsh.pod | 37 ++++++++++++++------------ tools/vsh.c | 66 +++++++++++++++++++++++++++++++++++++++++------ tools/vsh.h | 4 +++ 5 files changed, 137 insertions(+), 49 deletions(-)
Note specific nits from patch 3 I think the commit messages for patches 4-6 shouldn't be: "Reuse the approach and helper from the last patch." since "last patch" causes me to go find the "last patch"... cut-copy-paste what they're using vshBlockJobOptionBandwidth and allowing the --bytes on the set. If we really wanted to be picky, patch 3 could introduce the new function with patches 4-7 using it and indicating so in their commit messages. ACK series with at least the typos fixed and commit messages adjusted. Your choice if you want to extract out the function. John
On Thu, Mar 24, 2016 at 15:48:57 -0400, John Ferlan wrote:
On 03/18/2016 04:56 AM, Peter Krempa wrote:
Quite some time ago we've added support for byte granularity for block job bandwidth. Make it work in virsh and add support for scaled integers too.
Peter Krempa (6): vsh: Tweak error message for scaled integers vsh: Refactor vshCommandOptScaledInt virsh: blockjob: Support --bytes and scaled integers as bandwidth virsh: blockcommit: Support --bytes and scaled integers virsh: blockcopy: Support --bytes and scaled integers virsh: blockpull: Support --bytes and scaled integers
tests/virsh-optparse | 6 ++--- tools/virsh-domain.c | 73 ++++++++++++++++++++++++++++++++++++---------------- tools/virsh.pod | 37 ++++++++++++++------------ tools/vsh.c | 66 +++++++++++++++++++++++++++++++++++++++++------ tools/vsh.h | 4 +++ 5 files changed, 137 insertions(+), 49 deletions(-)
Note specific nits from patch 3
I think the commit messages for patches 4-6 shouldn't be:
"Reuse the approach and helper from the last patch."
since "last patch" causes me to go find the "last patch"...
cut-copy-paste what they're using vshBlockJobOptionBandwidth and allowing the --bytes on the set.
If we really wanted to be picky, patch 3 could introduce the new function with patches 4-7 using it and indicating so in their commit messages.
ACK series with at least the typos fixed and commit messages adjusted. Your choice if you want to extract out the function.
I've split out the function addition and fixed the commit messages and comments. Thanks; pushed. Peter
participants (2)
-
John Ferlan -
Peter Krempa