On 04/06/2012 02:29 PM, Eric Blake wrote:
From: Adam Litke <agl(a)us.ibm.com>
Without the VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC flag, libvirt will internally poll
using qemu's "query-block-jobs" API and will not return until the operation
has
been completed. API users are advised that this operation is unbounded and
further interaction with the domain during this period may block. Future
patches may refactor things to allow other queries in parallel with this
polling. Unfortunately, there's no good way to tell if qemu will emit the
new event, so this implementation always polls to deal with older qemu.
Signed-off-by: Adam Litke <agl(a)us.ibm.com>
Cc: Stefan Hajnoczi <stefanha(a)gmail.com>
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
src/qemu/qemu_driver.c | 55 +++++++++++++++++++++++++++++++++++++++++------
1 files changed, 48 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d9e35be..5f0eb05 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -11599,7 +11599,7 @@ cleanup:
static int
qemuDomainBlockJobImpl(virDomainPtr dom, const char *path, const char *base,
unsigned long bandwidth, virDomainBlockJobInfoPtr info,
- int mode)
+ int mode, unsigned int flags)
{
struct qemud_driver *driver = dom->conn->privateData;
virDomainObjPtr vm = NULL;
@@ -11641,6 +11641,45 @@ qemuDomainBlockJobImpl(virDomainPtr dom, const char *path, const
char *base,
ret = qemuMonitorBlockJob(priv->mon, device, base, bandwidth, info, mode);
qemuDomainObjExitMonitorWithDriver(driver, vm);
Do I understand correctly that qemu's abort was always asynchronous, and
prior to this, an abort call to libvirt would erroneously return
immediately? (I'm still trying to understand the code...)
+ /* Qemu provides asynchronous block job cancellation, but without
+ * the VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC flag libvirt guarantees a
+ * synchronous operation. Provide this behavior by waiting here,
+ * so we don't get confused by newly scheduled block jobs.
+ */
+ if (ret == 0 && mode == BLOCK_JOB_ABORT &&
+ !(flags & VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC)) {
+ ret = 1;
Why is ret set to 1? It will be assigned the return value of
qemuMonitorBlockJob before it's ever used for anything, so this seems
unnecessary.
+ while (1) {
+ /* Poll every 50ms */
+ struct timespec ts = { .tv_sec = 0,
+ .tv_nsec = 50 * 1000 * 1000ull };
This timespec could just as well be static const, couldn't it? No big
deal one way or the other, though.
+ virDomainBlockJobInfo dummy;
+
+ qemuDomainObjEnterMonitorWithDriver(driver, vm);
+ ret = qemuMonitorBlockJob(priv->mon, device, NULL, 0, &dummy,
+ BLOCK_JOB_INFO);
+ qemuDomainObjExitMonitorWithDriver(driver, vm);
+
+ if (ret <= 0)
+ break;
+
+ virDomainObjUnlock(vm);
+ qemuDriverUnlock(driver);
+
+ nanosleep(&ts, NULL);
+
+ qemuDriverLock(driver);
+ virDomainObjLock(vm);
+
+ if (!virDomainObjIsActive(vm)) {
+ qemuReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("domain is not running"));
+ ret = -1;
+ break;
+ }
+ }
+ }
+
endjob:
if (qemuDomainObjEndJob(driver, vm) == 0) {
vm = NULL;
@@ -11658,8 +11697,9 @@ cleanup:
static int
qemuDomainBlockJobAbort(virDomainPtr dom, const char *path, unsigned int flags)
{
- virCheckFlags(0, -1);
- return qemuDomainBlockJobImpl(dom, path, NULL, 0, NULL, BLOCK_JOB_ABORT);
+ virCheckFlags(VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC, -1);
+ return qemuDomainBlockJobImpl(dom, path, NULL, 0, NULL, BLOCK_JOB_ABORT,
+ flags);
}
static int
@@ -11667,7 +11707,8 @@ qemuDomainGetBlockJobInfo(virDomainPtr dom, const char *path,
virDomainBlockJobInfoPtr info, unsigned int flags)
{
virCheckFlags(0, -1);
- return qemuDomainBlockJobImpl(dom, path, NULL, 0, info, BLOCK_JOB_INFO);
+ return qemuDomainBlockJobImpl(dom, path, NULL, 0, info, BLOCK_JOB_INFO,
+ flags);
}
static int
@@ -11676,7 +11717,7 @@ qemuDomainBlockJobSetSpeed(virDomainPtr dom, const char *path,
{
virCheckFlags(0, -1);
return qemuDomainBlockJobImpl(dom, path, NULL, bandwidth, NULL,
- BLOCK_JOB_SPEED);
+ BLOCK_JOB_SPEED, flags);
}
static int
@@ -11687,10 +11728,10 @@ qemuDomainBlockRebase(virDomainPtr dom, const char *path, const
char *base,
virCheckFlags(0, -1);
ret = qemuDomainBlockJobImpl(dom, path, base, bandwidth, NULL,
- BLOCK_JOB_PULL);
+ BLOCK_JOB_PULL, flags);
if (ret == 0 && bandwidth != 0)
ret = qemuDomainBlockJobImpl(dom, path, NULL, bandwidth, NULL,
- BLOCK_JOB_SPEED);
+ BLOCK_JOB_SPEED, flags);
return ret;
}
ACK once the items above are addressed (either by explaining, changing
code, or telling me why I'm wrong)