Re: [Qemu-devel] [PATCH 7/8] qmp: print dirty bitmap

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2211 bytes --]

On 01/13/2015 10:02 AM, Vladimir Sementsov-Ogievskiy wrote:
> Adds qmp and hmp commands to print dirty bitmap. This is needed only for
> testing persistent dirty bitmap feature.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@parallels.com>
> ---

> +++ b/block.c
> @@ -5445,6 +5445,39 @@ int bdrv_store_dirty_bitmap(BdrvDirtyBitmap *bitmap)
>      return res;
>  }
>  
> +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");

> +++ b/blockdev.c
> @@ -2079,6 +2079,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);

Eww.  This assumes that stdout is usable.  But that is not the case for
QMP commands.  It would be better to package up the output in a
structured format and return it to the caller, and let the caller decide
how to print it.

> +++ b/qapi/block-core.json
> @@ -982,6 +982,9 @@
>  {'command': 'block-dirty-bitmap-disable',
>    'data': 'BlockDirtyBitmap' }
>  
> +{'command': 'block-dirty-bitmap-print',
> +  'data': 'BlockDirtyBitmap' }

Missing documentation, if we even want this command.  As mentioned
above, this should return ALL of the information necessary for the
client to then decide how to print the information, rather than directly
printing information to stdout itself.  And it might be better to name
this command in the 'query-' namespace.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]