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 <b...@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 <m...@nicira.com>
> Signed-off-by: Ben Pfaff <b...@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@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
