[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Vladimir Sementsov-Ogievskiy
Subject: Re: [Qemu-block] [Qemu-devel] [PATCH for-2.12 0/4] qmp dirty bitmap API
Date: Tue, 19 Dec 2017 19:07:29 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.0

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

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.


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)

Best regards,

reply via email to

[Prev in Thread] Current Thread [Next in Thread]