qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v2 14/17] block: Resize bitmaps on bdrv_truncate


From: Max Reitz
Subject: Re: [Qemu-devel] [PATCH v2 14/17] block: Resize bitmaps on bdrv_truncate
Date: Tue, 03 Mar 2015 11:02:24 -0500
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0

On 2015-03-02 at 18:20, John Snow wrote:
Signed-off-by: John Snow <address@hidden>
---
  block.c                | 22 ++++++++++++++++++++
  include/block/block.h  |  1 +
  include/qemu/hbitmap.h | 10 +++++++++
  util/hbitmap.c         | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++
  4 files changed, 88 insertions(+)

diff --git a/block.c b/block.c
index e6b2696..5eaa874 100644
--- a/block.c
+++ b/block.c
@@ -3543,6 +3543,7 @@ int bdrv_truncate(BlockDriverState *bs, int64_t offset)
      ret = drv->bdrv_truncate(bs, offset);
      if (ret == 0) {
          ret = refresh_total_sectors(bs, offset >> BDRV_SECTOR_BITS);
+        bdrv_dirty_bitmap_truncate(bs);
          if (bs->blk) {
              blk_dev_resize_cb(bs->blk);
          }
@@ -5562,6 +5563,27 @@ BdrvDirtyBitmap 
*bdrv_reclaim_dirty_bitmap(BlockDriverState *bs,
      return parent;
  }
+static void dirty_bitmap_truncate(BdrvDirtyBitmap *bitmap, uint64_t size)
+{
+    /* Should only be frozen during a block backup job, which should have
+     * blocked any resize actions. */
+    assert(!bdrv_dirty_bitmap_frozen(bitmap));
+    hbitmap_truncate(bitmap->bitmap, size);
+}
+
+void bdrv_dirty_bitmap_truncate(BlockDriverState *bs)
+{
+    BdrvDirtyBitmap *bitmap;
+    uint64_t size = bdrv_nb_sectors(bs);
+
+    QLIST_FOREACH(bitmap, &bs->dirty_bitmaps, list) {
+        if (bdrv_dirty_bitmap_frozen(bitmap)) {
+            continue;
+        }

Hm!

The assert() above doesn't do anything because this condition will just skip over frozen bitmaps. Maybe we should drop it?

I guess my R-b stands since bdrv_dirty_bitmap_frozen(bitmap) will always be false anyway (hence the assertion), but it would stand with this block dropped, too.

Max

+        dirty_bitmap_truncate(bitmap, size);
+    }
+}
+
  void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap)
  {
      BdrvDirtyBitmap *bm, *next;
diff --git a/include/block/block.h b/include/block/block.h
index a8c6369..3a85690 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -454,6 +454,7 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState 
*bs,
                                            uint32_t granularity,
                                            const char *name,
                                            Error **errp);
+void bdrv_dirty_bitmap_truncate(BlockDriverState *bs);
  int bdrv_dirty_bitmap_create_successor(BlockDriverState *bs,
                                         BdrvDirtyBitmap *bitmap,
                                         Error **errp);
diff --git a/include/qemu/hbitmap.h b/include/qemu/hbitmap.h
index c19c1cb..a75157e 100644
--- a/include/qemu/hbitmap.h
+++ b/include/qemu/hbitmap.h
@@ -65,6 +65,16 @@ struct HBitmapIter {
  HBitmap *hbitmap_alloc(uint64_t size, int granularity);
/**
+ * hbitmap_truncate:
+ * @hb: The bitmap to change the size of.
+ * @size: The number of elements to change the bitmap to accommodate.
+ *
+ * truncate or grow an existing bitmap to accommodate a new number of elements.
+ * This may invalidate existing HBitmapIterators.
+ */
+void hbitmap_truncate(HBitmap *hb, uint64_t size);
+
+/**
   * hbitmap_merge:
   * @a: The bitmap to store the result in.
   * @b: The bitmap to merge into @a.
diff --git a/util/hbitmap.c b/util/hbitmap.c
index 962ff29..dd8c2f3 100644
--- a/util/hbitmap.c
+++ b/util/hbitmap.c
@@ -90,6 +90,9 @@ struct HBitmap {
       * bitmap will still allocate HBITMAP_LEVELS arrays.
       */
      unsigned long *levels[HBITMAP_LEVELS];
+
+    /* The length of each levels[] array. */
+    uint64_t sizes[HBITMAP_LEVELS];
  };
/* Advance hbi to the next nonzero word and return it. hbi->pos
@@ -384,6 +387,7 @@ HBitmap *hbitmap_alloc(uint64_t size, int granularity)
      hb->granularity = granularity;
      for (i = HBITMAP_LEVELS; i-- > 0; ) {
          size = MAX((size + BITS_PER_LONG - 1) >> BITS_PER_LEVEL, 1);
+        hb->sizes[i] = size;
          hb->levels[i] = g_new0(unsigned long, size);
      }
@@ -396,6 +400,57 @@ HBitmap *hbitmap_alloc(uint64_t size, int granularity)
      return hb;
  }
+void hbitmap_truncate(HBitmap *hb, uint64_t size)
+{
+    bool truncate;
+    unsigned i;
+    uint64_t num_elements = size;
+    uint64_t old;
+
+    /* Size comes in as logical elements, adjust for granularity. */
+    size = (size + (1ULL << hb->granularity) - 1) >> hb->granularity;
+    assert(size <= ((uint64_t)1 << HBITMAP_LOG_MAX_SIZE));
+    truncate = size < hb->size;
+
+    if (size == hb->size) {
+        /* A hard day's work */
+        return;
+    }
+
+    hb->size = size;
+    for (i = HBITMAP_LEVELS; i-- > 0; ) {
+        size = MAX((size + BITS_PER_LONG - 1) >> BITS_PER_LEVEL, 1);
+        if (hb->sizes[i] == size) {
+            continue;
+        }
+        old = hb->sizes[i];
+        hb->sizes[i] = size;
+        hb->levels[i] = g_realloc_n(hb->levels[i], size, sizeof(unsigned 
long));
+        if (!truncate) {
+            memset(&hb->levels[i][old], 0x00,
+                   (size - old) * sizeof(*hb->levels[i]));
+        }
+    }
+    assert(size == 1);
+
+    /* Clear out any "extra space" we may have that the user didn't request:
+     * It may have garbage data in it, now. */
+    if (truncate) {
+        /* Due to granularity fuzziness, we may accidentally reset some of
+         * the last bits that are actually valid. So, record the current value,
+         * reset the "dead range," then re-set the one element we care about. 
*/
+        uint64_t fix_count = (hb->size << hb->granularity) - num_elements;
+        if (fix_count) {
+            bool set = hbitmap_get(hb, num_elements - 1);
+            hbitmap_reset(hb, num_elements, fix_count);
+            if (set) {
+                hbitmap_set(hb, num_elements - 1, 1);
+            }
+        }
+    }
+}
+
+
  /**
   * Given HBitmaps A and B, let A := A (BITOR) B.
   * Bitmap B will not be modified.




reply via email to

[Prev in Thread] Current Thread [Next in Thread]