On Mon, 08/03 20:13, Max Reitz wrote: > On 30.07.2015 08:39, Fam Zheng wrote: > >From: Stefan Hajnoczi <stefa...@redhat.com> > > > >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 <stefa...@redhat.com> > >Reviewed-by: Fam Zheng <f...@redhat.com> > >Reviewed-by: John Snow <js...@redhat.com> > >Signed-off-by: Fam Zheng <f...@redhat.com> > >--- > > 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