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 <b...@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 <al...@nicira.com>
>
> Acked-by: Ben Pfaff <b...@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
> >
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
