Take the previous patch one step further. In addition to
automatically selecting byte mode with fallback if the
user's result would be rounded, we also want to give the
user a way to guarantee byte mode even if rounding is not
required, with no fallback if the remote side doesn't
support byte mode. This way, virsh can be used to more
fully test the impact of setting the bytes flag.
* tools/virsh-domain.c (blockJobBandwidth): Adjust return type and
check for --bytes flag.
(cmdBlockCommit, cmdBlockPull, cmdBlockJob): Add --bytes flag.
(cmdBlockCopy): Likewise, and adjust caller.
(blockJobImpl): Adjust caller.
* tools/virsh.pod (blockcommit, blockcopy, blockpull, blockjob):
Document this.
Signed-off-by: Eric Blake <eblake@xxxxxxxxxx>
---
tools/virsh-domain.c | 57 ++++++++++++++++++++++++++++++++++------------------
tools/virsh.pod | 48 +++++++++++++++++++++----------------------
2 files changed, 61 insertions(+), 44 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 14d6921..c6ed90e 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -1474,14 +1474,16 @@ typedef enum {
/* Parse a block job bandwidth parameter. Caller must provide exactly
* one of @bandwidth or @bandwidthBytes. Returns -1 on error with
* message printed, 0 if no bandwidth needed, 1 if *bandwidth is set
- * to non-zero MiB/s, and 2 if *bandwidth or *bandwidthBytes is
- * non-zero bytes/s. */
+ * to non-zero MiB/s, 2 if *bandwidth or *bandwidthBytes is non-zero
+ * bytes/s but fallback to MiB/s is okay, and 3 if *bandwidth or
+ * *bandwidthBytes is non-zero bytes/s and fallback is not okay. */
static int
blockJobBandwidth(vshControl *ctl, const vshCmd *cmd,
unsigned long *bandwidth, unsigned long long *bandwidthBytes)
{
const char *value = NULL;
int ret = vshCommandOptString(cmd, "bandwidth", &value);
+ bool bytes = vshCommandOptBool(cmd, "bytes");
unsigned long long speed;
unsigned long long limit = ULLONG_MAX;
@@ -1489,7 +1491,7 @@ blockJobBandwidth(vshControl *ctl, const vshCmd *cmd,
*bandwidth = 0;
bandwidthBytes = &speed;
if (sizeof(*bandwidth) < sizeof(speed))
- limit = ULONG_MAX << 20ULL;
+ limit = ULONG_MAX << (bytes ? 0 : 20ULL);
} else {
*bandwidthBytes = 0;
}
@@ -1515,20 +1517,23 @@ blockJobBandwidth(vshControl *ctl, const vshCmd *cmd,
/* Read a number in bytes/s, but with default unit of MiB/s if no
* unit was specified. */
- if (vshCommandOptScaledInt(cmd, "bandwidth", bandwidthBytes, 1024 * 1024,
- limit) < 0) {
+ if (vshCommandOptScaledInt(cmd, "bandwidth", bandwidthBytes,
+ bytes ? 1 : 1024 * 1024, limit) < 0) {
vshError(ctl, _("could not parse bandwidth '%s'"), value);
return -1;
}
if (!bandwidth) /* bandwidthBytes is already correct */
- return 2;
+ return 3;
- /* If the number fits in unsigned long, is not a multiple of
- * MiB/s, and we haven't proven the new API flag will fail, then
- * let the caller try it directly. Otherwise, scale it to MiB/s
- * for the caller. What a pain. */
- if ((unsigned long) speed == speed && speed & (1024 * 1024 - 1) &&
- !ctl->blockJobNoBytes) {
+ /* If the caller did not force bytes, the number fits in unsigned
+ * long, the number is not a multiple of MiB/s, and we haven't
+ * proven the new API flag will fail, then let the caller try it
+ * directly. Otherwise, scale it to MiB/s for the caller. What a
+ * pain. */
+ if (bytes) {
+ ret = 3;
+ } else if ((unsigned long) speed == speed && speed & (1024 * 1024 - 1) &&
+ !ctl->blockJobNoBytes) {
ret = 2;
} else {
speed = VIR_DIV_UP(speed, 1024 * 1024);
@@ -1577,7 +1582,7 @@ blockJobImpl(vshControl *ctl, const vshCmd *cmd,
if (bandwidthScale > 1)
flags |= VIR_DOMAIN_BLOCK_JOB_SPEED_BANDWIDTH_BYTES;
rc = virDomainBlockJobSetSpeed(dom, path, bandwidth, flags);
- if (rc < 0 && bandwidthScale > 1 &&
+ if (rc < 0 && bandwidthScale == 2 &&
last_error->code == VIR_ERR_INVALID_ARG) {
/* try again with MiB/s */
ctl->blockJobNoBytes = true;
@@ -1602,7 +1607,7 @@ blockJobImpl(vshControl *ctl, const vshCmd *cmd,
else
rc = virDomainBlockPull(dom, path, bandwidth, flags);
- if (rc < 0 && bandwidthScale > 1 &&
+ if (rc < 0 && bandwidthScale == 2 &&
last_error->code == VIR_ERR_INVALID_ARG) {
/* try again with MiB/s */
ctl->blockJobNoBytes = true;
@@ -1634,7 +1639,7 @@ blockJobImpl(vshControl *ctl, const vshCmd *cmd,
if (bandwidthScale > 1)
flags |= VIR_DOMAIN_BLOCK_COMMIT_BANDWIDTH_BYTES;
rc = virDomainBlockCommit(dom, path, base, top, bandwidth, flags);
- if (rc < 0 && bandwidthScale > 1 &&
+ if (rc < 0 && bandwidthScale == 2 &&
last_error->code == VIR_ERR_INVALID_ARG) {
/* try again with MiB/s */
ctl->blockJobNoBytes = true;
@@ -1723,6 +1728,10 @@ static const vshCmdOptDef opts_block_commit[] = {
.type = VSH_OT_DATA,
.help = N_("bandwidth limit, as scaled integer (default MiB/s)")
},
+ {.name = "bytes",
+ .type = VSH_OT_BOOL,
+ .help = N_("treat bandwidth as bytes/s")
+ },
{.name = "base",
.type = VSH_OT_DATA,
.help = N_("path of base file to commit into (default bottom of chain)")
@@ -1939,6 +1948,10 @@ static const vshCmdOptDef opts_block_copy[] = {
.type = VSH_OT_DATA,
.help = N_("bandwidth limit, as scaled integer (default MiB/s)")
},
+ {.name = "bytes",
+ .type = VSH_OT_BOOL,
+ .help = N_("treat bandwidth as bytes/s")
+ },
{.name = "shallow",
.type = VSH_OT_BOOL,
.help = N_("make the copy share a backing chain")
@@ -2158,7 +2171,7 @@ cmdBlockCopy(vshControl *ctl, const vshCmd *cmd)
flags |= VIR_DOMAIN_BLOCK_REBASE_BANDWIDTH_BYTES;
rc = virDomainBlockRebase(dom, path, dest, bandwidth, flags);
- if (rc < 0 && bandwidthScale > 1 &&
+ if (rc < 0 && bandwidthScale == 2 &&
last_error->code == VIR_ERR_INVALID_ARG) {
/* try again with MiB/s */
ctl->blockJobNoBytes = true;
@@ -2291,7 +2304,8 @@ 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_("with --info or --bandwidth, treat bandwidth in bytes "
+ "rather than MiB/s")
},
{.name = "raw",
.type = VSH_OT_BOOL,
@@ -2343,9 +2357,8 @@ cmdBlockJob(vshControl *ctl, const vshCmd *cmd)
_("conflict between abort, info, and bandwidth modes"));
return false;
}
- /* XXX also support --bytes with bandwidth mode */
- if (bytes && (abortMode || bandwidth)) {
- vshError(ctl, "%s", _("--bytes requires info mode"));
+ if (bytes && abortMode) {
+ vshError(ctl, "%s", _("--bytes not compatible with abort mode"));
return false;
}
@@ -2460,6 +2473,10 @@ static const vshCmdOptDef opts_block_pull[] = {
.type = VSH_OT_DATA,
.help = N_("bandwidth limit, as scaled integer (default MiB/s)")
},
+ {.name = "bytes",
+ .type = VSH_OT_BOOL,
+ .help = N_("treat bandwidth as bytes/s")
+ },
{.name = "base",
.type = VSH_OT_DATA,
.help = N_("path of backing file in chain for a partial pull")
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 7aba571..c9b4c31 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -847,7 +847,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>}]
@@ -893,14 +893,14 @@ 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> is a scaled integer (see B<NOTES> above) which defaults to
-MiB/s if there is no suffix. It can be used to set a bandwidth limit for
-the commit job. For backward compatibility, a negative value is treated
-the same as 0 (unlimited); the actual upper limit accepted by the command
-may depend on the word size of both client and server, or by further limits
-of the hypervisor.
+MiB/s if there is no suffix, or to bytes/s if I<--bytes> is present. It
+can be used to set a bandwidth limit for the commit job. For backward
+compatibility, a negative value is treated the same as 0 (unlimited); the
+actual upper limit accepted by the command may depend on the word size of
+both client and server, or by further limits of the hypervisor.
=item B<blockcopy> I<domain> I<path> { I<dest> [I<format>] [I<--blockdev>]
-| I<xml> } [I<--shallow>] [I<--reuse-external>] [I<bandwidth>]
+| I<xml> } [I<--shallow>] [I<--reuse-external>] [I<bandwidth> [I<--bytes>]]
[I<--wait> [I<--async>] [I<--verbose>]] [{I<--pivot> | I<--finish>}]
[I<--timeout> B<seconds>] [I<granularity>] [I<buf-size>]
@@ -945,11 +945,11 @@ while longer until the job has actually cancelled.
I<path> specifies fully-qualified path of the disk.
I<bandwidth> is a scaled integer (see B<NOTES> above) which defaults to
-MiB/s if there is no suffix. It can be used to set a bandwidth limit for
-the copy job. For backward compatibility, a negative value is treated
-the same as 0 (unlimited); the actual upper limit accepted by the command
-may depend on the word size of both client and server, or by further limits
-of the hypervisor.
+MiB/s if there is no suffix, or to bytes/s if I<--bytes> is present. It
+can be used to set a bandwidth limit for the copy job. For backward
+compatibility, a negative value is treated the same as 0 (unlimited); the
+actual upper limit accepted by the command may depend on the word size of
+both client and server, or by further limits of the hypervisor.
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
@@ -958,7 +958,7 @@ 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>]
@@ -988,11 +988,11 @@ 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> is a scaled integer (see B<NOTES> above) which defaults to
-MiB/s if there is no suffix. It can be used to set a bandwidth limit for
-the pull job. For backward compatibility, a negative value is treated
-the same as 0 (unlimited); the actual upper limit accepted by the command
-may depend on the word size of both client and server, or by further limits
-of the hypervisor.
+MiB/s if there is no suffix, or to bytes/s if I<--bytes> is present. It
+can be used to set a bandwidth limit for the pull job. For backward
+compatibility, a negative value is treated the same as 0 (unlimited); the
+actual upper limit accepted by the command may depend on the word size of
+both client and server, or by further limits of the hypervisor.
=item B<blkdeviotune> I<domain> I<device>
[[I<--config>] [I<--live>] | [I<--current>]]
@@ -1029,7 +1029,7 @@ exclusive. If no flag is specified, behavior is different depending
on hypervisor.
=item B<blockjob> I<domain> I<path> { [I<--abort>] [I<--async>] [I<--pivot>] |
-[I<--info>] [I<--raw>] [I<--bytes>] | [I<bandwidth>] }
+[I<--info>] [I<--raw>] | [I<bandwidth>] } [I<--bytes>]
Manage active block operations. There are three mutually-exclusive modes:
I<--info>, I<bandwidth>, and I<--abort>. I<--async> and I<--pivot> imply
@@ -1057,11 +1057,11 @@ listed in MiB/s and human-readable output automatically selects the
best resolution supported by the server.
I<bandwidth> is a scaled integer (see B<NOTES> above) which defaults to
-MiB/s if there is no suffix. It can be used to set bandwidth limit for
-the active job. For backward compatibility, a negative value is treated
-the same as 0 (unlimited); the actual upper limit accepted by the command
-may depend on the word size of both client and server, or by further limits
-of the hypervisor.
+MiB/s if there is no suffix, or to bytes/s if I<--bytes> is present. It
+can be used to set bandwidth limit for the active job. For backward
+compatibility, a negative value is treated the same as 0 (unlimited); the
+actual upper limit accepted by the command may depend on the word size of
+both client and server, or by further limits of the hypervisor.
=item B<blockresize> I<domain> I<path> I<size>
--
1.9.3
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list