qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 00/22] parallels: Add full dirty bitmap support
@ 2024-03-11 18:18 Alexander Ivanov
  2024-03-11 18:18 ` [PATCH v5 01/22] parallels: Set s->used_bmap to NULL in parallels_free_used_bitmap() Alexander Ivanov
                   ` (21 more replies)
  0 siblings, 22 replies; 23+ messages in thread
From: Alexander Ivanov @ 2024-03-11 18:18 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-block, den, stefanha, vsementsov, kwolf, hreitz

Parallels format driver:
* make some preparation
* add dirty bitmap saving
* make dirty bitmap RW
* fix broken checks
* refactor leak check
* add parallels format support to several tests

You could find these patches in my repo:
https://github.com/AlexanderIvanov-Virtuozzo/qemu/tree/parallels-v5

v5:
6: Returned a comment about fallocate.
10: Added a motivation of used_bitmap usage to the commit message.
11: Renamed 'limit' variable. Added a few error messages. Fixed
    'remaining' value by extension header size. Simplified bh and l1_table
    pointer calculation. Added clusters cleaning after error. Add return
    value to parallels_store_persistent_dirty_bitmaps().
12: New patch, all the next patches will be shifted by 1. Added header
    unclean check to dirty bitmap loading.
13: Placed dirty bitmap loading after used bitmap initialization.
14: New patch, all the next patches will be shifted by 2. Preserved
    extensions cluster for non-transient extensions.
16: (previous numeration): Dropped the patch because the next patch deleted
    all the code of this patch. All the next patches will be shifted by 1.
18: Moved used bitmap parallels_check_unused_clusters() to parallels_check_leak().
19: Removed redundant bdrv_pwrite_zeroes() (see a note in the commit message).
    Added a break to outside-of-image clusters at image opening.

v4:
4: A new patch with limitation of search in parallels_mark_used.
5: Previously 4. Search is limited to (cluster_index + count).
6: Previously 5. Added GRAPH_RDLOCK annotation, added a note in the commit
   message.
11: Previously 10. Added GRAPH_RDLOCK annotation.
16-18: Added GRAPH_RDLOCK annotations.

v3:
1: Fixed the order of g_free() and s->used_bmap = NULL.
3,4: Made mark_used() a global function before mark_unused() addition. In
     this way we can avoid compilation warnings.
5-9: Patches shifted.
11: Added GRAPH_RDLOCK annotation to parallels_inactivate(). Guard
    parallels_close() with GRAPH_RDLOCK_GUARD_MAINLOOP().
12-21: Patches shifted.

v2:
1: New patch to fix double free error.
4: Fixed clusters leaks.
15: Fixed (end_off != s->used_bmap_size) handling in parallels_truncate_unused_clusters().
16,17: Changed the sequence of the patches - in this way we have correct leaks check.

Alexander Ivanov (21):
  parallels: Set s->used_bmap to NULL in parallels_free_used_bitmap()
  parallels: Move inactivation code to a separate function
  parallels: Make mark_used() a global function
  parallels: Limit search in parallels_mark_used to the last marked
    claster
  parallels: Add parallels_mark_unused() helper
  parallels: Move host clusters allocation to a separate function
  parallels: Set data_end value in parallels_check_leak()
  parallels: Recreate used bitmap in parallels_check_leak()
  parallels: Add a note about used bitmap in parallels_check_duplicate()
  parallels: Create used bitmap even if checks needed
  parallels: Add dirty bitmaps saving
  parallels: Let image extensions work in RW mode
  parallels: Preserve extensions cluster for non-transient extensions
  parallels: Handle L1 entries equal to one
  parallels: Make a loaded dirty bitmap persistent
  parallels: Reverse a conditional in parallels_check_leak() to reduce
    indents
  parallels: Check unused clusters in parallels_check_leak()
  parallels: Remove unnecessary data_end field
  tests: Add parallels images support to test 165
  tests: Turned on 256, 299, 304 and block-status-cache for parallels
    format
  tests: Add parallels format support to image-fleecing

Denis V. Lunev (1):
  parallels: drop dirty bitmap data if the image was not properly closed

 block/parallels-ext.c                       | 243 ++++++++++++-
 block/parallels.c                           | 367 ++++++++++++--------
 block/parallels.h                           |  14 +-
 tests/qemu-iotests/165                      |  40 ++-
 tests/qemu-iotests/256                      |   2 +-
 tests/qemu-iotests/299                      |   2 +-
 tests/qemu-iotests/304                      |   2 +-
 tests/qemu-iotests/tests/block-status-cache |   2 +-
 tests/qemu-iotests/tests/image-fleecing     |  13 +-
 9 files changed, 502 insertions(+), 183 deletions(-)

-- 
2.40.1



^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH v5 01/22] parallels: Set s->used_bmap to NULL in parallels_free_used_bitmap()
  2024-03-11 18:18 [PATCH v5 00/22] parallels: Add full dirty bitmap support Alexander Ivanov
@ 2024-03-11 18:18 ` Alexander Ivanov
  2024-03-11 18:18 ` [PATCH v5 02/22] parallels: Move inactivation code to a separate function Alexander Ivanov
                   ` (20 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: Alexander Ivanov @ 2024-03-11 18:18 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-block, den, stefanha, vsementsov, kwolf, hreitz

After used bitmap freeng s->used_bmap points to the freed memory. If we try
to free used bitmap one more time it leads to double free error.

Set s->used_bmap to NULL to exclude double free error.

Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
Reviewed-by: Denis V. Lunev <den@openvz.org>
---
 block/parallels.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/block/parallels.c b/block/parallels.c
index 9205a0864f..072b1efd78 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -245,6 +245,7 @@ static void parallels_free_used_bitmap(BlockDriverState *bs)
     BDRVParallelsState *s = bs->opaque;
     s->used_bmap_size = 0;
     g_free(s->used_bmap);
+    s->used_bmap = NULL;
 }
 
 static int64_t coroutine_fn GRAPH_RDLOCK
-- 
2.40.1



^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v5 02/22] parallels: Move inactivation code to a separate function
  2024-03-11 18:18 [PATCH v5 00/22] parallels: Add full dirty bitmap support Alexander Ivanov
  2024-03-11 18:18 ` [PATCH v5 01/22] parallels: Set s->used_bmap to NULL in parallels_free_used_bitmap() Alexander Ivanov
@ 2024-03-11 18:18 ` Alexander Ivanov
  2024-03-11 18:18 ` [PATCH v5 03/22] parallels: Make mark_used() a global function Alexander Ivanov
                   ` (19 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: Alexander Ivanov @ 2024-03-11 18:18 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-block, den, stefanha, vsementsov, kwolf, hreitz

We are going to add parallels image extensions storage and need a separate
function for inactivation code.

Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
Reviewed-by: Denis V. Lunev <den@openvz.org>
---
 block/parallels.c | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/block/parallels.c b/block/parallels.c
index 072b1efd78..992362ce29 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -1424,6 +1424,20 @@ fail:
     return ret;
 }
 
+static int GRAPH_RDLOCK parallels_inactivate(BlockDriverState *bs)
+{
+    BDRVParallelsState *s = bs->opaque;
+    int ret;
+
+    s->header->inuse = 0;
+    parallels_update_header(bs);
+
+    /* errors are ignored, so we might as well pass exact=true */
+    ret = bdrv_truncate(bs->file, s->data_end << BDRV_SECTOR_BITS, true,
+                        PREALLOC_MODE_OFF, 0, NULL);
+
+    return ret;
+}
 
 static void parallels_close(BlockDriverState *bs)
 {
@@ -1432,12 +1446,7 @@ static void parallels_close(BlockDriverState *bs)
     GRAPH_RDLOCK_GUARD_MAINLOOP();
 
     if ((bs->open_flags & BDRV_O_RDWR) && !(bs->open_flags & BDRV_O_INACTIVE)) {
-        s->header->inuse = 0;
-        parallels_update_header(bs);
-
-        /* errors are ignored, so we might as well pass exact=true */
-        bdrv_truncate(bs->file, s->data_end << BDRV_SECTOR_BITS, true,
-                      PREALLOC_MODE_OFF, 0, NULL);
+        parallels_inactivate(bs);
     }
 
     parallels_free_used_bitmap(bs);
@@ -1476,6 +1485,7 @@ static BlockDriver bdrv_parallels = {
     .bdrv_co_check              = parallels_co_check,
     .bdrv_co_pdiscard           = parallels_co_pdiscard,
     .bdrv_co_pwrite_zeroes      = parallels_co_pwrite_zeroes,
+    .bdrv_inactivate            = parallels_inactivate,
 };
 
 static void bdrv_parallels_init(void)
-- 
2.40.1



^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v5 03/22] parallels: Make mark_used() a global function
  2024-03-11 18:18 [PATCH v5 00/22] parallels: Add full dirty bitmap support Alexander Ivanov
  2024-03-11 18:18 ` [PATCH v5 01/22] parallels: Set s->used_bmap to NULL in parallels_free_used_bitmap() Alexander Ivanov
  2024-03-11 18:18 ` [PATCH v5 02/22] parallels: Move inactivation code to a separate function Alexander Ivanov
@ 2024-03-11 18:18 ` Alexander Ivanov
  2024-03-11 18:18 ` [PATCH v5 04/22] parallels: Limit search in parallels_mark_used to the last marked claster Alexander Ivanov
                   ` (18 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: Alexander Ivanov @ 2024-03-11 18:18 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-block, den, stefanha, vsementsov, kwolf, hreitz

We will need this function and a function for marking unused clusters (will
be added in the next patch) in parallels-ext.c too. Let it be a global
function parallels_mark_used().

Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
Reviewed-by: Denis V. Lunev <den@openvz.org>
---
 block/parallels.c | 14 ++++++++------
 block/parallels.h |  3 +++
 2 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/block/parallels.c b/block/parallels.c
index 992362ce29..ae524f1820 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -178,8 +178,8 @@ static void parallels_set_bat_entry(BDRVParallelsState *s,
     bitmap_set(s->bat_dirty_bmap, bat_entry_off(index) / s->bat_dirty_block, 1);
 }
 
-static int mark_used(BlockDriverState *bs, unsigned long *bitmap,
-                     uint32_t bitmap_size, int64_t off, uint32_t count)
+int parallels_mark_used(BlockDriverState *bs, unsigned long *bitmap,
+                        uint32_t bitmap_size, int64_t off, uint32_t count)
 {
     BDRVParallelsState *s = bs->opaque;
     uint32_t cluster_index = host_cluster_index(s, off);
@@ -232,7 +232,8 @@ static int GRAPH_RDLOCK parallels_fill_used_bitmap(BlockDriverState *bs)
             continue;
         }
 
-        err2 = mark_used(bs, s->used_bmap, s->used_bmap_size, host_off, 1);
+        err2 = parallels_mark_used(bs, s->used_bmap, s->used_bmap_size,
+                                   host_off, 1);
         if (err2 < 0 && err == 0) {
             err = err2;
         }
@@ -366,7 +367,8 @@ allocate_clusters(BlockDriverState *bs, int64_t sector_num,
         }
     }
 
