Re: [Qemu-devel] [PATCH v7 00/14] block: incremental backup transactions using BlockJobTxn

[John Snow]

On 09/21/2015 10:46 PM, Fam Zheng wrote:
> v7: Add Eric's rev-by in 1, 11.
>     Add Max's rev-by in 4, 5, 9, 10, 11.
>     Add John's rev-by in 5, 6, 8.
>     Fix wording for 6. [John]
>     Fix comment of block_job_txn_add_job() in 9. [Max]
>     Remove superfluous hunks, and document default value in 11. [Eric]
>     Update Makefile dep in 14. [Max]
> 
> v6: Address comments from Max and Eric (many thanks for reviewing!):
>     Add Max's reviews.
>     Don't leak txn. [Max]
>     Unusual comment ending "**/" -> "*/". [Eric]
>     Fix the stale block_job_txn_prepare_to_complete comment. [Max]
>     Move "block_job_txn_unref" to below FOREACH block. [Max]
>     Split "base" and "txn" in qapi schema, so that transactional-cancel is 
> only
>     visible in transaction. [Eric]
> 
> v5: Address comments from Max Reitz:
>     2.4 -> 2.5 in qapi docs.
>     Don't leak txn object.
>     English syntax fixes.
>     Really leave the "cancelled" status of failed job.
>     Remove a superfluous added line.
> 
> This is based on top of the work by Stefan Hajnoczi and John Snow.
> 
> Recap: motivation for block job transactions
> --------------------------------------------
> If an incremental backup block job fails then we reclaim the bitmap so
> the job can be retried.  The problem comes when multiple jobs are started as
> part of a qmp 'transaction' command.  We need to group these jobs in a
> transaction so that either all jobs complete successfully or all bitmaps are
> reclaimed.
> 
> Without transactions, there is a case where some jobs complete successfully 
> and
> throw away their bitmaps, making it impossible to retry the backup by 
> rerunning
> the command if one of the jobs fails.
> 
> How does this implementation work?
> ----------------------------------
> These patches add a BlockJobTxn object with the following API:
> 
>   txn = block_job_txn_new();
>   block_job_txn_add_job(txn, job1);
>   block_job_txn_add_job(txn, job2);
> 
> The jobs either both complete successfully or they both fail/cancel.  If the
> user cancels job1 then job2 will also be cancelled and vice versa.
> 
> Jobs objects stay alive waiting for other jobs to complete, even if the
> coroutines have returned.  They can be cancelled by the user during this time.
> Job blockers are still in effect and no other block job can run on this device
> in the meantime (since QEMU currently only allows 1 job per device).  This is
> the main drawback to this approach but reasonable since you probably don't 
> want
> to run other jobs/operations until you're sure the backup was successful (you
> won't be able to retry a failed backup if there's a new job running).
> 
> 
> Fam Zheng (6):
>   backup: Extract dirty bitmap handling as a separate function
>   blockjob: Introduce reference count
>   blockjob: Add .commit and .abort block job actions
>   blockjob: Add "completed" and "ret" in BlockJob
>   blockjob: Simplify block_job_finish_sync
>   block: Add block job transactions
> 
> John Snow (4):
>   qapi: Add transaction support to block-dirty-bitmap operations
>   iotests: add transactional incremental backup test
>   block: rename BlkTransactionState and BdrvActionOps
>   iotests: 124 - transactional failure test
> 
> Kashyap Chamarthy (1):
>   qmp-commands.hx: Update the supported 'transaction' operations
> 
> Stefan Hajnoczi (3):
>   blockdev: make BlockJobTxn available to qmp 'transaction'
>   block/backup: support block job transactions
>   tests: add BlockJobTxn unit test
> 
>  block.c                    |  19 ++-
>  block/backup.c             |  50 +++++--
>  block/mirror.c             |   2 +-
>  blockdev.c                 | 354 
> +++++++++++++++++++++++++++++++++++----------
>  blockjob.c                 | 185 +++++++++++++++++++----
>  docs/bitmaps.md            |   6 +-
>  include/block/block.h      |   2 +-
>  include/block/block_int.h  |   6 +-
>  include/block/blockjob.h   |  85 ++++++++++-
>  qapi-schema.json           |  10 +-
>  qapi/block-core.json       |  27 +++-
>  qmp-commands.hx            |  21 ++-
>  tests/Makefile             |   3 +
>  tests/qemu-iotests/124     | 182 ++++++++++++++++++++++-
>  tests/qemu-iotests/124.out |   4 +-
>  tests/test-blockjob-txn.c  | 244 +++++++++++++++++++++++++++++++
>  16 files changed, 1056 insertions(+), 144 deletions(-)
>  create mode 100644 tests/test-blockjob-txn.c
> 

Reminder: needed as a new patch -- an update to the documentation
concerning the transactional-cancel argument added in patch 11/14 for
docs/bitmaps.md.

--js