[Qemu-devel] [PATCH RFC] external backup api

[Qemu-devel] [PATCH RFC] external backup api

[Vladimir Sementsov-Ogievskiy]

Hi all.

This is the early begin of the series which aims to add external backup
api. This is needed to allow backup software use our dirty bitmaps.

Vmware and Parallels Cloud Server have this feature.

There is only one patch here, about querying dirty bitmap from qemu by
qmp command. It is just an updated and clipped (hmp command removed) old
my patch "[PATCH RFC v3 01/14] qmp: add query-block-dirty-bitmap".

Before writing the whole thing I'd like to discuss the details. Or, may
be there are existing plans on this topic, or may be someone already
works on it?

I see it like this:

=====

- add qmp commands for dirty-bitmap functions: create_successor, abdicate,
reclaime.
- make create-successor command transaction-able
- add query-block-dirty-bitmap qmp command

then, external backup:

qmp transaction {
    external-snapshot
    bitmap-create-successor
}

qmp query frozen bitmap, not acquiring aio context.

do external backup, using snapshot and bitmap

if (success backup)
    qmp bitmap-abdicate
else
    qmp bitmap-reclaime

qmp merge snapshot
=====


In the following patch query-bitmap acquires aio context. This must be
ofcourse dropped for frozen bitmap.
But to make it in true way, I think, I should check somehow that this is
not just frozen bitmap, but the bitmap frozen by qmp command, to avoid
incorrect quering of bitmap frozen by internal backup (or other
mechanizm).. May be, it is not necessary.



-- 
1.8.3.1

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

[Vladimir Sementsov-Ogievskiy]

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) {
+            ;
+        }
+
+        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' }
 
 ##
 # @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,
-- 
1.8.3.1

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

[Denis V. Lunev]

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!

> +
> +        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?


>   
>   ##
>   # @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?

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

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

[Denis V. Lunev]

