All of lore.kernel.org
 help / color / mirror / Atom feed
* [PULL 0/7] bitmaps patches for 2020-05-18
@ 2020-05-18 20:54 Eric Blake
  2020-05-18 20:54 ` [PULL 1/7] bitmaps: Update maintainer Eric Blake
                   ` (7 more replies)
  0 siblings, 8 replies; 10+ messages in thread
From: Eric Blake @ 2020-05-18 20:54 UTC (permalink / raw)
  To: qemu-devel

The following changes since commit a28c9c8c9fc42484efe1bf5a77affe842e54e38b:

  Merge remote-tracking branch 'remotes/kraxel/tags/vga-20200518-pull-request' into staging (2020-05-18 16:37:09 +0100)

are available in the Git repository at:

  https://repo.or.cz/qemu/ericb.git tags/pull-bitmaps-2020-05-18

for you to fetch changes up to 6660c3a69c668ff98e2a86bca0b1301648ff4b02:

  qemu-img: Add bitmap sub-command (2020-05-18 15:32:56 -0500)

----------------------------------------------------------------
bitmaps patches for 2020-05-18

- update bitmaps maintainers
- add 'qemu-img bitmap' subcommand

----------------------------------------------------------------
Eric Blake (7):
      bitmaps: Update maintainer
      docs: Sort sections on qemu-img subcommand parameters
      qemu-img: Fix stale comments on doc location
      block: Make it easier to learn which BDS support bitmaps
      blockdev: Promote several bitmap functions to non-static
      blockdev: Split off basic bitmap operations for qemu-img
      qemu-img: Add bitmap sub-command

 docs/tools/qemu-img.rst         |  72 ++++++---
 Makefile.objs                   |   3 +-
 block/qcow2.h                   |   1 +
 include/block/block_int.h       |  13 ++
 include/block/dirty-bitmap.h    |   1 +
 block/dirty-bitmap.c            |   9 ++
 block/monitor/bitmap-qmp-cmds.c | 321 ++++++++++++++++++++++++++++++++++++++++
 block/qcow2-bitmap.c            |   7 +
 block/qcow2.c                   |   2 +
 blockdev.c                      | 303 +------------------------------------
 qemu-img.c                      | 251 ++++++++++++++++++++++++++++++-
 MAINTAINERS                     |   8 +-
 block/monitor/Makefile.objs     |   1 +
 qemu-img-cmds.hx                |   9 +-
 14 files changed, 672 insertions(+), 329 deletions(-)
 create mode 100644 block/monitor/bitmap-qmp-cmds.c

-- 
2.26.2



^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PULL 1/7] bitmaps: Update maintainer
  2020-05-18 20:54 [PULL 0/7] bitmaps patches for 2020-05-18 Eric Blake
@ 2020-05-18 20:54 ` Eric Blake
  2020-05-18 20:54 ` [PULL 2/7] docs: Sort sections on qemu-img subcommand parameters Eric Blake
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Eric Blake @ 2020-05-18 20:54 UTC (permalink / raw)
  To: qemu-devel; +Cc: John Snow

Dirty bitmaps are important to incremental backups, including exposure
over NBD where I'm already maintainer.  Also, I'm aware that lately I
have been doing as much code/review on bitmaps as John Snow who is
trying to scale back in order to focus elsewhere; and many of the
recent patches have come from Vladimir, who is also interested in
taking on maintainer duties, but would like to start with
co-maintainership.  Therefore, it's time to revamp the ownership of
this category, as agreed between the three of us.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200514180003.325406-1-eblake@redhat.com>
Acked-by: John Snow <jsnow@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
---
 MAINTAINERS | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 47ef3139e6d6..4e99bb05dad8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2011,8 +2011,9 @@ F: qapi/transaction.json
 T: git https://repo.or.cz/qemu/armbru.git block-next

 Dirty Bitmaps
-M: John Snow <jsnow@redhat.com>
-R: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
+M: Eric Blake <eblake@redhat.com>
+M: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
+R: John Snow <jsnow@redhat.com>
 L: qemu-block@nongnu.org
 S: Supported
 F: include/qemu/hbitmap.h
@@ -2023,7 +2024,7 @@ F: migration/block-dirty-bitmap.c
 F: util/hbitmap.c
 F: tests/test-hbitmap.c
 F: docs/interop/bitmaps.rst
