[Qemu-devel] [RFC PATCH 0/2] bitmaps: add inconsistent bit

[Qemu-devel] [RFC PATCH 0/2] bitmaps: add inconsistent bit

[John Snow]

Allow QEMU to read in bitmaps that have the in-use bit set, for the
purposes of allowing users to clear or reset these bitmaps.

This is chosen in preference to a hard error on load to minimize
impact for a non-critical error, but to force the user or management
utility to acknowledge that the bitmap is no longer viable.

Requires: [PATCH v2 0/6] dirty-bitmaps: deprecate @status field
          (Which in turn requires my bitmaps staging branch.)

RFC: This is just for general approach, naming, and API.
     I chose NOT to overload the busy predicate so that it would be distinct,
     which unfortunately means many more manual checks across blockdev.

     - I chose "inconsistent" over "corrupt" to be more literal to the
     meaning of the "in-use" bit, but maybe this is overcautious.

     - I chose to make the field optional so that it disappears in
     normative cases, as the information is only really relevant when
     inconsistent=true.

     I also have NOT tested this and I didn't verify the saving logic
     for what happens if you don't delete or clear the bitmap,
     but I'm on PTO the next two days and I wanted this to see daylight.

John Snow (2):
  block/dirty-bitmaps: add inconsistent bit
  block/dirty-bitmap: implement inconsistent bit

 block/dirty-bitmap.c         | 34 ++++++++++++++++++++++++++++
 block/qcow2-bitmap.c         | 42 ++++++++++++++++++-----------------
 blockdev.c                   | 43 ++++++++++++++++++++++++++++++++++++
 include/block/dirty-bitmap.h |  3 +++
 qapi/block-core.json         |  9 ++++++--
 5 files changed, 109 insertions(+), 22 deletions(-)

-- 
2.17.2

[Qemu-devel] [RFC PATCH 1/2] block/dirty-bitmaps: add inconsistent bit

[John Snow]

Add an inconsistent bit to dirty-bitmaps that allows us to report a bitmap as
persistent but potentially inconsistent, i.e. if we find bitmaps on a qcow2
that have been marked as "in use".

Signed-off-by: John Snow <jsnow@redhat.com>
---
 block/dirty-bitmap.c         | 19 +++++++++++++++++++
 include/block/dirty-bitmap.h |  2 ++
 qapi/block-core.json         |  9 +++++++--
 3 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index fc390cae94..b1879d7fbd 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -47,6 +47,9 @@ struct BdrvDirtyBitmap {
                                    and this bitmap must remain unchanged while
                                    this flag is set. */
     bool persistent;            /* bitmap must be saved to owner disk image */
+    bool inconsistent;          /* bitmap is persistent, but not owned by QEMU.
+                                 * It cannot be used at all in any way, except
+                                 * a QMP user can remove or clear it. */
     bool migration;             /* Bitmap is selected for migration, it should
                                    not be stored on the next inactivation
                                    (persistent flag doesn't matter until next
@@ -461,6 +464,8 @@ BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs)
         info->recording = bdrv_dirty_bitmap_recording(bm);
         info->busy = bdrv_dirty_bitmap_busy(bm);
         info->persistent = bm->persistent;
+        info->has_inconsistent = bm->inconsistent;
+        info->inconsistent = bm->inconsistent;
         entry->value = info;
         *plist = entry;
         plist = &entry->next;
@@ -708,6 +713,15 @@ void bdrv_dirty_bitmap_set_persistance(BdrvDirtyBitmap *bitmap, bool persistent)
     qemu_mutex_unlock(bitmap->mutex);
 }
 
+/* Called with BQL taken. */
+void bdrv_dirty_bitmap_set_inconsistent(BdrvDirtyBitmap *bitmap, bool value)
+{
+    qemu_mutex_lock(bitmap->mutex);
+    bitmap->inconsistent = value;
+    bitmap->disabled = value;
+    qemu_mutex_unlock(bitmap->mutex);
+}
+
 /* Called with BQL taken. */
 void bdrv_dirty_bitmap_set_migration(BdrvDirtyBitmap *bitmap, bool migration)
 {
@@ -721,6 +735,11 @@ bool bdrv_dirty_bitmap_get_persistance(BdrvDirtyBitmap *bitmap)
     return bitmap->persistent && !bitmap->migration;
 }
 
+bool bdrv_dirty_bitmap_inconsistent(BdrvDirtyBitmap *bitmap)
+{
+    return bitmap->inconsistent;
+}
+
 bool bdrv_has_changed_persistent_bitmaps(BlockDriverState *bs)
 {
     BdrvDirtyBitmap *bm;
diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
index ba8477b73f..c0d37702fd 100644
--- a/include/block/dirty-bitmap.h
+++ b/include/block/dirty-bitmap.h
@@ -68,6 +68,7 @@ void bdrv_dirty_bitmap_deserialize_finish(BdrvDirtyBitmap *bitmap);
 void bdrv_dirty_bitmap_set_readonly(BdrvDirtyBitmap *bitmap, bool value);
 void bdrv_dirty_bitmap_set_persistance(BdrvDirtyBitmap *bitmap,
                                        bool persistent);
+void bdrv_dirty_bitmap_set_inconsistent(BdrvDirtyBitmap *bitmap, bool value);
 void bdrv_dirty_bitmap_set_busy(BdrvDirtyBitmap *bitmap, bool busy);
 void bdrv_merge_dirty_bitmap(BdrvDirtyBitmap *dest, const BdrvDirtyBitmap *src,
                              HBitmap **backup, Error **errp);
@@ -91,6 +92,7 @@ bool bdrv_dirty_bitmap_readonly(const BdrvDirtyBitmap *bitmap);
 bool bdrv_has_readonly_bitmaps(BlockDriverState *bs);
 bool bdrv_dirty_bitmap_get_autoload(const BdrvDirtyBitmap *bitmap);
 bool bdrv_dirty_bitmap_get_persistance(BdrvDirtyBitmap *bitmap);
+bool bdrv_dirty_bitmap_inconsistent(BdrvDirtyBitmap *bitmap);
 bool bdrv_dirty_bitmap_busy(BdrvDirtyBitmap *bitmap);
 bool bdrv_has_changed_persistent_bitmaps(BlockDriverState *bs);
 BdrvDirtyBitmap *bdrv_dirty_bitmap_next(BlockDriverState *bs,
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 5d1d182447..f6b6dc2aff 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -470,12 +470,17 @@
 # @persistent: true if the bitmap will eventually be flushed to persistent
 #              storage (since 4.0)
 #
+# @inconsistent: true if this is a persistent bitmap that QEMU does not own.
+#                Implies @recording to be false and @busy to be true. To
+#                reclaim ownership, see @block-dirty-bitmap-remove or
+#                @block-dirty-bitmap-clear (since 4.0)
+#
 # Since: 1.3
 ##
 { 'struct': 'BlockDirtyInfo',
   'data': {'*name': 'str', 'count': 'int', 'granularity': 'uint32',
-           'recording': 'bool', 'busy': 'bool',
-           'status': 'DirtyBitmapStatus', 'persistent': 'bool' } }
+           'recording': 'bool', 'busy': 'bool', 'status': 'DirtyBitmapStatus',
+           'persistent': 'bool', '*inconsistent': 'bool' } }
 
 ##
 # @Qcow2BitmapInfoFlags:
-- 
2.17.2

[Qemu-devel] [RFC PATCH 2/2] block/dirty-bitmap: implement inconsistent bit

[John Snow]

Signed-off-by: John Snow <jsnow@redhat.com>
---
 block/dirty-bitmap.c         | 15 +++++++++++++
 block/qcow2-bitmap.c         | 42 ++++++++++++++++++-----------------
 blockdev.c                   | 43 ++++++++++++++++++++++++++++++++++++
 include/block/dirty-bitmap.h |  1 +
 4 files changed, 81 insertions(+), 20 deletions(-)

diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index b1879d7fbd..06d8ee0d79 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -589,6 +589,7 @@ void bdrv_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap, HBitmap **out)
         *out = backup;
     }
     bdrv_dirty_bitmap_unlock(bitmap);
