Thanks.  Is that a review?

On Tue, Jun 18, 2013 at 09:27:18PM -0700, Reid Price wrote:
> Thanks, that seems like the right amount of information
> 
>   -Reid
> 
> 
> On Tue, Jun 18, 2013 at 9:02 PM, Ben Pfaff <b...@nicira.com> wrote:
> 
> > Previously, commands like this:
> >     ovs-vsctl add-br br0
> >     ovs-vsctl del-port br0
> > yielded an error message like:
> >     no port named br0
> > which is confusing.  This commit improves the error message to:
> >     cannot delete port br0 because it is the local port for bridge br0
> >     (deleting this port requires deleting the entire bridge)
> >
> > Bug #17994.
> > Reported-by: Reid Price <r...@nicira.com>
> > Signed-off-by: Ben Pfaff <b...@nicira.com>
> > ---
> >  tests/ovs-vsctl.at    |    6 +++++-
> >  utilities/ovs-vsctl.c |   10 +++++++---
> >  2 files changed, 12 insertions(+), 4 deletions(-)
> >
> > diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at
> > index ec08cd4..fa2c3ff 100644
> > --- a/tests/ovs-vsctl.at
> > +++ b/tests/ovs-vsctl.at
> > @@ -323,6 +323,10 @@ AT_CHECK([RUN_OVS_VSCTL(
> >  ], [], [OVS_VSCTL_CLEANUP])
> >  AT_CHECK([RUN_OVS_VSCTL([--may-exist add-port b b1])], [0], [], [],
> >    [OVS_VSCTL_CLEANUP])
> > +AT_CHECK([RUN_OVS_VSCTL([del-port a])], [1], [],
> > +  [ovs-vsctl: cannot delete port a because it is the local port for
> > bridge a (deleting this port requires deleting the entire bridge)
> > +],
> > +  [OVS_VSCTL_CLEANUP])
> >  AT_CHECK([RUN_OVS_VSCTL([--may-exist add-port a b1])], [1], [],
> >    [ovs-vsctl: "--may-exist add-port a b1" but b1 is actually attached to
> > bridge b
> >  ],
> > @@ -1246,4 +1250,4 @@ AT_CHECK([sed "/|bridge|WARN|/d" ovs-vswitchd.log >
> > ovs-vswitchd.log], [0], [],
> >  # Delete the port
> >  AT_CHECK([ovs-vsctl del-port br0 reserved_name], [0], [], [])])
> >  OVS_VSWITCHD_STOP
> > -AT_CLEANUP
> > \ No newline at end of file
> > +AT_CLEANUP
> > diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c
> > index 19ab472..2d8c7c7 100644
> > --- a/utilities/ovs-vsctl.c
> > +++ b/utilities/ovs-vsctl.c
> > @@ -2016,13 +2016,17 @@ cmd_del_port(struct vsctl_context *ctx)
> >  {
> >      bool must_exist = !shash_find(&ctx->options, "--if-exists");
> >      bool with_iface = shash_find(&ctx->options, "--with-iface") != NULL;
> > +    const char *target = ctx->argv[ctx->argc - 1];
> >      struct vsctl_port *port;
> >
> >      vsctl_context_populate_cache(ctx);
> > -    if (!with_iface) {
> > -        port = find_port(ctx, ctx->argv[ctx->argc - 1], must_exist);
> > +    if (find_bridge(ctx, target, false)) {
> > +        vsctl_fatal("cannot delete port %s because it is the local port "
> > +                    "for bridge %s (deleting this port requires deleting "
> > +                    "the entire bridge)", target, target);
> > +    } else if (!with_iface) {
> > +        port = find_port(ctx, target, must_exist);
> >      } else {
> > -        const char *target = ctx->argv[ctx->argc - 1];
> >          struct vsctl_iface *iface;
> >
> >          port = find_port(ctx, target, false);
> > --
> > 1.7.10.4
> >
> >
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to