[libvirt] [PATCH 0/6] virsh: Add support for byte granularity and scaled integers for virsh block APIs
3:56 a.m.
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
3:57 a.m.
New subject: [libvirt] [PATCH 1/6] vsh: Tweak error message for scaled integers
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
3:57 a.m.
New subject: [libvirt] [PATCH 2/6] vsh: Refactor vshCommandOptScaledInt
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
3:57 a.m.
New subject: [libvirt] [PATCH 3/6] virsh: blockjob: Support --bytes and scaled integers as bandwidth
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
2:48 p.m.
New subject: [libvirt] [PATCH 3/6] virsh: blockjob: Support --bytes and scaled integers as bandwidth
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
*/
[...]
3:57 a.m.
New subject: [libvirt] [PATCH 4/6] virsh: blockcommit: Support --bytes and scaled integers
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
3:57 a.m.
New subject: [libvirt] [PATCH 5/6] virsh: blockcopy: Support --bytes and scaled integers
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
3:57 a.m.
New subject: [libvirt] [PATCH 6/6] virsh: blockpull: Support --bytes and scaled integers
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
2:48 p.m.
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
9:02 a.m.
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
3161
days inactive
3172
days old
10 comments
2 participants
participants (2)
-
John Ferlan -
Peter Krempa