[Qemu-devel] [PATCHv3 1.8 0/9] qemu-img convert optimizations

[Qemu-devel] [PATCHv3 1.8 0/9] qemu-img convert optimizations

[Peter Lieven]

this series adds some optimizations for qemu-img during convert which
have been developed recently:
- skipping input based on get_block_status
- variable I/O buffer size
- align write requests to cluster_size

v2->v3:
  - added Paolos comments in Patch 1
  - changed the comment in patch 7 [Paolo]
  - remove the patch to add sector progress output
  - added a new patch to decrease the progress update interval.

v1->v2:
  - introduce opt_transfer_length in BlockLimits [Paolo]
  - remove knobs for iobuffer_size and alignment and
    use them unconditionally [Paolo]
  - calculate I/O buffer size by BlockLimits information [Paolo]
  - change the alignment patch to round down to the
    last and not to the next aligned sector [Paolo]
  - limit updates in the sector progress output
  - new patch to increase the default for min_sparse [Paolo]

Peter Lieven (9):
  qemu-img: add support for skipping zeroes in input during convert
  qemu-img: fix usage instruction for qemu-img convert
  block/iscsi: set bdi->cluster_size
  block: add opt_transfer_length to BlockLimits
  block/iscsi: set bs->bl.opt_transfer_length
  qemu-img: dynamically adjust iobuffer size during convert
  qemu-img: round down request length to an aligned sector
  qemu-img: increase min_sparse to 128 sectors (64kb)
  qemu-img: decrease progress update interval on convert

 block/iscsi.c             |   10 ++++
 include/block/block_int.h |    3 ++
 qemu-img.c                |  131 +++++++++++++++++++++++++++------------------
 qemu-img.texi             |    2 +-
 4 files changed, 93 insertions(+), 53 deletions(-)

-- 
1.7.9.5

[Qemu-devel] [PATCHv3 1.8 1/9] qemu-img: add support for skipping zeroes in input during convert

[Peter Lieven]

we currently do not check if a sector is allocated during convert.
This means if a sector is unallocated that we allocate a bounce
buffer of zeroes, find out its zero later and do not write it
in the best case. In the worst case this can lead to reading
blocks from a raw device (like iSCSI) altough we could easily
know via get_block_status that they are zero and simply skip them.

This patch also fixes the progress output not being at 100% after
a successful conversion.

Signed-off-by: Peter Lieven <pl@kamp.de>
---
 qemu-img.c |   80 +++++++++++++++++++++++++++++++-----------------------------
 1 file changed, 42 insertions(+), 38 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index dc0c2f0..49a123b 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1125,13 +1125,15 @@ out3:
 
 static int img_convert(int argc, char **argv)
 {
-    int c, ret = 0, n, n1, bs_n, bs_i, compress, cluster_size,
+    int c, n, n1, bs_n, bs_i, compress, cluster_size,
         cluster_sectors, skip_create;
+    int64_t ret = 0;
     int progress = 0, flags;
     const char *fmt, *out_fmt, *cache, *out_baseimg, *out_filename;
     BlockDriver *drv, *proto_drv;
     BlockDriverState **bs = NULL, *out_bs = NULL;
-    int64_t total_sectors, nb_sectors, sector_num, bs_offset;
+    int64_t total_sectors, nb_sectors, sector_num, bs_offset,
+            sector_num_next_status = 0;
     uint64_t bs_sectors;
     uint8_t * buf = NULL;
     const uint8_t *buf1;
@@ -1140,7 +1142,6 @@ static int img_convert(int argc, char **argv)
     QEMUOptionParameter *out_baseimg_param;
     char *options = NULL;
     const char *snapshot_name = NULL;
-    float local_progress = 0;
     int min_sparse = 8; /* Need at least 4k of zeros for sparse detection */
     bool quiet = false;
     Error *local_err = NULL;
@@ -1403,10 +1404,6 @@ static int img_convert(int argc, char **argv)
         sector_num = 0;
 
         nb_sectors = total_sectors;
-        if (nb_sectors != 0) {
-            local_progress = (float)100 /
-                (nb_sectors / MIN(nb_sectors, cluster_sectors));
-        }
 
         for(;;) {
             int64_t bs_num;
@@ -1464,7 +1461,7 @@ static int img_convert(int argc, char **argv)
                 }
             }
             sector_num += n;
-            qemu_progress_print(local_progress, 100);
+            qemu_progress_print(100.0 * sector_num / total_sectors, 0);
         }
         /* signal EOF to align */
         bdrv_write_compressed(out_bs, 0, NULL, 0);
@@ -1481,21 +1478,13 @@ static int img_convert(int argc, char **argv)
 
         sector_num = 0; // total number of sectors converted so far
         nb_sectors = total_sectors - sector_num;
-        if (nb_sectors != 0) {
-            local_progress = (float)100 /
-                (nb_sectors / MIN(nb_sectors, IO_BUF_SIZE / 512));
-        }
 
         for(;;) {
             nb_sectors = total_sectors - sector_num;
             if (nb_sectors <= 0) {
+                ret = 0;
                 break;
             }
-            if (nb_sectors >= (IO_BUF_SIZE / 512)) {
-                n = (IO_BUF_SIZE / 512);
-            } else {
-                n = nb_sectors;
-            }
 
             while (sector_num - bs_offset >= bs_sectors) {
                 bs_i ++;
@@ -1507,34 +1496,46 @@ static int img_convert(int argc, char **argv)
                    sector_num, bs_i, bs_offset, bs_sectors); */
             }
 