+    bdrv_dirty_bitmap_set_inconsistent(bitmap, false);
 }
 
 void bdrv_restore_dirty_bitmap(BdrvDirtyBitmap *bitmap, HBitmap *backup)
@@ -776,6 +777,13 @@ bool bdrv_dirty_bitmap_next_dirty_area(BdrvDirtyBitmap *bitmap,
     return hbitmap_next_dirty_area(bitmap->bitmap, offset, bytes);
 }
 
+void bdrv_dirty_bitmap_add_inconsistent_hint(Error **errp)
+{
+    error_append_hint(errp, "Try block-dirty-bitmap-clear to mark this "
+                      "bitmap consistent again, or block-dirty-bitmap-remove "
+                      "to delete it.");
+}
+
 void bdrv_merge_dirty_bitmap(BdrvDirtyBitmap *dest, const BdrvDirtyBitmap *src,
                              HBitmap **backup, Error **errp)
 {
@@ -798,6 +806,13 @@ void bdrv_merge_dirty_bitmap(BdrvDirtyBitmap *dest, const BdrvDirtyBitmap *src,
         goto out;
     }
 
+    if (bdrv_dirty_bitmap_inconsistent(dest)) {
+        error_setg(errp, "Bitmap '%s' is inconsistent and cannot be used as"
+                   " a merge target", dest->name);
+        bdrv_dirty_bitmap_add_inconsistent_hint(errp);
+        goto out;
+    }
+
     if (!hbitmap_can_merge(dest->bitmap, src->bitmap)) {
         error_setg(errp, "Bitmaps are incompatible and can't be merged");
         goto out;
diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
index 3ee524da4b..9bd8bc417f 100644
--- a/block/qcow2-bitmap.c
+++ b/block/qcow2-bitmap.c
@@ -343,9 +343,15 @@ static BdrvDirtyBitmap *load_bitmap(BlockDriverState *bs,
     uint32_t granularity;
     BdrvDirtyBitmap *bitmap = NULL;
 
+    granularity = 1U << bm->granularity_bits;
+    bitmap = bdrv_create_dirty_bitmap(bs, granularity, bm->name, errp);
+    if (bitmap == NULL) {
+        goto fail;
+    }
+
     if (bm->flags & BME_FLAG_IN_USE) {
-        error_setg(errp, "Bitmap '%s' is in use", bm->name);
-        goto fail;
+        /* Data is unusable, skip loading it */
+        return bitmap;
     }
 
     ret = bitmap_table_load(bs, &bm->table, &bitmap_table);
@@ -356,12 +362,6 @@ static BdrvDirtyBitmap *load_bitmap(BlockDriverState *bs,
         goto fail;
     }
 
-    granularity = 1U << bm->granularity_bits;
-    bitmap = bdrv_create_dirty_bitmap(bs, granularity, bm->name, errp);
-    if (bitmap == NULL) {
-        goto fail;
-    }
-
     ret = load_bitmap_data(bs, bitmap_table, bm->table.size, bitmap);
     if (ret < 0) {
         error_setg_errno(errp, -ret, "Could not read bitmap '%s' from image",
@@ -962,20 +962,22 @@ bool qcow2_load_dirty_bitmaps(BlockDriverState *bs, Error **errp)
     }
 
     QSIMPLEQ_FOREACH(bm, bm_list, entry) {
-        if (!(bm->flags & BME_FLAG_IN_USE)) {
-            BdrvDirtyBitmap *bitmap = load_bitmap(bs, bm, errp);
-            if (bitmap == NULL) {
-                goto fail;
-            }
+        BdrvDirtyBitmap *bitmap = load_bitmap(bs, bm, errp);
+        if (bitmap == NULL) {
+            goto fail;
+        }
+
+        if (bm->flags & BME_FLAG_IN_USE) {
+            bdrv_dirty_bitmap_set_inconsistent(bitmap, true);
+        }
 
-            if (!(bm->flags & BME_FLAG_AUTO)) {
-                bdrv_disable_dirty_bitmap(bitmap);
-            }
-            bdrv_dirty_bitmap_set_persistance(bitmap, true);
-            bm->flags |= BME_FLAG_IN_USE;
-            created_dirty_bitmaps =
-                    g_slist_append(created_dirty_bitmaps, bitmap);
+        if (!(bm->flags & BME_FLAG_AUTO)) {
+            bdrv_disable_dirty_bitmap(bitmap);
         }
+        bdrv_dirty_bitmap_set_persistance(bitmap, true);
+        bm->flags |= BME_FLAG_IN_USE;
+        created_dirty_bitmaps =
+            g_slist_append(created_dirty_bitmaps, bitmap);
     }
 
     if (created_dirty_bitmaps != NULL) {
diff --git a/blockdev.c b/blockdev.c
index 23a4bf136e..12c6f706dd 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1943,6 +1943,7 @@ typedef struct BlockDirtyBitmapState {
     HBitmap *backup;
     bool prepared;
     bool was_enabled;
+    bool was_inconsistent;
 } BlockDirtyBitmapState;
 
 static void block_dirty_bitmap_add_prepare(BlkActionState *common,
@@ -2016,6 +2017,7 @@ static void block_dirty_bitmap_clear_prepare(BlkActionState *common,
         return;
     }
 
+    state->was_inconsistent = bdrv_dirty_bitmap_inconsistent(state->bitmap);
     bdrv_clear_dirty_bitmap(state->bitmap, &state->backup);
 }
 
@@ -2027,6 +2029,9 @@ static void block_dirty_bitmap_restore(BlkActionState *common)
     if (state->backup) {
         bdrv_restore_dirty_bitmap(state->bitmap, state->backup);
     }
+    if (state->was_inconsistent) {
+        bdrv_dirty_bitmap_set_inconsistent(state->bitmap, true);
+    }
 }
 
 static void block_dirty_bitmap_free_backup(BlkActionState *common)
@@ -2063,6 +2068,12 @@ static void block_dirty_bitmap_enable_prepare(BlkActionState *common,
                    " and cannot be enabled", action->name);
         return;
     }
+    if (bdrv_dirty_bitmap_inconsistent(state->bitmap)) {
+        error_setg(errp, "Bitmap '%s' is inconsistent and cannot be enabled",
+                   action->name);
+        bdrv_dirty_bitmap_add_inconsistent_hint(errp);
+        return;
+    }
 
     state->was_enabled = bdrv_dirty_bitmap_enabled(state->bitmap);
     bdrv_enable_dirty_bitmap(state->bitmap);
@@ -2104,6 +2115,12 @@ static void block_dirty_bitmap_disable_prepare(BlkActionState *common,
                    " and cannot be disabled", action->name);
         return;
     }
+    if (bdrv_dirty_bitmap_inconsistent(state->bitmap)) {
+        error_setg(errp, "Bitmap '%s' is inconsistent and cannot be disabled",
+                   action->name);
+        bdrv_dirty_bitmap_add_inconsistent_hint(errp);
+        return;
+    }
 
     state->was_enabled = bdrv_dirty_bitmap_enabled(state->bitmap);
     bdrv_disable_dirty_bitmap(state->bitmap);
@@ -2948,6 +2965,13 @@ void qmp_block_dirty_bitmap_enable(const char *node, const char *name,
         return;
     }
 
+    if (bdrv_dirty_bitmap_inconsistent(bitmap)) {
+        error_setg(errp, "Bitmap '%s' is inconsistent and cannot be enabled",
+                   name);
+        bdrv_dirty_bitmap_add_inconsistent_hint(errp);
+        return;
+    }
+
     bdrv_enable_dirty_bitmap(bitmap);
 }
 
@@ -2969,6 +2993,13 @@ void qmp_block_dirty_bitmap_disable(const char *node, const char *name,
         return;
     }
 
+    if (bdrv_dirty_bitmap_inconsistent(bitmap)) {
+        error_setg(errp, "Bitmap '%s' is inconsistent and cannot be disabled",
+                   name);
+        bdrv_dirty_bitmap_add_inconsistent_hint(errp);
+        return;
+    }
+
     bdrv_disable_dirty_bitmap(bitmap);
 }
 
@@ -3544,6 +3575,12 @@ static BlockJob *do_drive_backup(DriveBackup *backup, JobTxn *txn,
                        " and cannot be used for backup", backup->bitmap);
             goto out;
         }
