From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42333) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z7n5A-0004Mb-SG for qemu-devel@nongnu.org; Wed, 24 Jun 2015 11:59:26 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z7n59-0006V5-Fn for qemu-devel@nongnu.org; Wed, 24 Jun 2015 11:59:24 -0400 Message-ID: <558AD3D3.2080201@redhat.com> Date: Wed, 24 Jun 2015 11:59:15 -0400 From: John Snow MIME-Version: 1.0 References: <1433541188-13491-1-git-send-email-jsnow@redhat.com> In-Reply-To: <1433541188-13491-1-git-send-email-jsnow@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v6 00/10] block: incremental backup transactions List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-block@nongnu.org Cc: kwolf@redhat.com, famz@redhat.com, qemu-devel@nongnu.org, mreitz@redhat.com, vsementsov@parallels.com, stefanha@redhat.com On 06/05/2015 05:52 PM, John Snow wrote: > This series adds incremental backup related commands to the QMP > transaction subsystem in order to accomplish some slightly more > sophisticated bitmap and backup management tasks that require > atomic actions. > > Patch 1 adds basic support for add and clear transactions. > Patch 2 tests this basic support. > Patches 3-4 refactor transactions a little bit, to add clarity. > Patch 5 adds the framework for error scenarios where only > some jobs that were launched by a transaction complete successfully, > and we need to perform context-sensitive cleanup after the transaction > itself has already completed. > Patches 6 adds necessary bookkeeping information to bitmap > data structures to take advantage of this new feature. > Patch 7 modifies qmp_drive_backup to support the new feature. > Patch 8 implements the new feature for drive_backup transaction actions. > Patch 9 tests the new feature. > Patch 10 updates documentation. > > === > v6: > === > > Key: > [----] : patches are identical > [####] : number of functional differences between upstream/downstream patch > [down] : patch is downstream-only > The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively > > 001/10:[----] [--] 'qapi: Add transaction support to block-dirty-bitmap operations' > 002/10:[----] [--] 'iotests: add transactional incremental backup test' > 003/10:[----] [--] 'block: rename BlkTransactionState and BdrvActionOps' > 004/10:[----] [--] 'block: re-add BlkTransactionState' > 005/10:[0016] [FC] 'block: add transactional callbacks feature' > 006/10:[0010] [FC] 'block: add delayed bitmap successor cleanup' > 007/10:[----] [--] 'qmp: Add an implementation wrapper for qmp_drive_backup' > 008/10:[0002] [FC] 'block: drive_backup transaction callback support' > 009/10:[----] [--] 'iotests: 124 - transactional failure test' > 010/10:[----] [--] 'qmp-commands.hx: Update the supported 'transaction' operations' > > 05: Replaced CallbackFn typedef by existing BlockCompletionFunc > 06: Assert that hbitmap_merge will work, instead of testing it > Add some (successor_refcnt == 0) assertions while we're at it > 08: Fallout from 05. > > === > v5: > === > > 04: Renamed "jobs" to "refcnt" in BlkTransactionState > 05: Fallout from above changes. > 06: Removed a lot of errp parameters from the BdrvDirtyBitmap helpers > Renamed "bdrv_dirty_bitmap_incref" to "bdrv_frozen_bitmap_incref" > xx: Deleted job refcount patch. > 07: Removed separate cb/opaque nullchecks in favor of a single check > 08: Broadly, the job reference counting was removed. > drive_backup_prepare should now read much more cleanly. > Removed (state->job == bs->job) assertion > Moved two different bs accesses under aio_context_acquire() > 10: Included Kashyap's new documentation patch. > > === > v4: > === > > 01: New approach for clear transaction. > bdrv_clear_dirty_bitmap (and undo_clear) moved to block_int.h > Removed more outdated documentation from bitmaps.md > 03: Fallout from adding a _clear_abort() method. > 11: New documentation patch from Kashyap. > > === > v3: > === > > 01: Removed "(Not yet implemented)" line from bitmaps.md. > Kept R-Bs, like a selfish person would. > 02: Rebased on latest transactionless series. > Added some transaction helpers. > 05: Removed superfluous deletion loop in put_blk_action_state. > 08: Rebased without the preceding code motion patch. > Re-added forward declaration for do_drive_backup. > 09: Fixed commit message whitespace. > Re-added block_job_cb forward declaration to avoid code motion patch. > 10: Rebased on latest transactionless series; > Added "wait_qmp_backup" function to complement "do_qmp_backup." > General bike-shedding. > > === > v2: > === > > 01: Fixed indentation. > Fixed QMP commands to behave with new bitmap_lookup from > transactionless-v4. > 2.3 --> 2.4. > 02: Folded in improvements to qemu-iotest 124 from transactional-v1. > 03: NEW > 04: NEW > 05: A lot: > Don't delete the comma in the transaction actions config > use g_new0 instead of g_malloc0 > Phrasing: "retcode" --> "Return code" > Use GCC attributes to mark functions as unused until future patches. > Added some data structure documentation. > Many structure and function renames, hopefully to improve readability. > Use just one list for all Actions instead of two separate lists. > Remove ActionState from the list upon deletion/decref > And many other small tweaks. > 06: Comment phrasing. > 07: Removed errp parameter from all functions introduced by this commit. > bdrv_dirty_bitmap_decref --> bdrv_frozen_bitmap_decref > 08: NEW > 09: _drive_backup --> do_drive_backup() > Forward declarations removed. > 10: Rebased on top of drastically modified #05. > Phrasing: "BackupBlockJob" instead of "BackupJob" in comments. > 11: Removed extra parameters to wait_incremental() in > test_transaction_failure() > > ________________________________________________________________________________ > > For convenience, this branch is available at: > https://github.com/jnsnow/qemu.git branch incremental-transactions > https://github.com/jnsnow/qemu/tree/incremental-transactions > > This version is tagged incremental-transactions-v6: > https://github.com/jnsnow/qemu/releases/tag/incremental-transactions-v6 > > John Snow (9): > qapi: Add transaction support to block-dirty-bitmap operations > iotests: add transactional incremental backup test > block: rename BlkTransactionState and BdrvActionOps > block: re-add BlkTransactionState > block: add transactional callbacks feature > block: add delayed bitmap successor cleanup > qmp: Add an implementation wrapper for qmp_drive_backup > block: drive_backup transaction callback support > iotests: 124 - transactional failure test > > Kashyap Chamarthy (1): > qmp-commands.hx: Update the supported 'transaction' operations > > block.c | 103 +++++++-- > block/backup.c | 40 ++-- > blockdev.c | 553 +++++++++++++++++++++++++++++++++++++++------ > docs/bitmaps.md | 6 +- > include/block/block.h | 11 +- > include/block/block_int.h | 19 ++ > qapi-schema.json | 6 +- > qmp-commands.hx | 21 +- > tests/qemu-iotests/124 | 174 +++++++++++++- > tests/qemu-iotests/124.out | 4 +- > 10 files changed, 812 insertions(+), 125 deletions(-) > NACK Stefan has proposed a new series that uses mutexes (mutices?) in the BlockJob to accomplish a similar transactional cleanup barrier with a bit less code and is likely the correct way forward. --js