[libvirt] [PATCH v4 6/7] blockjob: automatically use bytes for bandwidth in virsh
Eric Blake
eblake at redhat.com
Fri Sep 12 03:55:40 UTC 2014
Trickier than I had hoped; but now that we can parse numbers with
arbitrary scale, it's time to use those numbers to call into the
new API if the old API would have rounded the input, while still
gracefully falling back to the old API if the new one fails.
* tools/virsh-domain.c (blockJobBandwidth): Add parameter, adjust
return value.
(blockJobImpl, cmdBlockCopy): Adjust callers.
Signed-off-by: Eric Blake <eblake at redhat.com>
---
tools/virsh-domain.c | 154 +++++++++++++++++++++++++++++++++++++--------------
1 file changed, 113 insertions(+), 41 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 594647a..14d6921 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -1471,18 +1471,29 @@ typedef enum {
} vshCmdBlockJobMode;
-/* Parse a block job bandwidth parameter. Returns -1 on error with
- * message printed, 0 if no bandwidth needed, and 1 if *bandwidth is
- * set to non-zero. */
+/* 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. */
static int
blockJobBandwidth(vshControl *ctl, const vshCmd *cmd,
- unsigned long *bandwidth)
+ unsigned long *bandwidth, unsigned long long *bandwidthBytes)
{
const char *value = NULL;
int ret = vshCommandOptString(cmd, "bandwidth", &value);
unsigned long long speed;
+ unsigned long long limit = ULLONG_MAX;
+
+ if (bandwidth) {
+ *bandwidth = 0;
+ bandwidthBytes = &speed;
+ if (sizeof(*bandwidth) < sizeof(speed))
+ limit = ULONG_MAX << 20ULL;
+ } else {
+ *bandwidthBytes = 0;
+ }
- *bandwidth = 0;
if (ret <= 0)
return ret;
@@ -1504,19 +1515,29 @@ 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", &speed, 1024 * 1024,
- (sizeof(*bandwidth) < sizeof(speed) ?
- ULONG_MAX * 1024 * 1024 : ULLONG_MAX)) < 0) {
+ if (vshCommandOptScaledInt(cmd, "bandwidth", bandwidthBytes, 1024 * 1024,
+ limit) < 0) {
vshError(ctl, _("could not parse bandwidth '%s'"), value);
return -1;
}
+ if (!bandwidth) /* bandwidthBytes is already correct */
+ return 2;
- /* Now scale it to MiB/s for the caller. What a pain. */
- speed = VIR_DIV_UP(speed, 1024 * 1024);
+ /* 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) {
+ ret = 2;
+ } else {
+ speed = VIR_DIV_UP(speed, 1024 * 1024);
+ ret = 1;
+ }
/* We already checked that narrowing the type will fit. */
*bandwidth = speed;
- return !!speed;
+ return ret;
}
@@ -1527,10 +1548,12 @@ blockJobImpl(vshControl *ctl, const vshCmd *cmd,
virDomainPtr dom = NULL;
const char *path;
unsigned long bandwidth = 0;
+ int bandwidthScale;
bool ret = false;
const char *base = NULL;
const char *top = NULL;
unsigned int flags = 0;
+ int rc;
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
goto cleanup;
@@ -1538,7 +1561,7 @@ blockJobImpl(vshControl *ctl, const vshCmd *cmd,
if (vshCommandOptStringReq(ctl, cmd, "path", &path) < 0)
goto cleanup;
- if (blockJobBandwidth(ctl, cmd, &bandwidth) < 0)
+ if ((bandwidthScale = blockJobBandwidth(ctl, cmd, &bandwidth, NULL)) < 0)
goto cleanup;
switch (mode) {
@@ -1551,7 +1574,19 @@ blockJobImpl(vshControl *ctl, const vshCmd *cmd,
goto cleanup;
break;
case VSH_CMD_BLOCK_JOB_SPEED:
- if (virDomainBlockJobSetSpeed(dom, path, bandwidth, 0) < 0)
+ if (bandwidthScale > 1)
+ flags |= VIR_DOMAIN_BLOCK_JOB_SPEED_BANDWIDTH_BYTES;
+ rc = virDomainBlockJobSetSpeed(dom, path, bandwidth, flags);
+ if (rc < 0 && bandwidthScale > 1 &&
+ last_error->code == VIR_ERR_INVALID_ARG) {
+ /* try again with MiB/s */
+ ctl->blockJobNoBytes = true;
+ flags &= ~VIR_DOMAIN_BLOCK_JOB_SPEED_BANDWIDTH_BYTES;
+ bandwidth = VIR_DIV_UP(bandwidth, 1024 * 1024);
+ vshResetLibvirtError();
+ rc = virDomainBlockJobSetSpeed(dom, path, bandwidth, flags);
+ }
+ if (rc < 0)
goto cleanup;
break;
case VSH_CMD_BLOCK_JOB_PULL:
@@ -1559,15 +1594,28 @@ blockJobImpl(vshControl *ctl, const vshCmd *cmd,
goto cleanup;
if (vshCommandOptBool(cmd, "keep-relative"))
flags |= VIR_DOMAIN_BLOCK_REBASE_RELATIVE;
+ /* This flag intentionally has the same value for both APIs */
+ if (bandwidthScale > 1)
+ flags |= VIR_DOMAIN_BLOCK_PULL_BANDWIDTH_BYTES;
+ if (base || flags)
+ rc = virDomainBlockRebase(dom, path, base, bandwidth, flags);
+ else
+ rc = virDomainBlockPull(dom, path, bandwidth, flags);
- if (base || flags) {
- if (virDomainBlockRebase(dom, path, base, bandwidth, flags) < 0)
- goto cleanup;
- } else {
- if (virDomainBlockPull(dom, path, bandwidth, 0) < 0)
- goto cleanup;
+ if (rc < 0 && bandwidthScale > 1 &&
+ last_error->code == VIR_ERR_INVALID_ARG) {
+ /* try again with MiB/s */
+ ctl->blockJobNoBytes = true;
+ flags &= ~VIR_DOMAIN_BLOCK_PULL_BANDWIDTH_BYTES;
+ bandwidth = VIR_DIV_UP(bandwidth, 1024 * 1024);
+ vshResetLibvirtError();
+ if (base || flags)
+ rc = virDomainBlockRebase(dom, path, base, bandwidth, flags);
+ else
+ rc = virDomainBlockPull(dom, path, bandwidth, flags);
}
-
+ if (rc < 0)
+ goto cleanup;
break;
case VSH_CMD_BLOCK_JOB_COMMIT:
if (vshCommandOptStringReq(ctl, cmd, "base", &base) < 0 ||
@@ -1583,7 +1631,19 @@ blockJobImpl(vshControl *ctl, const vshCmd *cmd,
flags |= VIR_DOMAIN_BLOCK_COMMIT_ACTIVE;
if (vshCommandOptBool(cmd, "keep-relative"))
flags |= VIR_DOMAIN_BLOCK_COMMIT_RELATIVE;
- if (virDomainBlockCommit(dom, path, base, top, bandwidth, flags) < 0)
+ if (bandwidthScale > 1)
+ flags |= VIR_DOMAIN_BLOCK_COMMIT_BANDWIDTH_BYTES;
+ rc = virDomainBlockCommit(dom, path, base, top, bandwidth, flags);
+ if (rc < 0 && bandwidthScale > 1 &&
+ last_error->code == VIR_ERR_INVALID_ARG) {
+ /* try again with MiB/s */
+ ctl->blockJobNoBytes = true;
+ flags &= ~VIR_DOMAIN_BLOCK_PULL_BANDWIDTH_BYTES;
+ bandwidth = VIR_DIV_UP(bandwidth, 1024 * 1024);
+ vshResetLibvirtError();
+ rc = virDomainBlockCommit(dom, path, base, top, bandwidth, flags);
+ }
+ if (rc < 0)
goto cleanup;
break;
}
@@ -1944,7 +2004,6 @@ cmdBlockCopy(vshControl *ctl, const vshCmd *cmd)
virDomainPtr dom = NULL;
const char *dest = NULL;
const char *format = NULL;
- unsigned long bandwidth = 0;
unsigned int granularity = 0;
unsigned long long buf_size = 0;
unsigned int flags = 0;
@@ -2010,8 +2069,6 @@ cmdBlockCopy(vshControl *ctl, const vshCmd *cmd)
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
goto cleanup;
- if (blockJobBandwidth(ctl, cmd, &bandwidth) < 0)
- goto cleanup;
if (vshCommandOptUInt(cmd, "granularity", &granularity) < 0) {
vshError(ctl, "%s", _("granularity must be a number"));
goto cleanup;
@@ -2040,22 +2097,18 @@ cmdBlockCopy(vshControl *ctl, const vshCmd *cmd)
if (granularity || buf_size || (format && STRNEQ(format, "raw")) || xml) {
/* New API */
- if (bandwidth || granularity || buf_size) {
+ unsigned long long bandwidthBytes = 0;
+ if (blockJobBandwidth(ctl, cmd, NULL, &bandwidthBytes) < 0)
+ goto cleanup;
+
+ if (bandwidthBytes || 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 */
- if (bandwidth + 0ULL > ULLONG_MAX >> 20) {
- virReportError(VIR_ERR_OVERFLOW,
- _("bandwidth must be less than %llu"),
- ULLONG_MAX >> 20);
- }
- if (virTypedParameterAssign(¶ms[nparams++],
- VIR_DOMAIN_BLOCK_COPY_BANDWIDTH,
- VIR_TYPED_PARAM_ULLONG,
- bandwidth << 20ULL) < 0)
- goto cleanup;
- }
+ if (bandwidthBytes &&
+ virTypedParameterAssign(¶ms[nparams++],
+ VIR_DOMAIN_BLOCK_COPY_BANDWIDTH,
+ VIR_TYPED_PARAM_ULLONG,
+ bandwidthBytes) < 0)
+ goto cleanup;
if (granularity &&
virTypedParameterAssign(¶ms[nparams++],
VIR_DOMAIN_BLOCK_COPY_GRANULARITY,
@@ -2088,14 +2141,33 @@ cmdBlockCopy(vshControl *ctl, const vshCmd *cmd)
if (virDomainBlockCopy(dom, path, xmlstr, params, nparams, flags) < 0)
goto cleanup;
} else {
- /* Old API */
+ /* Old API, parse scaled bandwidth */
+ int rc;
+ unsigned long bandwidth = 0;
+ int bandwidthScale;
+
+ if ((bandwidthScale = blockJobBandwidth(ctl, cmd, &bandwidth,
+ NULL)) < 0)
+ goto cleanup;
flags |= VIR_DOMAIN_BLOCK_REBASE_COPY;
if (vshCommandOptBool(cmd, "blockdev"))
flags |= VIR_DOMAIN_BLOCK_REBASE_COPY_DEV;
if (STREQ_NULLABLE(format, "raw"))
flags |= VIR_DOMAIN_BLOCK_REBASE_COPY_RAW;
+ if (bandwidthScale > 1)
+ flags |= VIR_DOMAIN_BLOCK_REBASE_BANDWIDTH_BYTES;
- if (virDomainBlockRebase(dom, path, dest, bandwidth, flags) < 0)
+ rc = virDomainBlockRebase(dom, path, dest, bandwidth, flags);
+ if (rc < 0 && bandwidthScale > 1 &&
+ last_error->code == VIR_ERR_INVALID_ARG) {
+ /* try again with MiB/s */
+ ctl->blockJobNoBytes = true;
+ flags &= ~VIR_DOMAIN_BLOCK_REBASE_BANDWIDTH_BYTES;
+ bandwidth = VIR_DIV_UP(bandwidth, 1024 * 1024);
+ vshResetLibvirtError();
+ rc = virDomainBlockRebase(dom, path, dest, bandwidth, flags);
+ }
+ if (rc < 0)
goto cleanup;
}
--
1.9.3
More information about the libvir-list
mailing list