On 01/22/2016 08:28 PM, Vladimir Sementsov-Ogievskiy wrote:
> On 22.01.2016 20:22, Denis V. Lunev wrote:
>> On 01/22/2016 08:07 PM, Vladimir Sementsov-Ogievskiy wrote:
>>   { '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?
>

sorry, I mean bitmap description separately from bitmap bits aka data

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

[Eric Blake]

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

Re: [Qemu-devel] [PATCH RFC] external backup api

[John Snow]

On 01/22/2016 12:07 PM, Vladimir Sementsov-Ogievskiy wrote:
> Hi all.
> 
> This is the early begin of the series which aims to add external backup
> api. This is needed to allow backup software use our dirty bitmaps.
> 
> Vmware and Parallels Cloud Server have this feature.
> 

Have a link to the equivalent feature that VMWare exposes? (Or Parallels
Cloud Server) ... I'm curious about what the API there looks like.

> There is only one patch here, about querying dirty bitmap from qemu by
> qmp command. It is just an updated and clipped (hmp command removed) old
> my patch "[PATCH RFC v3 01/14] qmp: add query-block-dirty-bitmap".
> 
> Before writing the whole thing I'd like to discuss the details. Or, may
> be there are existing plans on this topic, or may be someone already
> works on it?
> 
> I see it like this:
> 
> =====
> 
> - add qmp commands for dirty-bitmap functions: create_successor, abdicate,
> reclaime.

Hm, why do we need such low-level control over splitting and merging
bitmaps from an external client?

> - make create-successor command transaction-able
> - add query-block-dirty-bitmap qmp command
> 
> then, external backup:
> 
> qmp transaction {
>     external-snapshot
>     bitmap-create-successor
> }
> 
> qmp query frozen bitmap, not acquiring aio context.
> 
> do external backup, using snapshot and bitmap
> 
> if (success backup)
>     qmp bitmap-abdicate
> else
>     qmp bitmap-reclaime
> 
> qmp merge snapshot
> =====
> 

Hm, I see -- so you're hoping to manage the backup *entirely*
externally, so you want to be able to reach inside of QEMU and control
some status conditions to guarantee it'll be safe.

I'm not convinced QEMU can guarantee such things -- due to various flush
properties, race conditions on write, etc. QEMU handles all of this
internally in a non-public way at the moment.

> 
> In the following patch query-bitmap acquires aio context. This must be
> ofcourse dropped for frozen bitmap.
> But to make it in true way, I think, I should check somehow that this is
> not just frozen bitmap, but the bitmap frozen by qmp command, to avoid
> incorrect quering of bitmap frozen by internal backup (or other
> mechanizm).. May be, it is not necessary.
> 
> 
> 

Re: [Qemu-devel] [PATCH RFC] external backup api

[Vladimir Sementsov-Ogievskiy]

On 05.02.2016 22:48, John Snow wrote:
>
> On 01/22/2016 12:07 PM, Vladimir Sementsov-Ogievskiy wrote:
>> Hi all.
>>
>> This is the early begin of the series which aims to add external backup
>> api. This is needed to allow backup software use our dirty bitmaps.
>>
>> Vmware and Parallels Cloud Server have this feature.
>>
> Have a link to the equivalent feature that VMWare exposes? (Or Parallels
> Cloud Server) ... I'm curious about what the API there looks like.

For VMware you need their Virtual Disk Api Programming Guide
http://pubs.vmware.com/vsphere-60/topic/com.vmware.ICbase/PDF/vddk60_programming.pdf
Look at Changed Block Tracking (CBT) , Backup and Restore.

For PCS here is part of SDK header, related to the topic:

====================================
/*
  * Builds a map of the disk contents changes between 2 PITs.
    Parameters
    hDisk :       A handle of type PHT_VIRTUAL_DISK identifying
                  the virtual disk.
    sPit1Uuid :   Uuid of the older PIT.
    sPit2Uuid :   Uuid of the later PIT.
    phMap :       A pointer to a variable which receives the
          result (a handle of type PHT_VIRTUAL_DISK_MAP).
    Returns
    PRL_RESULT.
*/
PRL_METHOD_DECL( PARALLELS_API_VER_5,
                 PrlDisk_GetChangesMap_Local, (
         PRL_HANDLE hDisk,
         PRL_CONST_STR sPit1Uuid,
         PRL_CONST_STR sPit2Uuid,
         PRL_HANDLE_PTR phMap) );

/*
  * Reports the number of significant bits in the map.
    Parameters
    hMap :        A handle of type PHT_VIRTUAL_DISK_MAP identifying
                  the changes map.
    phSize :      A pointer to a variable which receives the
          result.
    Returns
    PRL_RESULT.
*/
PRL_METHOD_DECL( PARALLELS_API_VER_5,
                 PrlDiskMap_GetSize, (
         PRL_HANDLE hMap,
         PRL_UINT32_PTR pnSize) );

/*
  * Reports the size (in bytes) of a block mapped by a single bit
  * in the map.
    Parameters
    hMap :        A handle of type PHT_VIRTUAL_DISK_MAP identifying
                  the changes map.
    phSize :      A pointer to a variable which receives the
          result.
    Returns
    PRL_RESULT.
*/
PRL_METHOD_DECL( PARALLELS_API_VER_5,
                 PrlDiskMap_GetGranularity, (
         PRL_HANDLE hMap,
         PRL_UINT32_PTR pnSize) );

/*
  * Returns bits from the blocks map.
    Parameters
    hMap :        A handle of type PHT_VIRTUAL_DISK_MAP identifying
                  the changes map.
    pBuffer :     A pointer to a store.
    pnCapacity :  A pointer to a variable holding the size
          of the buffer and receiving the number of
          bytes actually written.
    Returns
    PRL_RESULT.
*/
PRL_METHOD_DECL( PARALLELS_API_VER_5,
                 PrlDiskMap_Read, (
         PRL_HANDLE hMap,
         PRL_VOID_PTR pBuffer,
         PRL_UINT32_PTR pnCapacity) );

=======================================


>
>> There is only one patch here, about querying dirty bitmap from qemu by
>> qmp command. It is just an updated and clipped (hmp command removed) old
>> my patch "[PATCH RFC v3 01/14] qmp: add query-block-dirty-bitmap".
>>
>> Before writing the whole thing I'd like to discuss the details. Or, may
>> be there are existing plans on this topic, or may be someone already
>> works on it?
>>
>> I see it like this:
>>
>> =====
>>
>> - add qmp commands for dirty-bitmap functions: create_successor, abdicate,
>> reclaime.
> Hm, why do we need such low-level control over splitting and merging
> bitmaps from an external client?
>
>> - make create-successor command transaction-able
>> - add query-block-dirty-bitmap qmp command
>>
>> then, external backup:
>>
>> qmp transaction {
>>      external-snapshot
>>      bitmap-create-successor
>> }
>>
>> qmp query frozen bitmap, not acquiring aio context.
>>
>> do external backup, using snapshot and bitmap
>>
>> if (success backup)
>>      qmp bitmap-abdicate
>> else
>>      qmp bitmap-reclaime
>>
>> qmp merge snapshot
>> =====
>>
> Hm, I see -- so you're hoping to manage the backup *entirely*
> externally, so you want to be able to reach inside of QEMU and control
> some status conditions to guarantee it'll be safe.
>
> I'm not convinced QEMU can guarantee such things -- due to various flush
> properties, race conditions on write, etc. QEMU handles all of this
> internally in a non-public way at the moment.

Hm, can you be more concrete? What operations are dangerous? We can do 
them in paused state for example.

>
>> In the following patch query-bitmap acquires aio context. This must be
>> ofcourse dropped for frozen bitmap.
>> But to make it in true way, I think, I should check somehow that this is
>> not just frozen bitmap, but the bitmap frozen by qmp command, to avoid
>> incorrect quering of bitmap frozen by internal backup (or other
>> mechanizm).. May be, it is not necessary.
>>
>>
>>
>
>


-- 
Best regards,
Vladimir

Re: [Qemu-devel] [PATCH RFC] external backup api

[John Snow]

On 02/06/2016 04:19 AM, Vladimir Sementsov-Ogievskiy wrote:
> On 05.02.2016 22:48, John Snow wrote:
>>
>> On 01/22/2016 12:07 PM, Vladimir Sementsov-Ogievskiy wrote:
>>> Hi all.
>>>
>>> This is the early begin of the series which aims to add external backup
>>> api. This is needed to allow backup software use our dirty bitmaps.
>>>
>>> Vmware and Parallels Cloud Server have this feature.
>>>
>> Have a link to the equivalent feature that VMWare exposes? (Or Parallels
>> Cloud Server) ... I'm curious about what the API there looks like.
> 
> For VMware you need their Virtual Disk Api Programming Guide
> http://pubs.vmware.com/vsphere-60/topic/com.vmware.ICbase/PDF/vddk60_programming.pdf
> 

Great, thanks!

> Look at Changed Block Tracking (CBT) , Backup and Restore.
> 
> For PCS here is part of SDK header, related to the topic:
> 
> ====================================
> /*
>  * Builds a map of the disk contents changes between 2 PITs.
>    Parameters
>    hDisk :       A handle of type PHT_VIRTUAL_DISK identifying
>                  the virtual disk.
>    sPit1Uuid :   Uuid of the older PIT.
>    sPit2Uuid :   Uuid of the later PIT.
>    phMap :       A pointer to a variable which receives the
>          result (a handle of type PHT_VIRTUAL_DISK_MAP).
>    Returns
>    PRL_RESULT.
> */
> PRL_METHOD_DECL( PARALLELS_API_VER_5,
>                 PrlDisk_GetChangesMap_Local, (
>         PRL_HANDLE hDisk,
>         PRL_CONST_STR sPit1Uuid,
>         PRL_CONST_STR sPit2Uuid,
>         PRL_HANDLE_PTR phMap) );
> 

Effectively giving you a dirty bitmap diff between two snapshots.
Something we don't currently genuinely support in QEMU.

> /*
>  * Reports the number of significant bits in the map.
>    Parameters
>    hMap :        A handle of type PHT_VIRTUAL_DISK_MAP identifying
>                  the changes map.
>    phSize :      A pointer to a variable which receives the
>          result.
>    Returns
>    PRL_RESULT.
> */
> PRL_METHOD_DECL( PARALLELS_API_VER_5,
>                 PrlDiskMap_GetSize, (
>         PRL_HANDLE hMap,
>         PRL_UINT32_PTR pnSize) );
> 

I assume this is roughly the dirty bit count, for us, this would be
dirty clusters. (Or whatever granularity you specified, but usually
clusters.)

> /*
>  * Reports the size (in bytes) of a block mapped by a single bit
>  * in the map.
>    Parameters
>    hMap :        A handle of type PHT_VIRTUAL_DISK_MAP identifying
>                  the changes map.
>    phSize :      A pointer to a variable which receives the
>          result.
>    Returns
>    PRL_RESULT.
> */
> PRL_METHOD_DECL( PARALLELS_API_VER_5,
>                 PrlDiskMap_GetGranularity, (
>         PRL_HANDLE hMap,
>         PRL_UINT32_PTR pnSize) );
> 

Basically a granularity query.

> /*
>  * Returns bits from the blocks map.
>    Parameters
>    hMap :        A handle of type PHT_VIRTUAL_DISK_MAP identifying
>                  the changes map.
>    pBuffer :     A pointer to a store.
>    pnCapacity :  A pointer to a variable holding the size
>          of the buffer and receiving the number of
>          bytes actually written.
>    Returns
>    PRL_RESULT.
> */
> PRL_METHOD_DECL( PARALLELS_API_VER_5,
>                 PrlDiskMap_Read, (
>         PRL_HANDLE hMap,
>         PRL_VOID_PTR pBuffer,
>         PRL_UINT32_PTR pnCapacity) );
> 

And this would be a direct bitmap query.

Is the expected usage here that the third party client will use this
bitmap to read the source image? Or do you query for the data from API?

I think the thought among block devs would be to opt for more of the
second option, and less allowing clients to directly interface with the
image files.

> =======================================
> 
> 
>>
>>> There is only one patch here, about querying dirty bitmap from qemu by
>>> qmp command. It is just an updated and clipped (hmp command removed) old
>>> my patch "[PATCH RFC v3 01/14] qmp: add query-block-dirty-bitmap".
>>>
>>> Before writing the whole thing I'd like to discuss the details. Or, may
>>> be there are existing plans on this topic, or may be someone already
>>> works on it?
>>>
>>> I see it like this:
>>>
>>> =====
>>>
>>> - add qmp commands for dirty-bitmap functions: create_successor,
>>> abdicate,
>>> reclaime.
>> Hm, why do we need such low-level control over splitting and merging
>> bitmaps from an external client?
>>
>>> - make create-successor command transaction-able
>>> - add query-block-dirty-bitmap qmp command
>>>
>>> then, external backup:
>>>
>>> qmp transaction {
>>>      external-snapshot
>>>      bitmap-create-successor
>>> }
>>>
>>> qmp query frozen bitmap, not acquiring aio context.
>>>
>>> do external backup, using snapshot and bitmap
>>>
>>> if (success backup)
>>>      qmp bitmap-abdicate
>>> else
>>>      qmp bitmap-reclaime
>>>
>>> qmp merge snapshot
>>> =====
>>>
>> Hm, I see -- so you're hoping to manage the backup *entirely*
>> externally, so you want to be able to reach inside of QEMU and control
>> some status conditions to guarantee it'll be safe.
>>
>> I'm not convinced QEMU can guarantee such things -- due to various flush
>> properties, race conditions on write, etc. QEMU handles all of this
>> internally in a non-public way at the moment.
> 
> Hm, can you be more concrete? What operations are dangerous? We can do
> them in paused state for example.
> 

I suppose if you're going to pause the VM, then it should be reasonably
safe, but recently there have been endeavors to augment the .qcow2
format to prohibit concurrent access, which might include a paused VM as
well, I'm not clear on the implementation.

If you do it via paused only, then you also don't need to expose the
freeze/rollback mechanisms: the existing clear mechanism alone is
sufficient:

(A) The frozen backup fails. Nothing new has been written, so we don't
need to adjust anything, we can just try again.
(B) The frozen backup succeeds. We can just clear the bitmap before
unfreezing.

I definitely have reservations about using this as a live fleecing
mechanism -- the backup block job uses a write-notifier to make
just-in-time backups of data before it is altered, leaving it the only
"safe" live backup mechanism in QEMU currently. (Alongside mirror.)

I actually have some patches from Fam to introduce a live fleecing
mechanism into QEMU (The idea being you create a point-in-time drive you
can get data from via NBD, then delete it when done) that might be more
appropriate, but I ran into a lot of problems with the patch. I'll post
the WIP for that patch to try to solicit comments on the best way forward.

Otherwise, My biggest question here is:
"What does fleecing a backup externally provide as a benefit over
backing up to an NBD target?"

You can already today perform incremental backups to an NBD target to
copy the data out via an external mechanism, is this not sufficient for
Parallels? If not, why?

>>
>>> In the following patch query-bitmap acquires aio context. This must be
>>> ofcourse dropped for frozen bitmap.
>>> But to make it in true way, I think, I should check somehow that this is
>>> not just frozen bitmap, but the bitmap frozen by qmp command, to avoid
>>> incorrect quering of bitmap frozen by internal backup (or other
>>> mechanizm).. May be, it is not necessary.
>>>
>>>
>>>
>>
>>
> 

Re: [Qemu-devel] [PATCH RFC] external backup api

[Vladimir Sementsov-Ogievskiy]

On 09.02.2016 00:14, John Snow wrote:
>
> On 02/06/2016 04:19 AM, Vladimir Sementsov-Ogievskiy wrote:
>> On 05.02.2016 22:48, John Snow wrote:
>>> On 01/22/2016 12:07 PM, Vladimir Sementsov-Ogievskiy wrote:
>>>> Hi all.
>>>>
>>>> This is the early begin of the series which aims to add external backup
>>>> api. This is needed to allow backup software use our dirty bitmaps.
>>>>
>>>> Vmware and Parallels Cloud Server have this feature.
>>>>
>>> Have a link to the equivalent feature that VMWare exposes? (Or Parallels
>>> Cloud Server) ... I'm curious about what the API there looks like.
>> For VMware you need their Virtual Disk Api Programming Guide
>> http://pubs.vmware.com/vsphere-60/topic/com.vmware.ICbase/PDF/vddk60_programming.pdf
>>
> Great, thanks!
>
>> Look at Changed Block Tracking (CBT) , Backup and Restore.
>>
>> For PCS here is part of SDK header, related to the topic:
>>
>> ====================================
>> /*
>>   * Builds a map of the disk contents changes between 2 PITs.
>>     Parameters
>>     hDisk :       A handle of type PHT_VIRTUAL_DISK identifying
>>                   the virtual disk.
>>     sPit1Uuid :   Uuid of the older PIT.
>>     sPit2Uuid :   Uuid of the later PIT.
>>     phMap :       A pointer to a variable which receives the
>>           result (a handle of type PHT_VIRTUAL_DISK_MAP).
>>     Returns
>>     PRL_RESULT.
>> */
>> PRL_METHOD_DECL( PARALLELS_API_VER_5,
>>                  PrlDisk_GetChangesMap_Local, (
>>          PRL_HANDLE hDisk,
>>          PRL_CONST_STR sPit1Uuid,
>>          PRL_CONST_STR sPit2Uuid,
>>          PRL_HANDLE_PTR phMap) );
>>
> Effectively giving you a dirty bitmap diff between two snapshots.
> Something we don't currently genuinely support in QEMU.

Just start dirty bitmap at point a and stop at point b..

>
>> /*
>>   * Reports the number of significant bits in the map.
>>     Parameters
>>     hMap :        A handle of type PHT_VIRTUAL_DISK_MAP identifying
>>                   the changes map.
>>     phSize :      A pointer to a variable which receives the
>>           result.
>>     Returns
>>     PRL_RESULT.
>> */
>> PRL_METHOD_DECL( PARALLELS_API_VER_5,
>>                  PrlDiskMap_GetSize, (
>>          PRL_HANDLE hMap,
>>          PRL_UINT32_PTR pnSize) );
>>
> I assume this is roughly the dirty bit count, for us, this would be
> dirty clusters. (Or whatever granularity you specified, but usually
> clusters.)
>
>> /*
>>   * Reports the size (in bytes) of a block mapped by a single bit
>>   * in the map.
>>     Parameters
>>     hMap :        A handle of type PHT_VIRTUAL_DISK_MAP identifying
>>                   the changes map.
>>     phSize :      A pointer to a variable which receives the
>>           result.
>>     Returns
>>     PRL_RESULT.
>> */
>> PRL_METHOD_DECL( PARALLELS_API_VER_5,
>>                  PrlDiskMap_GetGranularity, (
>>          PRL_HANDLE hMap,
>>          PRL_UINT32_PTR pnSize) );
>>
> Basically a granularity query.
>
>> /*
>>   * Returns bits from the blocks map.
>>     Parameters
>>     hMap :        A handle of type PHT_VIRTUAL_DISK_MAP identifying
>>                   the changes map.
>>     pBuffer :     A pointer to a store.
>>     pnCapacity :  A pointer to a variable holding the size
>>           of the buffer and receiving the number of
>>           bytes actually written.
>>     Returns
>>     PRL_RESULT.
>> */
>> PRL_METHOD_DECL( PARALLELS_API_VER_5,
>>                  PrlDiskMap_Read, (
>>          PRL_HANDLE hMap,
>>          PRL_VOID_PTR pBuffer,
>>          PRL_UINT32_PTR pnCapacity) );
>>
> And this would be a direct bitmap query.
>
> Is the expected usage here that the third party client will use this
> bitmap to read the source image? Or do you query for the data from API?

- from API.

>
> I think the thought among block devs would be to opt for more of the
> second option, and less allowing clients to directly interface with the
> image files.
>
>> =======================================
>>
>>
>>>> There is only one patch here, about querying dirty bitmap from qemu by
>>>> qmp command. It is just an updated and clipped (hmp command removed) old
>>>> my patch "[PATCH RFC v3 01/14] qmp: add query-block-dirty-bitmap".
>>>>
>>>> Before writing the whole thing I'd like to discuss the details. Or, may
>>>> be there are existing plans on this topic, or may be someone already
>>>> works on it?
>>>>
>>>> I see it like this:
>>>>
>>>> =====
>>>>
>>>> - add qmp commands for dirty-bitmap functions: create_successor,
>>>> abdicate,
>>>> reclaime.
>>> Hm, why do we need such low-level control over splitting and merging
>>> bitmaps from an external client?
>>>
>>>> - make create-successor command transaction-able
>>>> - add query-block-dirty-bitmap qmp command
>>>>
>>>> then, external backup:
>>>>
>>>> qmp transaction {
>>>>       external-snapshot
>>>>       bitmap-create-successor
>>>> }
>>>>
>>>> qmp query frozen bitmap, not acquiring aio context.
>>>>
>>>> do external backup, using snapshot and bitmap
>>>>
>>>> if (success backup)
>>>>       qmp bitmap-abdicate
>>>> else
>>>>       qmp bitmap-reclaime
>>>>
>>>> qmp merge snapshot
>>>> =====
>>>>
>>> Hm, I see -- so you're hoping to manage the backup *entirely*
>>> externally, so you want to be able to reach inside of QEMU and control
>>> some status conditions to guarantee it'll be safe.
>>>
>>> I'm not convinced QEMU can guarantee such things -- due to various flush
>>> properties, race conditions on write, etc. QEMU handles all of this
>>> internally in a non-public way at the moment.
>> Hm, can you be more concrete? What operations are dangerous? We can do
>> them in paused state for example.
>>
> I suppose if you're going to pause the VM, then it should be reasonably
> safe, but recently there have been endeavors to augment the .qcow2
> format to prohibit concurrent access, which might include a paused VM as
> well, I'm not clear on the implementation.
>
> If you do it via paused only, then you also don't need to expose the
> freeze/rollback mechanisms: the existing clear mechanism alone is
> sufficient:
>
> (A) The frozen backup fails. Nothing new has been written, so we don't
> need to adjust anything, we can just try again.
> (B) The frozen backup succeeds. We can just clear the bitmap before
> unfreezing.

We can't query bitmap in paused state - it may take too much time.

>
> I definitely have reservations about using this as a live fleecing
> mechanism -- the backup block job uses a write-notifier to make
> just-in-time backups of data before it is altered, leaving it the only
> "safe" live backup mechanism in QEMU currently. (Alongside mirror.)
>
> I actually have some patches from Fam to introduce a live fleecing
> mechanism into QEMU (The idea being you create a point-in-time drive you
> can get data from via NBD, then delete it when done) that might be more
> appropriate, but I ran into a lot of problems with the patch. I'll post
> the WIP for that patch to try to solicit comments on the best way forward.

After adding
=============
--- a/block.c
+++ b/block.c
@@ -1276,6 +1276,9 @@ void bdrv_set_backing_hd(BlockDriverState *bs, 
BlockDriverState *backing_hd)
      /* Otherwise we won't be able to commit due to check in bdrv_commit */
      bdrv_op_unblock(backing_hd, BLOCK_OP_TYPE_COMMIT_TARGET,
                      bs->backing_blocker);
+
+    bdrv_op_unblock(backing_hd, BLOCK_OP_TYPE_BACKUP_SOURCE,
+                    bs->backing_blocker);
  out:
      bdrv_refresh_limits(bs, NULL);
  }
