[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PULL 17/20] block: use safe iteration over AioContext noti
From: |
Stefan Hajnoczi |
Subject: |
[Qemu-devel] [PULL 17/20] block: use safe iteration over AioContext notifiers |
Date: |
Mon, 20 Jun 2016 15:05:28 +0100 |
It's possible that an AioContext notifier user was close to finishing
when .detach_aio_context() or .attached_aio_context() is called. In
that case they may call bdrv_remove_aio_context_notifier() during the
callback.
Use safe iteration to avoid crashing when the notifier list is modified
during iteration. We must not only handle the case where the current
aio notifier is removed during a callback but also the one where any
other aio notifier is removed.
The next patch adds an AioContext notifier for block jobs and they
really could be terminating just as .detach_aio_context() is invoked.
Signed-off-by: Stefan Hajnoczi <address@hidden>
Reviewed-by: Paolo Bonzini <address@hidden>
Reviewed-by: Fam Zheng <address@hidden>
Message-id: address@hidden
---
block.c | 46 ++++++++++++++++++++++++++++++++++++----------
include/block/block_int.h | 2 ++
2 files changed, 38 insertions(+), 10 deletions(-)
diff --git a/block.c b/block.c
index b331eb9..c0ccc27 100644
--- a/block.c
+++ b/block.c
@@ -3609,18 +3609,34 @@ AioContext *bdrv_get_aio_context(BlockDriverState *bs)
return bs->aio_context;
}
+static void bdrv_do_remove_aio_context_notifier(BdrvAioNotifier *ban)
+{
+ QLIST_REMOVE(ban, list);
+ g_free(ban);
+}
+
void bdrv_detach_aio_context(BlockDriverState *bs)
{
- BdrvAioNotifier *baf;
+ BdrvAioNotifier *baf, *baf_tmp;
BdrvChild *child;
if (!bs->drv) {
return;
}
- QLIST_FOREACH(baf, &bs->aio_notifiers, list) {
- baf->detach_aio_context(baf->opaque);
+ assert(!bs->walking_aio_notifiers);
+ bs->walking_aio_notifiers = true;
+ QLIST_FOREACH_SAFE(baf, &bs->aio_notifiers, list, baf_tmp) {
+ if (baf->deleted) {
+ bdrv_do_remove_aio_context_notifier(baf);
+ } else {
+ baf->detach_aio_context(baf->opaque);
+ }
}
+ /* Never mind iterating again to check for ->deleted. bdrv_close() will
+ * remove remaining aio notifiers if we aren't called again.
+ */
+ bs->walking_aio_notifiers = false;
if (bs->drv->bdrv_detach_aio_context) {
bs->drv->bdrv_detach_aio_context(bs);
@@ -3635,7 +3651,7 @@ void bdrv_detach_aio_context(BlockDriverState *bs)
void bdrv_attach_aio_context(BlockDriverState *bs,
AioContext *new_context)
{
- BdrvAioNotifier *ban;
+ BdrvAioNotifier *ban, *ban_tmp;
BdrvChild *child;
if (!bs->drv) {
@@ -3651,9 +3667,16 @@ void bdrv_attach_aio_context(BlockDriverState *bs,
bs->drv->bdrv_attach_aio_context(bs, new_context);
}
- QLIST_FOREACH(ban, &bs->aio_notifiers, list) {
- ban->attached_aio_context(new_context, ban->opaque);
+ assert(!bs->walking_aio_notifiers);
+ bs->walking_aio_notifiers = true;
+ QLIST_FOREACH_SAFE(ban, &bs->aio_notifiers, list, ban_tmp) {
+ if (ban->deleted) {
+ bdrv_do_remove_aio_context_notifier(ban);
+ } else {
+ ban->attached_aio_context(new_context, ban->opaque);
+ }
}
+ bs->walking_aio_notifiers = false;
}
void bdrv_set_aio_context(BlockDriverState *bs, AioContext *new_context)
@@ -3695,11 +3718,14 @@ void bdrv_remove_aio_context_notifier(BlockDriverState
*bs,
QLIST_FOREACH_SAFE(ban, &bs->aio_notifiers, list, ban_next) {
if (ban->attached_aio_context == attached_aio_context &&
ban->detach_aio_context == detach_aio_context &&
- ban->opaque == opaque)
+ ban->opaque == opaque &&
+ ban->deleted == false)
{
- QLIST_REMOVE(ban, list);
- g_free(ban);
-
+ if (bs->walking_aio_notifiers) {
+ ban->deleted = true;
+ } else {
+ bdrv_do_remove_aio_context_notifier(ban);
+ }
return;
}
}
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 688c6be..2057156 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -361,6 +361,7 @@ typedef struct BdrvAioNotifier {
void (*detach_aio_context)(void *opaque);
void *opaque;
+ bool deleted;
QLIST_ENTRY(BdrvAioNotifier) list;
} BdrvAioNotifier;
@@ -427,6 +428,7 @@ struct BlockDriverState {
* BDS may register themselves in this list to be notified of changes
* regarding this BDS's context */
QLIST_HEAD(, BdrvAioNotifier) aio_notifiers;
+ bool walking_aio_notifiers; /* to make removal during iteration safe */
char filename[PATH_MAX];
char backing_file[PATH_MAX]; /* if non zero, the image is a diff of
--
2.5.5
- [Qemu-devel] [PULL 08/20] libqos: drop duplicated virtio_pci.h definitions, (continued)
- [Qemu-devel] [PULL 08/20] libqos: drop duplicated virtio_pci.h definitions, Stefan Hajnoczi, 2016/06/20
- [Qemu-devel] [PULL 07/20] libqos: drop duplicated virtio_scsi.h definitions, Stefan Hajnoczi, 2016/06/20
- [Qemu-devel] [PULL 09/20] libqos: add qvirtqueue_cleanup(), Stefan Hajnoczi, 2016/06/20
- [Qemu-devel] [PULL 11/20] block: fix race in bdrv_co_discard with drive-mirror, Stefan Hajnoczi, 2016/06/20
- [Qemu-devel] [PULL 10/20] block: fixed BdrvTrackedRequest filling in bdrv_co_discard, Stefan Hajnoczi, 2016/06/20
- [Qemu-devel] [PULL 12/20] block: process before_write_notifiers in bdrv_co_discard, Stefan Hajnoczi, 2016/06/20
- [Qemu-devel] [PULL 15/20] blockjob: add pause points, Stefan Hajnoczi, 2016/06/20
- [Qemu-devel] [PULL 14/20] blockjob: rename block_job_is_paused(), Stefan Hajnoczi, 2016/06/20
- [Qemu-devel] [PULL 13/20] blockjob: move iostatus reset out of block_job_enter(), Stefan Hajnoczi, 2016/06/20
- [Qemu-devel] [PULL 16/20] blockjob: add block_job_get_aio_context(), Stefan Hajnoczi, 2016/06/20
- [Qemu-devel] [PULL 17/20] block: use safe iteration over AioContext notifiers,
Stefan Hajnoczi <=
- [Qemu-devel] [PULL 18/20] blockjob: add AioContext attached callback, Stefan Hajnoczi, 2016/06/20
- [Qemu-devel] [PULL 20/20] backup: follow AioContext change gracefully, Stefan Hajnoczi, 2016/06/20
- [Qemu-devel] [PULL 19/20] mirror: follow AioContext change gracefully, Stefan Hajnoczi, 2016/06/20
- Re: [Qemu-devel] [PULL 00/20] Block patches, Peter Maydell, 2016/06/20