-    ret = mark_used(bs, s->used_bmap, s->used_bmap_size, host_off, to_allocate);
+    ret = parallels_mark_used(bs, s->used_bmap, s->used_bmap_size,
+                              host_off, to_allocate);
     if (ret < 0) {
         /* Image consistency is broken. Alarm! */
         return ret;
@@ -827,7 +829,7 @@ parallels_check_duplicate(BlockDriverState *bs, BdrvCheckResult *res,
             continue;
         }
 
-        ret = mark_used(bs, bitmap, bitmap_size, host_off, 1);
+        ret = parallels_mark_used(bs, bitmap, bitmap_size, host_off, 1);
         assert(ret != -E2BIG);
         if (ret == 0) {
             continue;
@@ -887,7 +889,7 @@ parallels_check_duplicate(BlockDriverState *bs, BdrvCheckResult *res,
          * considered, and the bitmap size doesn't change. This specifically
          * means that -E2BIG is OK.
          */
-        ret = mark_used(bs, bitmap, bitmap_size, host_off, 1);
+        ret = parallels_mark_used(bs, bitmap, bitmap_size, host_off, 1);
         if (ret == -EBUSY) {
             res->check_errors++;
             goto out_repair_bat;
diff --git a/block/parallels.h b/block/parallels.h
index 423b2ad727..68077416b1 100644
--- a/block/parallels.h
+++ b/block/parallels.h
@@ -90,6 +90,9 @@ typedef struct BDRVParallelsState {
     Error *migration_blocker;
 } BDRVParallelsState;
 
+int parallels_mark_used(BlockDriverState *bs, unsigned long *bitmap,
+                        uint32_t bitmap_size, int64_t off, uint32_t count);
+
 int GRAPH_RDLOCK
 parallels_read_format_extension(BlockDriverState *bs, int64_t ext_off,
                                 Error **errp);
-- 
2.40.1



^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v5 04/22] parallels: Limit search in parallels_mark_used to the last marked claster
  2024-03-11 18:18 [PATCH v5 00/22] parallels: Add full dirty bitmap support Alexander Ivanov
                   ` (2 preceding siblings ...)
  2024-03-11 18:18 ` [PATCH v5 03/22] parallels: Make mark_used() a global function Alexander Ivanov
@ 2024-03-11 18:18 ` Alexander Ivanov
  2024-03-11 18:18 ` [PATCH v5 05/22] parallels: Add parallels_mark_unused() helper Alexander Ivanov
                   ` (17 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: Alexander Ivanov @ 2024-03-11 18:18 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-block, den, stefanha, vsementsov, kwolf, hreitz

There is no necessity to search to the end of the bitmap. Limit the search
area as cluster_index + count.

Add cluster_end variable to avoid its calculation in a few places.

Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
Reviewed-by: Denis V. Lunev <den@openvz.org>
---
 block/parallels.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/block/parallels.c b/block/parallels.c
index ae524f1820..4470519656 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -182,13 +182,14 @@ int parallels_mark_used(BlockDriverState *bs, unsigned long *bitmap,
                         uint32_t bitmap_size, int64_t off, uint32_t count)
 {
     BDRVParallelsState *s = bs->opaque;
-    uint32_t cluster_index = host_cluster_index(s, off);
+    uint32_t cluster_end, cluster_index = host_cluster_index(s, off);
     unsigned long next_used;
-    if (cluster_index + count > bitmap_size) {
+    cluster_end = cluster_index + count;
+    if (cluster_end > bitmap_size) {
         return -E2BIG;
     }
-    next_used = find_next_bit(bitmap, bitmap_size, cluster_index);
-    if (next_used < cluster_index + count) {
+    next_used = find_next_bit(bitmap, cluster_end, cluster_index);
+    if (next_used < cluster_end) {
         return -EBUSY;
     }
     bitmap_set(bitmap, cluster_index, count);
-- 
2.40.1



^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v5 05/22] parallels: Add parallels_mark_unused() helper
  2024-03-11 18:18 [PATCH v5 00/22] parallels: Add full dirty bitmap support Alexander Ivanov
                   ` (3 preceding siblings ...)
  2024-03-11 18:18 ` [PATCH v5 04/22] parallels: Limit search in parallels_mark_used to the last marked claster Alexander Ivanov
@ 2024-03-11 18:18 ` Alexander Ivanov
  2024-03-11 18:18 ` [PATCH v5 06/22] parallels: Move host clusters allocation to a separate function Alexander Ivanov
                   ` (16 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: Alexander Ivanov @ 2024-03-11 18:18 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-block, den, stefanha, vsementsov, kwolf, hreitz

Add a helper to set unused areas in the used bitmap.

Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
Reviewed-by: Denis V. Lunev <den@openvz.org>
---
 block/parallels.c | 18 ++++++++++++++++++
 block/parallels.h |  2 ++
 2 files changed, 20 insertions(+)

diff --git a/block/parallels.c b/block/parallels.c
index 4470519656..13726fb3d5 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -196,6 +196,24 @@ int parallels_mark_used(BlockDriverState *bs, unsigned long *bitmap,
     return 0;
 }
 
+int parallels_mark_unused(BlockDriverState *bs, unsigned long *bitmap,
+                          uint32_t bitmap_size, int64_t off, uint32_t count)
+{
+    BDRVParallelsState *s = bs->opaque;
+    uint32_t cluster_end, cluster_index = host_cluster_index(s, off);
+    unsigned long next_unused;
+    cluster_end = cluster_index + count;
+    if (cluster_end > bitmap_size) {
+        return -E2BIG;
+    }
+    next_unused = find_next_zero_bit(bitmap, cluster_end, cluster_index);
+    if (next_unused < cluster_end) {
+        return -EINVAL;
+    }
+    bitmap_clear(bitmap, cluster_index, count);
+    return 0;
+}
+
 /*
  * Collect used bitmap. The image can contain errors, we should fill the
  * bitmap anyway, as much as we can. This information will be used for
diff --git a/block/parallels.h b/block/parallels.h
index 68077416b1..02b857b4a4 100644
--- a/block/parallels.h
+++ b/block/parallels.h
@@ -92,6 +92,8 @@ typedef struct BDRVParallelsState {
 
 int parallels_mark_used(BlockDriverState *bs, unsigned long *bitmap,
                         uint32_t bitmap_size, int64_t off, uint32_t count);
+int parallels_mark_unused(BlockDriverState *bs, unsigned long *bitmap,
+                          uint32_t bitmap_size, int64_t off, uint32_t count);
 
 int GRAPH_RDLOCK
 parallels_read_format_extension(BlockDriverState *bs, int64_t ext_off,
-- 
2.40.1



^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v5 06/22] parallels: Move host clusters allocation to a separate function
  2024-03-11 18:18 [PATCH v5 00/22] parallels: Add full dirty bitmap support Alexander Ivanov
                   ` (4 preceding siblings ...)
  2024-03-11 18:18 ` [PATCH v5 05/22] parallels: Add parallels_mark_unused() helper Alexander Ivanov
@ 2024-03-11 18:18 ` Alexander Ivanov
  2024-03-11 18:18 ` [PATCH v5 07/22] parallels: Set data_end value in parallels_check_leak() Alexander Ivanov
                   ` (15 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: Alexander Ivanov @ 2024-03-11 18:18 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-block, den, stefanha, vsementsov, kwolf, hreitz

For parallels images extensions we need to allocate host clusters
without any connection to BAT. Move host clusters allocation code to
parallels_allocate_host_clusters().

This function can be called not only from coroutines so all the
*_co_* functions were replaced by corresponding wrappers.

Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
Reviewed-by: Denis V. Lunev <den@openvz.org>
---
 block/parallels.c | 123 +++++++++++++++++++++++++---------------------
 block/parallels.h |   3 ++
 2 files changed, 71 insertions(+), 55 deletions(-)

diff --git a/block/parallels.c b/block/parallels.c
index 13726fb3d5..c6d82961c9 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -268,42 +268,21 @@ static void parallels_free_used_bitmap(BlockDriverState *bs)
     s->used_bmap = NULL;
 }
 
-static int64_t coroutine_fn GRAPH_RDLOCK
-allocate_clusters(BlockDriverState *bs, int64_t sector_num,
-                  int nb_sectors, int *pnum)
+int64_t GRAPH_RDLOCK parallels_allocate_host_clusters(BlockDriverState *bs,
+                                                      int64_t *clusters)
 {
-    int ret = 0;
     BDRVParallelsState *s = bs->opaque;
-    int64_t i, pos, idx, to_allocate, first_free, host_off;
-
-    pos = block_status(s, sector_num, nb_sectors, pnum);
-    if (pos > 0) {
-        return pos;
-    }
-
-    idx = sector_num / s->tracks;
-    to_allocate = DIV_ROUND_UP(sector_num + *pnum, s->tracks) - idx;
-
-    /*
-     * This function is called only by parallels_co_writev(), which will never
-     * pass a sector_num at or beyond the end of the image (because the block
-     * layer never passes such a sector_num to that function). Therefore, idx
-     * is always below s->bat_size.
-     * block_status() will limit *pnum so that sector_num + *pnum will not
-     * exceed the image end. Therefore, idx + to_allocate cannot exceed
-     * s->bat_size.
-     * Note that s->bat_size is an unsigned int, therefore idx + to_allocate
-     * will always fit into a uint32_t.
-     */
-    assert(idx < s->bat_size && idx + to_allocate <= s->bat_size);
+    int64_t first_free, next_used, host_off, prealloc_clusters;
+    int64_t bytes, prealloc_bytes;
+    uint32_t new_usedsize;
+    int ret = 0;
 
     first_free = find_first_zero_bit(s->used_bmap, s->used_bmap_size);
     if (first_free == s->used_bmap_size) {
-        uint32_t new_usedsize;
-        int64_t bytes = to_allocate * s->cluster_size;
-        bytes += s->prealloc_size * BDRV_SECTOR_SIZE;
-
         host_off = s->data_end * BDRV_SECTOR_SIZE;
+        prealloc_clusters = *clusters + s->prealloc_size / s->tracks;
+        bytes = *clusters * s->cluster_size;
+        prealloc_bytes = prealloc_clusters * s->cluster_size;
 
         /*
          * We require the expanded size to read back as zero. If the
@@ -311,15 +290,14 @@ allocate_clusters(BlockDriverState *bs, int64_t sector_num,
          * force the safer-but-slower fallocate.
          */
         if (s->prealloc_mode == PRL_PREALLOC_MODE_TRUNCATE) {
-            ret = bdrv_co_truncate(bs->file, host_off + bytes,
-                                   false, PREALLOC_MODE_OFF,
-                                   BDRV_REQ_ZERO_WRITE, NULL);
+            ret = bdrv_truncate(bs->file, host_off + prealloc_bytes, false,
+                                PREALLOC_MODE_OFF, BDRV_REQ_ZERO_WRITE, NULL);
             if (ret == -ENOTSUP) {
                 s->prealloc_mode = PRL_PREALLOC_MODE_FALLOCATE;
             }
         }
         if (s->prealloc_mode == PRL_PREALLOC_MODE_FALLOCATE) {
-            ret = bdrv_co_pwrite_zeroes(bs->file, host_off, bytes, 0);
+            ret = bdrv_pwrite_zeroes(bs->file, host_off, prealloc_bytes, 0);
         }
         if (ret < 0) {
             return ret;
@@ -329,15 +307,15 @@ allocate_clusters(BlockDriverState *bs, int64_t sector_num,
         s->used_bmap = bitmap_zero_extend(s->used_bmap, s->used_bmap_size,
                                           new_usedsize);
         s->used_bmap_size = new_usedsize;
+        if (host_off + bytes > s->data_end * BDRV_SECTOR_SIZE) {
+            s->data_end = (host_off + bytes) / BDRV_SECTOR_SIZE;
+        }
     } else {
-        int64_t next_used;
         next_used = find_next_bit(s->used_bmap, s->used_bmap_size, first_free);
 
         /* Not enough continuous clusters in the middle, adjust the size */
-        if (next_used - first_free < to_allocate) {
-            to_allocate = next_used - first_free;
-            *pnum = (idx + to_allocate) * s->tracks - sector_num;
-        }
+        *clusters = MIN(*clusters, next_used - first_free);
+        bytes = *clusters * s->cluster_size;
 
         host_off = s->data_start * BDRV_SECTOR_SIZE;
         host_off += first_free * s->cluster_size;
@@ -349,14 +327,59 @@ allocate_clusters(BlockDriverState *bs, int64_t sector_num,
          */
         if (s->prealloc_mode == PRL_PREALLOC_MODE_FALLOCATE &&
                 host_off < s->data_end * BDRV_SECTOR_SIZE) {
-            ret = bdrv_co_pwrite_zeroes(bs->file, host_off,
-                                        s->cluster_size * to_allocate, 0);
+            ret = bdrv_pwrite_zeroes(bs->file, host_off, bytes, 0);
             if (ret < 0) {
                 return ret;
             }
         }
     }
 
+    ret = parallels_mark_used(bs, s->used_bmap, s->used_bmap_size,
+                              host_off, *clusters);
+    if (ret < 0) {
+        /* Image consistency is broken. Alarm! */
+        return ret;
+    }
+
+    return host_off;
+}
+
+static int64_t coroutine_fn GRAPH_RDLOCK
+allocate_clusters(BlockDriverState *bs, int64_t sector_num,
+                  int nb_sectors, int *pnum)
+{
+    int ret = 0;
+    BDRVParallelsState *s = bs->opaque;
+    int64_t i, pos, idx, to_allocate, host_off;
+
+    pos = block_status(s, sector_num, nb_sectors, pnum);
+    if (pos > 0) {
+        return pos;
+    }
+
+    idx = sector_num / s->tracks;
+    to_allocate = DIV_ROUND_UP(sector_num + *pnum, s->tracks) - idx;
+
+    /*
+     * This function is called only by parallels_co_writev(), which will never
+     * pass a sector_num at or beyond the end of the image (because the block
+     * layer never passes such a sector_num to that function). Therefore, idx
+     * is always below s->bat_size.
+     * block_status() will limit *pnum so that sector_num + *pnum will not
+     * exceed the image end. Therefore, idx + to_allocate cannot exceed
+     * s->bat_size.
+     * Note that s->bat_size is an unsigned int, therefore idx + to_allocate
+     * will always fit into a uint32_t.
+     */
+    assert(idx < s->bat_size && idx + to_allocate <= s->bat_size);
+
+    host_off = parallels_allocate_host_clusters(bs, &to_allocate);
+    if (host_off < 0) {
+        return host_off;
+    }
+
+    *pnum = MIN(*pnum, (idx + to_allocate) * s->tracks - sector_num);
+
     /*
      * Try to read from backing to fill empty clusters
      * FIXME: 1. previous write_zeroes may be redundant
@@ -373,33 +396,23 @@ allocate_clusters(BlockDriverState *bs, int64_t sector_num,
 
         ret = bdrv_co_pread(bs->backing, idx * s->tracks * BDRV_SECTOR_SIZE,
                             nb_cow_bytes, buf, 0);
-        if (ret < 0) {
-            qemu_vfree(buf);
-            return ret;
+        if (ret == 0) {
+            ret = bdrv_co_pwrite(bs->file, host_off, nb_cow_bytes, buf, 0);
         }
 
-        ret = bdrv_co_pwrite(bs->file, s->data_end * BDRV_SECTOR_SIZE,
-                             nb_cow_bytes, buf, 0);
         qemu_vfree(buf);
         if (ret < 0) {
+            parallels_mark_unused(bs, s->used_bmap, s->used_bmap_size,
+                                  host_off, to_allocate);
             return ret;
         }
     }
 
-    ret = parallels_mark_used(bs, s->used_bmap, s->used_bmap_size,
-                              host_off, to_allocate);
-    if (ret < 0) {
-        /* Image consistency is broken. Alarm! */
-        return ret;
-    }
     for (i = 0; i < to_allocate; i++) {
         parallels_set_bat_entry(s, idx + i,
                 host_off / BDRV_SECTOR_SIZE / s->off_multiplier);
         host_off += s->cluster_size;
     }
-    if (host_off > s->data_end * BDRV_SECTOR_SIZE) {
-        s->data_end = host_off / BDRV_SECTOR_SIZE;
-    }
 
     return bat2sect(s, idx) + sector_num % s->tracks;
 }
diff --git a/block/parallels.h b/block/parallels.h
index 02b857b4a4..493c89e976 100644
--- a/block/parallels.h
+++ b/block/parallels.h
@@ -95,6 +95,9 @@ int parallels_mark_used(BlockDriverState *bs, unsigned long *bitmap,
 int parallels_mark_unused(BlockDriverState *bs, unsigned long *bitmap,
                           uint32_t bitmap_size, int64_t off, uint32_t count);
 
+int64_t GRAPH_RDLOCK parallels_allocate_host_clusters(BlockDriverState *bs,
+                                                      int64_t *clusters);
+
 int GRAPH_RDLOCK
 parallels_read_format_extension(BlockDriverState *bs, int64_t ext_off,
                                 Error **errp);
-- 
2.40.1



^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v5 07/22] parallels: Set data_end value in parallels_check_leak()
  2024-03-11 18:18 [PATCH v5 00/22] parallels: Add full dirty bitmap support Alexander Ivanov
                   ` (5 preceding siblings ...)
  2024-03-11 18:18 ` [PATCH v5 06/22] parallels: Move host clusters allocation to a separate function Alexander Ivanov
@ 2024-03-11 18:18 ` Alexander Ivanov
  2024-03-11 18:18 ` [PATCH v5 08/22] parallels: Recreate used bitmap " Alexander Ivanov
                   ` (14 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: Alexander Ivanov @ 2024-03-11 18:18 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-block, den, stefanha, vsementsov, kwolf, hreitz

In parallels_check_leak() we change file size but don't correct data_end
field of BDRVParallelsState structure. Fix it.

Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
Reviewed-by: Denis V. Lunev <den@openvz.org>
---
 block/parallels.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/block/parallels.c b/block/parallels.c
index c6d82961c9..e7c3763017 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -811,6 +811,7 @@ parallels_check_leak(BlockDriverState *bs, BdrvCheckResult *res,
                 res->check_errors++;
                 return ret;
             }
+            s->data_end = res->image_end_offset >> BDRV_SECTOR_BITS;
             if (explicit) {
                 res->leaks_fixed += count;
             }
-- 
2.40.1



^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v5 08/22] parallels: Recreate used bitmap in parallels_check_leak()
  2024-03-11 18:18 [PATCH v5 00/22] parallels: Add full dirty bitmap support Alexander Ivanov
                   ` (6 preceding siblings ...)
  2024-03-11 18:18 ` [PATCH v5 07/22] parallels: Set data_end value in parallels_check_leak() Alexander Ivanov
@ 2024-03-11 18:18 ` Alexander Ivanov
  2024-03-11 18:18 ` [PATCH v5 09/22] parallels: Add a note about used bitmap in parallels_check_duplicate() Alexander Ivanov
                   ` (13 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: Alexander Ivanov @ 2024-03-11 18:18 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-block, den, stefanha, vsementsov, kwolf, hreitz

In parallels_check_leak() file can be truncated. In this case the used
bitmap would not comply to the file. Recreate the bitmap after file
truncation.

Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
Reviewed-by: Denis V. Lunev <den@openvz.org>
---
 block/parallels.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/block/parallels.c b/block/parallels.c
index e7c3763017..9056dd6a11 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -812,6 +812,14 @@ parallels_check_leak(BlockDriverState *bs, BdrvCheckResult *res,
                 return ret;
             }
             s->data_end = res->image_end_offset >> BDRV_SECTOR_BITS;
+
+            parallels_free_used_bitmap(bs);
+            ret = parallels_fill_used_bitmap(bs);
+            if (ret == -ENOMEM) {
+                res->check_errors++;
+                return ret;
+            }
+
             if (explicit) {
                 res->leaks_fixed += count;
             }
-- 
2.40.1



^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v5 09/22] parallels: Add a note about used bitmap in parallels_check_duplicate()
  2024-03-11 18:18 [PATCH v5 00/22] parallels: Add full dirty bitmap support Alexander Ivanov
                   ` (7 preceding siblings ...)
  2024-03-11 18:18 ` [PATCH v5 08/22] parallels: Recreate used bitmap " Alexander Ivanov
@ 2024-03-11 18:18 ` Alexander Ivanov
  2024-03-11 18:18 ` [PATCH v5 10/22] parallels: Create used bitmap even if checks needed Alexander Ivanov
                   ` (12 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: Alexander Ivanov @ 2024-03-11 18:18 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-block, den, stefanha, vsementsov, kwolf, hreitz

In parallels_check_duplicate() We use a bitmap for duplication detection.
This bitmap is not related to used_bmap field in BDRVParallelsState. Add
a comment about it to avoid confusion.

Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
Reviewed-by: Denis V. Lunev <den@openvz.org>
---
 block/parallels.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/block/parallels.c b/block/parallels.c
index 9056dd6a11..66990925a8 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -842,7 +842,10 @@ parallels_check_duplicate(BlockDriverState *bs, BdrvCheckResult *res,
     bool fixed = false;
 
     /*
-     * Create a bitmap of used clusters.
+     * Create a bitmap of used clusters. Please note that this bitmap is not
+     * related to used_bmap field in BDRVParallelsState and is created only for
+     * local usage.
+     *
      * If a bit is set, there is a BAT entry pointing to this cluster.
      * Loop through the BAT entries, check bits relevant to an entry offset.
      * If bit is set, this entry is duplicated. Otherwise set the bit.
-- 
2.40.1



^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v5 10/22] parallels: Create used bitmap even if checks needed
  2024-03-11 18:18 [PATCH v5 00/22] parallels: Add full dirty bitmap support Alexander Ivanov
                   ` (8 preceding siblings ...)
  2024-03-11 18:18 ` [PATCH v5 09/22] parallels: Add a note about used bitmap in parallels_check_duplicate() Alexander Ivanov
@ 2024-03-11 18:18 ` Alexander Ivanov
  2024-03-11 18:18 ` [PATCH v5 11/22] parallels: Add dirty bitmaps saving Alexander Ivanov
                   ` (11 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: Alexander Ivanov @ 2024-03-11 18:18 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-block, den, stefanha, vsementsov, kwolf, hreitz

All the checks were fixed to work with used bitmap. Create used bitmap in
parallels_open() even if need_check is true.

In some checks (like duplication check) we need to work with used bitmap
so it have to be created before checks are running.

Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
 block/parallels.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/block/parallels.c b/block/parallels.c
index 66990925a8..86958506d0 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -1426,13 +1426,11 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags,
     }
     need_check = need_check || s->data_end > file_nb_sectors;
 
-    if (!need_check) {
-        ret = parallels_fill_used_bitmap(bs);
-        if (ret == -ENOMEM) {
-            goto fail;
-        }
-        need_check = need_check || ret < 0; /* These are correctable errors */
+    ret = parallels_fill_used_bitmap(bs);
+    if (ret == -ENOMEM) {
+        goto fail;
     }
+    need_check = need_check || ret < 0; /* These are correctable errors */
 
     /*
      * We don't repair the image here if it's opened for checks. Also we don't
-- 
2.40.1



^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v5 11/22] parallels: Add dirty bitmaps saving
  2024-03-11 18:18 [PATCH v5 00/22] parallels: Add full dirty bitmap support Alexander Ivanov
                   ` (9 preceding siblings ...)
  2024-03-11 18:18 ` [PATCH v5 10/22] parallels: Create used bitmap even if checks needed Alexander Ivanov
@ 2024-03-11 18:18 ` Alexander Ivanov
  2024-03-11 18:18 ` [PATCH v5 12/22] parallels: drop dirty bitmap data if the image was not properly closed Alexander Ivanov
                   ` (10 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: Alexander Ivanov @ 2024-03-11 18:18 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-block, den, stefanha, vsementsov, kwolf, hreitz

Now dirty bitmaps can be loaded but there is no their saving. Add code for
dirty bitmap storage.

Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
 block/parallels-ext.c | 187 ++++++++++++++++++++++++++++++++++++++++++
 block/parallels.c     |  16 +++-
 block/parallels.h     |   5 ++
 3 files changed, 206 insertions(+), 2 deletions(-)

diff --git a/block/parallels-ext.c b/block/parallels-ext.c
index b4e14c88f2..9cb8e65c0d 100644
--- a/block/parallels-ext.c
+++ b/block/parallels-ext.c
@@ -24,6 +24,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/error-report.h"
 #include "qapi/error.h"
 #include "block/block-io.h"
 #include "block/block_int.h"
@@ -300,3 +301,189 @@ out:
 
     return ret;
 }
+
+static void GRAPH_RDLOCK parallels_save_bitmap(BlockDriverState *bs,
+                                               BdrvDirtyBitmap *bitmap,
+                                               uint8_t **buf, int *buf_size)
+{
+    BDRVParallelsState *s = bs->opaque;
+    ParallelsFeatureHeader *fh;
+    ParallelsDirtyBitmapFeature *bh;
+    uint64_t *l1_table, l1_size, granularity, bits_per_cluster;
+    int64_t bm_size, ser_size, offset, buf_used;
+    int64_t idx, alloc_size = 1;
+    const char *name;
+    uint8_t *bm_buf;
+    QemuUUID uuid;
+    int ret = 0;
+
+    if (!bdrv_dirty_bitmap_get_persistence(bitmap) ||
+        bdrv_dirty_bitmap_inconsistent(bitmap)) {
+        return;
+    }
+
+    bm_size = bdrv_dirty_bitmap_size(bitmap);
+    granularity = bdrv_dirty_bitmap_granularity(bitmap);
+    bits_per_cluster = bdrv_dirty_bitmap_serialization_coverage(s->cluster_size,
+                                                                bitmap);
+    ser_size = bdrv_dirty_bitmap_serialization_size(bitmap, 0, bm_size);
+    l1_size = DIV_ROUND_UP(ser_size, s->cluster_size);
+
+    buf_used = l1_size * 8 + sizeof(*fh) + sizeof(*bh);
+    /* Check if there is enough space for the final section */
+    if (*buf_size - buf_used < sizeof(*fh)) {
+        error_report("Can't save dirty bitmap: not enoughf space");
+        return;
+    }
+
+    name = bdrv_dirty_bitmap_name(bitmap);
+    ret = qemu_uuid_parse(name, &uuid);
+    if (ret < 0) {
+        error_report("Can't save dirty bitmap '%s': ID parsing error", name);
+        return;
+    }
+
+    fh = (ParallelsFeatureHeader *)*buf;
+    bh = (ParallelsDirtyBitmapFeature *)(fh + 1);
+    l1_table = (uint64_t *)(bh + 1);
+
+    fh->magic = cpu_to_le64(PARALLELS_DIRTY_BITMAP_FEATURE_MAGIC);
+    fh->data_size = cpu_to_le32(l1_size * 8 + sizeof(*bh));
+
+    bh->l1_size = cpu_to_le32(l1_size);
+    bh->size = cpu_to_le64(bm_size >> BDRV_SECTOR_BITS);
+    bh->granularity = cpu_to_le32(granularity >> BDRV_SECTOR_BITS);
+    memcpy(bh->id, &uuid, sizeof(uuid));
+
+    bm_buf = qemu_blockalign(bs, s->cluster_size);
+
+    offset = 0;
+    while ((offset = bdrv_dirty_bitmap_next_dirty(bitmap, offset, bm_size)) >= 0) {
+        idx = offset / bits_per_cluster;
+        int64_t cluster_off, end, write_size;
+
+        offset = QEMU_ALIGN_DOWN(offset, bits_per_cluster);
+        end = MIN(bm_size, offset + bits_per_cluster);
+        write_size = bdrv_dirty_bitmap_serialization_size(bitmap, offset,
+                                                          end - offset);
+        assert(write_size <= s->cluster_size);
+
+        bdrv_dirty_bitmap_serialize_part(bitmap, bm_buf, offset, end - offset);
+        if (write_size < s->cluster_size) {
+            memset(bm_buf + write_size, 0, s->cluster_size - write_size);
+        }
+
+        l1_table[idx] = 0;
+
+        cluster_off = parallels_allocate_host_clusters(bs, &alloc_size);
+        if (cluster_off <= 0) {
+            error_report("Can't save dirty bitmap '%s': cluster allocation error",
+                         name);
+            goto clean_allocated_clusters;
+        }
+
+        l1_table[idx] = cpu_to_le64(cluster_off >> BDRV_SECTOR_BITS);
+
+        ret = bdrv_pwrite(bs->file, cluster_off, s->cluster_size, bm_buf, 0);
+        if (ret < 0) {
+            error_report("Can't save dirty bitmap '%s': write error", name);
+            memset(&fh->magic, 0, sizeof(fh->magic));
+            goto clean_allocated_clusters;
+        }
+
+        offset = end;
+    }
+
+    *buf_size -= buf_used;
+    *buf += buf_used;
+    qemu_vfree(bm_buf);
+    return;
+
+clean_allocated_clusters:
+    for (; idx >= 0; idx--) {
+        uint64_t cluster_off;
+        cluster_off = le64_to_cpu(l1_table[idx]);
+        if (cluster_off == 0 || cluster_off == 1) {
+            continue;
+        }
+        parallels_mark_unused(bs, s->used_bmap, s->used_bmap_size,
+                              cluster_off << BDRV_SECTOR_BITS, 1);
+    }
+    qemu_vfree(bm_buf);
+}
+
+int GRAPH_RDLOCK
+parallels_store_persistent_dirty_bitmaps(BlockDriverState *bs, Error **errp)
+{
+    BDRVParallelsState *s = bs->opaque;
+    BdrvDirtyBitmap *bitmap;
+    ParallelsFormatExtensionHeader *eh;
+    int remaining = s->cluster_size;
+    uint8_t *buf, *pos;
+    int64_t header_off, alloc_size = 1;
+    g_autofree uint8_t *hash = NULL;
+    size_t hash_len = 0;
+    int ret;
+
+    s->header->ext_off = 0;
+
+    if (!bdrv_has_named_bitmaps(bs)) {
+        return 0;
+    }
+
+    buf = qemu_blockalign0(bs, s->cluster_size);
+
+    eh = (ParallelsFormatExtensionHeader *)buf;
+    pos = buf + sizeof(*eh);
+    remaining -= sizeof(*eh);
+
+    eh->magic = cpu_to_le64(PARALLELS_FORMAT_EXTENSION_MAGIC);
+
+    FOR_EACH_DIRTY_BITMAP(bs, bitmap) {
+        parallels_save_bitmap(bs, bitmap, &pos, &remaining);
+    }
+
+    header_off = parallels_allocate_host_clusters(bs, &alloc_size);
+    if (header_off < 0) {
+        error_report("Can't save dirty bitmap: cluster allocation error");
+        ret = header_off;
+        goto end;
+    }
+
+    ret = qcrypto_hash_bytes(QCRYPTO_HASH_ALG_MD5,
+                             (const char *)(buf + sizeof(*eh)),
+                             s->cluster_size - sizeof(*eh),
+                             &hash, &hash_len, errp);
+    if (ret < 0 || hash_len != sizeof(eh->check_sum)) {
+        error_report("Can't save dirty bitmap: hash error");
+        ret = -EINVAL;
+        goto end;
+    }
+    memcpy(eh->check_sum, hash, hash_len);
+
+    ret = bdrv_pwrite(bs->file, header_off, s->cluster_size, buf, 0);
+    if (ret < 0) {
+        error_report("Can't save dirty bitmap: IO error");
+        parallels_mark_unused(bs, s->used_bmap, s->used_bmap_size,
+                              header_off, 1);
+        goto end;
+    }
+
+    s->header->ext_off = cpu_to_le64(header_off / BDRV_SECTOR_SIZE);
+end:
+    qemu_vfree(buf);
+    return ret;
+}
+
+bool coroutine_fn parallels_co_can_store_new_dirty_bitmap(BlockDriverState *bs,
+                                                          const char *name,
+                                                          uint32_t granularity,
+                                                          Error **errp)
+{
+    if (bdrv_find_dirty_bitmap(bs, name)) {
+        error_setg(errp, "Bitmap already exists: %s", name);
+        return false;
+    }
+
+    return true;
+}
diff --git a/block/parallels.c b/block/parallels.c
index 86958506d0..9536e4241e 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -1471,14 +1471,25 @@ fail:
 static int GRAPH_RDLOCK parallels_inactivate(BlockDriverState *bs)
 {
     BDRVParallelsState *s = bs->opaque;
+    Error *err = NULL;
     int ret;
 
+    ret = parallels_store_persistent_dirty_bitmaps(bs, &err);
+    if (ret != 0) {
+        error_reportf_err(err, "Lost persistent bitmaps during "
+                          "inactivation of node '%s': ",
+                          bdrv_get_device_or_node_name(bs));
+    }
+
     s->header->inuse = 0;
     parallels_update_header(bs);
 
     /* errors are ignored, so we might as well pass exact=true */
-    ret = bdrv_truncate(bs->file, s->data_end << BDRV_SECTOR_BITS, true,
-                        PREALLOC_MODE_OFF, 0, NULL);
+    ret = bdrv_truncate(bs->file, s->data_end << BDRV_SECTOR_BITS,
+                        true, PREALLOC_MODE_OFF, 0, NULL);
+    if (ret < 0) {
+        error_report("Failed to truncate image: %s", strerror(-ret));
+    }
 
     return ret;
 }
@@ -1530,6 +1541,7 @@ static BlockDriver bdrv_parallels = {
     .bdrv_co_pdiscard           = parallels_co_pdiscard,
     .bdrv_co_pwrite_zeroes      = parallels_co_pwrite_zeroes,
     .bdrv_inactivate            = parallels_inactivate,
+    .bdrv_co_can_store_new_dirty_bitmap = parallels_co_can_store_new_dirty_bitmap,
 };
 
 static void bdrv_parallels_init(void)
diff --git a/block/parallels.h b/block/parallels.h
index 493c89e976..d1e46dcfa8 100644
--- a/block/parallels.h
+++ b/block/parallels.h
@@ -101,5 +101,10 @@ int64_t GRAPH_RDLOCK parallels_allocate_host_clusters(BlockDriverState *bs,
 int GRAPH_RDLOCK
 parallels_read_format_extension(BlockDriverState *bs, int64_t ext_off,
                                 Error **errp);
+int GRAPH_RDLOCK
+parallels_store_persistent_dirty_bitmaps(BlockDriverState *bs, Error **errp);
+bool coroutine_fn
+parallels_co_can_store_new_dirty_bitmap(BlockDriverState *bs, const char *name,
+                                        uint32_t granularity, Error **errp);
 
 #endif
-- 
2.40.1



^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v5 12/22] parallels: drop dirty bitmap data if the image was not properly closed
  2024-03-11 18:18 [PATCH v5 00/22] parallels: Add full dirty bitmap support Alexander Ivanov
                   ` (10 preceding siblings ...)
  2024-03-11 18:18 ` [PATCH v5 11/22] parallels: Add dirty bitmaps saving Alexander Ivanov
@ 2024-03-11 18:18 ` Alexander Ivanov
  2024-03-11 18:18 ` [PATCH v5 13/22] parallels: Let image extensions work in RW mode Alexander Ivanov
                   ` (9 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: Alexander Ivanov @ 2024-03-11 18:18 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-block, den, stefanha, vsementsov, kwolf, hreitz

From: "Denis V. Lunev" <den@openvz.org>

This data is obsolete.

The approach is exactly the same like we use with QCOW2.

Signed-off-by: Denis V. Lunev <den@openvz.org>
---
 block/parallels-ext.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/block/parallels-ext.c b/block/parallels-ext.c
index 9cb8e65c0d..7fa14b057d 100644
--- a/block/parallels-ext.c
+++ b/block/parallels-ext.c
@@ -255,6 +255,14 @@ parallels_parse_format_extension(BlockDriverState *bs, uint8_t *ext_cluster,
             return 0;
 
         case PARALLELS_DIRTY_BITMAP_FEATURE_MAGIC:
+            if (s->header_unclean) {
+                /*
+                 * The image was not closed correctly and thus dirty bitmap
+                 * data inside the image is considered as incorrect and thus
+                 * it should be dropper, exactly like we do for QCOW2.
+                 */
+                break;
+            }
             bitmap = parallels_load_bitmap(bs, pos, fh.data_size, errp);
             if (!bitmap) {
                 goto fail;
-- 
2.40.1



^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v5 13/22] parallels: Let image extensions work in RW mode
  2024-03-11 18:18 [PATCH v5 00/22] parallels: Add full dirty bitmap support Alexander Ivanov
                   ` (11 preceding siblings ...)
  2024-03-11 18:18 ` [PATCH v5 12/22] parallels: drop dirty bitmap data if the image was not properly closed Alexander Ivanov
@ 2024-03-11 18:18 ` Alexander Ivanov
  2024-03-11 18:18 ` [PATCH v5 14/22] parallels: Preserve extensions cluster for non-transient extensions Alexander Ivanov
                   ` (8 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: Alexander Ivanov @ 2024-03-11 18:18 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-block, den, stefanha, vsementsov, kwolf, hreitz

Now we support extensions saving and can let to work with them in
read-write mode.

Place dirty bitmap loading after used bitmap initialization - in the next
patch we will work with used bitmap during dirty bitmap loading.

Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
 block/parallels-ext.c |  4 ----
 block/parallels.c     | 25 ++++++++-----------------
 2 files changed, 8 insertions(+), 21 deletions(-)

diff --git a/block/parallels-ext.c b/block/parallels-ext.c
index 7fa14b057d..2d839006a4 100644
--- a/block/parallels-ext.c
+++ b/block/parallels-ext.c
@@ -175,10 +175,6 @@ parallels_load_bitmap(BlockDriverState *bs, uint8_t *data, size_t data_size,
         return NULL;
     }
 
-    /* We support format extension only for RO parallels images. */
-    assert(!(bs->open_flags & BDRV_O_RDWR));
-    bdrv_dirty_bitmap_set_readonly(bitmap, true);
-
     return bitmap;
 }
 
diff --git a/block/parallels.c b/block/parallels.c
index 9536e4241e..f51e35788e 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -1378,23 +1378,6 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags,
         s->header_size = size;
     }
 
-    if (ph.ext_off) {
-        if (flags & BDRV_O_RDWR) {
-            /*
-             * It's unsafe to open image RW if there is an extension (as we
-             * don't support it). But parallels driver in QEMU historically
-             * ignores the extension, so print warning and don't care.
-             */
-            warn_report("Format Extension ignored in RW mode");
-        } else {
-            ret = parallels_read_format_extension(
-                    bs, le64_to_cpu(ph.ext_off) << BDRV_SECTOR_BITS, errp);
-            if (ret < 0) {
-                goto fail;
-            }
-        }
-    }
-
     if ((flags & BDRV_O_RDWR) && !(flags & BDRV_O_INACTIVE)) {
         s->header->inuse = cpu_to_le32(HEADER_INUSE_MAGIC);
         ret = parallels_update_header(bs);
@@ -1432,6 +1415,14 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags,
     }
     need_check = need_check || ret < 0; /* These are correctable errors */
 
+    if (ph.ext_off) {
+        ret = parallels_read_format_extension(
+                bs, le64_to_cpu(ph.ext_off) << BDRV_SECTOR_BITS, errp);
+        if (ret < 0) {
+            goto fail;
+        }
+    }
+
     /*
      * We don't repair the image here if it's opened for checks. Also we don't
      * want to change inactive images and can't change readonly images.
-- 
2.40.1



^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v5 14/22] parallels: Preserve extensions cluster for non-transient extensions
  2024-03-11 18:18 [PATCH v5 00/22] parallels: Add full dirty bitmap support Alexander Ivanov
                   ` (12 preceding siblings ...)
  2024-03-11 18:18 ` [PATCH v5 13/22] parallels: Let image extensions work in RW mode Alexander Ivanov
@ 2024-03-11 18:18 ` Alexander Ivanov
  2024-03-11 18:18 ` [PATCH v5 15/22] parallels: Handle L1 entries equal to one Alexander Ivanov
                   ` (7 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: Alexander Ivanov @ 2024-03-11 18:18 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-block, den, stefanha, vsementsov, kwolf, hreitz

There could be non-transient extensions that require presence of the
extensions cluster during work. Mark extensions cluster as used at
extensions loading end nullify l1 tables of dirty bitmaps.
Use this cluster at dirty bitmap saving if it exists.

Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
 block/parallels-ext.c | 49 ++++++++++++++++++++++++++++++-------------
 1 file changed, 35 insertions(+), 14 deletions(-)

diff --git a/block/parallels-ext.c b/block/parallels-ext.c
index 2d839006a4..1ec23aa1e7 100644
--- a/block/parallels-ext.c
+++ b/block/parallels-ext.c
@@ -122,7 +122,7 @@ finish:
  */
 static BdrvDirtyBitmap * GRAPH_RDLOCK
 parallels_load_bitmap(BlockDriverState *bs, uint8_t *data, size_t data_size,
-                      Error **errp)
+                      int64_t ext_off, Error **errp)
 {
     int ret;
     ParallelsDirtyBitmapFeature bf;
@@ -130,6 +130,7 @@ parallels_load_bitmap(BlockDriverState *bs, uint8_t *data, size_t data_size,
     BdrvDirtyBitmap *bitmap;
     QemuUUID uuid;
     char uuidstr[UUID_STR_LEN];
+    int64_t l1_off;
     int i;
 
     if (data_size < sizeof(bf)) {
@@ -171,16 +172,29 @@ parallels_load_bitmap(BlockDriverState *bs, uint8_t *data, size_t data_size,
 
     ret = parallels_load_bitmap_data(bs, l1_table, bf.l1_size, bitmap, errp);
     if (ret < 0) {
-        bdrv_release_dirty_bitmap(bitmap);
-        return NULL;
+        goto err;
+    }
+
+    if (!(bs->open_flags & BDRV_O_RDWR)) {
+        return bitmap;
+    }
+
+    l1_off = ext_off + sizeof(ParallelsDirtyBitmapFeature);
+    ret = bdrv_pwrite_zeroes(bs->file, l1_off,
+                             bf.l1_size * sizeof(uint64_t), 0);
+    if (ret < 0) {
+        goto err;
     }
 
     return bitmap;
+err:
+    bdrv_release_dirty_bitmap(bitmap);
+    return NULL;
 }
 
 static int GRAPH_RDLOCK
-parallels_parse_format_extension(BlockDriverState *bs, uint8_t *ext_cluster,
-                                 Error **errp)
+parallels_parse_format_extension(BlockDriverState *bs, int64_t ext_off,
+                                 uint8_t *ext_cluster, Error **errp)
 {
     BDRVParallelsState *s = bs->opaque;
     int ret;
@@ -259,7 +273,8 @@ parallels_parse_format_extension(BlockDriverState *bs, uint8_t *ext_cluster,
                  */
                 break;
             }
-            bitmap = parallels_load_bitmap(bs, pos, fh.data_size, errp);
+            bitmap = parallels_load_bitmap(bs, pos, fh.data_size,
+                                           ext_off + pos - ext_cluster, errp);
             if (!bitmap) {
                 goto fail;
             }
@@ -298,7 +313,11 @@ int parallels_read_format_extension(BlockDriverState *bs,
         goto out;
     }
 
-    ret = parallels_parse_format_extension(bs, ext_cluster, errp);
+    ret = parallels_parse_format_extension(bs, ext_off, ext_cluster, errp);
+    if (ret == 0) {
+        ret = parallels_mark_used(bs, s->used_bmap, s->used_bmap_size,
+                                  ext_off, 1);
+    }
 
 out:
     qemu_vfree(ext_cluster);
@@ -429,8 +448,6 @@ parallels_store_persistent_dirty_bitmaps(BlockDriverState *bs, Error **errp)
     size_t hash_len = 0;
     int ret;
 
-    s->header->ext_off = 0;
-
     if (!bdrv_has_named_bitmaps(bs)) {
         return 0;
     }
@@ -447,11 +464,15 @@ parallels_store_persistent_dirty_bitmaps(BlockDriverState *bs, Error **errp)
         parallels_save_bitmap(bs, bitmap, &pos, &remaining);
     }
 
-    header_off = parallels_allocate_host_clusters(bs, &alloc_size);
-    if (header_off < 0) {
-        error_report("Can't save dirty bitmap: cluster allocation error");
-        ret = header_off;
-        goto end;
+    if (s->header->ext_off) {
+        header_off = le64_to_cpu(s->header->ext_off) << BDRV_SECTOR_BITS;
+    } else {
+        header_off = parallels_allocate_host_clusters(bs, &alloc_size);
+        if (header_off < 0) {
+            error_report("Can't save dirty bitmap: cluster allocation error");
+            ret = header_off;
+            goto end;
+        }
     }
 
     ret = qcrypto_hash_bytes(QCRYPTO_HASH_ALG_MD5,
-- 
2.40.1



^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v5 15/22] parallels: Handle L1 entries equal to one
  2024-03-11 18:18 [PATCH v5 00/22] parallels: Add full dirty bitmap support Alexander Ivanov
                   ` (13 preceding siblings ...)
  2024-03-11 18:18 ` [PATCH v5 14/22] parallels: Preserve extensions cluster for non-transient extensions Alexander Ivanov
@ 2024-03-11 18:18 ` Alexander Ivanov
  2024-03-11 18:18 ` [PATCH v5 16/22] parallels: Make a loaded dirty bitmap persistent Alexander Ivanov
                   ` (6 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: Alexander Ivanov @ 2024-03-11 18:18 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-block, den, stefanha, vsementsov, kwolf, hreitz

If all the bits in a dirty bitmap cluster are ones, the cluster shouldn't
be written. Instead the corresponding L1 entry should be set to 1.

Check if all bits in a memory region are ones and set 1 to L1 entries
corresponding clusters filled with ones.

Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
 block/parallels-ext.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/block/parallels-ext.c b/block/parallels-ext.c
index 1ec23aa1e7..67fa3b73c2 100644
--- a/block/parallels-ext.c
+++ b/block/parallels-ext.c
@@ -383,7 +383,7 @@ static void GRAPH_RDLOCK parallels_save_bitmap(BlockDriverState *bs,
     offset = 0;
     while ((offset = bdrv_dirty_bitmap_next_dirty(bitmap, offset, bm_size)) >= 0) {
         idx = offset / bits_per_cluster;
-        int64_t cluster_off, end, write_size;
+        int64_t cluster_off, end, write_size, first_zero;
 
         offset = QEMU_ALIGN_DOWN(offset, bits_per_cluster);
         end = MIN(bm_size, offset + bits_per_cluster);
@@ -398,6 +398,16 @@ static void GRAPH_RDLOCK parallels_save_bitmap(BlockDriverState *bs,
 
         l1_table[idx] = 0;
 
+        first_zero = bdrv_dirty_bitmap_next_zero(bitmap, offset, bm_size);
+        if (first_zero < 0) {
+            goto clean_allocated_clusters;
+        }
+        if (first_zero - offset >= s->cluster_size) {
+            l1_table[idx] = 1;
+            offset = end;
+            continue;
+        }
+
         cluster_off = parallels_allocate_host_clusters(bs, &alloc_size);
         if (cluster_off <= 0) {
             error_report("Can't save dirty bitmap '%s': cluster allocation error",
-- 
2.40.1



^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v5 16/22] parallels: Make a loaded dirty bitmap persistent
  2024-03-11 18:18 [PATCH v5 00/22] parallels: Add full dirty bitmap support Alexander Ivanov
                   ` (14 preceding siblings ...)
  2024-03-11 18:18 ` [PATCH v5 15/22] parallels: Handle L1 entries equal to one Alexander Ivanov
@ 2024-03-11 18:18 ` Alexander Ivanov
  2024-03-11 18:18 ` [PATCH v5 17/22] parallels: Reverse a conditional in parallels_check_leak() to reduce indents Alexander Ivanov
                   ` (5 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: Alexander Ivanov @ 2024-03-11 18:18 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-block, den, stefanha, vsementsov, kwolf, hreitz

After bitmap loading the bitmap is not persistent and is removed on image
saving. Set bitmap persistence to true.

Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
Reviewed-by: Denis V. Lunev <den@openvz.org>
---
 block/parallels-ext.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/block/parallels-ext.c b/block/parallels-ext.c
index 67fa3b73c2..0816b9dc07 100644
--- a/block/parallels-ext.c
+++ b/block/parallels-ext.c
@@ -278,6 +278,7 @@ parallels_parse_format_extension(BlockDriverState *bs, int64_t ext_off,
             if (!bitmap) {
                 goto fail;
             }
+            bdrv_dirty_bitmap_set_persistence(bitmap, true);
             bitmaps = g_slist_append(bitmaps, bitmap);
             break;
 
-- 
2.40.1



^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v5 17/22] parallels: Reverse a conditional in parallels_check_leak() to reduce indents
  2024-03-11 18:18 [PATCH v5 00/22] parallels: Add full dirty bitmap support Alexander Ivanov
                   ` (15 preceding siblings ...)
  2024-03-11 18:18 ` [PATCH v5 16/22] parallels: Make a loaded dirty bitmap persistent Alexander Ivanov
@ 2024-03-11 18:18 ` Alexander Ivanov
  2024-03-11 18:18 ` [PATCH v5 18/22] parallels: Check unused clusters in parallels_check_leak() Alexander Ivanov
                   ` (4 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: Alexander Ivanov @ 2024-03-11 18:18 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-block, den, stefanha, vsementsov, kwolf, hreitz

Let the function return a success code if a file size is not bigger than
image_end_offset. Thus we can decrease indents in the next code block.

Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
Reviewed-by: Denis V. Lunev <den@openvz.org>
---
 block/parallels.c | 72 +++++++++++++++++++++++------------------------
 1 file changed, 36 insertions(+), 36 deletions(-)

diff --git a/block/parallels.c b/block/parallels.c
index f51e35788e..5155b8ac48 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -778,7 +778,7 @@ parallels_check_leak(BlockDriverState *bs, BdrvCheckResult *res,
                      BdrvCheckMode fix, bool explicit)
 {
     BDRVParallelsState *s = bs->opaque;
-    int64_t size;
+    int64_t size, count;
     int ret;
 
     size = bdrv_co_getlength(bs->file->bs);
@@ -786,43 +786,43 @@ parallels_check_leak(BlockDriverState *bs, BdrvCheckResult *res,
         res->check_errors++;
         return size;
     }
+    if (size <= res->image_end_offset) {
+        return 0;
+    }
+
+    count = DIV_ROUND_UP(size - res->image_end_offset, s->cluster_size);
+    if (explicit) {
+        fprintf(stderr,
+                "%s space leaked at the end of the image %" PRId64 "\n",
+                fix & BDRV_FIX_LEAKS ? "Repairing" : "ERROR",
+                size - res->image_end_offset);
+        res->leaks += count;
+    }
+    if (fix & BDRV_FIX_LEAKS) {
+        Error *local_err = NULL;
+
+        /*
+         * In order to really repair the image, we must shrink it.
+         * That means we have to pass exact=true.
+         */
+        ret = bdrv_co_truncate(bs->file, res->image_end_offset, true,
+                               PREALLOC_MODE_OFF, 0, &local_err);
+        if (ret < 0) {
+            error_report_err(local_err);
+            res->check_errors++;
+            return ret;
+        }
+        s->data_end = res->image_end_offset >> BDRV_SECTOR_BITS;
+
+        parallels_free_used_bitmap(bs);
+        ret = parallels_fill_used_bitmap(bs);
+        if (ret == -ENOMEM) {
+            res->check_errors++;
+            return ret;
+        }
 
-    if (size > res->image_end_offset) {
-        int64_t count;
-        count = DIV_ROUND_UP(size - res->image_end_offset, s->cluster_size);
         if (explicit) {
-            fprintf(stderr,
-                    "%s space leaked at the end of the image %" PRId64 "\n",
-                    fix & BDRV_FIX_LEAKS ? "Repairing" : "ERROR",
-                    size - res->image_end_offset);
-            res->leaks += count;
-        }
-        if (fix & BDRV_FIX_LEAKS) {
-            Error *local_err = NULL;
-
-            /*
-             * In order to really repair the image, we must shrink it.
-             * That means we have to pass exact=true.
-             */
-            ret = bdrv_co_truncate(bs->file, res->image_end_offset, true,
-                                   PREALLOC_MODE_OFF, 0, &local_err);
-            if (ret < 0) {
-                error_report_err(local_err);
-                res->check_errors++;
-                return ret;
-            }
-            s->data_end = res->image_end_offset >> BDRV_SECTOR_BITS;
-
-            parallels_free_used_bitmap(bs);
-            ret = parallels_fill_used_bitmap(bs);
-            if (ret == -ENOMEM) {
-                res->check_errors++;
-                return ret;
-            }
-
-            if (explicit) {
-                res->leaks_fixed += count;
-            }
+            res->leaks_fixed += count;
         }
     }
 
-- 
2.40.1



^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v5 18/22] parallels: Check unused clusters in parallels_check_leak()
  2024-03-11 18:18 [PATCH v5 00/22] parallels: Add full dirty bitmap support Alexander Ivanov
                   ` (16 preceding siblings ...)
  2024-03-11 18:18 ` [PATCH v5 17/22] parallels: Reverse a conditional in parallels_check_leak() to reduce indents Alexander Ivanov
@ 2024-03-11 18:18 ` Alexander Ivanov
  2024-03-11 18:18 ` [PATCH v5 19/22] parallels: Remove unnecessary data_end field Alexander Ivanov
                   ` (3 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: Alexander Ivanov @ 2024-03-11 18:18 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-block, den, stefanha, vsementsov, kwolf, hreitz

Since we have a used bitmap, current leak check is useless. Add
parallels_check_unused_clusters() helper that checks if there are unused
clusters at the end of image and truncates them (if needed). Use it for
leak check and at image inactivation.

In leak check we need to recreate used bitmap because it will be used
during work. At image inactivation we can skip it.

Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
 block/parallels.c | 106 +++++++++++++++++++++++++++++-----------------
 1 file changed, 68 insertions(+), 38 deletions(-)

diff --git a/block/parallels.c b/block/parallels.c
index 5155b8ac48..fd80179642 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -773,57 +773,89 @@ parallels_check_outside_image(BlockDriverState *bs, BdrvCheckResult *res,
     return 0;
 }
 
+static int64_t GRAPH_RDLOCK
+parallels_check_unused_clusters(BlockDriverState *bs, bool truncate)
+{
+    BDRVParallelsState *s = bs->opaque;
+    int64_t leak, file_size, end_off = 0;
+    int ret;
+
+    file_size = bdrv_getlength(bs->file->bs);
+    if (file_size < 0) {
+        return file_size;
+    }
+
+    if (s->used_bmap_size > 0) {
+        end_off = find_last_bit(s->used_bmap, s->used_bmap_size);
+        if (end_off == s->used_bmap_size) {
+            end_off = 0;
+        } else {
+            end_off = (end_off + 1) * s->cluster_size;
+        }
+    }
+
+    end_off += s->data_start * BDRV_SECTOR_SIZE;
+    leak = file_size - end_off;
+    if (leak < 0) {
+        return -EINVAL;
+    }
+    if (!truncate || leak == 0) {
+        return leak;
+    }
+
+    ret = bdrv_truncate(bs->file, end_off, true, PREALLOC_MODE_OFF, 0, NULL);
+    if (ret) {
+        return ret;
+    }
+
+    s->data_end = end_off / BDRV_SECTOR_SIZE;
+
+    return leak;
+}
+
 static int coroutine_fn GRAPH_RDLOCK
 parallels_check_leak(BlockDriverState *bs, BdrvCheckResult *res,
                      BdrvCheckMode fix, bool explicit)
 {
     BDRVParallelsState *s = bs->opaque;
-    int64_t size, count;
+    int64_t leak, count, size;
     int ret;
 
+    leak = parallels_check_unused_clusters(bs, fix & BDRV_FIX_LEAKS);
+    if (leak < 0) {
+        res->check_errors++;
+        return leak;
+    }
+    if (leak == 0) {
+        return 0;
+    }
+
+    parallels_free_used_bitmap(bs);
+    ret = parallels_fill_used_bitmap(bs);
+    if (ret < 0) {
+        res->check_errors++;
+        return ret;
+    }
+
     size = bdrv_co_getlength(bs->file->bs);
     if (size < 0) {
         res->check_errors++;
         return size;
     }
-    if (size <= res->image_end_offset) {
+    res->image_end_offset = size;
+
+    if (!explicit) {
         return 0;
     }
 
-    count = DIV_ROUND_UP(size - res->image_end_offset, s->cluster_size);
-    if (explicit) {
-        fprintf(stderr,
-                "%s space leaked at the end of the image %" PRId64 "\n",
-                fix & BDRV_FIX_LEAKS ? "Repairing" : "ERROR",
-                size - res->image_end_offset);
-        res->leaks += count;
-    }
+    count = DIV_ROUND_UP(leak, s->cluster_size);
+    fprintf(stderr,
+            "%s space leaked at the end of the image %" PRId64 "\n",
+            fix & BDRV_FIX_LEAKS ? "Repairing" : "ERROR", leak);
+    res->leaks += count;
+
     if (fix & BDRV_FIX_LEAKS) {
-        Error *local_err = NULL;
-
-        /*
-         * In order to really repair the image, we must shrink it.
-         * That means we have to pass exact=true.
-         */
-        ret = bdrv_co_truncate(bs->file, res->image_end_offset, true,
-                               PREALLOC_MODE_OFF, 0, &local_err);
-        if (ret < 0) {
-            error_report_err(local_err);
-            res->check_errors++;
-            return ret;
-        }
-        s->data_end = res->image_end_offset >> BDRV_SECTOR_BITS;
-
-        parallels_free_used_bitmap(bs);
-        ret = parallels_fill_used_bitmap(bs);
-        if (ret == -ENOMEM) {
-            res->check_errors++;
-            return ret;
-        }
-
-        if (explicit) {
-            res->leaks_fixed += count;
-        }
+        res->leaks_fixed += count;
     }
 
     return 0;
@@ -1475,9 +1507,7 @@ static int GRAPH_RDLOCK parallels_inactivate(BlockDriverState *bs)
     s->header->inuse = 0;
     parallels_update_header(bs);
 
-    /* errors are ignored, so we might as well pass exact=true */
-    ret = bdrv_truncate(bs->file, s->data_end << BDRV_SECTOR_BITS,
-                        true, PREALLOC_MODE_OFF, 0, NULL);
+    ret = parallels_check_unused_clusters(bs, true);
     if (ret < 0) {
         error_report("Failed to truncate image: %s", strerror(-ret));
     }
-- 
2.40.1



^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v5 19/22] parallels: Remove unnecessary data_end field
  2024-03-11 18:18 [PATCH v5 00/22] parallels: Add full dirty bitmap support Alexander Ivanov
                   ` (17 preceding siblings ...)
  2024-03-11 18:18 ` [PATCH v5 18/22] parallels: Check unused clusters in parallels_check_leak() Alexander Ivanov
@ 2024-03-11 18:18 ` Alexander Ivanov
  2024-03-11 18:18 ` [PATCH v5 20/22] tests: Add parallels images support to test 165 Alexander Ivanov
                   ` (2 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: Alexander Ivanov @ 2024-03-11 18:18 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-block, den, stefanha, vsementsov, kwolf, hreitz

Since we have used bitmap, field data_end in BDRVParallelsState is
redundant and can be removed.

Add parallels_data_end() helper and remove data_end handling.

Note: bdrv_pwrite_zeroes() was removed from the branch where we reallocate
clusters. It's redundant - bdrv_pwrite_zeroes() was already called for all
such clusters.

Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
 block/parallels.c | 45 +++++++++++++--------------------------------
 block/parallels.h |  1 -
 2 files changed, 13 insertions(+), 33 deletions(-)

diff --git a/block/parallels.c b/block/parallels.c
index fd80179642..11b6f97454 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -268,6 +268,13 @@ static void parallels_free_used_bitmap(BlockDriverState *bs)
     s->used_bmap = NULL;
 }
 
+static int64_t parallels_data_end(BDRVParallelsState *s)
+{
+    int64_t data_end = s->data_start * BDRV_SECTOR_SIZE;
+    data_end += s->used_bmap_size * s->cluster_size;
+    return data_end;
+}
+
 int64_t GRAPH_RDLOCK parallels_allocate_host_clusters(BlockDriverState *bs,
                                                       int64_t *clusters)
 {
@@ -279,7 +286,7 @@ int64_t GRAPH_RDLOCK parallels_allocate_host_clusters(BlockDriverState *bs,
 
     first_free = find_first_zero_bit(s->used_bmap, s->used_bmap_size);
     if (first_free == s->used_bmap_size) {
-        host_off = s->data_end * BDRV_SECTOR_SIZE;
+        host_off = parallels_data_end(s);
         prealloc_clusters = *clusters + s->prealloc_size / s->tracks;
         bytes = *clusters * s->cluster_size;
         prealloc_bytes = prealloc_clusters * s->cluster_size;
@@ -307,31 +314,14 @@ int64_t GRAPH_RDLOCK parallels_allocate_host_clusters(BlockDriverState *bs,
         s->used_bmap = bitmap_zero_extend(s->used_bmap, s->used_bmap_size,
                                           new_usedsize);
         s->used_bmap_size = new_usedsize;
-        if (host_off + bytes > s->data_end * BDRV_SECTOR_SIZE) {
-            s->data_end = (host_off + bytes) / BDRV_SECTOR_SIZE;
-        }
     } else {
         next_used = find_next_bit(s->used_bmap, s->used_bmap_size, first_free);
 
         /* Not enough continuous clusters in the middle, adjust the size */
         *clusters = MIN(*clusters, next_used - first_free);
-        bytes = *clusters * s->cluster_size;
 
         host_off = s->data_start * BDRV_SECTOR_SIZE;
         host_off += first_free * s->cluster_size;
-
-        /*
-         * No need to preallocate if we are using tail area from the above
-         * branch. In the other case we are likely re-using hole. Preallocate
-         * the space if required by the prealloc_mode.
-         */
-        if (s->prealloc_mode == PRL_PREALLOC_MODE_FALLOCATE &&
-                host_off < s->data_end * BDRV_SECTOR_SIZE) {
-            ret = bdrv_pwrite_zeroes(bs->file, host_off, bytes, 0);
-            if (ret < 0) {
-                return ret;
-            }
-        }
     }
 
     ret = parallels_mark_used(bs, s->used_bmap, s->used_bmap_size,
@@ -763,13 +753,7 @@ parallels_check_outside_image(BlockDriverState *bs, BdrvCheckResult *res,
         }
     }
 
-    if (high_off == 0) {
-        res->image_end_offset = s->data_end << BDRV_SECTOR_BITS;
-    } else {
-        res->image_end_offset = high_off + s->cluster_size;
-        s->data_end = res->image_end_offset >> BDRV_SECTOR_BITS;
-    }
-
+    res->image_end_offset = parallels_data_end(s);
     return 0;
 }
 
@@ -808,8 +792,6 @@ parallels_check_unused_clusters(BlockDriverState *bs, bool truncate)
         return ret;
     }
 
-    s->data_end = end_off / BDRV_SECTOR_SIZE;
-
     return leak;
 }
 
@@ -1401,8 +1383,7 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags,
     }
 
     s->data_start = data_start;
-    s->data_end = s->data_start;
-    if (s->data_end < (s->header_size >> BDRV_SECTOR_BITS)) {
+    if (s->data_start < (s->header_size >> BDRV_SECTOR_BITS)) {
         /*
          * There is not enough unused space to fit to block align between BAT
          * and actual data. We can't avoid read-modify-write...
@@ -1435,11 +1416,11 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags,
 
     for (i = 0; i < s->bat_size; i++) {
         sector = bat2sect(s, i);
-        if (sector + s->tracks > s->data_end) {
-            s->data_end = sector + s->tracks;
+        if (sector + s->tracks > file_nb_sectors) {
+            need_check = true;
+            break;
         }
     }
-    need_check = need_check || s->data_end > file_nb_sectors;
 
     ret = parallels_fill_used_bitmap(bs);
     if (ret == -ENOMEM) {
diff --git a/block/parallels.h b/block/parallels.h
index d1e46dcfa8..1d41a59b6f 100644
--- a/block/parallels.h
+++ b/block/parallels.h
@@ -79,7 +79,6 @@ typedef struct BDRVParallelsState {
     unsigned int bat_size;
 
     int64_t  data_start;
-    int64_t  data_end;
     uint64_t prealloc_size;
     ParallelsPreallocMode prealloc_mode;
 
-- 
2.40.1



^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v5 20/22] tests: Add parallels images support to test 165
  2024-03-11 18:18 [PATCH v5 00/22] parallels: Add full dirty bitmap support Alexander Ivanov
                   ` (18 preceding siblings ...)
  2024-03-11 18:18 ` [PATCH v5 19/22] parallels: Remove unnecessary data_end field Alexander Ivanov
@ 2024-03-11 18:18 ` Alexander Ivanov
  2024-03-11 18:18 ` [PATCH v5 21/22] tests: Turned on 256, 299, 304 and block-status-cache for parallels format Alexander Ivanov
  2024-03-11 18:18 ` [PATCH v5 22/22] tests: Add parallels format support to image-fleecing Alexander Ivanov
  21 siblings, 0 replies; 23+ messages in thread
From: Alexander Ivanov @ 2024-03-11 18:18 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-block, den, stefanha, vsementsov, kwolf, hreitz

Use a different bitmap name for parallels images because their has own ID
format, and can't contain an arbitrary string.

Replace image reopen by shutdown/launch VM because parallels images doesn't
support reopen.

Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
 tests/qemu-iotests/165 | 40 +++++++++++++++++++++++++---------------
 1 file changed, 25 insertions(+), 15 deletions(-)

diff --git a/tests/qemu-iotests/165 b/tests/qemu-iotests/165
index b24907a62f..f732db257c 100755
--- a/tests/qemu-iotests/165
+++ b/tests/qemu-iotests/165
@@ -38,6 +38,10 @@ class TestPersistentDirtyBitmap(iotests.QMPTestCase):
 
     def setUp(self):
         qemu_img('create', '-f', iotests.imgfmt, disk, str(disk_size))
+        if iotests.imgfmt == 'parallels':
+            self.bitmap_name = '00000000-0000-0000-0000-000000000000'
+        else:
+            self.bitmap_name = 'bitmap0'
 
     def tearDown(self):
         os.remove(disk)
@@ -50,12 +54,12 @@ class TestPersistentDirtyBitmap(iotests.QMPTestCase):
 
     def getSha256(self):
         result = self.vm.qmp('x-debug-block-dirty-bitmap-sha256',
-                             node='drive0', name='bitmap0')
+                             node='drive0', name=self.bitmap_name)
         return result['return']['sha256']
 
     def checkBitmap(self, sha256):
         result = self.vm.qmp('x-debug-block-dirty-bitmap-sha256',
-                             node='drive0', name='bitmap0')
+                             node='drive0', name=self.bitmap_name)
         self.assert_qmp(result, 'return/sha256', sha256);
 
     def writeRegions(self, regions):
@@ -65,7 +69,7 @@ class TestPersistentDirtyBitmap(iotests.QMPTestCase):
 
     def qmpAddBitmap(self):
         self.vm.qmp('block-dirty-bitmap-add', node='drive0',
-                    name='bitmap0', persistent=True)
+                    name=self.bitmap_name, persistent=True)
 
     def test_persistent(self):
         self.vm = self.mkVm()
@@ -117,7 +121,7 @@ class TestPersistentDirtyBitmap(iotests.QMPTestCase):
         assert sha256_1 != sha256_2 # Otherwise, it's not very interesting.
 
         self.vm.cmd('block-dirty-bitmap-clear', node='drive0',
-                    name='bitmap0')
+                    name=self.bitmap_name)
 
         # Start with regions1
 
@@ -135,16 +139,22 @@ class TestPersistentDirtyBitmap(iotests.QMPTestCase):
         self.writeRegions(regions2)
         assert sha256_1 == self.getSha256()
 
-        # Reopen to RW
-        self.vm.cmd('blockdev-reopen', options=[{
-            'node-name': 'node0',
-            'driver': iotests.imgfmt,
-            'file': {
-                'driver': 'file',
-                'filename': disk
-            },
-            'read-only': False
-        }])
+        if iotests.imgfmt == 'parallels':
+            # parallels doesn't support reopen
+            self.vm.shutdown()
+            self.vm = self.mkVm()
+            self.vm.launch()
+        else:
+            # Reopen to RW
+            self.vm.cmd('blockdev-reopen', options=[{
+                'node-name': 'node0',
+                'driver': iotests.imgfmt,
+                'file': {
+                    'driver': 'file',
+                    'filename': disk
+                },
+                'read-only': False
+            }])
 
         # Check that bitmap is reopened to RW and we can write to it.
         self.writeRegions(regions2)
@@ -154,6 +164,6 @@ class TestPersistentDirtyBitmap(iotests.QMPTestCase):
 
 
 if __name__ == '__main__':
-    iotests.main(supported_fmts=['qcow2'],
+    iotests.main(supported_fmts=['qcow2', 'parallels'],
                  supported_protocols=['file'],
                  unsupported_imgopts=['compat'])
-- 
2.40.1



^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v5 21/22] tests: Turned on 256, 299, 304 and block-status-cache for parallels format
  2024-03-11 18:18 [PATCH v5 00/22] parallels: Add full dirty bitmap support Alexander Ivanov
                   ` (19 preceding siblings ...)
  2024-03-11 18:18 ` [PATCH v5 20/22] tests: Add parallels images support to test 165 Alexander Ivanov
@ 2024-03-11 18:18 ` Alexander Ivanov
  2024-03-11 18:18 ` [PATCH v5 22/22] tests: Add parallels format support to image-fleecing Alexander Ivanov
  21 siblings, 0 replies; 23+ messages in thread
From: Alexander Ivanov @ 2024-03-11 18:18 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-block, den, stefanha, vsementsov, kwolf, hreitz

These tests pass with parallels format. Add parallels to supporting
formats for these tests.

Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
 tests/qemu-iotests/256                      | 2 +-
 tests/qemu-iotests/299                      | 2 +-
 tests/qemu-iotests/304                      | 2 +-
 tests/qemu-iotests/tests/block-status-cache | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/tests/qemu-iotests/256 b/tests/qemu-iotests/256
index f34af6cef7..1a4c9c6885 100755
--- a/tests/qemu-iotests/256
+++ b/tests/qemu-iotests/256
@@ -26,7 +26,7 @@ from iotests import log
 
 iotests.verify_virtio_scsi_pci_or_ccw()
 
-iotests.script_initialize(supported_fmts=['qcow2'])
+iotests.script_initialize(supported_fmts=['qcow2', 'parallels'])
 size = 64 * 1024 * 1024
 
 with iotests.FilePath('img0') as img0_path, \
diff --git a/tests/qemu-iotests/299 b/tests/qemu-iotests/299
index a7122941fd..d8c4399446 100755
--- a/tests/qemu-iotests/299
+++ b/tests/qemu-iotests/299
@@ -23,7 +23,7 @@ import iotests
 
 # The test is unrelated to formats, restrict it to qcow2 to avoid extra runs
 iotests.script_initialize(
-    supported_fmts=['qcow2'],
+    supported_fmts=['qcow2', 'parallels'],
 )
 
 nbd_sock = iotests.file_path('nbd.sock', base_dir=iotests.sock_dir)
diff --git a/tests/qemu-iotests/304 b/tests/qemu-iotests/304
index 198f282087..1ebf999930 100755
--- a/tests/qemu-iotests/304
+++ b/tests/qemu-iotests/304
@@ -23,7 +23,7 @@
 import iotests
 from iotests import qemu_img_create, qemu_img_log, file_path
 
-iotests.script_initialize(supported_fmts=['qcow2'],
+iotests.script_initialize(supported_fmts=['qcow2', 'parallels'],
                           supported_protocols=['file'])
 
 test_img = file_path('test.qcow2')
diff --git a/tests/qemu-iotests/tests/block-status-cache b/tests/qemu-iotests/tests/block-status-cache
index 5a7bc2c149..ade3d5b169 100755
--- a/tests/qemu-iotests/tests/block-status-cache
+++ b/tests/qemu-iotests/tests/block-status-cache
@@ -131,5 +131,5 @@ class TestBscWithNbd(iotests.QMPTestCase):
 if __name__ == '__main__':
     # The block-status cache only works on the protocol layer, so to test it,
     # we can only use the raw format
-    iotests.main(supported_fmts=['raw'],
+    iotests.main(supported_fmts=['raw', 'parallels'],
                  supported_protocols=['file'])
-- 
2.40.1



^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v5 22/22] tests: Add parallels format support to image-fleecing
  2024-03-11 18:18 [PATCH v5 00/22] parallels: Add full dirty bitmap support Alexander Ivanov
                   ` (20 preceding siblings ...)
  2024-03-11 18:18 ` [PATCH v5 21/22] tests: Turned on 256, 299, 304 and block-status-cache for parallels format Alexander Ivanov
@ 2024-03-11 18:18 ` Alexander Ivanov
  21 siblings, 0 replies; 23+ messages in thread
From: Alexander Ivanov @ 2024-03-11 18:18 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-block, den, stefanha, vsementsov, kwolf, hreitz

Use a different bitmap name for parallels images because their has own ID
format, and can't contain an arbitrary string.

Replace hardcoded 'qcow2' format to iotests.imgfmt.

Add 'parallels' to supported formats.

Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
 tests/qemu-iotests/tests/image-fleecing | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/tests/qemu-iotests/tests/image-fleecing b/tests/qemu-iotests/tests/image-fleecing
index 5e3b2c7e46..dd940b7203 100755
--- a/tests/qemu-iotests/tests/image-fleecing
+++ b/tests/qemu-iotests/tests/image-fleecing
@@ -28,7 +28,7 @@ import iotests
 from iotests import log, qemu_img, qemu_io
 
 iotests.script_initialize(
-    supported_fmts=['qcow2'],
+    supported_fmts=['qcow2', 'parallels'],
     supported_platforms=['linux'],
     required_fmts=['copy-before-write'],
     unsupported_imgopts=['compat']
@@ -61,12 +61,17 @@ def do_test(vm, use_cbw, use_snapshot_access_filter, base_img_path,
     if push_backup:
         assert use_cbw
 
+    if iotests.imgfmt == 'parallels':
+        bitmap_name = '00000000-0000-0000-0000-000000000000'
+    else:
+        bitmap_name = 'bitmap0'
+
     log('--- Setting up images ---')
     log('')
 
     qemu_img('create', '-f', iotests.imgfmt, base_img_path, '64M')
     if bitmap:
-        qemu_img('bitmap', '--add', base_img_path, 'bitmap0')
+        qemu_img('bitmap', '--add', base_img_path, bitmap_name)
 
     if use_snapshot_access_filter:
         assert use_cbw
@@ -75,7 +80,7 @@ def do_test(vm, use_cbw, use_snapshot_access_filter, base_img_path,
         qemu_img('create', '-f', 'qcow2', fleece_img_path, '64M')
 
     if push_backup:
-        qemu_img('create', '-f', 'qcow2', target_img_path, '64M')
+        qemu_img('create', '-f', iotests.imgfmt, target_img_path, '64M')
 
     for p in patterns:
         qemu_io('-f', iotests.imgfmt,
@@ -130,7 +135,7 @@ def do_test(vm, use_cbw, use_snapshot_access_filter, base_img_path,
         }
 
         if bitmap:
-            fl_cbw['bitmap'] = {'node': src_node, 'name': 'bitmap0'}
+            fl_cbw['bitmap'] = {'node': src_node, 'name': bitmap_name}
 
         log(vm.qmp('blockdev-add', fl_cbw))
 
-- 
2.40.1



^ permalink raw reply related	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2024-03-11 18:30 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-03-11 18:18 [PATCH v5 00/22] parallels: Add full dirty bitmap support Alexander Ivanov
2024-03-11 18:18 ` [PATCH v5 01/22] parallels: Set s->used_bmap to NULL in parallels_free_used_bitmap() Alexander Ivanov
2024-03-11 18:18 ` [PATCH v5 02/22] parallels: Move inactivation code to a separate function Alexander Ivanov
2024-03-11 18:18 ` [PATCH v5 03/22] parallels: Make mark_used() a global function Alexander Ivanov
2024-03-11 18:18 ` [PATCH v5 04/22] parallels: Limit search in parallels_mark_used to the last marked claster Alexander Ivanov
2024-03-11 18:18 ` [PATCH v5 05/22] parallels: Add parallels_mark_unused() helper Alexander Ivanov
2024-03-11 18:18 ` [PATCH v5 06/22] parallels: Move host clusters allocation to a separate function Alexander Ivanov
2024-03-11 18:18 ` [PATCH v5 07/22] parallels: Set data_end value in parallels_check_leak() Alexander Ivanov
2024-03-11 18:18 ` [PATCH v5 08/22] parallels: Recreate used bitmap " Alexander Ivanov
2024-03-11 18:18 ` [PATCH v5 09/22] parallels: Add a note about used bitmap in parallels_check_duplicate() Alexander Ivanov
2024-03-11 18:18 ` [PATCH v5 10/22] parallels: Create used bitmap even if checks needed Alexander Ivanov
2024-03-11 18:18 ` [PATCH v5 11/22] parallels: Add dirty bitmaps saving Alexander Ivanov
2024-03-11 18:18 ` [PATCH v5 12/22] parallels: drop dirty bitmap data if the image was not properly closed Alexander Ivanov
2024-03-11 18:18 ` [PATCH v5 13/22] parallels: Let image extensions work in RW mode Alexander Ivanov
2024-03-11 18:18 ` [PATCH v5 14/22] parallels: Preserve extensions cluster for non-transient extensions Alexander Ivanov
2024-03-11 18:18 ` [PATCH v5 15/22] parallels: Handle L1 entries equal to one Alexander Ivanov
2024-03-11 18:18 ` [PATCH v5 16/22] parallels: Make a loaded dirty bitmap persistent Alexander Ivanov
2024-03-11 18:18 ` [PATCH v5 17/22] parallels: Reverse a conditional in parallels_check_leak() to reduce indents Alexander Ivanov
2024-03-11 18:18 ` [PATCH v5 18/22] parallels: Check unused clusters in parallels_check_leak() Alexander Ivanov
2024-03-11 18:18 ` [PATCH v5 19/22] parallels: Remove unnecessary data_end field Alexander Ivanov
2024-03-11 18:18 ` [PATCH v5 20/22] tests: Add parallels images support to test 165 Alexander Ivanov
2024-03-11 18:18 ` [PATCH v5 21/22] tests: Turned on 256, 299, 304 and block-status-cache for parallels format Alexander Ivanov
2024-03-11 18:18 ` [PATCH v5 22/22] tests: Add parallels format support to image-fleecing Alexander Ivanov

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).