Re: [Qemu-block] [PATCH v5 5/6] qapi: new qmp command nbd-server-add-bitmap

[Nikolay Shirokovskiy]

On 20.06.2018 17:13, Vladimir Sementsov-Ogievskiy wrote:
> 20.06.2018 14:26, Eric Blake wrote:
>> On 06/09/2018 10:17 AM, Vladimir Sementsov-Ogievskiy wrote:
>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
>>> ---
>>>   qapi/block.json | 23 +++++++++++++++++++++++
>>>   blockdev-nbd.c  | 23 +++++++++++++++++++++++
>>>   2 files changed, 46 insertions(+)
>>
>> I'm tempted to temporarily name this x-nbd-server-add-bitmap, until I have 
>> the counterpart Libvirt patches tested, just in case testing turns up any 
>> tweaks we need to the interface.
>>
>>>
>>> diff --git a/qapi/block.json b/qapi/block.json
>>> index c694524002..ddbca2e286 100644
>>> --- a/qapi/block.json
>>> +++ b/qapi/block.json
>>> @@ -269,6 +269,29 @@
>>>     'data': {'name': 'str', '*mode': 'NbdServerRemoveMode'} }
>>>     ##
>>> +# @nbd-server-add-bitmap:
>>> +#
>>> +# Expose a dirty bitmap associated with the selected export. The bitmap 
>>> search
>>> +# starts at the device attached to the export, and includes all backing 
>>> files.
>>> +# The exported bitmap is then locked until the NBD export is removed.
>>
>> The fact that you search the backing chain is at least consistent with your 
>> code.
>>
>>> +#
>>> +# @name: Export name.
>>> +#
>>> +# @bitmap: Bitmap name to search for.
>>> +#
>>> +# @bitmap-export-name: How the bitmap will be seen by nbd clients
>>> +#                      (default @bitmap)
>>
>> Do we really need the flexibility of naming the bitmap differently to the 
>> NBD client than we do in qemu?
> 
> It was needed for our backup-api implementation. Nikolay?

Hi.

In case of checkpoints A, B, C in my implementation I created bitmaps A, B, C
respectively with A tracking changes from time A to time B for example. Later
if somebody needs to export CBT from A to last snapshot(checkpotin) I need to
merge A and B and create T bitmap for this purpuse. NBD client would like
to see this CBT under name A I guess thus I used @bitmap-export-name option.

But I could start with naming bitmaps delta-A, delta-B, delta-C and then
use name A for merge.

Nikolay

> 
>>
>>> +#
>>> +# Note: the client must use NBD_OPT_SET_META_CONTEXT with a query of
>>> +# "qemu:dirty-bitmap:NAME" (where NAME matches @bitmap-export-name) to 
>>> access
>>> +# the exposed bitmap.
>>> +#
>>> +# Since: 3.0
>>> +##
>>> +  { 'command': 'nbd-server-add-bitmap',
>>> +    'data': {'name': 'str', 'bitmap': 'str', '*bitmap-export-name': 'str'} 
>>> }
>>> +
>>> +##
>>>   # @nbd-server-stop:
>>>   #
>>>   # Stop QEMU's embedded NBD server, and unregister all devices previously
>>> diff --git a/blockdev-nbd.c b/blockdev-nbd.c
>>> index 65a84739ed..6b0c50732c 100644
>>> --- a/blockdev-nbd.c
>>> +++ b/blockdev-nbd.c
>>> @@ -220,3 +220,26 @@ void qmp_nbd_server_stop(Error **errp)
>>>       nbd_server_free(nbd_server);
>>>       nbd_server = NULL;
>>>   }
>>> +
>>> +void qmp_nbd_server_add_bitmap(const char *name, const char *bitmap,
>>> +                               bool has_bitmap_export_name,
>>> +                               const char *bitmap_export_name,
>>> +                               Error **errp)
>>> +{
>>> +    NBDExport *exp;
>>> +
>>> +    if (!nbd_server) {
>>> +        error_setg(errp, "NBD server not running");
>>> +        return;
>>> +    }
>>> +
>>> +    exp = nbd_export_find(name);
>>> +    if (exp == NULL) {
>>> +        error_setg(errp, "Export '%s' is not found", name);
>>> +        return;
>>> +    }
>>> +
>>> +    nbd_export_bitmap(exp, bitmap,
>>> +                      has_bitmap_export_name ? bitmap_export_name : bitmap,
>>> +                      errp);
>>> +}
>>>
>>
> 
>