diff mbox

[2/6] qmp: add query-block-dirty-bitmap-ranges

Message ID 1454151394-52320-3-git-send-email-vsementsov@virtuozzo.com (mailing list archive)
State New, archived
Headers show

Commit Message

Vladimir Sementsov-Ogievskiy Jan. 30, 2016, 10:56 a.m. UTC
Add qmp command to query dirty bitmap contents. This is needed for
external backup.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 block/dirty-bitmap.c         | 55 +++++++++++++++++++++++++++++++++++++++
 blockdev.c                   | 62 ++++++++++++++++++++++++++++++++++++++++++++
 include/block/dirty-bitmap.h |  7 +++++
 qapi/block-core.json         | 54 ++++++++++++++++++++++++++++++++++++++
 qmp-commands.hx              | 33 +++++++++++++++++++++++
 5 files changed, 211 insertions(+)

Comments

Stefan Hajnoczi Feb. 10, 2016, 10:08 a.m. UTC | #1
On Sat, Jan 30, 2016 at 01:56:30PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> Add qmp command to query dirty bitmap contents. This is needed for
> external backup.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  block/dirty-bitmap.c         | 55 +++++++++++++++++++++++++++++++++++++++
>  blockdev.c                   | 62 ++++++++++++++++++++++++++++++++++++++++++++
>  include/block/dirty-bitmap.h |  7 +++++
>  qapi/block-core.json         | 54 ++++++++++++++++++++++++++++++++++++++
>  qmp-commands.hx              | 33 +++++++++++++++++++++++
>  5 files changed, 211 insertions(+)

This API produces large replies and/or requires many calls to fetch all
bitmap data.  The worst case is a 101010... bitmap.

I consider the dirty bitmap to be data (vs control) and not something
that should be sent over a control channel like the QMP monitor.

How about writing the dirty bitmap to a file?  The new bitmap file
format that Fam is working on could be used.  That way the dirty bitmap
can be saved asynchronously without hogging the QMP monitor.
Denis V. Lunev Feb. 10, 2016, 1:57 p.m. UTC | #2
On 02/10/2016 01:08 PM, Stefan Hajnoczi wrote:
> On Sat, Jan 30, 2016 at 01:56:30PM +0300, Vladimir Sementsov-Ogievskiy wrote:
>> Add qmp command to query dirty bitmap contents. This is needed for
>> external backup.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>>   block/dirty-bitmap.c         | 55 +++++++++++++++++++++++++++++++++++++++
>>   blockdev.c                   | 62 ++++++++++++++++++++++++++++++++++++++++++++
>>   include/block/dirty-bitmap.h |  7 +++++
>>   qapi/block-core.json         | 54 ++++++++++++++++++++++++++++++++++++++
>>   qmp-commands.hx              | 33 +++++++++++++++++++++++
>>   5 files changed, 211 insertions(+)
> This API produces large replies and/or requires many calls to fetch all
> bitmap data.  The worst case is a 101010... bitmap.
>
> I consider the dirty bitmap to be data (vs control) and not something
> that should be sent over a control channel like the QMP monitor.
>
> How about writing the dirty bitmap to a file?  The new bitmap file
> format that Fam is working on could be used.  That way the dirty bitmap
> can be saved asynchronously without hogging the QMP monitor.
Reasonable point.

May be it would be better to setup "special" NBD server inside
QEMU which will allow to directly "read" bitmap data.

Any opinion?

Den
John Snow Feb. 10, 2016, 3:26 p.m. UTC | #3
On 02/10/2016 08:57 AM, Denis V. Lunev wrote:
> On 02/10/2016 01:08 PM, Stefan Hajnoczi wrote:
>> On Sat, Jan 30, 2016 at 01:56:30PM +0300, Vladimir Sementsov-Ogievskiy
>> wrote:
>>> Add qmp command to query dirty bitmap contents. This is needed for
>>> external backup.
>>>
>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>> ---
>>>   block/dirty-bitmap.c         | 55
>>> +++++++++++++++++++++++++++++++++++++++
>>>   blockdev.c                   | 62
>>> ++++++++++++++++++++++++++++++++++++++++++++
>>>   include/block/dirty-bitmap.h |  7 +++++
>>>   qapi/block-core.json         | 54
>>> ++++++++++++++++++++++++++++++++++++++
>>>   qmp-commands.hx              | 33 +++++++++++++++++++++++
>>>   5 files changed, 211 insertions(+)
>> This API produces large replies and/or requires many calls to fetch all
>> bitmap data.  The worst case is a 101010... bitmap.
>>
>> I consider the dirty bitmap to be data (vs control) and not something
>> that should be sent over a control channel like the QMP monitor.
>>
>> How about writing the dirty bitmap to a file?  The new bitmap file
>> format that Fam is working on could be used.  That way the dirty bitmap
>> can be saved asynchronously without hogging the QMP monitor.
> Reasonable point.
> 
> May be it would be better to setup "special" NBD server inside
> QEMU which will allow to directly "read" bitmap data.
> 
> Any opinion?
> 
> Den