-T: git https://github.com/jnsnow/qemu.git bitmaps
+T: git https://repo.or.cz/qemu/ericb.git bitmaps

 Character device backends
 M: Marc-André Lureau <marcandre.lureau@redhat.com>
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PULL 2/7] docs: Sort sections on qemu-img subcommand parameters
  2020-05-18 20:54 [PULL 0/7] bitmaps patches for 2020-05-18 Eric Blake
  2020-05-18 20:54 ` [PULL 1/7] bitmaps: Update maintainer Eric Blake
@ 2020-05-18 20:54 ` Eric Blake
  2020-05-18 20:54 ` [PULL 3/7] qemu-img: Fix stale comments on doc location Eric Blake
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Eric Blake @ 2020-05-18 20:54 UTC (permalink / raw)
  To: qemu-devel; +Cc: Vladimir Sementsov-Ogievskiy, Max Reitz

We already list the subcommand summaries alphabetically, we should do
the same for the documentation related to subcommand-specific
parameters.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20200513011648.166876-2-eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 docs/tools/qemu-img.rst | 48 ++++++++++++++++++++---------------------
 1 file changed, 24 insertions(+), 24 deletions(-)

diff --git a/docs/tools/qemu-img.rst b/docs/tools/qemu-img.rst
index 0080f83a76c9..7d08c48d308f 100644
--- a/docs/tools/qemu-img.rst
+++ b/docs/tools/qemu-img.rst
@@ -142,30 +142,6 @@ by the used format or see the format descriptions below for details.
   the documentation of the emulator's ``-drive cache=...`` option for allowed
   values.

-Parameters to snapshot subcommand:
-
-.. program:: qemu-img-snapshot
-
-.. option:: snapshot
-
-  Is the name of the snapshot to create, apply or delete
-
-.. option:: -a
-
-  Applies a snapshot (revert disk to saved state)
-
-.. option:: -c
-
-  Creates a snapshot
-
-.. option:: -d
-
-  Deletes a snapshot
-
-.. option:: -l
-
-  Lists all snapshots in the given image
-
 Parameters to compare subcommand:

 .. program:: qemu-img-compare
@@ -245,6 +221,30 @@ Parameters to dd subcommand:

   Sets the number of input blocks to skip

+Parameters to snapshot subcommand:
+
+.. program:: qemu-img-snapshot
+
+.. option:: snapshot
+
+  Is the name of the snapshot to create, apply or delete
+
+.. option:: -a
+
+  Applies a snapshot (revert disk to saved state)
+
+.. option:: -c
+
+  Creates a snapshot
+
+.. option:: -d
+
+  Deletes a snapshot
+
+.. option:: -l
+
+  Lists all snapshots in the given image
+
 Command description:

 .. program:: qemu-img-commands
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PULL 3/7] qemu-img: Fix stale comments on doc location
  2020-05-18 20:54 [PULL 0/7] bitmaps patches for 2020-05-18 Eric Blake
  2020-05-18 20:54 ` [PULL 1/7] bitmaps: Update maintainer Eric Blake
  2020-05-18 20:54 ` [PULL 2/7] docs: Sort sections on qemu-img subcommand parameters Eric Blake
@ 2020-05-18 20:54 ` Eric Blake
  2020-05-18 20:54 ` [PULL 4/7] block: Make it easier to learn which BDS support bitmaps Eric Blake
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Eric Blake @ 2020-05-18 20:54 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, Vladimir Sementsov-Ogievskiy,
	open list:Block layer core, Max Reitz

Missed in commit e13c59fa.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20200513011648.166876-3-eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 qemu-img.c       | 2 +-
 qemu-img-cmds.hx | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index 6a4327aaba56..b6e8af9202a5 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -108,7 +108,7 @@ static void QEMU_NORETURN unrecognized_option(const char *option)
     error_exit("unrecognized option '%s'", option);
 }

-/* Please keep in synch with qemu-img.texi */
+/* Please keep in synch with docs/tools/qemu-img.rst */
 static void QEMU_NORETURN help(void)
 {
     const char *help_msg =
diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
index c9c54de1df40..e0886437b1f2 100644
--- a/qemu-img-cmds.hx
+++ b/qemu-img-cmds.hx
@@ -7,7 +7,7 @@ HXCOMM command structures and help message.
 HXCOMM HXCOMM can be used for comments, discarded from both rST and C

 HXCOMM When amending the rST sections, please remember to copy the usage
-HXCOMM over to the per-command sections in qemu-img.texi.
+HXCOMM over to the per-command sections in docs/tools/qemu-img.rst.

 DEF("amend", img_amend,
     "amend [--object objectdef] [--image-opts] [-p] [-q] [-f fmt] [-t cache] -o options filename")
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PULL 4/7] block: Make it easier to learn which BDS support bitmaps
  2020-05-18 20:54 [PULL 0/7] bitmaps patches for 2020-05-18 Eric Blake
                   ` (2 preceding siblings ...)
  2020-05-18 20:54 ` [PULL 3/7] qemu-img: Fix stale comments on doc location Eric Blake
@ 2020-05-18 20:54 ` Eric Blake
  2020-05-18 20:54 ` [PULL 5/7] blockdev: Promote several bitmap functions to non-static Eric Blake
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Eric Blake @ 2020-05-18 20:54 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, Vladimir Sementsov-Ogievskiy, John Snow,
	open list:Dirty Bitmaps, Max Reitz

Upcoming patches will enhance bitmap support in qemu-img, but in doing
so, it turns out to be nice to suppress output when persistent bitmaps
make no sense (such as on a qcow2 v2 image).  Add a hook to make this
easier to query.

This patch adds a new callback .bdrv_supports_persistent_dirty_bitmap,
rather than trying to shoehorn the answer in via existing callbacks.
In particular, while it might have been possible to overload
.bdrv_co_can_store_new_dirty_bitmap to special-case a NULL input to
answer whether any persistent bitmaps are supported, that is at odds
with whether a particular bitmap can be stored (for example, even on
an image that supports persistent bitmaps but has currently filled up
the maximum number of bitmaps, attempts to store another one should
fail); and the new functionality doesn't require coroutine safety.
Similarly, we could have added one more piece of information to
.bdrv_get_info, but then again, most callers to that function tend to
already discard extraneous information, and making it a catch-all
rather than a series of dedicated scalar queries hasn't really
simplified life.

In the future, when we improve the ability to look up bitmaps through
a filter, we will probably also want to teach the block layer to
automatically let filters pass this request on through.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200513011648.166876-4-eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 block/qcow2.h                | 1 +
 include/block/block_int.h    | 1 +
 include/block/dirty-bitmap.h | 1 +
 block/dirty-bitmap.c         | 9 +++++++++
 block/qcow2-bitmap.c         | 7 +++++++
 block/qcow2.c                | 2 ++
 6 files changed, 21 insertions(+)

diff --git a/block/qcow2.h b/block/qcow2.h
index 6a8b82e6ccb1..402e8acb1cb7 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -782,6 +782,7 @@ bool qcow2_co_can_store_new_dirty_bitmap(BlockDriverState *bs,
 int qcow2_co_remove_persistent_dirty_bitmap(BlockDriverState *bs,
                                             const char *name,
                                             Error **errp);
+bool qcow2_supports_persistent_dirty_bitmap(BlockDriverState *bs);

 ssize_t coroutine_fn
 qcow2_co_compress(BlockDriverState *bs, void *dest, size_t dest_size,
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 7ba8c8903609..786bd2d092ea 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -561,6 +561,7 @@ struct BlockDriver {
                              uint64_t parent_perm, uint64_t parent_shared,
                              uint64_t *nperm, uint64_t *nshared);

+    bool (*bdrv_supports_persistent_dirty_bitmap)(BlockDriverState *bs);
     bool (*bdrv_co_can_store_new_dirty_bitmap)(BlockDriverState *bs,
                                                const char *name,
                                                uint32_t granularity,
diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
index 8a1002941892..5a8d52e4deaf 100644
--- a/include/block/dirty-bitmap.h
+++ b/include/block/dirty-bitmap.h
@@ -16,6 +16,7 @@ typedef enum BitmapCheckFlags {

 #define BDRV_BITMAP_MAX_NAME_SIZE 1023

+bool bdrv_supports_persistent_dirty_bitmap(BlockDriverState *bs);
 BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
                                           uint32_t granularity,
                                           const char *name,
diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index 063793e31606..f9bfc77985e8 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -478,6 +478,15 @@ int bdrv_remove_persistent_dirty_bitmap(BlockDriverState *bs, const char *name,
     }
 }

+bool
+bdrv_supports_persistent_dirty_bitmap(BlockDriverState *bs)
+{
+    if (bs->drv && bs->drv->bdrv_supports_persistent_dirty_bitmap) {
+        return bs->drv->bdrv_supports_persistent_dirty_bitmap(bs);
+    }
+    return false;
+}
+
 static bool coroutine_fn
 bdrv_co_can_store_new_dirty_bitmap(BlockDriverState *bs, const char *name,
                                    uint32_t granularity, Error **errp)
diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
index cb06954b4a5a..1cf6d2ab77a3 100644
--- a/block/qcow2-bitmap.c
+++ b/block/qcow2-bitmap.c
@@ -1748,3 +1748,10 @@ fail:
                   name, bdrv_get_device_or_node_name(bs));
     return false;
 }
+
+bool qcow2_supports_persistent_dirty_bitmap(BlockDriverState *bs)
+{
+    BDRVQcow2State *s = bs->opaque;
+
+    return s->qcow_version >= 3;
+}
diff --git a/block/qcow2.c b/block/qcow2.c
index ad9ab4fafa00..6406027a77eb 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -5783,6 +5783,8 @@ BlockDriver bdrv_qcow2 = {
     .bdrv_detach_aio_context  = qcow2_detach_aio_context,
     .bdrv_attach_aio_context  = qcow2_attach_aio_context,

+    .bdrv_supports_persistent_dirty_bitmap =
+            qcow2_supports_persistent_dirty_bitmap,
     .bdrv_co_can_store_new_dirty_bitmap = qcow2_co_can_store_new_dirty_bitmap,
     .bdrv_co_remove_persistent_dirty_bitmap =
             qcow2_co_remove_persistent_dirty_bitmap,
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PULL 5/7] blockdev: Promote several bitmap functions to non-static
  2020-05-18 20:54 [PULL 0/7] bitmaps patches for 2020-05-18 Eric Blake
                   ` (3 preceding siblings ...)
  2020-05-18 20:54 ` [PULL 4/7] block: Make it easier to learn which BDS support bitmaps Eric Blake
@ 2020-05-18 20:54 ` Eric Blake
  2020-05-18 20:54 ` [PULL 6/7] blockdev: Split off basic bitmap operations for qemu-img Eric Blake
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Eric Blake @ 2020-05-18 20:54 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, Vladimir Sementsov-Ogievskiy, Markus Armbruster,
	open list:Block layer core, Max Reitz

The next patch will split blockdev.c, which will require accessing
some previously-static functions from more than one .c file.  But part
of promoting a function to public is picking a naming scheme that does
not reek of exposing too many internals (two of the three functions
were named starting with 'do_').  To make future code motion easier,
perform the function rename and non-static promotion into its own
patch.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20200513011648.166876-5-eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 include/block/block_int.h | 12 ++++++++++
 blockdev.c                | 47 ++++++++++++++++-----------------------
 2 files changed, 31 insertions(+), 28 deletions(-)

diff --git a/include/block/block_int.h b/include/block/block_int.h
index 786bd2d092ea..4810e6fa5f2c 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -1345,4 +1345,16 @@ int coroutine_fn bdrv_co_create_opts_simple(BlockDriver *drv,
                                             Error **errp);
 extern QemuOptsList bdrv_create_opts_simple;

+BdrvDirtyBitmap *block_dirty_bitmap_lookup(const char *node,
+                                           const char *name,
+                                           BlockDriverState **pbs,
+                                           Error **errp);
+BdrvDirtyBitmap *block_dirty_bitmap_merge(const char *node, const char *target,
+                                          BlockDirtyBitmapMergeSourceList *bms,
+                                          HBitmap **backup, Error **errp);
+BdrvDirtyBitmap *block_dirty_bitmap_remove(const char *node, const char *name,
+                                           bool release,
+                                           BlockDriverState **bitmap_bs,
+                                           Error **errp);
+
 #endif /* BLOCK_INT_H */
diff --git a/blockdev.c b/blockdev.c
index b3c840ec0312..69a30613a3dd 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1197,10 +1197,10 @@ out_aio_context:
  *
  * @return: A bitmap object on success, or NULL on failure.
  */
-static BdrvDirtyBitmap *block_dirty_bitmap_lookup(const char *node,
-                                                  const char *name,
-                                                  BlockDriverState **pbs,
-                                                  Error **errp)
+BdrvDirtyBitmap *block_dirty_bitmap_lookup(const char *node,
+                                           const char *name,
+                                           BlockDriverState **pbs,
+                                           Error **errp)
 {
     BlockDriverState *bs;
     BdrvDirtyBitmap *bitmap;
@@ -2171,11 +2171,6 @@ static void block_dirty_bitmap_disable_abort(BlkActionState *common)
     }
 }

-static BdrvDirtyBitmap *do_block_dirty_bitmap_merge(
-        const char *node, const char *target,
-        BlockDirtyBitmapMergeSourceList *bitmaps,
-        HBitmap **backup, Error **errp);
-
 static void block_dirty_bitmap_merge_prepare(BlkActionState *common,
                                              Error **errp)
 {
@@ -2189,15 +2184,11 @@ static void block_dirty_bitmap_merge_prepare(BlkActionState *common,

     action = common->action->u.block_dirty_bitmap_merge.data;

-    state->bitmap = do_block_dirty_bitmap_merge(action->node, action->target,
-                                                action->bitmaps, &state->backup,
-                                                errp);
+    state->bitmap = block_dirty_bitmap_merge(action->node, action->target,
+                                             action->bitmaps, &state->backup,
+                                             errp);
 }

-static BdrvDirtyBitmap *do_block_dirty_bitmap_remove(
-        const char *node, const char *name, bool release,
-        BlockDriverState **bitmap_bs, Error **errp);
-
 static void block_dirty_bitmap_remove_prepare(BlkActionState *common,
                                               Error **errp)
 {
@@ -2211,8 +2202,8 @@ static void block_dirty_bitmap_remove_prepare(BlkActionState *common,

     action = common->action->u.block_dirty_bitmap_remove.data;

-    state->bitmap = do_block_dirty_bitmap_remove(action->node, action->name,
-                                                 false, &state->bs, errp);
+    state->bitmap = block_dirty_bitmap_remove(action->node, action->name,
+                                              false, &state->bs, errp);
     if (state->bitmap) {
         bdrv_dirty_bitmap_skip_store(state->bitmap, true);
         bdrv_dirty_bitmap_set_busy(state->bitmap, true);
@@ -2504,9 +2495,10 @@ out:
     aio_context_release(aio_context);
 }

-static BdrvDirtyBitmap *do_block_dirty_bitmap_remove(
-        const char *node, const char *name, bool release,
-        BlockDriverState **bitmap_bs, Error **errp)
+BdrvDirtyBitmap *block_dirty_bitmap_remove(const char *node, const char *name,
+                                           bool release,
+                                           BlockDriverState **bitmap_bs,
+                                           Error **errp)
 {
     BlockDriverState *bs;
     BdrvDirtyBitmap *bitmap;
@@ -2548,7 +2540,7 @@ static BdrvDirtyBitmap *do_block_dirty_bitmap_remove(
 void qmp_block_dirty_bitmap_remove(const char *node, const char *name,
                                    Error **errp)
 {
-    do_block_dirty_bitmap_remove(node, name, true, NULL, errp);
+    block_dirty_bitmap_remove(node, name, true, NULL, errp);
 }

 /**
@@ -2609,10 +2601,9 @@ void qmp_block_dirty_bitmap_disable(const char *node, const char *name,
     bdrv_disable_dirty_bitmap(bitmap);
 }

-static BdrvDirtyBitmap *do_block_dirty_bitmap_merge(
-        const char *node, const char *target,
-        BlockDirtyBitmapMergeSourceList *bitmaps,
-        HBitmap **backup, Error **errp)
+BdrvDirtyBitmap *block_dirty_bitmap_merge(const char *node, const char *target,
+                                          BlockDirtyBitmapMergeSourceList *bms,
+                                          HBitmap **backup, Error **errp)
 {
     BlockDriverState *bs;
     BdrvDirtyBitmap *dst, *src, *anon;
@@ -2630,7 +2621,7 @@ static BdrvDirtyBitmap *do_block_dirty_bitmap_merge(
         return NULL;
     }

-    for (lst = bitmaps; lst; lst = lst->next) {
+    for (lst = bms; lst; lst = lst->next) {
         switch (lst->value->type) {
             const char *name, *node;
         case QTYPE_QSTRING:
@@ -2675,7 +2666,7 @@ void qmp_block_dirty_bitmap_merge(const char *node, const char *target,
                                   BlockDirtyBitmapMergeSourceList *bitmaps,
                                   Error **errp)
 {
-    do_block_dirty_bitmap_merge(node, target, bitmaps, NULL, errp);
+    block_dirty_bitmap_merge(node, target, bitmaps, NULL, errp);
 }

 BlockDirtyBitmapSha256 *qmp_x_debug_block_dirty_bitmap_sha256(const char *node,
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PULL 6/7] blockdev: Split off basic bitmap operations for qemu-img
  2020-05-18 20:54 [PULL 0/7] bitmaps patches for 2020-05-18 Eric Blake
                   ` (4 preceding siblings ...)
  2020-05-18 20:54 ` [PULL 5/7] blockdev: Promote several bitmap functions to non-static Eric Blake
@ 2020-05-18 20:54 ` Eric Blake
  2020-05-18 20:54 ` [PULL 7/7] qemu-img: Add bitmap sub-command Eric Blake
  2020-05-19 12:42 ` [PULL 0/7] bitmaps patches for 2020-05-18 Peter Maydell
  7 siblings, 0 replies; 10+ messages in thread
