On Wed, Jan 15, 2020 at 3:51 AM Han Zhou <hz...@ovn.org> wrote:
>
> Hi Mark,
>
> Thanks for the review!
> In fact, if cfg was NULL, this function would never have a chance to set
> probe interval, because before setting the interval it would have already
> crashed, at this line:
>     int interval = smap_get_int(&cfg->external_ids,
>
> So, the behavior change of this patch is only avoiding the crash.
>
> Thanks,
> Han
>
> On Tue, Jan 14, 2020 at 1:24 PM Mark Michelson <mmich...@redhat.com> wrote:
> >
> > Hi Han.
> >
> > This patch introduces a small behavior change in the case where cfg is
> > NULL. In the earlier version, we would always set the probe interval on
> > the southbound database. With this version of the patch, we exit early
> > when cfg is NULL and do not set the probe interval.
> >
> > On 1/13/20 5:52 PM, Han Zhou wrote:
> > > In function update_sb_db(), it tries to access cfg->external_ids
> > > outside of the "if (cfg)" block. This patch fixes it.
> > >
> > > Signed-off-by: Han Zhou <hz...@ovn.org>

Acked-by: Numan Siddique <num...@ovn.org>

Numan

> > > ---
> > >   controller/ovn-controller.c | 8 ++++----
> > >   1 file changed, 4 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> > > index 17744d4..3d843f7 100644
> > > --- a/controller/ovn-controller.c
> > > +++ b/controller/ovn-controller.c
> > > @@ -431,12 +431,12 @@ static void
> > >   update_sb_db(struct ovsdb_idl *ovs_idl, struct ovsdb_idl *ovnsb_idl)
> > >   {
> > >       const struct ovsrec_open_vswitch *cfg =
> ovsrec_open_vswitch_first(ovs_idl);
> > > +    if (!cfg) {
> > > +        return;
> > > +    }
> > >
> > >       /* Set remote based on user configuration. */
> > > -    const char *remote = NULL;
> > > -    if (cfg) {
> > > -        remote = smap_get(&cfg->external_ids, "ovn-remote");
> > > -    }
> > > +    const char *remote = smap_get(&cfg->external_ids, "ovn-remote");
> > >       ovsdb_idl_set_remote(ovnsb_idl, remote, true);
> > >
> > >       /* Set probe interval, based on user configuration and the
> remote. */
> > >
> >
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to