qemu-block
[Top][All Lists]
Advanced

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

Re: [Qemu-block] [PATCH 07/21] qcow2: add bitmaps extension


From: Vladimir Sementsov-Ogievskiy
Subject: Re: [Qemu-block] [PATCH 07/21] qcow2: add bitmaps extension
Date: Wed, 14 Dec 2016 15:23:22 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.5.1

07.12.2016 21:25, Max Reitz wrote:
On 22.11.2016 18:26, Vladimir Sementsov-Ogievskiy wrote:
Add bitmap extension as specified in docs/specs/qcow2.txt.
For now, just mirror extension header into Qcow2 state and check
constraints.

For now, disable image resize if it has bitmaps. It will be fixed later.

Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
Reviewed-by: John Snow <address@hidden>
---
  block/qcow2.c | 107 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
  block/qcow2.h |  24 +++++++++++++
  2 files changed, 129 insertions(+), 2 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 6d5689a..7908657 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -63,6 +63,7 @@ typedef struct {
  #define  QCOW2_EXT_MAGIC_END 0
  #define  QCOW2_EXT_MAGIC_BACKING_FORMAT 0xE2792ACA
  #define  QCOW2_EXT_MAGIC_FEATURE_TABLE 0x6803f857
+#define  QCOW2_EXT_MAGIC_BITMAPS 0x23852875
static int qcow2_probe(const uint8_t *buf, int buf_size, const char *filename)
  {
@@ -92,6 +93,7 @@ static int qcow2_read_extensions(BlockDriverState *bs, 
uint64_t start_offset,
      QCowExtension ext;
      uint64_t offset;
      int ret;
+    Qcow2BitmapHeaderExt bitmaps_ext;
#ifdef DEBUG_EXT
      printf("qcow2_read_extensions: start=%ld end=%ld\n", start_offset, 
end_offset);
@@ -162,6 +164,81 @@ static int qcow2_read_extensions(BlockDriverState *bs, 
uint64_t start_offset,
              }
              break;
+ case QCOW2_EXT_MAGIC_BITMAPS:
+            if (ext.len != sizeof(bitmaps_ext)) {
+                error_setg_errno(errp, -ret, "bitmaps_ext: "
+                                 "Invalid extension length");
+                return -EINVAL;
+            }
+
+            if (!(s->autoclear_features & QCOW2_AUTOCLEAR_BITMAPS)) {
+                fprintf(stderr,
+                        "WARNING: a program lacking bitmap support modified "
+                        "this file, so all bitmaps are now considered "
+                        "inconsistent");
+                break;
+            }
I think It would be nice if qemu-img check could remove all bitmaps and
related data if this bit is not set. Otherwise, we're practically
leaking data.

Is it safe? Can old version somehow reuse these clusters? Actually, all data will be removed on check, as there will be no refcounts if this bit is not set.

So, IMHO, only extension header should be deleted additionally in qemu-img check.


(But this can be added in a later patch.)

Reviewed-by: Max Reitz <address@hidden>



--
Best regards,
Vladimir




reply via email to

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