From: Eric Blake @ 2020-05-18 20:54 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, Vladimir Sementsov-Ogievskiy,
	open list:Block layer core, Markus Armbruster, Max Reitz,
	John Snow

Upcoming patches want to add some basic bitmap manipulation abilities
to qemu-img.  But blockdev.o is too heavyweight to link into qemu-img
(among other things, it would drag in block jobs and transaction
support - qemu-img does offline manipulation, where atomicity is less
important because there are no concurrent modifications to compete
with), so it's time to split off the bare bones of what we will need
into a new file block/monitor/bitmap-qmp-cmds.o.

This is sufficient to expose 6 QMP commands for use by qemu-img (add,
remove, clear, enable, disable, merge), as well as move the three
helper functions touched in the previous patch.  Regarding
MAINTAINERS, the new file is automatically part of block core, but
also makes sense as related to other dirty bitmap files.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20200513011648.166876-6-eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 Makefile.objs                   |   3 +-
 block/monitor/bitmap-qmp-cmds.c | 321 ++++++++++++++++++++++++++++++++
 blockdev.c                      | 284 ----------------------------
 MAINTAINERS                     |   1 +
 block/monitor/Makefile.objs     |   1 +
 5 files changed, 324 insertions(+), 286 deletions(-)
 create mode 100644 block/monitor/bitmap-qmp-cmds.c

diff --git a/Makefile.objs b/Makefile.objs
index a7c967633acf..99774cfd2545 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -13,9 +13,8 @@ chardev-obj-y = chardev/

 authz-obj-y = authz/

-block-obj-y = nbd/
+block-obj-y = block/ block/monitor/ nbd/ scsi/
 block-obj-y += block.o blockjob.o job.o