Or perhaps something like migration, where the client receiving the data
opens a socket of some sort, and QEMU connects to that socket to send
the data.
Denis V. Lunev Feb. 10, 2016, 3:36 p.m. UTC | #4
On 02/10/2016 06:26 PM, John Snow wrote:
>
> On 02/10/2016 08:57 AM, Denis V. Lunev wrote:
>> On 02/10/2016 01:08 PM, Stefan Hajnoczi wrote:
>>> On Sat, Jan 30, 2016 at 01:56:30PM +0300, Vladimir Sementsov-Ogievskiy
>>> wrote:
>>>> Add qmp command to query dirty bitmap contents. This is needed for
>>>> external backup.
>>>>
>>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>>> ---
>>>>    block/dirty-bitmap.c         | 55
>>>> +++++++++++++++++++++++++++++++++++++++
>>>>    blockdev.c                   | 62
>>>> ++++++++++++++++++++++++++++++++++++++++++++
>>>>    include/block/dirty-bitmap.h |  7 +++++
>>>>    qapi/block-core.json         | 54
>>>> ++++++++++++++++++++++++++++++++++++++
>>>>    qmp-commands.hx              | 33 +++++++++++++++++++++++
>>>>    5 files changed, 211 insertions(+)
>>> This API produces large replies and/or requires many calls to fetch all
>>> bitmap data.  The worst case is a 101010... bitmap.
>>>
>>> I consider the dirty bitmap to be data (vs control) and not something
>>> that should be sent over a control channel like the QMP monitor.
>>>
>>> How about writing the dirty bitmap to a file?  The new bitmap file
>>> format that Fam is working on could be used.  That way the dirty bitmap
>>> can be saved asynchronously without hogging the QMP monitor.
>> Reasonable point.
>>
>> May be it would be better to setup "special" NBD server inside
>> QEMU which will allow to directly "read" bitmap data.
>>
>> Any opinion?
>>
>> Den
> Or perhaps something like migration, where the client receiving the data
> opens a socket of some sort, and QEMU connects to that socket to send
> the data.
no. The point is that QEMU should be queried for data.
May be even via several sockets to provide it in
parallel.

Den
John Snow Feb. 10, 2016, 3:37 p.m. UTC | #5
On 02/10/2016 10:36 AM, Denis V. Lunev wrote:
> On 02/10/2016 06:26 PM, John Snow wrote:
>>
>> On 02/10/2016 08:57 AM, Denis V. Lunev wrote:
>>> On 02/10/2016 01:08 PM, Stefan Hajnoczi wrote:
>>>> On Sat, Jan 30, 2016 at 01:56:30PM +0300, Vladimir Sementsov-Ogievskiy
>>>> wrote:
>>>>> Add qmp command to query dirty bitmap contents. This is needed for
>>>>> external backup.
>>>>>
>>>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>>>> ---
>>>>>    block/dirty-bitmap.c         | 55
>>>>> +++++++++++++++++++++++++++++++++++++++
>>>>>    blockdev.c                   | 62
>>>>> ++++++++++++++++++++++++++++++++++++++++++++
>>>>>    include/block/dirty-bitmap.h |  7 +++++
>>>>>    qapi/block-core.json         | 54
>>>>> ++++++++++++++++++++++++++++++++++++++
>>>>>    qmp-commands.hx              | 33 +++++++++++++++++++++++
>>>>>    5 files changed, 211 insertions(+)
>>>> This API produces large replies and/or requires many calls to fetch all
>>>> bitmap data.  The worst case is a 101010... bitmap.
>>>>
>>>> I consider the dirty bitmap to be data (vs control) and not something
>>>> that should be sent over a control channel like the QMP monitor.
>>>>
>>>> How about writing the dirty bitmap to a file?  The new bitmap file
>>>> format that Fam is working on could be used.  That way the dirty bitmap
>>>> can be saved asynchronously without hogging the QMP monitor.
>>> Reasonable point.
>>>
>>> May be it would be better to setup "special" NBD server inside
>>> QEMU which will allow to directly "read" bitmap data.
>>>
>>> Any opinion?
>>>
>>> Den
>> Or perhaps something like migration, where the client receiving the data
>> opens a socket of some sort, and QEMU connects to that socket to send
>> the data.
> no. The point is that QEMU should be queried for data.
> May be even via several sockets to provide it in
> parallel.
> 
> Den

