diff mbox series

[02/17] qcow2: No persistent dirty bitmaps for compat=0.10

Message ID 20171123020832.8165-3-mreitz@redhat.com
State New
Headers show
Series iotests: Fix iotests for weird formats/options | expand

Commit Message

Max Reitz Nov. 23, 2017, 2:08 a.m. UTC 
Persistent dirty bitmaps require a properly functioning
autoclear_features field, or we cannot track when an unsupporting
program might overwrite them.  Therefore, we cannot support them for
compat=0.10 images.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/qcow2-bitmap.c | 10 ++++++++++
 block/qcow2.c        | 14 +++++++++++---
 2 files changed, 21 insertions(+), 3 deletions(-)

Comments

Eric Blake Nov. 29, 2017, 4:23 p.m. UTC | #1 
On 11/22/2017 08:08 PM, Max Reitz wrote:
> Persistent dirty bitmaps require a properly functioning
> autoclear_features field, or we cannot track when an unsupporting
> program might overwrite them.  Therefore, we cannot support them for
> compat=0.10 images.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>   block/qcow2-bitmap.c | 10 ++++++++++
>   block/qcow2.c        | 14 +++++++++++---
>   2 files changed, 21 insertions(+), 3 deletions(-)

Missed Kevin's latest -rc3 pull request; oh well.

Reviewed-by: Eric Blake <eblake@redhat.com>
diff mbox series

Patch

diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
index f45e46cfbd..efa10c6663 100644
--- a/block/qcow2-bitmap.c
+++ b/block/qcow2-bitmap.c
@@ -1449,6 +1449,16 @@  bool qcow2_can_store_new_dirty_bitmap(BlockDriverState *bs,
     bool found;
     Qcow2BitmapList *bm_list;
 
+    if (s->qcow_version < 3) {
+        /* Without autoclear_features, we would always have to assume
+         * that a program without persistent dirty bitmap support has
+         * accessed this qcow2 file when opening it, and would thus
+         * have to drop all dirty bitmaps (defeating their purpose).
+         */
+        error_setg(errp, "Cannot store dirty bitmaps in qcow2 v2 files");
+        goto fail;
+    }
+
     if (check_constraints_on_bitmap(bs, name, granularity, errp) != 0) {
         goto fail;
     }
diff --git a/block/qcow2.c b/block/qcow2.c
index 1914a940e5..cabf93e43c 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -302,9 +302,17 @@  static int qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset,
             }
 
             if (!(s->autoclear_features & QCOW2_AUTOCLEAR_BITMAPS)) {
-                warn_report("a program lacking bitmap support "
-                            "modified this file, so all bitmaps are now "
-                            "considered inconsistent");
+                if (s->qcow_version < 3) {
+                    /* Let's be a bit more specific */
+                    warn_report("This qcow2 v2 image contains bitmaps, but "
+                                "they may have been modified by a program "
+                                "without persistent bitmap support; so now "
+                                "they must all be considered inconsistent");
+                } else {
+                    warn_report("a program lacking bitmap support "
+                                "modified this file, so all bitmaps are now "
+                                "considered inconsistent");
+                }
                 error_printf("Some clusters may be leaked, "
                              "run 'qemu-img check -r' on the image "
                              "file to fix.");