@@ -246,6 +246,13 @@ static void backup_abort(BlockJob *job)
static void backup_clean(BlockJob *job)
{
BackupBlockJob *s = container_of(job, BackupBlockJob, common);
+
+ /* Ensure that no I/O is using the notifier anymore before freeing
+ * the bitmap and the job.
+ */
+ blk_drain(job->blk);
+ g_free(s->done_bitmap);
+
assert(s->target);
blk_unref(s->target);
s->target = NULL;
@@ -526,12 +533,14 @@ static void coroutine_fn backup_run(void *opaque)
}
}
- notifier_with_return_remove(&job->before_write);
-
- /* wait until pending backup_do_cow() calls have completed */
+ /* At this point, all future invocations of the write notifier will
+ * find a 1 in the done_bitmap, but we still have to wait for pending
+ * backup_do_cow() calls to complete.
+ */
qemu_co_rwlock_wrlock(&job->flush_rwlock);
qemu_co_rwlock_unlock(&job->flush_rwlock);
- g_free(job->done_bitmap);
+
+ bdrv_remove_before_write_notifier(bs, &job->before_write);
data = g_malloc(sizeof(*data));
data->ret = ret;
@@ -2499,10 +2499,22 @@ bool bdrv_qiov_is_aligned(BlockDriverState *bs, QEMUIOVector *qiov)
return true;
}
+static QemuSpin notifiers_spin_lock;
+
void bdrv_add_before_write_notifier(BlockDriverState *bs,
NotifierWithReturn *notifier)
{
+ qemu_spin_lock(¬ifiers_spin_lock);
notifier_with_return_list_add(&bs->before_write_notifiers, notifier);
+ qemu_spin_unlock(¬ifiers_spin_lock);
+}
+
+void bdrv_remove_before_write_notifier(BlockDriverState *bs,
+ NotifierWithReturn *notifier)
+{
+ qemu_spin_lock(¬ifiers_spin_lock);
+ notifier_with_return_remove(notifier);
+ qemu_spin_unlock(¬ifiers_spin_lock);
}
void bdrv_io_plug(BlockDriverState *bs)
@@ -32,7 +32,7 @@ bool bdrv_write_threshold_is_set(const BlockDriverState *bs)
static void write_threshold_disable(BlockDriverState *bs)
{
if (bdrv_write_threshold_is_set(bs)) {
- notifier_with_return_remove(&bs->write_threshold_notifier);
+ bdrv_remove_before_write_notifier(bs, &bs->write_threshold_notifier);
bs->write_threshold_offset = 0;
}
}
@@ -707,6 +707,8 @@ void bdrv_parse_filename_strip_prefix(const char *filename, const char *prefix,
/**
* bdrv_add_before_write_notifier:
+ * @bs: The #BlockDriverState for which to register the notifier
+ * @notifier: The notifier to add
*
* Register a callback that is invoked before write requests are processed but
* after any throttling or waiting for overlapping requests.
@@ -715,6 +717,20 @@ void bdrv_add_before_write_notifier(BlockDriverState *bs,
NotifierWithReturn *notifier);
/**
+ * bdrv_remove_before_write_notifier:
+ * @bs: The #BlockDriverState for which to register the notifier
+ * @notifier: The notifier to add
+ *
+ * Unregister a callback that is invoked before write requests are processed but
+ * after any throttling or waiting for overlapping requests.
+ *
+ * Note that more I/O could be pending on @bs. You need to wait for
+ * it to finish, for example with bdrv_drain(), before freeing @notifier.
+ */
+void bdrv_remove_before_write_notifier(BlockDriverState *bs,
+ NotifierWithReturn *notifier);
+
+/**
* bdrv_detach_aio_context:
*
* May be called from .bdrv_detach_aio_context() to detach children from the
Reads access the list in RCU style, so be careful to avoid use-after-free scenarios in the backup block job. Apart from this, all that's needed is protecting updates with a mutex. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- block/backup.c | 17 +++++++++++++---- block/io.c | 12 ++++++++++++ block/write-threshold.c | 2 +- include/block/block_int.h | 16 ++++++++++++++++ 4 files changed, 42 insertions(+), 5 deletions(-)