diff mbox series

[ovs-dev,01/10] ovsdb-idl: Avoid sending transactions when the DB is not synced up.

Message ID 20181115060534.7146-1-blp@ovn.org
State Accepted
Headers show
Series [ovs-dev,01/10] ovsdb-idl: Avoid sending transactions when the DB is not synced up. | expand

Commit Message

Ben Pfaff Nov. 15, 2018, 6:05 a.m. UTC
Until now the code here would happily try to send transactions to the
database server even if the database connection was not in the correct
state.  In some cases this could lead to strange behavior, such as sending
a database transaction for a database that the IDL had just learned did not
exist on the server.

Signed-off-by: Ben Pfaff <blp@ovn.org>
---
 lib/ovsdb-idl.c | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Mark Michelson Nov. 16, 2018, 8:36 p.m. UTC | #1
For the series, looks good by my eyes. I have to admit I may have just 
skimmed patch 8 though :)

Acked-by: Mark Michelson <mmichels@redhat.com>

On 11/15/2018 01:05 AM, Ben Pfaff wrote:
> Until now the code here would happily try to send transactions to the
> database server even if the database connection was not in the correct
> state.  In some cases this could lead to strange behavior, such as sending
> a database transaction for a database that the IDL had just learned did not
> exist on the server.
> 
> Signed-off-by: Ben Pfaff <blp@ovn.org>
> ---
>   lib/ovsdb-idl.c | 7 +++++++
>   1 file changed, 7 insertions(+)
> 
> diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
> index 9f44cefd152b..a1f246d6f7b7 100644
> --- a/lib/ovsdb-idl.c
> +++ b/lib/ovsdb-idl.c
> @@ -3867,6 +3867,13 @@ ovsdb_idl_txn_commit(struct ovsdb_idl_txn *txn)
>           goto coverage_out;
>       }
>   
> +    /* If we're still connecting or re-connecting, don't bother sending a
> +     * transaction. */
> +    if (txn->db->idl->state != IDL_S_MONITORING) {
> +        txn->status = TXN_TRY_AGAIN;
> +        goto disassemble_out;
> +    }
> +
>       /* If we need a lock but don't have it, give up quickly. */
>       if (txn->db->lock_name && !txn->db->has_lock) {
>           txn->status = TXN_NOT_LOCKED;
>
Ben Pfaff Nov. 19, 2018, 5:01 p.m. UTC | #2
Thanks.  I applied patches 1 through 9 to master and backported an
appropriate subset to branch-2.10, and the use-after-free fix as far as
branch-2.7.  I'm going to shift patch 10 to a new series.

On Fri, Nov 16, 2018 at 03:36:11PM -0500, Mark Michelson wrote:
> For the series, looks good by my eyes. I have to admit I may have just
> skimmed patch 8 though :)
> 
> Acked-by: Mark Michelson <mmichels@redhat.com>
> 
> On 11/15/2018 01:05 AM, Ben Pfaff wrote:
> >Until now the code here would happily try to send transactions to the
> >database server even if the database connection was not in the correct
> >state.  In some cases this could lead to strange behavior, such as sending
> >a database transaction for a database that the IDL had just learned did not
> >exist on the server.
> >
> >Signed-off-by: Ben Pfaff <blp@ovn.org>
> >---
> >  lib/ovsdb-idl.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> >
> >diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
> >index 9f44cefd152b..a1f246d6f7b7 100644
> >--- a/lib/ovsdb-idl.c
> >+++ b/lib/ovsdb-idl.c
> >@@ -3867,6 +3867,13 @@ ovsdb_idl_txn_commit(struct ovsdb_idl_txn *txn)
> >          goto coverage_out;
> >      }
> >+    /* If we're still connecting or re-connecting, don't bother sending a
> >+     * transaction. */
> >+    if (txn->db->idl->state != IDL_S_MONITORING) {
> >+        txn->status = TXN_TRY_AGAIN;
> >+        goto disassemble_out;
> >+    }
> >+
> >      /* If we need a lock but don't have it, give up quickly. */
> >      if (txn->db->lock_name && !txn->db->has_lock) {
> >          txn->status = TXN_NOT_LOCKED;
> >
>
diff mbox series

Patch

diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
index 9f44cefd152b..a1f246d6f7b7 100644
--- a/lib/ovsdb-idl.c
+++ b/lib/ovsdb-idl.c
@@ -3867,6 +3867,13 @@  ovsdb_idl_txn_commit(struct ovsdb_idl_txn *txn)
         goto coverage_out;
     }
 
+    /* If we're still connecting or re-connecting, don't bother sending a
+     * transaction. */
+    if (txn->db->idl->state != IDL_S_MONITORING) {
+        txn->status = TXN_TRY_AGAIN;
+        goto disassemble_out;
+    }
+
     /* If we need a lock but don't have it, give up quickly. */
     if (txn->db->lock_name && !txn->db->has_lock) {
         txn->status = TXN_NOT_LOCKED;