I don't follow.

You'd use a QMP command to tell QEMU where to connect to send the data.
You're still "querying" QEMU, it's just not acting as the server for the
data channel.
Denis V. Lunev Feb. 10, 2016, 3:40 p.m. UTC | #6
On 02/10/2016 06:37 PM, John Snow wrote:
>
> On 02/10/2016 10:36 AM, Denis V. Lunev wrote:
>> On 02/10/2016 06:26 PM, John Snow wrote:
>>> On 02/10/2016 08:57 AM, Denis V. Lunev wrote:
>>>> On 02/10/2016 01:08 PM, Stefan Hajnoczi wrote:
>>>>> On Sat, Jan 30, 2016 at 01:56:30PM +0300, Vladimir Sementsov-Ogievskiy
>>>>> wrote:
>>>>>> Add qmp command to query dirty bitmap contents. This is needed for
>>>>>> external backup.
>>>>>>
>>>>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>>>>> ---
>>>>>>     block/dirty-bitmap.c         | 55
>>>>>> +++++++++++++++++++++++++++++++++++++++
>>>>>>     blockdev.c                   | 62
>>>>>> ++++++++++++++++++++++++++++++++++++++++++++
>>>>>>     include/block/dirty-bitmap.h |  7 +++++
>>>>>>     qapi/block-core.json         | 54
>>>>>> ++++++++++++++++++++++++++++++++++++++
>>>>>>     qmp-commands.hx              | 33 +++++++++++++++++++++++
>>>>>>     5 files changed, 211 insertions(+)
>>>>> This API produces large replies and/or requires many calls to fetch all
>>>>> bitmap data.  The worst case is a 101010... bitmap.
>>>>>
>>>>> I consider the dirty bitmap to be data (vs control) and not something
>>>>> that should be sent over a control channel like the QMP monitor.
>>>>>
>>>>> How about writing the dirty bitmap to a file?  The new bitmap file
>>>>> format that Fam is working on could be used.  That way the dirty bitmap
>>>>> can be saved asynchronously without hogging the QMP monitor.
>>>> Reasonable point.
>>>>
>>>> May be it would be better to setup "special" NBD server inside
>>>> QEMU which will allow to directly "read" bitmap data.
>>>>
>>>> Any opinion?
>>>>
>>>> Den
>>> Or perhaps something like migration, where the client receiving the data
>>> opens a socket of some sort, and QEMU connects to that socket to send
>>> the data.
>> no. The point is that QEMU should be queried for data.
>> May be even via several sockets to provide it in
>> parallel.
>>
>> Den
> I don't follow.
>
> You'd use a QMP command to tell QEMU where to connect to send the data.
> You're still "querying" QEMU, it's just not acting as the server for the
> data channel.
ok, I have understood you wrong. This looks working
at the first glance.
Fam Zheng Feb. 14, 2016, 5:05 a.m. UTC | #7
On Wed, 02/10 16:57, Denis V. Lunev wrote:
> On 02/10/2016 01:08 PM, Stefan Hajnoczi wrote:
> >On Sat, Jan 30, 2016 at 01:56:30PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> >>Add qmp command to query dirty bitmap contents. This is needed for
> >>external backup.
> >>
> >>Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> >>---
> >>  block/dirty-bitmap.c         | 55 +++++++++++++++++++++++++++++++++++++++
> >>  blockdev.c                   | 62 ++++++++++++++++++++++++++++++++++++++++++++
> >>  include/block/dirty-bitmap.h |  7 +++++
> >>  qapi/block-core.json         | 54 ++++++++++++++++++++++++++++++++++++++
> >>  qmp-commands.hx              | 33 +++++++++++++++++++++++
> >>  5 files changed, 211 insertions(+)
> >This API produces large replies and/or requires many calls to fetch all
> >bitmap data.  The worst case is a 101010... bitmap.
> >
> >I consider the dirty bitmap to be data (vs control) and not something
> >that should be sent over a control channel like the QMP monitor.
> >
> >How about writing the dirty bitmap to a file?  The new bitmap file
> >format that Fam is working on could be used.  That way the dirty bitmap
> >can be saved asynchronously without hogging the QMP monitor.
> Reasonable point.
> 
> May be it would be better to setup "special" NBD server inside
> QEMU which will allow to directly "read" bitmap data.
> 
> Any opinion?

