From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39610) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZYqHx-0003OJ-Os for qemu-devel@nongnu.org; Mon, 07 Sep 2015 02:52:27 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZYqHw-0008Dn-Rq for qemu-devel@nongnu.org; Mon, 07 Sep 2015 02:52:25 -0400 Received: from mx1.redhat.com ([209.132.183.28]:56459) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZYqHw-0008Dj-Nf for qemu-devel@nongnu.org; Mon, 07 Sep 2015 02:52:24 -0400 Date: Mon, 7 Sep 2015 14:52:21 +0800 From: Fam Zheng Message-ID: <20150907065221.GF3958@ad.nay.redhat.com> References: <1438238370-16212-1-git-send-email-famz@redhat.com> <1438238370-16212-11-git-send-email-famz@redhat.com> <55BFAF43.6060209@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <55BFAF43.6060209@redhat.com> Subject: Re: [Qemu-devel] [PATCH v4 10/14] blockdev: make BlockJobTxn available to qmp 'transaction' List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Max Reitz Cc: Kevin Wolf , Jeff Cody , qemu-devel@nongnu.org, vsementsov@parallels.com, stefanha@redhat.com, John Snow On Mon, 08/03 20:13, Max Reitz wrote: > On 30.07.2015 08:39, Fam Zheng wrote: > >From: Stefan Hajnoczi > > > >Provide a BlockJobTxn to actions executed in a qmp 'transaction' > >command. This allows actions to make their block jobs either complete > >as a group or fail/cancel together. > > > >The next patch adds the first user. > > > >Signed-off-by: Stefan Hajnoczi > >Reviewed-by: Fam Zheng > >Reviewed-by: John Snow > >Signed-off-by: Fam Zheng > >--- > > blockdev.c | 5 +++++ > > 1 file changed, 5 insertions(+) > > > >diff --git a/blockdev.c b/blockdev.c > >index 2eb4f34..aba9d78 100644 > >--- a/blockdev.c > >+++ b/blockdev.c > >@@ -1279,6 +1279,7 @@ typedef struct BlkActionOps { > > struct BlkActionState { > > TransactionAction *action; > > const BlkActionOps *ops; > >+ BlockJobTxn *block_job_txn; > > QSIMPLEQ_ENTRY(BlkActionState) entry; > > }; > > > >@@ -1883,12 +1884,15 @@ static const BlkActionOps actions[] = { > > void qmp_transaction(TransactionActionList *dev_list, Error **errp) > > { > > TransactionActionList *dev_entry = dev_list; > >+ BlockJobTxn *block_job_txn; > > BlkActionState *state, *next; > > Error *local_err = NULL; > > > > QSIMPLEQ_HEAD(snap_bdrv_states, BlkActionState) snap_bdrv_states; > > QSIMPLEQ_INIT(&snap_bdrv_states); > > > >+ block_job_txn = block_job_txn_new(); > >+ > > /* drain all i/o before any operations */ > > bdrv_drain_all(); > > > >@@ -1908,6 +1912,7 @@ void qmp_transaction(TransactionActionList *dev_list, Error **errp) > > state = g_malloc0(ops->instance_size); > > state->ops = ops; > > state->action = dev_info; > >+ state->block_job_txn = block_job_txn; > > QSIMPLEQ_INSERT_TAIL(&snap_bdrv_states, state, entry); > > > > state->ops->prepare(state, &local_err); > > > > And then state->block_job_txn is never used, thus leaked. I guess this is > fixed for backup jobs in the next patch (by "The transaction is > automatically freed when the last job completes or is cancelled"), but for > any transaction which doesn't contain any block job which is added to the > transaction, it probably remains leaked. Good catch! I'll add a reference count to BlockJobTxn. Fam