Re: [Qemu-devel] [PATCH for-2.12 0/4] qmp dirty bitmap API

[Nikolay Shirokovskiy]

On 20.12.2017 04:06, John Snow wrote:
> 
> 
> On 12/19/2017 11:07 AM, Vladimir Sementsov-Ogievskiy wrote:
>> 13.12.2017 07:12, Fam Zheng wrote:
>>> On Mon, 11/13 19:20, Vladimir Sementsov-Ogievskiy wrote:
>>>> Hi all.
>>>>
>>>> There are three qmp commands, needed to implement external backup API.
>>>>
>>>> Using these three commands, client may do all needed bitmap
>>>> management by
>>>> hand:
>>>>
>>>> on backup start we need to do a transaction:
>>>>   {disable old bitmap, create new bitmap}
>>>>
>>>> on backup success:
>>>>   drop old bitmap
>>>>
>>>> on backup fail:
>>>>   enable old bitmap
>>>>   merge new bitmap to old bitmap
>>>>   drop new bitmap
>>>>
>>>> Question: it may be better to make one command instead of two:
>>>> block-dirty-bitmap-set-enabled(bool enabled)
>>>>
>>>> Vladimir Sementsov-Ogievskiy (4):
>>>>    block/dirty-bitmap: add lock to bdrv_enable/disable_dirty_bitmap
>>>>    qapi: add block-dirty-bitmap-enable/disable
>>>>    qmp: transaction support for block-dirty-bitmap-enable/disable
>>>>    qapi: add block-dirty-bitmap-merge
>>>>
>>>>   qapi/block-core.json         |  80 +++++++++++++++++++++++
>>>>   qapi/transaction.json        |   4 ++
>>>>   include/block/dirty-bitmap.h |   2 +
>>>>   block/dirty-bitmap.c         |  21 ++++++
>>>>   blockdev.c                   | 151
>>>> +++++++++++++++++++++++++++++++++++++++++++
>>>>   5 files changed, 258 insertions(+)
>>>>
>>> I think tests are required to merge new features/commands.  Can we
>>> include tests
>>> on these new code please?  We should cover error handling, and also
>>> write tests
>>> that demonstrate the intended real world use cases.
>>>
>>> Also should we add new sections to docs/interop/bitmaps.rst?
>>>
>>> Meta: John started a long discussion about the API design but I think
>>> after all
>>> it turns out exposing dirty bitmap objects and the primitives is a
>>> reasonable
>>> approach to implement incremental backup functionalities. The comment
>>> I have is
>>> that we should ensure we have also reviewed it from a higher level
>>> (e.g. all the
>>> potential user requirements) to make sure this low level API is both
>>> sound and
>>> flexible. We shouldn't introduce a minimal set of low level commands
>>> just to
>>> support one particular use case, but look a bit further and broader
>>> and come up
>>> with a more complete design? Writing docs and tests might force us to
>>> think in
>>> this direction, which I think is a good thing to have for this series,
>>> too.
>>>
>>> Fam
>>
>> Nikolay, please describe what do you plan in libvirt over qmp bitmap API.
>>
>> Kirill, what do you think about this all?
>>
>> (brief history:
>> we are considering 3 new qmp commands for bitmap management, needed for
>> external incremental backup support
>>  - enable (bitmap will track disk changes)
>>  - disable (bitmap will stop tracking changes)
>>  - merge (merge bitmap A to bitmap B)
>> )
>>
> 
> Yeah, it would be helpful to know what the full workflow for the API
> will be ... before I get ahead of myself again (sorry) ...
> 
> but I'd like to see a quick writeup of your vision for the pull-mode
> backups (which I assume this is for, right?) from start-to-finish, like
> a mockup of annotated QMP output or something.
> 
> Nothing fancy, just something that lets me orient where we're headed,
> since you're doing most of the work and I just want to get out of your
> way, but having a roadmap helps me do that.
> 
> --js
> 

Hi, all.

In terms of API we want to be able to create an incremental backup
from any previous backup. But as in qemu it does cost a bitmap for every
point in time we want to do incremental backup from we plan to keep
only limited number of such checkpoints. Like last N checkpoints I guess.
So there will be an API to delete checkpoint as well.

In terms of implementations Vladimir already described usage of disable/enable/
merge commands in [1] for both creation and deletion API. 

There is also a RFC [2] in libvirt list which described in more detail
libvirt API.

[1] http://lists.nongnu.org/archive/html/qemu-devel/2017-12/msg01099.html
[2] https://www.redhat.com/archives/libvir-list/2017-November/msg00514.html