Make ovs-appctl easier to use and synchronize its interface with ovs-vsctl.

It is inconvenient to type the whole path to the Unix daemon socket when
using ovs-appctl.  Allow the name of the daemon to be used instead when
a pidfile exists in the default location, and contact ovs-vswitchd by
default.

Also, the various options for manipulating vlog were invented before the
general-purpose command mechanism existed.  Get rid of all of the action
options in favor of just specifying the command to be executed as
non-option arguments.

Finally, there simply wasn't much value in allowing multiple targets or
options to be specified; these variations were never used in practice.  So
simplify the interface by making it one target, one action per invocation.

Also, make ovs-vsctl use the same syntax for its --target option.

Based on work by Justin Pettit.

1 files changed, 115 insertions, 140 deletions

diff --git a/utilities/ovs-appctl.c b/utilities/ovs-appctl.c
index 934966235..060e8e286 100644
--- a/utilities/ovs-appctl.c
+++ b/utilities/ovs-appctl.c
@@ -13,193 +13,132 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+
 #include <config.h>
-#include "vlog.h"
 
-#include <dirent.h>
 #include <errno.h>
 #include <getopt.h>
-#include <stdarg.h>
 #include <stdio.h>
 #include <string.h>
 #include <stdlib.h>
 
 #include "command-line.h"
-#include "compiler.h"
+#include "daemon.h"
+#include "dirs.h"
+#include "dynamic-string.h"
 #include "timeval.h"
 #include "unixctl.h"
 #include "util.h"
 
-static void
-usage(char *prog_name, int exit_code)
-{
-    printf("Usage: %s [TARGET] [ACTION...]\n"
-           "Targets:\n"
-           "  -t, --target=TARGET  Path to Unix domain socket\n"
-           "Actions:\n"
-           "  -l, --list         List current settings\n"
-           "  -s, --set=MODULE[:FACILITY[:LEVEL]]\n"
-           "        Set MODULE and FACILITY log level to LEVEL\n"
-           "        MODULE may be any valid module name or 'ANY'\n"
-           "        FACILITY may be 'syslog', 'console', 'file', or 'ANY' (default)\n"
-           "        LEVEL may be 'emer', 'err', 'warn', 'info', or 'dbg' (default)\n"
-           "  -r, --reopen       Make the program reopen its log file\n"
-           "  -e, --execute=COMMAND  Execute control COMMAND and print its output\n"
-           "Other options:\n"
-           "  -h, --help         Print this helpful information\n"
-           "  -V, --version      Display version information\n",
-           prog_name);
-    exit(exit_code);
-}
+static void usage(void);
+static const char *parse_command_line(int argc, char *argv[]);
+static struct unixctl_client *connect_to_target(const char *target);
 
