[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v6 02/10] qmp: Add block-dirty-bitmap-add and bl
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v6 02/10] qmp: Add block-dirty-bitmap-add and block-dirty-bitmap-remove |
Date: |
Fri, 07 Nov 2014 14:00:14 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.2.0 |
On 10/30/2014 04:22 AM, Fam Zheng wrote:
> The new command pair is added to manage user created dirty bitmap. The
> dirty bitmap's name is mandatory and must be unique for the same device,
> but different devices can have bitmaps with the same names.
>
> Signed-off-by: Fam Zheng <address@hidden>
> ---
> blockdev.c | 55
> ++++++++++++++++++++++++++++++++++++++++++++++++++++
> qapi/block-core.json | 55
> ++++++++++++++++++++++++++++++++++++++++++++++++++++
> qmp-commands.hx | 49 ++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 159 insertions(+)
>
> + if (has_granularity) {
> + if (granularity < 512 || is_power_of_2(granularity)) {
> + error_setg(errp, "Granularity must be power of 2 "
> + "and greater than 512");
Assuming you fix the logic of the condition, the error message is still
wrong (you allow 512, not just greater than 512).
> +++ b/qapi/block-core.json
> @@ -865,6 +865,61 @@
> '*on-target-error': 'BlockdevOnError' } }
>
> ##
> +# @BlockDirtyBitmap
> +#
> +# @device: name of device which the bitmap is tracking
> +#
> +# @name: name of the dirty bitmap
> +#
> +# Since 2.3
> +##
> +{ 'type': 'BlockDirtyBitmap',
> + 'data': { 'device': 'str', 'name': 'str' } }
> +
> +##
> +# @BlockDirtyBitmapAdd
> +#
> +# @device: name of device which the bitmap is tracking
> +#
> +# @name: name of the dirty bitmap
> +#
> +# @granularity: #optional the bitmap granularity, default is 64k for
> +# block-dirty-bitmap-add
Do you still need to call out the command, given that it is the only
client of this type?
> +#
> +# Since 2.3
> +##
> +{ 'type': 'BlockDirtyBitmapAdd',
> + 'data': { 'device': 'str', 'name': 'str', '*granularity': 'int' } }
Is it worth using type inheritance, as in:
{ 'type': 'BlockDirtyBitmapAdd',
'base': 'BlockDirtyBitmap',
'data': { '*granularity': 'int' } }
> +
> +##
> +# @block-dirty-bitmap-add
> +#
> +# Create a dirty bitmap with a name on the device
> +#
> +# Returns: nothing on success
> +# If @device is not a valid block device, DeviceNotFound
> +# If @name is already taken, GenericError with an explaining message
s/explaining message/explanation/ (or even ditch the error mentions;
fewer commands are bothering with it these days)
> +
> +block-dirty-bitmap-add
> +----------------
Pad out the --- to the length of the command name.
> +
> +Create a dirty bitmap with a name on the device, and start tracking the
> writes.
> +
> +Arguments:
> +
> +- "device": device name to create dirty bitmap (json-string)
> +- "name": name of the new dirty bitmap (json-string)
> +- "granularity": granularity to track writes with. (int)
Inconsistent on whether you use trailing '.'
> +
> +Example:
> +
> +-> { "execute": "block-dirty-bitmap-add", "arguments": { "device": "drive0",
> + "name": "bitmap0" } }
> +<- { "return": {} }
> +
> +block-dirty-bitmap-remove
> +----------------
Pad out the ---
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature