On 04.04.2016 15:43, Alberto Garcia wrote: > This makes sure that the image we are steaming into is open in > read-write mode during the operation. > > The block job is created on the destination image, but operation > blockers are also set on the active layer. We do this in order to > prevent other block jobs from running in parallel in the same chain. > See here for details on why that is currently not supported: > > [Qemu-block] RFC: Status of the intermediate block streaming work > https://lists.gnu.org/archive/html/qemu-block/2015-12/msg00180.html > > Finally, this also unblocks the stream operation in backing files. > > Signed-off-by: Alberto Garcia <[email protected]> > --- > block.c | 4 +++- > block/stream.c | 39 ++++++++++++++++++++++++++++++++++++++- > blockdev.c | 2 +- > include/block/block_int.h | 5 ++++- > 4 files changed, 46 insertions(+), 4 deletions(-) > > diff --git a/block.c b/block.c > index 48638c9..f7698a1 100644 > --- a/block.c > +++ b/block.c > @@ -1252,9 +1252,11 @@ void bdrv_set_backing_hd(BlockDriverState *bs, > BlockDriverState *backing_hd) > backing_hd->drv ? backing_hd->drv->format_name : ""); > > bdrv_op_block_all(backing_hd, bs->backing_blocker); > - /* Otherwise we won't be able to commit due to check in bdrv_commit */ > + /* Otherwise we won't be able to commit or stream */ > bdrv_op_unblock(backing_hd, BLOCK_OP_TYPE_COMMIT_TARGET, > bs->backing_blocker); > + bdrv_op_unblock(backing_hd, BLOCK_OP_TYPE_STREAM, > + bs->backing_blocker); > out: > bdrv_refresh_limits(bs, NULL); > } > diff --git a/block/stream.c b/block/stream.c > index 332b9a1..ac02ddf 100644 > --- a/block/stream.c > +++ b/block/stream.c > @@ -35,8 +35,10 @@ typedef struct StreamBlockJob { > BlockJob common; > RateLimit limit; > BlockDriverState *base; > + BlockDriverState *active; > BlockdevOnError on_error; > char *backing_file_str; > + int bs_flags; > } StreamBlockJob; > > static int coroutine_fn stream_populate(BlockDriverState *bs, > @@ -66,6 +68,11 @@ static void stream_complete(BlockJob *job, void *opaque) > StreamCompleteData *data = opaque; > BlockDriverState *base = s->base; > > + if (s->active) { > + bdrv_op_unblock_all(s->active, s->common.blocker); > + bdrv_unref(s->active); > + } > + > if (!block_job_is_cancelled(&s->common) && data->reached_end && > data->ret == 0) { > const char *base_id = NULL, *base_fmt = NULL; > @@ -79,6 +86,11 @@ static void stream_complete(BlockJob *job, void *opaque) > bdrv_set_backing_hd(job->bs, base); > } > > + /* Reopen the image back in read-only mode if necessary */ > + if (s->bs_flags != bdrv_get_flags(job->bs)) { > + bdrv_reopen(job->bs, s->bs_flags, NULL); > + } > + > g_free(s->backing_file_str); > block_job_completed(&s->common, data->ret); > g_free(data); > @@ -216,13 +228,15 @@ static const BlockJobDriver stream_job_driver = { > .set_speed = stream_set_speed, > }; > > -void stream_start(BlockDriverState *bs, BlockDriverState *base, > +void stream_start(BlockDriverState *bs, BlockDriverState *active, > + BlockDriverState *base, > const char *backing_file_str, int64_t speed, > BlockdevOnError on_error, > BlockCompletionFunc *cb, > void *opaque, Error **errp) > { > StreamBlockJob *s; > + int orig_bs_flags; > > if ((on_error == BLOCKDEV_ON_ERROR_STOP || > on_error == BLOCKDEV_ON_ERROR_ENOSPC) && > @@ -231,13 +245,36 @@ void stream_start(BlockDriverState *bs, > BlockDriverState *base, > return; > } > > + /* Make sure that the image is opened in read-write mode */ > + orig_bs_flags = bdrv_get_flags(bs); > + if (!(orig_bs_flags & BDRV_O_RDWR)) { > + if (bdrv_reopen(bs, orig_bs_flags | BDRV_O_RDWR, errp) != 0) { > + return; > + } > + } > + > s = block_job_create(&stream_job_driver, bs, speed, cb, opaque, errp); > if (!s) { > return; > } > > + /* If we are streaming to an intermediate image, we need to block > + * the active layer. Due to a race condition, having several block > + * jobs running in the same chain is broken and we currently don't > + * support it. See here for details: > + * https://lists.gnu.org/archive/html/qemu-block/2015-12/msg00180.html > + */ > + if (active) { > + bdrv_op_block_all(active, s->common.blocker);
block_job_create() unblocks BLOCK_OP_TYPE_DATAPLANE. Maybe this should
do the same?
No other objections from me.
Max
> + /* Hold a reference to the active layer, otherwise
> + * bdrv_close_all() will destroy it if we shut down the VM */
> + bdrv_ref(active);
> + }
> +
> s->base = base;
> + s->active = active;
> s->backing_file_str = g_strdup(backing_file_str);
> + s->bs_flags = orig_bs_flags;
>
> s->on_error = on_error;
> s->common.co = qemu_coroutine_create(stream_run);
> diff --git a/blockdev.c b/blockdev.c
> index d1f5dfb..2e7712e 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -3038,7 +3038,7 @@ void qmp_block_stream(const char *device,
> /* backing_file string overrides base bs filename */
> base_name = has_backing_file ? backing_file : base_name;
>
> - stream_start(bs, base_bs, base_name, has_speed ? speed : 0,
> + stream_start(bs, NULL, base_bs, base_name, has_speed ? speed : 0,
> on_error, block_job_cb, bs, &local_err);
> if (local_err) {
> error_propagate(errp, local_err);
> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index 10d8759..0a53d5f 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -597,6 +597,8 @@ int is_windows_drive(const char *filename);
> /**
> * stream_start:
> * @bs: Block device to operate on.
> + * @active: The active layer of the chain where @bs belongs, or %NULL
> + * if @bs is already the topmost device.
> * @base: Block device that will become the new base, or %NULL to
> * flatten the whole backing file chain onto @bs.
> * @base_id: The file name that will be written to @bs as the new
> @@ -613,7 +615,8 @@ int is_windows_drive(const char *filename);
> * streaming job, the backing file of @bs will be changed to
> * @base_id in the written image and to @base in the live BlockDriverState.
> */
> -void stream_start(BlockDriverState *bs, BlockDriverState *base,
> +void stream_start(BlockDriverState *bs, BlockDriverState *active,
> + BlockDriverState *base,
> const char *base_id, int64_t speed, BlockdevOnError
> on_error,
> BlockCompletionFunc *cb,
> void *opaque, Error **errp);
>
signature.asc
Description: OpenPGP digital signature
