diff mbox series

[ovs-dev,v8,1/6] Add global option for JSON output to ovs-appctl.

Message ID 20240411144718.1921869-2-jmeng@redhat.com
State Superseded, archived
Headers show
Series Add global option to output JSON from ovs-appctl cmds. | expand

Checks

Context Check Description
ovsrobot/apply-robot warning apply and check: warning
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/intel-ovs-compilation fail test: fail

Commit Message

Jakob Meng April 11, 2024, 2:47 p.m. UTC
From: Jakob Meng <code@jakobmeng.de>

For monitoring systems such as Prometheus it would be beneficial if
OVS would expose statistics in a machine-readable format.

This patch introduces support for different output formats to ovs-xxx
tools, in particular ovs-appctl. The latter gains a global option
'-f,--format' which changes it to print a JSON document instead of
plain-text for humans. For example, a later patch implements support
for 'ovs-appctl --format json dpif/show'. By default, the output format
is plain-text as before.

A new 'set-options' command has been added to lib/unixctl.c which
allows to change the output format of the commands executed afterwards
on the same socket connection. It is supposed to be run by ovs-xxx
tools transparently for the user when a specific output format has been
requested.
For example, when a user calls 'ovs-appctl --format json dpif/show',
then ovs-appctl will call 'set-options' to set the output format as
requested by the user and afterwards it will call the actual command
'dpif/show'.
This ovs-appctl behaviour has been implemented in a backward compatible
way. One can use an updated client (ovs-appctl) with an old server
(ovs-vswitchd) and vice versa. Of course, JSON output only works when
both sides have been updated.

Two access functions unixctl_command_{get,set}_output_format() and
two unixctl_command_reply_{,error_}json functions have been added to
lib/unixctl.h:
unixctl_command_get_output_format() is supposed to be used in commands
like 'dpif/show' to query the requested output format. When JSON output
has been selected, both unixctl_command_reply_{,error_}json() functions
can be used to return JSON objects to the client (ovs-appctl) instead
of plain-text with the unixctl_command_reply{,_error}() functions.

When JSON has been requested but a command has not implemented JSON
output the plain-text output will be wrapped in a provisional JSON
document with the following structure:

  {"reply-format":"plain","reply":"$PLAIN_TEXT_HERE"}

Thus commands which have been executed successfully will not fail when
they try to render the output at a later stage.

In popular tools like kubectl the option for output control is usually
called '-o|--output' instead of '-f,--format'. But ovs-appctl already
has an short option '-o' which prints the available ovs-appctl options
('--option'). The now choosen name also better alignes with
ovsdb-client where '-f,--format' controls output formatting.

Reported-at: https://bugzilla.redhat.com/1824861
Signed-off-by: Jakob Meng <code@jakobmeng.de>
---
 NEWS                   |   3 +
 lib/command-line.c     |  36 ++++++++++
 lib/command-line.h     |  10 +++
 lib/unixctl.c          | 158 ++++++++++++++++++++++++++++++++---------
 lib/unixctl.h          |  11 +++
 tests/ovs-vswitchd.at  |  11 +++
 utilities/ovs-appctl.c |  98 +++++++++++++++++++++----
 7 files changed, 277 insertions(+), 50 deletions(-)

Comments

0-day Robot April 11, 2024, 2:59 p.m. UTC | #1
Bleep bloop.  Greetings Jakob Meng, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
WARNING: Line lacks whitespace around operator
WARNING: Line lacks whitespace around operator
#583 FILE: utilities/ovs-appctl.c:145:
  -f, --format=FMT   Output format. One of: 'json', or 'text'\n\

Lines checked: 675, Warnings: 2, Errors: 0


Please check this out.  If you feel there has been an error, please email aconole@redhat.com

Thanks,
0-day Robot
diff mbox series

Patch

diff --git a/NEWS b/NEWS
index b92cec532..03ef6486b 100644
--- a/NEWS
+++ b/NEWS
@@ -7,6 +7,9 @@  Post-v3.3.0
    - The primary development branch has been renamed from 'master' to 'main'.
      The OVS tree remains hosted on GitHub.
      https://github.com/openvswitch/ovs.git
+   - ovs-appctl:
+     * Added new option [-f|--format] to choose the output format, e.g. 'json'
+       or 'text' (by default).
 
 
 v3.3.0 - 16 Feb 2024