-block-obj-y += block/ scsi/
 block-obj-y += qemu-io-cmds.o
 block-obj-$(CONFIG_REPLICATION) += replication.o

diff --git a/block/monitor/bitmap-qmp-cmds.c b/block/monitor/bitmap-qmp-cmds.c
new file mode 100644
index 000000000000..9f11deec6467
--- /dev/null
+++ b/block/monitor/bitmap-qmp-cmds.c
@@ -0,0 +1,321 @@
+/*
+ * QEMU block dirty bitmap QMP commands
+ *
+ * Copyright (c) 2003-2008 Fabrice Bellard
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * later.  See the COPYING file in the top-level directory.
+ *
+ * This file incorporates work covered by the following copyright and
+ * permission notice:
+ *
+ * Copyright (c) 2003-2008 Fabrice Bellard
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu/osdep.h"
+
+#include "block/block_int.h"
+#include "qapi/qapi-commands-block.h"
+#include "qapi/error.h"
+
+/**
+ * block_dirty_bitmap_lookup:
+ * Return a dirty bitmap (if present), after validating
+ * the node reference and bitmap names.
+ *
+ * @node: The name of the BDS node to search for bitmaps
+ * @name: The name of the bitmap to search for
+ * @pbs: Output pointer for BDS lookup, if desired. Can be NULL.
+ * @errp: Output pointer for error information. Can be NULL.
+ *
+ * @return: A bitmap object on success, or NULL on failure.
+ */
+BdrvDirtyBitmap *block_dirty_bitmap_lookup(const char *node,
+                                           const char *name,
+                                           BlockDriverState **pbs,
+                                           Error **errp)
+{
+    BlockDriverState *bs;
+    BdrvDirtyBitmap *bitmap;
+
+    if (!node) {
+        error_setg(errp, "Node cannot be NULL");
+        return NULL;
+    }
+    if (!name) {
+        error_setg(errp, "Bitmap name cannot be NULL");
+        return NULL;
+    }
+    bs = bdrv_lookup_bs(node, node, NULL);
+    if (!bs) {
+        error_setg(errp, "Node '%s' not found", node);
+        return NULL;
+    }
+
+    bitmap = bdrv_find_dirty_bitmap(bs, name);
+    if (!bitmap) {
+        error_setg(errp, "Dirty bitmap '%s' not found", name);
+        return NULL;
+    }
+
+    if (pbs) {
+        *pbs = bs;
+    }
+
+    return bitmap;
+}
+
+void qmp_block_dirty_bitmap_add(const char *node, const char *name,
+                                bool has_granularity, uint32_t granularity,
+                                bool has_persistent, bool persistent,
+                                bool has_disabled, bool disabled,
+                                Error **errp)
+{
+    BlockDriverState *bs;
+    BdrvDirtyBitmap *bitmap;
+    AioContext *aio_context;
+
+    if (!name || name[0] == '\0') {
+        error_setg(errp, "Bitmap name cannot be empty");
+        return;
+    }
+
+    bs = bdrv_lookup_bs(node, node, errp);
+    if (!bs) {
+        return;
+    }
+
+    aio_context = bdrv_get_aio_context(bs);
+    aio_context_acquire(aio_context);
+
+    if (has_granularity) {
+        if (granularity < 512 || !is_power_of_2(granularity)) {
+            error_setg(errp, "Granularity must be power of 2 "
+                             "and at least 512");
+            goto out;
+        }
+    } else {
+        /* Default to cluster size, if available: */
+        granularity = bdrv_get_default_bitmap_granularity(bs);
+    }
+
+    if (!has_persistent) {
+        persistent = false;
+    }
+
+    if (!has_disabled) {
+        disabled = false;
+    }
+
+    if (persistent &&
+        !bdrv_can_store_new_dirty_bitmap(bs, name, granularity, errp))
+    {
+        goto out;
+    }
+
+    bitmap = bdrv_create_dirty_bitmap(bs, granularity, name, errp);
+    if (bitmap == NULL) {
+        goto out;
+    }
+
+    if (disabled) {
+        bdrv_disable_dirty_bitmap(bitmap);
+    }
+
+    bdrv_dirty_bitmap_set_persistence(bitmap, persistent);
+
+out:
+    aio_context_release(aio_context);
+}
+
+BdrvDirtyBitmap *block_dirty_bitmap_remove(const char *node, const char *name,
+                                           bool release,
+                                           BlockDriverState **bitmap_bs,
+                                           Error **errp)
+{
+    BlockDriverState *bs;
+    BdrvDirtyBitmap *bitmap;
+    AioContext *aio_context;
+
+    bitmap = block_dirty_bitmap_lookup(node, name, &bs, errp);
+    if (!bitmap || !bs) {
+        return NULL;
+    }
+
+    aio_context = bdrv_get_aio_context(bs);
+    aio_context_acquire(aio_context);
+
+    if (bdrv_dirty_bitmap_check(bitmap, BDRV_BITMAP_BUSY | BDRV_BITMAP_RO,
+                                errp)) {
+        aio_context_release(aio_context);
+        return NULL;
+    }
+
+    if (bdrv_dirty_bitmap_get_persistence(bitmap) &&
+        bdrv_remove_persistent_dirty_bitmap(bs, name, errp) < 0)
+    {
+        aio_context_release(aio_context);
+        return NULL;
+    }
+
+    if (release) {
+        bdrv_release_dirty_bitmap(bitmap);
+    }
+
+    if (bitmap_bs) {
+        *bitmap_bs = bs;
+    }
+
+    aio_context_release(aio_context);
+    return release ? NULL : bitmap;
+}
+
+void qmp_block_dirty_bitmap_remove(const char *node, const char *name,
+                                   Error **errp)
+{
+    block_dirty_bitmap_remove(node, name, true, NULL, errp);
+}
+
+/**
+ * Completely clear a bitmap, for the purposes of synchronizing a bitmap
+ * immediately after a full backup operation.
+ */
+void qmp_block_dirty_bitmap_clear(const char *node, const char *name,
+                                  Error **errp)
+{
+    BdrvDirtyBitmap *bitmap;
+    BlockDriverState *bs;
+
+    bitmap = block_dirty_bitmap_lookup(node, name, &bs, errp);
+    if (!bitmap || !bs) {
+        return;
+    }
+
+    if (bdrv_dirty_bitmap_check(bitmap, BDRV_BITMAP_DEFAULT, errp)) {
+        return;
+    }
+
+    bdrv_clear_dirty_bitmap(bitmap, NULL);
+}
+
+void qmp_block_dirty_bitmap_enable(const char *node, const char *name,
+                                   Error **errp)
+{
+    BlockDriverState *bs;
+    BdrvDirtyBitmap *bitmap;
+
+    bitmap = block_dirty_bitmap_lookup(node, name, &bs, errp);
+    if (!bitmap) {
+        return;
+    }
+
+    if (bdrv_dirty_bitmap_check(bitmap, BDRV_BITMAP_ALLOW_RO, errp)) {
+        return;
+    }
+
+    bdrv_enable_dirty_bitmap(bitmap);
+}
+
+void qmp_block_dirty_bitmap_disable(const char *node, const char *name,
+                                    Error **errp)
+{
+    BlockDriverState *bs;
+    BdrvDirtyBitmap *bitmap;
+
+    bitmap = block_dirty_bitmap_lookup(node, name, &bs, errp);
+    if (!bitmap) {
+        return;
+    }
+
+    if (bdrv_dirty_bitmap_check(bitmap, BDRV_BITMAP_ALLOW_RO, errp)) {
+        return;
+    }
+
+    bdrv_disable_dirty_bitmap(bitmap);
+}
+
+BdrvDirtyBitmap *block_dirty_bitmap_merge(const char *node, const char *target,
+                                          BlockDirtyBitmapMergeSourceList *bms,
+                                          HBitmap **backup, Error **errp)
+{
+    BlockDriverState *bs;
+    BdrvDirtyBitmap *dst, *src, *anon;
+    BlockDirtyBitmapMergeSourceList *lst;
+    Error *local_err = NULL;
+
+    dst = block_dirty_bitmap_lookup(node, target, &bs, errp);
+    if (!dst) {
+        return NULL;
+    }
+
+    anon = bdrv_create_dirty_bitmap(bs, bdrv_dirty_bitmap_granularity(dst),
+                                    NULL, errp);
+    if (!anon) {
+        return NULL;
+    }
+
+    for (lst = bms; lst; lst = lst->next) {
+        switch (lst->value->type) {
+            const char *name, *node;
+        case QTYPE_QSTRING:
+            name = lst->value->u.local;
+            src = bdrv_find_dirty_bitmap(bs, name);
+            if (!src) {
+                error_setg(errp, "Dirty bitmap '%s' not found", name);
+                dst = NULL;
+                goto out;
+            }
+            break;
+        case QTYPE_QDICT:
+            node = lst->value->u.external.node;
+            name = lst->value->u.external.name;
+            src = block_dirty_bitmap_lookup(node, name, NULL, errp);
+            if (!src) {
+                dst = NULL;
+                goto out;
+            }
+            break;
+        default:
+            abort();
+        }
+
+        bdrv_merge_dirty_bitmap(anon, src, NULL, &local_err);
+        if (local_err) {
+            error_propagate(errp, local_err);
+            dst = NULL;
+            goto out;
+        }
+    }
+
+    /* Merge into dst; dst is unchanged on failure. */
+    bdrv_merge_dirty_bitmap(dst, anon, backup, errp);
+
+ out:
+    bdrv_release_dirty_bitmap(anon);
+    return dst;
+}
+
+void qmp_block_dirty_bitmap_merge(const char *node, const char *target,
+                                  BlockDirtyBitmapMergeSourceList *bitmaps,
+                                  Error **errp)
+{
+    block_dirty_bitmap_merge(node, target, bitmaps, NULL, errp);
+}
diff --git a/blockdev.c b/blockdev.c
index 69a30613a3dd..72df193ca73b 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1185,53 +1185,6 @@ out_aio_context:
     return NULL;
 }

