From 28f36edd197b017e650a35e3203fd6b96e8c5bec Mon Sep 17 00:00:00 2001 From: Dumitru Ceara Date: Fri, 28 Jan 2022 10:08:45 +0100 Subject: [PATCH] ovsdb-idl: Only process successful txn in ovsdb_idl_loop_run. Otherwise we hide the transaction result from the user. This may cause problems as the user will not detect error cases. For example, if the server refuses a transaction with "constraint violation", the user should be notified because the transaction might need to be retried. For clients that process database changes incrementally (using change tracking) this lack of failure notification creates a problem if it occurs while no other database changes happen. In that case: - ovsdb_idl_loop_run() silently consumes the failure, initializes a new transaction. - no other table update was received from the server so the user will not add anything to the new transaction. - ovsdb_idl_loop_commit_and_wait() will "succeed" as nothing changed from the client's perspective. In reality, the first transaction failed and the client wasn't given the chance to handle the failure. Commit 0401cf5f9e06 ("ovsdb idl: Try committing the pending txn in ovsdb_idl_loop_run.") tried to optimize for the common, successful case. Maintain the same approach and optimize for transactions that succeeded but fall back to the old mechanism of processing failures within ovsdb_idl_loop_commit_and_wait() instead. Fixes: 0401cf5f9e06 ("ovsdb idl: Try committing the pending txn in ovsdb_idl_loop_run.") Signed-off-by: Dumitru Ceara Signed-off-by: Ilya Maximets --- lib/ovsdb-idl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c index 46f51a527..82e003ac9 100644 --- a/lib/ovsdb-idl.c +++ b/lib/ovsdb-idl.c @@ -4255,8 +4255,8 @@ ovsdb_idl_loop_run(struct ovsdb_idl_loop *loop) { ovsdb_idl_run(loop->idl); - /* See if we can commit the loop->committing_txn. */ - if (loop->committing_txn) { + /* See if the 'committing_txn' succeeded in the meantime. */ + if (loop->committing_txn && loop->committing_txn->status == TXN_SUCCESS) { ovsdb_idl_try_commit_loop_txn(loop, NULL); }