Re: [Qemu-devel] [PATCH v4 7/9] qmp: Add dirty-bitmap-enable and dirty-bitmap-disable

[Eric Blake <eblake@redhat.com>]

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

On 03/27/2014 03:09 AM, Fam Zheng wrote:
> This allows to put the dirty bitmap into a disabled state where no more
> writes will be tracked.
> 
> It will be used before backup or writing to persistent file.

Is it worth the add-bitmap command being able to add a bitmap in the
disabled state to begin with?  I'm trying to figure out if there is a
race between creating a bitmap and disabling it that might matter to any
control flow, where being able to create it disabled would break that race.

> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  block.c               | 15 +++++++++++++++
>  blockdev.c            | 44 ++++++++++++++++++++++++++++++++++++++++++++
>  include/block/block.h |  2 ++
>  qapi-schema.json      | 32 ++++++++++++++++++++++++++++++++
>  qmp-commands.hx       | 10 ++++++++++
>  5 files changed, 103 insertions(+)
> 
> +++ b/qapi-schema.json
> @@ -2254,6 +2254,38 @@
>    'data': { 'device': 'str', 'name': 'str' } }
>  
>  ##
> +# @dirty-bitmap-enable
> +#
> +# Enable a dirty bitmap on the device
> +#
> +# Setting granularity has no effect here.

See my comments on 2/9 about how this looks odd.

> +#
> +# Returns: nothing on success
> +#          If @device is not a valid block device, DeviceNotFound
> +#          If @name is not found, GenericError with an explaining message
> +#
> +# Since 2.1
> +##
> +{'command': 'dirty-bitmap-enable',
> +  'data': 'DirtyBitmap' }

Is it worth consolidating this into one command with a 'bool' parameter
that says whether to enable or disable, instead of two separate
commands?  Also, is there a counterpart query- command that I can use to
see the current state of a named dirty bitmap and whether it is
currently enabled, so that this isn't a write-only interface?

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