-/**
- * block_dirty_bitmap_lookup:
- * Return a dirty bitmap (if present), after validating
- * the node reference and bitmap names.
- *
- * @node: The name of the BDS node to search for bitmaps
- * @name: The name of the bitmap to search for
- * @pbs: Output pointer for BDS lookup, if desired. Can be NULL.
- * @errp: Output pointer for error information. Can be NULL.
- *
- * @return: A bitmap object on success, or NULL on failure.
- */
-BdrvDirtyBitmap *block_dirty_bitmap_lookup(const char *node,
-                                           const char *name,
-                                           BlockDriverState **pbs,
-                                           Error **errp)
-{
-    BlockDriverState *bs;
-    BdrvDirtyBitmap *bitmap;
-
-    if (!node) {
-        error_setg(errp, "Node cannot be NULL");
-        return NULL;
-    }
-    if (!name) {
-        error_setg(errp, "Bitmap name cannot be NULL");
-        return NULL;
-    }
-    bs = bdrv_lookup_bs(node, node, NULL);
-    if (!bs) {
-        error_setg(errp, "Node '%s' not found", node);
-        return NULL;
-    }
-
-    bitmap = bdrv_find_dirty_bitmap(bs, name);
-    if (!bitmap) {
-        error_setg(errp, "Dirty bitmap '%s' not found", name);
-        return NULL;
-    }
-
-    if (pbs) {
-        *pbs = bs;
-    }
-
-    return bitmap;
-}
-
 /* New and old BlockDriverState structs for atomic group operations */

 typedef struct BlkActionState BlkActionState;
@@ -2432,243 +2385,6 @@ void qmp_block_passwd(bool has_device, const char *device,
                "Setting block passwords directly is no longer supported");
 }

-void qmp_block_dirty_bitmap_add(const char *node, const char *name,
-                                bool has_granularity, uint32_t granularity,
-                                bool has_persistent, bool persistent,
-                                bool has_disabled, bool disabled,
-                                Error **errp)
-{
-    BlockDriverState *bs;
-    BdrvDirtyBitmap *bitmap;
-    AioContext *aio_context;
-
-    if (!name || name[0] == '\0') {
-        error_setg(errp, "Bitmap name cannot be empty");
-        return;
-    }
-
-    bs = bdrv_lookup_bs(node, node, errp);
-    if (!bs) {
-        return;
-    }
-
-    aio_context = bdrv_get_aio_context(bs);
-    aio_context_acquire(aio_context);
-
-    if (has_granularity) {
-        if (granularity < 512 || !is_power_of_2(granularity)) {
-            error_setg(errp, "Granularity must be power of 2 "
-                             "and at least 512");
-            goto out;
-        }
-    } else {
-        /* Default to cluster size, if available: */
-        granularity = bdrv_get_default_bitmap_granularity(bs);
-    }
-
-    if (!has_persistent) {
-        persistent = false;
-    }
-
-    if (!has_disabled) {
-        disabled = false;
-    }
-
-    if (persistent &&
-        !bdrv_can_store_new_dirty_bitmap(bs, name, granularity, errp))
-    {
-        goto out;
-    }
-
-    bitmap = bdrv_create_dirty_bitmap(bs, granularity, name, errp);
-    if (bitmap == NULL) {
-        goto out;
-    }
-
-    if (disabled) {
-        bdrv_disable_dirty_bitmap(bitmap);
-    }
-
-    bdrv_dirty_bitmap_set_persistence(bitmap, persistent);
-
-out:
-    aio_context_release(aio_context);
-}
-
-BdrvDirtyBitmap *block_dirty_bitmap_remove(const char *node, const char *name,
-                                           bool release,
-                                           BlockDriverState **bitmap_bs,
-                                           Error **errp)
-{
-    BlockDriverState *bs;
-    BdrvDirtyBitmap *bitmap;
-    AioContext *aio_context;
-
-    bitmap = block_dirty_bitmap_lookup(node, name, &bs, errp);
-    if (!bitmap || !bs) {
-        return NULL;
-    }
-
-    aio_context = bdrv_get_aio_context(bs);
-    aio_context_acquire(aio_context);
-
-    if (bdrv_dirty_bitmap_check(bitmap, BDRV_BITMAP_BUSY | BDRV_BITMAP_RO,
-                                errp)) {
-        aio_context_release(aio_context);
-        return NULL;
-    }
-
-    if (bdrv_dirty_bitmap_get_persistence(bitmap) &&
-        bdrv_remove_persistent_dirty_bitmap(bs, name, errp) < 0)
-    {
-        aio_context_release(aio_context);
-        return NULL;
-    }
-
-    if (release) {
-        bdrv_release_dirty_bitmap(bitmap);
-    }
-
-    if (bitmap_bs) {
-        *bitmap_bs = bs;
-    }
-
-    aio_context_release(aio_context);
-    return release ? NULL : bitmap;
-}
-
-void qmp_block_dirty_bitmap_remove(const char *node, const char *name,
-                                   Error **errp)
-{
-    block_dirty_bitmap_remove(node, name, true, NULL, errp);
-}
-
-/**
- * Completely clear a bitmap, for the purposes of synchronizing a bitmap
- * immediately after a full backup operation.
- */
-void qmp_block_dirty_bitmap_clear(const char *node, const char *name,
-                                  Error **errp)
-{
-    BdrvDirtyBitmap *bitmap;
-    BlockDriverState *bs;
-
-    bitmap = block_dirty_bitmap_lookup(node, name, &bs, errp);
-    if (!bitmap || !bs) {
-        return;
-    }
-
-    if (bdrv_dirty_bitmap_check(bitmap, BDRV_BITMAP_DEFAULT, errp)) {
-        return;
-    }
-
-    bdrv_clear_dirty_bitmap(bitmap, NULL);
-}
-
-void qmp_block_dirty_bitmap_enable(const char *node, const char *name,
-                                   Error **errp)
-{
-    BlockDriverState *bs;
-    BdrvDirtyBitmap *bitmap;
-
-    bitmap = block_dirty_bitmap_lookup(node, name, &bs, errp);
-    if (!bitmap) {
-        return;
-    }
-
-    if (bdrv_dirty_bitmap_check(bitmap, BDRV_BITMAP_ALLOW_RO, errp)) {
-        return;
-    }
-
-    bdrv_enable_dirty_bitmap(bitmap);
-}
-
-void qmp_block_dirty_bitmap_disable(const char *node, const char *name,
-                                    Error **errp)
-{
-    BlockDriverState *bs;
-    BdrvDirtyBitmap *bitmap;
-
-    bitmap = block_dirty_bitmap_lookup(node, name, &bs, errp);
-    if (!bitmap) {
-        return;
-    }
-
-    if (bdrv_dirty_bitmap_check(bitmap, BDRV_BITMAP_ALLOW_RO, errp)) {
-        return;
-    }
-
-    bdrv_disable_dirty_bitmap(bitmap);
-}
-
-BdrvDirtyBitmap *block_dirty_bitmap_merge(const char *node, const char *target,
-                                          BlockDirtyBitmapMergeSourceList *bms,
-                                          HBitmap **backup, Error **errp)
-{
-    BlockDriverState *bs;
-    BdrvDirtyBitmap *dst, *src, *anon;
-    BlockDirtyBitmapMergeSourceList *lst;
-    Error *local_err = NULL;
-
-    dst = block_dirty_bitmap_lookup(node, target, &bs, errp);
-    if (!dst) {
-        return NULL;
-    }
-
-    anon = bdrv_create_dirty_bitmap(bs, bdrv_dirty_bitmap_granularity(dst),
-                                    NULL, errp);
-    if (!anon) {
-        return NULL;
-    }
-
-    for (lst = bms; lst; lst = lst->next) {
-        switch (lst->value->type) {
-            const char *name, *node;
-        case QTYPE_QSTRING:
-            name = lst->value->u.local;
-            src = bdrv_find_dirty_bitmap(bs, name);
-            if (!src) {
-                error_setg(errp, "Dirty bitmap '%s' not found", name);
-                dst = NULL;
-                goto out;
-            }
-            break;
-        case QTYPE_QDICT:
-            node = lst->value->u.external.node;
-            name = lst->value->u.external.name;
-            src = block_dirty_bitmap_lookup(node, name, NULL, errp);
-            if (!src) {
-                dst = NULL;
-                goto out;
-            }
-            break;
-        default:
-            abort();
-        }
-
-        bdrv_merge_dirty_bitmap(anon, src, NULL, &local_err);
-        if (local_err) {
-            error_propagate(errp, local_err);
-            dst = NULL;
-            goto out;
-        }
-    }
-
-    /* Merge into dst; dst is unchanged on failure. */
-    bdrv_merge_dirty_bitmap(dst, anon, backup, errp);
-
- out:
-    bdrv_release_dirty_bitmap(anon);
-    return dst;
-}
-
-void qmp_block_dirty_bitmap_merge(const char *node, const char *target,
-                                  BlockDirtyBitmapMergeSourceList *bitmaps,
-                                  Error **errp)
-{
-    block_dirty_bitmap_merge(node, target, bitmaps, NULL, errp);
-}
-
 BlockDirtyBitmapSha256 *qmp_x_debug_block_dirty_bitmap_sha256(const char *node,
                                                               const char *name,
                                                               Error **errp)
