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

[Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>]

On 22.01.2016 20:22, Denis V. Lunev wrote:
> On 01/22/2016 08:07 PM, 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(+)
>>
>> diff --git a/block.c b/block.c
>> index 5709d3d..9a28589 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -3717,6 +3717,47 @@ void bdrv_set_dirty(BlockDriverState *bs, 
>> int64_t cur_sector,
>>       }
>>   }
>>   +BlockDirtyBitmapInfo *bdrv_query_dirty_bitmap(BlockDriverState *bs,
>> +                                              BdrvDirtyBitmap *bitmap)
>> +{
>> +    BlockDirtyBitmapInfo *info = g_new0(BlockDirtyBitmapInfo, 1);
>> +    BlockDirtyRegionList **plist = &info->dirty_regions;
>> +    uint64_t begin = 0, end = 0, size = bitmap->size;
>> +    HBitmap *hb = bitmap->bitmap;
>> +
>> +    info->dirty_count = bdrv_get_dirty_count(bitmap);
>> +    info->granularity = bdrv_dirty_bitmap_granularity(bitmap);
>> +    info->size = bitmap->size;
>> +    info->name = g_strdup(bitmap->name);
>> +    info->disabled = bitmap->disabled;
>> +    info->dirty_regions = NULL;
>> +
>> +    for (; begin < size; ++begin) {
>> +        BlockDirtyRegion *region;
>> +        BlockDirtyRegionList *entry;
>> +
>> +        if (!hbitmap_get(hb, begin)) {
>> +            continue;
>> +        }
>> +
>> +        for (end = begin + 1; end < size && hbitmap_get(hb, end); 
>> ++end) {
>> +            ;
>> +        }
> let us implemented faster finder. This would be useful in other 
> places. It could be
> 100 times faster!

I'll use iterators here, if the whole way is ok (querying regions, not blob)

>
>> +
>> +        region = g_new0(BlockDirtyRegion, 1);
>> +        entry = g_new0(BlockDirtyRegionList, 1);
>> +        region->start = begin;
>> +        region->count = end - begin;
>> +        entry->value = region;
>> +        *plist = entry;
>> +        plist = &entry->next;
>> +
>> +        begin = end;
>> +    }
>> +
>> +    return info;
>> +}
>> +
>>   /**
>>    * Advance an HBitmapIter to an arbitrary offset.
>>    */
>> diff --git a/blockdev.c b/blockdev.c
>> index 07cfe25..d2bc453 100644
>> --- a/blockdev.c
>> +++ b/blockdev.c
>> @@ -2734,6 +2734,27 @@ void qmp_block_dirty_bitmap_clear(const char 
>> *node, const char *name,
>>       aio_context_release(aio_context);
>>   }
>>   +BlockDirtyBitmapInfo *qmp_query_block_dirty_bitmap(const char *node,
>> +                                                   const char *name,
>> +                                                   Error **errp)
>> +{
>> +    AioContext *aio_context;
>> +    BdrvDirtyBitmap *bitmap;
>> +    BlockDriverState *bs;
>> +    BlockDirtyBitmapInfo *ret = NULL;
>> +
>> +    bitmap = block_dirty_bitmap_lookup(node, name, &bs, 
>> &aio_context, errp);
>> +    if (!bitmap) {
>> +        return NULL;
>> +    }
>> +
>> +    ret = bdrv_query_dirty_bitmap(bs, bitmap);
>> +
>> +    aio_context_release(aio_context);
>> +
>> +    return ret;
>> +}
>> +
>>   void hmp_drive_del(Monitor *mon, const QDict *qdict)
>>   {
>>       const char *id = qdict_get_str(qdict, "id");
>> diff --git a/include/block/block.h b/include/block/block.h
>> index 25f36dc..9d6bd33 100644
>> --- a/include/block/block.h
>> +++ b/include/block/block.h
>> @@ -505,6 +505,8 @@ void bdrv_set_dirty_bitmap(BdrvDirtyBitmap *bitmap,
>>                              int64_t cur_sector, int nr_sectors);
>>   void bdrv_reset_dirty_bitmap(BdrvDirtyBitmap *bitmap,
>>                                int64_t cur_sector, int nr_sectors);
>> +BlockDirtyBitmapInfo *bdrv_query_dirty_bitmap(BlockDriverState *bs,
>> +                                              BdrvDirtyBitmap *bitmap);
>>   void bdrv_dirty_iter_init(BdrvDirtyBitmap *bitmap, struct 
>> HBitmapIter *hbi);
>>   void bdrv_set_dirty_iter(struct HBitmapIter *hbi, int64_t offset);
>>   int64_t bdrv_get_dirty_count(BdrvDirtyBitmap *bitmap);
>> diff --git a/qapi/block-core.json b/qapi/block-core.json
>> index 0a915ed..12ed759 100644
>> --- a/qapi/block-core.json
>> +++ 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
>> +##
>> +{ '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
>> +##
>> +{ '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
>> +##
>> +{ 'command': 'query-block-dirty-bitmap',
>> +  'data': 'BlockDirtyBitmap',
>> +  'returns': 'BlockDirtyBitmapInfo' }
> 1) should we consider part-by-part retrieval? This could be useful for 
> large discs.
> 2) Change to since 2.6
> 3) Do you think that content should be retrieved separately than data?

3 - what do you mean?


>
>
>>     ##
>>   # @BlockDeviceTimedStats:
>> diff --git a/qmp-commands.hx b/qmp-commands.hx
>> index db072a6..75d9345 100644
>> --- a/qmp-commands.hx
>> +++ b/qmp-commands.hx
>> @@ -1457,6 +1457,28 @@ Example:
>>   EQMP
>>         {
>> +        .name       = "query-block-dirty-bitmap",
>> +        .args_type  = "node:B,name:s",
>> +        .mhandler.cmd_new = qmp_marshal_query_block_dirty_bitmap,
>> +    },
>> +
>> +SQMP
>> +
>> +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)
>> +- "name": name of the dirty bitmap to remove (json-string)
>> +
>> +EQMP
>> +
>> +    {
>>           .name       = "blockdev-snapshot-sync",
>>           .args_type  = 
>> "device:s?,node-name:s?,snapshot-file:s,snapshot-node-name:s?,format:s?,mode:s?",
>>           .mhandler.cmd_new = qmp_marshal_blockdev_snapshot_sync,
> can you pls use den@openvz.org addr sending patches to me?

ok, sorry.


-- 
Best regards,
Vladimir
* now, @virtuozzo.com instead of @parallels.com. Sorry for this inconvenience.