-            if (n > bs_offset + bs_sectors - sector_num) {
-                n = bs_offset + bs_sectors - sector_num;
-            }
-
-            /* If the output image is being created as a copy on write image,
-               assume that sectors which are unallocated in the input image
-               are present in both the output's and input's base images (no
-               need to copy them). */
-            if (out_baseimg) {
-                ret = bdrv_is_allocated(bs[bs_i], sector_num - bs_offset,
-                                        n, &n1);
+            if ((out_baseimg || has_zero_init) &&
+                sector_num >= sector_num_next_status) {
+                n = nb_sectors > INT_MAX ? INT_MAX : nb_sectors;
+                ret = bdrv_get_block_status(bs[bs_i], sector_num - bs_offset,
+                                            n, &n1);
                 if (ret < 0) {
-                    error_report("error while reading metadata for sector "
-                                 "%" PRId64 ": %s",
-                                 sector_num - bs_offset, strerror(-ret));
+                    error_report("error while reading block status of sector %"
+                                 PRId64 ": %s", sector_num - bs_offset,
+                                 strerror(-ret));
                     goto out;
                 }
-                if (!ret) {
+                /* If the output image is zero initialized, we are not working
+                 * on a shared base and the input is zero we can skip the next
+                 * n1 sectors */
+                if (has_zero_init && !out_baseimg && (ret & BDRV_BLOCK_ZERO)) {
                     sector_num += n1;
                     continue;
                 }
-                /* The next 'n1' sectors are allocated in the input image. Copy
-                   only those as they may be followed by unallocated sectors. */
-                n = n1;
-            } else {
-                n1 = n;
+                /* If the output image is being created as a copy on write
+                 * image, assume that sectors which are unallocated in the
+                 * input image are present in both the output's and input's
+                 * base images (no need to copy them). */
+                if (out_baseimg) {
+                    if (!(ret & BDRV_BLOCK_DATA)) {
+                        sector_num += n1;
+                        continue;
+                    }
+                    /* The next 'n1' sectors are allocated in the input image.
+                     * Copy only those as they may be followed by unallocated
+                     * sectors. */
+                    nb_sectors = n1;
+                }
+                /* avoid redundant callouts to get_block_status */
+                sector_num_next_status = sector_num + n1;
             }
 
+            n = MIN(nb_sectors, IO_BUF_SIZE / 512);
+            n = MIN(n, bs_sectors - (sector_num - bs_offset));
+            n1 = n;
+
             ret = bdrv_read(bs[bs_i], sector_num - bs_offset, buf, n);
             if (ret < 0) {
                 error_report("error while reading sector %" PRId64 ": %s",
@@ -1559,10 +1560,13 @@ static int img_convert(int argc, char **argv)
                 n -= n1;
                 buf1 += n1 * 512;
             }
-            qemu_progress_print(local_progress, 100);
+            qemu_progress_print(100.0 * sector_num / total_sectors, 0);
         }
     }
 out:
+    if (!ret) {
+        qemu_progress_print(100, 0);
+    }
     qemu_progress_end();
     free_option_parameters(create_options);
     free_option_parameters(param);
-- 
1.7.9.5

Re: [Qemu-devel] [PATCHv3 1.8 1/9] qemu-img: add support for skipping zeroes in input during convert

[Paolo Bonzini]

Il 27/11/2013 11:07, Peter Lieven ha scritto:
> we currently do not check if a sector is allocated during convert.
> This means if a sector is unallocated that we allocate a bounce
> buffer of zeroes, find out its zero later and do not write it
> in the best case. In the worst case this can lead to reading
> blocks from a raw device (like iSCSI) altough we could easily
> know via get_block_status that they are zero and simply skip them.
> 
> This patch also fixes the progress output not being at 100% after
> a successful conversion.
> 
> Signed-off-by: Peter Lieven <pl@kamp.de>
> ---
>  qemu-img.c |   80 +++++++++++++++++++++++++++++++-----------------------------
>  1 file changed, 42 insertions(+), 38 deletions(-)
> 
> diff --git a/qemu-img.c b/qemu-img.c
> index dc0c2f0..49a123b 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -1125,13 +1125,15 @@ out3:
>  
>  static int img_convert(int argc, char **argv)
>  {
> -    int c, ret = 0, n, n1, bs_n, bs_i, compress, cluster_size,
> +    int c, n, n1, bs_n, bs_i, compress, cluster_size,
>          cluster_sectors, skip_create;
> +    int64_t ret = 0;
>      int progress = 0, flags;
>      const char *fmt, *out_fmt, *cache, *out_baseimg, *out_filename;
>      BlockDriver *drv, *proto_drv;
>      BlockDriverState **bs = NULL, *out_bs = NULL;
> -    int64_t total_sectors, nb_sectors, sector_num, bs_offset;
> +    int64_t total_sectors, nb_sectors, sector_num, bs_offset,
> +            sector_num_next_status = 0;
>      uint64_t bs_sectors;
>      uint8_t * buf = NULL;
>      const uint8_t *buf1;
> @@ -1140,7 +1142,6 @@ static int img_convert(int argc, char **argv)
>      QEMUOptionParameter *out_baseimg_param;
>      char *options = NULL;
>      const char *snapshot_name = NULL;
> -    float local_progress = 0;
>      int min_sparse = 8; /* Need at least 4k of zeros for sparse detection */
>      bool quiet = false;
>      Error *local_err = NULL;
> @@ -1403,10 +1404,6 @@ static int img_convert(int argc, char **argv)
>          sector_num = 0;
>  
>          nb_sectors = total_sectors;
> -        if (nb_sectors != 0) {
> -            local_progress = (float)100 /
> -                (nb_sectors / MIN(nb_sectors, cluster_sectors));
> -        }
>  
>          for(;;) {
>              int64_t bs_num;
> @@ -1464,7 +1461,7 @@ static int img_convert(int argc, char **argv)
>                  }
>              }
>              sector_num += n;
> -            qemu_progress_print(local_progress, 100);
> +            qemu_progress_print(100.0 * sector_num / total_sectors, 0);
>          }
>          /* signal EOF to align */
>          bdrv_write_compressed(out_bs, 0, NULL, 0);
> @@ -1481,21 +1478,13 @@ static int img_convert(int argc, char **argv)
>  
>          sector_num = 0; // total number of sectors converted so far
>          nb_sectors = total_sectors - sector_num;
> -        if (nb_sectors != 0) {
> -            local_progress = (float)100 /
> -                (nb_sectors / MIN(nb_sectors, IO_BUF_SIZE / 512));
> -        }
>  
>          for(;;) {
>              nb_sectors = total_sectors - sector_num;
>              if (nb_sectors <= 0) {
> +                ret = 0;
>                  break;
>              }
> -            if (nb_sectors >= (IO_BUF_SIZE / 512)) {
> -                n = (IO_BUF_SIZE / 512);
> -            } else {
> -                n = nb_sectors;
> -            }
>  
>              while (sector_num - bs_offset >= bs_sectors) {
>                  bs_i ++;
> @@ -1507,34 +1496,46 @@ static int img_convert(int argc, char **argv)
>                     sector_num, bs_i, bs_offset, bs_sectors); */
>              }
>  
> -            if (n > bs_offset + bs_sectors - sector_num) {
> -                n = bs_offset + bs_sectors - sector_num;
> -            }
> -
> -            /* If the output image is being created as a copy on write image,
> -               assume that sectors which are unallocated in the input image
> -               are present in both the output's and input's base images (no
> -               need to copy them). */
> -            if (out_baseimg) {
> -                ret = bdrv_is_allocated(bs[bs_i], sector_num - bs_offset,
> -                                        n, &n1);
> +            if ((out_baseimg || has_zero_init) &&
> +                sector_num >= sector_num_next_status) {
> +                n = nb_sectors > INT_MAX ? INT_MAX : nb_sectors;
> +                ret = bdrv_get_block_status(bs[bs_i], sector_num - bs_offset,
> +                                            n, &n1);
>                  if (ret < 0) {
> -                    error_report("error while reading metadata for sector "
> -                                 "%" PRId64 ": %s",
> -                                 sector_num - bs_offset, strerror(-ret));
> +                    error_report("error while reading block status of sector %"
> +                                 PRId64 ": %s", sector_num - bs_offset,
> +                                 strerror(-ret));
>                      goto out;
>                  }
> -                if (!ret) {
> +                /* If the output image is zero initialized, we are not working
> +                 * on a shared base and the input is zero we can skip the next
> +                 * n1 sectors */
> +                if (has_zero_init && !out_baseimg && (ret & BDRV_BLOCK_ZERO)) {
>                      sector_num += n1;
>                      continue;
>                  }
> -                /* The next 'n1' sectors are allocated in the input image. Copy
> -                   only those as they may be followed by unallocated sectors. */
> -                n = n1;
> -            } else {
> -                n1 = n;
> +                /* If the output image is being created as a copy on write
> +                 * image, assume that sectors which are unallocated in the
> +                 * input image are present in both the output's and input's
> +                 * base images (no need to copy them). */
> +                if (out_baseimg) {
> +                    if (!(ret & BDRV_BLOCK_DATA)) {
> +                        sector_num += n1;
> +                        continue;
> +                    }
> +                    /* The next 'n1' sectors are allocated in the input image.
> +                     * Copy only those as they may be followed by unallocated
> +                     * sectors. */
> +                    nb_sectors = n1;
> +                }
> +                /* avoid redundant callouts to get_block_status */
> +                sector_num_next_status = sector_num + n1;
>              }
>  
> +            n = MIN(nb_sectors, IO_BUF_SIZE / 512);
> +            n = MIN(n, bs_sectors - (sector_num - bs_offset));
> +            n1 = n;
> +
>              ret = bdrv_read(bs[bs_i], sector_num - bs_offset, buf, n);
>              if (ret < 0) {
>                  error_report("error while reading sector %" PRId64 ": %s",
> @@ -1559,10 +1560,13 @@ static int img_convert(int argc, char **argv)
>                  n -= n1;
>                  buf1 += n1 * 512;
>              }
> -            qemu_progress_print(local_progress, 100);
> +            qemu_progress_print(100.0 * sector_num / total_sectors, 0);
>          }
>      }
>  out:
> +    if (!ret) {
> +        qemu_progress_print(100, 0);
> +    }
>      qemu_progress_end();
>      free_option_parameters(create_options);
>      free_option_parameters(param);
> 

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Re: [Qemu-devel] [PATCHv3 1.8 1/9] qemu-img: add support for skipping zeroes in input during convert

[Stefan Hajnoczi]

On Wed, Nov 27, 2013 at 11:07:01AM +0100, Peter Lieven wrote:
> +                /* If the output image is being created as a copy on write
> +                 * image, assume that sectors which are unallocated in the
> +                 * input image are present in both the output's and input's
> +                 * base images (no need to copy them). */
> +                if (out_baseimg) {
> +                    if (!(ret & BDRV_BLOCK_DATA)) {
> +                        sector_num += n1;
> +                        continue;
> +                    }
> +                    /* The next 'n1' sectors are allocated in the input image.
> +                     * Copy only those as they may be followed by unallocated
> +                     * sectors. */
> +                    nb_sectors = n1;
> +                }
> +                /* avoid redundant callouts to get_block_status */
> +                sector_num_next_status = sector_num + n1;

Can you explain when we need sector_num_next_status?  It's not clear to
me from this patch when we will loop around already knowing that blocks
are allocated.

Re: [Qemu-devel] [PATCHv3 1.8 1/9] qemu-img: add support for skipping zeroes in input during convert

[Peter Lieven]

Am 04.12.2013 17:46, schrieb Stefan Hajnoczi:
> On Wed, Nov 27, 2013 at 11:07:01AM +0100, Peter Lieven wrote:
>> +                /* If the output image is being created as a copy on write
>> +                 * image, assume that sectors which are unallocated in the
>> +                 * input image are present in both the output's and input's
>> +                 * base images (no need to copy them). */
>> +                if (out_baseimg) {
>> +                    if (!(ret & BDRV_BLOCK_DATA)) {
>> +                        sector_num += n1;
>> +                        continue;
>> +                    }
>> +                    /* The next 'n1' sectors are allocated in the input image.
>> +                     * Copy only those as they may be followed by unallocated
>> +                     * sectors. */
>> +                    nb_sectors = n1;
>> +                }
>> +                /* avoid redundant callouts to get_block_status */
>> +                sector_num_next_status = sector_num + n1;
> Can you explain when we need sector_num_next_status?  It's not clear to
> me from this patch when we will loop around already knowing that blocks
> are allocated.
We call get_block_status with MIN(INT_MAX, nb_sectors). So we might
receive an allocation status for a huge area. Later we trim the request
size to MIN(iobuf_size, nb_sectors) and eventually align the request.

For example take a fully allocated image on an iSCSI san. I can easily get
that information with the first get_block_status call, but I repeat these
calls over and over and in case of the iSCSI SAN these calls are quite
expensive.

Peter

Re: [Qemu-devel] [PATCHv3 1.8 1/9] qemu-img: add support for skipping zeroes in input during convert

[Stefan Hajnoczi]

On Wed, Dec 04, 2013 at 05:51:20PM +0100, Peter Lieven wrote:
> Am 04.12.2013 17:46, schrieb Stefan Hajnoczi:
> > On Wed, Nov 27, 2013 at 11:07:01AM +0100, Peter Lieven wrote:
> >> +                /* If the output image is being created as a copy on write
> >> +                 * image, assume that sectors which are unallocated in the
> >> +                 * input image are present in both the output's and input's
> >> +                 * base images (no need to copy them). */
> >> +                if (out_baseimg) {
> >> +                    if (!(ret & BDRV_BLOCK_DATA)) {
> >> +                        sector_num += n1;
> >> +                        continue;
> >> +                    }
> >> +                    /* The next 'n1' sectors are allocated in the input image.
> >> +                     * Copy only those as they may be followed by unallocated
> >> +                     * sectors. */
> >> +                    nb_sectors = n1;
> >> +                }
> >> +                /* avoid redundant callouts to get_block_status */
> >> +                sector_num_next_status = sector_num + n1;
> > Can you explain when we need sector_num_next_status?  It's not clear to
> > me from this patch when we will loop around already knowing that blocks
> > are allocated.
> We call get_block_status with MIN(INT_MAX, nb_sectors). So we might
> receive an allocation status for a huge area. Later we trim the request
> size to MIN(iobuf_size, nb_sectors) and eventually align the request.
> 
> For example take a fully allocated image on an iSCSI san. I can easily get
> that information with the first get_block_status call, but I repeat these
> calls over and over and in case of the iSCSI SAN these calls are quite
> expensive.

Makes sense, thanks!

[Qemu-devel] [PATCHv3 1.8 2/9] qemu-img: fix usage instruction for qemu-img convert

[Peter Lieven]

Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Peter Lieven <pl@kamp.de>
---
 qemu-img.c |    1 -
 1 file changed, 1 deletion(-)

diff --git a/qemu-img.c b/qemu-img.c
index 49a123b..0c2e557 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -105,7 +105,6 @@ static void help(void)
            "       conversion. If the number of bytes is 0, the source will not be scanned for\n"
            "       unallocated or zero sectors, and the destination image will always be\n"
            "       fully allocated\n"
-           "       images will always be fully allocated\n"
            "  '--output' takes the format in which the output must be done (human or json)\n"
            "  '-n' skips the target volume creation (useful if the volume is created\n"
            "       prior to running qemu-img)\n"
-- 
1.7.9.5

[Qemu-devel] [PATCHv3 1.8 3/9] block/iscsi: set bdi->cluster_size

[Peter Lieven]

this patch aims to set bdi->cluster_size to the internal page size
of the iscsi target so that enabled callers can align requests
properly.

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Peter Lieven <pl@kamp.de>
---
 block/iscsi.c |    7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/block/iscsi.c b/block/iscsi.c
index 93fee6d..75d6b87 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -1580,6 +1580,13 @@ static int iscsi_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
     IscsiLun *iscsilun = bs->opaque;
     bdi->unallocated_blocks_are_zero = !!iscsilun->lbprz;
     bdi->can_write_zeroes_with_unmap = iscsilun->lbprz && iscsilun->lbp.lbpws;
+    /* Guess the internal cluster (page) size of the iscsi target by the means
+     * of opt_unmap_gran. Transfer the unmap granularity only if it has a
+     * reasonable size for bdi->cluster_size */
+    if (iscsilun->bl.opt_unmap_gran * iscsilun->block_size >= 64 * 1024 &&
+        iscsilun->bl.opt_unmap_gran * iscsilun->block_size <= 16 * 1024 * 1024) {
+        bdi->cluster_size = iscsilun->bl.opt_unmap_gran * iscsilun->block_size;
+    }
     return 0;
 }
 
