From: Jakob Meng <[email protected]> A previous patch had changed the JSON-RPC API in lib/unixctl.* (and its Python counterpart) in order to allow transporting the requested output format from ovs-xxx tools to ovs-vswitchd across unix sockets:
The meaning of 'method' and 'params' had be changed: 'method' would be 'execute/v1' when an output format other than 'text' is requested. In that case, the first parameter of the JSON array 'params' would be the designated command, the second one the output format and the rest will be command args. That change allowed to transport the output format in a backward compatible way: One could use an updated client (ovs-appctl) with an old server (ovs-vswitchd) and vice versa. Of course, JSON output would only work when both sides have been updated. This patch reverts the JSON-RPC API to the old behaviour: The command will be copied to JSON-RPC's 'method' parameter while command args will be copied to the 'params' parameter. The client will inject the output format into the command args and the server will parse and remove it before passing the args to the command callback. The advantage of this (old) approach is a simpler JSON-RPC API but at the cost of inferior usability: ovs-xxx tools are no longer able to detect missing JSON support at the server side. Old servers will simply pass the output format as an arg to the command which would then result in an error such as 'command takes at most ... arguments' or a non-\ uniform error from the command callback. Signed-off-by: Jakob Meng <[email protected]> --- lib/unixctl.c | 147 ++++++++++++++++------------------- python/ovs/unixctl/client.py | 19 ++--- python/ovs/unixctl/server.py | 37 ++++----- python/ovs/util.py | 1 - 4 files changed, 91 insertions(+), 113 deletions(-) diff --git a/lib/unixctl.c b/lib/unixctl.c index e7edbb154..1eaf6edb0 100644 --- a/lib/unixctl.c +++ b/lib/unixctl.c @@ -63,8 +63,6 @@ static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 5); static struct shash commands = SHASH_INITIALIZER(&commands); -static const char *rpc_marker = "execute/v1"; - static void unixctl_list_commands(struct unixctl_conn *conn, int argc OVS_UNUSED, const char *argv[] OVS_UNUSED, @@ -292,11 +290,9 @@ process_command(struct unixctl_conn *conn, struct jsonrpc_msg *request) struct unixctl_command *command; struct json_array *params; - const char *method; - enum ovs_output_fmt fmt; + enum ovs_output_fmt fmt = OVS_OUTPUT_FMT_TEXT; struct svec argv = SVEC_EMPTY_INITIALIZER; - int args_offset; - bool plain_rpc; + int argc; COVERAGE_INC(unixctl_received); conn->request_id = json_clone(request->id); @@ -310,20 +306,9 @@ process_command(struct unixctl_conn *conn, struct jsonrpc_msg *request) free(id_s); } - /* The JSON-RPC API requires an indirection in order to allow transporting - * additional data like the output format besides command and args. For - * backward compatibility with older clients the plain RPC is still - * supported. */ - plain_rpc = strcmp(request->method, rpc_marker); - args_offset = plain_rpc ? 0 : 2; - params = json_array(request->params); - if (!plain_rpc && (params->n < 2)) { - error = xasprintf("JSON-RPC API mismatch: Unexpected # of params:"\ - " %"PRIuSIZE, params->n); - goto error; - } + /* Verify type of arguments. */ for (size_t i = 0; i < params->n; i++) { if (params->elems[i]->type != JSON_STRING) { error = xasprintf("command has non-string argument: %s", @@ -332,54 +317,70 @@ process_command(struct unixctl_conn *conn, struct jsonrpc_msg *request) } } - /* Extract method name. */ - method = plain_rpc ? request->method : json_string(params->elems[0]); + argc = params->n; - /* Extract output format. */ - if (plain_rpc) { - fmt = OVS_OUTPUT_FMT_TEXT; - } else { - if (!ovs_output_fmt_from_string(json_string(params->elems[1]), &fmt)) { - error = xasprintf("invalid output format: %s", - json_string(params->elems[1])); - goto error; + /* Extract and process command args. */ + svec_add(&argv, request->method); + for (size_t i = 0; i < params->n; i++) { + if (!strcmp(json_string(params->elems[i]), "-f") || + !strcmp(json_string(params->elems[i]), "--format")) + { + /* Parse output format argument. */ + + if ((i + 1) == (params->n)) { + error = xasprintf("option requires an argument -- %s", + json_string(params->elems[i])); + goto error; + } + + /* Move index to option argument. */ + i++; + + if (!ovs_output_fmt_from_string(json_string(params->elems[i]), + &fmt)) + { + error = xasprintf("option %s has invalid value %s", + json_string(params->elems[i - 1]), + json_string(params->elems[i])); + goto error; + } + + argc -= 2; + } else { + /* Pass other arguments to command. */ + svec_add(&argv, json_string(params->elems[i])); } } + svec_terminate(&argv); /* Find command with method name. */ - command = shash_find_data(&commands, method); + command = shash_find_data(&commands, request->method); /* Verify that method call complies with command requirements. */ if (!command) { error = xasprintf("\"%s\" is not a valid command (use " "\"list-commands\" to see a list of valid commands)", - method); + request->method); goto error; - } else if ((params->n - args_offset) < command->min_args) { + } else if (argc < command->min_args) { error = xasprintf("\"%s\" command requires at least %d arguments", - method, command->min_args); + request->method, command->min_args); goto error; - } else if ((params->n - args_offset) > command->max_args) { + } else if (argc > command->max_args) { error = xasprintf("\"%s\" command takes at most %d arguments", - method, command->max_args); + request->method, command->max_args); goto error; } else if ((!command->output_fmts && fmt != OVS_OUTPUT_FMT_TEXT) || (command->output_fmts && !(fmt & command->output_fmts))) { error = xasprintf("\"%s\" command does not support output format"\ - " \"%s\" %d %d", method, - ovs_output_fmt_to_string(fmt), command->output_fmts, + " \"%s\" %d %d", request->method, + ovs_output_fmt_to_string(fmt), + command->output_fmts, fmt); goto error; } - /* Extract command args. */ - svec_add(&argv, method); - for (size_t i = args_offset; i < params->n; i++) { - svec_add(&argv, json_string(params->elems[i])); - } - svec_terminate(&argv); - command->cb(conn, argv.n, (const char **) argv.names, fmt, command->aux); svec_destroy(&argv); @@ -387,6 +388,9 @@ process_command(struct unixctl_conn *conn, struct jsonrpc_msg *request) return; error: unixctl_command_reply_error(conn, error); + if (!svec_is_empty(&argv)) { + svec_destroy(&argv); + } free(error); } @@ -558,39 +562,32 @@ unixctl_client_transact(struct jsonrpc *client, const char *command, int argc, { struct jsonrpc_msg *request, *reply; struct json **json_args, *params; - int error, i; - /* The JSON-RPC API requires an indirection in order to allow transporting - * additional data like the output format besides command and args. For - * backward compatibility with older servers the plain RPC is still - * supported. */ - bool plain_rpc = (fmt == OVS_OUTPUT_FMT_TEXT); + int error, i, argcx = 0; + + if (fmt != OVS_OUTPUT_FMT_TEXT) { + argcx += 2; + } *result = NULL; *err = NULL; - if (plain_rpc) { - json_args = xmalloc(argc * sizeof *json_args); - for (i = 0; i < argc; i++) { - json_args[i] = json_string_create(argv[i]); - } - - params = json_array_create(json_args, argc); - request = jsonrpc_create_request(command, params, NULL); - } else { - json_args = xmalloc((argc + 2) * sizeof *json_args); - json_args[0] = json_string_create(command); - json_args[1] = ovs_output_fmt_to_json(fmt); - for (i = 0; i < argc; i++) { - json_args[i + 2] = json_string_create(argv[i]); - } - - params = json_array_create(json_args, argc + 2); + json_args = xmalloc((argc + argcx) * sizeof *json_args); + for (i = 0; i < argc; i++) { + json_args[i] = json_string_create(argv[i]); + } - /* Use a versioned command to ensure that both server and client - * use the same JSON-RPC API. */ - request = jsonrpc_create_request(rpc_marker, params, NULL); + /* Pass output format for non-text only to keep compatibility with old + servers. */ + if (fmt != OVS_OUTPUT_FMT_TEXT) { + json_args[i] = json_string_create("--format"); + ++i; + json_args[i] = ovs_output_fmt_to_json(fmt); + ++i; } + params = json_array_create(json_args, argc + argcx); + request = jsonrpc_create_request(command, params, NULL); + error = jsonrpc_transact_block(client, request, &reply); if (error) { VLOG_WARN("error communicating with %s: %s", jsonrpc_get_name(client), @@ -600,17 +597,7 @@ unixctl_client_transact(struct jsonrpc *client, const char *command, int argc, if (reply->error) { if (reply->error->type == JSON_STRING) { - /* Catch incompatible server and return helpful error msg. */ - char *plain_rpc_error = xasprintf("\"%s\" is not a valid command", - rpc_marker); - if (!strncmp(plain_rpc_error, json_string(reply->error), - strlen(plain_rpc_error))) { - *err = xstrdup("JSON RPC reply indicates incompatible server. " - "Please upgrade server side to newer version."); - } else { - *err = xstrdup(json_string(reply->error)); - } - free(plain_rpc_error); + *err = xstrdup(json_string(reply->error)); } else { VLOG_WARN("%s: unexpected error type in JSON RPC reply: %s", jsonrpc_get_name(client), diff --git a/python/ovs/unixctl/client.py b/python/ovs/unixctl/client.py index 29a5f3df9..bd690d099 100644 --- a/python/ovs/unixctl/client.py +++ b/python/ovs/unixctl/client.py @@ -32,13 +32,11 @@ class UnixctlClient(object): for arg in argv: assert isinstance(arg, str) assert isinstance(fmt, ovs.util.OutputFormat) - plain_rpc = (fmt == ovs.util.OutputFormat.TEXT) - if plain_rpc: - request = ovs.jsonrpc.Message.create_request(command, argv) - else: - request = ovs.jsonrpc.Message.create_request( - "execute/v1", [command, fmt.name.lower()] + argv) + request = ovs.jsonrpc.Message.create_request( + command, + argv + ([] if (fmt == ovs.util.OutputFormat.TEXT) + else ["--format", fmt.name.lower()])) error, reply = self._conn.transact_block(request) @@ -48,14 +46,7 @@ class UnixctlClient(object): return error, None, None if reply.error is not None: - plain_rpc_error = ('"%s" is not a valid command' % - ovs.util.RPC_MARKER) - if str(reply.error).startswith(plain_rpc_error): - return 0, "JSON RPC reply indicates incompatible "\ - "server. Please upgrade server side to "\ - "newer version.", None - else: - return 0, str(reply.error), None + return 0, str(reply.error), None else: assert reply.result is not None return 0, None, str(reply.result) diff --git a/python/ovs/unixctl/server.py b/python/ovs/unixctl/server.py index 849739e89..2db855948 100644 --- a/python/ovs/unixctl/server.py +++ b/python/ovs/unixctl/server.py @@ -12,6 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +import argparse import copy import errno import os @@ -105,34 +106,32 @@ class UnixctlConnection(object): self._request_id = request.id try: - plain_rpc = request.method != ovs.util.RPC_MARKER - args_offset = 0 if plain_rpc else 2 - - if not plain_rpc and len(request.params) < 2: - raise ValueError( - "JSON-RPC API mismatch: Unexpected # of params: %d" - % len(request.params)) - for param in request.params: if not isinstance(param, str): raise ValueError( "command has non-string argument: %s" % param) - method = request.method if plain_rpc else request.params[0] - if plain_rpc: - fmt = ovs.util.OutputFormat.TEXT - else: - fmt = ovs.util.OutputFormat[request.params[1].upper()] - params = request.params[args_offset:] + params = request.params + + parser = argparse.ArgumentParser() + parser.add_argument("argv", nargs="*") + parser.add_argument("-f", "--format", + default="text", + choices=[fmt.name.lower() + for fmt in ovs.util.OutputFormat]) + args = parser.parse_args(params) + fmt = ovs.util.OutputFormat[args.format.upper()] + method = request.method command = ovs.unixctl.commands.get(method) + if command is None: raise ValueError('"%s" is not a valid command' % method) - elif len(params) < command.min_args: + elif len(args.argv) < command.min_args: raise ValueError( '"%s" command requires at least %d arguments' % (method, command.min_args)) - elif len(params) > command.max_args: + elif len(args.argv) > command.max_args: raise ValueError( '"%s" command takes at most %d arguments' % (method, command.max_args)) @@ -141,8 +140,10 @@ class UnixctlConnection(object): ' "%s" %d %d' % (method, fmt.name.lower(), command.output_fmts, fmt)) - unicode_params = [str(p) for p in params] - command.callback(self, unicode_params, fmt, command.aux) + command.callback(self, + args.argv, + fmt, + command.aux) except Exception as e: self.reply_error(str(e)) diff --git a/python/ovs/util.py b/python/ovs/util.py index 75ba4ee12..272ca683d 100644 --- a/python/ovs/util.py +++ b/python/ovs/util.py @@ -19,7 +19,6 @@ import enum PROGRAM_NAME = os.path.basename(sys.argv[0]) EOF = -1 -RPC_MARKER = "execute/v1" @enum.unique -- 2.39.2 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
