Looks good, thanks. Ethan
On Tue, May 1, 2012 at 2:28 PM, Ben Pfaff <[email protected]> wrote: > I wish to add some unixctl commands to the Python vlog module. However, > importing ovs.unixctl in ovs.vlog creates a circular dependency, because > ovs.unixctl imports ovs.vlog already. The solution, in this commit, is to > break the unixctl module into three parts: a register (ovs.unixctl) that > does not depend on ovs.vlog, and client (ovs.unixctl.client) and server > (ovs.unixctl.server) modules that do. This breaks the circular dependency. > > Signed-off-by: Ben Pfaff <[email protected]> > --- > debian/ovs-monitor-ipsec | 3 +- > python/automake.mk | 4 +- > python/ovs/unixctl/__init__.py | 83 +++++++++++++++++++ > python/ovs/unixctl/client.py | 70 ++++++++++++++++ > python/ovs/{unixctl.py => unixctl/server.py} | 85 > +++----------------- > tests/appctl.py | 3 +- > tests/test-unixctl.py | 3 +- > .../usr_share_openvswitch_scripts_ovs-xapi-sync | 3 +- > 8 files changed, 174 insertions(+), 80 deletions(-) > create mode 100644 python/ovs/unixctl/__init__.py > create mode 100644 python/ovs/unixctl/client.py > rename python/ovs/{unixctl.py => unixctl/server.py} (75%) > > diff --git a/debian/ovs-monitor-ipsec b/debian/ovs-monitor-ipsec > index 5024277..a50a669 100755 > --- a/debian/ovs-monitor-ipsec > +++ b/debian/ovs-monitor-ipsec > @@ -38,6 +38,7 @@ import ovs.util > import ovs.daemon > import ovs.db.idl > import ovs.unixctl > +import ovs.unixctl.server > import ovs.vlog > > vlog = ovs.vlog.Vlog("ovs-monitor-ipsec") > @@ -414,7 +415,7 @@ def main(): > ovs.daemon.daemonize() > > ovs.unixctl.command_register("exit", "", 0, 0, unixctl_exit, None) > - error, unixctl_server = ovs.unixctl.UnixctlServer.create(None) > + error, unixctl_server = ovs.unixctl.server.UnixctlServer.create(None) > if error: > ovs.util.ovs_fatal(error, "could not create unixctl server", vlog) > > diff --git a/python/automake.mk b/python/automake.mk > index 59db000..01056a4 100644 > --- a/python/automake.mk > +++ b/python/automake.mk > @@ -29,7 +29,9 @@ ovs_pyfiles = \ > python/ovs/socket_util.py \ > python/ovs/stream.py \ > python/ovs/timeval.py \ > - python/ovs/unixctl.py \ > + python/ovs/unixctl/__init__.py \ > + python/ovs/unixctl/client.py \ > + python/ovs/unixctl/server.py \ > python/ovs/util.py \ > python/ovs/version.py \ > python/ovs/vlog.py > diff --git a/python/ovs/unixctl/__init__.py b/python/ovs/unixctl/__init__.py > new file mode 100644 > index 0000000..85643df > --- /dev/null > +++ b/python/ovs/unixctl/__init__.py > @@ -0,0 +1,83 @@ > +# Copyright (c) 2011, 2012 Nicira Networks > +# > +# Licensed under the Apache License, Version 2.0 (the "License"); > +# you may not use this file except in compliance with the License. > +# You may obtain a copy of the License at: > +# > +# http://www.apache.org/licenses/LICENSE-2.0 > +# > +# Unless required by applicable law or agreed to in writing, software > +# distributed under the License is distributed on an "AS IS" BASIS, > +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. > +# See the License for the specific language governing permissions and > +# limitations under the License. > + > +import types > + > +import ovs.util > + > +commands = {} > +strtypes = types.StringTypes > + > + > +class _UnixctlCommand(object): > + def __init__(self, usage, min_args, max_args, callback, aux): > + self.usage = usage > + self.min_args = min_args > + self.max_args = max_args > + self.callback = callback > + self.aux = aux > + > + > +def _unixctl_help(conn, unused_argv, unused_aux): > + reply = "The available commands are:\n" > + command_names = sorted(commands.keys()) > + for name in command_names: > + reply += " " > + usage = commands[name].usage > + if usage: > + reply += "%-23s %s" % (name, usage) > + else: > + reply += name > + reply += "\n" > + conn.reply(reply) > + > + > +def command_register(name, usage, min_args, max_args, callback, aux): > + """ Registers a command with the given 'name' to be exposed by the > + UnixctlServer. 'usage' describes the arguments to the command; it is used > + only for presentation to the user in "help" output. > + > + 'callback' is called when the command is received. It is passed a > + UnixctlConnection object, the list of arguments as unicode strings, and > + 'aux'. Normally 'callback' should reply by calling > + UnixctlConnection.reply() or UnixctlConnection.reply_error() before it > + returns, but if the command cannot be handled immediately, then it can > + defer the reply until later. A given connection can only process a > single > + request at a time, so a reply must be made eventually to avoid blocking > + that connection.""" > + > + assert isinstance(name, strtypes) > + assert isinstance(usage, strtypes) > + assert isinstance(min_args, int) > + assert isinstance(max_args, int) > + assert isinstance(callback, types.FunctionType) > + > + if name not in commands: > + commands[name] = _UnixctlCommand(usage, min_args, max_args, callback, > + aux) > + > +def socket_name_from_target(target): > + assert isinstance(target, strtypes) > + > + if target.startswith("/"): > + return 0, target > + > + pidfile_name = "%s/%s.pid" % (ovs.dirs.RUNDIR, target) > + pid = ovs.daemon.read_pidfile(pidfile_name) > + if pid < 0: > + return -pid, "cannot read pidfile \"%s\"" % pidfile_name > + > + return 0, "%s/%s.%d.ctl" % (ovs.dirs.RUNDIR, target, pid) > + > +command_register("help", "", 0, 0, _unixctl_help, None) > diff --git a/python/ovs/unixctl/client.py b/python/ovs/unixctl/client.py > new file mode 100644 > index 0000000..7005821 > --- /dev/null > +++ b/python/ovs/unixctl/client.py > @@ -0,0 +1,70 @@ > +# Copyright (c) 2011, 2012 Nicira Networks > +# > +# Licensed under the Apache License, Version 2.0 (the "License"); > +# you may not use this file except in compliance with the License. > +# You may obtain a copy of the License at: > +# > +# http://www.apache.org/licenses/LICENSE-2.0 > +# > +# Unless required by applicable law or agreed to in writing, software > +# distributed under the License is distributed on an "AS IS" BASIS, > +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. > +# See the License for the specific language governing permissions and > +# limitations under the License. > + > +import copy > +import errno > +import os > +import types > + > +import ovs.jsonrpc > +import ovs.stream > +import ovs.util > + > + > +vlog = ovs.vlog.Vlog("unixctl_client") > +strtypes = types.StringTypes > + > + > +class UnixctlClient(object): > + def __init__(self, conn): > + assert isinstance(conn, ovs.jsonrpc.Connection) > + self._conn = conn > + > + def transact(self, command, argv): > + assert isinstance(command, strtypes) > + assert isinstance(argv, list) > + for arg in argv: > + assert isinstance(arg, strtypes) > + > + request = ovs.jsonrpc.Message.create_request(command, argv) > + error, reply = self._conn.transact_block(request) > + > + if error: > + vlog.warn("error communicating with %s: %s" > + % (self._conn.name, os.strerror(error))) > + return error, None, None > + > + if reply.error is not None: > + return 0, str(reply.error), None > + else: > + assert reply.result is not None > + return 0, None, str(reply.result) > + > + def close(self): > + self._conn.close() > + self.conn = None > + > + @staticmethod > + def create(path): > + assert isinstance(path, str) > + > + unix = "unix:%s" % ovs.util.abs_file_name(ovs.dirs.RUNDIR, path) > + error, stream = ovs.stream.Stream.open_block( > + ovs.stream.Stream.open(unix)) > + > + if error: > + vlog.warn("failed to connect to %s" % path) > + return error, None > + > + return 0, UnixctlClient(ovs.jsonrpc.Connection(stream)) > diff --git a/python/ovs/unixctl.py b/python/ovs/unixctl/server.py > similarity index 75% > rename from python/ovs/unixctl.py > rename to python/ovs/unixctl/server.py > index 396d1be..c40b121 100644 > --- a/python/ovs/unixctl.py > +++ b/python/ovs/unixctl/server.py > @@ -17,89 +17,19 @@ import errno > import os > import types > > -import ovs.daemon > import ovs.dirs > import ovs.jsonrpc > import ovs.stream > +import ovs.unixctl > import ovs.util > import ovs.version > import ovs.vlog > > Message = ovs.jsonrpc.Message > -vlog = ovs.vlog.Vlog("unixctl") > -commands = {} > +vlog = ovs.vlog.Vlog("unixctl_server") > strtypes = types.StringTypes > > > -class _UnixctlCommand(object): > - def __init__(self, usage, min_args, max_args, callback, aux): > - self.usage = usage > - self.min_args = min_args > - self.max_args = max_args > - self.callback = callback > - self.aux = aux > - > - > -def _unixctl_help(conn, unused_argv, unused_aux): > - assert isinstance(conn, UnixctlConnection) > - reply = "The available commands are:\n" > - command_names = sorted(commands.keys()) > - for name in command_names: > - reply += " " > - usage = commands[name].usage > - if usage: > - reply += "%-23s %s" % (name, usage) > - else: > - reply += name > - reply += "\n" > - conn.reply(reply) > - > - > -def _unixctl_version(conn, unused_argv, version): > - assert isinstance(conn, UnixctlConnection) > - version = "%s (Open vSwitch) %s" % (ovs.util.PROGRAM_NAME, version) > - conn.reply(version) > - > - > -def command_register(name, usage, min_args, max_args, callback, aux): > - """ Registers a command with the given 'name' to be exposed by the > - UnixctlServer. 'usage' describes the arguments to the command; it is used > - only for presentation to the user in "help" output. > - > - 'callback' is called when the command is received. It is passed a > - UnixctlConnection object, the list of arguments as unicode strings, and > - 'aux'. Normally 'callback' should reply by calling > - UnixctlConnection.reply() or UnixctlConnection.reply_error() before it > - returns, but if the command cannot be handled immediately, then it can > - defer the reply until later. A given connection can only process a > single > - request at a time, so a reply must be made eventually to avoid blocking > - that connection.""" > - > - assert isinstance(name, strtypes) > - assert isinstance(usage, strtypes) > - assert isinstance(min_args, int) > - assert isinstance(max_args, int) > - assert isinstance(callback, types.FunctionType) > - > - if name not in commands: > - commands[name] = _UnixctlCommand(usage, min_args, max_args, callback, > - aux) > - > - > -def socket_name_from_target(target): > - assert isinstance(target, strtypes) > - > - if target.startswith("/"): > - return 0, target > - > - pidfile_name = "%s/%s.pid" % (ovs.dirs.RUNDIR, target) > - pid = ovs.daemon.read_pidfile(pidfile_name) > - if pid < 0: > - return -pid, "cannot read pidfile \"%s\"" % pidfile_name > - > - return 0, "%s/%s.%d.ctl" % (ovs.dirs.RUNDIR, target, pid) > - > - > class UnixctlConnection(object): > def __init__(self, rpc): > assert isinstance(rpc, ovs.jsonrpc.Connection) > @@ -177,7 +107,7 @@ class UnixctlConnection(object): > error = None > params = request.params > method = request.method > - command = commands.get(method) > + command = ovs.unixctl.commands.get(method) > if command is None: > error = '"%s" is not a valid command' % method > elif len(params) < command.min_args: > @@ -200,6 +130,11 @@ class UnixctlConnection(object): > self.reply_error(error) > > > +def _unixctl_version(conn, unused_argv, version): > + assert isinstance(conn, UnixctlConnection) > + version = "%s (Open vSwitch) %s" % (ovs.util.PROGRAM_NAME, version) > + conn.reply(version) > + > class UnixctlServer(object): > def __init__(self, listener): > assert isinstance(listener, ovs.stream.PassiveStream) > @@ -262,8 +197,8 @@ class UnixctlServer(object): > % path) > return error, None > > - command_register("help", "", 0, 0, _unixctl_help, None) > - command_register("version", "", 0, 0, _unixctl_version, version) > + ovs.unixctl.command_register("version", "", 0, 0, _unixctl_version, > + version) > > return 0, UnixctlServer(listener) > > diff --git a/tests/appctl.py b/tests/appctl.py > index 4a2865e..ed61c7f 100644 > --- a/tests/appctl.py > +++ b/tests/appctl.py > @@ -18,6 +18,7 @@ import sys > > import ovs.daemon > import ovs.unixctl > +import ovs.unixctl.client > import ovs.util > import ovs.vlog > > @@ -29,7 +30,7 @@ def connect_to_target(target): > else: > socket_name = str_result > > - error, client = ovs.unixctl.UnixctlClient.create(socket_name) > + error, client = ovs.unixctl.client.UnixctlClient.create(socket_name) > if error: > ovs.util.ovs_fatal(error, "cannot connect to \"%s\"" % socket_name) > > diff --git a/tests/test-unixctl.py b/tests/test-unixctl.py > index c262a95..392999e 100644 > --- a/tests/test-unixctl.py > +++ b/tests/test-unixctl.py > @@ -17,6 +17,7 @@ import sys > > import ovs.daemon > import ovs.unixctl > +import ovs.unixctl.server > > vlog = ovs.vlog.Vlog("test-unixctl") > exiting = False > @@ -55,7 +56,7 @@ def main(): > ovs.vlog.handle_args(args) > > ovs.daemon.daemonize_start() > - error, server = ovs.unixctl.UnixctlServer.create(args.unixctl) > + error, server = ovs.unixctl.server.UnixctlServer.create(args.unixctl) > if error: > ovs.util.ovs_fatal(error, "could not create unixctl server at %s" > % args.unixctl, vlog) > diff --git a/xenserver/usr_share_openvswitch_scripts_ovs-xapi-sync > b/xenserver/usr_share_openvswitch_scripts_ovs-xapi-sync > index 77c07bc..63cd614 100755 > --- a/xenserver/usr_share_openvswitch_scripts_ovs-xapi-sync > +++ b/xenserver/usr_share_openvswitch_scripts_ovs-xapi-sync > @@ -35,6 +35,7 @@ from ovs.db import types > import ovs.daemon > import ovs.db.idl > import ovs.unixctl > +import ovs.unixctl.server > > vlog = ovs.vlog.Vlog("ovs-xapi-sync") > session = None > @@ -236,7 +237,7 @@ def main(): > ovs.unixctl.command_register("exit", "", 0, 0, unixctl_exit, None) > ovs.unixctl.command_register("flush-cache", "", 0, 0, unixctl_flush_cache, > None) > - error, unixctl_server = ovs.unixctl.UnixctlServer.create(None) > + error, unixctl_server = ovs.unixctl.server.UnixctlServer.create(None) > if error: > ovs.util.ovs_fatal(error, "could not create unixctl server", vlog) > > -- > 1.7.2.5 > > _______________________________________________ > dev mailing list > [email protected] > http://openvswitch.org/mailman/listinfo/dev _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
