On Sat, Jul 25, 2015 at 11:04:13PM -0700, Alex Wang wrote:
> On Sun, Jul 19, 2015 at 3:44 PM, Ben Pfaff <b...@nicira.com> wrote:
>
> > It's unsafe to hold a pointer to a row in the IDL across calls to
> > ovsdb_idl_run() for that IDL.
> >
> > Signed-off-by: Ben Pfaff <b...@nicira.com>
> > ---
> > ovn/controller/ovn-controller.c | 17 ++++++++---------
> > 1 file changed, 8 insertions(+), 9 deletions(-)
> >
> > diff --git a/ovn/controller/ovn-controller.c
> > b/ovn/controller/ovn-controller.c
> > index fda1534..e571bb5 100644
> > --- a/ovn/controller/ovn-controller.c
> > +++ b/ovn/controller/ovn-controller.c
> > @@ -95,16 +95,15 @@ get_bridge(struct controller_ctx *ctx, const char
> > *name)
> > static void
> > get_core_config(struct controller_ctx *ctx)
> > {
> > - const struct ovsrec_open_vswitch *cfg;
> > -
> > - cfg = ovsrec_open_vswitch_first(ctx->ovs_idl);
> > - if (!cfg) {
> > - VLOG_ERR("No Open_vSwitch row defined.");
> > - ovsdb_idl_destroy(ctx->ovs_idl);
> > - exit(EXIT_FAILURE);
> > - }
> > -
> > while (1) {
> > + const struct ovsrec_open_vswitch *cfg;
> > + cfg = ovsrec_open_vswitch_first(ctx->ovs_idl);
> > + if (!cfg) {
> > + VLOG_ERR("No Open_vSwitch row defined.");
> > + ovsdb_idl_destroy(ctx->ovs_idl);
> > + exit(EXIT_FAILURE);
> > + }
> > +
> >
>
> Curious, why don't you call ovsdb_idl_run(ctx->ovs_idl) first in the while
> loop? seems to me, theoretically, cfg could still get changed after
> ovsdb_idl_run(ctx->ovs_idl).
You're right, that was a dumb "fix".
Here's an incremental that fixes that problem.
I'll fix for v2.
diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
index e571bb5..bd3ef0d 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -96,6 +96,8 @@ static void
get_core_config(struct controller_ctx *ctx)
{
while (1) {
+ ovsdb_idl_run(ctx->ovs_idl);
+
const struct ovsrec_open_vswitch *cfg;
cfg = ovsrec_open_vswitch_first(ctx->ovs_idl);
if (!cfg) {
@@ -107,8 +109,6 @@ get_core_config(struct controller_ctx *ctx)
const struct ovsrec_bridge *br_int;
const char *remote, *system_id, *br_int_name;
- ovsdb_idl_run(ctx->ovs_idl);
-
br_int_name = smap_get(&cfg->external_ids, "ovn-bridge");
if (!br_int_name) {
br_int_name = DEFAULT_BRIDGE_NAME;
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