Since Stefan has mentioned "the format" I'm working on, yes, I think it will be
possible to expose the persistent bitmap through NBD if the driver assigns
node-names to the bitmap BDS.

Let me prototype this on top of my branch.

Fam
diff mbox

Patch

diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index 6da27d9..4c108e4 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -495,3 +495,58 @@  int64_t bdrv_dirty_bitmap_next_zero(BdrvDirtyBitmap *bitmap, uint64_t start)
 {
     return hbitmap_next_zero(bitmap->bitmap, start);
 }
+
+/* bdrv_query_dirty_bitmap_ranges
+ * start, count and resulting ranges are all in bytes
+ */
+BlockDirtyRangeList *bdrv_query_dirty_bitmap_ranges(BlockDriverState *bs,
+        BdrvDirtyBitmap *bitmap, uint64_t start, uint64_t count,
+        uint32_t max_ranges)
+{
+    BlockDirtyRangeList *list = NULL, *last_entry = NULL;
+    BlockDirtyRangeList **plist = &list;
+    uint64_t begin, end;
+    uint64_t start_sector = start >> BDRV_SECTOR_BITS;
+    uint64_t last_sector = (start + count - 1) >> BDRV_SECTOR_BITS;
+    uint32_t nb_ranges = 0;
+
+    BdrvDirtyBitmapIter *it = bdrv_dirty_iter_new(bitmap, start_sector);
+
+    while ((begin = bdrv_dirty_iter_next(it)) != -1 && nb_ranges < max_ranges) {
+        BlockDirtyRange *region;
+        if (begin > last_sector) {
+            break;
+        }
+
+        end = bdrv_dirty_bitmap_next_zero(bitmap, begin + 1);
+
+        region = g_new0(BlockDirtyRange, 1);
+        region->start = begin << BDRV_SECTOR_BITS;
+        region->count = (end - begin) << BDRV_SECTOR_BITS;
+
+        last_entry = g_new0(BlockDirtyRangeList, 1);
+        last_entry->value = region;
+
+        *plist = last_entry;
+        plist = &last_entry->next;
+        nb_ranges++;
+
+        bdrv_set_dirty_iter(it, end + 1);
+    }
+
+    bdrv_dirty_iter_free(it);
+
+    if (list != NULL) {
+        if (list->value->start < start) {
+            list->value->count -= start - list->value->start;
+            list->value->start = start;
+        }
+
+        if (last_entry->value->start +
+            last_entry->value->count > start + count) {
+            last_entry->value->count = start + count - last_entry->value->start;
+        }
+    }
+
+    return list;
+}
diff --git a/blockdev.c b/blockdev.c
index 1392fff..fa34cf6 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2733,6 +2733,68 @@  void qmp_block_dirty_bitmap_clear(const char *node, const char *name,
     aio_context_release(aio_context);
 }
 
