| Message ID | 20240722152125.3682051-1-i.maximets@ovn.org |
|---|---|
| State | Accepted |
| Commit | a5023d597c1dc6799a3899cd1ee66d4dd55405ce |
| Delegated to: | Ilya Maximets |
| Headers | show |
| Series | [ovs-dev] ovsdb: transaction: Remove incorrect transaction abort in pre-commit. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/intel-ovs-compilation | fail | test: fail |
On Mon, Jul 22, 2024 at 11:21 AM Ilya Maximets <i.maximets@ovn.org> wrote: > > Pre-commit must not abort the transaction, otherwise the upper layers > may crash accessing it. E.g. ovsdb_trigger_try() checks the state of > the transaction after trying to commit it. > > This particular failure can't actually happen, because the function > determine_changes() can't fail. However, the code is still wrong and > a bit misleading, so should be fixed. > > Fixes: 53178986d7fc ("ovsdb: Add support for online schema conversion.") > Signed-off-by: Ilya Maximets <i.maximets@ovn.org> Acked-by: Mike Pattrick <mkp@redhat.com>
On 7/24/24 17:28, Mike Pattrick wrote: > On Mon, Jul 22, 2024 at 11:21 AM Ilya Maximets <i.maximets@ovn.org> wrote: >> >> Pre-commit must not abort the transaction, otherwise the upper layers >> may crash accessing it. E.g. ovsdb_trigger_try() checks the state of >> the transaction after trying to commit it. >> >> This particular failure can't actually happen, because the function >> determine_changes() can't fail. However, the code is still wrong and >> a bit misleading, so should be fixed. >> >> Fixes: 53178986d7fc ("ovsdb: Add support for online schema conversion.") >> Signed-off-by: Ilya Maximets <i.maximets@ovn.org> > > Acked-by: Mike Pattrick <mkp@redhat.com> > Thanks, Mike! Applied and backported down to 2.17. Best regards, Ilya Maximets.
diff --git a/ovsdb/transaction.c b/ovsdb/transaction.c index 65eca6478..98fff1a74 100644 --- a/ovsdb/transaction.c +++ b/ovsdb/transaction.c @@ -1090,7 +1090,6 @@ ovsdb_txn_precommit(struct ovsdb_txn *txn) * was really a no-op. */ error = for_each_txn_row(txn, determine_changes); if (error) { - ovsdb_txn_abort(txn); return OVSDB_WRAP_BUG("can't happen", error); } if (ovs_list_is_empty(&txn->txn_tables)) {
Pre-commit must not abort the transaction, otherwise the upper layers may crash accessing it. E.g. ovsdb_trigger_try() checks the state of the transaction after trying to commit it. This particular failure can't actually happen, because the function determine_changes() can't fail. However, the code is still wrong and a bit misleading, so should be fixed. Fixes: 53178986d7fc ("ovsdb: Add support for online schema conversion.") Signed-off-by: Ilya Maximets <i.maximets@ovn.org> --- ovsdb/transaction.c | 1 - 1 file changed, 1 deletion(-)