[ovs-dev,ovn] Add -u option to ovn-nbctl
diff mbox series

Message ID 20190801184338.4310-1-mmichels@redhat.com
State New
Headers show
Series
  • [ovs-dev,ovn] Add -u option to ovn-nbctl
Related show

Commit Message

Mark Michelson Aug. 1, 2019, 6:43 p.m. UTC
This option can be used in one of two ways.

When paired with --detach, this creates a unixctl socket with the name
given as the argument to '-u'.

When not paired with --detach, this tells the ovn-nbctl client the name
of the unixctl socket to attempt to connect to.

Signed-off-by: Mark Michelson <mmichels@redhat.com>
---
 tests/ovn.at              | 19 +++++++++++++++++++
 utilities/ovn-nbctl.8.xml | 20 ++++++++++++++++++++
 utilities/ovn-nbctl.c     | 17 +++++++++++++++--
 3 files changed, 54 insertions(+), 2 deletions(-)

Comments

Numan Siddique Aug. 2, 2019, 5:04 p.m. UTC | #1
On Fri, Aug 2, 2019 at 12:14 AM Mark Michelson <mmichels@redhat.com> wrote:

> This option can be used in one of two ways.
>
> When paired with --detach, this creates a unixctl socket with the name
> given as the argument to '-u'.
>
> When not paired with --detach, this tells the ovn-nbctl client the name
> of the unixctl socket to attempt to connect to.
>
> Signed-off-by: Mark Michelson <mmichels@redhat.com>
>

Acked-by: Numan Siddique <nusiddiq@redhat.com>