-- 
1.7.9.5

[Qemu-devel] [PATCHv3 1.8 4/9] block: add opt_transfer_length to BlockLimits

[Peter Lieven]

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Peter Lieven <pl@kamp.de>
---
 include/block/block_int.h |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/block/block_int.h b/include/block/block_int.h
index 95140b6..a09f1b5 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -242,6 +242,9 @@ typedef struct BlockLimits {
 
     /* optimal alignment for write zeroes requests in sectors */
     int64_t write_zeroes_alignment;
+
+    /* optimal transfer length in sectors */
+    int opt_transfer_length;
 } BlockLimits;
 
 /*
-- 
1.7.9.5

[Qemu-devel] [PATCHv3 1.8 5/9] block/iscsi: set bs->bl.opt_transfer_length

[Peter Lieven]

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Peter Lieven <pl@kamp.de>
---
 block/iscsi.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/block/iscsi.c b/block/iscsi.c
index 75d6b87..829d444 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -1457,6 +1457,9 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
         }
         bs->bl.write_zeroes_alignment = sector_lun2qemu(iscsilun->bl.opt_unmap_gran,
                                                         iscsilun);
+
+        bs->bl.opt_transfer_length = sector_lun2qemu(iscsilun->bl.opt_xfer_len,
+                                                     iscsilun);
     }
 
 #if defined(LIBISCSI_FEATURE_NOP_COUNTER)
-- 
1.7.9.5

[Qemu-devel] [PATCHv3 1.8 6/9] qemu-img: dynamically adjust iobuffer size during convert

[Peter Lieven]

since the convert process is basically a sync operation it might
be benificial in some case to change the hardcoded I/O buffer
size to a greater value.

This patch increases the I/O buffer size if the output
driver advertises an optimal transfer length or discard alignment
that is greater than the default buffer size of 2M.

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Peter Lieven <pl@kamp.de>
---
 qemu-img.c |   16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index 0c2e557..15f423b 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1135,6 +1135,7 @@ static int img_convert(int argc, char **argv)
             sector_num_next_status = 0;
     uint64_t bs_sectors;
     uint8_t * buf = NULL;
+    size_t bufsectors = IO_BUF_SIZE / BDRV_SECTOR_SIZE;
     const uint8_t *buf1;
     BlockDriverInfo bdi;
     QEMUOptionParameter *param = NULL, *create_options = NULL;
@@ -1371,7 +1372,16 @@ static int img_convert(int argc, char **argv)
     bs_i = 0;
     bs_offset = 0;
     bdrv_get_geometry(bs[0], &bs_sectors);
-    buf = qemu_blockalign(out_bs, IO_BUF_SIZE);
+
+    /* increase bufsectors from the default 4096 (2M) if opt_transfer_length
+     * or discard_alignment of the out_bs is greater. Limit to 32768 (16MB)
+     * as maximum. */
+    bufsectors = MIN(32768,
+                     MAX(bufsectors, MAX(out_bs->bl.opt_transfer_length,
+                                         out_bs->bl.discard_alignment))
+                    );
+
+    buf = qemu_blockalign(out_bs, bufsectors * BDRV_SECTOR_SIZE);
 
     if (skip_create) {
         int64_t output_length = bdrv_getlength(out_bs);
@@ -1394,7 +1404,7 @@ static int img_convert(int argc, char **argv)
             goto out;
         }
         cluster_size = bdi.cluster_size;
