[ovs-dev] [PATCH] ovs-vsctl: Fix behavioral regression for "--if-exists del-port <bridge>".

[Justin Pettit]

The behavior seems a little surprising, since the "--if-exists" sounds like it will be deleted if it exists, but it's actually just avoiding a fatal error and the port will still exist.  It would be nicer to give some sort of non-fatal warning in that case.  However, that's likely to cause breakage in scripts and most people know not to delete the local port, so I'm also fine if you prefer this approach.

--Justin


On Jul 8, 2013, at 10:48 AM, Ben Pfaff <blp at nicira.com> wrote:

> Commit 89f3c258fe (ovs-vsctl: Improve error message for "ovs-vsctl del-port
> <bridge>".) changed the behavior of
>    ovs-vsctl --if-exists del-port <bridge>
> from a silent no-op to a hard failure.  This commit fixes this regression.
> 
> This caused problems on XenServer, for which the Open vSwitch integration
> runs commands like:
>    /usr/bin/ovs-vsctl --timeout=20 \
>        -- --with-iface --if-exists del-port xapi103 \
>        -- --if-exists del-br xapi103
> 
> Bug #18276.
> Reported-by: Michael Hu <mhu at nicira.com>
> Signed-off-by: Ben Pfaff <blp at nicira.com>
> ---
> tests/ovs-vsctl.at    |    2 ++
> utilities/ovs-vsctl.c |    9 ++++++---
> 2 files changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at
> index fa2c3ff..4449f7a 100644
> --- a/tests/ovs-vsctl.at
> +++ b/tests/ovs-vsctl.at
> @@ -327,6 +327,8 @@ 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([--if-exists del-port a])], [0], [], [],
> +  [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
> ],
> diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c
> index 2d8c7c7..e679e0d 100644
> --- a/utilities/ovs-vsctl.c
> +++ b/utilities/ovs-vsctl.c
> @@ -2021,9 +2021,12 @@ cmd_del_port(struct vsctl_context *ctx)
> 
>     vsctl_context_populate_cache(ctx);
>     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);
> +        if (must_exist) {
> +            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);
> +        }
> +        port = NULL;
>     } else if (!with_iface) {
>         port = find_port(ctx, target, must_exist);
>     } else {
> -- 
> 1.7.2.5
> 
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev