[Qemu-devel] [PATCH v3 0/8] drive-mirror improvements

[Qemu-devel] [PATCH v3 0/8] drive-mirror improvements

[Denis V. Lunev]

This patchset contains patches dealing with known-to-be-zero areas in drive
mirror from [PATCH 0/9] major rework of drive-mirror patchset.

Changes from v2:
- added mirror_throttle helper (patch 3) to address Eric' comment about
  last_time_ns
- comment tweaks (thank you, Eric)
- marked mirror_dirty_init as couroutine_fn

Changes from v1:
- only patches dealing with zeroes remains
- ported to current HEAD
- fixed issue with dirty-bitmap, int length is changed with int64
- fixed sectors_in_flight usage
- patch 6 is reworked taken into account bugs found in active mirror
- fixed patch 7
- direct checking of .bdrv_co_write_zeroes is replaced with
  bdrv_can_write_zeroes_with_unmap
  - added fixes for bdrv_can_write_zeroes_with_unmap

Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Stefan Hajnoczi <stefanha@redhat.com>
CC: Fam Zheng <famz@redhat.com>
CC: Kevin Wolf <kwolf@redhat.com>
CC: Max Reitz <mreitz@redhat.com>
CC: Jeff Cody <jcody@redhat.com>
CC: Eric Blake <eblake@redhat.com>

Denis V. Lunev (8):
  dirty-bitmap: operate with int64_t amount
  mirror: make sectors_in_flight int64_t
  mirror: create mirror_throttle helper
  mirror: create mirror_dirty_init helper for mirror_run
  block: remove extra condition in bdrv_can_write_zeroes_with_unmap
  mirror: optimize dirty bitmap filling in mirror_run a bit
  mirror: efficiently zero out target
  mirror: improve performance of mirroring of empty disk

 block.c                      |   2 +-
 block/dirty-bitmap.c         |   6 +-
 block/mirror.c               | 145 +++++++++++++++++++++++++++++--------------
 include/block/block_int.h    |   2 +-
 include/block/dirty-bitmap.h |   4 +-
 5 files changed, 106 insertions(+), 53 deletions(-)

-- 
2.5.0

[Qemu-devel] [PATCH v3 1/8] dirty-bitmap: operate with int64_t amount

[Denis V. Lunev]

Underlying HBitmap operates even with uint64_t. Thus this change is safe.
This would be useful f.e. to mark entire bitmap dirty in one call.

Signed-off-by: Denis V. Lunev <den@openvz.org>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
CC: Kevin Wolf <kwolf@redhat.com>
CC: Max Reitz <mreitz@redhat.com>
CC: Jeff Cody <jcody@redhat.com>
---
 block/dirty-bitmap.c         | 6 +++---
 include/block/block_int.h    | 2 +-
 include/block/dirty-bitmap.h | 4 ++--
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index 4902ca5..f2bfdcf 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -326,14 +326,14 @@ void bdrv_dirty_iter_init(BdrvDirtyBitmap *bitmap, HBitmapIter *hbi)
 }
 
 void bdrv_set_dirty_bitmap(BdrvDirtyBitmap *bitmap,
-                           int64_t cur_sector, int nr_sectors)
+                           int64_t cur_sector, int64_t nr_sectors)
 {
     assert(bdrv_dirty_bitmap_enabled(bitmap));
     hbitmap_set(bitmap->bitmap, cur_sector, nr_sectors);
 }
 
 void bdrv_reset_dirty_bitmap(BdrvDirtyBitmap *bitmap,
-                             int64_t cur_sector, int nr_sectors)
+                             int64_t cur_sector, int64_t nr_sectors)
 {
     assert(bdrv_dirty_bitmap_enabled(bitmap));
     hbitmap_reset(bitmap->bitmap, cur_sector, nr_sectors);
@@ -361,7 +361,7 @@ void bdrv_undo_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap, HBitmap *in)
 }
 
 void bdrv_set_dirty(BlockDriverState *bs, int64_t cur_sector,
-                    int nr_sectors)
+                    int64_t nr_sectors)
 {
     BdrvDirtyBitmap *bitmap;
     QLIST_FOREACH(bitmap, &bs->dirty_bitmaps, list) {
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 47b9aac..042c118 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -767,7 +767,7 @@ void blk_dev_eject_request(BlockBackend *blk, bool force);
 bool blk_dev_is_tray_open(BlockBackend *blk);
 bool blk_dev_is_medium_locked(BlockBackend *blk);
 
-void bdrv_set_dirty(BlockDriverState *bs, int64_t cur_sector, int nr_sectors);
+void bdrv_set_dirty(BlockDriverState *bs, int64_t cur_sector, int64_t nr_sect);
 bool bdrv_requests_pending(BlockDriverState *bs);
 
 void bdrv_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap, HBitmap **out);
diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
index 80afe60..ee3388f 100644
--- a/include/block/dirty-bitmap.h
+++ b/include/block/dirty-bitmap.h
@@ -33,9 +33,9 @@ DirtyBitmapStatus bdrv_dirty_bitmap_status(BdrvDirtyBitmap *bitmap);
 int bdrv_get_dirty(BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
                    int64_t sector);
 void bdrv_set_dirty_bitmap(BdrvDirtyBitmap *bitmap,
-                           int64_t cur_sector, int nr_sectors);
+                           int64_t cur_sector, int64_t nr_sectors);
 void bdrv_reset_dirty_bitmap(BdrvDirtyBitmap *bitmap,
-                             int64_t cur_sector, int nr_sectors);
+                             int64_t cur_sector, int64_t nr_sectors);
 void bdrv_dirty_iter_init(BdrvDirtyBitmap *bitmap, struct HBitmapIter *hbi);
 void bdrv_set_dirty_iter(struct HBitmapIter *hbi, int64_t offset);
 int64_t bdrv_get_dirty_count(BdrvDirtyBitmap *bitmap);