-        if (cluster_size <= 0 || cluster_size > IO_BUF_SIZE) {
+        if (cluster_size <= 0 || cluster_size > bufsectors * BDRV_SECTOR_SIZE) {
             error_report("invalid cluster size");
             ret = -1;
             goto out;
@@ -1531,7 +1541,7 @@ static int img_convert(int argc, char **argv)
                 sector_num_next_status = sector_num + n1;
             }
 
-            n = MIN(nb_sectors, IO_BUF_SIZE / 512);
+            n = MIN(nb_sectors, bufsectors);
             n = MIN(n, bs_sectors - (sector_num - bs_offset));
             n1 = n;
 
-- 
1.7.9.5

Re: [Qemu-devel] [PATCHv3 1.8 6/9] qemu-img: dynamically adjust iobuffer size during convert

[Eric Blake]

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

On 11/27/2013 03:07 AM, Peter Lieven wrote:
> since the convert process is basically a sync operation it might
> be benificial in some case to change the hardcoded I/O buffer

s/benificial/beneficial/

> size to a greater value.
> 
> This patch increases the I/O buffer size if the output
> driver advertises an optimal transfer length or discard alignment
> that is greater than the default buffer size of 2M.
> 

-- 
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: 621 bytes --]

[Qemu-devel] [PATCHv3 1.8 7/9] qemu-img: round down request length to an aligned sector