==============
and tiny fix for qemu_io interface in iotest

Fam's "qemu-iotests: Image fleecing test case 089" works for me. Isn't 
it enough?



>
> Otherwise, My biggest question here is:
> "What does fleecing a backup externally provide as a benefit over
> backing up to an NBD target?"

Look at our answers on v2 of these series:

On 05.02.2016 11:28, Denis V. Lunev wrote:
> On 02/03/2016 11:14 AM, Fam Zheng wrote:
>> On Sat, 01/30 13:56, Vladimir Sementsov-Ogievskiy wrote:
>>> Hi all.
>>>
>>> These series which aims to add external backup api. This is needed 
>>> to allow
>>> backup software use our dirty bitmaps.
>>>
>>> Vmware and Parallels Cloud Server have this feature.
>> What is the advantage of this appraoch over "drive-backup 
>> sync=incremental
>> ..."?
>
> This will allow third-party vendors to backup QEMU VMs into
> their own formats or to the cloud etc.


>
> You can already today perform incremental backups to an NBD target to
> copy the data out via an external mechanism, is this not sufficient for
> Parallels? If not, why?
>
>>>> In the following patch query-bitmap acquires aio context. This must be
>>>> ofcourse dropped for frozen bitmap.
>>>> But to make it in true way, I think, I should check somehow that this is
>>>> not just frozen bitmap, but the bitmap frozen by qmp command, to avoid
>>>> incorrect quering of bitmap frozen by internal backup (or other
>>>> mechanizm).. May be, it is not necessary.
>>>>
>>>>
>>>>
>>>


