[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PATCH 25/46] block: fix streaming/closing race
From: |
Kevin Wolf |
Subject: |
[Qemu-devel] [PATCH 25/46] block: fix streaming/closing race |
Date: |
Thu, 5 Apr 2012 17:52:03 +0200 |
From: Paolo Bonzini <address@hidden>
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, on the other hand qemu_aio_wait
will not restart a coroutine that is sleeping in co_sleep. So add
a flag saying whether streaming has in-flight I/O. If the busy flag
is false, the coroutine is quiescent and, when cancelled, will not
issue any new I/O.
This protects streaming against closing, but not against deleting.
We have a reference count protecting us against concurrent deletion,
but I still added an assertion to ensure nothing bad happens.
Signed-off-by: Paolo Bonzini <address@hidden>
Reviewed-by: Stefan Hajnoczi <address@hidden>
Signed-off-by: Kevin Wolf <address@hidden>
---
block.c | 16 ++++++++++++++++
block/stream.c | 6 ++++--
block_int.h | 2 ++
3 files changed, 22 insertions(+), 2 deletions(-)
diff --git a/block.c b/block.c
index 0344673..16e14fa 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;
}
@@ -966,6 +969,8 @@ void bdrv_append(BlockDriverState *bs_new, BlockDriverState
*bs_top)
void bdrv_delete(BlockDriverState *bs)
{
assert(!bs->dev);
+ assert(!bs->job);
+ assert(!bs->in_use);
/* remove from list, if necessary */
bdrv_make_anon(bs);
@@ -4095,3 +4100,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) {
+ qemu_aio_wait();
+ }
+}
diff --git a/block/stream.c b/block/stream.c
index d1b3986..f186bfd 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -175,7 +175,7 @@ retry:
break;
}
-
+ s->common.busy = true;
if (base) {
ret = is_allocated_base(bs, base, sector_num,
STREAM_BUFFER_SIZE / BDRV_SECTOR_SIZE, &n);
@@ -189,6 +189,7 @@ retry:
if (s->common.speed) {
uint64_t delay_ns = ratelimit_calculate_delay(&s->limit, n);
if (delay_ns > 0) {
+ s->common.busy = false;
co_sleep_ns(rt_clock, delay_ns);
/* Recheck cancellation and that sectors are unallocated */
@@ -208,6 +209,7 @@ retry:
/* Note that even when no rate limit is applied we need to yield
* with no pending I/O here so that qemu_aio_flush() returns.
*/
+ s->common.busy = false;
co_sleep_ns(rt_clock, 0);
}
@@ -215,7 +217,7 @@ retry:
bdrv_disable_copy_on_read(bs);
}
- if (sector_num == end && ret == 0) {
+ if (!block_job_is_cancelled(&s->common) && sector_num == end && ret == 0) {
const char *base_id = NULL;
if (base) {
base_id = s->backing_file_id;
diff --git a/block_int.h b/block_int.h
index 22c86a5..a96aabd 100644
--- a/block_int.h
+++ b/block_int.h
@@ -83,6 +83,7 @@ struct BlockJob {
const BlockJobType *job_type;
BlockDriverState *bs;
bool cancelled;
+ bool busy;
/* These fields are published by the query-block-jobs QMP API */
int64_t offset;
@@ -311,6 +312,7 @@ void block_job_complete(BlockJob *job, int ret);
int block_job_set_speed(BlockJob *job, int64_t value);
void block_job_cancel(BlockJob *job);
bool block_job_is_cancelled(BlockJob *job);
+void block_job_cancel_sync(BlockJob *job);
int stream_start(BlockDriverState *bs, BlockDriverState *base,
const char *base_id, BlockDriverCompletionFunc *cb,
--
1.7.6.5
- [Qemu-devel] [PATCH 22/46] Use DMADirection type for dma_bdrv_io, (continued)
- [Qemu-devel] [PATCH 22/46] Use DMADirection type for dma_bdrv_io, Kevin Wolf, 2012/04/05
- [Qemu-devel] [PATCH 43/46] qed: add bdrv_invalidate_cache to be called after incoming live migration, Kevin Wolf, 2012/04/05
- [Qemu-devel] [PATCH 37/46] qemu-iotests: Fix call syntax for qemu-img, Kevin Wolf, 2012/04/05
- [Qemu-devel] [PATCH 38/46] qemu-iotests: Fix call syntax for qemu-io, Kevin Wolf, 2012/04/05
- [Qemu-devel] [PATCH 28/46] qemu-img: add image fragmentation statistics, Kevin Wolf, 2012/04/05
- [Qemu-devel] [PATCH 21/46] vdi: change goto to loop, Kevin Wolf, 2012/04/05
- [Qemu-devel] [PATCH 32/46] block: bdrv_append() fixes, Kevin Wolf, 2012/04/05
- [Qemu-devel] [PATCH 13/46] qdev: add blocksize property type, Kevin Wolf, 2012/04/05
- [Qemu-devel] [PATCH 20/46] vdi: do not create useless iovecs, Kevin Wolf, 2012/04/05
- [Qemu-devel] [PATCH 42/46] blockdev: open images with BDRV_O_INCOMING on incoming live migration, Kevin Wolf, 2012/04/05
- [Qemu-devel] [PATCH 25/46] block: fix streaming/closing race,
Kevin Wolf <=
- [Qemu-devel] [PATCH 36/46] qemu-iotests: Test unknown qcow2 header extensions, Kevin Wolf, 2012/04/05
- [Qemu-devel] [PATCH 33/46] sheepdog: implement SD_OP_FLUSH_VDI operation, Kevin Wolf, 2012/04/05
- [Qemu-devel] [PATCH 11/46] block/vpc: write checksum back to footer after check, Kevin Wolf, 2012/04/05
- [Qemu-devel] [PATCH 15/46] vdi: basic conversion to coroutines, Kevin Wolf, 2012/04/05
- [Qemu-devel] [PATCH 09/46] ide: Change serial number strncpy() to pstrcpy(), Kevin Wolf, 2012/04/05
- [Qemu-devel] [PATCH 27/46] block: document job API, Kevin Wolf, 2012/04/05
- [Qemu-devel] [PATCH 19/46] vdi: leave bounce buffering to block layer, Kevin Wolf, 2012/04/05
- [Qemu-devel] [PATCH 29/46] qed: image fragmentation statistics, Kevin Wolf, 2012/04/05
- [Qemu-devel] [PATCH 14/46] block: enforce constraints on block size properties, Kevin Wolf, 2012/04/05
- [Qemu-devel] [PATCH 12/46] qerror: fix QERR_PROPERTY_VALUE_OUT_OF_RANGE description, Kevin Wolf, 2012/04/05