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 v6 03/10] qcow2: Assert that cluster operations are aligned
Date: Tue,  7 Mar 2017 20:54:21 -0600	[thread overview]
Message-ID: <20170308025428.1037-4-eblake@redhat.com> (raw)
In-Reply-To: <20170308025428.1037-1-eblake@redhat.com>

qcow2_discard_clusters() is set up to silently ignore sub-cluster
head or tail on unaligned requests.  However, it is easy to audit
the various callers: qcow2_snapshot_create() has always passed
aligned data since the call was introduced in 1ebf561;
qcow2_co_pdiscard() has passed aligned clusters since commit
ecdbead taught the block layer the preferred discard alignment (the
block layer can still pass sub-cluster values, but those are
handled directly in qcow2_co_pdiscard()); and qcow2_make_empty()
was fixed to pass aligned clusters in commit a3e1505.  Replace
rounding with assertions to hold us to the tighter contract,
eliminating the now-impossible case of an early exit for a
sub-cluster request.

qcow2_zero_clusters() has always been called with cluster-aligned
arguments from its lone caller qcow2_co_pwrite_zeroes() (like
qcow2_co_pdiscard(), the caller takes care of sub-cluster requests
from the block layer; and qcow2_zero_clusters() would have
misbehaved on unaligned requests), but it deserves the same
assertion for symmetry.

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

---
v6: avoid assertion on non-cluster-aligned image, use s->cluster_sectors
to avoid a shift, drop R-b
v5: no change
v4: new patch
---
 block/qcow2-cluster.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 78c11d4..046fbd8 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -1519,13 +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;
-    }
+    /* Caller must pass aligned values, except at image end */
+    assert(QEMU_IS_ALIGNED(offset, s->cluster_size));
+    assert(QEMU_IS_ALIGNED(end_offset, s->cluster_size) ||
+           end_offset == bs->total_sectors << BDRV_SECTOR_BITS);

     nb_clusters = size_to_clusters(s, end_offset - offset);

@@ -1600,6 +1597,10 @@ int qcow2_zero_clusters(BlockDriverState *bs, uint64_t offset, int nb_sectors,
     uint64_t nb_clusters;
     int ret;

+    /* Caller must pass aligned values */
+    assert(QEMU_IS_ALIGNED(offset, s->cluster_size));
+    assert(QEMU_IS_ALIGNED(nb_sectors, s->cluster_sectors));
+
     /* The zero flag is only supported by version 3 and newer */
     if (s->qcow_version < 3) {
         return -ENOTSUP;
-- 
2.9.3

  parent reply	other threads:[~2017-03-08  2:54 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-08  2:54 [Qemu-devel] [PATCH v6 00/10] add blkdebug tests Eric Blake
2017-03-08  2:54 ` [Qemu-devel] [PATCH v6 01/10] iotests: fix 097 when run with qcow Eric Blake
2017-03-08  2:54 ` [Qemu-devel] [PATCH v6 02/10] iotests: Improve image-clear tests on non-aligned image Eric Blake
2017-03-13 22:00   ` Max Reitz
2017-03-30  2:13     ` Eric Blake
2017-03-30 18:39       ` Eric Blake
2017-03-30 21:25         ` Max Reitz
2017-03-08  2:54 ` Eric Blake [this message]
2017-03-13 22:07   ` [Qemu-devel] [PATCH v6 03/10] qcow2: Assert that cluster operations are aligned Max Reitz
2017-03-08  2:54 ` [Qemu-devel] [PATCH v6 04/10] qcow2: Discard/zero clusters by byte count Eric Blake
2017-03-08  2:54 ` [Qemu-devel] [PATCH v6 05/10] blkdebug: Sanity check block layer guarantees Eric Blake
2017-03-08  2:54 ` [Qemu-devel] [PATCH v6 06/10] blkdebug: Refactor error injection Eric Blake
2017-03-13 22:16   ` Max Reitz
2017-03-08  2:54 ` [Qemu-devel] [PATCH v6 07/10] blkdebug: Add pass-through write_zero and discard support Eric Blake
2017-03-13 22:20   ` Max Reitz
2017-03-08  2:54 ` [Qemu-devel] [PATCH v6 08/10] blkdebug: Simplify override logic Eric Blake
2017-03-08  2:54 ` [Qemu-devel] [PATCH v6 09/10] blkdebug: Add ability to override unmap geometries Eric Blake
2017-03-13 22:22   ` Max Reitz
2017-03-08  2:54 ` [Qemu-devel] [PATCH v6 10/10] tests: Add coverage for recent block geometry fixes Eric Blake
2017-03-08  8:40 ` [Qemu-devel] [PATCH v6 00/10] add blkdebug tests Kevin Wolf

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=20170308025428.1037-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.