[ovs-dev] [PATCH] ovs-vsctl: Allow bridge name to be omitted from del-port command.

Ian Campbell Ian.Campbell at citrix.com
Fri Oct 16 08:31:43 UTC 2009


On Thu, 2009-10-15 at 20:47 +0100, Ben Pfaff wrote:
> The 'bridge' argument to ovs-vsctl's del-port command is only supplied as
> a form of error checking.  Sometimes the name of the bridge isn't readily
> available, so for such situations this commit allows the user to omit the
> name of the bridge entirely.

Looks good. Thanks.

Ian.


> 
> CC: Ian Campbell <Ian.Campbell at citrix.com>
> ---
>  tests/ovs-vsctl.at       |    4 ++--
>  utilities/ovs-vsctl.8.in |    6 ++++--
>  utilities/ovs-vsctl.in   |   42 ++++++++++++++++++++++++++++--------------
>  3 files changed, 34 insertions(+), 18 deletions(-)
> 
> diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at
> index dcf6d20..15e5b1e 100644
> --- a/tests/ovs-vsctl.at
> +++ b/tests/ovs-vsctl.at
> @@ -165,12 +165,12 @@ CHECK_PORTS([b], [b1])
>  CHECK_IFACES([b], [b1])
>  AT_CLEANUP
>  
> -AT_SETUP([add-br a, add-bond a bond0 a1 a2 a3, del-port a bond0])
> +AT_SETUP([add-br a, add-bond a bond0 a1 a2 a3, del-port bond0])
>  AT_KEYWORDS([ovs-vsctl])
>  AT_CHECK([RUN_OVS_VSCTL(
>    [add-br a], 
>    [add-bond a bond0 a1 a2 a3],
> -  [del-port a bond0])])
> +  [del-port bond0])])
>  AT_CHECK([cat conf], [0], [dnl
>  bridge.a.port=a
>  ])
> diff --git a/utilities/ovs-vsctl.8.in b/utilities/ovs-vsctl.8.in
> index acb4a80..b05a955 100644
> --- a/utilities/ovs-vsctl.8.in
> +++ b/utilities/ovs-vsctl.8.in
> @@ -157,8 +157,10 @@ Creates on \fIbridge\fR a new port named \fIport\fR that bonds
>  together the network devices given as each \fIiface\fR.  At least two
>  interfaces must be named.
>  .
> -.IP "\fBdel\-port \fIbridge port\fR"
> -Deletes \fBport\fR from \fIbridge\fR.
> +.IP "\fBdel\-port \fR[\fIbridge\fR] \fIport\fR"
> +Deletes \fIport\fR.  If \fIbridge\fR is omitted, \fIport\fR is removed
> +from whatever bridge contains it; if \fIbridge\fR is specified, it
> +must be the real or fake bridge that contains \fIport\fR.
>  .
>  .IP "\fBport\-to\-br \fIport\fR"
>  Prints the name of the bridge that contains \fIport\fR on standard
> diff --git a/utilities/ovs-vsctl.in b/utilities/ovs-vsctl.in
> index cc5c0be..ac55c42 100755
> --- a/utilities/ovs-vsctl.in
> +++ b/utilities/ovs-vsctl.in
> @@ -310,6 +310,14 @@ def del_port(cfg, port):
>          if fnmatch.fnmatch(key, 'bridge.*.port'):
>              cfg[key] = [s for s in cfg[key] if s != port]
>  
> +# Returns the name of the (real or fake) bridge in 'cfg' that contains
> +# port 'port', or None if there is no such port.
> +def port_to_bridge(cfg, port):
> +    for bridge, parent, vlan in get_bridge_info(cfg):
> +        if port != bridge and port in get_bridge_ports(cfg, parent, vlan):
> +            return bridge
> +    return None
> +
>  def usage():
>      print """%(argv0)s: ovs-vswitchd management utility
>  usage: %(argv0)s [OPTIONS] COMMAND [ARG...]
> @@ -327,7 +335,7 @@ Port commands:
>    list-ports BRIDGE           print the names of all the ports on BRIDGE
>    add-port BRIDGE PORT        add network device PORT to BRIDGE
>    add-bond BRIDGE PORT IFACE...  add new bonded port PORT in BRIDGE from IFACES
> -  del-port BRIDGE PORT        delete PORT (which may be bonded) from BRIDGE
> +  del-port [BRIDGE] PORT      delete PORT (which may be bonded) from BRIDGE
>    port-to-br PORT             print name of bridge that contains PORT
>  A bond is considered to be a single port.
>  
> @@ -439,24 +447,30 @@ def cmd_add_bond(bridge, port, *slaves):
>      cfg['bonding.%s.slave' % port] = list(slaves)
>      cfg_save(cfg, VSWITCHD_CONF)
>  
> -def cmd_del_port(bridge, port):
> +def cmd_del_port(*args):
>      cfg = cfg_read(VSWITCHD_CONF, True)
> -    parent, vlan = find_bridge(cfg, bridge)
> -    if port not in get_bridge_ports(cfg, parent, vlan):
> -        if port in get_bridge_ports(cfg, parent, -1):
> -            raise Error("bridge %s does not have a port %s (although its parent bridge %s does)" % (bridge, port, parent))
> -        else:
> -            raise Error("bridge %s does not have a port %s" % (bridge, port))
> +    if len(args) == 2:
> +        bridge, port = args
> +        parent, vlan = find_bridge(cfg, bridge)
> +        if port not in get_bridge_ports(cfg, parent, vlan):
> +            if port in get_bridge_ports(cfg, parent, -1):
> +                raise Error("bridge %s does not have a port %s (although its parent bridge %s does)" % (bridge, port, parent))
> +            else:
> +                raise Error("bridge %s does not have a port %s" % (bridge, port))
> +    else:
> +        port, = args
> +        if not port_to_bridge(cfg, port):
> +            raise Error("no port %s on any bridge" % port)
>      del_port(cfg, port)
>      cfg_save(cfg, VSWITCHD_CONF)
>  
>  def cmd_port_to_br(port):
>      cfg = cfg_read(VSWITCHD_CONF)
> -    for bridge, parent, vlan in get_bridge_info(cfg):
> -        if port != bridge and port in get_bridge_ports(cfg, parent, vlan):
> -            print bridge
> -            return
> -    raise Error("no port named %s" % port)
> +    bridge = port_to_bridge(cfg, port)
> +    if bridge:
> +        print bridge
> +    else:
> +        raise Error("no port named %s" % port)
>  
>  def cmd_list_ifaces(bridge):
>      cfg = cfg_read(VSWITCHD_CONF)
> @@ -537,7 +551,7 @@ def main():
>                  'list-ports': (cmd_list_ports, 1),
>                  'add-port': (cmd_add_port, 2),
>                  'add-bond': (cmd_add_bond, lambda n: n >= 4),
> -                'del-port': (cmd_del_port, 2),
> +                'del-port': (cmd_del_port, lambda n: n == 1 or n == 2),
>                  'port-to-br': (cmd_port_to_br, 1),
>                  'br-to-vlan': (cmd_br_to_vlan, 1),
>                  'br-to-parent': (cmd_br_to_parent, 1),






More information about the dev mailing list