[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 10/14] blockdev: make BlockJobTxn available t
From: |
Fam Zheng |
Subject: |
Re: [Qemu-devel] [PATCH v4 10/14] blockdev: make BlockJobTxn available to qmp 'transaction' |
Date: |
Mon, 7 Sep 2015 14:52:21 +0800 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
On Mon, 08/03 20:13, Max Reitz wrote:
> On 30.07.2015 08:39, Fam Zheng wrote:
> >From: Stefan Hajnoczi <address@hidden>
> >
> >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 <address@hidden>
> >Reviewed-by: Fam Zheng <address@hidden>
> >Reviewed-by: John Snow <address@hidden>
> >Signed-off-by: Fam Zheng <address@hidden>
> >---
> > 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
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [Qemu-devel] [PATCH v4 10/14] blockdev: make BlockJobTxn available to qmp 'transaction',
Fam Zheng <=