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).

Author: blp

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(

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

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,57 +396,47 @@ 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
     for port in set(get_bridge_ports(cfg, parent, vlan) + [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)
     cfg['bridge.%s.port' % parent].append(port)
     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,13 +488,15 @@ def main():
                                            "target=",
                                            "no-reload",
                                            "no-syslog",
+                                           "oneline",
                                            "help",
                                            "version"])
     except getopt.GetoptError, msg:
         sys.stderr.write("%s: %s (use --help for help)\n" % (argv0, msg))
         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,35 +531,45 @@ 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:
         sys.stderr.write("%s: unknown command '%s' (use --help for help)\n"
                          % (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)
     elif not callable(nargs) and len(args) != nargs:
         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__":