[Top][All Lists]

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

Re: [Qemu-block] [PATCH 12/22] qcow2-bitmap: add IN_USE flag

From: Vladimir Sementsov-Ogievskiy
Subject: Re: [Qemu-block] [PATCH 12/22] qcow2-bitmap: add IN_USE flag
Date: Fri, 21 Oct 2016 18:34:12 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0

07.10.2016 22:44, Max Reitz пишет:
On 30.09.2016 12:53, Vladimir Sementsov-Ogievskiy wrote:
This flag means that the bitmap is now in use by the software or was not
successfully saved. In any way, with this flag set the bitmap data must
be considered inconsistent and should not be loaded.

With current implementation this flag is never set, as we just remove
bitmaps from the image after loading. But it defined in qcow2 spec and
must be handled. Also, it can be used in future, if async schemes of
bitmap loading/saving are implemented.

We also remove in-use bitmaps from the image on open.

Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
  block/qcow2-bitmap.c | 17 ++++++++++++++++-
  1 file changed, 16 insertions(+), 1 deletion(-)
Don't you want to make use of this flag? It would appear useful to me if
you just marked autoload bitmaps as in_use instead of deleting them from
the image when it's opened and then overwrite them when the image is closed.

And what is the use of it?

Any way, storing an invalid copy of online data is bad I think. Therefore I will have to free bitmap data clusters and zero bitmap table in file.

diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
index a5be25a..8cf40f0 100644
--- a/block/qcow2-bitmap.c
+++ b/block/qcow2-bitmap.c
@@ -28,6 +28,7 @@
  #include "qemu/osdep.h"
  #include "qapi/error.h"
  #include "qemu/cutils.h"
+#include "exec/log.h"
#include "block/block_int.h"
  #include "block/qcow2.h"
@@ -44,7 +45,8 @@
  #define BME_MAX_NAME_SIZE 1023
/* Bitmap directory entry flags */
-#define BME_RESERVED_FLAGS 0xfffffffd
+#define BME_RESERVED_FLAGS 0xfffffffc
+#define BME_FLAG_IN_USE 1
This should be (1U << 0) to be consistent with the definition of

  #define BME_FLAG_AUTO   (1U << 1)
/* bits [1, 8] U [56, 63] are reserved */
@@ -287,6 +289,11 @@ static BdrvDirtyBitmap *load_bitmap(BlockDriverState *bs,
      BdrvDirtyBitmap *bitmap = NULL;
      char *name = g_strndup(dir_entry_name_notcstr(entry), entry->name_size);
+ if (entry->flags & BME_FLAG_IN_USE) {
+        error_setg(errp, "Bitmap '%s' is in use", name);
+        goto fail;
+    }
      ret = bitmap_table_load(bs, entry, &bitmap_table);
      if (ret < 0) {
          error_setg_errno(errp, -ret,
@@ -533,6 +540,14 @@ int qcow2_read_bitmaps(BlockDriverState *bs, Error **errp)
      for_each_bitmap_dir_entry(e, dir, dir_size) {
          uint32_t fl = e->flags;
+ if (fl & BME_FLAG_IN_USE) {
+            qemu_log("qcow2 warning: "
+                     "removing in_use bitmap '%.*s' for image %s.\n",
+                     e->name_size, (char *)(e + 1), 
I'm not sure whether qemu_log() is the right way to do this. I think
fprintf() to stderr might actually be more appropriate. qemu_log() looks
like it's mostly used for debugging purposes.

Also, this is not a warning. You are just doing it. You are not warning
someone about a cliff when he's already fallen off.

But you can actually make it a warning: Just leave the bitmap in the
image (maybe someone can do something with it?) and instead let qemu-img
check clean it up. The warning should then just be "Warning: Ignoring
in_use bitmap %.*s, use qemu-img check -r all to delete it".


+            continue;
+        }
          if (fl & BME_FLAG_AUTO) {
              BdrvDirtyBitmap *bitmap = load_bitmap(bs, e, errp);
              if (bitmap == NULL) {

Best regards,

reply via email to

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