[libvirt] [PATCH] qemu: don't modify domain on failed blockiotune
If you have a qemu build that lacks the blockio tune monitor command, then this command: $ virsh blkdeviotune rhel6u2 hda --total_bytes_sec 1000 error: Unable to change block I/O throttle error: internal error Unexpected error fails as expected (well, the error message is lousy), but the next dumpxml shows that the domain was modified anyway. Worse, that means if you save the domain then restore it, the restore will likely fail due to throttling being unsupported, even though no throttling should even be active because the monitor command failed in the first place. * src/qemu/qemu_driver.c (qemuDomainSetBlockIoTune): Check for error before making modification permanent. --- src/qemu/qemu_driver.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index a36c348..2fde15b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12011,9 +12011,9 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, qemuDomainObjEnterMonitorWithDriver(driver, vm); ret = qemuMonitorSetBlockIoThrottle(priv->mon, device, &info); qemuDomainObjExitMonitorWithDriver(driver, vm); - vm->def->disks[idx]->blkdeviotune = info; if (ret < 0) goto endjob; + vm->def->disks[idx]->blkdeviotune = info; } if (flags & VIR_DOMAIN_AFFECT_CONFIG) { -- 1.7.7.6
Commit 4c82f09e added a capability check for qemu per-device io throttling, but only applied it to domain startup. As mentioned in the previous commit, we stil end up with an internal error if the monitor command doesn't also exist, not to mention the confusion of allowing tuning on inactive domains only to then be rejected when starting the domain. * src/qemu/qemu_driver.c (qemuDomainSetBlockIoTune): Reject offline tuning if online can't match it. --- src/qemu/qemu_driver.c | 8 +++++++- 1 files changed, 7 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2fde15b..c100a1a 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -11936,6 +11936,13 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, _("no domain with matching uuid '%s'"), uuidstr); goto cleanup; } + priv = vm->privateData; + if (!qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DRIVE_IOTUNE)) { + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("block I/O throttling not supported with this " + "QEMU binary")); + goto cleanup; + } device = qemuDiskPathToAlias(vm, disk, &idx); if (!device) { @@ -12007,7 +12014,6 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, info.read_iops_sec = oldinfo->read_iops_sec; info.write_iops_sec = oldinfo->write_iops_sec; } - priv = vm->privateData; qemuDomainObjEnterMonitorWithDriver(driver, vm); ret = qemuMonitorSetBlockIoThrottle(priv->mon, device, &info); qemuDomainObjExitMonitorWithDriver(driver, vm); -- 1.7.7.6
On Fri, May 04, 2012 at 11:21:51 -0600, Eric Blake wrote:
Commit 4c82f09e added a capability check for qemu per-device io throttling, but only applied it to domain startup. As mentioned in the previous commit, we stil end up with an internal error
s/stil/still/
if the monitor command doesn't also exist, not to mention the confusion of allowing tuning on inactive domains only to then be rejected when starting the domain.
* src/qemu/qemu_driver.c (qemuDomainSetBlockIoTune): Reject offline tuning if online can't match it. --- src/qemu/qemu_driver.c | 8 +++++++- 1 files changed, 7 insertions(+), 1 deletions(-)
ACK Jirka
On 05/04/2012 04:04 PM, Jiri Denemark wrote:
On Fri, May 04, 2012 at 11:21:51 -0600, Eric Blake wrote:
Commit 4c82f09e added a capability check for qemu per-device io throttling, but only applied it to domain startup. As mentioned in the previous commit, we stil end up with an internal error
s/stil/still/
if the monitor command doesn't also exist, not to mention the confusion of allowing tuning on inactive domains only to then be rejected when starting the domain.
* src/qemu/qemu_driver.c (qemuDomainSetBlockIoTune): Reject offline tuning if online can't match it. --- src/qemu/qemu_driver.c | 8 +++++++- 1 files changed, 7 insertions(+), 1 deletions(-)
ACK
Thanks; series pushed. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Fri, May 04, 2012 at 09:52:44 -0600, Eric Blake wrote:
If you have a qemu build that lacks the blockio tune monitor command, then this command:
$ virsh blkdeviotune rhel6u2 hda --total_bytes_sec 1000 error: Unable to change block I/O throttle error: internal error Unexpected error
fails as expected (well, the error message is lousy), but the next dumpxml shows that the domain was modified anyway. Worse, that means if you save the domain then restore it, the restore will likely fail due to throttling being unsupported, even though no throttling should even be active because the monitor command failed in the first place.
* src/qemu/qemu_driver.c (qemuDomainSetBlockIoTune): Check for error before making modification permanent. --- src/qemu/qemu_driver.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
ACK Jirka
participants (2)
-
Eric Blake -
Jiri Denemark