[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH v9 06/11] block: Support streaming to an interme
From: |
Max Reitz |
Subject: |
Re: [Qemu-block] [PATCH v9 06/11] block: Support streaming to an intermediate layer |
Date: |
Wed, 27 Apr 2016 15:04:57 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.7.2 |
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 <address@hidden>
> ---
> 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
- [Qemu-block] [PATCH for-2.7 v9 00/11] Support streaming to an intermediate layer, Alberto Garcia, 2016/04/04
- [Qemu-block] [PATCH v9 04/11] block: use the block job list in bdrv_close(), Alberto Garcia, 2016/04/04
- [Qemu-block] [PATCH v9 06/11] block: Support streaming to an intermediate layer, Alberto Garcia, 2016/04/04
- [Qemu-block] [PATCH v9 09/11] qemu-iotests: test streaming to an intermediate layer, Alberto Garcia, 2016/04/04
- [Qemu-block] [PATCH v9 01/11] block: keep a list of block jobs, Alberto Garcia, 2016/04/04
- [Qemu-block] [PATCH v9 10/11] qemu-iotests: test overlapping block-stream operations, Alberto Garcia, 2016/04/04
- [Qemu-block] [PATCH v9 03/11] block: use the block job list in qmp_query_block_jobs(), Alberto Garcia, 2016/04/04