[Peter Lieven]

this patch shortens requests to end at an aligned sector so that
the next request starts aligned.

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Peter Lieven <pl@kamp.de>
---
 qemu-img.c |   34 ++++++++++++++++++++++++----------
 1 file changed, 24 insertions(+), 10 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index 15f423b..9bb1f6f 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1124,8 +1124,7 @@ out3:
 
 static int img_convert(int argc, char **argv)
 {
-    int c, n, n1, bs_n, bs_i, compress, cluster_size,
-        cluster_sectors, skip_create;
+    int c, n, n1, bs_n, bs_i, compress, cluster_sectors, skip_create;
     int64_t ret = 0;
     int progress = 0, flags;
     const char *fmt, *out_fmt, *cache, *out_baseimg, *out_filename;
@@ -1397,19 +1396,21 @@ static int img_convert(int argc, char **argv)
         }
     }
 
+    cluster_sectors = 0;
+    ret = bdrv_get_info(out_bs, &bdi);
+    if (ret < 0 && compress) {
+        error_report("could not get block driver info");
+        goto out;
+    } else {
+        cluster_sectors = bdi.cluster_size / BDRV_SECTOR_SIZE;
+    }
+
     if (compress) {
-        ret = bdrv_get_info(out_bs, &bdi);
-        if (ret < 0) {
-            error_report("could not get block driver info");
-            goto out;
-        }
-        cluster_size = bdi.cluster_size;
-        if (cluster_size <= 0 || cluster_size > bufsectors * BDRV_SECTOR_SIZE) {
+        if (cluster_sectors <= 0 || cluster_sectors > bufsectors) {
             error_report("invalid cluster size");
             ret = -1;
             goto out;
         }
-        cluster_sectors = cluster_size >> 9;
         sector_num = 0;
 
         nb_sectors = total_sectors;
@@ -1542,6 +1543,19 @@ static int img_convert(int argc, char **argv)
             }
 
             n = MIN(nb_sectors, bufsectors);
+
+            /* round down request length to an aligned sector, but
+             * do not bother doing this on short requests. They happen
+             * when we found an all-zero area, and the next sector to
+             * write will not be sector_num + n. */
+            if (cluster_sectors > 0 && n >= cluster_sectors) {
+                int64_t next_aligned_sector = (sector_num + n);
+                next_aligned_sector -= next_aligned_sector % cluster_sectors;
+                if (sector_num + n > next_aligned_sector) {
+                    n = next_aligned_sector - sector_num;
+                }
+            }
+
             n = MIN(n, bs_sectors - (sector_num - bs_offset));
             n1 = n;
 
-- 
1.7.9.5

Re: [Qemu-devel] [PATCHv3 1.8 7/9] qemu-img: round down request length to an aligned sector

[Stefan Hajnoczi]

On Wed, Nov 27, 2013 at 11:07:07AM +0100, Peter Lieven wrote:
> @@ -1397,19 +1396,21 @@ static int img_convert(int argc, char **argv)
>          }
>      }
>  
> +    cluster_sectors = 0;
> +    ret = bdrv_get_info(out_bs, &bdi);
> +    if (ret < 0 && compress) {
> +        error_report("could not get block driver info");
> +        goto out;
> +    } else {
> +        cluster_sectors = bdi.cluster_size / BDRV_SECTOR_SIZE;
> +    }

