| Message ID | 20181221093529.23855-2-jsnow@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | bitmaps: remove x- prefix from QMP api | expand |
On 12/21/18 3:35 AM, John Snow wrote: > Presently, we abort transactions in the same order they were processed in. > Bitmap commands, though, attempt to restore backup data structures on abort. > > That's not valid, they need to be aborted in reverse chronological order. > > Replace the QSIMPLEQ data structure with a QTAILQ one, so we can iterate > in reverse for the abort phase of the transaction. This patch conflicts with Paolo's improvements to QTAILQ that just landed; the resolution should be obvious, but at this point, you'll want to post a v7. Or did you want me to try and fix the conflict, and take the series through my NBD tree since I have patches based on top of it? > > Signed-off-by: John Snow <jsnow@redhat.com> > Reviewed-by: Eric Blake <eblake@redhat.com> > Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > --- > blockdev.c | 14 +++++++------- > 1 file changed, 7 insertions(+), 7 deletions(-) >
On 1/11/19 11:52 AM, Eric Blake wrote: > On 12/21/18 3:35 AM, John Snow wrote: >> Presently, we abort transactions in the same order they were processed in. >> Bitmap commands, though, attempt to restore backup data structures on abort. >> >> That's not valid, they need to be aborted in reverse chronological order. >> >> Replace the QSIMPLEQ data structure with a QTAILQ one, so we can iterate >> in reverse for the abort phase of the transaction. > > This patch conflicts with Paolo's improvements to QTAILQ that just > landed; the resolution should be obvious, but at this point, you'll want > to post a v7. > > Or did you want me to try and fix the conflict, and take the series > through my NBD tree since I have patches based on top of it? Actually, I went ahead and queued this series in my NBD tree after making the fixes, so I can get the removal of x- in place sooner for easier integration testing on my libvirt code.
diff --git a/blockdev.c b/blockdev.c index a6f71f9d83..43e4c22da5 100644 --- a/blockdev.c +++ b/blockdev.c @@ -1339,7 +1339,7 @@ struct BlkActionState { const BlkActionOps *ops; JobTxn *block_job_txn; TransactionProperties *txn_props; - QSIMPLEQ_ENTRY(BlkActionState) entry; + QTAILQ_ENTRY(BlkActionState) entry; }; /* internal snapshot private data */ @@ -2266,8 +2266,8 @@ void qmp_transaction(TransactionActionList *dev_list, BlkActionState *state, *next; Error *local_err = NULL; - QSIMPLEQ_HEAD(snap_bdrv_states, BlkActionState) snap_bdrv_states; - QSIMPLEQ_INIT(&snap_bdrv_states); + QTAILQ_HEAD(snap_bdrv_states, BlkActionState) snap_bdrv_states; + QTAILQ_INIT(&snap_bdrv_states); /* Does this transaction get canceled as a group on failure? * If not, we don't really need to make a JobTxn. @@ -2298,7 +2298,7 @@ void qmp_transaction(TransactionActionList *dev_list, state->action = dev_info; state->block_job_txn = block_job_txn; state->txn_props = props; - QSIMPLEQ_INSERT_TAIL(&snap_bdrv_states, state, entry); + QTAILQ_INSERT_TAIL(&snap_bdrv_states, state, entry); state->ops->prepare(state, &local_err); if (local_err) { @@ -2307,7 +2307,7 @@ void qmp_transaction(TransactionActionList *dev_list, } } - QSIMPLEQ_FOREACH(state, &snap_bdrv_states, entry) { + QTAILQ_FOREACH(state, &snap_bdrv_states, entry) { if (state->ops->commit) { state->ops->commit(state); } @@ -2318,13 +2318,13 @@ void qmp_transaction(TransactionActionList *dev_list, delete_and_fail: /* failure, and it is all-or-none; roll back all operations */ - QSIMPLEQ_FOREACH(state, &snap_bdrv_states, entry) { + QTAILQ_FOREACH_REVERSE(state, &snap_bdrv_states, snap_bdrv_states, entry) { if (state->ops->abort) { state->ops->abort(state); } } exit: - QSIMPLEQ_FOREACH_SAFE(state, &snap_bdrv_states, entry, next) { + QTAILQ_FOREACH_SAFE(state, &snap_bdrv_states, entry, next) { if (state->ops->clean) { state->ops->clean(state); }