+BlockDirtyRangeList *qmp_query_block_dirty_bitmap_ranges(const char *node,
+        const char *name, bool has_start, uint64_t start, bool has_count,
+        uint64_t count, uint32_t max_ranges, bool has_block_io, bool block_io,
+        Error **errp)
+{
+    AioContext *aio_context;
+    BdrvDirtyBitmap *bitmap;
+    BlockDriverState *bs;
+    BlockDirtyRangeList *ret = NULL;
+    int64_t disk_size;
+
+    if (!has_block_io) {
+        block_io = true;
+    }
+
+    bitmap = block_dirty_bitmap_lookup(node, name, &bs,
+                                       block_io ? &aio_context : NULL,
+                                       errp);
+    if (!bitmap) {
+        return NULL;
+    }
+
+    if (!block_io && bdrv_dirty_bitmap_enabled(bitmap)) {
+        error_setg(errp, "The bitmap is enabled, block-io must be on");
+        return NULL;
+    }
+
+    disk_size = bdrv_getlength(bs);
+    if (disk_size < 0) {
+        error_setg_errno(errp, -disk_size, "bdrv_getlength failed");
+        goto out;
+    }
+
+    if (has_start) {
+        if (start >= disk_size) {
+            error_setg(errp, "Start must be less than disk size");
+            goto out;
+        }
+    } else {
+        start = 0;
+    }
+
+    if (has_count) {
+        if (start + count > disk_size) {
+            error_setg(errp,
+                       "(start + count) must not be greater than disk size");
+            goto out;
+        }
+    } else {
+        count = disk_size - start;
+    }
+
+    ret = bdrv_query_dirty_bitmap_ranges(bs, bitmap, start, count, max_ranges);
+
+out:
+    if (block_io) {
+        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/dirty-bitmap.h b/include/block/dirty-bitmap.h
index 0033942..7918310 100644
--- a/include/block/dirty-bitmap.h
+++ b/include/block/dirty-bitmap.h
@@ -70,4 +70,11 @@  void bdrv_dirty_bitmap_deserialize_finish(BdrvDirtyBitmap *bitmap);
 
 int64_t bdrv_dirty_bitmap_next_zero(BdrvDirtyBitmap *bitmap, uint64_t start);
 
+/* bdrv_query_dirty_bitmap_ranges
+ * start, count and resulting ranges are all in bytes
+ */
+BlockDirtyRangeList *bdrv_query_dirty_bitmap_ranges(BlockDriverState *bs,
+        BdrvDirtyBitmap *bitmap, uint64_t start, uint64_t count,
+        uint32_t max_ranges);
+
 #endif
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 0a915ed..89bdeaf 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -414,6 +414,60 @@ 
 ##
 { 'command': 'query-block', 'returns': ['BlockInfo'] }
 
+##
+# @BlockDirtyRegion:
+#
+# Region in bytes.
+#
+# @start: first byte
+#
+# @count: number of bytes in the region
+#
+# Since: 2.6
+##
+{ 'struct': 'BlockDirtyRange',
+  'data': { 'start': 'uint64', 'count': 'uint64' } }
+
+##
+# @BlockDirtyBitmapQueryRanges
+#
+# @node: name of device/node which the bitmap is tracking
+#
+# @name: name of the dirty bitmap
+#
+# @start: #optional start of quered range, default is zero for
+#         query-block-dirty-bitmap-ranges
+#
+# @count: #optional lenght of quered range, default is (disk-size - @start)
+#         for query-block-dirty-bitmap-ranges
+#
+# @max-ranges: max dirty ranges to be returned. If there are more than
+#              @max-ranges dirty ranges within quered range, then first
+#              @max-ranges dirty ranges from quered range will be returned.
+#
+# @block-io: #optional wheter aio-context should be acquired while oparating.
+#            False value is allowed only for frozen or disabled bitmaps.
+#            Default is true for query-block-dirty-bitmap-ranges.
+#
+# Since 2.6
+##
+{ 'struct': 'BlockDirtyBitmapQueryRanges',
+  'data': { 'node': 'str', 'name': 'str', '*start': 'uint64',
+            '*count': 'uint64', 'max-ranges': 'uint32', '*block-io': 'bool'} }
+
+##
+# @query-block-dirty-bitmap-ranges
+#
+# Get a description for specified dirty bitmap including it's dirty regions.
+# This command is in general for testing purposes.
+#
+# Returns: @BlockDirtyBitmapInfo
+#
+# Since: 2.6
+##
+{ 'command': 'query-block-dirty-bitmap-ranges',
+  'data': 'BlockDirtyBitmapQueryRanges',
+  'returns': ['BlockDirtyRange'] }
 
 ##
 # @BlockDeviceTimedStats:
diff --git a/qmp-commands.hx b/qmp-commands.hx
index db072a6..02de44c 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1457,6 +1457,39 @@  Example:
 EQMP
 
     {
+        .name       = "query-block-dirty-bitmap-ranges",
+        .args_type  = "node:B,name:s,start:i?,count:i?,max-ranges:i,"
+                      "block-io:b?",
+        .mhandler.cmd_new = qmp_marshal_query_block_dirty_bitmap_ranges,
+    },
+
+SQMP
+
+query-block-dirty-bitmap-ranges
+------------------------
+Since 2.6
+
+Get contens of specified dirty bitmap. Bitmap data is returned as array of
+dirty ranges.
+
+Arguments:
+
+- "node": device/node on which to remove dirty bitmap (json-string)
+- "name": name of the dirty bitmap (json-string)
+- "start": start of quered range in bytes (json-int, optional, default 0)
+- "count": lenght of quered range in bytes
+           (json-int, optional, default (disk-size - @start))
+- "max-ranges": max dirty ranges to be returned. If there are more than
+                @max-ranges dirty ranges within quered range, then first
+                @max-ranges dirty ranges from quered range will be returned.
+                (json-int)
+- "block-io": wheter aio-context should be acquired while oparating. False
+              value is allowed only for frozen or disabled bitmaps.
+              (json-bool, optional, default true)
+
+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,