Why do we only report error if 'compress' is set?  cluster_sectors must
be valid and we cannot guarantee that if bdrv_get_info() failed.

Re: [Qemu-devel] [PATCHv3 1.8 7/9] qemu-img: round down request length to an aligned sector

[Peter Lieven]

Am 04.12.2013 16:49, schrieb Stefan Hajnoczi:
> On Wed, Nov 27, 2013 at 11:07:07AM +0100, Peter Lieven wrote:
>> @@ -1397,19 +1396,21 @@ static int img_convert(int argc, char **argv)
>>          }
>>      }
>>  
>> +    cluster_sectors = 0;
>> +    ret = bdrv_get_info(out_bs, &bdi);
>> +    if (ret < 0 && compress) {
>> +        error_report("could not get block driver info");
>> +        goto out;
>> +    } else {
>> +        cluster_sectors = bdi.cluster_size / BDRV_SECTOR_SIZE;
>> +    }
> Why do we only report error if 'compress' is set?  cluster_sectors must
> be valid and we cannot guarantee that if bdrv_get_info() failed.
You mean this should be:

+    if (ret < 0) {
+        if (compress) {
+            error_report("could not get block driver info");
+            goto out;
+        }
+    } else {
+        cluster_sectors = bdi.cluster_size / BDRV_SECTOR_SIZE;
+    }


if cluster_sectors is 0 the alignment logic is skipped, but we cannot
guarantee that bdi is zero and stays zero if the call fails.

can you fix that when you pick up the patch?

Peter

Re: [Qemu-devel] [PATCHv3 1.8 7/9] qemu-img: round down request length to an aligned sector

[Stefan Hajnoczi]

On Wed, Dec 04, 2013 at 04:56:19PM +0100, Peter Lieven wrote:
> Am 04.12.2013 16:49, schrieb Stefan Hajnoczi:
> > On Wed, Nov 27, 2013 at 11:07:07AM +0100, Peter Lieven wrote:
> >> @@ -1397,19 +1396,21 @@ static int img_convert(int argc, char **argv)
> >>          }
> >>      }
> >>  
> >> +    cluster_sectors = 0;
> >> +    ret = bdrv_get_info(out_bs, &bdi);
> >> +    if (ret < 0 && compress) {
> >> +        error_report("could not get block driver info");
> >> +        goto out;
> >> +    } else {
> >> +        cluster_sectors = bdi.cluster_size / BDRV_SECTOR_SIZE;
> >> +    }
> > Why do we only report error if 'compress' is set?  cluster_sectors must
> > be valid and we cannot guarantee that if bdrv_get_info() failed.
> You mean this should be:
> 
> +    if (ret < 0) {
> +        if (compress) {
> +            error_report("could not get block driver info");
> +            goto out;
> +        }
> +    } else {
> +        cluster_sectors = bdi.cluster_size / BDRV_SECTOR_SIZE;
> +    }
> 
> 
> if cluster_sectors is 0 the alignment logic is skipped, but we cannot
> guarantee that bdi is zero and stays zero if the call fails.
> 
> can you fix that when you pick up the patch?

Sure.

Stefan

[Qemu-devel] [PATCHv3 1.8 8/9] qemu-img: increase min_sparse to 128 sectors (64kb)

[Peter Lieven]

Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Peter Lieven <pl@kamp.de>
---
 qemu-img.c    |    4 ++--
 qemu-img.texi |    2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index 9bb1f6f..8b5f3da 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -100,7 +100,7 @@ static void help(void)
            "  '-h' with or without a command shows this help and lists the supported formats\n"
            "  '-p' show progress of command (only certain commands)\n"
            "  '-q' use Quiet mode - do not print any output (except errors)\n"
-           "  '-S' indicates the consecutive number of bytes (defaults to 4k) that must\n"
+           "  '-S' indicates the consecutive number of bytes (defaults to 64k) that must\n"
            "       contain only zeros for qemu-img to create a sparse image during\n"
            "       conversion. If the number of bytes is 0, the source will not be scanned for\n"
            "       unallocated or zero sectors, and the destination image will always be\n"
@@ -1141,7 +1141,7 @@ static int img_convert(int argc, char **argv)
     QEMUOptionParameter *out_baseimg_param;
     char *options = NULL;
     const char *snapshot_name = NULL;
-    int min_sparse = 8; /* Need at least 4k of zeros for sparse detection */
+    int min_sparse = 128; /* Need at least 64k of zeros for sparse detection */
     bool quiet = false;
     Error *local_err = NULL;
 
diff --git a/qemu-img.texi b/qemu-img.texi
index da36975..4add6ce 100644
--- a/qemu-img.texi
+++ b/qemu-img.texi
@@ -193,7 +193,7 @@ Image conversion is also useful to get smaller image when using a
 growable format such as @code{qcow} or @code{cow}: the empty sectors
 are detected and suppressed from the destination image.
 
-@var{sparse_size} indicates the consecutive number of bytes (defaults to 4k)
+@var{sparse_size} indicates the consecutive number of bytes (defaults to 64k)
 that must contain only zeros for qemu-img to create a sparse image during
 conversion. If @var{sparse_size} is 0, the source will not be scanned for
 unallocated or zero sectors, and the destination image will always be
-- 
1.7.9.5

Re: [Qemu-devel] [PATCHv3 1.8 8/9] qemu-img: increase min_sparse to 128 sectors (64kb)

[Stefan Hajnoczi]

On Wed, Nov 27, 2013 at 11:07:08AM +0100, Peter Lieven wrote:
> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Peter Lieven <pl@kamp.de>
> ---
>  qemu-img.c    |    4 ++--
>  qemu-img.texi |    2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/qemu-img.c b/qemu-img.c
> index 9bb1f6f..8b5f3da 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -100,7 +100,7 @@ static void help(void)
>             "  '-h' with or without a command shows this help and lists the supported formats\n"
>             "  '-p' show progress of command (only certain commands)\n"
>             "  '-q' use Quiet mode - do not print any output (except errors)\n"
> -           "  '-S' indicates the consecutive number of bytes (defaults to 4k) that must\n"
> +           "  '-S' indicates the consecutive number of bytes (defaults to 64k) that must\n"
>             "       contain only zeros for qemu-img to create a sparse image during\n"
>             "       conversion. If the number of bytes is 0, the source will not be scanned for\n"
>             "       unallocated or zero sectors, and the destination image will always be\n"
> @@ -1141,7 +1141,7 @@ static int img_convert(int argc, char **argv)
>      QEMUOptionParameter *out_baseimg_param;
>      char *options = NULL;
>      const char *snapshot_name = NULL;
> -    int min_sparse = 8; /* Need at least 4k of zeros for sparse detection */
> +    int min_sparse = 128; /* Need at least 64k of zeros for sparse detection */
>      bool quiet = false;
>      Error *local_err = NULL;

