All of lore.kernel.org
 help / color / mirror / Atom feed
From: John Snow <jsnow@redhat.com>
To: Peter Maydell <peter.maydell@linaro.org>, qemu-devel@nongnu.org
Cc: Fam Zheng <fam@euphon.net>, Kevin Wolf <kwolf@redhat.com>,
	Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>,
	qemu-block@nongnu.org, Juan Quintela <quintela@redhat.com>,
	libvir-list@redhat.com, John Snow <jsnow@redhat.com>,
	"Dr. David Alan Gilbert" <dgilbert@redhat.com>,
	Max Reitz <mreitz@redhat.com>,
	Stefan Hajnoczi <stefanha@redhat.com>,
	Markus Armbruster <armbru@redhat.com>
Subject: [PULL 01/19] util/hbitmap: strict hbitmap_reset
Date: Fri, 11 Oct 2019 17:25:32 -0400	[thread overview]
Message-ID: <20191011212550.27269-2-jsnow@redhat.com> (raw)
In-Reply-To: <20191011212550.27269-1-jsnow@redhat.com>

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

hbitmap_reset has an unobvious property: it rounds requested region up.
It may provoke bugs, like in recently fixed write-blocking mode of
mirror: user calls reset on unaligned region, not keeping in mind that
there are possible unrelated dirty bytes, covered by rounded-up region
and information of this unrelated "dirtiness" will be lost.

