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

[Vladimir Sementsov-Ogievskiy]

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

external backup:

0. we have active_disk and attached to it dirty bitmap bitmap0
1. qmp blockdev-add tmp_disk (backing=active_disk)
2. guest fsfreeze
3. qmp transaction:
        - block-dirty-bitmap-add node=active_disk name=bitmap1
        - block-dirty-bitmap-disable node=active_disk name=bitmap0
        - blockdev-backup drive=active_disk target=tmp_disk sync=none
4. guest fsthaw
5. (? not designed yet) qmp blockdev-add filter_node - special filter 
node over tmp_disk for synchronization of nbd-reads and 
backup(sync=none) cow requests (like it is done in block/replication)
6. qmp nbd-server-start
7. qmp nbd-server-add filter_node (there should be possibility of 
exporting bitmap of child node filter_node->tmp_disk->active_disk->bitmap0)

then, external tool can connect to nbd server and get exported bitmap 
and read data (including bitmap0) accordingly to nbd specification.
(also, external tool may get a merge of several bitmaps, if we already 
have a sequence of them)
then, after backup finish, what can be done:

1. qmp block-job-cancel device=active_disk (stop our backup(sync=none))
2. qmp nbd-server-stop (or qmp nbd-server-remove filter_node)
3. qmp blockdev-remove filter_node
4. qmp blockdev-remove tmp_disk

on successful backup, you can drop old bitmap if you want (or do not 
drop it, if you need to keep sequence of disabled bitmaps):
1. block-dirty-bitmap-remove node=active_disk name=bitmap0

on failed backup, you can merge bitmaps, to make it look like nothing 
happened:
1. qmp transaction:
       - block-dirty-bitmap-merge node=active_disk name-source=bitmap1 
name-target=bitmap0
       - block-dirty-bitmap-remove node=active_disk name=bitmap1
       - block-dirty-bitmap-enable node=active_disk name=bitmap0

--
Best regards,
Vladimir