qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [PATCH v11 00/13] block: Incremental backup series


From: Fam Zheng
Subject: Re: [Qemu-devel] [PATCH v11 00/13] block: Incremental backup series
Date: Tue, 13 Jan 2015 09:21:05 +0800
User-agent: Mutt/1.5.23 (2014-03-12)

On Mon, 01/12 11:30, John Snow wrote:
> Welcome to version 11. I hope you are enjoying our regular newsletter.
> 
> This patchset enables the in-memory part of the incremental backup
> feature. A patchset by Vladimir Sementsov-Ogievskiy enables the
> migration of in-memory dirty bitmaps, and a future patchset will
> enable the storage and retrieval of dirty bitmaps to and from permanent
> storage.
> 
> Enough changes have been made that most Reviewed-By lines from
> previous iterations have been removed. (Sorry!)
> 
> This series was originally authored by Fam Zheng;
> his cover letter is included below.

Speaking of the authorship, if there is v12, I don't mind switching all my
patches to your name, considering how long you've been taken them along since
very early versions (look at the changelog below :).

Fam

> 
> ~John Snow
> 
> =================================================================
> 
> This is the in memory part of the incremental backup feature.
> 
> With the added commands, we can create a bitmap on a block
> backend, from which point of time all the writes are tracked by
> the bitmap, marking sectors as dirty. Later, we call drive-backup
> and pass the bitmap to it, to do an incremental backup.
> 
> See the last patch which adds some tests for this use case.
> 
> Fam
> 
> =================================================================
> 
> For convenience, this patchset is available on github:
>                 https://github.com/jnsnow/qemu/commits/dbm-backup
> 
> v11:
> 
>  - Instead of copying BdrvDirtyBitmaps and keeping a pointer to the
>    object we were copied from, we instead "freeze" a bitmap in-place
>    without copying it. On success, we thaw and delete the bitmap.
>    On failure, we merge the bitmap with a "successor," which is an
>    anonymous bitmap attached as a child that records writes for us
>    for the duration of the backup operation.
> 
>    This means that incremental backups can NEVER BE RETRIED in a
>    deterministic fashion. If an incremental backup fails on a set
>    of dirty sectors {x}, and a new set of dirty sectors {y} are
>    introduced during the backup, then any possible retry action
>    on an incremental backup can only operate on {x,y}. There is no
>    way to get an incremental backup "as it would have been."
> 
>    So, the failure mode for incremental backup is to try again,
>    and the resulting image will simply be a differential from the
>    last successful dirty bitmap backup.
> 
>  - Removed hbitmap_copy and bdrv_dirty_bitmap_copy.
> 
>  - Added a small fixup patch:
>    - Update all granularity fields to be uint64_t.
>    - Update documentation around BdrvDirtyBitmap structure.
> 
>  - Modified transactions to obey frozen attribute of BdrvDirtyBitmaps.
> 
>  - Added frozen attribute to the info query.
> 
> v10 (Overview):
> 
>  - I've included one of Vladimir's bitmap fixes as patch #1.
> 
>  - QMP commands and transactions are now protected via
>    aio_context functions.
> 
>  - QMP commands use "node-ref" as a parameter name now. Reasoning
>    is thus: Either a "device name" or a "node name" can be used to
>    reference a BDS, which is the key item we are actually acting
>    on with these bitmap commands. Thus, I refer to this unified
>    parameter as a "Node Reference," or "node-ref."
> 
>    We could also argue for "backend-ref" or "device-ref" for the
>    reverse semantics: where we accept a unified parameter, but we
>    intend to resolve it to the BlockBackend instead of resolving
>    the parameter given to the BlockDriverState.
> 
>    Or, we could use "reference" for both cases, and to match the
>    existing BlockdevRef command.
> 
>  - All QMP commands added are now per-node via a unified parameter
>    name. Though backup only operates on "devices," you are free to
>    create bitmaps for any arbitrary node you wish. It is obviously
>    only useful for the root node, currently.
> 
>  - Bitmap Sync modes (CONSUME, RESET) are removed. See below
>    (changelog, RFC questions) for more details.
> 
>  - Code to recover the bitmap after a failure has been added,
>    but I have some major questions about drive_backup guarantees.
>    See RFC questions.
> 
> v10 (Detailed Changelog):
> 
>    (1/13) New Patch:
>  - Included Vladimir Sementsov-Ogievskiy's patch that clarifies
>    the semantics of the bitmap primitives.
> 
>    (3/13):
>  - Edited function names for consistency (Stefanha)
>  - Removed compile-time constants (Stefanha)
>  - Acquire aio_context for bitmap add/remove (Stefanha)
>  - Documented optional granularity for bitmap-add in
>    qmp-commands.hx file (Eric Blake)
>  - block_dirty_bitmap_lookup moved forward to this patch to allow
>    more re-use. (Stefanha)
>  - Fixed a problem where the block_dirty_bitmap_lookup didn't
>    always set an error if it returned NULL.
>  - Added an optional return BDS lookup parameter to
>    bdrv_dirty_bitmap_lookup.
>  - Renamed "device" to "node-ref" for block_dirty_bitmap_lookup,
>    adjusted calls to bdrv_lookup_bs() to reflect unified
>    parameter usage.
>  - qmp_block_dirty_bitmap_{add,remove} now reference arbitrary
>    node names via @node-ref.
> 
>    (4/13):
>  - Default granularity and granularity getters are both
>    now uint64_t (Stefanha)
> 
>    (5/13):
>  - Added documentation to warn about the necessity of updating
>    the hbitmap deep copy. (Stefanha)
> 
>    (6/13)
>  - Renamed bdrv_reset_dirty_bitmap to bdrv_clear_dirty_bitmap
>    to be consistent with Vladimir's patches.
>  - Removed const qualifier for bdrv_copy_dirty_bitmap,
>    to accommodate patch 8.
> 
>    (7/13) New Patch:
>  - Added an hbitmap_merge operation to join two bitmaps together.
> 
>    (8/13) New Patch:
>  - Added bdrv_reclaim_dirty_bitmap() to allow us to "roll back" a
>    bitmap into the bitmap that it was spawned from. This will let
>    us maintain an accurate account of dirty sectors even after a
>    failure.
>  - This adds an "originator" pointer to the BdrvDirtyBitmap and
>    is why "const" was removed for copy.
> 
>    (9/13):
>  - QMP semantics changes as outlined for Patch #3.
>  - Enable/Disable now protected by aio_context (Stefanha)
> 
>    (10/13):
>  - Add coroutine_fn annotation to block backup helper. (Stefanha)
>  - Removed sync_bitmap_gran from BackupBlockJob, just use the
>    getter on the BdrvDirtyBitmap instead. (Stefanha)
>  - bdrv_dirty_iter_set was renamed to bdrv_set_dirty_iter.
>  - Calls to bdrv_reset_dirty_bitmap are modified to
>    bdrv_clear_dirty_bitmap to reflect the rename in patch #6.
>  - Bitmap usage modes (RESET vs. CONSUME) has been deleted, for
>    the reason of targeting a simpler core usage first before
>    targeting optimizations. CONSUME is particularly problematic
>    in the case of errors; so this mode is omitted for now.
>  - Adjusted error message to use MirrorSyncMode enum, not
>    (incorrectly) the BitmapSyncMode enum.
>  - In the event of a failure, the sync_bitmap is now merged back
>    into the original bitmap so that we do not lose any dirty
>    bitmap information needlessly.
> 
>    (11/13):
>  - Changed block_dirty_bitmap_add_abort to use
>    qmp_block_dirty_bitmap_remove:
>     - Old code used bdrv_lookup_bs to acquire bitmap and bs
>       anyway, and ignored the failure case.
>     - New code relies on the same information, and with a NULL
>       errp pointer we will similarly ignore the failure case.
>       prepare() should have properly vetted this information
>       before this point anyway.
>       This new code is also now protected via aio_context through
>       the use of the QMP commands.
>  - More code re-use of block_dirty_bitmap_lookup to get both the
>    bs and bitmap pointers.
>  - aio_context protection and cleanup in new abort methods.
> 
>    (13/13):
>  - Modified test for new parameter names.
> 
> v9:
>  - Edited commit message, for English embetterment (02/10)
>  - Rebased on top of stefanha/block-next (06,08/10)
>  - Adjusted error message and line length (07/10)
> 
> v8:
>  - Changed int64_t return for bdrv_dbm_calc_def_granularity to uint64_t (2/10)
>  - Updated commit message (2/10)
>  - Removed redundant check for null in device parameter (2/10)
>  - Removed comment cruft. (2/10)
>  - Removed redundant local_err propagation (several)
>  - Updated commit message (3/10)
>  - Fix HBitmap copy loop index (4/10)
>  - Remove redundant ternary (5/10)
>  - Shift up the block_dirty_bitmap_lookup function (6/10)
>  - Error messages cleanup (7/10)
>  - Add an assertion to explain the re-use of .prepare() for two transactions.
>    (8/10)
>  - Removed BDS argument from bitmap enable/disable helper; it was unused. 
> (8/10)
> 
> v7: (highlights)
>  - First version being authored by jsnow
>  - Addressed most list feedback from V6, many small changes.
>    All feedback was either addressed on-list (as a wontfix) or patched.
>  - Replaced all error_set with error_setg
>  - Replaced all bdrv_find with bdrv_lookup_bs()
>  - Adjusted the max granularity to share a common function with
>    backup/mirror that attempts to "guess" a reasonable default.
>    It clamps between [4K,64K] currently.
>  - The BdrvDirtyBitmap object now counts granularity exclusively in
>    bytes to match its interface.
>    It leaves the sector granularity concerns to HBitmap.
>  - Reworked the backup loop to utilize the hbitmap iterator.
>    There are some extra concerns to handle arrhythmic cases where the
>    granularity of the bitmap does not match the backup cluster size.
>    This iteration works best when it does match, but it's not a
>    deal-breaker if it doesn't -- it just gets less efficient.
>  - Reworked the transactional functions so that abort() wouldn't "undo"
>    a redundant command. They now have been split into a prepare and a
>    commit function (with state) and do not provide an abort command.
>  - Added a block_dirty_bitmap_lookup(device, name, errp) function to
>    shorten a few of the commands added in this series, particularly
>    qmp_enable, qmp_disable, and the transaction preparations.
> 
> v6: Re-send of v5.
> 
> v5: Rebase to master.
> 
> v4: Last version tailored by Fam Zheng.
> 
> ==
> 
> Fam Zheng (7):
>   qapi: Add optional field "name" to block dirty bitmap
>   qmp: Add block-dirty-bitmap-add and block-dirty-bitmap-remove
>   block: Introduce bdrv_dirty_bitmap_granularity()
>   qmp: Add block-dirty-bitmap-enable and block-dirty-bitmap-disable
>   qapi: Add transaction support to block-dirty-bitmap-{add, enable,
>     disable}
>   qmp: Add dirty bitmap status fields in query-block
>   qemu-iotests: Add tests for drive-backup sync=dirty-bitmap
> 
> John Snow (5):
>   block: Add bdrv_clear_dirty_bitmap
>   hbitmap: add hbitmap_merge
>   block: Add bitmap successors
>   qmp: Add support of "dirty-bitmap" sync mode for drive-backup
>   block: BdrvDirtyBitmap miscellaneous fixup
> 
> Vladimir Sementsov-Ogievskiy (1):
>   block: fix spoiling all dirty bitmaps by mirror and migration
> 
>  block.c                       | 254 +++++++++++++++++++++++++++++++++++++--
>  block/backup.c                | 120 +++++++++++++++----
>  block/mirror.c                |  27 ++---
>  blockdev.c                    | 273 
> +++++++++++++++++++++++++++++++++++++++++-
>  hmp.c                         |   3 +-
>  include/block/block.h         |  31 ++++-
>  include/block/block_int.h     |   2 +
>  include/qemu/hbitmap.h        |  11 ++
>  migration/block.c             |   7 +-
>  qapi-schema.json              |   5 +-
>  qapi/block-core.json          | 103 +++++++++++++++-
>  qmp-commands.hx               |  68 ++++++++++-
>  tests/qemu-iotests/056        |  33 ++++-
>  tests/qemu-iotests/056.out    |   4 +-
>  tests/qemu-iotests/iotests.py |   8 ++
>  util/hbitmap.c                |  28 +++++
>  16 files changed, 910 insertions(+), 67 deletions(-)
> 
> -- 
> 1.9.3
> 



reply via email to

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