diff mbox

[1/2] qapi: Add "detect-zeroes" option to drive-mirror

Message ID 1433747185-16797-2-git-send-email-famz@redhat.com
State New
Headers show

Commit Message

Fam Zheng June 8, 2015, 7:06 a.m. UTC
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 <famz@redhat.com>
---
 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(-)

Comments

Andrey Korolyov June 10, 2015, 3:41 p.m. UTC | #1
On Mon, Jun 8, 2015 at 10:06 AM, Fam Zheng <famz@redhat.com> 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 <famz@redhat.com>
> ---
>  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" } }'
Andrey Korolyov June 10, 2015, 3:48 p.m. UTC | #2
> '{"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" } }'

Sorry, forgot to mention - of course I`ve pulled all previous
zeroing-related queue, so I haven`t had only the QMP-related fix
running on top of the master :)
Alexandre DERUMIER June 10, 2015, 4:04 p.m. UTC | #3
>>Sorry, forgot to mention - of course I`ve pulled all previous 
>>zeroing-related queue, so I haven`t had only the QMP-related fix 
>>running on top of the master :) 

Hi, I had a discussion about rbd mirroring, detect-zeroes and sparse target,  some months ago with paolo

http://qemu.11.n7.nabble.com/qemu-drive-mirror-to-rbd-storage-no-sparse-rbd-image-td293642.html

Not sure it's related but 

"Lack of bdrv_co_write_zeroes is why detect-zeroes does not work. 

Lack of bdrv_get_block_status is why sparse->sparse does not work 
without detect-zeroes. 

Paolo 
"
----- Mail original -----
De: "Andrey Korolyov" <andrey@xdel.ru>
À: "Fam Zheng" <famz@redhat.com>
Cc: "Kevin Wolf" <kwolf@redhat.com>, "qemu block" <qemu-block@nongnu.org>, "Jeff Cody" <jcody@redhat.com>, "qemu-devel" <qemu-devel@nongnu.org>, "Markus Armbruster" <armbru@redhat.com>, "stefanha" <stefanha@redhat.com>, "pbonzini" <pbonzini@redhat.com>
Envoyé: Mercredi 10 Juin 2015 17:48:54
Objet: Re: [Qemu-devel] [PATCH 1/2] qapi: Add "detect-zeroes" option to	drive-mirror

> '{"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" } }' 

Sorry, forgot to mention - of course I`ve pulled all previous 
zeroing-related queue, so I haven`t had only the QMP-related fix 
running on top of the master :)
Andrey Korolyov June 10, 2015, 4:09 p.m. UTC | #4
On Wed, Jun 10, 2015 at 7:04 PM, Alexandre DERUMIER <aderumier@odiso.com> wrote:
>>>Sorry, forgot to mention - of course I`ve pulled all previous
>>>zeroing-related queue, so I haven`t had only the QMP-related fix
>>>running on top of the master :)
>
> Hi, I had a discussion about rbd mirroring, detect-zeroes and sparse target,  some months ago with paolo
>
> http://qemu.11.n7.nabble.com/qemu-drive-mirror-to-rbd-storage-no-sparse-rbd-image-td293642.html
>
> Not sure it's related but
>
> "Lack of bdrv_co_write_zeroes is why detect-zeroes does not work.
>
> Lack of bdrv_get_block_status is why sparse->sparse does not work
> without detect-zeroes.
>

Yes, I remember this one and the recent queue pull was the reason for
me to re-test it, despite the current patch variant would more likely
to be updated by comments from Eric, the behavior for non-file
backends like rbd can/will stay the same.
diff mbox

Patch

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