diff mbox

[RFC,v2,1/8] qmp: print dirty bitmap

Message ID 1422356197-5285-2-git-send-email-vsementsov@parallels.com
State New
Headers show

Commit Message

Vladimir Sementsov-Ogievskiy Jan. 27, 2015, 10:56 a.m. UTC
Adds qmp and hmp commands to print dirty bitmap. This is needed only for
testing.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@parallels.com>
---
 block.c               | 33 +++++++++++++++++++++++++++++++++
 blockdev.c            | 13 +++++++++++++
 hmp-commands.hx       | 15 +++++++++++++++
 hmp.c                 |  8 ++++++++
 hmp.h                 |  1 +
 include/block/block.h |  2 ++
 qapi-schema.json      |  3 ++-
 qapi/block-core.json  |  3 +++
 qmp-commands.hx       |  5 +++++
 9 files changed, 82 insertions(+), 1 deletion(-)

Comments

Eric Blake Jan. 27, 2015, 4:17 p.m. UTC | #1
On 01/27/2015 03:56 AM, Vladimir Sementsov-Ogievskiy wrote:
> Adds qmp and hmp commands to print dirty bitmap. This is needed only for
> testing.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@parallels.com>
> ---
>  block.c               | 33 +++++++++++++++++++++++++++++++++
>  blockdev.c            | 13 +++++++++++++
>  hmp-commands.hx       | 15 +++++++++++++++
>  hmp.c                 |  8 ++++++++
>  hmp.h                 |  1 +
>  include/block/block.h |  2 ++
>  qapi-schema.json      |  3 ++-
>  qapi/block-core.json  |  3 +++
>  qmp-commands.hx       |  5 +++++
>  9 files changed, 82 insertions(+), 1 deletion(-)
> 

