diff mbox

[v10,03/16] block: Use block_job_add_bdrv() in mirror_start_job()

Message ID dbfce11ca1a7e7bf55f7754623bd14958ca9ab77.1475757437.git.berto@igalia.com (mailing list archive)
State New, archived
Headers show

Commit Message

Alberto Garcia Oct. 6, 2016, 1:02 p.m. UTC
Use block_job_add_bdrv() instead of blocking all operations in
mirror_start_job() and unblocking them in mirror_exit().

Signed-off-by: Alberto Garcia <berto@igalia.com>
---
 block/mirror.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Comments

Kevin Wolf Oct. 10, 2016, 4:03 p.m. UTC | #1
Am 06.10.2016 um 15:02 hat Alberto Garcia geschrieben:
> Use block_job_add_bdrv() instead of blocking all operations in
> mirror_start_job() and unblocking them in mirror_exit().
> 
> Signed-off-by: Alberto Garcia <berto@igalia.com>

Compared to the old code, this unblocks BLOCK_OP_TYPE_DATAPLANE, i.e.
you can now run a dataplane device on a BDS used as the mirror target.

This means that the target could require a different AioContext than the
source, which we can't support. So it seems unlikely to me that we can
lift this restriction.

Kevin
Paolo Bonzini Oct. 11, 2016, 8:20 a.m. UTC | #2
On 10/10/2016 18:03, Kevin Wolf wrote:
>> > Use block_job_add_bdrv() instead of blocking all operations in
>> > mirror_start_job() and unblocking them in mirror_exit().
>> > 
>> > Signed-off-by: Alberto Garcia <berto@igalia.com>
> Compared to the old code, this unblocks BLOCK_OP_TYPE_DATAPLANE, i.e.
> you can now run a dataplane device on a BDS used as the mirror target.
> 
> This means that the target could require a different AioContext than the
> source, which we can't support. So it seems unlikely to me that we can
> lift this restriction.

Indeed, this is in the pipeline, but still quite far away!

Paolo
Alberto Garcia Oct. 11, 2016, 1:46 p.m. UTC | #3
On Mon 10 Oct 2016 06:03:41 PM CEST, Kevin Wolf wrote:

>> Use block_job_add_bdrv() instead of blocking all operations in
>> mirror_start_job() and unblocking them in mirror_exit().
>> 
>> Signed-off-by: Alberto Garcia <berto@igalia.com>
>
> Compared to the old code, this unblocks BLOCK_OP_TYPE_DATAPLANE, i.e.
> you can now run a dataplane device on a BDS used as the mirror target.
>
> This means that the target could require a different AioContext than
> the source, which we can't support. So it seems unlikely to me that we
> can lift this restriction.

Thanks, I'll fix it.

What happens if you run a dataplane on the source, though? That's
currently allowed as far as I'm aware. Wouldn't that have a similar
effect?

Berto
Kevin Wolf Oct. 11, 2016, 2:01 p.m. UTC | #4
Am 11.10.2016 um 15:46 hat Alberto Garcia geschrieben:
> On Mon 10 Oct 2016 06:03:41 PM CEST, Kevin Wolf wrote:
> 
> >> Use block_job_add_bdrv() instead of blocking all operations in
> >> mirror_start_job() and unblocking them in mirror_exit().
> >> 
> >> Signed-off-by: Alberto Garcia <berto@igalia.com>
> >
> > Compared to the old code, this unblocks BLOCK_OP_TYPE_DATAPLANE, i.e.
> > you can now run a dataplane device on a BDS used as the mirror target.
> >
> > This means that the target could require a different AioContext than
> > the source, which we can't support. So it seems unlikely to me that we
> > can lift this restriction.
> 
> Thanks, I'll fix it.
> 
> What happens if you run a dataplane on the source, though? That's
> currently allowed as far as I'm aware. Wouldn't that have a similar
> effect?

The block job takes care to put the target into the same dataplane
AioContext then. The job doesn't really care whether it works in the
main thread or a separate I/O thread, it just requires that it's a
single context, which is currently defined by the source.

Kevin
diff mbox

Patch

diff --git a/block/mirror.c b/block/mirror.c
index f9d1fec..9b5159f 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -526,7 +526,6 @@  static void mirror_exit(BlockJob *job, void *opaque)
         aio_context_release(replace_aio_context);
     }
     g_free(s->replaces);
-    bdrv_op_unblock_all(target_bs, s->common.blocker);
     blk_unref(s->target);
     block_job_completed(&s->common, data->ret);
     g_free(data);
@@ -965,7 +964,7 @@  static void mirror_start_job(const char *job_id, BlockDriverState *bs,
         return;
     }
 
-    bdrv_op_block_all(target, s->common.blocker);
+    block_job_add_bdrv(&s->common, target);
 
     s->common.co = qemu_coroutine_create(mirror_run, s);
     trace_mirror_start(bs, s, s->common.co, opaque);