[ovs-dev] [PATCH] ovs-vsctl: Improve error message for "ovs-vsctl del-port <bridge>".
Ben Pfaff
blp at nicira.com
Thu Jun 20 06:07:11 UTC 2013
Good enough, thanks. Applied.
On Thu, Jun 20, 2013 at 12:31:09AM -0500, Reid Price wrote:
> Yes, especially if it passed the unit test, looks correct.
>
>
> On Wed, Jun 19, 2013 at 3:26 PM, Ben Pfaff <blp at nicira.com> wrote:
>
> > 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 <blp at 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 <reid at nicira.com>
> > > > Signed-off-by: Ben Pfaff <blp at 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
> > > >
> > > >
> >
More information about the dev
mailing list