diff --git a/MAINTAINERS b/MAINTAINERS
index 4e99bb05dad8..87a412c229b3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2018,6 +2018,7 @@ L: qemu-block@nongnu.org
 S: Supported
 F: include/qemu/hbitmap.h
 F: include/block/dirty-bitmap.h
+F: block/monitor/bitmap-qmp-cmds.c
 F: block/dirty-bitmap.c
 F: block/qcow2-bitmap.c
 F: migration/block-dirty-bitmap.c
diff --git a/block/monitor/Makefile.objs b/block/monitor/Makefile.objs
index 0a74f9a8b5b7..39acf8502224 100644
--- a/block/monitor/Makefile.objs
+++ b/block/monitor/Makefile.objs
@@ -1 +1,2 @@
 common-obj-y += block-hmp-cmds.o
+block-obj-y += bitmap-qmp-cmds.o
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PULL 7/7] qemu-img: Add bitmap sub-command
  2020-05-18 20:54 [PULL 0/7] bitmaps patches for 2020-05-18 Eric Blake
                   ` (5 preceding siblings ...)
  2020-05-18 20:54 ` [PULL 6/7] blockdev: Split off basic bitmap operations for qemu-img Eric Blake
@ 2020-05-18 20:54 ` Eric Blake
  2020-05-19 12:42 ` [PULL 0/7] bitmaps patches for 2020-05-18 Peter Maydell
  7 siblings, 0 replies; 10+ messages in thread
From: Eric Blake @ 2020-05-18 20:54 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, Vladimir Sementsov-Ogievskiy,
	open list:Block layer core, Max Reitz

Include actions for --add, --remove, --clear, --enable, --disable, and
--merge (note that --clear is a bit of fluff, because the same can be
accomplished by removing a bitmap and then adding a new one in its
place, but it matches what QMP commands exist).  Listing is omitted,
because it does not require a bitmap name and because it was already
possible with 'qemu-img info'.  A single command line can play one or
more bitmap commands in sequence on the same bitmap name (although all
added bitmaps share the same granularity, and and all merged bitmaps
come from the same source file).  Merge defaults to other bitmaps in
the primary image, but can also be told to merge bitmaps from a
distinct image.

While this supports --image-opts for the file being modified, I did
not think it worth the extra complexity to support that for the source
file in a cross-file merges.  Likewise, I chose to have --merge only
take a single source rather than following the QMP support for
multiple merges in one go (although you can still use more than one
--merge in the command line); in part because qemu-img is offline and
therefore atomicity is not an issue.

Upcoming patches will add iotest coverage of these commands while
also testing other features.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20200513011648.166876-7-eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 docs/tools/qemu-img.rst |  24 ++++
 qemu-img.c              | 249 ++++++++++++++++++++++++++++++++++++++++
 qemu-img-cmds.hx        |   7 ++
 3 files changed, 280 insertions(+)

diff --git a/docs/tools/qemu-img.rst b/docs/tools/qemu-img.rst
index 7d08c48d308f..219483cec279 100644
--- a/docs/tools/qemu-img.rst
+++ b/docs/tools/qemu-img.rst
@@ -281,6 +281,30 @@ Command description:
   For write tests, by default a buffer filled with zeros is written. This can be
   overridden with a pattern byte specified by *PATTERN*.

+.. option:: bitmap (--merge SOURCE | --add | --remove | --clear | --enable | --disable)... [-b SOURCE_FILE [-F SOURCE_FMT]] [-g GRANULARITY] [--object OBJECTDEF] [--image-opts | -f FMT] FILENAME BITMAP
+
+  Perform one or more modifications of the persistent bitmap *BITMAP*
+  in the disk image *FILENAME*.  The various modifications are:
+
+  ``--add`` to create *BITMAP*, enabled to record future edits.
+
+  ``--remove`` to remove *BITMAP*.
+
+  ``--clear`` to clear *BITMAP*.
+
+  ``--enable`` to change *BITMAP* to start recording future edits.
+
+  ``--disable`` to change *BITMAP* to stop recording future edits.
+
+  ``--merge`` to merge the contents of *SOURCE_BITMAP* into *BITMAP*.
+
+  Additional options include ``-g`` which sets a non-default
+  *GRANULARITY* for ``--add``, and ``-b`` and ``-F`` which select an
+  alternative source file for all *SOURCE* bitmaps used by
+  ``--merge``.
+
+  To see what bitmaps are present in an image, use ``qemu-img info``.
+
 .. option:: check [--object OBJECTDEF] [--image-opts] [-q] [-f FMT] [--output=OFMT] [-r [leaks | all]] [-T SRC_CACHE] [-U] FILENAME

   Perform a consistency check on the disk image *FILENAME*. The command can
diff --git a/qemu-img.c b/qemu-img.c
index b6e8af9202a5..f940137cb0e5 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -28,6 +28,7 @@
 #include "qemu-common.h"
 #include "qemu-version.h"
 #include "qapi/error.h"
