Re: [Qemu-devel] [PATCH v2 02/11] blockdev: acquire AioContext in do_qmp_query_block_jobs_one()

[Stefan Hajnoczi]

On Wed, Oct 22, 2014 at 01:10:45PM +0200, Max Reitz wrote:
> On 2014-10-21 at 13:03, Stefan Hajnoczi wrote:
> >Make sure that query-block-jobs acquires the BlockDriverState
> >AioContext so that the blockjob isn't running in another thread while we
> >access its state.
> >
> >Signed-off-by: Stefan Hajnoczi <stefa...@redhat.com>
> >Reviewed-by: Max Reitz <mre...@redhat.com>
> >---
> >  blockdev.c | 9 +++++++--
> >  1 file changed, 7 insertions(+), 2 deletions(-)
> >
> >diff --git a/blockdev.c b/blockdev.c
> >index 1c7dab4..fd55904 100644
> >--- a/blockdev.c
> >+++ b/blockdev.c
> >@@ -2649,14 +2649,19 @@ fail:
> >  static void do_qmp_query_block_jobs_one(void *opaque, BlockDriverState *bs)
> >  {
> >      BlockJobInfoList **prev = opaque;
> >-    BlockJob *job = bs->job;
> >+    AioContext *aio_context;
> >+
> >+    aio_context = bdrv_get_aio_context(bs);
> >+    aio_context_acquire(aio_context);
> >-    if (job) {
> >+    if (bs->job) {
> >          BlockJobInfoList *elem = g_new0(BlockJobInfoList, 1);
> >          elem->value = block_job_query(bs->job);
> >          (*prev)->next = elem;
> >          *prev = elem;
> >      }
> >+
> >+    aio_context_release(aio_context);
> >  }
> >  BlockJobInfoList *qmp_query_block_jobs(Error **errp)
> 
> As already said in my review for v1, this conflicts with Markus's patch
> "block: Eliminate bdrv_iterate(), use bdrv_next()"; it's still not in
> master, but by now the pull request has been sent so I guess it's up to you
> to resolve it. ;-)

Yes.  The conflict is small, here is the resolution:

diff --git a/blockdev.c b/blockdev.c
index 501473d..40fc5d6 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2628,12 +2628,18 @@ BlockJobInfoList *qmp_query_block_jobs(Error **errp)
     BlockDriverState *bs;
 
     for (bs = bdrv_next(NULL); bs; bs = bdrv_next(bs)) {
+        AioContext *aio_context = bdrv_get_aio_context(bs);
+
+        aio_context_acquire(aio_context);
+
         if (bs->job) {
             BlockJobInfoList *elem = g_new0(BlockJobInfoList, 1);
             elem->value = block_job_query(bs->job);
             *p_next = elem;
             p_next = &elem->next;
         }
+
+        aio_context_release(aio_context);
     }
 
     return head;

pgpf3q10fFIVl.pgp
Description: PGP signature