Re: [Qemu-block] [Qemu-devel] [PATCH v2 1/6] block: maintain persistent disabled bitmaps

2018-01-22 Thread Vladimir Sementsov-Ogievskiy 

20.01.2018 02:43, John Snow wrote:


On 01/16/2018 07:54 AM, Vladimir Sementsov-Ogievskiy wrote:

To maintain load/store disabled bitmap there is new approach:

  - deprecate @autoload flag of block-dirty-bitmap-add, make it ignored
  - store enabled bitmaps as "auto" to qcow2
  - store disabled bitmaps without "auto" flag to qcow2
  - on qcow2 open load "auto" bitmaps as enabled and others
as disabled (except in_use bitmaps)

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
  qapi/block-core.json |  6 +++---
  block/qcow2.h|  2 +-
  include/block/dirty-bitmap.h |  1 -
  block/dirty-bitmap.c | 18 --
  block/qcow2-bitmap.c | 12 +++-
  block/qcow2.c|  2 +-
  blockdev.c   | 10 ++
  7 files changed, 14 insertions(+), 37 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index ab96e348e6..827254db22 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1593,9 +1593,9 @@
  #  Qcow2 disks support persistent bitmaps. Default is false for
  #  block-dirty-bitmap-add. (Since: 2.10)
  #
-# @autoload: the bitmap will be automatically loaded when the image it is 
stored
-#in is opened. This flag may only be specified for persistent
-#bitmaps. Default is false for block-dirty-bitmap-add. (Since: 
2.10)
+# @autoload: ignored and deprecated since 2.12.
+#Currently, all dirty tracking bitmaps are loaded from Qcow2 on
+#open.

Hmm, so we're going to say that *all* persistent bitmaps are loaded into
memory, but they may-or-may-not-be enabled, is that the approach we're
taking now?


yes.




  #
  # Since: 2.4
  ##
diff --git a/block/qcow2.h b/block/qcow2.h
index 782a206ecb..a3e29276fc 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -672,7 +672,7 @@ void qcow2_cache_discard(BlockDriverState *bs, Qcow2Cache 
*c, void *table);
  int qcow2_check_bitmaps_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
void **refcount_table,
int64_t *refcount_table_size);
-bool qcow2_load_autoloading_dirty_bitmaps(BlockDriverState *bs, Error **errp);
+bool qcow2_load_dirty_bitmaps(BlockDriverState *bs, Error **errp);
  int qcow2_reopen_bitmaps_rw(BlockDriverState *bs, Error **errp);
  void qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs, Error **errp);
  int qcow2_reopen_bitmaps_ro(BlockDriverState *bs, Error **errp);
diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
index 3579a7597c..144e77a879 100644
--- a/include/block/dirty-bitmap.h
+++ b/include/block/dirty-bitmap.h
@@ -65,7 +65,6 @@ void bdrv_dirty_bitmap_deserialize_ones(BdrvDirtyBitmap 
*bitmap,
  void bdrv_dirty_bitmap_deserialize_finish(BdrvDirtyBitmap *bitmap);
  
  void bdrv_dirty_bitmap_set_readonly(BdrvDirtyBitmap *bitmap, bool value);

-void bdrv_dirty_bitmap_set_autoload(BdrvDirtyBitmap *bitmap, bool autoload);
  void bdrv_dirty_bitmap_set_persistance(BdrvDirtyBitmap *bitmap,
 bool persistent);
  
diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c

index bd04e991b1..3777be1985 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -52,8 +52,6 @@ struct BdrvDirtyBitmap {
 Such operations must fail and both the 
image
 and this bitmap must remain unchanged while
 this flag is set. */
-bool autoload;  /* For persistent bitmaps: bitmap must be
-   autoloaded on image opening */
  bool persistent;/* bitmap must be saved to owner disk image */
  QLIST_ENTRY(BdrvDirtyBitmap) list;
  };
@@ -104,7 +102,6 @@ void bdrv_dirty_bitmap_make_anon(BdrvDirtyBitmap *bitmap)
  g_free(bitmap->name);
  bitmap->name = NULL;
  bitmap->persistent = false;
-bitmap->autoload = false;
  }
  
  /* Called with BQL taken.  */

@@ -261,8 +258,6 @@ BdrvDirtyBitmap 
*bdrv_dirty_bitmap_abdicate(BlockDriverState *bs,
  bitmap->successor = NULL;
  successor->persistent = bitmap->persistent;
  bitmap->persistent = false;
-successor->autoload = bitmap->autoload;
-bitmap->autoload = false;
  bdrv_release_dirty_bitmap(bs, bitmap);
  
  return successor;

@@ -667,19 +662,6 @@ bool bdrv_has_readonly_bitmaps(BlockDriverState *bs)
  }
  
  /* Called with BQL taken. */

