[ovs-dev] [PATCH] bridge: Execute bridge_run() only after retrieving db contents.

[Alex Wang]

How about this?

+    } else if (!ovsdb_idl_has_lock(idl)
+               || !ovsdb_idl_has_ever_connected(idl)) {
+        /* Returns if not holding the lock or not done retrieving db
+         * contents. */
         return;
     }


On Wed, Apr 1, 2015 at 4:55 PM, Ben Pfaff <blp at nicira.com> wrote:

> On Wed, Apr 01, 2015 at 04:52:24PM -0700, Alex Wang wrote:
> > During upgrade of ovs-vswitchd, we do not want to recreate the kernel
> > interfaces.  Especially when IP address is assigned to the internal port,
> > the recreation will cause the lost of connection.  Therefore,
> ovs-vswitchd
> > should read current ovsdb content first and then reuse the existing
> kernel
> > interfaces that are configured in ovsdb.  In terms of the code language,
> > ovs-vswitchd should only execute bridge_run() after it finishes reading
> > the ovsdb content.
> >
> > However, this expected behavior is broken by the recent commit d18e52e
> > (ovsdb-idl: Tolerate missing tables and columns.) which causes the
> > execution of bridge_run() before getting the hint of configured
> interfaces
> > from ovsdb.
> >
> > To fix the issue, this commit makes sure that the execution of
> bridge_run()
> > happens only after retrieving the ovsdb contents.
> >
> > VMware-BZ: #1424342
> >
> > Signed-off-by: Alex Wang <alexw at nicira.com>
>
> Acked-by: Ben Pfaff <blp at nicira.com>
>
> The new code seems fine but I don't like having a blank line and a
> comment in the middle of an "if".  It's likely to confuse
> people. Maybe the comment could be inside the {} block instead of
> above it?
> > -    } else if (!ovsdb_idl_has_lock(idl)) {
> > +
> > +    /* Only proceeds when holding the lock and done retrieving db
> > +     * contents. */
> > +    } else if (!ovsdb_idl_has_lock(idl)
> > +               || !ovsdb_idl_has_ever_connected(idl)) {
> >          return;
> >      }
> >      cfg = ovsrec_open_vswitch_first(idl);
> > --
> > 1.7.9.5
> >
>