+        if (bdrv_dirty_bitmap_inconsistent(bmap)) {
+            error_setg(errp, "Bitmap '%s' is inconsistent and cannot be used for"
+                       " a backup operation", backup->bitmap);
+            bdrv_dirty_bitmap_add_inconsistent_hint(errp);
+            goto out;
+        }
     }
     if (!backup->auto_finalize) {
         job_flags |= JOB_MANUAL_FINALIZE;
@@ -3657,6 +3694,12 @@ BlockJob *do_blockdev_backup(BlockdevBackup *backup, JobTxn *txn,
                        " and cannot be used for backup", backup->bitmap);
             goto out;
         }
+        if (bdrv_dirty_bitmap_inconsistent(bmap)) {
+            error_setg(errp, "Bitmap '%s' is inconsistent and cannot be used for"
+                       " a backup operation", backup->bitmap);
+            bdrv_dirty_bitmap_add_inconsistent_hint(errp);
+            goto out;
+        }
     }
 
     if (!backup->auto_finalize) {
diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
index c0d37702fd..b8485d2c94 100644
--- a/include/block/dirty-bitmap.h
+++ b/include/block/dirty-bitmap.h
@@ -105,5 +105,6 @@ bool bdrv_dirty_bitmap_next_dirty_area(BdrvDirtyBitmap *bitmap,
 BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap_locked(BlockDriverState *bs,
                                                   BdrvDirtyBitmap *bitmap,
                                                   Error **errp);
+void bdrv_dirty_bitmap_add_inconsistent_hint(Error **errp);
 
 #endif
-- 
2.17.2

Re: [Qemu-devel] [RFC PATCH 1/2] block/dirty-bitmaps: add inconsistent bit

[Vladimir Sementsov-Ogievskiy]

14.02.2019 2:36, John Snow wrote:
> Add an inconsistent bit to dirty-bitmaps that allows us to report a bitmap as
> persistent but potentially inconsistent, i.e. if we find bitmaps on a qcow2
> that have been marked as "in use".
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>   block/dirty-bitmap.c         | 19 +++++++++++++++++++
>   include/block/dirty-bitmap.h |  2 ++
>   qapi/block-core.json         |  9 +++++++--
>   3 files changed, 28 insertions(+), 2 deletions(-)
> 
> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
> index fc390cae94..b1879d7fbd 100644
> --- a/block/dirty-bitmap.c
> +++ b/block/dirty-bitmap.c
> @@ -47,6 +47,9 @@ struct BdrvDirtyBitmap {
>                                      and this bitmap must remain unchanged while
>                                      this flag is set. */
>       bool persistent;            /* bitmap must be saved to owner disk image */
> +    bool inconsistent;          /* bitmap is persistent, but not owned by QEMU.
> +                                 * It cannot be used at all in any way, except
> +                                 * a QMP user can remove or clear it. */
>       bool migration;             /* Bitmap is selected for migration, it should
>                                      not be stored on the next inactivation
>                                      (persistent flag doesn't matter until next
> @@ -461,6 +464,8 @@ BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs)
>           info->recording = bdrv_dirty_bitmap_recording(bm);
>           info->busy = bdrv_dirty_bitmap_busy(bm);
>           info->persistent = bm->persistent;
> +        info->has_inconsistent = bm->inconsistent;
> +        info->inconsistent = bm->inconsistent;
>           entry->value = info;
>           *plist = entry;
>           plist = &entry->next;
> @@ -708,6 +713,15 @@ void bdrv_dirty_bitmap_set_persistance(BdrvDirtyBitmap *bitmap, bool persistent)
>       qemu_mutex_unlock(bitmap->mutex);
>   }
>   
> +/* Called with BQL taken. */

Do we need BQL if we use mutex explicitly?

> +void bdrv_dirty_bitmap_set_inconsistent(BdrvDirtyBitmap *bitmap, bool value)
> +{
> +    qemu_mutex_lock(bitmap->mutex);
> +    bitmap->inconsistent = value;
> +    bitmap->disabled = value;

So, set inconsistent to false will enable the bitmap? Seems tricky.

> +    qemu_mutex_unlock(bitmap->mutex);
> +}
> +
>   /* Called with BQL taken. */
>   void bdrv_dirty_bitmap_set_migration(BdrvDirtyBitmap *bitmap, bool migration)
>   {
> @@ -721,6 +735,11 @@ bool bdrv_dirty_bitmap_get_persistance(BdrvDirtyBitmap *bitmap)
>       return bitmap->persistent && !bitmap->migration;
>   }
>   
> +bool bdrv_dirty_bitmap_inconsistent(BdrvDirtyBitmap *bitmap)
> +{
> +    return bitmap->inconsistent;
> +}
> +
>   bool bdrv_has_changed_persistent_bitmaps(BlockDriverState *bs)
>   {
>       BdrvDirtyBitmap *bm;
> diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
> index ba8477b73f..c0d37702fd 100644
> --- a/include/block/dirty-bitmap.h
> +++ b/include/block/dirty-bitmap.h
> @@ -68,6 +68,7 @@ void bdrv_dirty_bitmap_deserialize_finish(BdrvDirtyBitmap *bitmap);
>   void bdrv_dirty_bitmap_set_readonly(BdrvDirtyBitmap *bitmap, bool value);
>   void bdrv_dirty_bitmap_set_persistance(BdrvDirtyBitmap *bitmap,
>                                          bool persistent);
> +void bdrv_dirty_bitmap_set_inconsistent(BdrvDirtyBitmap *bitmap, bool value);
>   void bdrv_dirty_bitmap_set_busy(BdrvDirtyBitmap *bitmap, bool busy);
>   void bdrv_merge_dirty_bitmap(BdrvDirtyBitmap *dest, const BdrvDirtyBitmap *src,
>                                HBitmap **backup, Error **errp);
> @@ -91,6 +92,7 @@ bool bdrv_dirty_bitmap_readonly(const BdrvDirtyBitmap *bitmap);
>   bool bdrv_has_readonly_bitmaps(BlockDriverState *bs);
>   bool bdrv_dirty_bitmap_get_autoload(const BdrvDirtyBitmap *bitmap);
>   bool bdrv_dirty_bitmap_get_persistance(BdrvDirtyBitmap *bitmap);
> +bool bdrv_dirty_bitmap_inconsistent(BdrvDirtyBitmap *bitmap);
>   bool bdrv_dirty_bitmap_busy(BdrvDirtyBitmap *bitmap);
>   bool bdrv_has_changed_persistent_bitmaps(BlockDriverState *bs);
>   BdrvDirtyBitmap *bdrv_dirty_bitmap_next(BlockDriverState *bs,
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 5d1d182447..f6b6dc2aff 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -470,12 +470,17 @@
>   # @persistent: true if the bitmap will eventually be flushed to persistent
>   #              storage (since 4.0)
>   #
> +# @inconsistent: true if this is a persistent bitmap that QEMU does not own.
> +#                Implies @recording to be false and @busy to be true. To
> +#                reclaim ownership, see @block-dirty-bitmap-remove or
> +#                @block-dirty-bitmap-clear (since 4.0)
> +#
>   # Since: 1.3
>   ##
>   { 'struct': 'BlockDirtyInfo',
>     'data': {'*name': 'str', 'count': 'int', 'granularity': 'uint32',
> -           'recording': 'bool', 'busy': 'bool',
> -           'status': 'DirtyBitmapStatus', 'persistent': 'bool' } }
> +           'recording': 'bool', 'busy': 'bool', 'status': 'DirtyBitmapStatus',
> +           'persistent': 'bool', '*inconsistent': 'bool' } }
>   
>   ##
>   # @Qcow2BitmapInfoFlags:
> 


-- 
Best regards,
Vladimir

Re: [Qemu-devel] [RFC PATCH 0/2] bitmaps: add inconsistent bit

[Vladimir Sementsov-Ogievskiy]

14.02.2019 2:36, John Snow wrote:
> Allow QEMU to read in bitmaps that have the in-use bit set, for the
> purposes of allowing users to clear or reset these bitmaps.
> 
> This is chosen in preference to a hard error on load to minimize
> impact for a non-critical error, but to force the user or management
> utility to acknowledge that the bitmap is no longer viable.
> 
> Requires: [PATCH v2 0/6] dirty-bitmaps: deprecate @status field
>            (Which in turn requires my bitmaps staging branch.)
> 
> RFC: This is just for general approach, naming, and API.
>       I chose NOT to overload the busy predicate so that it would be distinct,
>       which unfortunately means many more manual checks across blockdev.

I think we just need a helper predicate with errp parameter, which will call both
_busy and _inconsistent.

> 
>       - I chose "inconsistent" over "corrupt" to be more literal to the
>       meaning of the "in-use" bit, but maybe this is overcautious.

I'm ok with both

> 
>       - I chose to make the field optional so that it disappears in
>       normative cases, as the information is only really relevant when
>       inconsistent=true.

Agree

> 
>       I also have NOT tested this and I didn't verify the saving logic
>       for what happens if you don't delete or clear the bitmap,
>       but I'm on PTO the next two days and I wanted this to see daylight.

I think, we should not change clear() logic to make bitmaps consistent. Is
there real usage of it?

1. bitmap (and its name) is related to some checkpoint, and clearing don't make it consistent
2. it is extra difficulty
3. we can add this logic separately, later, on demand, if needed
4. without it, we can use the following simple interface and easily save RAM:

no bdrv_dirty_bitmap_set_inconsistent(), but just bdrv_create_inconsistent_dirty_bitmap()
(ahaha cool function name), and call it in qcow2 code, when found in-use bitmap. And, in
this bdrv_create_inconsitent_dirty_bitmap() do not allocate HBitmap, which may save a
lot of memory.

> 
> John Snow (2):
>    block/dirty-bitmaps: add inconsistent bit
>    block/dirty-bitmap: implement inconsistent bit
> 
>   block/dirty-bitmap.c         | 34 ++++++++++++++++++++++++++++
>   block/qcow2-bitmap.c         | 42 ++++++++++++++++++-----------------
>   blockdev.c                   | 43 ++++++++++++++++++++++++++++++++++++
>   include/block/dirty-bitmap.h |  3 +++
>   qapi/block-core.json         |  9 ++++++--
>   5 files changed, 109 insertions(+), 22 deletions(-)
> 


-- 
Best regards,
Vladimir

Re: [Qemu-devel] [RFC PATCH 2/2] block/dirty-bitmap: implement inconsistent bit

[Vladimir Sementsov-Ogievskiy]

14.02.2019 2:36, John Snow wrote:
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>   block/dirty-bitmap.c         | 15 +++++++++++++
>   block/qcow2-bitmap.c         | 42 ++++++++++++++++++-----------------
>   blockdev.c                   | 43 ++++++++++++++++++++++++++++++++++++
>   include/block/dirty-bitmap.h |  1 +
>   4 files changed, 81 insertions(+), 20 deletions(-)
> 
> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
> index b1879d7fbd..06d8ee0d79 100644
> --- a/block/dirty-bitmap.c
> +++ b/block/dirty-bitmap.c
> @@ -589,6 +589,7 @@ void bdrv_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap, HBitmap **out)
>           *out = backup;
>       }
>       bdrv_dirty_bitmap_unlock(bitmap);
> +    bdrv_dirty_bitmap_set_inconsistent(bitmap, false);
>   }
>   
>   void bdrv_restore_dirty_bitmap(BdrvDirtyBitmap *bitmap, HBitmap *backup)
> @@ -776,6 +777,13 @@ bool bdrv_dirty_bitmap_next_dirty_area(BdrvDirtyBitmap *bitmap,
>       return hbitmap_next_dirty_area(bitmap->bitmap, offset, bytes);
>   }
>   
> +void bdrv_dirty_bitmap_add_inconsistent_hint(Error **errp)
> +{
> +    error_append_hint(errp, "Try block-dirty-bitmap-clear to mark this "
> +                      "bitmap consistent again, or block-dirty-bitmap-remove "
> +                      "to delete it.");

bitmaps created by libvirt (or somebody) are related to some checkpoint. And their name is
probably (Eric?) related to this checkpoint too. So, clear will never make them consistent..
Only clear :)

