[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] block: fix streaming/closing race
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [PATCH] block: fix streaming/closing race |
Date: |
Fri, 30 Mar 2012 08:19:21 +0100 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Thu, Mar 29, 2012 at 06:08:40PM +0200, Paolo Bonzini wrote:
> Streaming can issue I/O while qcow2_close is running. This causes the
> L2 caches to become very confused or, alternatively, could cause a
> segfault when the streaming coroutine is reentered after closing its
> block device. The fix is to cancel streaming jobs when closing their
> underlying device. The cancellation must be synchronous, so add a flag
> saying whether streaming has in-flight I/O.
>
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
> block.c | 14 ++++++++++++++
> block/stream.c | 4 +++-
> block_int.h | 2 ++
> 3 files changed, 19 insertions(+), 1 deletions(-)
>
> diff --git a/block.c b/block.c
> index b88ee90..1aab4bf 100644
> --- a/block.c
> +++ b/block.c
> @@ -813,6 +813,9 @@ unlink_and_fail:
> void bdrv_close(BlockDriverState *bs)
> {
> if (bs->drv) {
> + if (bs->job) {
> + block_job_cancel_sync(bs->job);
> + }
> if (bs == bs_snapshots) {
> bs_snapshots = NULL;
> }
> @@ -4069,3 +4072,14 @@ bool block_job_is_cancelled(BlockJob *job)
> {
> return job->cancelled;
> }
> +
> +void block_job_cancel_sync(BlockJob *job)
> +{
> + BlockDriverState *bs = job->bs;
> +
> + assert(bs->job == job);
> + block_job_cancel(job);
> + while (bs->job != NULL && bs->job->busy) {
It's not clear to me why we have a busy flag. Is canceling and waiting
for bs->job == NULL not enough? Perhaps you're trying to take a
shortcut and not wait until the job is fully stopped - it's not worth it
since the streaming block job does no significant work after detecting
cancelation.
Stefan