[ovs-dev] [PATCH 3/3] ovs-vsctl: Add the ability to perform multiple operations in a single run.

reid at nicira.com reid at nicira.com
Fri Oct 16 03:02:18 UTC 2009


One other note regarding the oneline output option with multiple commands.
It seems as though it might be useful for parsing this output if commands
which do not have output (i.e.: return None) have a blank line or some
other marker printed, to save the parsing user the trouble of determining
which commands were supposed to have output and which weren't.  In the
interactive command-line case I can see that this would probably just be
more clutter or confusion - defaulting off, with a command line option to
turn this on would probably be preferable.

  -Reid

On Thu, 15 Oct 2009 22:50:32 -0400, <reid at nicira.com> wrote:
> Looks useful =).  Prefer append in split_commands.  I assume that
removing
> the final sys.exit(0) was deliberate.
> 
> On Thu, 15 Oct 2009 15:38:51 -0700, Ben Pfaff <blp at nicira.com> wrote:
>> CC: Ian Campbell <Ian.Campbell at citrix.com>
>> ---
>>  tests/ovs-vsctl.at       |   19 +++++++-
>>  utilities/ovs-vsctl.8.in |   16 ++++++-
>>  utilities/ovs-vsctl.in   |  114
>> +++++++++++++++++++++++++++++-----------------
>>  3 files changed, 103 insertions(+), 46 deletions(-)
>>
>> diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at
>> index 834ecc6..5f19665 100644
>> --- a/tests/ovs-vsctl.at
>> +++ b/tests/ovs-vsctl.at
>> @@ -7,6 +7,15 @@ m4_define([RUN_OVS_VSCTL],
>>  m4_foreach([command], [$@], [ovs-vsctl --no-reload --config=conf
> command
>>  ])])
>>
>> +dnl RUN_OVS_VSCTL_TOGETHER(COMMAND, ...)
>> +dnl
>> +dnl Executes each ovs-vsctl COMMAND on a file named "conf" in the
>> +dnl current directory, in a single run of ovs-vsctl.  Creates "conf" if
>> it
>> +dnl does not already exist.
>> +m4_define([RUN_OVS_VSCTL_TOGETHER],
>> +  [: >> conf
>> +   ovs-vsctl --no-reload --config=conf m4_join([ -- ], $@)])
>> +
>>  dnl CHECK_BRIDGES([BRIDGE, PARENT, VLAN], ...)
>>  dnl
>>  dnl Verifies that "ovs-vsctl list-br" prints the specified list of
>> bridges,
>> @@ -24,6 +33,12 @@ m4_define([_CHECK_BRIDGE],
>>     # 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
>> +])
>> +
>> +   # Check multiple queries in a single run.
>> +   AT_CHECK([RUN_OVS_VSCTL_TOGETHER([br-to-parent $1], [br-to-vlan
> $1])],
>> [0],
>> +[$2
>> +$3
>>  ])])
>>  m4_define([CHECK_BRIDGES],
>>    [dnl Check that the bridges appear on list-br, without --oneline.
>> @@ -152,7 +167,7 @@ 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(
>> +AT_CHECK([RUN_OVS_VSCTL_TOGETHER(
>>     [add-br a],
>>     [add-br b],
>>     [add-port a a1],
>> @@ -206,7 +221,7 @@ AT_CLEANUP
>>
>>  AT_SETUP([add-br a, add-bond a bond0 a1 a2 a3, del-port bond0])
>>  AT_KEYWORDS([ovs-vsctl])
>> -AT_CHECK([RUN_OVS_VSCTL(
>> +AT_CHECK([RUN_OVS_VSCTL_TOGETHER(
>>    [add-br a],
>>    [add-bond a bond0 a1 a2 a3],
>>    [del-port bond0])])
>> diff --git a/utilities/ovs-vsctl.8.in b/utilities/ovs-vsctl.8.in
>> index bc4cb11..9968ebe 100644
>> --- a/utilities/ovs-vsctl.8.in
>> +++ b/utilities/ovs-vsctl.8.in
>> @@ -12,6 +12,7 @@ ovs\-vsctl \- utility for querying and configuring
>> \fBovs\-vswitchd\fR
>>  .
>>  .SH SYNOPSIS
>>  \fBovs\-vsctl\fR [\fIoptions\fR] \fIcommand \fR[\fIargs\fR\&...]
>> +[\fB\-\-\fR \fIcommand \fR[\fIargs\fR\&...]]
>>  .
>>  .SH DESCRIPTION
>>  The \fBovs\-vsctl\fR program configures \fBovs\-vswitchd\fR(8), mainly
>> @@ -22,7 +23,8 @@ when \fBovs\-vswitchd\fR is running, but it can also
> be
>> used when
>>  changes will only take effect when \fBovs\-vswitchd\fR is started.
>>  .PP
>>  By default, each time \fBovs\-vsctl\fR runs, it examines and,
>> -depending on the requested command, possibly applies changes to an
>> +depending on the requested command or commands, possibly applies
>> +changes to an
>>  \fBovs\-vswitchd.conf\fR file.  Then, if it applied any changes and if
>>  \fBovs\-vswitchd\fR is running, it tells \fBovs\-vswitchd\fR to reload
>>  the modified configuration file and waits for the reload to complete
>> @@ -101,7 +103,7 @@ 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
>> +lines are printed as \fB\\n\fR, and any instances of \fB\\\fR that
>>  would otherwise appear in the output are doubled.
>>  .
>>  .SH COMMANDS
>> @@ -185,6 +187,16 @@ list.
>>  .IP "\fBiface\-to\-br \fIiface\fR"
>>  Prints the name of the bridge that contains \fIiface\fR on standard
>>  output.
>> +.SH "EXAMPLES"
>> +Create a new bridge named br0 and add port eth0 to it:
>> +.IP
>> +.B "ovs-vsctl add\-br br0"
>> +.br
>> +.B "ovs-vsctl add\-port br0 eth0"
>> +.PP
>> +Alternatively, perform both operations in a single atomic transaction:
>> +.IP
>> +.B "ovs-vsctl add\-br br0 \-\- add\-port br0 eth0"
>>  .
>>  .SH "EXIT STATUS"
>>  .IP "0"
>> diff --git a/utilities/ovs-vsctl.in b/utilities/ovs-vsctl.in
>> index 012ce4e..a419149 100755
>> --- a/utilities/ovs-vsctl.in
>> +++ b/utilities/ovs-vsctl.in
>> @@ -480,17 +480,66 @@ def cmd_br_to_parent(cfg, bridge):
>>      parent, vlan = find_bridge(cfg, bridge)
>>      return parent,
>>
>> +cmdTable = {'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)}
>> +
>> +# Break up commands at -- boundaries.
>> +def split_commands(args):
>> +    commands = []
>> +    command = []
>> +    for arg in args:
>> +        if arg == '--':
>> +            if command:
>> +                commands += [command]
>> +            command = []
>> +        else:
>> +            command += [arg]
>> +    if command:
>> +        commands += [command]
>> +    return commands
>> +
>> +def check_command(args):
>> +    command, args = args[0], args[1:]
>> +    if command not in cmdTable:
>> +        sys.stderr.write("%s: unknown command '%s' (use --help for
>> help)\n"
>> +                         % (argv0, command))
>> +        sys.exit(1)
>> +
>> +    function, is_mutator, nargs = cmdTable[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)
>> +
>> +def run_command(cfg, args):
>> +    command, args = args[0], args[1:]
>> +    function, need_lock, nargs = cmdTable[command]
>> +    return function(cfg, *args)
>> +
>>  def main():
>>      # Parse command line.
>>      try:
>> -        options, args = getopt.gnu_getopt(sys.argv[1:], "c:t:hV",
>> -                                          ["config=",
>> -                                           "target=",
>> -                                           "no-reload",
>> -                                           "no-syslog",
>> -                                           "oneline",
>> -                                           "help",
>> -                                           "version"])
>> +        options, args = getopt.getopt(sys.argv[1:], "c:t:hV",
>> +                                      ["config=",
>> +                                       "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)
>> @@ -525,51 +574,32 @@ def main():
>>          syslog.openlog("ovs-vsctl")
>>          log("Called as " + ' '.join(sys.argv[1:]))
>>
>> -    # Execute commands.
>> -    if not args:
>> +    # Break arguments into a series of commands.
>> +    commands = split_commands(args)
>> +    if not commands:
>>          sys.stderr.write("%s: missing command name (use --help for
>> help)\n"
>>                           % argv0)
>>          sys.exit(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)
>> +    # Check command syntax.
>> +    need_lock = False
>> +    for command in commands:
>> +        check_command(command)
>> +        if cmdTable[command[0]][1]:
>> +            need_lock = True
>>
>> -    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:
>> -        cfg = cfg_read(VSWITCHD_CONF, is_mutator)
>> -        output = function(cfg, *args)
>> +    # Execute commands.
>> +    cfg = cfg_read(VSWITCHD_CONF, need_lock)
>> +    for command in commands:
>> +        output = run_command(cfg, command)
>>          if output != None:
>>              if oneline:
>>                  print '\\n'.join([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 need_lock:
>> +        cfg_save(cfg, VSWITCHD_CONF)
>>
>>  if __name__ == "__main__":
>>      try:
>> --
>> 1.6.3.3
>>
>>
>> _______________________________________________
>> dev mailing list
>> dev at openvswitch.org
>> http://openvswitch.org/mailman/listinfo/dev_openvswitch.org
> 
> 
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev_openvswitch.org





More information about the dev mailing list