Re: [Qemu-devel] [PATCH v5 11/14] block/backup: support block job transactions

[Max Reitz]

On 07.09.2015 09:34, Fam Zheng wrote:
> From: Stefan Hajnoczi <address@hidden>
> 
> Join the transaction when the 'transactional-cancel' QMP argument is
> true.
> 
> This ensures that the sync bitmap is not thrown away if another block
> job in the transaction is cancelled or fails.  This is critical so
> incremental backup with multiple disks can be retried in case of
> cancellation/failure.
> 
> Signed-off-by: Stefan Hajnoczi <address@hidden>
> Signed-off-by: Fam Zheng <address@hidden>
> ---
>  block/backup.c            |  25 +++++++--
>  blockdev.c                | 139 
> ++++++++++++++++++++++++++++++++++++----------
>  hmp.c                     |   2 +-
>  include/block/block_int.h |   3 +-
>  qapi/block-core.json      |  16 +++++-
>  5 files changed, 148 insertions(+), 37 deletions(-)
> 
> diff --git a/block/backup.c b/block/backup.c
> index 9776d9c..3a3dccc 100644
> --- a/block/backup.c
> +++ b/block/backup.c

[snip]

> @@ -545,6 +559,7 @@ void backup_start(BlockDriverState *bs, BlockDriverState 
> *target,
>                         sync_bitmap : NULL;
>      job->common.len = len;
>      job->common.co = qemu_coroutine_create(backup_run);
> +    block_job_txn_add_job(txn, &job->common);

You're calling this for every single job, so the reference count of txn
will probably be job count + 1. However, block_job_txn_unref() is only
called for one block job by block_job_completed_txn_{abort,success}(). I
think you need to call block_job_txn_unref() for every completed block
job, otherwise it will be leaked if there is more than a single job in
the txn.

Max

>      qemu_coroutine_enter(job->common.co, job);
>      return;
>

signature.asc
Description: OpenPGP digital signature