> ---
>  tests/ovn.at              | 19 +++++++++++++++++++
>  utilities/ovn-nbctl.8.xml | 20 ++++++++++++++++++++
>  utilities/ovn-nbctl.c     | 17 +++++++++++++++--
>  3 files changed, 54 insertions(+), 2 deletions(-)
>
> diff --git a/tests/ovn.at b/tests/ovn.at
> index e88cffa20..533594849 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -14990,3 +14990,22 @@ OVN_CHECK_PACKETS([hv2/vif3-tx.pcap], [expected])
>
>  OVN_CLEANUP([hv1], [hv2])
>  AT_CLEANUP
> +
> +AT_SETUP([ovn -- unixctl socket])
> +ovn_start
> +
> +sockfile="$at_group_dir/my_sock.ctl"
> +
> +# Specifying -u should fail since we have no daemon running
> +AT_CHECK([ovn-nbctl -u $sockfile show], [1], [ignore], [ignore])
> +
> +AT_CHECK_UNQUOTED([ovn-nbctl --detach -u $sockfile --pidfile], [0],
> [$sockfile
> +])
> +AT_CHECK([if test -f "$sockfile" ; then exit 99 ; fi])
> +on_exit 'kill $(cat ovn-nbctl.pid)'
> +
> +# We can't confirm that the nbctl client is actually using the sockfile,
> +# but we can still ensure that the command is successful.
> +AT_CHECK([ovn-nbctl -u $sockfile show])
> +
> +AT_CLEANUP
> diff --git a/utilities/ovn-nbctl.8.xml b/utilities/ovn-nbctl.8.xml
> index 41d50b694..fd75c0e44 100644
> --- a/utilities/ovn-nbctl.8.xml
> +++ b/utilities/ovn-nbctl.8.xml
> @@ -1085,6 +1085,26 @@
>        unset OVN_NB_DAEMON
>      </pre>
>
> +    <p>
> +      When using daemon mode, an alternative to the OVN_NB_DAEMON
> environment
> +      variable is to specify a path for the Unix socket. When starting the
> +      ovn-nbctl daemon, specify the <code>-u</code> option with a full
> path to
> +      the location of the socket file. Here is an exmple:
> +    </p>
> +
> +    <pre fixed="yes">
> +      ovn-nbctl --detach -u /tmp/mysock.ctl
> +    </pre>
> +
> +    <p>
> +      Then to connect to the running daemon, use the <code>-u</code>
> option
> +      with the full path to the socket created when the daemon was
> started:
> +    </p>
> +
> +    <pre fixed="yes">
> +      ovn-nbctl -u /tmp/mysock.ctl show
> +    </pre>
> +
>      <p>
>        Daemon mode is experimental.
>      </p>
> diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
> index ad999dd96..9a2e84530 100644
> --- a/utilities/ovn-nbctl.c
> +++ b/utilities/ovn-nbctl.c
> @@ -167,8 +167,9 @@ main(int argc, char *argv[])
>       *    - An OVN_NB_DAEMON environment variable implies client mode.
>       *
>       *    - Otherwise, we're in direct mode. */
> -    char *socket_name = getenv("OVN_NB_DAEMON");
> -    if (socket_name && socket_name[0]
> +    char *socket_name = unixctl_path ?: getenv("OVN_NB_DAEMON");
> +    if (((socket_name && socket_name[0])
> +         || has_option(parsed_options, n_parsed_options, 'u'))
>          && !will_detach(parsed_options, n_parsed_options)) {
>          nbctl_client(socket_name, parsed_options, n_parsed_options,
>                       argc, argv);
> @@ -422,6 +423,7 @@ get_all_options(void)
>          {"shuffle-remotes", no_argument, NULL, OPT_SHUFFLE_REMOTES},
>          {"no-shuffle-remotes", no_argument, NULL, OPT_NO_SHUFFLE_REMOTES},
>          {"version", no_argument, NULL, 'V'},
> +        {"unixctl", required_argument, NULL, 'u'},
>          MAIN_LOOP_LONG_OPTIONS,
>          DAEMON_LONG_OPTIONS,
>          VLOG_LONG_OPTIONS,
> @@ -533,6 +535,10 @@ apply_options_direct(const struct
> ovs_cmdl_parsed_option *parsed_options,
>              shuffle_remotes = false;
>              break;
>
> +        case 'u':
> +            unixctl_path = optarg;
> +            break;
> +
>          case 'V':
>              ovs_print_version(0, 0);
>              printf("DB Schema %s\n", nbrec_get_db_version());
> @@ -5996,6 +6002,10 @@ nbctl_client(const char *socket_name,
>                        po->o->name);
>              break;
>
> +        case 'u':
> +            socket_name = optarg;
> +            break;
> +
>          case 'V':
>              ovs_print_version(0, 0);
>              printf("DB Schema %s\n", nbrec_get_db_version());
> @@ -6020,6 +6030,9 @@ nbctl_client(const char *socket_name,
>              break;
>          }
>      }
> +
> +    ovs_assert(socket_name && socket_name[0]);
> +
>      svec_add(&args, "--");
>      for (int i = optind; i < argc; i++) {
>          svec_add(&args, argv[i]);
> --
> 2.14.5
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
Mark Michelson Aug. 6, 2019, 2:49 p.m. UTC | #2
Thanks, I pushed this to master.

On 8/2/19 1:04 PM, Numan Siddique wrote:
> 
> 
> On Fri, Aug 2, 2019 at 12:14 AM Mark Michelson <mmichels@redhat.com 
> <mailto:mmichels@redhat.com>> wrote:
> 
>     This option can be used in one of two ways.
> 
>     When paired with --detach, this creates a unixctl socket with the name
>     given as the argument to '-u'.
> 
>     When not paired with --detach, this tells the ovn-nbctl client the name
>     of the unixctl socket to attempt to connect to.
> 
>     Signed-off-by: Mark Michelson <mmichels@redhat.com
>     <mailto:mmichels@redhat.com>>
> 
> 
> Acked-by: Numan Siddique <nusiddiq@redhat.com <mailto:nusiddiq@redhat.com>>
> 
>     ---
>       tests/ovn.at <http://ovn.at>              | 19 +++++++++++++++++++
>       utilities/ovn-nbctl.8.xml | 20 ++++++++++++++++++++
>       utilities/ovn-nbctl.c     | 17 +++++++++++++++--
>       3 files changed, 54 insertions(+), 2 deletions(-)
> 
>     diff --git a/tests/ovn.at <http://ovn.at> b/tests/ovn.at <http://ovn.at>
>     index e88cffa20..533594849 100644
>     --- a/tests/ovn.at <http://ovn.at>
>     +++ b/tests/ovn.at <http://ovn.at>
>     @@ -14990,3 +14990,22 @@ OVN_CHECK_PACKETS([hv2/vif3-tx.pcap],
>     [expected])
> 
>       OVN_CLEANUP([hv1], [hv2])
>       AT_CLEANUP
>     +
>     +AT_SETUP([ovn -- unixctl socket])
>     +ovn_start
>     +
>     +sockfile="$at_group_dir/my_sock.ctl"
>     +
>     +# Specifying -u should fail since we have no daemon running
>     +AT_CHECK([ovn-nbctl -u $sockfile show], [1], [ignore], [ignore])
>     +
>     +AT_CHECK_UNQUOTED([ovn-nbctl --detach -u $sockfile --pidfile], [0],
>     [$sockfile
>     +])
>     +AT_CHECK([if test -f "$sockfile" ; then exit 99 ; fi])
>     +on_exit 'kill $(cat ovn-nbctl.pid)'
>     +
>     +# We can't confirm that the nbctl client is actually using the
>     sockfile,
>     +# but we can still ensure that the command is successful.
>     +AT_CHECK([ovn-nbctl -u $sockfile show])
>     +
>     +AT_CLEANUP
>     diff --git a/utilities/ovn-nbctl.8.xml b/utilities/ovn-nbctl.8.xml
>     index 41d50b694..fd75c0e44 100644
>     --- a/utilities/ovn-nbctl.8.xml
>     +++ b/utilities/ovn-nbctl.8.xml
>     @@ -1085,6 +1085,26 @@
>             unset OVN_NB_DAEMON
>           </pre>
> 
>     +    <p>
>     +      When using daemon mode, an alternative to the OVN_NB_DAEMON
>     environment
>     +      variable is to specify a path for the Unix socket. When
>     starting the
>     +      ovn-nbctl daemon, specify the <code>-u</code> option with a
>     full path to
>     +      the location of the socket file. Here is an exmple:
>     +    </p>
>     +
>     +    <pre fixed="yes">
>     +      ovn-nbctl --detach -u /tmp/mysock.ctl
>     +    </pre>
>     +
>     +    <p>
>     +      Then to connect to the running daemon, use the
>     <code>-u</code> option
>     +      with the full path to the socket created when the daemon was
>     started:
>     +    </p>
>     +
>     +    <pre fixed="yes">
>     +      ovn-nbctl -u /tmp/mysock.ctl show
>     +    </pre>
>     +
>           <p>
>             Daemon mode is experimental.
>           </p>
>     diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
>     index ad999dd96..9a2e84530 100644
>     --- a/utilities/ovn-nbctl.c
>     +++ b/utilities/ovn-nbctl.c
>     @@ -167,8 +167,9 @@ main(int argc, char *argv[])
>            *    - An OVN_NB_DAEMON environment variable implies client mode.
>            *
>            *    - Otherwise, we're in direct mode. */
>     -    char *socket_name = getenv("OVN_NB_DAEMON");
>     -    if (socket_name && socket_name[0]
>     +    char *socket_name = unixctl_path ?: getenv("OVN_NB_DAEMON");
>     +    if (((socket_name && socket_name[0])
>     +         || has_option(parsed_options, n_parsed_options, 'u'))
>               && !will_detach(parsed_options, n_parsed_options)) {
>               nbctl_client(socket_name, parsed_options, n_parsed_options,
>                            argc, argv);
>     @@ -422,6 +423,7 @@ get_all_options(void)
>               {"shuffle-remotes", no_argument, NULL, OPT_SHUFFLE_REMOTES},
>               {"no-shuffle-remotes", no_argument, NULL,
>     OPT_NO_SHUFFLE_REMOTES},
>               {"version", no_argument, NULL, 'V'},
>     +        {"unixctl", required_argument, NULL, 'u'},
>               MAIN_LOOP_LONG_OPTIONS,
>               DAEMON_LONG_OPTIONS,
>               VLOG_LONG_OPTIONS,
>     @@ -533,6 +535,10 @@ apply_options_direct(const struct
>     ovs_cmdl_parsed_option *parsed_options,
>                   shuffle_remotes = false;
>                   break;
> 
>     +        case 'u':
>     +            unixctl_path = optarg;
>     +            break;
>     +
>               case 'V':
>                   ovs_print_version(0, 0);
>                   printf("DB Schema %s\n", nbrec_get_db_version());
>     @@ -5996,6 +6002,10 @@ nbctl_client(const char *socket_name,
>                             po->o->name);
>                   break;
> 
>     +        case 'u':
>     +            socket_name = optarg;
>     +            break;
>     +
>               case 'V':
>                   ovs_print_version(0, 0);
>                   printf("DB Schema %s\n", nbrec_get_db_version());
>     @@ -6020,6 +6030,9 @@ nbctl_client(const char *socket_name,
>                   break;
>               }
>           }
>     +
>     +    ovs_assert(socket_name && socket_name[0]);
>     +
>           svec_add(&args, "--");
>           for (int i = optind; i < argc; i++) {
>               svec_add(&args, argv[i]);
>     -- 
>     2.14.5
> 
>     _______________________________________________
>     dev mailing list
>     dev@openvswitch.org <mailto:dev@openvswitch.org>
>     https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>

Patch
diff mbox series

diff --git a/tests/ovn.at b/tests/ovn.at
index e88cffa20..533594849 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -14990,3 +14990,22 @@  OVN_CHECK_PACKETS([hv2/vif3-tx.pcap], [expected])
 
 OVN_CLEANUP([hv1], [hv2])
 AT_CLEANUP
+
+AT_SETUP([ovn -- unixctl socket])
+ovn_start
+
+sockfile="$at_group_dir/my_sock.ctl"
+
+# Specifying -u should fail since we have no daemon running
+AT_CHECK([ovn-nbctl -u $sockfile show], [1], [ignore], [ignore])
+
+AT_CHECK_UNQUOTED([ovn-nbctl --detach -u $sockfile --pidfile], [0], [$sockfile
+])
+AT_CHECK([if test -f "$sockfile" ; then exit 99 ; fi])
+on_exit 'kill $(cat ovn-nbctl.pid)'
+
+# We can't confirm that the nbctl client is actually using the sockfile,
+# but we can still ensure that the command is successful.
+AT_CHECK([ovn-nbctl -u $sockfile show])
+
+AT_CLEANUP
diff --git a/utilities/ovn-nbctl.8.xml b/utilities/ovn-nbctl.8.xml
index 41d50b694..fd75c0e44 100644
--- a/utilities/ovn-nbctl.8.xml
+++ b/utilities/ovn-nbctl.8.xml
@@ -1085,6 +1085,26 @@ 
       unset OVN_NB_DAEMON
     </pre>
 
+    <p>
+      When using daemon mode, an alternative to the OVN_NB_DAEMON environment
+      variable is to specify a path for the Unix socket. When starting the
+      ovn-nbctl daemon, specify the <code>-u</code> option with a full path to
+      the location of the socket file. Here is an exmple:
+    </p>
+
+    <pre fixed="yes">
+      ovn-nbctl --detach -u /tmp/mysock.ctl
+    </pre>
+
+    <p>
+      Then to connect to the running daemon, use the <code>-u</code> option
+      with the full path to the socket created when the daemon was started:
+    </p>
+
+    <pre fixed="yes">
+      ovn-nbctl -u /tmp/mysock.ctl show
+    </pre>
+
     <p>
       Daemon mode is experimental.
     </p>
diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
index ad999dd96..9a2e84530 100644
--- a/utilities/ovn-nbctl.c
+++ b/utilities/ovn-nbctl.c
@@ -167,8 +167,9 @@  main(int argc, char *argv[])
      *    - An OVN_NB_DAEMON environment variable implies client mode.
      *
      *    - Otherwise, we're in direct mode. */
-    char *socket_name = getenv("OVN_NB_DAEMON");
-    if (socket_name && socket_name[0]
+    char *socket_name = unixctl_path ?: getenv("OVN_NB_DAEMON");
+    if (((socket_name && socket_name[0])
+         || has_option(parsed_options, n_parsed_options, 'u'))
         && !will_detach(parsed_options, n_parsed_options)) {
         nbctl_client(socket_name, parsed_options, n_parsed_options,
                      argc, argv);
@@ -422,6 +423,7 @@  get_all_options(void)
         {"shuffle-remotes", no_argument, NULL, OPT_SHUFFLE_REMOTES},
         {"no-shuffle-remotes", no_argument, NULL, OPT_NO_SHUFFLE_REMOTES},
         {"version", no_argument, NULL, 'V'},
+        {"unixctl", required_argument, NULL, 'u'},
         MAIN_LOOP_LONG_OPTIONS,
         DAEMON_LONG_OPTIONS,
         VLOG_LONG_OPTIONS,
@@ -533,6 +535,10 @@  apply_options_direct(const struct ovs_cmdl_parsed_option *parsed_options,
             shuffle_remotes = false;
             break;
 
+        case 'u':
+            unixctl_path = optarg;
+            break;
+
         case 'V':
             ovs_print_version(0, 0);
             printf("DB Schema %s\n", nbrec_get_db_version());
@@ -5996,6 +6002,10 @@  nbctl_client(const char *socket_name,
                       po->o->name);
             break;
 
+        case 'u':
+            socket_name = optarg;
+            break;
+
         case 'V':
             ovs_print_version(0, 0);
             printf("DB Schema %s\n", nbrec_get_db_version());
@@ -6020,6 +6030,9 @@  nbctl_client(const char *socket_name,
             break;
         }
     }
+
+    ovs_assert(socket_name && socket_name[0]);
+
     svec_add(&args, "--");
     for (int i = optind; i < argc; i++) {
         svec_add(&args, argv[i]);