-- 
2.5.0

[Qemu-devel] [PATCH v3 2/8] mirror: make sectors_in_flight int64_t

[Denis V. Lunev]

We keep here the sum of int fields. Thus this could easily overflow,
especially when we will start sending big requests in next patches.

Signed-off-by: Denis V. Lunev <den@openvz.org>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
CC: Kevin Wolf <kwolf@redhat.com>
CC: Max Reitz <mreitz@redhat.com>
CC: Jeff Cody <jcody@redhat.com>
---
 block/mirror.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/mirror.c b/block/mirror.c
index 8d96049..9850e54 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -60,7 +60,7 @@ typedef struct MirrorBlockJob {
 
     unsigned long *in_flight_bitmap;
     int in_flight;
-    int sectors_in_flight;
+    int64_t sectors_in_flight;
     int ret;
     bool unmap;
     bool waiting_for_io;
-- 
2.5.0

[Qemu-devel] [PATCH v3 3/8] mirror: create mirror_throttle helper

[Denis V. Lunev]

The patch also places last_pause_ns from stack in mirror_run into
MirrorBlockJob structure. This helper will be useful in next patches.

Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
CC: Eric Blake <eblake@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
CC: Fam Zheng <famz@redhat.com>
CC: Kevin Wolf <kwolf@redhat.com>
CC: Max Reitz <mreitz@redhat.com>
CC: Jeff Cody <jcody@redhat.com>
---
 block/mirror.c | 31 +++++++++++++++++++------------
 1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/block/mirror.c b/block/mirror.c
index 9850e54..93e2896 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -58,6 +58,7 @@ typedef struct MirrorBlockJob {
     QSIMPLEQ_HEAD(, MirrorBuffer) buf_free;
     int buf_free_count;
 
+    uint64_t last_pause_ns;
     unsigned long *in_flight_bitmap;
     int in_flight;
     int64_t sectors_in_flight;
@@ -512,6 +513,18 @@ static void mirror_exit(BlockJob *job, void *opaque)
     bdrv_unref(src);
 }
 
+static void mirror_throttle(MirrorBlockJob *s)
+{
+    int64_t now = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
+
+    if (now - s->last_pause_ns > SLICE_TIME) {
+        s->last_pause_ns = now;
+        block_job_sleep_ns(&s->common, QEMU_CLOCK_REALTIME, 0);
+    } else {
+        block_job_pause_point(&s->common);
+    }
+}
+
 static void coroutine_fn mirror_run(void *opaque)
 {
     MirrorBlockJob *s = opaque;
@@ -519,7 +532,6 @@ static void coroutine_fn mirror_run(void *opaque)
     BlockDriverState *bs = blk_bs(s->common.blk);
     BlockDriverState *target_bs = blk_bs(s->target);
     int64_t sector_num, end, length;
-    uint64_t last_pause_ns;
     BlockDriverInfo bdi;
     char backing_filename[2]; /* we only need 2 characters because we are only
                                  checking for a NULL string */
@@ -575,7 +587,7 @@ static void coroutine_fn mirror_run(void *opaque)
 
     mirror_free_init(s);
 
-    last_pause_ns = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
+    s->last_pause_ns = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
     if (!s->is_none_mode) {
         /* First part, loop on the sectors and initialize the dirty bitmap.  */
         BlockDriverState *base = s->base;
@@ -585,14 +597,8 @@ static void coroutine_fn mirror_run(void *opaque)
             /* Just to make sure we are not exceeding int limit. */
             int nb_sectors = MIN(INT_MAX >> BDRV_SECTOR_BITS,
                                  end - sector_num);
-            int64_t now = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
 
-            if (now - last_pause_ns > SLICE_TIME) {
-                last_pause_ns = now;
-                block_job_sleep_ns(&s->common, QEMU_CLOCK_REALTIME, 0);
-            } else {
-                block_job_pause_point(&s->common);
-            }
+            mirror_throttle(s);
 
             if (block_job_is_cancelled(&s->common)) {
                 goto immediate_exit;
@@ -615,7 +621,7 @@ static void coroutine_fn mirror_run(void *opaque)
     bdrv_dirty_iter_init(s->dirty_bitmap, &s->hbi);
     for (;;) {
         uint64_t delay_ns = 0;
-        int64_t cnt;
+        int64_t cnt, delta;
         bool should_complete;
 
         if (s->ret < 0) {
@@ -638,7 +644,8 @@ static void coroutine_fn mirror_run(void *opaque)
          * We do so every SLICE_TIME nanoseconds, or when there is an error,
          * or when the source is clean, whichever comes first.
          */
-        if (qemu_clock_get_ns(QEMU_CLOCK_REALTIME) - last_pause_ns < SLICE_TIME &&
+        delta = qemu_clock_get_ns(QEMU_CLOCK_REALTIME) - s->last_pause_ns;
+        if (delta < SLICE_TIME &&
             s->common.iostatus == BLOCK_DEVICE_IO_STATUS_OK) {
             if (s->in_flight == MAX_IN_FLIGHT || s->buf_free_count == 0 ||
                 (cnt == 0 && s->in_flight > 0)) {
@@ -708,7 +715,7 @@ static void coroutine_fn mirror_run(void *opaque)
             s->common.cancelled = false;
             break;
         }
-        last_pause_ns = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
+        s->last_pause_ns = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
     }
 
 immediate_exit:
-- 
2.5.0

Re: [Qemu-devel] [PATCH v3 3/8] mirror: create mirror_throttle helper

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 813 bytes --]

On 07/14/2016 07:33 AM, Denis V. Lunev wrote:
> The patch also places last_pause_ns from stack in mirror_run into
> MirrorBlockJob structure. This helper will be useful in next patches.
> 
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> CC: Eric Blake <eblake@redhat.com>
> CC: Stefan Hajnoczi <stefanha@redhat.com>
> CC: Fam Zheng <famz@redhat.com>
> CC: Kevin Wolf <kwolf@redhat.com>
> CC: Max Reitz <mreitz@redhat.com>
> CC: Jeff Cody <jcody@redhat.com>
> ---
>  block/mirror.c | 31 +++++++++++++++++++------------
>  1 file changed, 19 insertions(+), 12 deletions(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

[Qemu-devel] [PATCH v3 4/8] mirror: create mirror_dirty_init helper for mirror_run

[Denis V. Lunev]

The code inside the helper will be extended in the next patch. mirror_run
itself is overbloated at the moment.

Signed-off-by: Denis V. Lunev <den@openvz.org>
Reviewed-by: Vladimir Sementsov-Ogievskiy<vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
CC: Kevin Wolf <kwolf@redhat.com>
CC: Max Reitz <mreitz@redhat.com>
CC: Jeff Cody <jcody@redhat.com>
CC: Eric Blake <eblake@redhat.com>
---
 block/mirror.c | 70 ++++++++++++++++++++++++++++++++++------------------------
 1 file changed, 41 insertions(+), 29 deletions(-)

diff --git a/block/mirror.c b/block/mirror.c
index 93e2896..3185ccb 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -525,18 +525,54 @@ static void mirror_throttle(MirrorBlockJob *s)
     }
 }
 
+static int coroutine_fn mirror_dirty_init(MirrorBlockJob *s)
+{
+    int64_t sector_num, end;
+    BlockDriverState *base = s->base;
+    BlockDriverState *bs = blk_bs(s->common.blk);
+    BlockDriverState *target_bs = blk_bs(s->target);
+    bool mark_all_dirty = base == NULL && !bdrv_has_zero_init(target_bs);
+    int ret, n;
+
+    end = s->bdev_length / BDRV_SECTOR_SIZE;
+
+    /* First part, loop on the sectors and initialize the dirty bitmap.  */
+    for (sector_num = 0; sector_num < end; ) {
+        /* Just to make sure we are not exceeding int limit. */
+        int nb_sectors = MIN(INT_MAX >> BDRV_SECTOR_BITS,
+                             end - sector_num);
+
+        mirror_throttle(s);
+
+        if (block_job_is_cancelled(&s->common)) {
+            return 0;
+        }
+
+        ret = bdrv_is_allocated_above(bs, base, sector_num, nb_sectors, &n);
+        if (ret < 0) {
+            return ret;
+        }
+
+        assert(n > 0);
+        if (ret == 1 || mark_all_dirty) {
+            bdrv_set_dirty_bitmap(s->dirty_bitmap, sector_num, n);
+        }
+        sector_num += n;
+    }
+    return 0;
+}
+
 static void coroutine_fn mirror_run(void *opaque)
 {
     MirrorBlockJob *s = opaque;
     MirrorExitData *data;
     BlockDriverState *bs = blk_bs(s->common.blk);
     BlockDriverState *target_bs = blk_bs(s->target);
-    int64_t sector_num, end, length;
+    int64_t length;
     BlockDriverInfo bdi;
     char backing_filename[2]; /* we only need 2 characters because we are only
                                  checking for a NULL string */
     int ret = 0;
-    int n;
     int target_cluster_size = BDRV_SECTOR_SIZE;
 
     if (block_job_is_cancelled(&s->common)) {
@@ -578,7 +614,6 @@ static void coroutine_fn mirror_run(void *opaque)
     s->target_cluster_sectors = target_cluster_size >> BDRV_SECTOR_BITS;
     s->max_iov = MIN(bs->bl.max_iov, target_bs->bl.max_iov);
 
-    end = s->bdev_length / BDRV_SECTOR_SIZE;
     s->buf = qemu_try_blockalign(bs, s->buf_size);
     if (s->buf == NULL) {
         ret = -ENOMEM;
@@ -589,32 +624,9 @@ static void coroutine_fn mirror_run(void *opaque)
 
     s->last_pause_ns = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
     if (!s->is_none_mode) {
-        /* First part, loop on the sectors and initialize the dirty bitmap.  */
-        BlockDriverState *base = s->base;
-        bool mark_all_dirty = s->base == NULL && !bdrv_has_zero_init(target_bs);
-
-        for (sector_num = 0; sector_num < end; ) {
-            /* Just to make sure we are not exceeding int limit. */
-            int nb_sectors = MIN(INT_MAX >> BDRV_SECTOR_BITS,
-                                 end - sector_num);
-
-            mirror_throttle(s);
-
-            if (block_job_is_cancelled(&s->common)) {
-                goto immediate_exit;
-            }
-
-            ret = bdrv_is_allocated_above(bs, base, sector_num, nb_sectors, &n);
-
-            if (ret < 0) {
-                goto immediate_exit;
-            }
-
-            assert(n > 0);
-            if (ret == 1 || mark_all_dirty) {
-                bdrv_set_dirty_bitmap(s->dirty_bitmap, sector_num, n);
-            }
-            sector_num += n;
+        ret = mirror_dirty_init(s);
+        if (ret < 0 || block_job_is_cancelled(&s->common)) {
+            goto immediate_exit;
         }
     }
 
-- 
2.5.0

Re: [Qemu-devel] [PATCH v3 4/8] mirror: create mirror_dirty_init helper for mirror_run

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1455 bytes --]

On 07/14/2016 07:33 AM, Denis V. Lunev wrote:
> The code inside the helper will be extended in the next patch. mirror_run
> itself is overbloated at the moment.
> 
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> Reviewed-by: Vladimir Sementsov-Ogievskiy<vsementsov@virtuozzo.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>

I did NOT give R-b on v2 3/7.  In particular, this patch has semantic
changes that I requested on v2, and so you want to make sure I re-review
it (I often skip re-reviewing a patch that has my R-b listed, on the
grounds that I'm trusting your judgment that it hasn't substantially
changed since my last time through it; but here, you have changed it
since last time).

> Reviewed-by: Fam Zheng <famz@redhat.com>
> CC: Stefan Hajnoczi <stefanha@redhat.com>
> CC: Kevin Wolf <kwolf@redhat.com>
> CC: Max Reitz <mreitz@redhat.com>
> CC: Jeff Cody <jcody@redhat.com>
> CC: Eric Blake <eblake@redhat.com>
> ---
>  block/mirror.c | 70 ++++++++++++++++++++++++++++++++++------------------------
>  1 file changed, 41 insertions(+), 29 deletions(-)

That said, I've looked through the changes (the rebase on top of moving
last_pause_ns to being a member of s, so that a single timestamp is
shared across both functions), and can now safely state:

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH v3 4/8] mirror: create mirror_dirty_init helper for mirror_run

[Denis V. Lunev]

On 07/14/2016 07:19 PM, Eric Blake wrote:
> On 07/14/2016 07:33 AM, Denis V. Lunev wrote:
>> The code inside the helper will be extended in the next patch. mirror_run
>> itself is overbloated at the moment.
>>
>> Signed-off-by: Denis V. Lunev <den@openvz.org>
>> Reviewed-by: Vladimir Sementsov-Ogievskiy<vsementsov@virtuozzo.com>
>> Reviewed-by: Eric Blake <eblake@redhat.com>
> I did NOT give R-b on v2 3/7.  In particular, this patch has semantic
> changes that I requested on v2, and so you want to make sure I re-review
> it (I often skip re-reviewing a patch that has my R-b listed, on the
> grounds that I'm trusting your judgment that it hasn't substantially
> changed since my last time through it; but here, you have changed it
> since last time).
>
sorry :(

[Qemu-devel] [PATCH v3 5/8] block: remove extra condition in bdrv_can_write_zeroes_with_unmap

[Denis V. Lunev]

All .bdrv_co_write_zeroes callbacks nowadays work perfectly even
with backing store attached. If future new callbacks would be unable to do
that - they have a chance to block this in bdrv_get_info().

Signed-off-by: Denis V. Lunev <den@openvz.org>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
CC: Kevin Wolf <kwolf@redhat.com>
CC: Max Reitz <mreitz@redhat.com>
CC: Jeff Cody <jcody@redhat.com>
---
 block.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block.c b/block.c
index 823ff1d..c2fb8bd 100644
--- a/block.c
+++ b/block.c
@@ -2834,7 +2834,7 @@ bool bdrv_can_write_zeroes_with_unmap(BlockDriverState *bs)
 {
     BlockDriverInfo bdi;
 
-    if (bs->backing || !(bs->open_flags & BDRV_O_UNMAP)) {
+    if (!(bs->open_flags & BDRV_O_UNMAP)) {
         return false;
     }
 
-- 
2.5.0

[Qemu-devel] [PATCH v3 6/8] mirror: optimize dirty bitmap filling in mirror_run a bit

[Denis V. Lunev]

There is no need to scan allocation tables if we have mark_all_dirty flag
set. Just mark it all dirty.

Signed-off-by: Denis V. Lunev <den@openvz.org>
Reviewed-by: Vladimir Sementsov-Ogievskiy<vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
CC: Kevin Wolf <kwolf@redhat.com>
CC: Max Reitz <mreitz@redhat.com>
CC: Jeff Cody <jcody@redhat.com>
---
 block/mirror.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/block/mirror.c b/block/mirror.c
index 3185ccb..d0fa457 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -531,11 +531,15 @@ static int coroutine_fn mirror_dirty_init(MirrorBlockJob *s)
     BlockDriverState *base = s->base;
     BlockDriverState *bs = blk_bs(s->common.blk);
     BlockDriverState *target_bs = blk_bs(s->target);
-    bool mark_all_dirty = base == NULL && !bdrv_has_zero_init(target_bs);
     int ret, n;
 
     end = s->bdev_length / BDRV_SECTOR_SIZE;
 
+    if (base == NULL && !bdrv_has_zero_init(target_bs)) {
+        bdrv_set_dirty_bitmap(s->dirty_bitmap, 0, end);
+        return 0;
+    }
+
     /* First part, loop on the sectors and initialize the dirty bitmap.  */
     for (sector_num = 0; sector_num < end; ) {
         /* Just to make sure we are not exceeding int limit. */
@@ -554,7 +558,7 @@ static int coroutine_fn mirror_dirty_init(MirrorBlockJob *s)
         }
 
         assert(n > 0);
-        if (ret == 1 || mark_all_dirty) {
+        if (ret == 1) {
             bdrv_set_dirty_bitmap(s->dirty_bitmap, sector_num, n);
         }
         sector_num += n;
-- 
2.5.0

[Qemu-devel] [PATCH v3 7/8] mirror: efficiently zero out target

[Denis V. Lunev]

With a bdrv_co_write_zeroes method on a target BDS and when this method
is working as indicated by the bdrv_can_write_zeroes_with_unmap(), zeroes
will not be placed into the wire. Thus the target could be very efficiently
zeroed out. This should be done with the largest chunk possible.

Signed-off-by: Denis V. Lunev <den@openvz.org>
Reviewed-by: Vladimir Sementsov-Ogievskiy<vsementsov@virtuozzo.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
CC: Kevin Wolf <kwolf@redhat.com>
CC: Max Reitz <mreitz@redhat.com>
CC: Jeff Cody <jcody@redhat.com>
CC: Eric Blake <eblake@redhat.com>
---
 block/mirror.c | 28 ++++++++++++++++++++++++++--
 1 file changed, 26 insertions(+), 2 deletions(-)

diff --git a/block/mirror.c b/block/mirror.c
index d0fa457..ae70c3b 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -536,8 +536,32 @@ static int coroutine_fn mirror_dirty_init(MirrorBlockJob *s)
     end = s->bdev_length / BDRV_SECTOR_SIZE;
 
     if (base == NULL && !bdrv_has_zero_init(target_bs)) {
-        bdrv_set_dirty_bitmap(s->dirty_bitmap, 0, end);
-        return 0;
+        if (!bdrv_can_write_zeroes_with_unmap(target_bs)) {
+            bdrv_set_dirty_bitmap(s->dirty_bitmap, 0, end);
+            return 0;
+        }
+
+        for (sector_num = 0; sector_num < end; ) {
+            int nb_sectors = MIN(end - sector_num,
+                QEMU_ALIGN_DOWN(INT_MAX, s->granularity) >> BDRV_SECTOR_BITS);
+
+            mirror_throttle(s);
+
+            if (block_job_is_cancelled(&s->common)) {
+                return 0;
+            }
+
+            if (s->in_flight >= MAX_IN_FLIGHT) {
+                trace_mirror_yield(s, s->in_flight, s->buf_free_count, -1);
+                mirror_wait_for_io(s);
+                continue;
+            }
+
+            mirror_do_zero_or_discard(s, sector_num, nb_sectors, false);
+            sector_num += nb_sectors;
+        }
+
+        mirror_drain(s);
     }
 
     /* First part, loop on the sectors and initialize the dirty bitmap.  */
-- 
2.5.0

[Qemu-devel] [PATCH v3 8/8] mirror: improve performance of mirroring of empty disk

[Denis V. Lunev]

We should not take into account zero blocks for delay calculations.
They are not read and thus IO throttling is not required. In the
other case VM migration with 16 Tb QCOW2 disk with 4 Gb of data takes
days.

Signed-off-by: Denis V. Lunev <den@openvz.org>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
CC: Kevin Wolf <kwolf@redhat.com>
CC: Max Reitz <mreitz@redhat.com>
CC: Jeff Cody <jcody@redhat.com>
CC: Eric Blake <eblake@redhat.com>
---
 block/mirror.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/block/mirror.c b/block/mirror.c
index ae70c3b..1787a6a 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -323,6 +323,7 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s)
     int nb_chunks = 1;
     int64_t end = s->bdev_length / BDRV_SECTOR_SIZE;
     int sectors_per_chunk = s->granularity >> BDRV_SECTOR_BITS;
+    bool write_zeroes_ok = bdrv_can_write_zeroes_with_unmap(blk_bs(s->target));
 
     sector_num = hbitmap_iter_next(&s->hbi);
     if (sector_num < 0) {
@@ -373,7 +374,7 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s)
     bitmap_set(s->in_flight_bitmap, sector_num / sectors_per_chunk, nb_chunks);
     while (nb_chunks > 0 && sector_num < end) {
         int ret;
-        int io_sectors;
+        int io_sectors, io_sectors_acct;
         BlockDriverState *file;
         enum MirrorMethod {
             MIRROR_METHOD_COPY,
@@ -410,12 +411,17 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s)
         switch (mirror_method) {
         case MIRROR_METHOD_COPY:
             io_sectors = mirror_do_read(s, sector_num, io_sectors);
+            io_sectors_acct = io_sectors;
             break;
         case MIRROR_METHOD_ZERO:
-            mirror_do_zero_or_discard(s, sector_num, io_sectors, false);
-            break;
         case MIRROR_METHOD_DISCARD:
-            mirror_do_zero_or_discard(s, sector_num, io_sectors, true);
+            mirror_do_zero_or_discard(s, sector_num, io_sectors,
+                                      mirror_method == MIRROR_METHOD_DISCARD);
+            if (write_zeroes_ok) {
+                io_sectors_acct = 0;
+            } else {
+                io_sectors_acct = io_sectors;
+            }
             break;
         default:
             abort();
@@ -423,7 +429,7 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s)
         assert(io_sectors);
         sector_num += io_sectors;
         nb_chunks -= DIV_ROUND_UP(io_sectors, sectors_per_chunk);
-        delay_ns += ratelimit_calculate_delay(&s->limit, io_sectors);
+        delay_ns += ratelimit_calculate_delay(&s->limit, io_sectors_acct);
     }
     return delay_ns;
 }
-- 
2.5.0

Re: [Qemu-devel] [PATCH v3 0/8] drive-mirror improvements

[Denis V. Lunev]

On 07/14/2016 04:33 PM, Denis V. Lunev wrote:
> This patchset contains patches dealing with known-to-be-zero areas in drive
> mirror from [PATCH 0/9] major rework of drive-mirror patchset.
>
> Changes from v2:
> - added mirror_throttle helper (patch 3) to address Eric' comment about
>    last_time_ns
> - comment tweaks (thank you, Eric)
> - marked mirror_dirty_init as couroutine_fn
>
> Changes from v1:
> - only patches dealing with zeroes remains
> - ported to current HEAD
> - fixed issue with dirty-bitmap, int length is changed with int64
> - fixed sectors_in_flight usage
> - patch 6 is reworked taken into account bugs found in active mirror
> - fixed patch 7
> - direct checking of .bdrv_co_write_zeroes is replaced with
>    bdrv_can_write_zeroes_with_unmap
>    - added fixes for bdrv_can_write_zeroes_with_unmap
>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Stefan Hajnoczi <stefanha@redhat.com>
> CC: Fam Zheng <famz@redhat.com>
> CC: Kevin Wolf <kwolf@redhat.com>
> CC: Max Reitz <mreitz@redhat.com>
> CC: Jeff Cody <jcody@redhat.com>
> CC: Eric Blake <eblake@redhat.com>
>
> Denis V. Lunev (8):
>    dirty-bitmap: operate with int64_t amount
>    mirror: make sectors_in_flight int64_t
>    mirror: create mirror_throttle helper
>    mirror: create mirror_dirty_init helper for mirror_run
>    block: remove extra condition in bdrv_can_write_zeroes_with_unmap
>    mirror: optimize dirty bitmap filling in mirror_run a bit
>    mirror: efficiently zero out target
>    mirror: improve performance of mirroring of empty disk
>
>   block.c                      |   2 +-
>   block/dirty-bitmap.c         |   6 +-
>   block/mirror.c               | 145 +++++++++++++++++++++++++++++--------------
>   include/block/block_int.h    |   2 +-
>   include/block/dirty-bitmap.h |   4 +-
>   5 files changed, 106 insertions(+), 53 deletions(-)
>
ping

Re: [Qemu-devel] [PATCH v3 0/8] drive-mirror improvements

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 2088 bytes --]

On Mon, Jul 18, 2016 at 06:37:15PM +0300, Denis V. Lunev wrote:
> On 07/14/2016 04:33 PM, Denis V. Lunev wrote:
> > This patchset contains patches dealing with known-to-be-zero areas in drive
> > mirror from [PATCH 0/9] major rework of drive-mirror patchset.
> > 
> > Changes from v2:
> > - added mirror_throttle helper (patch 3) to address Eric' comment about
> >    last_time_ns
> > - comment tweaks (thank you, Eric)
> > - marked mirror_dirty_init as couroutine_fn
> > 
> > Changes from v1:
> > - only patches dealing with zeroes remains
> > - ported to current HEAD
> > - fixed issue with dirty-bitmap, int length is changed with int64
> > - fixed sectors_in_flight usage
> > - patch 6 is reworked taken into account bugs found in active mirror
> > - fixed patch 7
> > - direct checking of .bdrv_co_write_zeroes is replaced with
> >    bdrv_can_write_zeroes_with_unmap
> >    - added fixes for bdrv_can_write_zeroes_with_unmap
> > 
> > Signed-off-by: Denis V. Lunev <den@openvz.org>
> > CC: Stefan Hajnoczi <stefanha@redhat.com>
> > CC: Fam Zheng <famz@redhat.com>
> > CC: Kevin Wolf <kwolf@redhat.com>
> > CC: Max Reitz <mreitz@redhat.com>
> > CC: Jeff Cody <jcody@redhat.com>
> > CC: Eric Blake <eblake@redhat.com>
> > 
> > Denis V. Lunev (8):
> >    dirty-bitmap: operate with int64_t amount
> >    mirror: make sectors_in_flight int64_t
> >    mirror: create mirror_throttle helper
> >    mirror: create mirror_dirty_init helper for mirror_run
> >    block: remove extra condition in bdrv_can_write_zeroes_with_unmap
> >    mirror: optimize dirty bitmap filling in mirror_run a bit
> >    mirror: efficiently zero out target
> >    mirror: improve performance of mirroring of empty disk
> > 
> >   block.c                      |   2 +-
> >   block/dirty-bitmap.c         |   6 +-
> >   block/mirror.c               | 145 +++++++++++++++++++++++++++++--------------
> >   include/block/block_int.h    |   2 +-
> >   include/block/dirty-bitmap.h |   4 +-
> >   5 files changed, 106 insertions(+), 53 deletions(-)
> > 
> ping

Jeff?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [Qemu-devel] [PATCH v3 0/8] drive-mirror improvements

[Jeff Cody]

On Thu, Jul 14, 2016 at 04:33:21PM +0300, Denis V. Lunev wrote:
> This patchset contains patches dealing with known-to-be-zero areas in drive
> mirror from [PATCH 0/9] major rework of drive-mirror patchset.
> 
> Changes from v2:
> - added mirror_throttle helper (patch 3) to address Eric' comment about
>   last_time_ns
> - comment tweaks (thank you, Eric)
> - marked mirror_dirty_init as couroutine_fn
> 
> Changes from v1:
> - only patches dealing with zeroes remains
> - ported to current HEAD
> - fixed issue with dirty-bitmap, int length is changed with int64
> - fixed sectors_in_flight usage
> - patch 6 is reworked taken into account bugs found in active mirror
> - fixed patch 7
> - direct checking of .bdrv_co_write_zeroes is replaced with
>   bdrv_can_write_zeroes_with_unmap
>   - added fixes for bdrv_can_write_zeroes_with_unmap
> 
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Stefan Hajnoczi <stefanha@redhat.com>
> CC: Fam Zheng <famz@redhat.com>
> CC: Kevin Wolf <kwolf@redhat.com>
> CC: Max Reitz <mreitz@redhat.com>
> CC: Jeff Cody <jcody@redhat.com>
> CC: Eric Blake <eblake@redhat.com>
> 
> Denis V. Lunev (8):
>   dirty-bitmap: operate with int64_t amount
>   mirror: make sectors_in_flight int64_t
>   mirror: create mirror_throttle helper
>   mirror: create mirror_dirty_init helper for mirror_run
>   block: remove extra condition in bdrv_can_write_zeroes_with_unmap
>   mirror: optimize dirty bitmap filling in mirror_run a bit
>   mirror: efficiently zero out target
>   mirror: improve performance of mirroring of empty disk
> 
>  block.c                      |   2 +-
>  block/dirty-bitmap.c         |   6 +-
>  block/mirror.c               | 145 +++++++++++++++++++++++++++++--------------
>  include/block/block_int.h    |   2 +-
>  include/block/dirty-bitmap.h |   4 +-
>  5 files changed, 106 insertions(+), 53 deletions(-)
> 
> -- 
> 2.5.0
> 

Thanks,

Applied to my block branch:

git://github.com/codyprime/qemu-kvm-jtc.git block

-Jeff