-static char *
-transact(struct unixctl_client *client, const char *request, bool *ok)
+int
+main(int argc, char *argv[])
 {
-    int code;
+    struct unixctl_client *client;
+    const char *target;
+    struct ds request;
+    int code, error;
     char *reply;
-    int error = unixctl_client_transact(client, request, &code, &reply);
-    if (error) {
-        fprintf(stderr, "%s: transaction error: %s\n",
-                unixctl_client_target(client), strerror(error));
-        *ok = false;
-        return xstrdup("");
-    } else {
-        if (code / 100 != 2) {
-            fprintf(stderr, "%s: server returned reply code %03d\n",
-                    unixctl_client_target(client), code);
+    int i;
+
+    set_program_name(argv[0]);
+    time_init();
+
+    /* Parse command line and connect to target. */
+    target = parse_command_line(argc, argv);
+    client = connect_to_target(target);
+
+    /* Compose request. */
+    ds_init(&request);
+    for (i = optind; i < argc; i++) {
+        if (i != optind) {
+            ds_put_char(&request, ' ');
         }
-        return reply;
+        ds_put_cstr(&request, argv[i]);
     }
-}
 
-static void
-transact_ack(struct unixctl_client *client, const char *request, bool *ok)
-{
-    free(transact(client, request, ok));
-}
-
-static void
-execute_command(struct unixctl_client *client, const char *request, bool *ok)
-{
-    int code;
-    char *reply;
-    int error = unixctl_client_transact(client, request, &code, &reply);
+    /* Transact request and process reply. */
+    error = unixctl_client_transact(client, ds_cstr(&request), &code, &reply);
     if (error) {
-        fprintf(stderr, "%s: transaction error: %s\n",
-                unixctl_client_target(client), strerror(error));
-        *ok = false;
-    } else {
-        if (code / 100 != 2) {
-            fprintf(stderr, "%s: server returned reply code %03d\n",
-                    unixctl_client_target(client), code);
-            fputs(reply, stderr);
-            *ok = false;
-        } else {
-            fputs(reply, stdout);
-        }
+        ovs_fatal(error, "%s: transaction error", target);
+    }
+    if (code / 100 != 2) {
+        ovs_error(0, "%s: server returned reply code %03d", target, code);
+        exit(2);
     }
+    fputs(reply, stdout);
+
+    return 0;
 }
 
 static void
-add_target(struct unixctl_client ***clients, size_t *n_clients,
-           const char *path, bool *ok)
+usage(void)
 {
-    struct unixctl_client *client;
-    int error = unixctl_client_create(path, &client);
-    if (error) {
-        fprintf(stderr, "Error connecting to \"%s\": %s\n",
-                path, strerror(error));
-        *ok = false;
-    } else {
-        *clients = xrealloc(*clients, sizeof *clients * (*n_clients + 1));
-        (*clients)[*n_clients] = client;
-        ++*n_clients;
-    }
+    printf("%s, for querying and controlling Open vSwitch daemon\n"
+           "usage: %s [TARGET] COMMAND [ARG...]\n"
+           "Targets:\n"
+           "  -t, --target=TARGET  pidfile or socket to contact\n"
+           "Common commands:"
+           "  help               List commands supported by the target\n"
+           "  vlog/list          List current logging levels\n"
+           "  vlog/set MODULE[:FACILITY[:LEVEL]]\n"
+           "        Set MODULE and FACILITY log level to LEVEL\n"
+           "        MODULE may be any valid module name or 'ANY'\n"
+           "        FACILITY may be 'syslog', 'console', 'file', or 'ANY' (default)\n"
+           "        LEVEL may be 'emer', 'err', 'warn', 'info', or 'dbg' (default)\n"
+           "  vlog/reopen        Make the program reopen its log file\n"
+           "Other options:\n"
+           "  -h, --help         Print this helpful information\n"
+           "  -V, --version      Display version information\n",
+           program_name, program_name);
+    exit(EXIT_SUCCESS);
 }
 
-int main(int argc, char *argv[])
+static const char *
+parse_command_line(int argc, char *argv[])
 {
     static const struct option long_options[] = {
-        /* Target options must come first. */
         {"target", required_argument, NULL, 't'},
+        {"execute", no_argument, NULL, 'e'},
         {"help", no_argument, NULL, 'h'},
         {"version", no_argument, NULL, 'V'},
-
-        /* Action options come afterward. */
-        {"list", no_argument, NULL, 'l'},
-        {"set", required_argument, NULL, 's'},
-        {"reopen", no_argument, NULL, 'r'},
-        {"execute", required_argument, NULL, 'e'},
         {0, 0, 0, 0},
     };
-    char *short_options;
-
-    /* Determine targets. */
-    bool ok = true;
-    int n_actions = 0;
-    struct unixctl_client **clients = NULL;
-    size_t n_clients = 0;
+    const char *target;
+    int e_options;
 
-    set_program_name(argv[0]);
-    time_init();
-
-    short_options = long_options_to_short_options(long_options);
+    target = NULL;
+    e_options = 0;
     for (;;) {
         int option;
-        size_t i;
 
-        option = getopt_long(argc, argv, short_options, long_options, NULL);
+        option = getopt_long(argc, argv, "+t:hVe", long_options, NULL);
         if (option == -1) {
             break;
         }
-        if (!strchr("thV", option) && n_clients == 0) {
-            ovs_fatal(0, "no targets specified (use --help for help)");
-        } else {
-            ++n_actions;
-        }
         switch (option) {
         case 't':
-            add_target(&clients, &n_clients, optarg, &ok);
-            break;
-
-        case 'l':
-            for (i = 0; i < n_clients; i++) {
-                struct unixctl_client *client = clients[i];
-                char *reply;
-
-                printf("%s:\n", unixctl_client_target(client));
-                reply = transact(client, "vlog/list", &ok);
-                fputs(reply, stdout);
-                free(reply);
-            }
-            break;
-
-        case 's':
-            for (i = 0; i < n_clients; i++) {
-                struct unixctl_client *client = clients[i];
-                char *request = xasprintf("vlog/set %s", optarg);
-                transact_ack(client, request, &ok);
-                free(request);
-            }
-            break;
-
-        case 'r':
-            for (i = 0; i < n_clients; i++) {
-                struct unixctl_client *client = clients[i];
-                char *request = xstrdup("vlog/reopen");
-                transact_ack(client, request, &ok);
-                free(request);
+            if (target) {
+                ovs_fatal(0, "-t or --target may be specified only once");
             }
+            target = optarg;
             break;
 
         case 'e':
-            for (i = 0; i < n_clients; i++) {
-                execute_command(clients[i], optarg, &ok);
+            /* We ignore -e for compatibility.  Older versions specified the
+             * command as the argument to -e.  Since the current version takes
+             * the command as non-option arguments and we say that -e has no
+             * arguments, this just works in the common case. */
+            if (e_options++) {
+                ovs_fatal(0, "-e or --execute may be speciifed only once");
             }
             break;
 
         case 'h':
-            usage(argv[0], EXIT_SUCCESS);
+            usage();
             break;
 
         case 'V':
@@ -213,9 +152,45 @@ int main(int argc, char *argv[])
             NOT_REACHED();
         }
     }
-    if (!n_actions) {
-        fprintf(stderr,
-                "warning: no actions specified (use --help for help)\n");
+
+    if (optind >= argc) {
+        ovs_fatal(0, "at least one non-option argument is required "
+                  "(use --help for help)");
+    }
+
+    return target ? target : "ovs-vswitchd";
+}
+
+static struct unixctl_client *
+connect_to_target(const char *target)
+{
+    struct unixctl_client *client;
+    char *socket_name;
+    int error;
+
+    if (target[0] != '/') {
+        char *pidfile_name;
+        char *socket_name;
+        pid_t pid;
+
+        pidfile_name = xasprintf("%s/%s.pid", ovs_rundir, target);
+        pid = read_pidfile(pidfile_name);
+        if (pid < 0) {
+            ovs_fatal(-pid, "cannot read pidfile \"%s\"", pidfile_name);
+        }
+        free(pidfile_name);
+        socket_name = xasprintf("%s/%s.%ld.ctl",
+                                ovs_rundir, target, (long int) pid);
+    } else {
+        socket_name = xstrdup(target);
+    }
+
+    error = unixctl_client_create(socket_name, &client);
+    if (error) {
+        ovs_fatal(error, "cannot connect to \"%s\"", socket_name);
     }
-    exit(ok ? 0 : 1);
+    free(socket_name);
+
+    return client;
 }
+