Thanks.  I pushed this.

On Mon, Jul 25, 2011 at 02:05:05PM -0700, Ethan Jackson wrote:
> Looks Good.
> 
> Ethan
> 
> On Fri, Jul 8, 2011 at 12:31, Ben Pfaff <[email protected]> wrote:
> > The database prevents multiple ports or interfaces with a single name, but
> > duplicates can still occur if, for example, two bridges' "ports" columns
> > both point to a single Port record. ?The existing warning just says in this
> > case that the database contains a duplicate port name. ?This prompts users
> > to dump the Port table to look for the duplicate. ?Of course there isn't
> > one, so then they ask me to point out the problem.
> >
> > This commit improves the log message to point out the actual problem.
> > ---
> > ?utilities/ovs-vsctl.c | ? 36 ++++++++++++++++++++++++++++--------
> > ?1 files changed, 28 insertions(+), 8 deletions(-)
> >
> > diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c
> > index c6fc8a4..1c67b11 100644
> > --- a/utilities/ovs-vsctl.c
> > +++ b/utilities/ovs-vsctl.c
> > @@ -785,8 +785,7 @@ get_info(struct vsctl_context *ctx, struct vsctl_info 
> > *info)
> > ? ? ? ? ? ? struct ovsrec_port *port_cfg = br_cfg->ports[j];
> >
> > ? ? ? ? ? ? if (!sset_add(&ports, port_cfg->name)) {
> > - ? ? ? ? ? ? ? ?VLOG_WARN("%s: database contains duplicate port name",
> > - ? ? ? ? ? ? ? ? ? ? ? ? ?port_cfg->name);
> > + ? ? ? ? ? ? ? ?/* Duplicate port name. ?(We will warn about that later.) 
> > */
> > ? ? ? ? ? ? ? ? continue;
> > ? ? ? ? ? ? }
> >
> > @@ -800,7 +799,6 @@ get_info(struct vsctl_context *ctx, struct vsctl_info 
> > *info)
> > ? ? sset_destroy(&ports);
> >
> > ? ? sset_init(&bridges);
> > - ? ?sset_init(&ports);
> > ? ? for (i = 0; i < ovs->n_bridges; i++) {
> > ? ? ? ? struct ovsrec_bridge *br_cfg = ovs->bridges[i];
> > ? ? ? ? struct vsctl_bridge *br;
> > @@ -815,7 +813,18 @@ get_info(struct vsctl_context *ctx, struct vsctl_info 
> > *info)
> > ? ? ? ? ? ? struct vsctl_port *port;
> > ? ? ? ? ? ? size_t k;
> >
> > - ? ? ? ? ? ?if (!sset_add(&ports, port_cfg->name)) {
> > + ? ? ? ? ? ?port = shash_find_data(&info->ports, port_cfg->name);
> > + ? ? ? ? ? ?if (port) {
> > + ? ? ? ? ? ? ? ?if (port_cfg == port->port_cfg) {
> > + ? ? ? ? ? ? ? ? ? ?VLOG_WARN("%s: port is in multiple bridges (%s and 
> > %s)",
> > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?port_cfg->name, br->name, 
> > port->bridge->name);
> > + ? ? ? ? ? ? ? ?} else {
> > + ? ? ? ? ? ? ? ? ? ?/* Log as an error because this violates the database's
> > + ? ? ? ? ? ? ? ? ? ? * uniqueness constraints, so the database server 
> > shouldn't
> > + ? ? ? ? ? ? ? ? ? ? * have allowed it. */
> > + ? ? ? ? ? ? ? ? ? ?VLOG_ERR("%s: database contains duplicate port name",
> > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? port_cfg->name);
> > + ? ? ? ? ? ? ? ?}
> > ? ? ? ? ? ? ? ? continue;
> > ? ? ? ? ? ? }
> >
> > @@ -841,9 +850,21 @@ get_info(struct vsctl_context *ctx, struct vsctl_info 
> > *info)
> > ? ? ? ? ? ? ? ? struct ovsrec_interface *iface_cfg = 
> > port_cfg->interfaces[k];
> > ? ? ? ? ? ? ? ? struct vsctl_iface *iface;
> >
> > - ? ? ? ? ? ? ? ?if (shash_find(&info->ifaces, iface_cfg->name)) {
> > - ? ? ? ? ? ? ? ? ? ?VLOG_WARN("%s: database contains duplicate interface 
> > name",
> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?iface_cfg->name);
> > + ? ? ? ? ? ? ? ?iface = shash_find_data(&info->ifaces, iface_cfg->name);
> > + ? ? ? ? ? ? ? ?if (iface) {
> > + ? ? ? ? ? ? ? ? ? ?if (iface_cfg == iface->iface_cfg) {
> > + ? ? ? ? ? ? ? ? ? ? ? ?VLOG_WARN("%s: interface is in multiple ports "
> > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?"(%s and %s)",
> > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?iface_cfg->name,
> > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?iface->port->port_cfg->name,
> > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?port->port_cfg->name);
> > + ? ? ? ? ? ? ? ? ? ?} else {
> > + ? ? ? ? ? ? ? ? ? ? ? ?/* Log as an error because this violates the 
> > database's
> > + ? ? ? ? ? ? ? ? ? ? ? ? * uniqueness constraints, so the database server
> > + ? ? ? ? ? ? ? ? ? ? ? ? * shouldn't have allowed it. */
> > + ? ? ? ? ? ? ? ? ? ? ? ?VLOG_ERR("%s: database contains duplicate 
> > interface "
> > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "name", iface_cfg->name);
> > + ? ? ? ? ? ? ? ? ? ?}
> > ? ? ? ? ? ? ? ? ? ? continue;
> > ? ? ? ? ? ? ? ? }
> >
> > @@ -855,7 +876,6 @@ get_info(struct vsctl_context *ctx, struct vsctl_info 
> > *info)
> > ? ? ? ? }
> > ? ? }
> > ? ? sset_destroy(&bridges);
> > - ? ?sset_destroy(&ports);
> > ?}
> >
> > ?static void
> > --
> > 1.7.4.4
> >
> > _______________________________________________
> > dev mailing list
> > [email protected]
> > http://openvswitch.org/mailman/listinfo/dev
> >
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev

Reply via email to