On Sat, 04/12 20:57, Max Reitz wrote:
> Allow QMP users to manipulate the granularity used in the block-commit
> command.
>
> Signed-off-by: Max Reitz <mre...@redhat.com>
> Reviewed-by: Eric Blake <ebl...@redhat.com>
> ---
> block/commit.c | 16 +++++++++++++---
> block/mirror.c | 4 ++--
> blockdev.c | 22 ++++++++++++++++------
> include/block/block_int.h | 10 ++++++----
> qapi-schema.json | 6 +++++-
> 5 files changed, 42 insertions(+), 16 deletions(-)
>
> diff --git a/block/commit.c b/block/commit.c
> index acec4ac..cf0e500 100644
> --- a/block/commit.c
> +++ b/block/commit.c
> @@ -37,6 +37,7 @@ typedef struct CommitBlockJob {
> BlockdevOnError on_error;
> int base_flags;
> int orig_overlay_flags;
> + int64_t granularity;
> } CommitBlockJob;
>
> static int coroutine_fn commit_populate(BlockDriverState *bs,
> @@ -93,7 +94,7 @@ static void coroutine_fn commit_run(void *opaque)
> }
>
> end = s->common.len >> BDRV_SECTOR_BITS;
> - buf = qemu_blockalign(top, COMMIT_BUFFER_SIZE);
> + buf = qemu_blockalign(top, s->granularity);
>
> for (sector_num = 0; sector_num < end; sector_num += n) {
> uint64_t delay_ns = 0;
> @@ -109,7 +110,7 @@ wait:
> }
> /* Copy if allocated above the base */
> ret = bdrv_is_allocated_above(top, base, sector_num,
> - COMMIT_BUFFER_SIZE / BDRV_SECTOR_SIZE,
> + s->granularity / BDRV_SECTOR_SIZE,
> &n);
> copy = (ret == 1);
> trace_commit_one_iteration(s, sector_num, n, ret);
> @@ -180,7 +181,7 @@ static const BlockJobDriver commit_job_driver = {
> };
>
> void commit_start(BlockDriverState *bs, BlockDriverState *base,
> - BlockDriverState *top, int64_t speed,
> + BlockDriverState *top, int64_t speed, int64_t granularity,
> BlockdevOnError on_error, BlockDriverCompletionFunc *cb,
> void *opaque, Error **errp)
> {
> @@ -214,6 +215,13 @@ void commit_start(BlockDriverState *bs, BlockDriverState
> *base,
> orig_base_flags = bdrv_get_flags(base);
> orig_overlay_flags = bdrv_get_flags(overlay_bs);
>
> + if (!granularity) {
> + granularity = COMMIT_BUFFER_SIZE;
> + }
> +
> + assert(granularity >= BDRV_SECTOR_SIZE);
> + assert(is_power_of_2(granularity));
> +
> /* convert base & overlay_bs to r/w, if necessary */
> if (!(orig_base_flags & BDRV_O_RDWR)) {
> reopen_queue = bdrv_reopen_queue(reopen_queue, base,
> @@ -244,6 +252,8 @@ void commit_start(BlockDriverState *bs, BlockDriverState
> *base,
> s->base_flags = orig_base_flags;
> s->orig_overlay_flags = orig_overlay_flags;
>
> + s->granularity = granularity;
> +
> s->on_error = on_error;
> s->common.co = qemu_coroutine_create(commit_run);
>
> diff --git a/block/mirror.c b/block/mirror.c
> index 0ef41f9..5b1ebb2 100644
> --- a/block/mirror.c
> +++ b/block/mirror.c
> @@ -632,7 +632,7 @@ void mirror_start(BlockDriverState *bs, BlockDriverState
> *target,
> }
>
> void commit_active_start(BlockDriverState *bs, BlockDriverState *base,
> - int64_t speed,
> + int64_t speed, int64_t granularity,
> BlockdevOnError on_error,
> BlockDriverCompletionFunc *cb,
> void *opaque, Error **errp)
> @@ -674,7 +674,7 @@ void commit_active_start(BlockDriverState *bs,
> BlockDriverState *base,
> }
>
> bdrv_ref(base);
> - mirror_start_job(bs, base, speed, 0, 0,
> + mirror_start_job(bs, base, speed, granularity, 0,
> on_error, on_error, cb, opaque, &local_err,
> &commit_active_job_driver, false, base);
> if (error_is_set(&local_err)) {
> diff --git a/blockdev.c b/blockdev.c
> index 5dd01ea..9d7bd04 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -1865,8 +1865,8 @@ void qmp_block_stream(const char *device, bool has_base,
>
> void qmp_block_commit(const char *device,
> bool has_base, const char *base, const char *top,
> - bool has_speed, int64_t speed,
> - Error **errp)
> + bool has_speed, int64_t speed, bool has_granularity,
> + int64_t granularity, Error **errp)
> {
> BlockDriverState *bs;
> BlockDriverState *base_bs, *top_bs;
> @@ -1879,6 +1879,16 @@ void qmp_block_commit(const char *device,
> if (!has_speed) {
> speed = 0;
> }
> + if (!has_granularity) {
> + granularity = 0;
> + }
> +
> + if (granularity &&
> + (granularity < BDRV_SECTOR_SIZE || !is_power_of_2(granularity)))
> + {
> + error_set(errp, QERR_INVALID_PARAMETER, "granularity");
> + return;
> + }
>
> /* drain all i/o before commits */
> bdrv_drain_all();
> @@ -1915,11 +1925,11 @@ void qmp_block_commit(const char *device,
> }
>
> if (top_bs == bs) {
> - commit_active_start(bs, base_bs, speed, on_error, block_job_cb,
> - bs, &local_err);
> + commit_active_start(bs, base_bs, speed, granularity, on_error,
> + block_job_cb, bs, &local_err);
> } else {
> - commit_start(bs, base_bs, top_bs, speed, on_error, block_job_cb, bs,
> - &local_err);
> + commit_start(bs, base_bs, top_bs, speed, granularity, on_error,
> + block_job_cb, bs, &local_err);
> }
> if (local_err != NULL) {
> error_propagate(errp, local_err);
> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index cd5bc73..c01b4aa 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -426,6 +426,7 @@ void stream_start(BlockDriverState *bs, BlockDriverState
> *base,
> * @top: Top block device to be committed.
> * @base: Block device that will be written into, and become the new top.
> * @speed: The maximum speed, in bytes per second, or 0 for unlimited.
> + * @granularity: The granularity, in bytes, or 0 for a default value.
> * @on_error: The action to take upon error.
> * @cb: Completion function for the job.
> * @opaque: Opaque pointer value passed to @cb.
> @@ -433,14 +434,15 @@ void stream_start(BlockDriverState *bs,
> BlockDriverState *base,
> *
> */
> void commit_start(BlockDriverState *bs, BlockDriverState *base,
> - BlockDriverState *top, int64_t speed,
> - BlockdevOnError on_error, BlockDriverCompletionFunc *cb,
> - void *opaque, Error **errp);
> + BlockDriverState *top, int64_t speed, int64_t granularity,
> + BlockdevOnError on_error, BlockDriverCompletionFunc *cb,
> + void *opaque, Error **errp);
> /**
> * commit_active_start:
> * @bs: Active block device to be committed.
> * @base: Block device that will be written into, and become the new top.
> * @speed: The maximum speed, in bytes per second, or 0 for unlimited.
> + * @granularity: The granularity, in bytes, or 0 for cluster size.
> * @on_error: The action to take upon error.
> * @cb: Completion function for the job.
> * @opaque: Opaque pointer value passed to @cb.
> @@ -448,7 +450,7 @@ void commit_start(BlockDriverState *bs, BlockDriverState
> *base,
> *
> */
> void commit_active_start(BlockDriverState *bs, BlockDriverState *base,
> - int64_t speed,
> + int64_t speed, int64_t granularity,
> BlockdevOnError on_error,
> BlockDriverCompletionFunc *cb,
> void *opaque, Error **errp);
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 391356f..151fad3 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -2112,6 +2112,10 @@
> #
> # @speed: #optional the maximum speed, in bytes per second
> #
> +# @granularity: #optional the granularity to be used for the operation, in
> +# bytes; has to be a power of two and at least 512, or 0 for
> the
> +# default value (since 2.1)
> +#
> # Returns: Nothing on success
> # If commit or stream is already active on this device, DeviceInUse
> # If @device does not exist, DeviceNotFound
> @@ -2124,7 +2128,7 @@
> ##
> { 'command': 'block-commit',
> 'data': { 'device': 'str', '*base': 'str', 'top': 'str',
> - '*speed': 'int' } }
> + '*speed': 'int', '*granularity': 'int' } }
>
> ##
> # @drive-backup
> --
> 1.9.2
>
>
Reviewed-by: Fam Zheng <f...@redhat.com>
