On Mon, Mar 28, 2016 at 4:42 PM, Aaron Conole <acon...@redhat.com> wrote:
> Hi Russell,
>
> Russell Bryant <russ...@ovn.org> writes:
>
> > Publish ovn-controller's local bridge mappings configuration
> > in the external_ids column of the Chassis table. Having this
> > information available for reading is useful to applications
> > integrating with OVN.
> >
> > Signed-off-by: Russell Bryant <russ...@ovn.org>
> > ---
> > ovn/controller/chassis.c | 23 +++++++++++++++++++++++
> > ovn/ovn-sb.xml | 7 +++++++
> > 2 files changed, 30 insertions(+)
> >
> > diff --git a/ovn/controller/chassis.c b/ovn/controller/chassis.c
> > index 52c9993..cd4f787 100644
> > --- a/ovn/controller/chassis.c
> > +++ b/ovn/controller/chassis.c
> > @@ -18,6 +18,7 @@
> >
> > #include "chassis.h"
> >
> > +#include "lib/smap.h"
> > #include "lib/vswitch-idl.h"
> > #include "openvswitch/dynamic-string.h"
> > #include "openvswitch/vlog.h"
> > @@ -102,6 +103,12 @@ chassis_run(struct controller_ctx *ctx, const char
> *chassis_id)
> > hostname = hostname_;
> > }
> >
> > + const char *bridge_mappings = smap_get(&cfg->external_ids,
> > + "ovn-bridge-mappings");
> > + if (!bridge_mappings) {
> > + bridge_mappings = "";
> > + }
> > +
> > const struct sbrec_chassis *chassis_rec
> > = get_chassis(ctx->ovnsb_idl, chassis_id);
> >
> > @@ -110,6 +117,19 @@ chassis_run(struct controller_ctx *ctx, const char
> *chassis_id)
> > sbrec_chassis_set_hostname(chassis_rec, hostname);
> > }
> >
> > + const char *chassis_bridge_mappings
> > + = smap_get(&chassis_rec->external_ids,
> "ovn-bridge-mappings");
> > + if (!chassis_bridge_mappings) {
> > + chassis_bridge_mappings = "";
> > + }
> > + if (strcmp(bridge_mappings, chassis_bridge_mappings)) {
> > + struct smap new_ids;
> > + smap_clone(&new_ids, &chassis_rec->external_ids);
> > + smap_replace(&new_ids, "ovn-bridge-mappings",
> bridge_mappings);
> > + sbrec_chassis_set_external_ids(chassis_rec, &new_ids);
> > + smap_destroy(&new_ids);
> > + }
> > +
>
> This is going to be really nit-picky (so feel free to reject it), but
> would something like:
>
> static inline const char *
> get_bridge_mappings(struct smap *external_ids)
> {
> const char *ret = smap_get(external_ids, "ovn-bridge-mappings");
> if (!ret) {
> ret = "";
> }
> return ret;
> }
>
> be preferable? that would reduce some of the complexity in the two hunks
> above, and let you refactor at least ovn/controller/patch.c starting on
> line 144. I don't know if it makes sense, though.
>
Nice suggestion, thanks. I'll do that.
--
Russell Bryant
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