I guess a sane size would be cluster size.  For a raw file 4 KB is
reasonable since that's the file system block size.

Is it necessary to increase to 64 KB here?

Re: [Qemu-devel] [PATCHv3 1.8 8/9] qemu-img: increase min_sparse to 128 sectors (64kb)

[Peter Lieven]

Am 04.12.2013 17:43, schrieb Stefan Hajnoczi:
> On Wed, Nov 27, 2013 at 11:07:08AM +0100, Peter Lieven wrote:
>> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
>> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
>> Signed-off-by: Peter Lieven <pl@kamp.de>
>> ---
>>  qemu-img.c    |    4 ++--
>>  qemu-img.texi |    2 +-
>>  2 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/qemu-img.c b/qemu-img.c
>> index 9bb1f6f..8b5f3da 100644
>> --- a/qemu-img.c
>> +++ b/qemu-img.c
>> @@ -100,7 +100,7 @@ static void help(void)
>>             "  '-h' with or without a command shows this help and lists the supported formats\n"
>>             "  '-p' show progress of command (only certain commands)\n"
>>             "  '-q' use Quiet mode - do not print any output (except errors)\n"
>> -           "  '-S' indicates the consecutive number of bytes (defaults to 4k) that must\n"
>> +           "  '-S' indicates the consecutive number of bytes (defaults to 64k) that must\n"
>>             "       contain only zeros for qemu-img to create a sparse image during\n"
>>             "       conversion. If the number of bytes is 0, the source will not be scanned for\n"
>>             "       unallocated or zero sectors, and the destination image will always be\n"
>> @@ -1141,7 +1141,7 @@ static int img_convert(int argc, char **argv)
>>      QEMUOptionParameter *out_baseimg_param;
>>      char *options = NULL;
>>      const char *snapshot_name = NULL;
>> -    int min_sparse = 8; /* Need at least 4k of zeros for sparse detection */
>> +    int min_sparse = 128; /* Need at least 64k of zeros for sparse detection */
>>      bool quiet = false;
>>      Error *local_err = NULL;
> I guess a sane size would be cluster size.  For a raw file 4 KB is
> reasonable since that's the file system block size.
in case of iscsi the cluster size could be much too high as for example
my storage has a cluster_size of 15MB.
>
> Is it necessary to increase to 64 KB here?
No, its indepent of the rest. Paolo suggested to increase it and I can confirm
that for my usage case its faster than 4K.

I would drop this patch for now.

Peter

Re: [Qemu-devel] [PATCHv3 1.8 8/9] qemu-img: increase min_sparse to 128 sectors (64kb)

[Eric Blake]

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

On 12/04/2013 09:46 AM, Peter Lieven wrote:

>> I guess a sane size would be cluster size.  For a raw file 4 KB is
>> reasonable since that's the file system block size.
> in case of iscsi the cluster size could be much too high as for example
> my storage has a cluster_size of 15MB.
>>
>> Is it necessary to increase to 64 KB here?
> No, its indepent of the rest. Paolo suggested to increase it and I can confirm
> that for my usage case its faster than 4K.

At least on NTFS file systems, 64k is the minimum size of a hole in a
sparse file.  While many file systems support smaller holes, there are
definitely systems where trying to detect smaller holes only results in
wasted efforts.  Is it worth making the default dynamic based on stat()
information regarding optimum IO size for the given destination file system?

-- 
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: 621 bytes --]

Re: [Qemu-devel] [PATCHv3 1.8 8/9] qemu-img: increase min_sparse to 128 sectors (64kb)

[Peter Lieven]

> Am 05.12.2013 um 03:12 schrieb Eric Blake <eblake@redhat.com>:
> 
> On 12/04/2013 09:46 AM, Peter Lieven wrote:
> 
>>> I guess a sane size would be cluster size.  For a raw file 4 KB is
>>> reasonable since that's the file system block size.
>> in case of iscsi the cluster size could be much too high as for example
>> my storage has a cluster_size of 15MB.
>>> 
>>> Is it necessary to increase to 64 KB here?
>> No, its indepent of the rest. Paolo suggested to increase it and I can confirm
>> that for my usage case its faster than 4K.
> 
> At least on NTFS file systems, 64k is the minimum size of a hole in a
> sparse file.  While many file systems support smaller holes, there are
> definitely systems where trying to detect smaller holes only results in
> wasted efforts.  Is it worth making the default dynamic based on stat()
> information regarding optimum IO size for the given destination file system?

it is definetely worth it, but i would require additional work and testing. the current code does not create holes that are aligned to min_sparse and min_sparse has to be limited to a reasonable size. and i wonder if the right value is bs->bl.opt_transfer_lenght, bs->bl.discard_alignment or bdi->cluster_size/9. maybe depepnding on if its a cow Image or not.

i can look at this. but i would leave the patch out for now.

Peter

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

Re: [Qemu-devel] [PATCHv3 1.8 8/9] qemu-img: increase min_sparse to 128 sectors (64kb)

[Stefan Hajnoczi]

