diff mbox series

[ovs-dev,ovn,v2] ovn-nbctl: Create daemon control socket in ovn run dir

Message ID 20200407111453.1156652-1-numans@ovn.org
State Superseded
Headers show
Series [ovs-dev,ovn,v2] ovn-nbctl: Create daemon control socket in ovn run dir | expand

Commit Message

Numan Siddique April 7, 2020, 11:14 a.m. UTC
From: Numan Siddique <numans@ovn.org>

ovn-nbctl when run as a daemon is creating the ctl socket in
the ovs rundir. This patch fixes this issue by creating it in
the ovn rundir.

When an ovn service is run with -u option (which specifies the
ctl socket path) and if this path is not absolute, the ovn
ctl socket path is created in the ovs run dir. This patch
also fixes this issue by creating it in the ovn run dir.

Reported-by: Dan Williams <dcbw@redhat.com>
Signed-off-by: Numan Siddique <numans@ovn.org>
---
v1 -> v2 
------
  * Addressed the review comments from Ilya. If the ctl socket
    path is specified and if it is not absolute path it is
    now created in the ovn run dir.

 controller/ovn-controller.c |  2 +-
 ic/ovn-ic.c                 | 10 +++-------
 lib/ovn-util.c              |  9 +++++----
 lib/ovn-util.h              |  2 +-
 northd/ovn-northd.c         | 10 +++-------
 utilities/ovn-nbctl.c       |  6 +++++-
 6 files changed, 18 insertions(+), 21 deletions(-)

Comments

Ilya Maximets April 7, 2020, 1:04 p.m. UTC | #1
On 4/7/20 1:14 PM, numans@ovn.org wrote:
> From: Numan Siddique <numans@ovn.org>
> 
> ovn-nbctl when run as a daemon is creating the ctl socket in
> the ovs rundir. This patch fixes this issue by creating it in
> the ovn rundir.
> 
> When an ovn service is run with -u option (which specifies the
> ctl socket path) and if this path is not absolute, the ovn
> ctl socket path is created in the ovs run dir. This patch
> also fixes this issue by creating it in the ovn run dir.
> 
> Reported-by: Dan Williams <dcbw@redhat.com>
> Signed-off-by: Numan Siddique <numans@ovn.org>
> ---
> v1 -> v2 
> ------
>   * Addressed the review comments from Ilya. If the ctl socket
>     path is specified and if it is not absolute path it is
>     now created in the ovn run dir.
> 
>  controller/ovn-controller.c |  2 +-
>  ic/ovn-ic.c                 | 10 +++-------
>  lib/ovn-util.c              |  9 +++++----
>  lib/ovn-util.h              |  2 +-
>  northd/ovn-northd.c         | 10 +++-------
>  utilities/ovn-nbctl.c       |  6 +++++-
>  6 files changed, 18 insertions(+), 21 deletions(-)

What about ovn-controller-vtep and ovn-trace?
Numan Siddique April 8, 2020, 2:37 p.m. UTC | #2
On Tue, Apr 7, 2020 at 6:34 PM Ilya Maximets <i.maximets@ovn.org> wrote:
>
> On 4/7/20 1:14 PM, numans@ovn.org wrote:
> > From: Numan Siddique <numans@ovn.org>
> >
> > ovn-nbctl when run as a daemon is creating the ctl socket in
> > the ovs rundir. This patch fixes this issue by creating it in
> > the ovn rundir.
> >
> > When an ovn service is run with -u option (which specifies the
> > ctl socket path) and if this path is not absolute, the ovn
> > ctl socket path is created in the ovs run dir. This patch
> > also fixes this issue by creating it in the ovn run dir.
> >
> > Reported-by: Dan Williams <dcbw@redhat.com>
> > Signed-off-by: Numan Siddique <numans@ovn.org>
> > ---
> > v1 -> v2
> > ------
> >   * Addressed the review comments from Ilya. If the ctl socket
> >     path is specified and if it is not absolute path it is
> >     now created in the ovn run dir.
> >
> >  controller/ovn-controller.c |  2 +-
> >  ic/ovn-ic.c                 | 10 +++-------
> >  lib/ovn-util.c              |  9 +++++----
> >  lib/ovn-util.h              |  2 +-
> >  northd/ovn-northd.c         | 10 +++-------
> >  utilities/ovn-nbctl.c       |  6 +++++-
> >  6 files changed, 18 insertions(+), 21 deletions(-)
>
> What about ovn-controller-vtep and ovn-trace?

I missed them out of ignorance. Thanks.

I'll submit v3 soon.

