[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
>
- [Qemu-devel] [PATCH v11 08/13] block: Add bitmap successors, (continued)
- [Qemu-devel] [PATCH v11 12/13] qemu-iotests: Add tests for drive-backup sync=dirty-bitmap, John Snow, 2015/01/12
- [Qemu-devel] [PATCH v11 10/13] qapi: Add transaction support to block-dirty-bitmap-{add, enable, disable}, John Snow, 2015/01/12
- [Qemu-devel] [PATCH v11 13/13] block: BdrvDirtyBitmap miscellaneous fixup, John Snow, 2015/01/12
- Re: [Qemu-devel] [PATCH v11 00/13] block: Incremental backup series,
Fam Zheng <=
- Re: [Qemu-devel] [PATCH v11 00/13] block: Incremental backup series, John Snow, 2015/01/13
- Re: [Qemu-devel] [PATCH v11 00/13] block: Incremental backup series, John Snow, 2015/01/29
- Re: [Qemu-devel] [PATCH v11 00/13] block: Incremental backup series, Vladimir Sementsov-Ogievskiy, 2015/01/30