-- 
Best regards,
Vladimir

Re: [Qemu-devel] [PATCH RFC] external backup api

[John Snow]

On 02/09/2016 10:54 AM, Vladimir Sementsov-Ogievskiy wrote:
> On 09.02.2016 00:14, John Snow wrote:
>>
>> On 02/06/2016 04:19 AM, Vladimir Sementsov-Ogievskiy wrote:
>>> On 05.02.2016 22:48, John Snow wrote:
>>>> On 01/22/2016 12:07 PM, Vladimir Sementsov-Ogievskiy wrote:
>>>>> Hi all.
>>>>>
>>>>> This is the early begin of the series which aims to add external
>>>>> backup
>>>>> api. This is needed to allow backup software use our dirty bitmaps.
>>>>>
>>>>> Vmware and Parallels Cloud Server have this feature.
>>>>>
>>>> Have a link to the equivalent feature that VMWare exposes? (Or
>>>> Parallels
>>>> Cloud Server) ... I'm curious about what the API there looks like.
>>> For VMware you need their Virtual Disk Api Programming Guide
>>> http://pubs.vmware.com/vsphere-60/topic/com.vmware.ICbase/PDF/vddk60_programming.pdf
>>>
>>>
>> Great, thanks!
>>
>>> Look at Changed Block Tracking (CBT) , Backup and Restore.
>>>
>>> For PCS here is part of SDK header, related to the topic:
>>>
>>> ====================================
>>> /*
>>>   * Builds a map of the disk contents changes between 2 PITs.
>>>     Parameters
>>>     hDisk :       A handle of type PHT_VIRTUAL_DISK identifying
>>>                   the virtual disk.
>>>     sPit1Uuid :   Uuid of the older PIT.
>>>     sPit2Uuid :   Uuid of the later PIT.
>>>     phMap :       A pointer to a variable which receives the
>>>           result (a handle of type PHT_VIRTUAL_DISK_MAP).
>>>     Returns
>>>     PRL_RESULT.
>>> */
>>> PRL_METHOD_DECL( PARALLELS_API_VER_5,
>>>                  PrlDisk_GetChangesMap_Local, (
>>>          PRL_HANDLE hDisk,
>>>          PRL_CONST_STR sPit1Uuid,
>>>          PRL_CONST_STR sPit2Uuid,
>>>          PRL_HANDLE_PTR phMap) );
>>>
>> Effectively giving you a dirty bitmap diff between two snapshots.
>> Something we don't currently genuinely support in QEMU.
> 
> Just start dirty bitmap at point a and stop at point b..
> 
>>
>>> /*
>>>   * Reports the number of significant bits in the map.
>>>     Parameters
>>>     hMap :        A handle of type PHT_VIRTUAL_DISK_MAP identifying
>>>                   the changes map.
>>>     phSize :      A pointer to a variable which receives the
>>>           result.
>>>     Returns
>>>     PRL_RESULT.
>>> */
>>> PRL_METHOD_DECL( PARALLELS_API_VER_5,
>>>                  PrlDiskMap_GetSize, (
>>>          PRL_HANDLE hMap,
>>>          PRL_UINT32_PTR pnSize) );
>>>
>> I assume this is roughly the dirty bit count, for us, this would be
>> dirty clusters. (Or whatever granularity you specified, but usually
>> clusters.)
>>
>>> /*
>>>   * Reports the size (in bytes) of a block mapped by a single bit
>>>   * in the map.
>>>     Parameters
>>>     hMap :        A handle of type PHT_VIRTUAL_DISK_MAP identifying
>>>                   the changes map.
>>>     phSize :      A pointer to a variable which receives the
>>>           result.
>>>     Returns
>>>     PRL_RESULT.
>>> */
>>> PRL_METHOD_DECL( PARALLELS_API_VER_5,
>>>                  PrlDiskMap_GetGranularity, (
>>>          PRL_HANDLE hMap,
>>>          PRL_UINT32_PTR pnSize) );
>>>
>> Basically a granularity query.
>>
>>> /*
>>>   * Returns bits from the blocks map.
>>>     Parameters
>>>     hMap :        A handle of type PHT_VIRTUAL_DISK_MAP identifying
>>>                   the changes map.
>>>     pBuffer :     A pointer to a store.
>>>     pnCapacity :  A pointer to a variable holding the size
>>>           of the buffer and receiving the number of
>>>           bytes actually written.
>>>     Returns
>>>     PRL_RESULT.
>>> */
>>> PRL_METHOD_DECL( PARALLELS_API_VER_5,
>>>                  PrlDiskMap_Read, (
>>>          PRL_HANDLE hMap,
>>>          PRL_VOID_PTR pBuffer,
>>>          PRL_UINT32_PTR pnCapacity) );
>>>
>> And this would be a direct bitmap query.
>>
>> Is the expected usage here that the third party client will use this
>> bitmap to read the source image? Or do you query for the data from API?
> 
> - from API.
> 
>>
>> I think the thought among block devs would be to opt for more of the
>> second option, and less allowing clients to directly interface with the
>> image files.
>>
>>> =======================================
>>>
>>>
>>>>> There is only one patch here, about querying dirty bitmap from qemu by
>>>>> qmp command. It is just an updated and clipped (hmp command
>>>>> removed) old
>>>>> my patch "[PATCH RFC v3 01/14] qmp: add query-block-dirty-bitmap".
>>>>>
>>>>> Before writing the whole thing I'd like to discuss the details. Or,
>>>>> may
>>>>> be there are existing plans on this topic, or may be someone already
>>>>> works on it?
>>>>>
>>>>> I see it like this:
>>>>>
>>>>> =====
>>>>>
>>>>> - add qmp commands for dirty-bitmap functions: create_successor,
>>>>> abdicate,
>>>>> reclaime.
>>>> Hm, why do we need such low-level control over splitting and merging
>>>> bitmaps from an external client?
>>>>
>>>>> - make create-successor command transaction-able
>>>>> - add query-block-dirty-bitmap qmp command
>>>>>
>>>>> then, external backup:
>>>>>
>>>>> qmp transaction {
>>>>>       external-snapshot
>>>>>       bitmap-create-successor
>>>>> }
>>>>>
>>>>> qmp query frozen bitmap, not acquiring aio context.
>>>>>
>>>>> do external backup, using snapshot and bitmap
>>>>>
>>>>> if (success backup)
>>>>>       qmp bitmap-abdicate
>>>>> else
>>>>>       qmp bitmap-reclaime
>>>>>
>>>>> qmp merge snapshot
>>>>> =====
>>>>>
>>>> Hm, I see -- so you're hoping to manage the backup *entirely*
>>>> externally, so you want to be able to reach inside of QEMU and control
>>>> some status conditions to guarantee it'll be safe.
>>>>
>>>> I'm not convinced QEMU can guarantee such things -- due to various
>>>> flush
>>>> properties, race conditions on write, etc. QEMU handles all of this
>>>> internally in a non-public way at the moment.
>>> Hm, can you be more concrete? What operations are dangerous? We can do
>>> them in paused state for example.
>>>
>> I suppose if you're going to pause the VM, then it should be reasonably
>> safe, but recently there have been endeavors to augment the .qcow2
>> format to prohibit concurrent access, which might include a paused VM as
>> well, I'm not clear on the implementation.
>>
>> If you do it via paused only, then you also don't need to expose the
>> freeze/rollback mechanisms: the existing clear mechanism alone is
>> sufficient:
>>
>> (A) The frozen backup fails. Nothing new has been written, so we don't
>> need to adjust anything, we can just try again.
>> (B) The frozen backup succeeds. We can just clear the bitmap before
>> unfreezing.
> 
> We can't query bitmap in paused state - it may take too much time.
> 

