qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Kevin Wolf <kwolf@redhat.com>
To: qemu-block@nongnu.org
Cc: kwolf@redhat.com, qemu-devel@nongnu.org
Subject: [Qemu-devel] [PULL 21/35] qemu-img: Change img_rebase() to be byte-based
Date: Thu, 26 Oct 2017 15:17:27 +0200	[thread overview]
Message-ID: <20171026131741.5059-22-kwolf@redhat.com> (raw)
In-Reply-To: <20171026131741.5059-1-kwolf@redhat.com>

From: Eric Blake <eblake@redhat.com>

In the continuing quest to make more things byte-based, change
the internal iteration of img_rebase().  We can finally drop the
TODO assertion added earlier, now that the entire algorithm is
byte-based and no longer has to shift from bytes to sectors.

Most of the change is mechanical ('num_sectors' becomes 'size',
'sector' becomes 'offset', 'n' goes from sectors to bytes); some
of it is also a cleanup (use of MIN() instead of open-coding,
loss of variable 'count' added earlier in commit d6a644bb).

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 qemu-img.c | 84 +++++++++++++++++++++++++-------------------------------------
 1 file changed, 34 insertions(+), 50 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index c6b6263853..994365c6a4 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -3248,70 +3248,58 @@ static int img_rebase(int argc, char **argv)
      * the image is the same as the original one at any time.
      */
     if (!unsafe) {
-        int64_t num_sectors;
-        int64_t old_backing_num_sectors;
-        int64_t new_backing_num_sectors = 0;
-        uint64_t sector;
-        int n;
-        int64_t count;
+        int64_t size;
+        int64_t old_backing_size;
+        int64_t new_backing_size = 0;
+        uint64_t offset;
+        int64_t n;
         float local_progress = 0;
 
         buf_old = blk_blockalign(blk, IO_BUF_SIZE);
         buf_new = blk_blockalign(blk, IO_BUF_SIZE);
 
-        num_sectors = blk_nb_sectors(blk);
-        if (num_sectors < 0) {
+        size = blk_getlength(blk);
+        if (size < 0) {
             error_report("Could not get size of '%s': %s",
-                         filename, strerror(-num_sectors));
+                         filename, strerror(-size));
             ret = -1;
             goto out;
         }
-        old_backing_num_sectors = blk_nb_sectors(blk_old_backing);
-        if (old_backing_num_sectors < 0) {
+        old_backing_size = blk_getlength(blk_old_backing);
+        if (old_backing_size < 0) {
             char backing_name[PATH_MAX];
 
             bdrv_get_backing_filename(bs, backing_name, sizeof(backing_name));
             error_report("Could not get size of '%s': %s",
-                         backing_name, strerror(-old_backing_num_sectors));
+                         backing_name, strerror(-old_backing_size));
             ret = -1;
             goto out;
         }
         if (blk_new_backing) {
-            new_backing_num_sectors = blk_nb_sectors(blk_new_backing);
-            if (new_backing_num_sectors < 0) {
+            new_backing_size = blk_getlength(blk_new_backing);
+            if (new_backing_size < 0) {
                 error_report("Could not get size of '%s': %s",
-                             out_baseimg, strerror(-new_backing_num_sectors));
+                             out_baseimg, strerror(-new_backing_size));
                 ret = -1;
                 goto out;
             }
         }
 
-        if (num_sectors != 0) {
-            local_progress = (float)100 /
-                (num_sectors / MIN(num_sectors, IO_BUF_SIZE / 512));
+        if (size != 0) {
+            local_progress = (float)100 / (size / MIN(size, IO_BUF_SIZE));
         }
 
-        for (sector = 0; sector < num_sectors; sector += n) {
-
-            /* How many sectors can we handle with the next read? */
-            if (sector + (IO_BUF_SIZE / 512) <= num_sectors) {
-                n = (IO_BUF_SIZE / 512);
-            } else {
-                n = num_sectors - sector;
-            }
+        for (offset = 0; offset < size; offset += n) {
+            /* How many bytes can we handle with the next read? */
+            n = MIN(IO_BUF_SIZE, size - offset);
 
             /* If the cluster is allocated, we don't need to take action */
-            ret = bdrv_is_allocated(bs, sector << BDRV_SECTOR_BITS,
-                                    n << BDRV_SECTOR_BITS, &count);
+            ret = bdrv_is_allocated(bs, offset, n, &n);
             if (ret < 0) {
                 error_report("error while reading image metadata: %s",
                              strerror(-ret));
                 goto out;
             }
-            /* TODO relax this once bdrv_is_allocated does not enforce
-             * sector alignment */
-            assert(QEMU_IS_ALIGNED(count, BDRV_SECTOR_SIZE));
-            n = count >> BDRV_SECTOR_BITS;
             if (ret) {
                 continue;
             }
@@ -3320,30 +3308,28 @@ static int img_rebase(int argc, char **argv)
              * Read old and new backing file and take into consideration that
              * backing files may be smaller than the COW image.
              */
-            if (sector >= old_backing_num_sectors) {
-                memset(buf_old, 0, n * BDRV_SECTOR_SIZE);
+            if (offset >= old_backing_size) {
+                memset(buf_old, 0, n);
             } else {
-                if (sector + n > old_backing_num_sectors) {
-                    n = old_backing_num_sectors - sector;
+                if (offset + n > old_backing_size) {
+                    n = old_backing_size - offset;
                 }
 
-                ret = blk_pread(blk_old_backing, sector << BDRV_SECTOR_BITS,
-                                buf_old, n << BDRV_SECTOR_BITS);
+                ret = blk_pread(blk_old_backing, offset, buf_old, n);
                 if (ret < 0) {
                     error_report("error while reading from old backing file");
                     goto out;
                 }
             }
 
-            if (sector >= new_backing_num_sectors || !blk_new_backing) {
-                memset(buf_new, 0, n * BDRV_SECTOR_SIZE);
+            if (offset >= new_backing_size || !blk_new_backing) {
+                memset(buf_new, 0, n);
             } else {
-                if (sector + n > new_backing_num_sectors) {
-                    n = new_backing_num_sectors - sector;
+                if (offset + n > new_backing_size) {
+                    n = new_backing_size - offset;
                 }
 
-                ret = blk_pread(blk_new_backing, sector << BDRV_SECTOR_BITS,
-                                buf_new, n << BDRV_SECTOR_BITS);
+                ret = blk_pread(blk_new_backing, offset, buf_new, n);
                 if (ret < 0) {
                     error_report("error while reading from new backing file");
                     goto out;
@@ -3353,15 +3339,13 @@ static int img_rebase(int argc, char **argv)
             /* If they differ, we need to write to the COW file */
             uint64_t written = 0;
 
-            while (written < n * BDRV_SECTOR_SIZE) {
+            while (written < n) {
                 int64_t pnum;
 
-                if (compare_buffers(buf_old + written,
-                                    buf_new + written,
-                                    n * BDRV_SECTOR_SIZE - written, &pnum))
+                if (compare_buffers(buf_old + written, buf_new + written,
+                                    n - written, &pnum))
                 {
-                    ret = blk_pwrite(blk,
-                                     (sector << BDRV_SECTOR_BITS) + written,
+                    ret = blk_pwrite(blk, offset + written,
                                      buf_old + written, pnum, 0);
                     if (ret < 0) {
                         error_report("Error while writing to COW image: %s",
-- 
2.13.6

  parent reply	other threads:[~2017-10-26 13:18 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-26 13:17 [Qemu-devel] [PULL 00/35] Block layer patches Kevin Wolf
2017-10-26 13:17 ` [Qemu-devel] [PULL 01/35] block: don't add 'driver' to options when referring to backing via node name Kevin Wolf
2017-10-26 13:17 ` [Qemu-devel] [PULL 02/35] qemu-iotests: Test backing_fmt with backing node reference Kevin Wolf
2017-10-26 13:17 ` [Qemu-devel] [PULL 03/35] block: Allow NULL file for bdrv_get_block_status() Kevin Wolf
2017-10-26 13:17 ` [Qemu-devel] [PULL 04/35] block: Add flag to avoid wasted work in bdrv_is_allocated() Kevin Wolf
2017-10-26 13:17 ` [Qemu-devel] [PULL 05/35] block: Make bdrv_round_to_clusters() signature more useful Kevin Wolf
2017-10-26 13:17 ` [Qemu-devel] [PULL 06/35] qcow2: Switch is_zero_sectors() to byte-based Kevin Wolf
2017-10-26 13:17 ` [Qemu-devel] [PULL 07/35] block: Switch bdrv_make_zero() " Kevin Wolf
2017-10-26 13:17 ` [Qemu-devel] [PULL 08/35] qemu-img: Switch get_block_status() " Kevin Wolf
2017-10-26 13:17 ` [Qemu-devel] [PULL 09/35] block: Convert bdrv_get_block_status() to bytes Kevin Wolf
2017-10-26 13:17 ` [Qemu-devel] [PULL 10/35] block: Switch bdrv_co_get_block_status() to byte-based Kevin Wolf
2017-10-26 13:17 ` [Qemu-devel] [PULL 11/35] block: Switch BdrvCoGetBlockStatusData " Kevin Wolf
2017-10-26 13:17 ` [Qemu-devel] [PULL 12/35] block: Switch bdrv_common_block_status_above() " Kevin Wolf
2017-10-26 13:17 ` [Qemu-devel] [PULL 13/35] block: Switch bdrv_co_get_block_status_above() " Kevin Wolf
2017-10-26 13:17 ` [Qemu-devel] [PULL 14/35] block: Convert bdrv_get_block_status_above() to bytes Kevin Wolf
2017-10-26 13:17 ` [Qemu-devel] [PULL 15/35] qemu-img: Simplify logic in img_compare() Kevin Wolf
2017-10-26 13:17 ` [Qemu-devel] [PULL 16/35] qemu-img: Speed up compare on pre-allocated larger file Kevin Wolf
2017-10-26 13:17 ` [Qemu-devel] [PULL 17/35] qemu-img: Add find_nonzero() Kevin Wolf
2017-10-26 13:17 ` [Qemu-devel] [PULL 18/35] qemu-img: Drop redundant error message in compare Kevin Wolf
2017-10-26 13:17 ` [Qemu-devel] [PULL 19/35] qemu-img: Change check_empty_sectors() to byte-based Kevin Wolf
2017-10-26 13:17 ` [Qemu-devel] [PULL 20/35] qemu-img: Change compare_sectors() to be byte-based Kevin Wolf
2017-10-26 13:17 ` Kevin Wolf [this message]
2017-10-26 13:17 ` [Qemu-devel] [PULL 22/35] qemu-img: Change img_compare() " Kevin Wolf
2017-10-26 13:17 ` [Qemu-devel] [PULL 23/35] block: Align block status requests Kevin Wolf
2017-10-26 13:17 ` [Qemu-devel] [PULL 24/35] block: Reduce bdrv_aligned_preadv() rounding Kevin Wolf
2017-10-26 13:17 ` [Qemu-devel] [PULL 25/35] qcow2: Reduce is_zero() rounding Kevin Wolf
2017-10-26 13:17 ` [Qemu-devel] [PULL 26/35] qemu-io: Relax 'alloc' now that block-status doesn't assert Kevin Wolf
2017-10-26 13:17 ` [Qemu-devel] [PULL 27/35] qemu-img.1: Image invalidation on qemu-img commit Kevin Wolf
2017-10-26 13:17 ` [Qemu-devel] [PULL 28/35] qcow2: Use BDRV_SECTOR_BITS instead of its literal value Kevin Wolf
2017-10-26 13:17 ` [Qemu-devel] [PULL 29/35] iotests: Add test for dataplane mirroring Kevin Wolf
2017-10-26 13:17 ` [Qemu-devel] [PULL 30/35] iotests: Pull _filter_actual_image_size from 67/87 Kevin Wolf
2017-10-26 13:17 ` [Qemu-devel] [PULL 31/35] iotests: Filter actual image size in 184 and 191 Kevin Wolf
2017-10-26 13:17 ` [Qemu-devel] [PULL 32/35] qcow2: Emit errp when truncating the image tail Kevin Wolf
2017-10-26 13:17 ` [Qemu-devel] [PULL 33/35] qcow2: Fix unaligned preallocated truncation Kevin Wolf
2017-10-26 13:17 ` [Qemu-devel] [PULL 34/35] qcow2: Always execute preallocate() in a coroutine Kevin Wolf
2017-10-26 13:17 ` [Qemu-devel] [PULL 35/35] iotests: Add cluster_size=64k to 125 Kevin Wolf
2017-10-27 10:13 ` [Qemu-devel] [PULL 00/35] Block layer patches Peter Maydell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20171026131741.5059-22-kwolf@redhat.com \
    --to=kwolf@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).