From fa4478d5c8b74a5f0c8b93cc00590ec007be5016 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 8 May 2012 16:51:46 +0200 Subject: block: wait for job callback in block_job_cancel_sync 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 Signed-off-by: Kevin Wolf --- block_int.h | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) (limited to 'block_int.h') diff --git a/block_int.h b/block_int.h index 3bf23670b7..b80e66db6e 100644 --- a/block_int.h +++ b/block_int.h @@ -94,6 +94,12 @@ struct BlockJob { /** The block device on which the job is operating. */ 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 @@ -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: -- cgit v1.2.3