Re: [Qemu-devel] [PATCH] qmp: add query-block-dirty-bitmap

[Eric Blake <eblake@redhat.com>]

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

On 01/22/2016 10:07 AM, Vladimir Sementsov-Ogievskiy wrote:
> Add qmp command to query dirty bitmap. This is needed for external
> backup.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  block.c               | 41 ++++++++++++++++++++++++++++++++++++++++
>  blockdev.c            | 21 +++++++++++++++++++++
>  include/block/block.h |  2 ++
>  qapi/block-core.json  | 52 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  qmp-commands.hx       | 22 ++++++++++++++++++++++
>  5 files changed, 138 insertions(+)

Just an interface review at this time:

> +++ b/qapi/block-core.json
> @@ -414,6 +414,58 @@
>  ##
>  { 'command': 'query-block', 'returns': ['BlockInfo'] }
>  
> +##
> +# @BlockDirtyRegion:
> +#
> +# Region in bytes.
> +#
> +# @start: first byte
> +#
> +# @count: number of bytes in the region
> +#
> +# Since: 2.3

Don't you mean 2.6?

> +##
> +{ 'struct': 'BlockDirtyRegion',
> +  'data': { 'start': 'int', 'count': 'int' } }
> +
> +##
> +# @BlockDirtyBitmapInfo
> +#
> +# @name: the name of the dirty bitmap
> +#
> +# @size: size of the dirty bitmap in sectors
> +#
> +# @granularity: granularity of the dirty bitmap in bytes
> +#
> +# @disabled: whether the dirty bitmap is disabled
> +#
> +# @dirty-count: number of dirty bytes according to the dirty bitmap
> +#
> +# @dirty-regions: dirty regions of the bitmap
> +#
> +# Since 2.3

and again

> +##
> +{ 'struct': 'BlockDirtyBitmapInfo',
> +  'data': { 'name': 'str',
> +            'size': 'int',
> +            'granularity': 'int',
> +            'disabled': 'bool',
> +            'dirty-count': 'int',
> +            'dirty-regions': ['BlockDirtyRegion'] } }
> +
> +##
> +# @query-block-dirty-bitmap
> +#
> +# Get a description for specified dirty bitmap including it's dirty regions.
> +# This command is in general for testing purposes.
> +#
> +# Returns: @BlockDirtyBitmapInfo
> +#
> +# Since: 2.3

and again

> +##
> +{ 'command': 'query-block-dirty-bitmap',
> +  'data': 'BlockDirtyBitmap',
> +  'returns': 'BlockDirtyBitmapInfo' }

Seems reasonable for getting at the information for one bitmap.  But
would it be smarter to have:

{ 'command': 'query-block-dirty-bitmap',
  'data': { 'node':'str', '*name':'str' },
  'returns': [ 'BlockDirtyBitmapInfo' ] }

so that you could get ALL the dirty bitmaps for a single node (with
optional filtering to get an array of just one, if 'name' was provided)?
 Or, should BlockDirtyBitmapInfo also include a node name, then you
could query all dirty bitmaps for all nodes at once?  Is that too much
data for one QMP command?


> +
> +query-block-dirty-bitmap
> +------------------------
> +Since 2.6
> +
> +Get dirty bitmap info, including contents. Bitmap data are returned as array of
> +dirty regions
> +
> +Arguments:
> +
> +- "node": device/node on which to remove dirty bitmap (json-string)

Too much copy-and-paste; you aren't removing the bitmap.

> +- "name": name of the dirty bitmap to remove (json-string)
> +
> +EQMP

Worth showing example output?


-- 
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 --]