Re: [Qemu-devel] [PATCH v6 01/11] blockdev: abort transactions in reverse order
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. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v6 01/11] blockdev: abort transactions in reverse order
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 > Reviewed-by: Eric Blake > Reviewed-by: Vladimir Sementsov-Ogievskiy > --- > blockdev.c | 14 +++--- > 1 file changed, 7 insertions(+), 7 deletions(-) > -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature
[Qemu-devel] [PATCH v6 01/11] blockdev: abort transactions in reverse order
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.
Signed-off-by: John Snow
Reviewed-by: Eric Blake
Reviewed-by: Vladimir Sementsov-Ogievskiy
---
blockdev.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
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(_bdrv_states);
+QTAILQ_HEAD(snap_bdrv_states, BlkActionState) snap_bdrv_states;
+QTAILQ_INIT(_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(_bdrv_states, state, entry);
+QTAILQ_INSERT_TAIL(_bdrv_states, state, entry);
state->ops->prepare(state, _err);
if (local_err) {
@@ -2307,7 +2307,7 @@ void qmp_transaction(TransactionActionList *dev_list,
}
}
-QSIMPLEQ_FOREACH(state, _bdrv_states, entry) {
+QTAILQ_FOREACH(state, _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, _bdrv_states, entry) {
+QTAILQ_FOREACH_REVERSE(state, _bdrv_states, snap_bdrv_states, entry) {
if (state->ops->abort) {
state->ops->abort(state);
}
}
exit:
-QSIMPLEQ_FOREACH_SAFE(state, _bdrv_states, entry, next) {
+QTAILQ_FOREACH_SAFE(state, _bdrv_states, entry, next) {
if (state->ops->clean) {
state->ops->clean(state);
}
--
2.17.2
