[ovs-dev] [InBandOVSDB 4/4] vswitchd: Enable in-band control to managers.

[Ben Pfaff]

On Mon, Apr 26, 2010 at 01:24:08AM -0700, Justin Pettit wrote:
> On Apr 20, 2010, at 4:37 PM, Ben Pfaff wrote:
> 
> > static void bridge_reconfigure_controller(const struct ovsrec_open_vswitch *,
> > -                                          struct bridge *);
> > +                                          struct bridge *,
> > +                                          const struct sockaddr_in *managers,
> > +                                          size_t n_managers);
> 
> To be consistently vague, do you think we should rename this function
> "bridge_reconfigure_controller"?

Rename it to what?

> > +/* Looks at the list of managers in 'ovs_cfg' and extracts their remote IP
> > + * addresses and ports into '*managersp' and '*n_managersp'.  The caller is
> > + * responsible for freeing '*managersp' (with free()).
> > + *
> > + * You may be asking yourself "WTF does ovs-vswitchd care?", because
> 
> :-) I've been trying to think how you can work more masked profanity
> into our relatively dry mailing lists.  I know you're fond of saying
> "patches accepted" when people get demandy of new features, but I just
> came up with a new one you can try: "Bits or GTFO".

I think the :-) means that you're joking, but just in case I changed the
"WTF" to "Why".

(I was working at a then-startup a few years ago.  When it was about to
be acquired, the requirement came down the pike to purge profanity from
source code comments because "due diligence" people consider it to be
some kind of a risk.  Funny.)

> > +        managers = xmalloc(ovs_cfg->n_managers * sizeof *managersp);
> 
> Should that be "sizeof *managers"?

Yes, thanks.

> > @@ -792,7 +837,7 @@ bridge_reconfigure(const struct ovsrec_open_vswitch *ovs_cfg)
> >          * yet; when a controller is configured, resetting the datapath ID will
> >          * immediately disconnect from the controller, so it's better to set
> >          * the datapath ID before the controller. */
> > -        bridge_reconfigure_controller(ovs_cfg, br);
> > +        bridge_reconfigure_controller(ovs_cfg, br, managers, n_managers);
> >     }
> 
> I don't see any place where "managers" gets freed.

Oops, thanks.  Fixed.