+#include "qapi/qapi-commands-block-core.h"
 #include "qapi/qapi-visit-block-core.h"
 #include "qapi/qobject-output-visitor.h"
 #include "qapi/qmp/qjson.h"
@@ -71,6 +72,12 @@ enum {
     OPTION_SHRINK = 266,
     OPTION_SALVAGE = 267,
     OPTION_TARGET_IS_ZERO = 268,
+    OPTION_ADD = 269,
+    OPTION_REMOVE = 270,
+    OPTION_CLEAR = 271,
+    OPTION_ENABLE = 272,
+    OPTION_DISABLE = 273,
+    OPTION_MERGE = 274,
 };

 typedef enum OutputFormat {
@@ -169,6 +176,14 @@ static void QEMU_NORETURN help(void)
            "  '-n' skips the target volume creation (useful if the volume is created\n"
            "       prior to running qemu-img)\n"
            "\n"
+           "Parameters to bitmap subcommand:\n"
+           "  'bitmap' is the name of the bitmap to manipulate, through one or more\n"
+           "       actions from '--add', '--remove', '--clear', '--enable', '--disable',\n"
+           "       or '--merge source'\n"
+           "  '-g granularity' sets the granularity for '--add' actions\n"
+           "  '-b source' and '-F src_fmt' tell '--merge' actions to find the source\n"
+           "       bitmaps from an alternative file\n"
+           "\n"
            "Parameters to check subcommand:\n"
            "  '-r' tries to repair any inconsistencies that are found during the check.\n"
            "       '-r leaks' repairs only cluster leaks, whereas '-r all' fixes all\n"
@@ -4461,6 +4476,240 @@ out:
     return 0;
 }

+enum ImgBitmapAct {
+    BITMAP_ADD,
+    BITMAP_REMOVE,
+    BITMAP_CLEAR,
+    BITMAP_ENABLE,
+    BITMAP_DISABLE,
+    BITMAP_MERGE,
+};
+typedef struct ImgBitmapAction {
+    enum ImgBitmapAct act;
+    const char *src; /* only used for merge */
+    QSIMPLEQ_ENTRY(ImgBitmapAction) next;
+} ImgBitmapAction;
+
+static int img_bitmap(int argc, char **argv)
+{
+    Error *err = NULL;
+    int c, ret = 1;
+    QemuOpts *opts = NULL;
+    const char *fmt = NULL, *src_fmt = NULL, *src_filename = NULL;
+    const char *filename, *bitmap;
+    BlockBackend *blk = NULL, *src = NULL;
+    BlockDriverState *bs = NULL, *src_bs = NULL;
+    bool image_opts = false;
+    int64_t granularity = 0;
+    bool add = false, merge = false;
+    QSIMPLEQ_HEAD(, ImgBitmapAction) actions;
+    ImgBitmapAction *act, *act_next;
+    const char *op;
+
+    QSIMPLEQ_INIT(&actions);
+
+    for (;;) {
+        static const struct option long_options[] = {
+            {"help", no_argument, 0, 'h'},
+            {"object", required_argument, 0, OPTION_OBJECT},
+            {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
+            {"add", no_argument, 0, OPTION_ADD},
+            {"remove", no_argument, 0, OPTION_REMOVE},
+            {"clear", no_argument, 0, OPTION_CLEAR},
+            {"enable", no_argument, 0, OPTION_ENABLE},
+            {"disable", no_argument, 0, OPTION_DISABLE},
+            {"merge", required_argument, 0, OPTION_MERGE},
+            {"granularity", required_argument, 0, 'g'},
+            {"source-file", required_argument, 0, 'b'},
+            {"source-format", required_argument, 0, 'F'},
+            {0, 0, 0, 0}
+        };
+        c = getopt_long(argc, argv, ":b:f:F:g:h", long_options, NULL);
+        if (c == -1) {
+            break;
+        }
+
+        switch (c) {
+        case ':':
+            missing_argument(argv[optind - 1]);
+            break;
+        case '?':
+            unrecognized_option(argv[optind - 1]);
+            break;
+        case 'h':
+            help();
+            break;
+        case 'b':
+            src_filename = optarg;
+            break;
+        case 'f':
+            fmt = optarg;
+            break;
+        case 'F':
+            src_fmt = optarg;
+            break;
+        case 'g':
+            granularity = cvtnum(optarg);
+            if (granularity < 0) {
+                error_report("Invalid granularity specified");
+                return 1;
+            }
+            break;
+        case OPTION_ADD:
+            act = g_new0(ImgBitmapAction, 1);
+            act->act = BITMAP_ADD;
+            QSIMPLEQ_INSERT_TAIL(&actions, act, next);
+            add = true;
+            break;
+        case OPTION_REMOVE:
+            act = g_new0(ImgBitmapAction, 1);
+            act->act = BITMAP_REMOVE;
+            QSIMPLEQ_INSERT_TAIL(&actions, act, next);
+            break;
+        case OPTION_CLEAR:
+            act = g_new0(ImgBitmapAction, 1);
+            act->act = BITMAP_CLEAR;
+            QSIMPLEQ_INSERT_TAIL(&actions, act, next);
+            break;
+        case OPTION_ENABLE:
+            act = g_new0(ImgBitmapAction, 1);
+            act->act = BITMAP_ENABLE;
+            QSIMPLEQ_INSERT_TAIL(&actions, act, next);
+            break;
+        case OPTION_DISABLE:
+            act = g_new0(ImgBitmapAction, 1);
+            act->act = BITMAP_DISABLE;
+            QSIMPLEQ_INSERT_TAIL(&actions, act, next);
+            break;
+        case OPTION_MERGE:
+            act = g_new0(ImgBitmapAction, 1);
+            act->act = BITMAP_MERGE;
+            act->src = optarg;
+            QSIMPLEQ_INSERT_TAIL(&actions, act, next);
+            merge = true;
+            break;
+        case OPTION_OBJECT:
+            opts = qemu_opts_parse_noisily(&qemu_object_opts, optarg, true);
+            if (!opts) {
+                goto out;
+            }
+            break;
+        case OPTION_IMAGE_OPTS:
+            image_opts = true;
+            break;
+        }
+    }
+
+    if (qemu_opts_foreach(&qemu_object_opts,
+                          user_creatable_add_opts_foreach,
+                          qemu_img_object_print_help, &error_fatal)) {
+        goto out;
+    }
+
+    if (QSIMPLEQ_EMPTY(&actions)) {
+        error_report("Need at least one of --add, --remove, --clear, "
+                     "--enable, --disable, or --merge");
+        goto out;
+    }
+
+    if (granularity && !add) {
+        error_report("granularity only supported with --add");
+        goto out;
+    }
+    if (src_fmt && !src_filename) {
+        error_report("-F only supported with -b");
+        goto out;
+    }
+    if (src_filename && !merge) {
+        error_report("Merge bitmap source file only supported with "
+                     "--merge");
+        goto out;
+    }
+
+    if (optind != argc - 2) {
+        error_report("Expecting filename and bitmap name");
+        goto out;
+    }
+
+    filename = argv[optind];
+    bitmap = argv[optind + 1];
+
+    blk = img_open(image_opts, filename, fmt, BDRV_O_RDWR, false, false,
+                   false);
+    if (!blk) {
+        goto out;
+    }
+    bs = blk_bs(blk);
+    if (src_filename) {
+        src = img_open(false, src_filename, src_fmt, 0, false, false, false);
+        if (!src) {
+            goto out;
+        }
+        src_bs = blk_bs(src);
+    } else {
+        src_bs = bs;
+    }
+
+    QSIMPLEQ_FOREACH_SAFE(act, &actions, next, act_next) {
+        switch (act->act) {
+        case BITMAP_ADD:
+            qmp_block_dirty_bitmap_add(bs->node_name, bitmap,
+                                       !!granularity, granularity, true, true,
+                                       false, false, &err);
+            op = "add";
+            break;
+        case BITMAP_REMOVE:
+            qmp_block_dirty_bitmap_remove(bs->node_name, bitmap, &err);
+            op = "remove";
+            break;
+        case BITMAP_CLEAR:
+            qmp_block_dirty_bitmap_clear(bs->node_name, bitmap, &err);
+            op = "clear";
+            break;
+        case BITMAP_ENABLE:
+            qmp_block_dirty_bitmap_enable(bs->node_name, bitmap, &err);
+            op = "enable";
+            break;
+        case BITMAP_DISABLE:
+            qmp_block_dirty_bitmap_disable(bs->node_name, bitmap, &err);
+            op = "disable";
+            break;
+        case BITMAP_MERGE: {
+            BlockDirtyBitmapMergeSource *merge_src;
+            BlockDirtyBitmapMergeSourceList *list;
+
+            merge_src = g_new0(BlockDirtyBitmapMergeSource, 1);
+            merge_src->type = QTYPE_QDICT;
+            merge_src->u.external.node = g_strdup(src_bs->node_name);
+            merge_src->u.external.name = g_strdup(act->src);
+            list = g_new0(BlockDirtyBitmapMergeSourceList, 1);
+            list->value = merge_src;
+            qmp_block_dirty_bitmap_merge(bs->node_name, bitmap, list, &err);
+            qapi_free_BlockDirtyBitmapMergeSourceList(list);
+            op = "merge";
+            break;
+        }
+        default:
+            g_assert_not_reached();
+        }
+
+        if (err) {
+            error_reportf_err(err, "Operation %s on bitmap %s failed: ",
+                              op, bitmap);
+            goto out;
+        }
+        g_free(act);
+    }
+
+    ret = 0;
+
+ out:
+    blk_unref(src);
+    blk_unref(blk);
+    qemu_opts_del(opts);
+    return ret;
+}
+
 #define C_BS      01
 #define C_COUNT   02
 #define C_IF      04
diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
index e0886437b1f2..011688245668 100644
--- a/qemu-img-cmds.hx
+++ b/qemu-img-cmds.hx
@@ -20,6 +20,13 @@ DEF("bench", img_bench,
 SRST
 .. option:: bench [-c COUNT] [-d DEPTH] [-f FMT] [--flush-interval=FLUSH_INTERVAL] [-i AIO] [-n] [--no-drain] [-o OFFSET] [--pattern=PATTERN] [-q] [-s BUFFER_SIZE] [-S STEP_SIZE] [-t CACHE] [-w] [-U] FILENAME
 ERST
+
+DEF("bitmap", img_bitmap,
+    "bitmap (--merge SOURCE | --add | --remove | --clear | --enable | --disable)... [-b source_file [-F source_fmt]] [-g granularity] [--object objectdef] [--image-opts | -f fmt] filename bitmap")
+SRST
+.. option:: bitmap (--merge SOURCE | --add | --remove | --clear | --enable | --disable)... [-b SOURCE_FILE [-F SOURCE_FMT]] [-g GRANULARITY] [--object OBJECTDEF] [--image-opts | -f FMT] FILENAME BITMAP
+ERST
+
 DEF("check", img_check,
     "check [--object objectdef] [--image-opts] [-q] [-f fmt] [--output=ofmt] [-r [leaks | all]] [-T src_cache] [-U] filename")
 SRST
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PULL 0/7] bitmaps patches for 2020-05-18
  2020-05-18 20:54 [PULL 0/7] bitmaps patches for 2020-05-18 Eric Blake
                   ` (6 preceding siblings ...)
  2020-05-18 20:54 ` [PULL 7/7] qemu-img: Add bitmap sub-command Eric Blake
@ 2020-05-19 12:42 ` Peter Maydell
  2020-05-19 15:30   ` Eric Blake
  7 siblings, 1 reply; 10+ messages in thread
From: Peter Maydell @ 2020-05-19 12:42 UTC (permalink / raw)
  To: Eric Blake; +Cc: QEMU Developers

On Mon, 18 May 2020 at 21:55, Eric Blake <eblake@redhat.com> wrote:
>
> The following changes since commit a28c9c8c9fc42484efe1bf5a77affe842e54e38b:
>
>   Merge remote-tracking branch 'remotes/kraxel/tags/vga-20200518-pull-request' into staging (2020-05-18 16:37:09 +0100)
>
> are available in the Git repository at:
>
>   https://repo.or.cz/qemu/ericb.git tags/pull-bitmaps-2020-05-18
>
> for you to fetch changes up to 6660c3a69c668ff98e2a86bca0b1301648ff4b02:
>
>   qemu-img: Add bitmap sub-command (2020-05-18 15:32:56 -0500)
>
> ----------------------------------------------------------------
> bitmaps patches for 2020-05-18
>
> - update bitmaps maintainers
> - add 'qemu-img bitmap' subcommand

Hi; this fails to compile:

/home/petmay01/linaro/qemu-for-merges/qemu-img.c: In function ‘img_bitmap’:
/home/petmay01/linaro/qemu-for-merges/qemu-img.c:4593:27: error: too
few arguments to function ‘cvtnum’
             granularity = cvtnum(optarg);
                           ^~~~~~
/home/petmay01/linaro/qemu-for-merges/qemu-img.c:510:16: note: declared here
 static int64_t cvtnum(const char *name, const char *value)
                ^~~~~~


thanks
-- PMM


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PULL 0/7] bitmaps patches for 2020-05-18
  2020-05-19 12:42 ` [PULL 0/7] bitmaps patches for 2020-05-18 Peter Maydell
@ 2020-05-19 15:30   ` Eric Blake
  0 siblings, 0 replies; 10+ messages in thread
