[ovs-dev] [PATCH v2 1/3] unixctl: Make path to unixctl_server socket available to the client.

[Aaron Conole]

Ben Pfaff <blp at ovn.org> writes:

> Acked-by: Alin Gabriel Serdean <aserdean at ovn.org>
> Signed-off-by: Ben Pfaff <blp at ovn.org>
> ---
>  lib/unixctl.c   | 52 ++++++++++++++++++++++++++++------------------------
>  lib/unixctl.h   |  2 ++
>  tests/daemon.at |  4 ++--
>  3 files changed, 32 insertions(+), 26 deletions(-)
>
> diff --git a/lib/unixctl.c b/lib/unixctl.c
> index bd9c1caeedef..9b3b0671f33c 100644
> --- a/lib/unixctl.c
> +++ b/lib/unixctl.c
> @@ -56,6 +56,7 @@ struct unixctl_conn {
>  struct unixctl_server {
>      struct pstream *listener;
>      struct ovs_list conns;
> +    char *path;
>  };
>  
>  static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 5);
> @@ -216,48 +217,44 @@ unixctl_command_reply_error(struct unixctl_conn *conn, const char *error)
>  int
>  unixctl_server_create(const char *path, struct unixctl_server **serverp)
>  {
> -    struct unixctl_server *server;
> -    struct pstream *listener;
> -    char *punix_path;
> -    int error;
> -
>      *serverp = NULL;
>      if (path && !strcmp(path, "none")) {
>          return 0;
>      }
>  
> -    if (path) {
> -        char *abs_path;
> -        abs_path = abs_file_name(ovs_rundir(), path);
> -        punix_path = xasprintf("punix:%s", abs_path);
> -        free(abs_path);
> -    } else {
> -#ifndef _WIN32
> -        punix_path = xasprintf("punix:%s/%s.%ld.ctl", ovs_rundir(),
> -                               program_name, (long int) getpid());
> +#ifdef _WIN32
> +    enum { WINDOWS = 1 };
>  #else
> -        punix_path = xasprintf("punix:%s/%s.ctl", ovs_rundir(), program_name);
> +    enum { WINDOWS = 0 };
>  #endif
> -    }
>  
> -    error = pstream_open(punix_path, &listener, 0);
> +    long int pid = getpid();
> +    char *abs_path
> +        = (path ? abs_file_name(ovs_rundir(), path)
> +           : WINDOWS ? xasprintf("%s/%s.ctl", ovs_rundir(), program_name)
> +           : xasprintf("%s/%s.%ld.ctl", ovs_rundir(), program_name, pid));
> +
> +    struct pstream *listener;
> +    char *punix_path = xasprintf("punix:%s", abs_path);
> +    int error = pstream_open(punix_path, &listener, 0);
> +    free(punix_path);
> +
>      if (error) {
> -        ovs_error(error, "could not initialize control socket %s", punix_path);
> -        goto exit;
> +        ovs_error(error, "%s: could not initialize control socket", abs_path);
> +        free(abs_path);
> +        return error;
>      }
>  
>      unixctl_command_register("list-commands", "", 0, 0, unixctl_list_commands,
>                               NULL);
>      unixctl_command_register("version", "", 0, 0, unixctl_version, NULL);
>  
> -    server = xmalloc(sizeof *server);
> +    struct unixctl_server *server = xmalloc(sizeof *server);
>      server->listener = listener;
> +    server->path = abs_path;
>      ovs_list_init(&server->conns);
>      *serverp = server;
> -
> -exit:
> -    free(punix_path);
> -    return error;
> +    return 0;
>  }
>  
>  static void
> @@ -429,10 +426,17 @@ unixctl_server_destroy(struct unixctl_server *server)
>              kill_connection(conn);
>          }
>  
> +        free (server->path);

Just a small nit, this looks like gnu style.

>          pstream_close(server->listener);
>          free(server);
>      }
>  }
> +
> +const char *
> +unixctl_server_get_path(const struct unixctl_server *server)
> +{
> +    return server ? server->path : NULL;
> +}
>  
>  /* On POSIX based systems, connects to a unixctl server socket.  'path' should
>   * be the name of a unixctl server socket.  If it does not start with '/', it
> diff --git a/lib/unixctl.h b/lib/unixctl.h
> index ce43893c6a7d..4562dbc49113 100644
> --- a/lib/unixctl.h
> +++ b/lib/unixctl.h
> @@ -28,6 +28,8 @@ void unixctl_server_run(struct unixctl_server *);
>  void unixctl_server_wait(struct unixctl_server *);
>  void unixctl_server_destroy(struct unixctl_server *);
>  
> +const char *unixctl_server_get_path(const struct unixctl_server *);
> +
>  /* Client for Unix domain socket control connection. */
>  struct jsonrpc;
>  int unixctl_client_create(const char *path, struct jsonrpc **client);
> diff --git a/tests/daemon.at b/tests/daemon.at
> index 952d5a7c7bbe..b379fa83f9aa 100644
> --- a/tests/daemon.at
> +++ b/tests/daemon.at
> @@ -149,7 +149,7 @@ AT_SETUP([daemon --detach startup errors])
>  AT_CAPTURE_FILE([pid])
>  OVSDB_INIT([db])
>  AT_CHECK([ovsdb-server --detach --no-chdir --pidfile --unixctl=nonexistent/unixctl db], [1], [], [stderr])
> -AT_CHECK([grep 'ovsdb-server: could not initialize control socket' stderr],
> +AT_CHECK([grep 'could not initialize control socket' stderr],
>    [0], [ignore])
>  AT_CHECK([test ! -s pid])
>  AT_CLEANUP
> @@ -159,7 +159,7 @@ AT_SKIP_IF([test "$IS_WIN32" = "yes"])
>  AT_CAPTURE_FILE([pid])
>  OVSDB_INIT([db])
>  AT_CHECK([ovsdb-server --detach --no-chdir --pidfile --monitor --unixctl=nonexistent/unixctl db], [1], [], [stderr])
> -AT_CHECK([grep 'ovsdb-server: could not initialize control socket' stderr],
> +AT_CHECK([grep 'could not initialize control socket' stderr],
>    [0], [ignore])
>  AT_CHECK([test ! -s pid])
>  AT_CLEANUP