From feb36b08913089f97ecd1515bd1e77f2ebf40a68 Mon Sep 17 00:00:00 2001 Message-Id: From: Kevin Wolf Date: Tue, 3 Mar 2015 11:08:28 -0500 Subject: [CHANGE] qemu-img: add support for skipping zeroes in input during convert To: rhvirt-patches@redhat.com, jen@redhat.com RH-Author: Kevin Wolf Message-id: <1425380908-5070-1-git-send-email-kwolf@redhat.com> Patchwork-id: 64107 O-Subject: [RHEL-6.7 qemu-kvm PATCH v2] qemu-img: add support for skipping zeroes in input during convert Bugzilla: 1006681 RH-Acked-by: Stefan Hajnoczi RH-Acked-by: Max Reitz RH-Acked-by: Paolo Bonzini From: Peter Lieven Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1006681 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 Reviewed-by: Paolo Bonzini Signed-off-by: Stefan Hajnoczi (cherry picked from commit 13c28af87a5541a9b09a59502b876a1725fb502d) Signed-off-by: Jeff E. Nelson Conflicts: qemu-img.c Trivial conflict because upstream declares a few more local variables. v2: Fix copy length for the loop iteration (must be limited to where the next block status starts). This is downstream-only, upstream will get a different fix. [Max] Signed-off-by: Kevin Wolf --- Brew: http://brewweb.devel.redhat.com/brew/taskinfo?taskID=8799875 === git-compile-check output === Performing a test compile on 1 patches 1/1: compiling: 9f46436: qemu-img: add support for skipping zeroes in input during convert All patches in rhel6/master..HEAD compiled successfully! === git-backport-diff output === Key: [----] : patches are identical [####] : number of functional differences between upstream/downstream patch [down] : patch is downstream-only The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively 001/1:[0008] [FC] 'qemu-img: add support for skipping zeroes in input during convert' --- qemu-img.c | 84 ++++++++++++++++++++++++++++++++++---------------------------- 1 file changed, 46 insertions(+), 38 deletions(-) Signed-off-by: Jeff E. Nelson --- qemu-img.c | 84 ++++++++++++++++++++++++++++++++++---------------------------- 1 file changed, 46 insertions(+), 38 deletions(-) diff --git a/qemu-img.c b/qemu-img.c index 8252173..a736d9d 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -1078,12 +1078,14 @@ out3: static int img_convert(int argc, char **argv) { - int c, ret = 0, n, n1, bs_n, bs_i, compress, cluster_size, cluster_sectors; + int c, n, n1, bs_n, bs_i, compress, cluster_size, cluster_sectors; + int64_t ret = 0; int progress = 0, flags, src_flags; const char *fmt, *out_fmt, *cache, *src_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; @@ -1091,7 +1093,6 @@ static int img_convert(int argc, char **argv) QEMUOptionParameter *param = NULL, *create_options = NULL; QEMUOptionParameter *out_baseimg_param; char *options = NULL; - float local_progress = 0; int min_sparse = 8; /* Need at least 4k of zeros for sparse detection */ fmt = NULL; @@ -1314,10 +1315,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; @@ -1379,7 +1376,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); @@ -1388,21 +1385,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 ++; @@ -1414,34 +1403,50 @@ 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); + if (sector_num_next_status) { + assert(sector_num_next_status > sector_num); + n = MIN(n, sector_num_next_status - sector_num); + } + 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", @@ -1466,10 +1471,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); -- 2.1.0