From: Eric Blake @ 2020-05-19 15:30 UTC (permalink / raw)
  To: Peter Maydell; +Cc: QEMU Developers

On 5/19/20 7:42 AM, Peter Maydell wrote:
> On Mon, 18 May 2020 at 21:55, Eric Blake <eblake@redhat.com> wrote:
>>
>> The following changes since commit a28c9c8c9fc42484efe1bf5a77affe842e54e38b:
>>
>>    Merge remote-tracking branch 'remotes/kraxel/tags/vga-20200518-pull-request' into staging (2020-05-18 16:37:09 +0100)
>>
>> are available in the Git repository at:
>>
>>    https://repo.or.cz/qemu/ericb.git tags/pull-bitmaps-2020-05-18
>>
>> for you to fetch changes up to 6660c3a69c668ff98e2a86bca0b1301648ff4b02:
>>
>>    qemu-img: Add bitmap sub-command (2020-05-18 15:32:56 -0500)
>>
>> ----------------------------------------------------------------
>> bitmaps patches for 2020-05-18
>>
>> - update bitmaps maintainers
>> - add 'qemu-img bitmap' subcommand
> 
> Hi; this fails to compile:
> 
> /home/petmay01/linaro/qemu-for-merges/qemu-img.c: In function ‘img_bitmap’:
> /home/petmay01/linaro/qemu-for-merges/qemu-img.c:4593:27: error: too
> few arguments to function ‘cvtnum’
>               granularity = cvtnum(optarg);
>                             ^~~~~~

Semantic conflict with my NBD queue that got merged first. Will post v2 
pull request shortly.

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



^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2020-05-19 15:41 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-18 20:54 [PULL 0/7] bitmaps patches for 2020-05-18 Eric Blake
2020-05-18 20:54 ` [PULL 1/7] bitmaps: Update maintainer Eric Blake
2020-05-18 20:54 ` [PULL 2/7] docs: Sort sections on qemu-img subcommand parameters Eric Blake
2020-05-18 20:54 ` [PULL 3/7] qemu-img: Fix stale comments on doc location Eric Blake
2020-05-18 20:54 ` [PULL 4/7] block: Make it easier to learn which BDS support bitmaps Eric Blake
2020-05-18 20:54 ` [PULL 5/7] blockdev: Promote several bitmap functions to non-static Eric Blake
2020-05-18 20:54 ` [PULL 6/7] blockdev: Split off basic bitmap operations for qemu-img Eric Blake
2020-05-18 20:54 ` [PULL 7/7] qemu-img: Add bitmap sub-command Eric Blake
2020-05-19 12:42 ` [PULL 0/7] bitmaps patches for 2020-05-18 Peter Maydell
2020-05-19 15:30   ` Eric Blake

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.