All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: qemu-devel@nongnu.org
Cc: kwolf@redhat.com, qemu-block@nongnu.org, mreitz@redhat.com
Subject: [Qemu-devel] [PATCH v7 3/3] qcow2: Discard unaligned tail when wiping image
Date: Thu, 30 Mar 2017 17:36:45 -0500	[thread overview]
Message-ID: <20170330223645.349-4-eblake@redhat.com> (raw)
In-Reply-To: <20170330223645.349-1-eblake@redhat.com>

The previous commit pointed out a subtle difference between the
fast and slow path of qcow2_make_empty(), where we failed to discard
the final (partial) cluster of an unaligned image.

The problem stems from the fact that qcow2_discard_clusters() was
silently ignoring sub-cluster head and tail on unaligned requests.
A quick audit of all callers shows that qcow2_snapshot_create() has
always passed a cluster-aligned request since the call was added
in commit 1ebf561; qcow2_co_pdiscard() has passed a cluster-aligned
request since commit ecdbead taught the block layer about preferred
discard alignment; and qcow2_make_empty() was fixed to pass an
aligned start (but not necessarily end) in commit a3e1505.

Asserting that the start is always aligned also points out that we
now have a dead check: rounding the end offset down can never result
in a value less than the aligned start offset (the check was rendered
dead with commit ecdbead).  Meanwhile, we do not want to round the
end cluster down in the one case of the end offset matching the
(unaligned) file size - that final partial cluster should still be
discarded.

With those fixes in place, we can adjust the testsuite so that
qemu-iotests 97 and 176 are once again identical for qcow2, showing
that fast and slow paths are back in sync.

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

---
v7: new patch
---
 block/qcow2-cluster.c      | 10 ++++------
 tests/qemu-iotests/176.out |  2 +-
 2 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 78c11d4..100398c 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -1519,12 +1519,10 @@ int qcow2_discard_clusters(BlockDriverState *bs, uint64_t offset,

     end_offset = offset + (nb_sectors << BDRV_SECTOR_BITS);

-    /* Round start up and end down */
-    offset = align_offset(offset, s->cluster_size);
-    end_offset = start_of_cluster(s, end_offset);
-
-    if (offset > end_offset) {
-        return 0;
+    /* The caller must cluster-align start; round end down except at EOF */
+    assert(QEMU_IS_ALIGNED(offset, s->cluster_size));
+    if (end_offset != bs->total_sectors * BDRV_SECTOR_SIZE) {
+        end_offset = start_of_cluster(s, end_offset);
     }

     nb_clusters = size_to_clusters(s, end_offset - offset);
diff --git a/tests/qemu-iotests/176.out b/tests/qemu-iotests/176.out
index 990a41c..6271fa7 100644
--- a/tests/qemu-iotests/176.out
+++ b/tests/qemu-iotests/176.out
@@ -35,7 +35,7 @@ Offset          Length          File
 Offset          Length          File
 0x7ffd0000      0x10000         TEST_DIR/t.IMGFMT.base
 0x7ffe0000      0x20000         TEST_DIR/t.IMGFMT.itmd
-0x83400000      0x200           TEST_DIR/t.IMGFMT
+0x83400000      0x200           TEST_DIR/t.IMGFMT.itmd

 === Test pass 1 ===

-- 
2.9.3

  parent reply	other threads:[~2017-03-30 22:36 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-30 22:36 [Qemu-devel] [PATCH for-2.9 v7 0/3] Fix wipe of unaligned qcow2 image [was add blkdebug tests] Eric Blake
2017-03-30 22:36 ` [Qemu-devel] [PATCH v7 1/3] iotests: fix 097 when run with qcow Eric Blake
2017-03-30 22:36 ` [Qemu-devel] [PATCH v7 2/3] iotests: Improve image-clear tests on non-aligned image Eric Blake
2017-03-31 12:40   ` Max Reitz
2017-03-30 22:36 ` Eric Blake [this message]
2017-03-31 12:51   ` [Qemu-devel] [PATCH v7 3/3] qcow2: Discard unaligned tail when wiping image Max Reitz
2017-03-31 13:56     ` Eric Blake
2017-03-31 14:01       ` Max Reitz
2017-03-31 14:11         ` Eric Blake

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=20170330223645.349-4-eblake@redhat.com \
    --to=eblake@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=mreitz@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.