Make hbitmap_reset strict: assert that arguments are aligned, allowing
only one exception when @start + @count == hb->orig_size. It's needed
to comfort users of hbitmap_next_dirty_area, which cares about
hb->orig_size.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20190806152611.280389-1-vsementsov@virtuozzo.com>
[Maintainer edit: Max's suggestions from on-list. --js]
Signed-off-by: John Snow <jsnow@redhat.com>
---
 include/qemu/hbitmap.h | 5 +++++
 tests/test-hbitmap.c   | 2 +-
 util/hbitmap.c         | 4 ++++
 3 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/include/qemu/hbitmap.h b/include/qemu/hbitmap.h
index 4afbe6292e3..1bf944ca3d1 100644
--- a/include/qemu/hbitmap.h
+++ b/include/qemu/hbitmap.h
@@ -132,6 +132,11 @@ void hbitmap_set(HBitmap *hb, uint64_t start, uint64_t count);
  * @count: Number of bits to reset.
  *
  * Reset a consecutive range of bits in an HBitmap.
+ * @start and @count must be aligned to bitmap granularity. The only exception
+ * is resetting the tail of the bitmap: @count may be equal to hb->orig_size -
+ * @start, in this case @count may be not aligned. The sum of @start + @count is
+ * allowed to be greater than hb->orig_size, but only if @start < hb->orig_size
+ * and @start + @count = ALIGN_UP(hb->orig_size, granularity).
  */
 void hbitmap_reset(HBitmap *hb, uint64_t start, uint64_t count);
 
diff --git a/tests/test-hbitmap.c b/tests/test-hbitmap.c
index eed5d288cbc..e1f867085f4 100644
--- a/tests/test-hbitmap.c
+++ b/tests/test-hbitmap.c
@@ -423,7 +423,7 @@ static void test_hbitmap_granularity(TestHBitmapData *data,
     hbitmap_test_check(data, 0);
     hbitmap_test_set(data, 0, 3);
     g_assert_cmpint(hbitmap_count(data->hb), ==, 4);
-    hbitmap_test_reset(data, 0, 1);
+    hbitmap_test_reset(data, 0, 2);
     g_assert_cmpint(hbitmap_count(data->hb), ==, 2);
 }
 
diff --git a/util/hbitmap.c b/util/hbitmap.c
index fd44c897ab0..757d39e360a 100644
--- a/util/hbitmap.c
+++ b/util/hbitmap.c
@@ -476,6 +476,10 @@ void hbitmap_reset(HBitmap *hb, uint64_t start, uint64_t count)
     /* Compute range in the last layer.  */
     uint64_t first;
     uint64_t last = start + count - 1;
+    uint64_t gran = 1ULL << hb->granularity;
+
+    assert(!(start & (gran - 1)));
+    assert(!(count & (gran - 1)) || (start + count == hb->orig_size));
 
     trace_hbitmap_reset(hb, start, count,
                         start >> hb->granularity, last >> hb->granularity);
-- 
2.21.0



  reply	other threads:[~2019-10-11 21:28 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-11 21:25 [PULL 00/19] Bitmaps patches John Snow
2019-10-11 21:25 ` John Snow [this message]
2019-10-11 21:48   ` [PULL 01/19] util/hbitmap: strict hbitmap_reset Eric Blake
2019-10-11 23:18     ` John Snow
2019-10-14 18:10       ` John Snow
2019-10-15  8:44         ` Kevin Wolf
2019-10-15 12:55           ` John Snow
2019-10-11 21:25 ` [PULL 02/19] block: move bdrv_can_store_new_dirty_bitmap to block/dirty-bitmap.c John Snow
2019-10-11 21:25 ` [PULL 03/19] block/dirty-bitmap: return int from bdrv_remove_persistent_dirty_bitmap John Snow
2019-10-11 21:25 ` [PULL 04/19] block/qcow2: proper locking on bitmap add/remove paths John Snow
2019-10-11 21:25 ` [PULL 05/19] block/dirty-bitmap: drop meta John Snow
2019-10-11 21:25 ` [PULL 06/19] block/dirty-bitmap: add bs link John Snow
2019-10-11 21:25 ` [PULL 07/19] block/dirty-bitmap: drop BdrvDirtyBitmap.mutex John Snow
2019-10-11 21:25 ` [PULL 08/19] block/dirty-bitmap: refactor bdrv_dirty_bitmap_next John Snow
2019-10-11 21:25 ` [PULL 09/19] block: switch reopen queue from QSIMPLEQ to QTAILQ John Snow
2019-10-11 21:25 ` [PULL 10/19] block: reverse order for reopen commits John Snow
2019-10-11 21:25 ` [PULL 11/19] iotests: add test-case to 165 to test reopening qcow2 bitmaps to RW John Snow
2019-10-11 21:25 ` [PULL 12/19] block/qcow2-bitmap: get rid of bdrv_has_changed_persistent_bitmaps John Snow
2019-10-11 21:25 ` [PULL 13/19] block/qcow2-bitmap: drop qcow2_reopen_bitmaps_rw_hint() John Snow
2019-10-11 21:25 ` [PULL 14/19] block/qcow2-bitmap: do not remove bitmaps on reopen-ro John Snow
2019-10-11 21:25 ` [PULL 15/19] iotests: add test 260 to check bitmap life after snapshot + commit John Snow
2019-10-11 21:25 ` [PULL 16/19] block/qcow2-bitmap: fix and improve qcow2_reopen_bitmaps_rw John Snow
2019-10-11 21:25 ` [PULL 17/19] qcow2-bitmap: move bitmap reopen-rw code to qcow2_reopen_commit John Snow
2019-10-11 21:25 ` [PULL 18/19] MAINTAINERS: Add Vladimir as a reviewer for bitmaps John Snow
2019-10-11 21:25 ` [PULL 19/19] dirty-bitmaps: remove deprecated autoload parameter John Snow
2019-10-14 16:11 ` [PULL 00/19] Bitmaps 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=20191011212550.27269-2-jsnow@redhat.com \
    --to=jsnow@redhat.com \
    --cc=armbru@redhat.com \
    --cc=dgilbert@redhat.com \
    --cc=fam@euphon.net \
    --cc=kwolf@redhat.com \
    --cc=libvir-list@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=quintela@redhat.com \
    --cc=stefanha@redhat.com \
    --cc=vsementsov@virtuozzo.com \
    /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.