ovs-vsctl: Refactor internals to increase flexibility.

This changes the interface of each of the command implementations, making
them take the configuration as an argument and return the output.  This
will make it easier to support alternate output formats and to execute more
than one command per invocation (both happening in upcoming commits).

diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at
index 15e5b1ee00395b71f31a547cfa4a24b43244dff0..834ecc6c9da1998389c486c61853d1710bcf8a02 100644 (file)
--- a/tests/ovs-vsctl.at
+++ b/tests/ovs-vsctl.at
@@ -15,16 +15,31 @@ dnl specified PARENT and is on the given VLAN.
 m4_define([_CHECK_BRIDGE],
   [AT_CHECK([RUN_OVS_VSCTL([br-to-parent $1])], [0], [$2
 ])
+
+   # Check br-to-vlan, without --oneline.
    AT_CHECK([RUN_OVS_VSCTL([br-to-vlan $1])], [0], [$3
+])
+   # Check br-to-vlan, with --oneline.
+   # (This particular test is interesting with --oneline because it returns
+   # an integer instead of a string and that can cause type mismatches inside
+   # python if not done carefully.)
+   AT_CHECK([RUN_OVS_VSCTL([--oneline br-to-vlan $1])], [0], [$3
 ])])
 m4_define([CHECK_BRIDGES],
-  [dnl Check that the bridges appear on list-br.
+  [dnl Check that the bridges appear on list-br, without --oneline.
    AT_CHECK(
      [RUN_OVS_VSCTL([list-br])],
      [0],
      [m4_foreach([brinfo], [$@], [m4_car(brinfo)
 ])])
 
+   dnl Check that the bridges appear on list-br, with --oneline.
+   AT_CHECK(
+     [RUN_OVS_VSCTL([--oneline list-br])],
+     [0],
+     [m4_join([\n], m4_foreach([brinfo], [$@], [m4_car(brinfo),]))
+])
+
    dnl Check that each bridge exists according to br-exists and that
    dnl a bridge that should not exist does not.
    m4_foreach([brinfo], [$@], 
@@ -41,11 +56,19 @@ dnl list of ports, which must be in alphabetical order.  Also checks
 dnl that "ovs-vsctl port-to-br" reports that each port is
 dnl in BRIDGE.
 m4_define([CHECK_PORTS],
-  [AT_CHECK(
+  [dnl Check ports without --oneline.
+   AT_CHECK(
      [RUN_OVS_VSCTL([list-ports $1])],
      [0],
      [m4_foreach([port], m4_cdr($@), [port
 ])])
+
+   dnl Check ports with --oneline.
+   AT_CHECK(
+     [RUN_OVS_VSCTL([--oneline list-ports $1])],
+     [0],
+     [m4_join([\n], m4_shift($@))
+])
    AT_CHECK([RUN_OVS_VSCTL([port-to-br $1])], [1], [], [ovs-vsctl: no port named $1
 ])
    m4_foreach(
@@ -111,6 +134,22 @@ CHECK_PORTS([b])
 CHECK_IFACES([b])
 AT_CLEANUP
 
+AT_SETUP([add-br a, add-port a a1, add-port a a2])
+AT_KEYWORDS([ovs-vsctl])
+AT_CHECK([RUN_OVS_VSCTL(
+   [add-br a], 
+   [add-port a a1],
+   [add-port a a2])])
+AT_CHECK([cat conf], [0],
+  [bridge.a.port=a
+bridge.a.port=a1
+bridge.a.port=a2
+])
+CHECK_BRIDGES([a, a, 0])
+CHECK_PORTS([a], [a1], [a2])
+CHECK_IFACES([a], [a1], [a2])
+AT_CLEANUP
+
 AT_SETUP([add-br a b, add-port a a1, add-port b b1, del-br a])
 AT_KEYWORDS([ovs-vsctl])
 AT_CHECK([RUN_OVS_VSCTL(

diff --git a/utilities/ovs-vsctl.8.in b/utilities/ovs-vsctl.8.in
index b05a9551c21471d30adc1859ea523cb8f6c88fde..bc4cb11ab60440dc7913d0aa80ce233af7ff35ad 100644 (file)
--- a/utilities/ovs-vsctl.8.in
+++ b/utilities/ovs-vsctl.8.in
@@ -98,6 +98,11 @@ its configuration file.
 By default, \fBovs\-vsctl\fR logs its arguments and the details of any
 changes that it makes to the system log.  This option disables this
 logging.
+.IP "\fB\-\-oneline\fR"
+Modifies the output format so that the output for a command is printed
+on a single line.  New-line characters that would otherwise separate
+lines are printed as \fB\\n\fR, and any instances of \fB\\fR that
+would otherwise appear in the output are doubled.
 .
 .SH COMMANDS
 The commands implemented by \fBovs\-vsctl\fR are described in the

diff --git a/utilities/ovs-vsctl.in b/utilities/ovs-vsctl.in
index dde4dad8631f206a7e5dc4804d258bfb6e9819fe..d8b4404588e529e7ff0f731bb8f6720b9a8c8794 100755 (executable)
--- a/utilities/ovs-vsctl.in
+++ b/utilities/ovs-vsctl.in
@@ -373,9 +373,7 @@ def check_conflicts(cfg, name, op):
         if name in get_bridge_ifaces(cfg, parent, vlan):
             raise Error("%s because an interface named %s already exists on bridge %s" % (op, name, bridge))
     
-def cmd_add_br(bridge, parent=None, vlan=None):
-    cfg = cfg_read(VSWITCHD_CONF, True)
-
+def cmd_add_br(cfg, bridge, parent=None, vlan=None):
     check_conflicts(cfg, bridge, "cannot create a bridge named %s" % bridge)
     
     if parent and vlan:
@@ -398,10 +396,8 @@ def cmd_add_br(bridge, parent=None, vlan=None):
         cfg['bridge.%s.port' % parent].append(bridge)
     else:
         cfg['bridge.%s.port' % bridge] = [bridge]
-    cfg_save(cfg, VSWITCHD_CONF)
 
-def cmd_del_br(bridge):
-    cfg = cfg_read(VSWITCHD_CONF, True)
+def cmd_del_br(cfg, bridge):
     parent, vlan = find_bridge(cfg, bridge)
     if vlan == 0:
         vlan = -1
@@ -409,24 +405,21 @@ def cmd_del_br(bridge):
         del_port(cfg, port)
     if vlan < 0: 
         del_matching_keys(cfg, 'bridge.%s.[!0-9]*' % bridge)
-    cfg_save(cfg, VSWITCHD_CONF)
 
-def cmd_list_br():
-    cfg = cfg_read(VSWITCHD_CONF)
-    for bridge in get_all_bridges(cfg):
-        print bridge
+def cmd_list_br(cfg):
+    return get_all_bridges(cfg)
 
-def cmd_br_exists(bridge):
-    cfg = cfg_read(VSWITCHD_CONF)
+def cmd_br_exists(cfg, bridge):
     if bridge not in get_all_bridges(cfg):
         sys.exit(2)
 
-def cmd_list_ports(bridge):
-    cfg = cfg_read(VSWITCHD_CONF)
+def cmd_list_ports(cfg, bridge):
+    ports = []
     parent, vlan = find_bridge(cfg, bridge)
     for port in get_bridge_ports(cfg, parent, vlan):
         if port != bridge:
-            print port
+            ports.append(port)
+    return ports
 
 def do_add_port(cfg, bridge, parent, port, vlan):
     check_conflicts(cfg, port, "cannot create a port named %s" % port)
@@ -434,21 +427,16 @@ def do_add_port(cfg, bridge, parent, port, vlan):
     if vlan > 0:
         cfg['vlan.%s.tag' % port] = [vlan]
 
-def cmd_add_port(bridge, port):
-    cfg = cfg_read(VSWITCHD_CONF, True)
+def cmd_add_port(cfg, bridge, port):
     parent, vlan = find_bridge(cfg, bridge)
     do_add_port(cfg, bridge, parent, port, vlan)
-    cfg_save(cfg, VSWITCHD_CONF)
 
-def cmd_add_bond(bridge, port, *slaves):
-    cfg = cfg_read(VSWITCHD_CONF, True)
+def cmd_add_bond(cfg, bridge, port, *slaves):
     parent, vlan = find_bridge(cfg, bridge)
     do_add_port(cfg, bridge, parent, port, vlan)
     cfg['bonding.%s.slave' % port] = list(slaves)
-    cfg_save(cfg, VSWITCHD_CONF)
 
-def cmd_del_port(*args):
-    cfg = cfg_read(VSWITCHD_CONF, True)
+def cmd_del_port(cfg, *args):
     if len(args) == 2:
         bridge, port = args
         parent, vlan = find_bridge(cfg, bridge)
@@ -462,40 +450,35 @@ def cmd_del_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)
+def cmd_port_to_br(cfg, port):
     bridge = port_to_bridge(cfg, port)
     if bridge:
-        print bridge
+        return (bridge,)
     else:
         raise Error("no port named %s" % port)
 
-def cmd_list_ifaces(bridge):
-    cfg = cfg_read(VSWITCHD_CONF)
+def cmd_list_ifaces(cfg, bridge):
+    ifaces = []
     parent, vlan = find_bridge(cfg, bridge)
     for iface in get_bridge_ifaces(cfg, parent, vlan):
         if iface != bridge:
-            print iface
+            ifaces.append(iface)
+    return ifaces
 
-def cmd_iface_to_br(iface):
-    cfg = cfg_read(VSWITCHD_CONF)
+def cmd_iface_to_br(cfg, iface):
     for bridge, parent, vlan in get_bridge_info(cfg):
         if iface != bridge and iface in get_bridge_ifaces(cfg, parent, vlan):
-            print bridge
-            return
+            return (bridge,)
     raise Error("no interface named %s" % iface)
 
-def cmd_br_to_vlan(bridge):
-    cfg = cfg_read(VSWITCHD_CONF)
+def cmd_br_to_vlan(cfg, bridge):
     parent, vlan = find_bridge(cfg, bridge)
-    print vlan
+    return (vlan,)
 
-def cmd_br_to_parent(bridge):
-    cfg = cfg_read(VSWITCHD_CONF)
+def cmd_br_to_parent(cfg, bridge):
     parent, vlan = find_bridge(cfg, bridge)
-    print parent
+    return (parent,)
     
 def main():
     # Parse command line.
@@ -505,6 +488,7 @@ def main():
                                            "target=",
                                            "no-reload",
                                            "no-syslog",
+                                           "oneline",
                                            "help",
                                            "version"])
     except getopt.GetoptError, msg:
@@ -512,6 +496,7 @@ def main():
         sys.exit(1)
 
     # Handle options.
+    oneline = False
     for opt, optarg in options:
         if opt == "-c" or opt == "--config":
             global VSWITCHD_CONF
@@ -531,6 +516,8 @@ def main():
         elif opt == "--no-syslog":
             global SYSLOG
             SYSLOG = False
+        elif opt == "--oneline":
+            oneline = True
         else:
             raise RuntimeError("unhandled option %s" % opt)
 
@@ -544,19 +531,19 @@ def main():
                          % argv0)
         sys.exit(1)
 
-    commands = {'add-br': (cmd_add_br, lambda n: n == 1 or n == 3),
-                'del-br': (cmd_del_br, 1),
-                'list-br': (cmd_list_br, 0),
-                'br-exists': (cmd_br_exists, 1),
-                '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, 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),
-                'list-ifaces': (cmd_list_ifaces, 1),
-                'iface-to-br': (cmd_iface_to_br, 1)}
+    commands = {'add-br': (cmd_add_br, True, lambda n: n == 1 or n == 3),
+                'del-br': (cmd_del_br, True, 1),
+                'list-br': (cmd_list_br, False, 0),
+                'br-exists': (cmd_br_exists, False, 1),
+                'list-ports': (cmd_list_ports, False, 1),
+                'add-port': (cmd_add_port, True, 2),
+                'add-bond': (cmd_add_bond, True, lambda n: n >= 4),
+                'del-port': (cmd_del_port, True, lambda n: n == 1 or n == 2),
+                'port-to-br': (cmd_port_to_br, False, 1),
+                'br-to-vlan': (cmd_br_to_vlan, False, 1),
+                'br-to-parent': (cmd_br_to_parent, False, 1),
+                'list-ifaces': (cmd_list_ifaces, False, 1),
+                'iface-to-br': (cmd_iface_to_br, False, 1)}
     command = args[0]
     args = args[1:]
     if command not in commands:
@@ -564,7 +551,7 @@ def main():
                          % (argv0, command))
         sys.exit(1)
 
-    function, nargs = commands[command]
+    function, is_mutator, nargs = commands[command]
     if callable(nargs) and not nargs(len(args)):
         sys.stderr.write("%s: '%s' command does not accept %d arguments (use --help for help)\n" % (argv0, command, len(args)))
         sys.exit(1)
@@ -572,7 +559,17 @@ def main():
         sys.stderr.write("%s: '%s' command takes %d arguments but %d were supplied (use --help for help)\n" % (argv0, command, nargs, len(args)))
         sys.exit(1)
     else:
-        function(*args)
+        cfg = cfg_read(VSWITCHD_CONF, is_mutator)
+        output = function(cfg, *args)
+        if output != None:
+            if oneline:
+                print '\\n'.join([str(s).replace('\\', '\\\\')
+                                  for s in output])
+            else:
+                for line in output:
+                    print line
+        if is_mutator:
+            cfg_save(cfg, VSWITCHD_CONF)
         sys.exit(0)
 
 if __name__ == "__main__":