Thanks
Numan

> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
diff mbox series

Patch

diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index 2893eaac1..4d21ba0fd 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -1729,7 +1729,7 @@  main(int argc, char *argv[])
 
     daemonize_start(true);
 
-    char *abs_unixctl_path = get_abs_unix_ctl_path();
+    char *abs_unixctl_path = get_abs_unix_ctl_path(NULL);
     retval = unixctl_server_create(abs_unixctl_path, &unixctl);
     free(abs_unixctl_path);
     if (retval) {
diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c
index bf8205de2..d931ca50f 100644
--- a/ic/ovn-ic.c
+++ b/ic/ovn-ic.c
@@ -1575,13 +1575,9 @@  main(int argc, char *argv[])
 
     daemonize_start(false);
 
-    if (!unixctl_path) {
-        char *abs_unixctl_path = get_abs_unix_ctl_path();
-        retval = unixctl_server_create(abs_unixctl_path, &unixctl);
-        free(abs_unixctl_path);
-    } else {
-        retval = unixctl_server_create(unixctl_path, &unixctl);
-    }
+    char *abs_unixctl_path = get_abs_unix_ctl_path(unixctl_path);
+    retval = unixctl_server_create(abs_unixctl_path, &unixctl);
+    free(abs_unixctl_path);
 
     if (retval) {
         exit(EXIT_FAILURE);
diff --git a/lib/ovn-util.c b/lib/ovn-util.c
index df18fda89..514e2489f 100644
--- a/lib/ovn-util.c
+++ b/lib/ovn-util.c
@@ -377,7 +377,7 @@  default_ic_sb_db(void)
 }
 
 char *
-get_abs_unix_ctl_path(void)
+get_abs_unix_ctl_path(const char *path)
 {
 #ifdef _WIN32
     enum { WINDOWS = 1 };
@@ -386,9 +386,10 @@  get_abs_unix_ctl_path(void)
 #endif
 
     long int pid = getpid();
-    char *abs_path =
-        WINDOWS ? xasprintf("%s/%s.ctl", ovn_rundir(), program_name)
-                : xasprintf("%s/%s.%ld.ctl", ovn_rundir(), program_name, pid);
+    char *abs_path
+        = (path ? abs_file_name(ovn_rundir(), path)
+           : WINDOWS ? xasprintf("%s/%s.ctl", ovn_rundir(), program_name)
+           : xasprintf("%s/%s.%ld.ctl", ovn_rundir(), program_name, pid));
     return abs_path;
 }
 
diff --git a/lib/ovn-util.h b/lib/ovn-util.h
index 32c8334b0..11238f61c 100644
--- a/lib/ovn-util.h
+++ b/lib/ovn-util.h
@@ -82,7 +82,7 @@  const char *default_nb_db(void);
 const char *default_sb_db(void);
 const char *default_ic_nb_db(void);
 const char *default_ic_sb_db(void);
-char *get_abs_unix_ctl_path(void);
+char *get_abs_unix_ctl_path(const char *path);
 
 struct ovsdb_idl_table_class;
 const char *db_table_usage(struct ds *tables,
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 076278197..29e1fd450 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -11600,13 +11600,9 @@  main(int argc, char *argv[])
 
     daemonize_start(false);
 
-    if (!unixctl_path) {
-        char *abs_unixctl_path = get_abs_unix_ctl_path();
-        retval = unixctl_server_create(abs_unixctl_path, &unixctl);
-        free(abs_unixctl_path);
-    } else {
-        retval = unixctl_server_create(unixctl_path, &unixctl);
-    }
+    char *abs_unixctl_path = get_abs_unix_ctl_path(unixctl_path);
+    retval = unixctl_server_create(abs_unixctl_path, &unixctl);
+    free(abs_unixctl_path);
 
     if (retval) {
         exit(EXIT_FAILURE);
diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
index 59abe0051..a88c1ddc2 100644
--- a/utilities/ovn-nbctl.c
+++ b/utilities/ovn-nbctl.c
@@ -6436,7 +6436,11 @@  server_loop(struct ovsdb_idl *idl, int argc, char *argv[])
 
     service_start(&argc, &argv);
     daemonize_start(false);
-    int error = unixctl_server_create(unixctl_path, &server);
+
+    char *abs_unixctl_path = get_abs_unix_ctl_path(unixctl_path);
+    int error = unixctl_server_create(abs_unixctl_path, &server);
+    free(abs_unixctl_path);
+
     if (error) {
         ctl_fatal("failed to create unixctl server (%s)",
                   ovs_retval_to_string(error));