And I think it is currently unsafe to fetch the data from disk while the
VM is running, so you'll have to solve one or the other problem...

>>
>> I definitely have reservations about using this as a live fleecing
>> mechanism -- the backup block job uses a write-notifier to make
>> just-in-time backups of data before it is altered, leaving it the only
>> "safe" live backup mechanism in QEMU currently. (Alongside mirror.)
>>
>> I actually have some patches from Fam to introduce a live fleecing
>> mechanism into QEMU (The idea being you create a point-in-time drive you
>> can get data from via NBD, then delete it when done) that might be more
>> appropriate, but I ran into a lot of problems with the patch. I'll post
>> the WIP for that patch to try to solicit comments on the best way
>> forward.
> 
> After adding
> =============
> --- a/block.c
> +++ b/block.c
> @@ -1276,6 +1276,9 @@ void bdrv_set_backing_hd(BlockDriverState *bs,
> BlockDriverState *backing_hd)
>      /* Otherwise we won't be able to commit due to check in bdrv_commit */
>      bdrv_op_unblock(backing_hd, BLOCK_OP_TYPE_COMMIT_TARGET,
>                      bs->backing_blocker);
> +
> +    bdrv_op_unblock(backing_hd, BLOCK_OP_TYPE_BACKUP_SOURCE,
> +                    bs->backing_blocker);
>  out:
>      bdrv_refresh_limits(bs, NULL);
>  }
> ==============
> and tiny fix for qemu_io interface in iotest
> 
> Fam's "qemu-iotests: Image fleecing test case 089" works for me. Isn't
> it enough?
> 
> 
> 
>>
>> Otherwise, My biggest question here is:
>> "What does fleecing a backup externally provide as a benefit over
>> backing up to an NBD target?"
> 
> Look at our answers on v2 of these series:
> 
> On 05.02.2016 11:28, Denis V. Lunev wrote:
>> On 02/03/2016 11:14 AM, Fam Zheng wrote:
>>> On Sat, 01/30 13:56, Vladimir Sementsov-Ogievskiy wrote:
>>>> Hi all.
>>>>
>>>> These series which aims to add external backup api. This is needed
>>>> to allow
>>>> backup software use our dirty bitmaps.
>>>>
>>>> Vmware and Parallels Cloud Server have this feature.
>>> What is the advantage of this appraoch over "drive-backup
>>> sync=incremental
>>> ..."?
>>
>> This will allow third-party vendors to backup QEMU VMs into
>> their own formats or to the cloud etc.
> 
> 
>>
>> You can already today perform incremental backups to an NBD target to
>> copy the data out via an external mechanism, is this not sufficient for
>> Parallels? If not, why?
>>
>>>>> In the following patch query-bitmap acquires aio context. This must be
>>>>> ofcourse dropped for frozen bitmap.
>>>>> But to make it in true way, I think, I should check somehow that
>>>>> this is
>>>>> not just frozen bitmap, but the bitmap frozen by qmp command, to avoid
>>>>> incorrect quering of bitmap frozen by internal backup (or other
>>>>> mechanizm).. May be, it is not necessary.
>>>>>
>>>>>
>>>>>
>>>>
> 
>