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

[Vladimir Sementsov-Ogievskiy]

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 <address@hidden>
> > ---
> >   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 address@hidden addr sending patches to me?

ok, sorry.


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