-void bdrv_dirty_bitmap_set_autoload(BdrvDirtyBitmap *bitmap, bool autoload)
-{
-qemu_mutex_lock(bitmap->mutex);
-bitmap->autoload = autoload;
-qemu_mutex_unlock(bitmap->mutex);
-}
-
-bool bdrv_dirty_bitmap_get_autoload(const BdrvDirtyBitmap *bitmap)
-{
-return bitmap->autoload;
-}
-
-/* Called with BQL taken. */
  void bdrv_dirty_bitmap_set_persistance(BdrvDirtyBitmap *bitmap, bool 
persistent)
  {

Re: [Qemu-block] [Qemu-devel] [PATCH v2 1/6] block: maintain persistent disabled bitmaps

2018-01-19 Thread John Snow 


On 01/16/2018 07:54 AM, Vladimir Sementsov-Ogievskiy wrote:
> To maintain load/store disabled bitmap there is new approach:
> 
>  - deprecate @autoload flag of block-dirty-bitmap-add, make it ignored
>  - store enabled bitmaps as "auto" to qcow2
>  - store disabled bitmaps without "auto" flag to qcow2
>  - on qcow2 open load "auto" bitmaps as enabled and others
>as disabled (except in_use bitmaps)
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> ---
>  qapi/block-core.json |  6 +++---
>  block/qcow2.h|  2 +-
>  include/block/dirty-bitmap.h |  1 -
>  block/dirty-bitmap.c | 18 --
>  block/qcow2-bitmap.c | 12 +++-
>  block/qcow2.c|  2 +-
>  blockdev.c   | 10 ++
>  7 files changed, 14 insertions(+), 37 deletions(-)
> 
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index ab96e348e6..827254db22 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -1593,9 +1593,9 @@
>  #  Qcow2 disks support persistent bitmaps. Default is false for
>  #  block-dirty-bitmap-add. (Since: 2.10)
>  #
> -# @autoload: the bitmap will be automatically loaded when the image it is 
> stored
> -#in is opened. This flag may only be specified for persistent
> -#bitmaps. Default is false for block-dirty-bitmap-add. (Since: 
> 2.10)
> +# @autoload: ignored and deprecated since 2.12.
> +#Currently, all dirty tracking bitmaps are loaded from Qcow2 on
> +#open.

Hmm, so we're going to say that *all* persistent bitmaps are loaded into
memory, but they may-or-may-not-be enabled, is that the approach we're
taking now?

>  #
>  # Since: 2.4
>  ##
> diff --git a/block/qcow2.h b/block/qcow2.h
> index 782a206ecb..a3e29276fc 100644
> --- a/block/qcow2.h
> +++ b/block/qcow2.h
> @@ -672,7 +672,7 @@ void qcow2_cache_discard(BlockDriverState *bs, Qcow2Cache 
> *c, void *table);
>  int qcow2_check_bitmaps_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
>void **refcount_table,
>int64_t *refcount_table_size);
> -bool qcow2_load_autoloading_dirty_bitmaps(BlockDriverState *bs, Error 
> **errp);
> +bool qcow2_load_dirty_bitmaps(BlockDriverState *bs, Error **errp);
>  int qcow2_reopen_bitmaps_rw(BlockDriverState *bs, Error **errp);
>  void qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs, Error 
> **errp);
>  int qcow2_reopen_bitmaps_ro(BlockDriverState *bs, Error **errp);
> diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
> index 3579a7597c..144e77a879 100644
> --- a/include/block/dirty-bitmap.h
> +++ b/include/block/dirty-bitmap.h
> @@ -65,7 +65,6 @@ void bdrv_dirty_bitmap_deserialize_ones(BdrvDirtyBitmap 
> *bitmap,
>  void bdrv_dirty_bitmap_deserialize_finish(BdrvDirtyBitmap *bitmap);
>  
>  void bdrv_dirty_bitmap_set_readonly(BdrvDirtyBitmap *bitmap, bool value);
> -void bdrv_dirty_bitmap_set_autoload(BdrvDirtyBitmap *bitmap, bool autoload);
>  void bdrv_dirty_bitmap_set_persistance(BdrvDirtyBitmap *bitmap,
> bool persistent);
>  
> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
> index bd04e991b1..3777be1985 100644
> --- a/block/dirty-bitmap.c
> +++ b/block/dirty-bitmap.c
> @@ -52,8 +52,6 @@ struct BdrvDirtyBitmap {
> Such operations must fail and both the 
> image
> and this bitmap must remain unchanged 
> while
> this flag is set. */
> -bool autoload;  /* For persistent bitmaps: bitmap must be
> -   autoloaded on image opening */
>  bool persistent;/* bitmap must be saved to owner disk image 
> */
>  QLIST_ENTRY(BdrvDirtyBitmap) list;
>  };
> @@ -104,7 +102,6 @@ void bdrv_dirty_bitmap_make_anon(BdrvDirtyBitmap *bitmap)
>  g_free(bitmap->name);
>  bitmap->name = NULL;
>  bitmap->persistent = false;
> -bitmap->autoload = false;
>  }
>  
>  /* Called with BQL taken.  */
> @@ -261,8 +258,6 @@ BdrvDirtyBitmap 
> *bdrv_dirty_bitmap_abdicate(BlockDriverState *bs,
>  bitmap->successor = NULL;
>  successor->persistent = bitmap->persistent;
>  bitmap->persistent = false;
> -successor->autoload = bitmap->autoload;
> -bitmap->autoload = false;
>  bdrv_release_dirty_bitmap(bs, bitmap);
>  
>  return successor;
> @@ -667,19 +662,6 @@ bool bdrv_has_readonly_bitmaps(BlockDriverState *bs)
>  }
>  
>  /* Called with BQL taken. */
> -void bdrv_dirty_bitmap_set_autoload(BdrvDirtyBitmap *bitmap, bool autoload)
> -{
> -qemu_mutex_lock(bitmap->mutex);
> -bitmap->autoload = autoload;
> -qemu_mutex_unlock(bitmap->mutex);
> -}
> -
> -bool bdrv_dirty_bitmap_get_autoload(const BdrvDirtyBitmap *bitmap)
> -{
> -return bitmap->autoload;
> -}
> -
> -/* Called