qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 05/11] blockjob: add block_job_defer_to_main_loo


From: Max Reitz
Subject: Re: [Qemu-devel] [PATCH 05/11] blockjob: add block_job_defer_to_main_loop()
Date: Wed, 01 Oct 2014 21:06:36 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.1.2

On 01.10.2014 19:01, Stefan Hajnoczi wrote:
Block jobs will run in the BlockDriverState's AioContext, which may not
always be the QEMU main loop.

There are some block layer APIs that are either not thread-safe or risk
lock ordering problems.  This includes bdrv_unref(), bdrv_close(), and
anything that calls bdrv_drain_all().

The block_job_defer_to_main_loop() API allows a block job to schedule a
function to run in the main loop with the BlockDriverState AioContext
held.

This function will be used to perform cleanup and backing chain
manipulations in block jobs.

Signed-off-by: Stefan Hajnoczi <address@hidden>
---
  blockjob.c               | 35 +++++++++++++++++++++++++++++++++++
  include/block/blockjob.h | 19 +++++++++++++++++++
  2 files changed, 54 insertions(+)

diff --git a/blockjob.c b/blockjob.c
index 0689fdd..24a64d8 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -313,3 +313,38 @@ BlockErrorAction block_job_error_action(BlockJob *job, 
BlockDriverState *bs,
      }
      return action;
  }
+
+typedef struct {
+    BlockJob *job;
+    QEMUBH *bh;
+    AioContext *aio_context;
+    BlockJobDeferToMainLoopFn *fn;
+    void *opaque;
+} BlockJobDeferToMainLoopData;
+
+static void block_job_defer_to_main_loop_bh(void *opaque)
+{
+    BlockJobDeferToMainLoopData *data = opaque;
+
+    qemu_bh_delete(data->bh);
+
+    aio_context_acquire(data->aio_context);
+    data->fn(data->job, data->opaque);
+    aio_context_release(data->aio_context);
+
+    g_free(data);
+}
+
+void block_job_defer_to_main_loop(BlockJob *job,
+                                  BlockJobDeferToMainLoopFn *fn,
+                                  void *opaque)
+{
+    BlockJobDeferToMainLoopData *data = g_malloc(sizeof(*data));
+    data->job = job;
+    data->bh = qemu_bh_new(block_job_defer_to_main_loop_bh, data);
+    data->aio_context = bdrv_get_aio_context(job->bs);
+    data->fn = fn;
+    data->opaque = opaque;
+
+    qemu_bh_schedule(data->bh);
+}

I'm not so sure whether it'd be possible to change the BDS's AIO context (in another thread) after the call to bdrv_get_aio_context() in block_job_defer_to_main_loop() and before block_job_defer_to_main_loop_bh() is run. If so, this might create race conditions.

Other than that, this patch looks good.

Max



reply via email to

[Prev in Thread] Current Thread [Next in Thread]