[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [Qemu-devel] [PATCH 1/2] qapi: Add "detect-zeroes" opti
|
From: |
Andrey Korolyov |
|
Subject: |
Re: [Qemu-block] [Qemu-devel] [PATCH 1/2] qapi: Add "detect-zeroes" option to drive-mirror |
|
Date: |
Wed, 10 Jun 2015 18:41:12 +0300 |
On Mon, Jun 8, 2015 at 10:06 AM, Fam Zheng <address@hidden> wrote:
> The new optional flag defaults to true, in which case, mirror job would
> check the read sectors and use sparse write if they are zero. Otherwise
> data will be fully copied.
>
> Signed-off-by: Fam Zheng <address@hidden>
> ---
> block/mirror.c | 21 +++++++++++++++------
> blockdev.c | 6 +++++-
> hmp.c | 2 +-
> include/block/block_int.h | 3 ++-
> qapi/block-core.json | 4 +++-
> qmp-commands.hx | 4 +++-
> 6 files changed, 29 insertions(+), 11 deletions(-)
>
> diff --git a/block/mirror.c b/block/mirror.c
> index 3c38695..261373c 100644
> --- a/block/mirror.c
> +++ b/block/mirror.c
> @@ -58,6 +58,7 @@ typedef struct MirrorBlockJob {
> int sectors_in_flight;
> int ret;
> bool unmap;
> + bool detect_zeroes;
> } MirrorBlockJob;
>
> typedef struct MirrorOp {
> @@ -153,8 +154,14 @@ static void mirror_read_complete(void *opaque, int ret)
> mirror_iteration_done(op, ret);
> return;
> }
> - bdrv_aio_writev(s->target, op->sector_num, &op->qiov, op->nb_sectors,
> - mirror_write_complete, op);
> + if (s->detect_zeroes && qemu_iovec_is_zero(&op->qiov)) {
> + bdrv_aio_write_zeroes(s->target, op->sector_num, op->nb_sectors,
> + s->unmap ? BDRV_REQ_MAY_UNMAP : 0,
> + mirror_write_complete, op);
> + } else {
> + bdrv_aio_writev(s->target, op->sector_num, &op->qiov, op->nb_sectors,
> + mirror_write_complete, op);
> + }
> }
>
> static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s)
> @@ -668,7 +675,7 @@ static void mirror_start_job(BlockDriverState *bs,
> BlockDriverState *target,
> int64_t buf_size,
> BlockdevOnError on_source_error,
> BlockdevOnError on_target_error,
> - bool unmap,
> + bool unmap, bool detect_zeroes,
> BlockCompletionFunc *cb,
> void *opaque, Error **errp,
> const BlockJobDriver *driver,
> @@ -704,6 +711,7 @@ static void mirror_start_job(BlockDriverState *bs,
> BlockDriverState *target,
> s->granularity = granularity;
> s->buf_size = MAX(buf_size, granularity);
> s->unmap = unmap;
> + s->detect_zeroes = detect_zeroes;
>
> s->dirty_bitmap = bdrv_create_dirty_bitmap(bs, granularity, NULL, errp);
> if (!s->dirty_bitmap) {
> @@ -722,7 +730,7 @@ void mirror_start(BlockDriverState *bs, BlockDriverState
> *target,
> int64_t speed, uint32_t granularity, int64_t buf_size,
> MirrorSyncMode mode, BlockdevOnError on_source_error,
> BlockdevOnError on_target_error,
> - bool unmap,
> + bool unmap, bool detect_zeroes,
> BlockCompletionFunc *cb,
> void *opaque, Error **errp)
> {
> @@ -737,7 +745,8 @@ void mirror_start(BlockDriverState *bs, BlockDriverState
> *target,
> base = mode == MIRROR_SYNC_MODE_TOP ? bs->backing_hd : NULL;
> mirror_start_job(bs, target, replaces,
> speed, granularity, buf_size,
> - on_source_error, on_target_error, unmap, cb, opaque,
> errp,
> + on_source_error, on_target_error, unmap,
> + detect_zeroes, cb, opaque, errp,
> &mirror_job_driver, is_none_mode, base);
> }
>
> @@ -785,7 +794,7 @@ void commit_active_start(BlockDriverState *bs,
> BlockDriverState *base,
>
> bdrv_ref(base);
> mirror_start_job(bs, base, NULL, speed, 0, 0,
> - on_error, on_error, false, cb, opaque, &local_err,
> + on_error, on_error, false, false, cb, opaque,
> &local_err,
> &commit_active_job_driver, false, base);
> if (local_err) {
> error_propagate(errp, local_err);
> diff --git a/blockdev.c b/blockdev.c
> index 4387e14..d86ec1c 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -2633,6 +2633,7 @@ void qmp_drive_mirror(const char *device, const char
> *target,
> bool has_on_source_error, BlockdevOnError
> on_source_error,
> bool has_on_target_error, BlockdevOnError
> on_target_error,
> bool has_unmap, bool unmap,
> + bool has_detect_zeroes, bool detect_zeroes,
> Error **errp)
> {
> BlockBackend *blk;
> @@ -2667,6 +2668,9 @@ void qmp_drive_mirror(const char *device, const char
> *target,
> if (!has_unmap) {
> unmap = true;
> }
> + if (!has_detect_zeroes) {
> + detect_zeroes = true;
> + }
>
> if (granularity != 0 && (granularity < 512 || granularity > 1048576 *
> 64)) {
> error_set(errp, QERR_INVALID_PARAMETER_VALUE, "granularity",
> @@ -2806,7 +2810,7 @@ void qmp_drive_mirror(const char *device, const char
> *target,
> has_replaces ? replaces : NULL,
> speed, granularity, buf_size, sync,
> on_source_error, on_target_error,
> - unmap,
> + unmap, detect_zeroes,
> block_job_cb, bs, &local_err);
> if (local_err != NULL) {
> bdrv_unref(target_bs);
> diff --git a/hmp.c b/hmp.c
> index 62c53e0..28a597f 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -1056,7 +1056,7 @@ void hmp_drive_mirror(Monitor *mon, const QDict *qdict)
> false, NULL, false, NULL,
> full ? MIRROR_SYNC_MODE_FULL : MIRROR_SYNC_MODE_TOP,
> true, mode, false, 0, false, 0, false, 0,
> - false, 0, false, 0, false, true, &err);
> + false, 0, false, 0, false, true, false, true, &err);
> hmp_handle_error(mon, &err);
> }
>
> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index 459fe1c..10715a6 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -591,6 +591,7 @@ void commit_active_start(BlockDriverState *bs,
> BlockDriverState *base,
> * @on_source_error: The action to take upon error reading from the source.
> * @on_target_error: The action to take upon error writing to the target.
> * @unmap: Whether to unmap target where source sectors only contain zeroes.
> + * @detect_zero: Whether to do zero detect of source sectors.
> * @cb: Completion function for the job.
> * @opaque: Opaque pointer value passed to @cb.
> * @errp: Error object.
> @@ -605,7 +606,7 @@ void mirror_start(BlockDriverState *bs, BlockDriverState
> *target,
> int64_t speed, uint32_t granularity, int64_t buf_size,
> MirrorSyncMode mode, BlockdevOnError on_source_error,
> BlockdevOnError on_target_error,
> - bool unmap,
> + bool unmap, bool detect_zeroes,
> BlockCompletionFunc *cb,
> void *opaque, Error **errp);
>
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index a59063d..2014dc6 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -959,6 +959,8 @@
> # target image sectors will be unmapped; otherwise, zeroes will be
> # written. Both will result in identical contents.
> # Default is true. (Since 2.4)
> +# @detect-zeroes: #optional Whether to detect zero sectors in source and use
> +# zero write on target. Default is true. (Since 2.4)
> #
> # Returns: nothing on success
> # If @device is not a valid block device, DeviceNotFound
> @@ -972,7 +974,7 @@
> '*speed': 'int', '*granularity': 'uint32',
> '*buf-size': 'int', '*on-source-error': 'BlockdevOnError',
> '*on-target-error': 'BlockdevOnError',
> - '*unmap': 'bool' } }
> + '*unmap': 'bool', '*detect-zeroes': 'bool' } }
>
> ##
> # @BlockDirtyBitmap
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 63c86fc..cac07fc 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -1503,7 +1503,7 @@ EQMP
> .args_type = "sync:s,device:B,target:s,speed:i?,mode:s?,format:s?,"
> "node-name:s?,replaces:s?,"
> "on-source-error:s?,on-target-error:s?,"
> - "unmap:b?,"
> + "unmap:b?,detect-zeroes:b?"
> "granularity:i?,buf-size:i?",
> .mhandler.cmd_new = qmp_marshal_input_drive_mirror,
> },
> @@ -1545,6 +1545,8 @@ Arguments:
> (BlockdevOnError, default 'report')
> - "unmap": whether the target sectors should be discarded where source has
> only
> zeroes. (json-bool, optional, default true)
> +- "detect-zeroes": if true, the source sectors that are zeroes will be
> written as
> + sparse on target. (json-bool, optional, default true)
>
> The default value of the granularity is the image cluster size clamped
> between 4096 and 65536, if the image format defines one. If the format
> --
> 2.4.2
>
>
Hi Fam,
just checked this one with slight adaptation to the current master on
an RBD backend, it looks like despite the source returns zeroes (no
reads in stat), the same zeroes are happily allocated at the target by
the drivemirror:
2015-06-10 18:33:10.376619 mon.0 [INF] pgmap v6126601: 384 pgs: 384
active+clean; 453 GB data, 1788 GB used, 20511 GB / 22300 GB avail;
5164KB/s wr, 1op/s
2015-06-10 18:33:11.387558 mon.0 [INF] pgmap v6126602: 384 pgs: 384
active+clean; 453 GB data, 1788 GB used, 20511 GB / 22300 GB avail;
14251KB/s wr, 3op/s
2015-06-10 18:33:13.393758 mon.0 [INF] pgmap v6126603: 384 pgs: 384
active+clean; 453 GB data, 1788 GB used, 20511 GB / 22300 GB avail;
12582KB/s wr, 3op/s
2015-06-10 18:33:14.396960 mon.0 [INF] pgmap v6126604: 384 pgs: 384
active+clean; 453 GB data, 1788 GB used, 20511 GB / 22300 GB avail;
2039B/s rd, 11237KB/s wr, 4op/s
2015-06-10 18:33:15.406989 mon.0 [INF] pgmap v6126605: 384 pgs: 384
active+clean; 453 GB data, 1788 GB used, 20511 GB / 22300 GB avail;
3054B/s rd, 14255KB/s wr, 5op/s
2015-06-10 18:33:16.414723 mon.0 [INF] pgmap v6126606: 384 pgs: 384
active+clean; 453 GB data, 1789 GB used, 20511 GB / 22300 GB avail;
14579KB/s wr, 5op/s
2015-06-10 18:33:18.421028 mon.0 [INF] pgmap v6126607: 384 pgs: 384
active+clean; 453 GB data, 1789 GB used, 20511 GB / 22300 GB avail;
13816KB/s wr, 4op/s
2015-06-10 18:33:19.423108 mon.0 [INF] pgmap v6126608: 384 pgs: 384
active+clean; 453 GB data, 1789 GB used, 20511 GB / 22300 GB avail;
9519KB/s wr, 2op/s
2015-06-10 18:33:20.433639 mon.0 [INF] pgmap v6126609: 384 pgs: 384
active+clean; 453 GB data, 1789 GB used, 20510 GB / 22300 GB avail;
10186KB/s wr, 2op/s
2015-06-10 18:33:21.436827 mon.0 [INF] pgmap v6126610: 384 pgs: 384
active+clean; 453 GB data, 1789 GB used, 20510 GB / 22300 GB avail;
15123KB/s wr, 4op/s
2015-06-10 18:33:23.446657 mon.0 [INF] pgmap v6126611: 384 pgs: 384
active+clean; 453 GB data, 1789 GB used, 20510 GB / 22300 GB avail;
2037B/s rd, 12813KB/s wr, 5op/s
2015-06-10 18:33:25.452221 mon.0 [INF] pgmap v6126612: 384 pgs: 384
active+clean; 453 GB data, 1789 GB used, 20510 GB / 22300 GB avail;
1530B/s rd, 8160KB/s wr, 2op/s
2015-06-10 18:33:26.455455 mon.0 [INF] pgmap v6126613: 384 pgs: 384
active+clean; 454 GB data, 1789 GB used, 20510 GB / 22300 GB avail;
12799KB/s wr, 4op/s
2015-06-10 18:33:28.479202 mon.0 [INF] pgmap v6126614: 384 pgs: 384
active+clean; 454 GB data, 1789 GB used, 20510 GB / 22300 GB avail;
12828KB/s wr, 4op/s
2015-06-10 18:33:30.501242 mon.0 [INF] pgmap v6126615: 384 pgs: 384
active+clean; 454 GB data, 1789 GB used, 20510 GB / 22300 GB avail;
5105KB/s wr, 1op/s
2015-06-10 18:33:31.504592 mon.0 [INF] pgmap v6126616: 384 pgs: 384
active+clean; 454 GB data, 1789 GB used, 20510 GB / 22300 GB avail;
14829KB/s wr, 4op/s
2015-06-10 18:33:33.524838 mon.0 [INF] pgmap v6126617: 384 pgs: 384
active+clean; 454 GB data, 1789 GB used, 20510 GB / 22300 GB avail;
2034B/s rd, 16863KB/s wr, 6op/s
2015-06-10 18:33:35.530814 mon.0 [INF] pgmap v6126618: 384 pgs: 384
active+clean; 454 GB data, 1789 GB used, 20510 GB / 22300 GB avail;
1526B/s rd, 6546KB/s wr, 2op/s
2015-06-10 18:33:36.535921 mon.0 [INF] pgmap v6126619: 384 pgs: 384
active+clean; 454 GB data, 1790 GB used, 20510 GB / 22300 GB avail;
13574KB/s wr, 3op/s
comparing to the real data inbetween:
2015-06-10 18:30:55.489146 mon.0 [INF] pgmap v6126503: 384 pgs: 384
active+clean; 452 GB data, 1784 GB used, 20515 GB / 22300 GB avail;
6154KB/s rd, 6336KB/s wr, 9op/s
2015-06-10 18:30:56.496681 mon.0 [INF] pgmap v6126504: 384 pgs: 384
active+clean; 452 GB data, 1784 GB used, 20515 GB / 22300 GB avail;
2759KB/s rd, 5449KB/s wr, 9op/s
2015-06-10 18:30:58.506213 mon.0 [INF] pgmap v6126505: 384 pgs: 384
active+clean; 452 GB data, 1784 GB used, 20515 GB / 22300 GB avail;
5489KB/s rd, 9230KB/s wr, 9op/s
2015-06-10 18:31:00.512232 mon.0 [INF] pgmap v6126506: 384 pgs: 384
active+clean; 452 GB data, 1784 GB used, 20515 GB / 22300 GB avail;
6133KB/s rd, 6915KB/s wr, 5op/s
2015-06-10 18:31:01.521438 mon.0 [INF] pgmap v6126507: 384 pgs: 384
active+clean; 452 GB data, 1784 GB used, 20515 GB / 22300 GB avail;
9578KB/s rd, 9744KB/s wr, 7op/s
2015-06-10 18:31:03.526486 mon.0 [INF] pgmap v6126508: 384 pgs: 384
active+clean; 452 GB data, 1784 GB used, 20515 GB / 22300 GB avail;
7283KB/s rd, 8383KB/s wr, 17op/s
2015-06-10 18:31:05.532062 mon.0 [INF] pgmap v6126509: 384 pgs: 384
active+clean; 452 GB data, 1784 GB used, 20515 GB / 22300 GB avail;
3379KB/s rd, 4160KB/s wr, 11op/s
2015-06-10 18:31:06.535260 mon.0 [INF] pgmap v6126510: 384 pgs: 384
active+clean; 452 GB data, 1784 GB used, 20515 GB / 22300 GB avail;
7829KB/s rd, 8062KB/s wr, 11op/s
2015-06-10 18:31:08.548050 mon.0 [INF] pgmap v6126511: 384 pgs: 384
active+clean; 452 GB data, 1784 GB used, 20515 GB / 22300 GB avail;
7815KB/s rd, 6629KB/s wr, 10op/s
The RBD backend should recognize discard for sure, so either there is
something wrong with the particular driver coupled with the block job
or with the discard logic. Sample invocation is given below:
'{"execute":"drive-mirror", "arguments": { "device":
"drive-virtio-disk0", "target":
"rbd:dev-rack2/vm33090-dest:id=qemukvm:key=xxx:auth_supported=cephx\\;none:mon_host=10.6.0.1\\:6789\\;10.6.0.3\\:6789\\;10.6.0.4\\:6789",
"mode": "existing", "sync": "full", "detect-zeroes": true, "format":
"raw" } }'
[Qemu-block] [PATCH 2/2] iotests: Add test cases for drive-mirror "detect-zeroes" option, Fam Zheng, 2015/06/08