11:02 p.m.
qemu treats blockjob bandwidth as a 64-bit number, in the units
of bytes/second. But we stupidly modeled block job bandwidth
after migration bandwidth, which in turn was an 'unsigned long'
and therefore subject to 32-bit vs. 64-bit interpretations, and
with a scale of MiB/s. Our code already has to convert between
the two scales, and report overflow as appropriate; although
this conversion currently lives in the monitor code.
On the bright side, our use of MiB/s means that even with a
32-bit unsigned long, we still have no problem representing a
bandwidth of 2GiB/s, which is starting to be more feasible as
10-gigabit or even faster interfaces are used. And once you
get past the physical speeds of existing interfaces, any larger
bandwidth number behaves the same - effectively unlimited.
But on the low side, the granularity of 1MiB/s tuning is rather
coarse. So the new virDomainBlockJob API decided to go with
a direct 64-bit bytes/sec number instead of the scaled number
that prior blockjob APIs had used. But there is no point in
rounding this number to MiB/s just to scale it back to bytes/s
for handing to qemu.
In order to make future code sharing possible between the old
virDomainBlockRebase and the new virDomainBlockCopy, this patch
moves the scaling and overflow detection into the driver code.
Several of the block job calls that can set speed are fed
through a common interface, so it was easier to adjust all block
jobs at once, for consistency.
* src/qemu/qemu_monitor.h (qemuMonitorBlockJob)
(qemuMonitorBlockCommit, qemuMonitorDriveMirror): Change
parameter type and scale.
* src/qemu/qemu_monitor.c (qemuMonitorBlockJob)
(qemuMonitorBlockCommit, qemuMonitorDriveMirror): Move scaling
and overflow detection...
* src/qemu/qemu_driver.c (qemuDomainBlockJobImpl)
(qemuDomainBlockRebase, qemuDomainBlockCommit): ...here.
(qemuDomainBlockCopy): Use bytes/sec.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
src/qemu/qemu_driver.c | 40 +++++++++++++++++++++++++++++---
src/qemu/qemu_monitor.c | 61 +++++++++++--------------------------------------
src/qemu/qemu_monitor.h | 6 ++---
3 files changed, 53 insertions(+), 54 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index f208ba0..e86ebf5 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -14989,6 +14989,8 @@ qemuDomainBlockPivot(virConnectPtr conn,
return ret;
}
+
+/* bandwidth in MiB/s per public API */
static int
qemuDomainBlockJobImpl(virDomainObjPtr vm,
virConnectPtr conn,
@@ -15011,6 +15013,7 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,
char *backingPath = NULL;
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
bool save = false;
+ unsigned long long speed = bandwidth;
if (!virDomainObjIsActive(vm)) {
virReportError(VIR_ERR_OPERATION_INVALID, "%s",
@@ -15122,9 +15125,18 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,
}
}
+ /* Convert bandwidth MiB to bytes */
+ if (speed > LLONG_MAX >> 20) {
+ virReportError(VIR_ERR_OVERFLOW,
+ _("bandwidth must be less than %llu"),
+ LLONG_MAX >> 20);
+ goto endjob;
+ }
+ speed <<= 20;
+
qemuDomainObjEnterMonitor(driver, vm);
ret = qemuMonitorBlockJob(priv->mon, device, basePath, backingPath,
- bandwidth, mode, async);
+ speed, mode, async);
qemuDomainObjExitMonitor(driver, vm);
if (ret < 0) {
if (mode == BLOCK_JOB_ABORT && disk->mirror)
@@ -15324,12 +15336,14 @@ qemuDomainBlockJobSetSpeed(virDomainPtr dom, const char *path,
BLOCK_JOB_SPEED, flags);
}
+
+/* bandwidth in bytes/s */
static int
qemuDomainBlockCopy(virDomainObjPtr vm,
virConnectPtr conn,
const char *path,
const char *dest, const char *format,
- unsigned long bandwidth, unsigned int flags)
+ unsigned long long bandwidth, unsigned int flags)
{
virQEMUDriverPtr driver = conn->privateData;
qemuDomainObjPrivatePtr priv;
@@ -15512,6 +15526,7 @@ qemuDomainBlockRebase(virDomainPtr dom, const char *path, const
char *base,
virDomainObjPtr vm;
const char *format = NULL;
int ret = -1;
+ unsigned long long speed = bandwidth;
virCheckFlags(VIR_DOMAIN_BLOCK_REBASE_SHALLOW |
VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT |
@@ -15535,6 +15550,15 @@ qemuDomainBlockRebase(virDomainPtr dom, const char *path, const
char *base,
if (flags & VIR_DOMAIN_BLOCK_REBASE_COPY_RAW)
format = "raw";
+ /* Convert bandwidth MiB to bytes */
+ if (speed > LLONG_MAX >> 20) {
+ virReportError(VIR_ERR_OVERFLOW,
+ _("bandwidth must be less than %llu"),
+ LLONG_MAX >> 20);
+ goto cleanup;
+ }
+ speed <<= 20;
+
/* XXX: If we are doing a shallow copy but not reusing an external
* file, we should attempt to pre-create the destination with a
* relative backing chain instead of qemu's default of absolute */
@@ -15600,6 +15624,7 @@ qemuDomainBlockCommit(virDomainPtr dom,
char *basePath = NULL;
char *backingPath = NULL;
virStorageSourcePtr mirror = NULL;
+ unsigned long long speed = bandwidth;
/* XXX Add support for COMMIT_DELETE */
virCheckFlags(VIR_DOMAIN_BLOCK_COMMIT_SHALLOW |
@@ -15630,6 +15655,15 @@ qemuDomainBlockCommit(virDomainPtr dom,
goto endjob;
}
+ /* Convert bandwidth MiB to bytes */
+ if (speed > LLONG_MAX >> 20) {
+ virReportError(VIR_ERR_OVERFLOW,
+ _("bandwidth must be less than %llu"),
+ LLONG_MAX >> 20);
+ goto endjob;
+ }
+ speed <<= 20;
+
device = qemuDiskPathToAlias(vm, path, &idx);
if (!device)
goto endjob;
@@ -15764,7 +15798,7 @@ qemuDomainBlockCommit(virDomainPtr dom,
qemuDomainObjEnterMonitor(driver, vm);
ret = qemuMonitorBlockCommit(priv->mon, device,
topPath, basePath, backingPath,
- bandwidth);
+ speed);
qemuDomainObjExitMonitor(driver, vm);
if (mirror) {
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 4493051..ef35e6a 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -3178,33 +3178,21 @@ qemuMonitorDiskSnapshot(qemuMonitorPtr mon, virJSONValuePtr
actions,
return ret;
}
-/* Start a drive-mirror block job. bandwidth is in MiB/sec. */
+/* Start a drive-mirror block job. bandwidth is in bytes/sec. */
int
qemuMonitorDriveMirror(qemuMonitorPtr mon,
const char *device, const char *file,
- const char *format, unsigned long bandwidth,
+ const char *format, unsigned long long bandwidth,
unsigned int flags)
{
int ret = -1;
- unsigned long long speed;
- VIR_DEBUG("mon=%p, device=%s, file=%s, format=%s, bandwidth=%ld, "
+ VIR_DEBUG("mon=%p, device=%s, file=%s, format=%s, bandwidth=%lld, "
"flags=%x",
mon, device, file, NULLSTR(format), bandwidth, flags);
- /* Convert bandwidth MiB to bytes - unfortunately the JSON QMP protocol is
- * limited to LLONG_MAX also for unsigned values */
- speed = bandwidth;
- if (speed > LLONG_MAX >> 20) {
- virReportError(VIR_ERR_OVERFLOW,
- _("bandwidth must be less than %llu"),
- LLONG_MAX >> 20);
- return -1;
- }
- speed <<= 20;
-
if (mon->json)
- ret = qemuMonitorJSONDriveMirror(mon, device, file, format, speed,
+ ret = qemuMonitorJSONDriveMirror(mon, device, file, format, bandwidth,
flags);
else
virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
@@ -3228,33 +3216,22 @@ qemuMonitorTransaction(qemuMonitorPtr mon, virJSONValuePtr
actions)
return ret;
}
-/* Start a block-commit block job. bandwidth is in MiB/sec. */
+/* Start a block-commit block job. bandwidth is in bytes/sec. */
int
qemuMonitorBlockCommit(qemuMonitorPtr mon, const char *device,
const char *top, const char *base,
const char *backingName,
- unsigned long bandwidth)
+ unsigned long long bandwidth)
{
int ret = -1;
- unsigned long long speed;
- VIR_DEBUG("mon=%p, device=%s, top=%s, base=%s, backingName=%s,
bandwidth=%lu",
+ VIR_DEBUG("mon=%p, device=%s, top=%s, base=%s, backingName=%s, "
+ "bandwidth=%llu",
mon, device, top, base, NULLSTR(backingName), bandwidth);
- /* Convert bandwidth MiB to bytes - unfortunately the JSON QMP protocol is
- * limited to LLONG_MAX also for unsigned values */
- speed = bandwidth;
- if (speed > LLONG_MAX >> 20) {
- virReportError(VIR_ERR_OVERFLOW,
- _("bandwidth must be less than %llu"),
- LLONG_MAX >> 20);
- return -1;
- }
- speed <<= 20;
-
if (mon->json)
ret = qemuMonitorJSONBlockCommit(mon, device, top, base,
- backingName, speed);
+ backingName, bandwidth);
else
virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
_("block-commit requires JSON monitor"));
@@ -3359,38 +3336,26 @@ int qemuMonitorScreendump(qemuMonitorPtr mon,
return ret;
}
-/* bandwidth is in MiB/sec */
+/* bandwidth is in bytes/sec */
int
qemuMonitorBlockJob(qemuMonitorPtr mon,
const char *device,
const char *base,
const char *backingName,
- unsigned long bandwidth,
+ unsigned long long bandwidth,
qemuMonitorBlockJobCmd mode,
bool modern)
{
int ret = -1;
- unsigned long long speed;
- VIR_DEBUG("mon=%p, device=%s, base=%s, backingName=%s, bandwidth=%luM, "
+ VIR_DEBUG("mon=%p, device=%s, base=%s, backingName=%s, bandwidth=%lluB, "
"mode=%o, modern=%d",
mon, device, NULLSTR(base), NULLSTR(backingName),
bandwidth, mode, modern);
- /* Convert bandwidth MiB to bytes - unfortunately the JSON QMP protocol is
- * limited to LLONG_MAX also for unsigned values */
- speed = bandwidth;
- if (speed > LLONG_MAX >> 20) {
- virReportError(VIR_ERR_OVERFLOW,
- _("bandwidth must be less than %llu"),
- LLONG_MAX >> 20);
- return -1;
- }
- speed <<= 20;
-
if (mon->json)
ret = qemuMonitorJSONBlockJob(mon, device, base, backingName,
- speed, mode, modern);
+ bandwidth, mode, modern);
else
virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
_("block jobs require JSON monitor"));
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 947ca34..35d9546 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -649,7 +649,7 @@ int qemuMonitorDriveMirror(qemuMonitorPtr mon,
const char *device,
const char *file,
const char *format,
- unsigned long bandwidth,
+ unsigned long long bandwidth,
unsigned int flags)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
int qemuMonitorDrivePivot(qemuMonitorPtr mon,
@@ -663,7 +663,7 @@ int qemuMonitorBlockCommit(qemuMonitorPtr mon,
const char *top,
const char *base,
const char *backingName,
- unsigned long bandwidth)
+ unsigned long long bandwidth)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
ATTRIBUTE_NONNULL(4);
bool qemuMonitorSupportsActiveCommit(qemuMonitorPtr mon);
@@ -693,7 +693,7 @@ int qemuMonitorBlockJob(qemuMonitorPtr mon,
const char *device,
const char *base,
const char *backingName,
- unsigned long bandwidth,
+ unsigned long long bandwidth,
qemuMonitorBlockJobCmd mode,
bool modern)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
--
1.9.3