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