So, I don't like idea of clearing in-use bitmaps.

> +}
> +
>   void bdrv_merge_dirty_bitmap(BdrvDirtyBitmap *dest, const BdrvDirtyBitmap *src,
>                                HBitmap **backup, Error **errp)
>   {
> @@ -798,6 +806,13 @@ void bdrv_merge_dirty_bitmap(BdrvDirtyBitmap *dest, const BdrvDirtyBitmap *src,
>           goto out;
>       }
>   
> +    if (bdrv_dirty_bitmap_inconsistent(dest)) {
> +        error_setg(errp, "Bitmap '%s' is inconsistent and cannot be used as"
> +                   " a merge target", dest->name);
> +        bdrv_dirty_bitmap_add_inconsistent_hint(errp);
> +        goto out;
> +    }

I think, we need common predicate which will combine busy and inconsistent, as almost in all cases
we need both to be false to do any operation.

bdrv_dirty_bitmap_usable() ? :)

and pass errp to this helper.

Actually, we already need it, to fill errp, which we almost always fill in the same manner.

> +
>       if (!hbitmap_can_merge(dest->bitmap, src->bitmap)) {
>           error_setg(errp, "Bitmaps are incompatible and can't be merged");
>           goto out;
> diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
> index 3ee524da4b..9bd8bc417f 100644
> --- a/block/qcow2-bitmap.c
> +++ b/block/qcow2-bitmap.c

hmm, I also think we should report our deprecated status as locked for inconsistent bitmaps..


-- 
Best regards,
Vladimir

Re: [Qemu-devel] [RFC PATCH 2/2] block/dirty-bitmap: implement inconsistent bit

[Eric Blake]

On 2/18/19 12:13 PM, Vladimir Sementsov-Ogievskiy wrote:
> 14.02.2019 2:36, John Snow wrote:
>> Signed-off-by: John Snow <jsnow@redhat.com>
>> ---
>>   block/dirty-bitmap.c         | 15 +++++++++++++
>>   block/qcow2-bitmap.c         | 42 ++++++++++++++++++-----------------
>>   blockdev.c                   | 43 ++++++++++++++++++++++++++++++++++++
>>   include/block/dirty-bitmap.h |  1 +
>>   4 files changed, 81 insertions(+), 20 deletions(-)

>> +void bdrv_dirty_bitmap_add_inconsistent_hint(Error **errp)
>> +{
>> +    error_append_hint(errp, "Try block-dirty-bitmap-clear to mark this "
>> +                      "bitmap consistent again, or block-dirty-bitmap-remove "
>> +                      "to delete it.");
> 
> bitmaps created by libvirt (or somebody) are related to some checkpoint. And their name is
> probably (Eric?) related to this checkpoint too. So, clear will never make them consistent..
> Only clear :)
> 
> So, I don't like idea of clearing in-use bitmaps.

It's always possible to delete a bitmap and then create a new one by the
same name, to get the same effect of clearing an in-use bitmap. So let's
start simple and declare that the only valid operation on an
inconsistent bitmap is deletion.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [RFC PATCH 2/2] block/dirty-bitmap: implement inconsistent bit

[John Snow]

On 2/18/19 3:37 PM, Eric Blake wrote:
> On 2/18/19 12:13 PM, Vladimir Sementsov-Ogievskiy wrote:
>> 14.02.2019 2:36, John Snow wrote:
>>> Signed-off-by: John Snow <jsnow@redhat.com>
>>> ---
>>>   block/dirty-bitmap.c         | 15 +++++++++++++
>>>   block/qcow2-bitmap.c         | 42 ++++++++++++++++++-----------------
>>>   blockdev.c                   | 43 ++++++++++++++++++++++++++++++++++++
>>>   include/block/dirty-bitmap.h |  1 +
>>>   4 files changed, 81 insertions(+), 20 deletions(-)
> 
>>> +void bdrv_dirty_bitmap_add_inconsistent_hint(Error **errp)
>>> +{
>>> +    error_append_hint(errp, "Try block-dirty-bitmap-clear to mark this "
>>> +                      "bitmap consistent again, or block-dirty-bitmap-remove "
>>> +                      "to delete it.");
>>
>> bitmaps created by libvirt (or somebody) are related to some checkpoint. And their name is
>> probably (Eric?) related to this checkpoint too. So, clear will never make them consistent..
>> Only clear :)
>>
>> So, I don't like idea of clearing in-use bitmaps.
> 
> It's always possible to delete a bitmap and then create a new one by the
> same name, to get the same effect of clearing an in-use bitmap. So let's
> start simple and declare that the only valid operation on an
> inconsistent bitmap is deletion.
> 

OK, from the viewpoint of checkpoints specifically, that's a convincing
argument against it for now.

I'll tighten this.

Re: [Qemu-devel] [RFC PATCH 1/2] block/dirty-bitmaps: add inconsistent bit

[John Snow]

On 2/18/19 12:46 PM, Vladimir Sementsov-Ogievskiy wrote:
> 14.02.2019 2:36, John Snow wrote:
>> Add an inconsistent bit to dirty-bitmaps that allows us to report a bitmap as
>> persistent but potentially inconsistent, i.e. if we find bitmaps on a qcow2
>> that have been marked as "in use".
>>
>> Signed-off-by: John Snow <jsnow@redhat.com>
>> ---
>>   block/dirty-bitmap.c         | 19 +++++++++++++++++++
>>   include/block/dirty-bitmap.h |  2 ++
>>   qapi/block-core.json         |  9 +++++++--
>>   3 files changed, 28 insertions(+), 2 deletions(-)
>>
>> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
>> index fc390cae94..b1879d7fbd 100644
>> --- a/block/dirty-bitmap.c
>> +++ b/block/dirty-bitmap.c
>> @@ -47,6 +47,9 @@ struct BdrvDirtyBitmap {
>>                                      and this bitmap must remain unchanged while
>>                                      this flag is set. */
>>       bool persistent;            /* bitmap must be saved to owner disk image */
>> +    bool inconsistent;          /* bitmap is persistent, but not owned by QEMU.
>> +                                 * It cannot be used at all in any way, except
>> +                                 * a QMP user can remove or clear it. */
>>       bool migration;             /* Bitmap is selected for migration, it should
>>                                      not be stored on the next inactivation
>>                                      (persistent flag doesn't matter until next
>> @@ -461,6 +464,8 @@ BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs)
>>           info->recording = bdrv_dirty_bitmap_recording(bm);
>>           info->busy = bdrv_dirty_bitmap_busy(bm);
>>           info->persistent = bm->persistent;
>> +        info->has_inconsistent = bm->inconsistent;
>> +        info->inconsistent = bm->inconsistent;
>>           entry->value = info;
>>           *plist = entry;
>>           plist = &entry->next;
>> @@ -708,6 +713,15 @@ void bdrv_dirty_bitmap_set_persistance(BdrvDirtyBitmap *bitmap, bool persistent)
>>       qemu_mutex_unlock(bitmap->mutex);
>>   }
>>   
>> +/* Called with BQL taken. */
> 
> Do we need BQL if we use mutex explicitly?
> 
>> +void bdrv_dirty_bitmap_set_inconsistent(BdrvDirtyBitmap *bitmap, bool value)
>> +{
>> +    qemu_mutex_lock(bitmap->mutex);
>> +    bitmap->inconsistent = value;
>> +    bitmap->disabled = value;
> 
> So, set inconsistent to false will enable the bitmap? Seems tricky.
> 

It was really meant to be inconsistent = disabled = true. It falls apart
in the reverse. However, if we don't allow clear, this can just be a
constant.

>> +    qemu_mutex_unlock(bitmap->mutex);
>> +}
>> +
>>   /* Called with BQL taken. */
>>   void bdrv_dirty_bitmap_set_migration(BdrvDirtyBitmap *bitmap, bool migration)
>>   {
>> @@ -721,6 +735,11 @@ bool bdrv_dirty_bitmap_get_persistance(BdrvDirtyBitmap *bitmap)
>>       return bitmap->persistent && !bitmap->migration;
>>   }
>>   
>> +bool bdrv_dirty_bitmap_inconsistent(BdrvDirtyBitmap *bitmap)
>> +{
>> +    return bitmap->inconsistent;
>> +}
>> +
>>   bool bdrv_has_changed_persistent_bitmaps(BlockDriverState *bs)
>>   {
>>       BdrvDirtyBitmap *bm;
>> diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
>> index ba8477b73f..c0d37702fd 100644
>> --- a/include/block/dirty-bitmap.h
>> +++ b/include/block/dirty-bitmap.h
>> @@ -68,6 +68,7 @@ void bdrv_dirty_bitmap_deserialize_finish(BdrvDirtyBitmap *bitmap);
>>   void bdrv_dirty_bitmap_set_readonly(BdrvDirtyBitmap *bitmap, bool value);
>>   void bdrv_dirty_bitmap_set_persistance(BdrvDirtyBitmap *bitmap,
>>                                          bool persistent);
>> +void bdrv_dirty_bitmap_set_inconsistent(BdrvDirtyBitmap *bitmap, bool value);
>>   void bdrv_dirty_bitmap_set_busy(BdrvDirtyBitmap *bitmap, bool busy);
>>   void bdrv_merge_dirty_bitmap(BdrvDirtyBitmap *dest, const BdrvDirtyBitmap *src,
>>                                HBitmap **backup, Error **errp);
>> @@ -91,6 +92,7 @@ bool bdrv_dirty_bitmap_readonly(const BdrvDirtyBitmap *bitmap);
>>   bool bdrv_has_readonly_bitmaps(BlockDriverState *bs);
>>   bool bdrv_dirty_bitmap_get_autoload(const BdrvDirtyBitmap *bitmap);
>>   bool bdrv_dirty_bitmap_get_persistance(BdrvDirtyBitmap *bitmap);
>> +bool bdrv_dirty_bitmap_inconsistent(BdrvDirtyBitmap *bitmap);
>>   bool bdrv_dirty_bitmap_busy(BdrvDirtyBitmap *bitmap);
>>   bool bdrv_has_changed_persistent_bitmaps(BlockDriverState *bs);
>>   BdrvDirtyBitmap *bdrv_dirty_bitmap_next(BlockDriverState *bs,
>> diff --git a/qapi/block-core.json b/qapi/block-core.json
>> index 5d1d182447..f6b6dc2aff 100644
>> --- a/qapi/block-core.json
>> +++ b/qapi/block-core.json
>> @@ -470,12 +470,17 @@
>>   # @persistent: true if the bitmap will eventually be flushed to persistent
>>   #              storage (since 4.0)
>>   #
>> +# @inconsistent: true if this is a persistent bitmap that QEMU does not own.
>> +#                Implies @recording to be false and @busy to be true. To
>> +#                reclaim ownership, see @block-dirty-bitmap-remove or
>> +#                @block-dirty-bitmap-clear (since 4.0)
>> +#
>>   # Since: 1.3
>>   ##
>>   { 'struct': 'BlockDirtyInfo',
>>     'data': {'*name': 'str', 'count': 'int', 'granularity': 'uint32',
>> -           'recording': 'bool', 'busy': 'bool',
>> -           'status': 'DirtyBitmapStatus', 'persistent': 'bool' } }
>> +           'recording': 'bool', 'busy': 'bool', 'status': 'DirtyBitmapStatus',
>> +           'persistent': 'bool', '*inconsistent': 'bool' } }
>>   
>>   ##
>>   # @Qcow2BitmapInfoFlags:
>>
> 
> 

Re: [Qemu-devel] [RFC PATCH 2/2] block/dirty-bitmap: implement inconsistent bit

[John Snow]

On 2/18/19 1:13 PM, Vladimir Sementsov-Ogievskiy wrote:
> 14.02.2019 2:36, John Snow wrote:
>> Signed-off-by: John Snow <jsnow@redhat.com>
>> ---
>>   block/dirty-bitmap.c         | 15 +++++++++++++
>>   block/qcow2-bitmap.c         | 42 ++++++++++++++++++-----------------
>>   blockdev.c                   | 43 ++++++++++++++++++++++++++++++++++++
>>   include/block/dirty-bitmap.h |  1 +
>>   4 files changed, 81 insertions(+), 20 deletions(-)
>>
>> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
>> index b1879d7fbd..06d8ee0d79 100644
>> --- a/block/dirty-bitmap.c
>> +++ b/block/dirty-bitmap.c
>> @@ -589,6 +589,7 @@ void bdrv_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap, HBitmap **out)
>>           *out = backup;
>>       }
>>       bdrv_dirty_bitmap_unlock(bitmap);
>> +    bdrv_dirty_bitmap_set_inconsistent(bitmap, false);
>>   }
>>   
>>   void bdrv_restore_dirty_bitmap(BdrvDirtyBitmap *bitmap, HBitmap *backup)
>> @@ -776,6 +777,13 @@ bool bdrv_dirty_bitmap_next_dirty_area(BdrvDirtyBitmap *bitmap,
>>       return hbitmap_next_dirty_area(bitmap->bitmap, offset, bytes);
>>   }
>>   
>> +void bdrv_dirty_bitmap_add_inconsistent_hint(Error **errp)
>> +{
>> +    error_append_hint(errp, "Try block-dirty-bitmap-clear to mark this "
>> +                      "bitmap consistent again, or block-dirty-bitmap-remove "
>> +                      "to delete it.");
> 
> bitmaps created by libvirt (or somebody) are related to some checkpoint. And their name is
> probably (Eric?) related to this checkpoint too. So, clear will never make them consistent..
> Only clear :)
> 
> So, I don't like idea of clearing in-use bitmaps.
> 
>> +}
>> +
>>   void bdrv_merge_dirty_bitmap(BdrvDirtyBitmap *dest, const BdrvDirtyBitmap *src,
>>                                HBitmap **backup, Error **errp)
>>   {
>> @@ -798,6 +806,13 @@ void bdrv_merge_dirty_bitmap(BdrvDirtyBitmap *dest, const BdrvDirtyBitmap *src,
>>           goto out;
>>       }
>>   
>> +    if (bdrv_dirty_bitmap_inconsistent(dest)) {
>> +        error_setg(errp, "Bitmap '%s' is inconsistent and cannot be used as"
>> +                   " a merge target", dest->name);
>> +        bdrv_dirty_bitmap_add_inconsistent_hint(errp);
>> +        goto out;
>> +    }
> 
> I think, we need common predicate which will combine busy and inconsistent, as almost in all cases
> we need both to be false to do any operation.
> 
> bdrv_dirty_bitmap_usable() ? :)
> 
> and pass errp to this helper.
> 
> Actually, we already need it, to fill errp, which we almost always fill in the same manner.
> 
>> +
>>       if (!hbitmap_can_merge(dest->bitmap, src->bitmap)) {
>>           error_setg(errp, "Bitmaps are incompatible and can't be merged");
>>           goto out;
>> diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
>> index 3ee524da4b..9bd8bc417f 100644
>> --- a/block/qcow2-bitmap.c
>> +++ b/block/qcow2-bitmap.c
> 
> hmm, I also think we should report our deprecated status as locked for inconsistent bitmaps..
> 
> 

Though we're trying to deprecate the field altogether, I *could* add a
special status flag that makes it unambiguous. This will catch the
attention of anyone using the old API.

--js

Re: [Qemu-devel] [RFC PATCH 2/2] block/dirty-bitmap: implement inconsistent bit

[Vladimir Sementsov-Ogievskiy]

20.02.2019 1:00, John Snow wrote:
> 
> 
> On 2/18/19 1:13 PM, Vladimir Sementsov-Ogievskiy wrote:
>> 14.02.2019 2:36, John Snow wrote:
>>> Signed-off-by: John Snow <jsnow@redhat.com>
>>> ---
>>>    block/dirty-bitmap.c         | 15 +++++++++++++
>>>    block/qcow2-bitmap.c         | 42 ++++++++++++++++++-----------------
>>>    blockdev.c                   | 43 ++++++++++++++++++++++++++++++++++++
>>>    include/block/dirty-bitmap.h |  1 +
>>>    4 files changed, 81 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
>>> index b1879d7fbd..06d8ee0d79 100644
>>> --- a/block/dirty-bitmap.c
>>> +++ b/block/dirty-bitmap.c
>>> @@ -589,6 +589,7 @@ void bdrv_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap, HBitmap **out)
>>>            *out = backup;
>>>        }
>>>        bdrv_dirty_bitmap_unlock(bitmap);
>>> +    bdrv_dirty_bitmap_set_inconsistent(bitmap, false);
>>>    }
>>>    
>>>    void bdrv_restore_dirty_bitmap(BdrvDirtyBitmap *bitmap, HBitmap *backup)
>>> @@ -776,6 +777,13 @@ bool bdrv_dirty_bitmap_next_dirty_area(BdrvDirtyBitmap *bitmap,
>>>        return hbitmap_next_dirty_area(bitmap->bitmap, offset, bytes);
>>>    }
>>>    
>>> +void bdrv_dirty_bitmap_add_inconsistent_hint(Error **errp)
>>> +{
>>> +    error_append_hint(errp, "Try block-dirty-bitmap-clear to mark this "
>>> +                      "bitmap consistent again, or block-dirty-bitmap-remove "
>>> +                      "to delete it.");
>>
>> bitmaps created by libvirt (or somebody) are related to some checkpoint. And their name is
>> probably (Eric?) related to this checkpoint too. So, clear will never make them consistent..
>> Only clear :)
>>
>> So, I don't like idea of clearing in-use bitmaps.
>>
>>> +}
>>> +
>>>    void bdrv_merge_dirty_bitmap(BdrvDirtyBitmap *dest, const BdrvDirtyBitmap *src,
>>>                                 HBitmap **backup, Error **errp)
>>>    {
>>> @@ -798,6 +806,13 @@ void bdrv_merge_dirty_bitmap(BdrvDirtyBitmap *dest, const BdrvDirtyBitmap *src,
>>>            goto out;
>>>        }
>>>    
>>> +    if (bdrv_dirty_bitmap_inconsistent(dest)) {
>>> +        error_setg(errp, "Bitmap '%s' is inconsistent and cannot be used as"
>>> +                   " a merge target", dest->name);
>>> +        bdrv_dirty_bitmap_add_inconsistent_hint(errp);
>>> +        goto out;
>>> +    }
>>
>> I think, we need common predicate which will combine busy and inconsistent, as almost in all cases
>> we need both to be false to do any operation.
>>
>> bdrv_dirty_bitmap_usable() ? :)
>>
>> and pass errp to this helper.
>>
>> Actually, we already need it, to fill errp, which we almost always fill in the same manner.
>>
>>> +
>>>        if (!hbitmap_can_merge(dest->bitmap, src->bitmap)) {
>>>            error_setg(errp, "Bitmaps are incompatible and can't be merged");
>>>            goto out;
>>> diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
>>> index 3ee524da4b..9bd8bc417f 100644
>>> --- a/block/qcow2-bitmap.c
>>> +++ b/block/qcow2-bitmap.c
>>
>> hmm, I also think we should report our deprecated status as locked for inconsistent bitmaps..
>>
>>
> 
> Though we're trying to deprecate the field altogether, I *could* add a
> special status flag that makes it unambiguous. This will catch the
> attention of anyone using the old API.

I agree.


-- 
Best regards,
Vladimir

Re: [Qemu-devel] [RFC PATCH 2/2] block/dirty-bitmap: implement inconsistent bit

[Eric Blake]

On 2/19/19 4:00 PM, John Snow wrote:

>>
>> hmm, I also think we should report our deprecated status as locked for inconsistent bitmaps..
>>
>>
> 
> Though we're trying to deprecate the field altogether, I *could* add a
> special status flag that makes it unambiguous. This will catch the
> attention of anyone using the old API.

Adding to the enum, even though it is going away in the future, is still
helpful in the present, so I can live with that approach.


-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org