[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PATCH 16/30] block: wait for job callback in block_job_can
From: |
Kevin Wolf |
Subject: |
[Qemu-devel] [PATCH 16/30] block: wait for job callback in block_job_cancel_sync |
Date: |
Thu, 10 May 2012 13:49:20 +0200 |
From: Paolo Bonzini <address@hidden>
The limitation on not having I/O after cancellation cannot really be
kept. Even streaming has a very small race window where you could
cancel a job and have it report completion. If this window is hit,
bdrv_change_backing_file() will yield and possibly cause accesses to
dangling pointers etc.
So, let's just assume that we cannot know exactly what will happen
after the coroutine has set busy to false. We can set a very lax
condition:
- if we cancel the job, the coroutine won't set it to false again
(and hence will not call co_sleep_ns again).
- block_job_cancel_sync will wait for the coroutine to exit, which
pretty much ensures no race.
Instead, we track the coroutine that executes the job and put very
strict conditions on what to do while it is quiescent (busy = false).
First of all, the coroutine must never set busy = false while the job
has been cancelled. Second, the coroutine can be reentered arbitrarily
while it is quiescent, so you cannot really do anything but co_sleep_ns at
that time. This condition is obeyed by the block_job_sleep_ns function.
Signed-off-by: Paolo Bonzini <address@hidden>
Signed-off-by: Kevin Wolf <address@hidden>
---
block.c | 36 ++++++++++++++++++++++++++++++++++--
block/stream.c | 7 +++----
block_int.h | 11 ++++++++++-
3 files changed, 47 insertions(+), 7 deletions(-)
diff --git a/block.c b/block.c
index f9a11aa..2978135 100644
--- a/block.c
+++ b/block.c
@@ -4238,6 +4238,9 @@ void block_job_set_speed(BlockJob *job, int64_t speed,
Error **errp)
void block_job_cancel(BlockJob *job)
{
job->cancelled = true;
+ if (job->co && !job->busy) {
+ qemu_coroutine_enter(job->co, NULL);
+ }
}
bool block_job_is_cancelled(BlockJob *job)
@@ -4245,15 +4248,44 @@ bool block_job_is_cancelled(BlockJob *job)
return job->cancelled;
}
-void block_job_cancel_sync(BlockJob *job)
+struct BlockCancelData {
+ BlockJob *job;
+ BlockDriverCompletionFunc *cb;
+ void *opaque;
+ bool cancelled;
+ int ret;
+};
+
+static void block_job_cancel_cb(void *opaque, int ret)
{
+ struct BlockCancelData *data = opaque;
+
+ data->cancelled = block_job_is_cancelled(data->job);
+ data->ret = ret;
+ data->cb(data->opaque, ret);
+}
+
+int block_job_cancel_sync(BlockJob *job)
+{
+ struct BlockCancelData data;
BlockDriverState *bs = job->bs;
assert(bs->job == job);
+
+ /* Set up our own callback to store the result and chain to
+ * the original callback.
+ */
+ data.job = job;
+ data.cb = job->cb;
+ data.opaque = job->opaque;
+ data.ret = -EINPROGRESS;
+ job->cb = block_job_cancel_cb;
+ job->opaque = &data;
block_job_cancel(job);
- while (bs->job != NULL && bs->job->busy) {
+ while (data.ret == -EINPROGRESS) {
qemu_aio_wait();
}
+ return (data.cancelled && data.ret == 0) ? -ECANCELED : data.ret;
}
void block_job_sleep_ns(BlockJob *job, QEMUClock *clock, int64_t ns)
diff --git a/block/stream.c b/block/stream.c
index b279acd..930e3cf 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -270,7 +270,6 @@ void stream_start(BlockDriverState *bs, BlockDriverState
*base,
void *opaque, Error **errp)
{
StreamBlockJob *s;
- Coroutine *co;
s = block_job_create(&stream_job_type, bs, speed, cb, opaque, errp);
if (!s) {
@@ -282,7 +281,7 @@ void stream_start(BlockDriverState *bs, BlockDriverState
*base,
pstrcpy(s->backing_file_id, sizeof(s->backing_file_id), base_id);
}
- co = qemu_coroutine_create(stream_run);
- trace_stream_start(bs, base, s, co, opaque);
- qemu_coroutine_enter(co, s);
+ s->common.co = qemu_coroutine_create(stream_run);
+ trace_stream_start(bs, base, s, s->common.co, opaque);
+ qemu_coroutine_enter(s->common.co, s);
}
diff --git a/block_int.h b/block_int.h
index 3bf2367..b80e66d 100644
--- a/block_int.h
+++ b/block_int.h
@@ -95,6 +95,12 @@ struct BlockJob {
BlockDriverState *bs;
/**
+ * The coroutine that executes the job. If not NULL, it is
+ * reentered when busy is false and the job is cancelled.
+ */
+ Coroutine *co;
+
+ /**
* Set to true if the job should cancel itself. The flag must
* always be tested just before toggling the busy flag from false
* to true. After a job has been cancelled, it should only yield
@@ -418,8 +424,11 @@ bool block_job_is_cancelled(BlockJob *job);
* immediately after #block_job_cancel_sync. Users of block jobs
* will usually protect the BlockDriverState objects with a reference
* count, should this be a concern.
+ *
+ * Returns the return value from the job if the job actually completed
+ * during the call, or -ECANCELED if it was canceled.
*/
-void block_job_cancel_sync(BlockJob *job);
+int block_job_cancel_sync(BlockJob *job);
/**
* stream_start:
--
1.7.6.5
- [Qemu-devel] [PATCH 10/30] qtest: Add floppy test, (continued)
- [Qemu-devel] [PATCH 10/30] qtest: Add floppy test, Kevin Wolf, 2012/05/10
- [Qemu-devel] [PATCH 06/30] block: open backing file as read-only when probing for size, Kevin Wolf, 2012/05/10
- [Qemu-devel] [PATCH 09/30] qtest: Add function to send QMP commands, Kevin Wolf, 2012/05/10
- [Qemu-devel] [PATCH 12/30] block: another bdrv_append fix, Kevin Wolf, 2012/05/10
- [Qemu-devel] [PATCH 07/30] block: fix allocation size for dirty bitmap, Kevin Wolf, 2012/05/10
- [Qemu-devel] [PATCH 02/30] block: add mode argument to blockdev-snapshot-sync, Kevin Wolf, 2012/05/10
- [Qemu-devel] [PATCH 05/30] block: update in-memory backing file and format, Kevin Wolf, 2012/05/10
- [Qemu-devel] [PATCH 18/30] block: protect path_has_protocol from filenames with colons, Kevin Wolf, 2012/05/10
- [Qemu-devel] [PATCH 11/30] block: fix snapshot on QED, Kevin Wolf, 2012/05/10
- [Qemu-devel] [PATCH 15/30] block: add block_job_sleep_ns, Kevin Wolf, 2012/05/10
- [Qemu-devel] [PATCH 16/30] block: wait for job callback in block_job_cancel_sync,
Kevin Wolf <=
- [Qemu-devel] [PATCH 23/30] stream: fix sectors not allocated test, Kevin Wolf, 2012/05/10
- [Qemu-devel] [PATCH 19/30] block: move field reset from bdrv_open_common to bdrv_close, Kevin Wolf, 2012/05/10
- [Qemu-devel] [PATCH 26/30] stream: fix HMP block_job_set_speed, Kevin Wolf, 2012/05/10
- [Qemu-devel] [PATCH 27/30] stream: fix ratelimiting corner case, Kevin Wolf, 2012/05/10
- [Qemu-devel] [PATCH 22/30] qemu-io: fix the alloc command, Kevin Wolf, 2012/05/10
- [Qemu-devel] [PATCH 24/30] stream: add testcase for partial streaming, Kevin Wolf, 2012/05/10
- [Qemu-devel] [PATCH 25/30] stream: pass new base image format to bdrv_change_backing_file, Kevin Wolf, 2012/05/10
- [Qemu-devel] [PATCH 28/30] stream: do not copy unallocated sectors from the base, Kevin Wolf, 2012/05/10
- [Qemu-devel] [PATCH 29/30] tests/Makefile: Add missing $(EXESUF), Kevin Wolf, 2012/05/10
- [Qemu-devel] [PATCH 30/30] declare ECANCELED on all machines, Kevin Wolf, 2012/05/10