On Thu, Dec 05, 2013 at 05:55:22AM +0100, Peter Lieven wrote:
> 
> 
> > Am 05.12.2013 um 03:12 schrieb Eric Blake <eblake@redhat.com>:
> > 
> > On 12/04/2013 09:46 AM, Peter Lieven wrote:
> > 
> >>> I guess a sane size would be cluster size.  For a raw file 4 KB is
> >>> reasonable since that's the file system block size.
> >> in case of iscsi the cluster size could be much too high as for example
> >> my storage has a cluster_size of 15MB.
> >>> 
> >>> Is it necessary to increase to 64 KB here?
> >> No, its indepent of the rest. Paolo suggested to increase it and I can confirm
> >> that for my usage case its faster than 4K.
> > 
> > At least on NTFS file systems, 64k is the minimum size of a hole in a
> > sparse file.  While many file systems support smaller holes, there are
> > definitely systems where trying to detect smaller holes only results in
> > wasted efforts.  Is it worth making the default dynamic based on stat()
> > information regarding optimum IO size for the given destination file system?
> 
> it is definetely worth it, but i would require additional work and testing. the current code does not create holes that are aligned to min_sparse and min_sparse has to be limited to a reasonable size. and i wonder if the right value is bs->bl.opt_transfer_lenght, bs->bl.discard_alignment or bdi->cluster_size/9. maybe depepnding on if its a cow Image or not.
> 
> i can look at this. but i would leave the patch out for now.

Okay, I'll drop this patch for now.  It can be improved in a separate
series.

Stefan

[Qemu-devel] [PATCHv3 1.8 9/9] qemu-img: decrease progress update interval on convert

[Peter Lieven]

when doing very large jobs updating the progress only every 2%
is too rare.

Signed-off-by: Peter Lieven <pl@kamp.de>
---
 qemu-img.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qemu-img.c b/qemu-img.c
index 8b5f3da..be72274 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1227,7 +1227,7 @@ static int img_convert(int argc, char **argv)
     out_filename = argv[argc - 1];
 
     /* Initialize before goto out */
-    qemu_progress_init(progress, 2.0);
+    qemu_progress_init(progress, 1.0);
 
     if (options && is_help_option(options)) {
         ret = print_block_option_help(out_filename, out_fmt);
-- 
1.7.9.5

Re: [Qemu-devel] [PATCHv3 1.8 0/9] qemu-img convert optimizations

[Stefan Hajnoczi]

On Wed, Nov 27, 2013 at 11:07:00AM +0100, Peter Lieven wrote:
> this series adds some optimizations for qemu-img during convert which
> have been developed recently:
> - skipping input based on get_block_status
> - variable I/O buffer size
> - align write requests to cluster_size
> 
> v2->v3:
>   - added Paolos comments in Patch 1
>   - changed the comment in patch 7 [Paolo]
>   - remove the patch to add sector progress output
>   - added a new patch to decrease the progress update interval.
> 
> v1->v2:
>   - introduce opt_transfer_length in BlockLimits [Paolo]
>   - remove knobs for iobuffer_size and alignment and
>     use them unconditionally [Paolo]
>   - calculate I/O buffer size by BlockLimits information [Paolo]
>   - change the alignment patch to round down to the
>     last and not to the next aligned sector [Paolo]
>   - limit updates in the sector progress output
>   - new patch to increase the default for min_sparse [Paolo]
> 
> Peter Lieven (9):
>   qemu-img: add support for skipping zeroes in input during convert
>   qemu-img: fix usage instruction for qemu-img convert
>   block/iscsi: set bdi->cluster_size
>   block: add opt_transfer_length to BlockLimits
>   block/iscsi: set bs->bl.opt_transfer_length
>   qemu-img: dynamically adjust iobuffer size during convert
>   qemu-img: round down request length to an aligned sector
>   qemu-img: increase min_sparse to 128 sectors (64kb)
>   qemu-img: decrease progress update interval on convert
> 
>  block/iscsi.c             |   10 ++++
>  include/block/block_int.h |    3 ++
>  qemu-img.c                |  131 +++++++++++++++++++++++++++------------------
>  qemu-img.texi             |    2 +-
>  4 files changed, 93 insertions(+), 53 deletions(-)

Merged all except patch 8/9.

Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan

Re: [Qemu-devel] [PATCHv3 1.8 0/9] qemu-img convert optimizations

[Peter Lieven]

Am 05.12.2013 13:15, schrieb Stefan Hajnoczi:
> On Wed, Nov 27, 2013 at 11:07:00AM +0100, Peter Lieven wrote:
>> this series adds some optimizations for qemu-img during convert which
>> have been developed recently:
>> - skipping input based on get_block_status
>> - variable I/O buffer size
>> - align write requests to cluster_size
>>
>> v2->v3:
>>   - added Paolos comments in Patch 1
>>   - changed the comment in patch 7 [Paolo]
>>   - remove the patch to add sector progress output
>>   - added a new patch to decrease the progress update interval.
>>
>> v1->v2:
>>   - introduce opt_transfer_length in BlockLimits [Paolo]
>>   - remove knobs for iobuffer_size and alignment and
>>     use them unconditionally [Paolo]
>>   - calculate I/O buffer size by BlockLimits information [Paolo]
>>   - change the alignment patch to round down to the
>>     last and not to the next aligned sector [Paolo]
>>   - limit updates in the sector progress output
>>   - new patch to increase the default for min_sparse [Paolo]
>>
>> Peter Lieven (9):
>>   qemu-img: add support for skipping zeroes in input during convert
>>   qemu-img: fix usage instruction for qemu-img convert
>>   block/iscsi: set bdi->cluster_size
>>   block: add opt_transfer_length to BlockLimits
>>   block/iscsi: set bs->bl.opt_transfer_length
>>   qemu-img: dynamically adjust iobuffer size during convert
>>   qemu-img: round down request length to an aligned sector
>>   qemu-img: increase min_sparse to 128 sectors (64kb)
>>   qemu-img: decrease progress update interval on convert
>>
>>  block/iscsi.c             |   10 ++++
>>  include/block/block_int.h |    3 ++
>>  qemu-img.c                |  131 +++++++++++++++++++++++++++------------------
>>  qemu-img.texi             |    2 +-
>>  4 files changed, 93 insertions(+), 53 deletions(-)
> Merged all except patch 8/9.
>
> Thanks, applied to my block tree:
> https://github.com/stefanha/qemu/commits/block
Thank you.

As discussed I've sent the follow-up patch

    [PATCH] qemu-img: make progress output more accurate during convert

to the list some minutes ago.

Peter