diff --git a/lib/command-line.c b/lib/command-line.c
index 967f4f5d5..775e0430a 100644
--- a/lib/command-line.c
+++ b/lib/command-line.c
@@ -24,6 +24,7 @@ 
 #include "ovs-thread.h"
 #include "util.h"
 #include "openvswitch/vlog.h"
+#include "openvswitch/json.h"
 
 VLOG_DEFINE_THIS_MODULE(command_line);
 
@@ -53,6 +54,41 @@  ovs_cmdl_long_options_to_short_options(const struct option options[])
     return xstrdup(short_options);
 }
 
+const char *
+ovs_output_fmt_to_string(enum ovs_output_fmt fmt)
+{
+    switch (fmt) {
+    case OVS_OUTPUT_FMT_TEXT:
+        return "text";
+
+    case OVS_OUTPUT_FMT_JSON:
+        return "json";
+
+    default:
+        return NULL;
+    }
+}
+
+struct json *
+ovs_output_fmt_to_json(enum ovs_output_fmt fmt)
+{
+    const char *string = ovs_output_fmt_to_string(fmt);
+    return string ? json_string_create(string) : NULL;
+}
+
+bool
+ovs_output_fmt_from_string(const char *string, enum ovs_output_fmt *fmt)
+{
+    if (!strcmp(string, "text")) {
+        *fmt = OVS_OUTPUT_FMT_TEXT;
+    } else if (!strcmp(string, "json")) {
+        *fmt = OVS_OUTPUT_FMT_JSON;
+    } else {
+        return false;
+    }
+    return true;
+}
+
 static char * OVS_WARN_UNUSED_RESULT
 build_short_options(const struct option *long_options)
 {
diff --git a/lib/command-line.h b/lib/command-line.h
index fc2a2690f..494274cec 100644
--- a/lib/command-line.h
+++ b/lib/command-line.h
@@ -20,6 +20,7 @@ 
 /* Utilities for command-line parsing. */
 
 #include "compiler.h"
+#include <openvswitch/list.h>
 
 struct option;
 
@@ -46,6 +47,15 @@  struct ovs_cmdl_command {
 
 char *ovs_cmdl_long_options_to_short_options(const struct option *options);
 
+enum ovs_output_fmt {
+    OVS_OUTPUT_FMT_TEXT = 1 << 0,
+    OVS_OUTPUT_FMT_JSON = 1 << 1
+};
+
+const char *ovs_output_fmt_to_string(enum ovs_output_fmt);
+struct json *ovs_output_fmt_to_json(enum ovs_output_fmt);
+bool ovs_output_fmt_from_string(const char *, enum ovs_output_fmt *);
+
 struct ovs_cmdl_parsed_option {
     const struct option *o;
     char *arg;
diff --git a/lib/unixctl.c b/lib/unixctl.c
index 103357ee9..18638d86a 100644
--- a/lib/unixctl.c
+++ b/lib/unixctl.c
@@ -21,7 +21,6 @@ 
 #include "coverage.h"
 #include "dirs.h"
 #include "openvswitch/dynamic-string.h"
-#include "openvswitch/json.h"
 #include "jsonrpc.h"
 #include "openvswitch/list.h"
 #include "openvswitch/poll-loop.h"
@@ -50,6 +49,8 @@  struct unixctl_conn {
     /* Only one request can be in progress at a time.  While the request is
      * being processed, 'request_id' is populated, otherwise it is null. */
     struct json *request_id;   /* ID of the currently active request. */
+
+    enum ovs_output_fmt fmt;
 };
 
 /* Server for control connection. */
@@ -94,6 +95,39 @@  unixctl_version(struct unixctl_conn *conn, int argc OVS_UNUSED,
     unixctl_command_reply(conn, ovs_get_program_version());
 }
 
+static void
+unixctl_set_options(struct unixctl_conn *conn, int argc, const char *argv[],
+                    void *aux OVS_UNUSED)
+{
+    char * error = NULL;
+
+    for (size_t i = 1; i < argc; i++) {
+        if ((i + 1) == argc) {
+            error = xasprintf("option requires an argument -- %s", argv[i]);
+            goto error;
+        } else if (!strcmp(argv[i], "--format")) {
+            /* Move index to option argument. */
+            i++;
+
+            /* Parse output format argument. */
+            if (!ovs_output_fmt_from_string(argv[i], &conn->fmt)) {
+                error = xasprintf("option %s has invalid value %s",
+                                  argv[i - 1], argv[i]);
+                goto error;
+            }
+        } else {
+            error = xasprintf("invalid option -- %s", argv[i]);
+            goto error;
+
+        }
+    }
+    unixctl_command_reply(conn, NULL);
+    return;
+error:
+    unixctl_command_reply_error(conn, error);
+    free(error);
+}
+
 /* Registers a unixctl command with the given 'name'.  'usage' describes the
  * arguments to the command; it is used only for presentation to the user in
  * "list-commands" output.  (If 'usage' is NULL, then the command is hidden.)
@@ -128,36 +162,68 @@  unixctl_command_register(const char *name, const char *usage,
     shash_add(&commands, name, command);
 }
 
-static void
-unixctl_command_reply__(struct unixctl_conn *conn,
-                        bool success, const char *body)
+enum ovs_output_fmt
+unixctl_command_get_output_format(struct unixctl_conn *conn)
 {
-    struct json *body_json;
-    struct jsonrpc_msg *reply;
+    return conn->fmt;
+}
 
-    COVERAGE_INC(unixctl_replied);
-    ovs_assert(conn->request_id);
+void
+unixctl_command_set_output_format(struct unixctl_conn *conn,
+                                  enum ovs_output_fmt fmt)
+{
+    conn->fmt = fmt;
+}
+
+static struct json *
+json_reply_create__(struct unixctl_conn *conn, const char *body)
+{
+    struct json * json_body;
 
     if (!body) {
         body = "";
     }
 
     if (body[0] && body[strlen(body) - 1] != '\n') {
-        body_json = json_string_create_nocopy(xasprintf("%s\n", body));
+        json_body = json_string_create_nocopy(xasprintf("%s\n", body));
     } else {
-        body_json = json_string_create(body);
+        json_body = json_string_create(body);
     }
 
+    if (conn->fmt == OVS_OUTPUT_FMT_TEXT) {
+        return json_body;
+    }
+
+    struct json *json_reply = json_object_create();
+
+    json_object_put_string(json_reply, "reply-format", "plain");
+    json_object_put(json_reply, "reply", json_body);
+
+    return json_reply;
+}
+
+/* Takes ownership of 'body'. */
+static void
+unixctl_command_reply__(struct unixctl_conn *conn,
+                        bool success, struct json *body)
+{
+    struct jsonrpc_msg *reply;
+
+    COVERAGE_INC(unixctl_replied);
+    ovs_assert(conn->request_id);
+
     if (success) {
-        reply = jsonrpc_create_reply(body_json, conn->request_id);
+        reply = jsonrpc_create_reply(body, conn->request_id);
     } else {
-        reply = jsonrpc_create_error(body_json, conn->request_id);
+        reply = jsonrpc_create_error(body, conn->request_id);
     }
 
     if (VLOG_IS_DBG_ENABLED()) {
         char *id = json_to_string(conn->request_id, 0);
+        char *msg = json_to_string(body, 0);
         VLOG_DBG("replying with %s, id=%s: \"%s\"",
-                 success ? "success" : "error", id, body);
+                 success ? "success" : "error", id, msg);
+        free(msg);
         free(id);
     }
 
@@ -170,20 +236,40 @@  unixctl_command_reply__(struct unixctl_conn *conn,
 
 /* Replies to the active unixctl connection 'conn'.  'result' is sent to the
  * client indicating the command was processed successfully.  Only one call to
- * unixctl_command_reply() or unixctl_command_reply_error() may be made per
- * request. */
+ * the unixctl_command_reply*() functions may be made per request. */
 void
 unixctl_command_reply(struct unixctl_conn *conn, const char *result)
 {
-    unixctl_command_reply__(conn, true, result);
+    unixctl_command_reply__(conn, true, json_reply_create__(conn, result));
+}
+
+/* Replies to the active unixctl connection 'conn'.  'result' is sent to the
+ * client indicating the command was processed successfully.  Only one call to
+ * the unixctl_command_reply*() functions may be made per request.
+ *
+ * Takes ownership of 'body'. */
+void
+unixctl_command_reply_json(struct unixctl_conn *conn, struct json *body)
+{
+    unixctl_command_reply__(conn, true, body);
 }
 
 /* Replies to the active unixctl connection 'conn'. 'error' is sent to the
- * client indicating an error occurred processing the command.  Only one call to
- * unixctl_command_reply() or unixctl_command_reply_error() may be made per
- * request. */
+ * client indicating an error occurred processing the command.  Only one call
+ * to the unixctl_command_reply*() functions may be made per request. */
 void
 unixctl_command_reply_error(struct unixctl_conn *conn, const char *error)
+{
+    unixctl_command_reply__(conn, false, json_reply_create__(conn, error));
+}
+
+/* Replies to the active unixctl connection 'conn'.  'error' is sent to the
+ * client indicating an error occurred processing the command.  Only one call
+ * to the unixctl_command_reply*() functions may be made per request.
+ *
+ * Takes ownership of 'error'. */
+void
+unixctl_command_reply_error_json(struct unixctl_conn *conn, struct json *error)
 {
     unixctl_command_reply__(conn, false, error);
 }
@@ -250,6 +336,8 @@  unixctl_server_create(const char *path, struct unixctl_server **serverp)
     unixctl_command_register("list-commands", "", 0, 0, unixctl_list_commands,
                              NULL);
     unixctl_command_register("version", "", 0, 0, unixctl_version, NULL);
+    unixctl_command_register("set-options", "[--format text|json]", 2, 2,
+                             unixctl_set_options, NULL);
 
     struct unixctl_server *server = xmalloc(sizeof *server);
     server->listener = listener;
@@ -381,6 +469,7 @@  unixctl_server_run(struct unixctl_server *server)
             struct unixctl_conn *conn = xzalloc(sizeof *conn);
             ovs_list_push_back(&server->conns, &conn->node);
             conn->rpc = jsonrpc_open(stream);
+            conn->fmt = OVS_OUTPUT_FMT_TEXT;
         } else if (error == EAGAIN) {
             break;
         } else {
@@ -483,10 +572,12 @@  unixctl_client_create(const char *path, struct jsonrpc **client)
  * '*err' if not NULL. */
 int
 unixctl_client_transact(struct jsonrpc *client, const char *command, int argc,
-                        char *argv[], char **result, char **err)
+                        char *argv[], enum ovs_output_fmt fmt, char **result,
+                        char **err)
 {
     struct jsonrpc_msg *request, *reply;
-    struct json **json_args, *params;
+    struct json **json_args, *params, *output_src;
+    char **output_dst;
     int error, i;
 
     *result = NULL;
@@ -506,22 +597,19 @@  unixctl_client_transact(struct jsonrpc *client, const char *command, int argc,
         return error;
     }
 
-    if (reply->error) {
-        if (reply->error->type == JSON_STRING) {
-            *err = xstrdup(json_string(reply->error));
+    output_src = reply->error ? reply->error : reply->result;
+    output_dst = reply->error ? err : result;
+
+    if (output_src) {
+        if (fmt == OVS_OUTPUT_FMT_TEXT && output_src->type == JSON_STRING) {
+            *output_dst = xstrdup(json_string(output_src));
+        } else if (fmt == OVS_OUTPUT_FMT_JSON) {
+            *output_dst = json_to_string(output_src, 0);
         } else {
-            VLOG_WARN("%s: unexpected error type in JSON RPC reply: %s",
+            VLOG_WARN("%s: unexpected %s type in JSON rpc reply: %s",
                       jsonrpc_get_name(client),
-                      json_type_to_string(reply->error->type));
-            error = EINVAL;
-        }
-    } else if (reply->result) {
-        if (reply->result->type == JSON_STRING) {
-            *result = xstrdup(json_string(reply->result));
-        } else {
-            VLOG_WARN("%s: unexpected result type in JSON rpc reply: %s",
-                      jsonrpc_get_name(client),
-                      json_type_to_string(reply->result->type));
+                      reply->error ? "error" : "result",
+                      json_type_to_string(output_src->type));
             error = EINVAL;
         }
     }
diff --git a/lib/unixctl.h b/lib/unixctl.h
index 4562dbc49..b30bcf092 100644
--- a/lib/unixctl.h
+++ b/lib/unixctl.h
@@ -17,6 +17,9 @@ 
 #ifndef UNIXCTL_H
 #define UNIXCTL_H 1
 
+#include "openvswitch/json.h"
+#include "command-line.h"
+
 #ifdef  __cplusplus
 extern "C" {
 #endif
@@ -36,6 +39,7 @@  int unixctl_client_create(const char *path, struct jsonrpc **client);
 int unixctl_client_transact(struct jsonrpc *client,
                             const char *command,
                             int argc, char *argv[],
+                            enum ovs_output_fmt fmt,
                             char **result, char **error);
 
 /* Command registration. */
@@ -45,8 +49,15 @@  typedef void unixctl_cb_func(struct unixctl_conn *,
 void unixctl_command_register(const char *name, const char *usage,
                               int min_args, int max_args,
                               unixctl_cb_func *cb, void *aux);
+enum ovs_output_fmt unixctl_command_get_output_format(struct unixctl_conn *);
+void unixctl_command_set_output_format(struct unixctl_conn *,
+                                       enum ovs_output_fmt);
 void unixctl_command_reply_error(struct unixctl_conn *, const char *error);
+void unixctl_command_reply_error_json(struct unixctl_conn *,
+                                      struct json *error);
 void unixctl_command_reply(struct unixctl_conn *, const char *body);
+void unixctl_command_reply_json(struct unixctl_conn *,
+                                struct json *body);
 
 #ifdef  __cplusplus
 }
diff --git a/tests/ovs-vswitchd.at b/tests/ovs-vswitchd.at
index 977b2eba1..32ca2c6e7 100644
--- a/tests/ovs-vswitchd.at
+++ b/tests/ovs-vswitchd.at
@@ -265,3 +265,14 @@  OFPT_FEATURES_REPLY: dpid:$orig_dpid
 
 OVS_VSWITCHD_STOP
 AT_CLEANUP
+
+AT_SETUP([ovs-vswitchd version])
+OVS_VSWITCHD_START
+
+AT_CHECK([ovs-appctl version], [0], [ignore])
+ovs_version=$(ovs-appctl version)
+
+AT_CHECK_UNQUOTED([ovs-appctl --format json version], [0], [dnl
+{"reply-format":"plain","reply":"$ovs_version\n"}])
+
+AT_CLEANUP
diff --git a/utilities/ovs-appctl.c b/utilities/ovs-appctl.c
index ba0c172e6..4d4503597 100644
--- a/utilities/ovs-appctl.c
+++ b/utilities/ovs-appctl.c
@@ -29,44 +29,84 @@ 
 #include "jsonrpc.h"
 #include "process.h"
 #include "timeval.h"
+#include "svec.h"
 #include "unixctl.h"
 #include "util.h"
 #include "openvswitch/vlog.h"
 
 static void usage(void);
-static const char *parse_command_line(int argc, char *argv[]);
+
+/* Parsed command line args. */
+struct cmdl_args {
+    enum ovs_output_fmt format;
+    char *target;
+};
+
+static struct cmdl_args *cmdl_args_create(void);
+static void cmdl_args_destroy(struct cmdl_args *);
+static struct cmdl_args *parse_command_line(int argc, char *argv[]);
 static struct jsonrpc *connect_to_target(const char *target);
 
 int
 main(int argc, char *argv[])
 {
+    struct svec opt_argv = SVEC_EMPTY_INITIALIZER;
     char *cmd_result, *cmd_error;
     struct jsonrpc *client;
+    struct cmdl_args *args;
     char *cmd, **cmd_argv;
-    const char *target;
     int cmd_argc;
     int error;
 
     set_program_name(argv[0]);
 
     /* Parse command line and connect to target. */
-    target = parse_command_line(argc, argv);
-    client = connect_to_target(target);
+    args = parse_command_line(argc, argv);
+    client = connect_to_target(args->target);
 
-    /* Transact request and process reply. */
+    /* Transact options request (if required) and process reply */
+    if (args->format != OVS_OUTPUT_FMT_TEXT) {
+        svec_add(&opt_argv, "--format");
+        svec_add(&opt_argv, ovs_output_fmt_to_string(args->format));
+    }
+    svec_terminate(&opt_argv);
+
+    if (!svec_is_empty(&opt_argv)) {
+        error = unixctl_client_transact(client, "set-options",
+                                        opt_argv.n, opt_argv.names,
+                                        args->format, &cmd_result,
+                                        &cmd_error);
+
+        if (error) {
+            ovs_fatal(error, "%s: transaction error", args->target);
+        }
+
+        if (cmd_error) {
+            jsonrpc_close(client);
+            fputs(cmd_error, stderr);
+            ovs_error(0, "%s: server returned an error", args->target);
+            exit(2);
+        }
+
+        free(cmd_result);
+        free(cmd_error);
+    }
+    svec_destroy(&opt_argv);
+
+    /* Transact command request and process reply. */
     cmd = argv[optind++];
     cmd_argc = argc - optind;
     cmd_argv = cmd_argc ? argv + optind : NULL;
     error = unixctl_client_transact(client, cmd, cmd_argc, cmd_argv,
-                                    &cmd_result, &cmd_error);
+                                    args->format, &cmd_result, &cmd_error);
     if (error) {
-        ovs_fatal(error, "%s: transaction error", target);
+        ovs_fatal(error, "%s: transaction error", args->target);
     }
 
     if (cmd_error) {
         jsonrpc_close(client);
         fputs(cmd_error, stderr);
-        ovs_error(0, "%s: server returned an error", target);
+        ovs_error(0, "%s: server returned an error", args->target);
         exit(2);
     } else if (cmd_result) {
         fputs(cmd_result, stdout);
@@ -74,6 +114,7 @@  main(int argc, char *argv[])
         OVS_NOT_REACHED();
     }
 
+    cmdl_args_destroy(args);
     jsonrpc_close(client);
     free(cmd_result);
     free(cmd_error);
@@ -101,13 +142,31 @@  Common commands:\n\
   vlog/reopen        Make the program reopen its log file\n\
 Other options:\n\
   --timeout=SECS     wait at most SECS seconds for a response\n\
+  -f, --format=FMT   Output format. One of: 'json', or 'text'\n\
+                     ('text', by default)\n\
   -h, --help         Print this helpful information\n\
   -V, --version      Display ovs-appctl version information\n",
            program_name, program_name);
     exit(EXIT_SUCCESS);
 }
 
-static const char *
+static struct cmdl_args *
+cmdl_args_create(void) {
+    struct cmdl_args *args = xmalloc(sizeof *args);
+
+    args->format = OVS_OUTPUT_FMT_TEXT;
+    args->target = NULL;
+
+    return args;
+}
+
+static void
+cmdl_args_destroy(struct cmdl_args *args) {
+    free(args->target);
+    free(args);
+}
+
+static struct cmdl_args *
 parse_command_line(int argc, char *argv[])
 {
     enum {
@@ -117,6 +176,7 @@  parse_command_line(int argc, char *argv[])
     static const struct option long_options[] = {
         {"target", required_argument, NULL, 't'},
         {"execute", no_argument, NULL, 'e'},
+        {"format", required_argument, NULL, 'f'},
         {"help", no_argument, NULL, 'h'},
         {"option", no_argument, NULL, 'o'},
         {"version", no_argument, NULL, 'V'},
@@ -126,11 +186,10 @@  parse_command_line(int argc, char *argv[])
     };
     char *short_options_ = ovs_cmdl_long_options_to_short_options(long_options);
     char *short_options = xasprintf("+%s", short_options_);
-    const char *target;
-    int e_options;
+    struct cmdl_args *args = cmdl_args_create();
     unsigned int timeout = 0;
+    int e_options;
 
-    target = NULL;
     e_options = 0;
     for (;;) {
         int option;
@@ -141,10 +200,10 @@  parse_command_line(int argc, char *argv[])
         }
         switch (option) {
         case 't':
-            if (target) {
+            if (args->target) {
                 ovs_fatal(0, "-t or --target may be specified only once");
             }
-            target = optarg;
+            args->target = xstrdup(optarg);
             break;
 
         case 'e':
@@ -157,6 +216,12 @@  parse_command_line(int argc, char *argv[])
             }
             break;
 
+        case 'f':
+            if (!ovs_output_fmt_from_string(optarg, &args->format)) {
+                ovs_fatal(0, "value %s on -f or --format is invalid", optarg);
+            }
+            break;
+
         case 'h':
             usage();
             break;
@@ -194,7 +259,10 @@  parse_command_line(int argc, char *argv[])
                   "(use --help for help)");
     }
 
-    return target ? target : "ovs-vswitchd";
+    if (!args->target) {
+        args->target = xstrdup("ovs-vswitchd");
+    }
+    return args;
 }
 
 static struct jsonrpc *