> +void bdrv_print_dirty_bitmap(BdrvDirtyBitmap *bitmap)
> +{
> +    unsigned long a = 0, b = 0;
> +
> +    printf("bitmap '%s'\n", bitmap->name ? bitmap->name : "no name");
> +    printf("enabled: %s\n", bitmap->enabled ? "true" : "false");
> +    printf("size: %" PRId64 "\n", bitmap->size);
> +    printf("granularity: %" PRId64 "\n", bitmap->granularity);
> +    printf("dirty regions begin:\n");
> +

> +void qmp_block_dirty_bitmap_print(const char *node_ref, const char *name,
> +                                  Error **errp)
> +{
> +    BdrvDirtyBitmap *bitmap;
> +
> +    bitmap = block_dirty_bitmap_lookup(node_ref, name, NULL, errp);
> +    if (!bitmap) {
> +        return;
> +    }
> +
> +    bdrv_print_dirty_bitmap(bitmap);

Won't work.  You cannot assume that stdout is usable when invoked from
QMP.  The only sane thing to do is to bundle up the structured data into
JSON, pass that back over the QMP connection, and let the client decide
how to print it.

I'm opposed to adding this command as-is.
Vladimir Sementsov-Ogievskiy Jan. 27, 2015, 4:23 p.m. UTC | #2
Ok, I agree. It was just a simple way to test the other staff. I'll 
rewrite it in the following versions of my two series.

Best regards,
Vladimir

On 27.01.2015 19:17, Eric Blake wrote:
> On 01/27/2015 03:56 AM, Vladimir Sementsov-Ogievskiy wrote:
>> Adds qmp and hmp commands to print dirty bitmap. This is needed only for
>> testing.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@parallels.com>
>> ---
>>   block.c               | 33 +++++++++++++++++++++++++++++++++
>>   blockdev.c            | 13 +++++++++++++
>>   hmp-commands.hx       | 15 +++++++++++++++
>>   hmp.c                 |  8 ++++++++
>>   hmp.h                 |  1 +
>>   include/block/block.h |  2 ++
>>   qapi-schema.json      |  3 ++-
>>   qapi/block-core.json  |  3 +++
>>   qmp-commands.hx       |  5 +++++
>>   9 files changed, 82 insertions(+), 1 deletion(-)
>>
>> +void bdrv_print_dirty_bitmap(BdrvDirtyBitmap *bitmap)
>> +{
>> +    unsigned long a = 0, b = 0;
>> +
>> +    printf("bitmap '%s'\n", bitmap->name ? bitmap->name : "no name");
>> +    printf("enabled: %s\n", bitmap->enabled ? "true" : "false");
>> +    printf("size: %" PRId64 "\n", bitmap->size);
>> +    printf("granularity: %" PRId64 "\n", bitmap->granularity);
>> +    printf("dirty regions begin:\n");
>> +
>> +void qmp_block_dirty_bitmap_print(const char *node_ref, const char *name,
>> +                                  Error **errp)
>> +{
>> +    BdrvDirtyBitmap *bitmap;
>> +
>> +    bitmap = block_dirty_bitmap_lookup(node_ref, name, NULL, errp);
>> +    if (!bitmap) {
>> +        return;
>> +    }
>> +
>> +    bdrv_print_dirty_bitmap(bitmap);
> Won't work.  You cannot assume that stdout is usable when invoked from
> QMP.  The only sane thing to do is to bundle up the structured data into
> JSON, pass that back over the QMP connection, and let the client decide
> how to print it.
>
> I'm opposed to adding this command as-is.
>
John Snow Feb. 10, 2015, 9:28 p.m. UTC | #3
On 01/27/2015 05:56 AM, Vladimir Sementsov-Ogievskiy wrote:
> Adds qmp and hmp commands to print dirty bitmap. This is needed only for
> testing.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@parallels.com>
> ---
>   block.c               | 33 +++++++++++++++++++++++++++++++++
>   blockdev.c            | 13 +++++++++++++
>   hmp-commands.hx       | 15 +++++++++++++++
>   hmp.c                 |  8 ++++++++
>   hmp.h                 |  1 +
>   include/block/block.h |  2 ++
>   qapi-schema.json      |  3 ++-
>   qapi/block-core.json  |  3 +++
>   qmp-commands.hx       |  5 +++++
>   9 files changed, 82 insertions(+), 1 deletion(-)
>
> diff --git a/block.c b/block.c
> index 2466ba8..6d3f0b2 100644
> --- a/block.c
> +++ b/block.c
> @@ -5374,6 +5374,39 @@ BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap(BlockDriverState *bs,
>   }
>
>
> +void bdrv_print_dirty_bitmap(BdrvDirtyBitmap *bitmap)
> +{
> +    unsigned long a = 0, b = 0;
> +
> +    printf("bitmap '%s'\n", bitmap->name ? bitmap->name : "no name");
> +    printf("enabled: %s\n", bitmap->enabled ? "true" : "false");
> +    printf("size: %" PRId64 "\n", bitmap->size);
> +    printf("granularity: %" PRId64 "\n", bitmap->granularity);
> +    printf("dirty regions begin:\n");
> +
> +    while (true) {
> +        for (a = b; a < bitmap->size && !hbitmap_get(bitmap->bitmap, a); ++a) {
> +            ;
> +        }
> +        if (a >= bitmap->size) {
> +            break;
> +        }
> +
> +        for (b = a + 1;
> +             b < bitmap->size && hbitmap_get(bitmap->bitmap, b);
> +             ++b) {
> +            ;
> +        }
> +
> +        printf("%ld -> %ld\n", a, b - 1);
> +        if (b >= bitmap->size) {
> +            break;
> +        }
> +    }
> +
> +    printf("dirty regions end\n");
> +}
> +
>   BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
>                                             int granularity,
>                                             const char *name,
> diff --git a/blockdev.c b/blockdev.c
> index 209fedd..66f0437 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -2074,6 +2074,19 @@ void qmp_block_dirty_bitmap_add(const char *node_ref, const char *name,
>       aio_context_release(aio_context);
>   }
>
> +void qmp_block_dirty_bitmap_print(const char *node_ref, const char *name,
> +                                  Error **errp)
> +{
> +    BdrvDirtyBitmap *bitmap;
> +
> +    bitmap = block_dirty_bitmap_lookup(node_ref, name, NULL, errp);
> +    if (!bitmap) {
> +        return;
> +    }
> +
> +    bdrv_print_dirty_bitmap(bitmap);
> +}
> +
>   void qmp_block_dirty_bitmap_remove(const char *node_ref, const char *name,
>                                      Error **errp)
>   {
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index e37bc8b..a9be506 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -58,6 +58,21 @@ Quit the emulator.
>   ETEXI
>
>       {
> +        .name       = "print_dirty_bitmap",
> +        .args_type  = "device:B,bitmap:s",
> +        .params     = "device bitmap",
> +        .help       = "print dirty bitmap",
> +        .user_print = monitor_user_noop,
> +        .mhandler.cmd = hmp_print_dirty_bitmap,
> +    },
> +
> +STEXI
> +@item print_dirty_bitmap device_id bitmap_name
> +@findex print_dirty_bitmap
> +Print dirty bitmap meta information and dirty regions.
> +ETEXI
> +
> +    {
>           .name       = "block_resize",
>           .args_type  = "device:B,size:o",
>           .params     = "device size",
> diff --git a/hmp.c b/hmp.c
> index 63b19c7..a269145 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -782,6 +782,14 @@ void hmp_info_tpm(Monitor *mon, const QDict *qdict)
>       qapi_free_TPMInfoList(info_list);
>   }
>
> +void hmp_print_dirty_bitmap(Monitor *mon, const QDict *qdict)
> +{
> +    const char *device = qdict_get_str(qdict, "device");
> +    const char *name = qdict_get_str(qdict, "bitmap");
> +
> +    qmp_block_dirty_bitmap_print(device, name, NULL);
> +}
> +
>   void hmp_quit(Monitor *mon, const QDict *qdict)
>   {
>       monitor_suspend(mon);
> diff --git a/hmp.h b/hmp.h
> index 4bb5dca..6bbbc33 100644
> --- a/hmp.h
> +++ b/hmp.h
> @@ -19,6 +19,7 @@
>   #include "qapi-types.h"
>   #include "qapi/qmp/qdict.h"
>
> +void hmp_print_dirty_bitmap(Monitor *mon, const QDict *qdict);
>   void hmp_info_name(Monitor *mon, const QDict *qdict);
>   void hmp_info_version(Monitor *mon, const QDict *qdict);
>   void hmp_info_kvm(Monitor *mon, const QDict *qdict);
> diff --git a/include/block/block.h b/include/block/block.h
> index cb1f28d..6cf067f 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -459,6 +459,8 @@ void bdrv_dirty_iter_init(BlockDriverState *bs,
>   void bdrv_set_dirty_iter(struct HBitmapIter *hbi, int64_t offset);
>   int64_t bdrv_get_dirty_count(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
>
> +void bdrv_print_dirty_bitmap(BdrvDirtyBitmap *bitmap);
> +
>   void bdrv_enable_copy_on_read(BlockDriverState *bs);
>   void bdrv_disable_copy_on_read(BlockDriverState *bs);
>
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 85f55d9..1475f69 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -1263,7 +1263,8 @@
>          'blockdev-snapshot-internal-sync': 'BlockdevSnapshotInternal',
>          'block-dirty-bitmap-add': 'BlockDirtyBitmapAdd',
>          'block-dirty-bitmap-enable': 'BlockDirtyBitmap',
> -       'block-dirty-bitmap-disable': 'BlockDirtyBitmap'
> +       'block-dirty-bitmap-disable': 'BlockDirtyBitmap',
> +       'block-dirty-bitmap-print': 'BlockDirtyBitmap'
>      } }
>
>   ##
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 1892b50..3e1edb1 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -982,6 +982,9 @@
>   {'command': 'block-dirty-bitmap-disable',
>     'data': 'BlockDirtyBitmap' }
>
> +{'command': 'block-dirty-bitmap-print',
> +  'data': 'BlockDirtyBitmap' }
> +
>   ##
>   # @block_set_io_throttle:
>   #
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 479d4f5..8065715 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -1222,6 +1222,11 @@ EQMP
>           .args_type  = "node-ref:B,name:s",
>           .mhandler.cmd_new = qmp_marshal_input_block_dirty_bitmap_disable,
>       },
> +    {
> +        .name       = "block-dirty-bitmap-print",
> +        .args_type  = "node-ref:B,name:s",
> +        .mhandler.cmd_new = qmp_marshal_input_block_dirty_bitmap_print,
> +    },
>
>   SQMP
>
>

If we need it only for testing, perhaps just returning JSON data like 
checksums or similar enough to verify the migration went well should be 
sufficient.

Or maybe if you really want to see every bit, just return the bitmap as 
an array of hexstrings or something reasonably compact.

In terms of iotests, I'd be a fan of just a checksum of the lower layer 
to verify accurate transmission.
diff mbox

Patch

diff --git a/block.c b/block.c
index 2466ba8..6d3f0b2 100644
--- a/block.c
+++ b/block.c
@@ -5374,6 +5374,39 @@  BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap(BlockDriverState *bs,
 }
 
 
+void bdrv_print_dirty_bitmap(BdrvDirtyBitmap *bitmap)
+{
+    unsigned long a = 0, b = 0;
+
+    printf("bitmap '%s'\n", bitmap->name ? bitmap->name : "no name");
+    printf("enabled: %s\n", bitmap->enabled ? "true" : "false");
+    printf("size: %" PRId64 "\n", bitmap->size);
+    printf("granularity: %" PRId64 "\n", bitmap->granularity);
+    printf("dirty regions begin:\n");
+
+    while (true) {
+        for (a = b; a < bitmap->size && !hbitmap_get(bitmap->bitmap, a); ++a) {
+            ;
+        }
+        if (a >= bitmap->size) {
+            break;
+        }
+
+        for (b = a + 1;
+             b < bitmap->size && hbitmap_get(bitmap->bitmap, b);
+             ++b) {
+            ;
+        }
+
+        printf("%ld -> %ld\n", a, b - 1);
+        if (b >= bitmap->size) {
+            break;
+        }
+    }
+
+    printf("dirty regions end\n");
+}
+
 BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
                                           int granularity,
                                           const char *name,
diff --git a/blockdev.c b/blockdev.c
index 209fedd..66f0437 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2074,6 +2074,19 @@  void qmp_block_dirty_bitmap_add(const char *node_ref, const char *name,
     aio_context_release(aio_context);
 }
 
+void qmp_block_dirty_bitmap_print(const char *node_ref, const char *name,
+                                  Error **errp)
+{
+    BdrvDirtyBitmap *bitmap;
+
+    bitmap = block_dirty_bitmap_lookup(node_ref, name, NULL, errp);
+    if (!bitmap) {
+        return;
+    }
+
+    bdrv_print_dirty_bitmap(bitmap);
+}
+
 void qmp_block_dirty_bitmap_remove(const char *node_ref, const char *name,
                                    Error **errp)
 {
diff --git a/hmp-commands.hx b/hmp-commands.hx
index e37bc8b..a9be506 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -58,6 +58,21 @@  Quit the emulator.
 ETEXI
 
     {
+        .name       = "print_dirty_bitmap",
+        .args_type  = "device:B,bitmap:s",
+        .params     = "device bitmap",
+        .help       = "print dirty bitmap",
+        .user_print = monitor_user_noop,
+        .mhandler.cmd = hmp_print_dirty_bitmap,
+    },
+
+STEXI
+@item print_dirty_bitmap device_id bitmap_name
+@findex print_dirty_bitmap
+Print dirty bitmap meta information and dirty regions.
+ETEXI
+
+    {
         .name       = "block_resize",
         .args_type  = "device:B,size:o",
         .params     = "device size",
diff --git a/hmp.c b/hmp.c
index 63b19c7..a269145 100644
--- a/hmp.c
+++ b/hmp.c
@@ -782,6 +782,14 @@  void hmp_info_tpm(Monitor *mon, const QDict *qdict)
     qapi_free_TPMInfoList(info_list);
 }
 
+void hmp_print_dirty_bitmap(Monitor *mon, const QDict *qdict)
+{
+    const char *device = qdict_get_str(qdict, "device");
+    const char *name = qdict_get_str(qdict, "bitmap");
+
+    qmp_block_dirty_bitmap_print(device, name, NULL);
+}
+
 void hmp_quit(Monitor *mon, const QDict *qdict)
 {
     monitor_suspend(mon);
diff --git a/hmp.h b/hmp.h
index 4bb5dca..6bbbc33 100644
--- a/hmp.h
+++ b/hmp.h
@@ -19,6 +19,7 @@ 
 #include "qapi-types.h"
 #include "qapi/qmp/qdict.h"
 
+void hmp_print_dirty_bitmap(Monitor *mon, const QDict *qdict);
 void hmp_info_name(Monitor *mon, const QDict *qdict);
 void hmp_info_version(Monitor *mon, const QDict *qdict);
 void hmp_info_kvm(Monitor *mon, const QDict *qdict);
diff --git a/include/block/block.h b/include/block/block.h
index cb1f28d..6cf067f 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -459,6 +459,8 @@  void bdrv_dirty_iter_init(BlockDriverState *bs,
 void bdrv_set_dirty_iter(struct HBitmapIter *hbi, int64_t offset);
 int64_t bdrv_get_dirty_count(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
 
+void bdrv_print_dirty_bitmap(BdrvDirtyBitmap *bitmap);
+
 void bdrv_enable_copy_on_read(BlockDriverState *bs);
 void bdrv_disable_copy_on_read(BlockDriverState *bs);
 
diff --git a/qapi-schema.json b/qapi-schema.json
index 85f55d9..1475f69 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1263,7 +1263,8 @@ 
        'blockdev-snapshot-internal-sync': 'BlockdevSnapshotInternal',
        'block-dirty-bitmap-add': 'BlockDirtyBitmapAdd',
        'block-dirty-bitmap-enable': 'BlockDirtyBitmap',
-       'block-dirty-bitmap-disable': 'BlockDirtyBitmap'
+       'block-dirty-bitmap-disable': 'BlockDirtyBitmap',
+       'block-dirty-bitmap-print': 'BlockDirtyBitmap'
    } }
 
 ##
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 1892b50..3e1edb1 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -982,6 +982,9 @@ 
 {'command': 'block-dirty-bitmap-disable',
   'data': 'BlockDirtyBitmap' }
 
+{'command': 'block-dirty-bitmap-print',
+  'data': 'BlockDirtyBitmap' }
+
 ##
 # @block_set_io_throttle:
 #
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 479d4f5..8065715 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1222,6 +1222,11 @@  EQMP
         .args_type  = "node-ref:B,name:s",
         .mhandler.cmd_new = qmp_marshal_input_block_dirty_bitmap_disable,
     },
+    {
+        .name       = "block-dirty-bitmap-print",
+        .args_type  = "node-ref:B,name:s",
+        .mhandler.cmd_new = qmp_marshal_input_block_dirty_bitmap_print,
+    },
 
 SQMP