On Wed, 05/13 16:27, Alberto Garcia wrote:
> This makes sure that the image we are steaming into is open in
> read-write mode during the operation.
>
> Operation blockers are also set in all intermediate nodes, since they
> will be removed from the chain afterwards.
>
> Finally, this also unblocks the stream operation in backing files.
>
> Signed-off-by: Alberto Garcia <be...@igalia.com>
> Reviewed-by: Max Reitz <mre...@redhat.com>
> ---
> block.c | 4 +++-
> block/stream.c | 33 +++++++++++++++++++++++++++++++++
> 2 files changed, 36 insertions(+), 1 deletion(-)
>
> diff --git a/block.c b/block.c
> index 7904098..8fab7e4 100644
> --- a/block.c
> +++ b/block.c
> @@ -1044,9 +1044,11 @@ void bdrv_set_backing_hd(BlockDriverState *bs,
> BlockDriverState *backing_hd)
> backing_hd->drv ? backing_hd->drv->format_name : "");
>
> bdrv_op_block_all(bs->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(bs->backing_hd, BLOCK_OP_TYPE_COMMIT_TARGET,
> bs->backing_blocker);
> + bdrv_op_unblock(bs->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 37bfd8b..8ba5a24 100644
> --- a/block/stream.c
> +++ b/block/stream.c
> @@ -33,6 +33,8 @@ typedef struct StreamBlockJob {
> BlockDriverState *base;
> BlockdevOnError on_error;
> char *backing_file_str;
> + int bs_flags;
> + Error *blocker;
> } StreamBlockJob;
>
> static int coroutine_fn stream_populate(BlockDriverState *bs,
> @@ -89,6 +91,13 @@ static void stream_complete(BlockJob *job, void *opaque)
> StreamBlockJob *s = container_of(job, StreamBlockJob, common);
> StreamCompleteData *data = opaque;
> BlockDriverState *base = s->base;
> + BlockDriverState *bs;
> +
> + /* Remove all blockers set in stream_start() */
> + for (bs = job->bs->backing_hd; bs && bs != s->base; bs = bs->backing_hd)
> {
> + bdrv_op_unblock_all(bs, s->blocker);
> + }
> + error_free(s->blocker);
>
> if (!block_job_is_cancelled(&s->common) && data->reached_end &&
> data->ret == 0) {
> @@ -103,6 +112,11 @@ static void stream_complete(BlockJob *job, void *opaque)
> close_unused_images(job->bs, base, base_id);
> }
>
> + /* 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);
> @@ -246,7 +260,9 @@ void stream_start(BlockDriverState *bs, BlockDriverState
> *base,
> BlockCompletionFunc *cb,
> void *opaque, Error **errp)
> {
> + BlockDriverState *iter;
> StreamBlockJob *s;
> + int orig_bs_flags;
>
> if ((on_error == BLOCKDEV_ON_ERROR_STOP ||
> on_error == BLOCKDEV_ON_ERROR_ENOSPC) &&
> @@ -255,13 +271,30 @@ 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) {
I don't think all bdrv_reopen implementation will guarantee to set errp
on error :(. For example:
static int qcow2_reopen_prepare(BDRVReopenState *state,
BlockReopenQueue *queue, Error **errp)
{
int ret;
if ((state->flags & BDRV_O_RDWR) == 0) {
ret = bdrv_flush(state->bs);
if (ret < 0) {
return ret;
}
ret = qcow2_mark_clean(state->bs);
if (ret < 0) {
return ret;
}
}
return 0;
}
Maybe we should set one here so the user sees the failure?
Fam
> + return;
> + }
> + }
> +
> s = block_job_create(&stream_job_driver, bs, speed, cb, opaque, errp);
> if (!s) {
> return;
> }
>
> + /* Block all intermediate nodes between bs and base, because they
> + * will disappear from the chain after this operation */
> + error_setg(&s->blocker, "blocked by the block-stream operation in '%s'",
> + bdrv_get_device_or_node_name(bs));
> + for (iter = bs->backing_hd; iter && iter != base; iter =
> iter->backing_hd) {
> + bdrv_op_block_all(iter, s->blocker);
> + }
> +
> s->base = base;
> 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);
> --
> 2.1.4
>
>
