diff mbox series

[ovs-dev,ovn] Create daemon pidfiles in ovn run dir.

Message ID 20200421124130.973829-1-numans@ovn.org
State Superseded
Headers show
Series [ovs-dev,ovn] Create daemon pidfiles in ovn run dir. | expand

Commit Message

Numan Siddique April 21, 2020, 12:41 p.m. UTC 
From: Numan Siddique <numans@ovn.org>

If an OVN service is started with --pidfile option, the pidfile
is created in the ovs rundir. This patch fixes it by using the ovn rundir
if either the pidfile is not specified or if specified, it is not
absolute path.

Signed-off-by: Numan Siddique <numans@ovn.org>
---
 controller-vtep/ovn-controller-vtep.c |  6 +--
 controller/ovn-controller.c           |  6 +--
 lib/ovn-util.c                        | 26 ++++++++++++
 lib/ovn-util.h                        | 61 +++++++++++++++++++++++++++
 northd/ovn-northd.c                   |  6 +--
 utilities/ovn-nbctl.c                 | 10 ++---
 6 files changed, 101 insertions(+), 14 deletions(-)

Comments

0-day Robot April 21, 2020, 1:01 p.m. UTC | #1 
Bleep bloop.  Greetings Numan Siddique, 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.


git-am:
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch' to see the failed patch
Patch failed at 0001 Create daemon pidfiles in ovn run dir.
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".


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

Thanks,
0-day Robot
Dumitru Ceara April 22, 2020, 7:40 p.m. UTC | #2 
On 4/21/20 2:41 PM, numans@ovn.org wrote:
> From: Numan Siddique <numans@ovn.org>
> 
> If an OVN service is started with --pidfile option, the pidfile
> is created in the ovs rundir. This patch fixes it by using the ovn rundir
> if either the pidfile is not specified or if specified, it is not
> absolute path.
> 
> Signed-off-by: Numan Siddique <numans@ovn.org>

Hi Numan,

Overall, looks good to me, however shouldn't we use
OVN_DAEMON_OPTION_ENUMS in ovn-trace and ovn-ic too?

A few minor comments inline.

Thanks,
Dumitru

> ---
>  controller-vtep/ovn-controller-vtep.c |  6 +--
>  controller/ovn-controller.c           |  6 +--
>  lib/ovn-util.c                        | 26 ++++++++++++
>  lib/ovn-util.h                        | 61 +++++++++++++++++++++++++++
>  northd/ovn-northd.c                   |  6 +--
>  utilities/ovn-nbctl.c                 | 10 ++---
>  6 files changed, 101 insertions(+), 14 deletions(-)
> 
> diff --git a/controller-vtep/ovn-controller-vtep.c b/controller-vtep/ovn-controller-vtep.c
> index 253a709ab..c13280bc0 100644
> --- a/controller-vtep/ovn-controller-vtep.c
> +++ b/controller-vtep/ovn-controller-vtep.c
> @@ -169,7 +169,7 @@ parse_options(int argc, char *argv[])
>          OPT_PEER_CA_CERT = UCHAR_MAX + 1,
>          OPT_BOOTSTRAP_CA_CERT,
>          VLOG_OPTION_ENUMS,
> -        DAEMON_OPTION_ENUMS,
> +        OVN_DAEMON_OPTION_ENUMS,
>          SSL_OPTION_ENUMS,
>      };
>  
> @@ -179,7 +179,7 @@ parse_options(int argc, char *argv[])
>          {"help", no_argument, NULL, 'h'},
>          {"version", no_argument, NULL, 'V'},
>          VLOG_LONG_OPTIONS,
> -        DAEMON_LONG_OPTIONS,
> +        OVN_DAEMON_LONG_OPTIONS,
>          STREAM_SSL_LONG_OPTIONS,
>          {"peer-ca-cert", required_argument, NULL, OPT_PEER_CA_CERT},
>          {"bootstrap-ca-cert", required_argument, NULL, OPT_BOOTSTRAP_CA_CERT},
> @@ -212,7 +212,7 @@ parse_options(int argc, char *argv[])
>              exit(EXIT_SUCCESS);
>  
>          VLOG_OPTION_HANDLERS
> -        DAEMON_OPTION_HANDLERS
> +        OVN_DAEMON_OPTION_HANDLERS
>          STREAM_SSL_OPTION_HANDLERS
>  
>          case OPT_PEER_CA_CERT:
> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> index 4d21ba0fd..6ff897325 100644
> --- a/controller/ovn-controller.c
> +++ b/controller/ovn-controller.c
> @@ -2268,7 +2268,7 @@ parse_options(int argc, char *argv[])
>          OPT_PEER_CA_CERT = UCHAR_MAX + 1,
>          OPT_BOOTSTRAP_CA_CERT,
>          VLOG_OPTION_ENUMS,
> -        DAEMON_OPTION_ENUMS,
> +        OVN_DAEMON_OPTION_ENUMS,
>          SSL_OPTION_ENUMS,
>      };
>  
> @@ -2276,7 +2276,7 @@ parse_options(int argc, char *argv[])
>          {"help", no_argument, NULL, 'h'},
>          {"version", no_argument, NULL, 'V'},
>          VLOG_LONG_OPTIONS,
> -        DAEMON_LONG_OPTIONS,
> +        OVN_DAEMON_LONG_OPTIONS,
>          STREAM_SSL_LONG_OPTIONS,
>          {"peer-ca-cert", required_argument, NULL, OPT_PEER_CA_CERT},
>          {"bootstrap-ca-cert", required_argument, NULL, OPT_BOOTSTRAP_CA_CERT},
> @@ -2301,7 +2301,7 @@ parse_options(int argc, char *argv[])
>              exit(EXIT_SUCCESS);
>  
>          VLOG_OPTION_HANDLERS
> -        DAEMON_OPTION_HANDLERS
> +        OVN_DAEMON_OPTION_HANDLERS
>          STREAM_SSL_OPTION_HANDLERS
>  
>          case OPT_PEER_CA_CERT:
> diff --git a/lib/ovn-util.c b/lib/ovn-util.c
> index 514e2489f..353e8ab2d 100644
> --- a/lib/ovn-util.c
> +++ b/lib/ovn-util.c
> @@ -15,6 +15,7 @@
>  #include <config.h>
>  #include <unistd.h>
>  
> +#include "daemon.h"
>  #include "ovn-util.h"
>  #include "ovn-dirs.h"
>  #include "openvswitch/vlog.h"
> @@ -393,6 +394,31 @@ get_abs_unix_ctl_path(const char *path)
>      return abs_path;
>  }
>  
> +void
> +ovn_set_pidfile(const char *name)
> +{
> +    char *pidfile_name = NULL;
> +
> +#ifndef _WIN32
> +    pidfile_name = name ? abs_file_name(ovn_rundir(), name)
> +                        : xasprintf("%s/%s.pid", ovn_rundir(), program_name);
> +#else
> +    if (name) {
> +        if (strchr(name, ':')) {
> +            pidfile_name = xstrdup(name);
> +        } else {
> +            pidfile_name = xasprintf("%s/%s", ovn_rundir(), name);
> +        }
> +    } else {
> +        pidfile_name = xasprintf("%s/%s.pid", ovn_rundir(), program_name);
> +    }
> +#endif
> +
> +    /* Call openvswitch lib function. */
> +    set_pidfile(pidfile_name);
> +    free(pidfile_name);
> +}
> +
>  /* l3gateway, chassisredirect, and patch
>   * are not in this list since they are
>   * only set in the SB DB by northd
> diff --git a/lib/ovn-util.h b/lib/ovn-util.h
> index 11238f61c..400457618 100644
> --- a/lib/ovn-util.h
> +++ b/lib/ovn-util.h
> @@ -112,6 +112,7 @@ uint32_t ovn_allocate_tnlid(struct hmap *set, const char *name, uint32_t min,
>                              uint32_t max, uint32_t *hint);
>  
>  char *ovn_chassis_redirect_name(const char *port_name);
> +void ovn_set_pidfile(const char *name);
>  
>  /* An IPv4 or IPv6 address */
>  struct v46_ip {
> @@ -124,4 +125,64 @@ struct v46_ip {
>  bool ip46_parse_cidr(const char *str, struct v46_ip *prefix,
>                       unsigned int *plen);
>  bool ip46_equals(const struct v46_ip *addr1, const struct v46_ip *addr2);
> +
> +
> +/* OVN daemon options. Taken from ovs/lib/daemon.h. */
> +#define OVN_DAEMON_OPTION_ENUMS                     \
> +    OVN_OPT_DETACH,                                 \
> +    OVN_OPT_NO_SELF_CONFINEMENT,                    \
> +    OVN_OPT_NO_CHDIR,                               \
> +    OVN_OPT_OVERWRITE_PIDFILE,                      \
> +    OVN_OPT_PIDFILE,                                \
> +    OVN_OPT_MONITOR,                                \
> +    OVN_OPT_USER_GROUP
> +
> +#define OVN_DAEMON_LONG_OPTIONS                                              \
> +        {"detach",            no_argument, NULL, OVN_OPT_DETACH},            \
> +        {"no-self-confinement", no_argument, NULL, \
> +         OVN_OPT_NO_SELF_CONFINEMENT}, \

Nit: I'd align the "\" with the rest of the macro definition.

> +        {"no-chdir",          no_argument, NULL, OVN_OPT_NO_CHDIR},          \
> +        {"pidfile",           optional_argument, NULL, OVN_OPT_PIDFILE},     \
> +        {"overwrite-pidfile", no_argument, NULL, OVN_OPT_OVERWRITE_PIDFILE}, \
> +        {"monitor",           no_argument, NULL, OVN_OPT_MONITOR},           \
> +        {"user",              required_argument, NULL, OVN_OPT_USER_GROUP}
> +
> +#define OVN_DAEMON_OPTION_HANDLERS                  \
> +        case OVN_OPT_DETACH:                        \
> +            set_detach();                       \
> +            break;                              \
> +                                                \
> +        case OVN_OPT_NO_SELF_CONFINEMENT:           \
> +            daemon_disable_self_confinement();  \
> +            break;                              \
> +                                                \
> +        case OVN_OPT_NO_CHDIR:                      \
> +            set_no_chdir();                     \
> +            break;                              \
> +                                                \
> +        case OVN_OPT_PIDFILE:                       \
> +            ovn_set_pidfile(optarg);                \
> +            break;                              \
> +                                                \
> +        case OVN_OPT_OVERWRITE_PIDFILE:             \
> +            ignore_existing_pidfile();          \
> +            break;                              \
> +                                                \
> +        case OVN_OPT_MONITOR:                       \
> +            daemon_set_monitor();               \
> +            break;                              \
> +                                                \
> +        case OVN_OPT_USER_GROUP:                    \
> +            daemon_set_new_user(optarg);        \
> +            break;
> +> +#define OVN_DAEMON_OPTION_CASES                     \
> +        case OVN_OPT_DETACH:                        \
> +        case OVN_OPT_NO_SELF_CONFINEMENT:           \
> +        case OVN_OPT_NO_CHDIR:                      \
> +        case OVN_OPT_PIDFILE:                       \
> +        case OVN_OPT_OVERWRITE_PIDFILE:             \
> +        case OVN_OPT_MONITOR:                       \
> +        case OVN_OPT_USER_GROUP:
> +

Nit: Same here, I'd align the "\" with the rest of the macro definition.

>  #endif
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index 324835080..cab4db7a5 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -11627,7 +11627,7 @@ static void
>  parse_options(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
>  {
>      enum {
> -        DAEMON_OPTION_ENUMS,
> +        OVN_DAEMON_OPTION_ENUMS,
>          VLOG_OPTION_ENUMS,
>          SSL_OPTION_ENUMS,
>      };
> @@ -11638,7 +11638,7 @@ parse_options(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
>          {"help", no_argument, NULL, 'h'},
>          {"options", no_argument, NULL, 'o'},
>          {"version", no_argument, NULL, 'V'},
> -        DAEMON_LONG_OPTIONS,
> +        OVN_DAEMON_LONG_OPTIONS,
>          VLOG_LONG_OPTIONS,
>          STREAM_SSL_LONG_OPTIONS,
>          {NULL, 0, NULL, 0},
> @@ -11654,7 +11654,7 @@ parse_options(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
>          }
>  
>          switch (c) {
> -        DAEMON_OPTION_HANDLERS;
> +        OVN_DAEMON_OPTION_HANDLERS;
>          VLOG_OPTION_HANDLERS;
>          STREAM_SSL_OPTION_HANDLERS;
>  
> diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
> index 18bf91e32..db460d04f 100644
> --- a/utilities/ovn-nbctl.c
> +++ b/utilities/ovn-nbctl.c
> @@ -324,7 +324,7 @@ enum {
>      OPT_NO_SHUFFLE_REMOTES,
>      OPT_BOOTSTRAP_CA_CERT,
>      MAIN_LOOP_OPTION_ENUMS,
> -    DAEMON_OPTION_ENUMS,
> +    OVN_DAEMON_OPTION_ENUMS,
>      VLOG_OPTION_ENUMS,
>      TABLE_OPTION_ENUMS,
>      SSL_OPTION_ENUMS,
> @@ -428,7 +428,7 @@ get_all_options(void)
>          {"version", no_argument, NULL, 'V'},
>          {"unixctl", required_argument, NULL, 'u'},
>          MAIN_LOOP_LONG_OPTIONS,
> -        DAEMON_LONG_OPTIONS,
> +        OVN_DAEMON_LONG_OPTIONS,
>          VLOG_LONG_OPTIONS,
>          STREAM_SSL_LONG_OPTIONS,
>          {"bootstrap-ca-cert", required_argument, NULL, OPT_BOOTSTRAP_CA_CERT},
> @@ -460,7 +460,7 @@ has_option(const struct ovs_cmdl_parsed_option *parsed_options, size_t n,
>  static bool
>  will_detach(const struct ovs_cmdl_parsed_option *parsed_options, size_t n)
>  {
> -    return has_option(parsed_options, n, OPT_DETACH);
> +    return has_option(parsed_options, n, OVN_OPT_DETACH);
>  }
>  
>  static char * OVS_WARN_UNUSED_RESULT
> @@ -547,7 +547,7 @@ apply_options_direct(const struct ovs_cmdl_parsed_option *parsed_options,
>              printf("DB Schema %s\n", nbrec_get_db_version());
>              exit(EXIT_SUCCESS);
>  
> -        DAEMON_OPTION_HANDLERS
> +        OVN_DAEMON_OPTION_HANDLERS
>          VLOG_OPTION_HANDLERS
>          TABLE_OPTION_HANDLERS(&table_style)
>          STREAM_SSL_OPTION_HANDLERS
> @@ -6598,7 +6598,7 @@ nbctl_client(const char *socket_name,
>          case OPT_NO_SHUFFLE_REMOTES:
>          case OPT_BOOTSTRAP_CA_CERT:
>          STREAM_SSL_CASES
> -        DAEMON_OPTION_CASES
> +        OVN_DAEMON_OPTION_CASES
>              VLOG_INFO("using ovn-nbctl daemon, ignoring %s option",
>                        po->o->name);
>              break;
>
Numan Siddique April 23, 2020, 7:24 a.m. UTC | #3 
On Thu, Apr 23, 2020 at 1:10 AM Dumitru Ceara <dceara@redhat.com> wrote:
>
> On 4/21/20 2:41 PM, numans@ovn.org wrote:
> > From: Numan Siddique <numans@ovn.org>
> >
> > If an OVN service is started with --pidfile option, the pidfile
> > is created in the ovs rundir. This patch fixes it by using the ovn rundir
> > if either the pidfile is not specified or if specified, it is not
> > absolute path.
> >
> > Signed-off-by: Numan Siddique <numans@ovn.org>
>
> Hi Numan,
>
> Overall, looks good to me, however shouldn't we use
> OVN_DAEMON_OPTION_ENUMS in ovn-trace and ovn-ic too?
>

Yes. I missed it v1.

Thanks for the review. I addressed the comments and submitted v2.

Thanks
Numan

> A few minor comments inline.
>
> Thanks,
> Dumitru
>
> > ---
> >  controller-vtep/ovn-controller-vtep.c |  6 +--
> >  controller/ovn-controller.c           |  6 +--
> >  lib/ovn-util.c                        | 26 ++++++++++++
> >  lib/ovn-util.h                        | 61 +++++++++++++++++++++++++++
> >  northd/ovn-northd.c                   |  6 +--
> >  utilities/ovn-nbctl.c                 | 10 ++---
> >  6 files changed, 101 insertions(+), 14 deletions(-)
> >
> > diff --git a/controller-vtep/ovn-controller-vtep.c b/controller-vtep/ovn-controller-vtep.c
> > index 253a709ab..c13280bc0 100644
> > --- a/controller-vtep/ovn-controller-vtep.c
> > +++ b/controller-vtep/ovn-controller-vtep.c
> > @@ -169,7 +169,7 @@ parse_options(int argc, char *argv[])
> >          OPT_PEER_CA_CERT = UCHAR_MAX + 1,
> >          OPT_BOOTSTRAP_CA_CERT,
> >          VLOG_OPTION_ENUMS,
> > -        DAEMON_OPTION_ENUMS,
> > +        OVN_DAEMON_OPTION_ENUMS,
> >          SSL_OPTION_ENUMS,
> >      };
> >
> > @@ -179,7 +179,7 @@ parse_options(int argc, char *argv[])
> >          {"help", no_argument, NULL, 'h'},
> >          {"version", no_argument, NULL, 'V'},
> >          VLOG_LONG_OPTIONS,
> > -        DAEMON_LONG_OPTIONS,
> > +        OVN_DAEMON_LONG_OPTIONS,
> >          STREAM_SSL_LONG_OPTIONS,
> >          {"peer-ca-cert", required_argument, NULL, OPT_PEER_CA_CERT},
> >          {"bootstrap-ca-cert", required_argument, NULL, OPT_BOOTSTRAP_CA_CERT},
> > @@ -212,7 +212,7 @@ parse_options(int argc, char *argv[])
> >              exit(EXIT_SUCCESS);
> >
> >          VLOG_OPTION_HANDLERS
> > -        DAEMON_OPTION_HANDLERS
> > +        OVN_DAEMON_OPTION_HANDLERS
> >          STREAM_SSL_OPTION_HANDLERS
> >
> >          case OPT_PEER_CA_CERT:
> > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> > index 4d21ba0fd..6ff897325 100644
> > --- a/controller/ovn-controller.c
> > +++ b/controller/ovn-controller.c
> > @@ -2268,7 +2268,7 @@ parse_options(int argc, char *argv[])
> >          OPT_PEER_CA_CERT = UCHAR_MAX + 1,
> >          OPT_BOOTSTRAP_CA_CERT,
> >          VLOG_OPTION_ENUMS,
> > -        DAEMON_OPTION_ENUMS,
> > +        OVN_DAEMON_OPTION_ENUMS,
> >          SSL_OPTION_ENUMS,
> >      };
> >
> > @@ -2276,7 +2276,7 @@ parse_options(int argc, char *argv[])
> >          {"help", no_argument, NULL, 'h'},
> >          {"version", no_argument, NULL, 'V'},
> >          VLOG_LONG_OPTIONS,
> > -        DAEMON_LONG_OPTIONS,
> > +        OVN_DAEMON_LONG_OPTIONS,
> >          STREAM_SSL_LONG_OPTIONS,
> >          {"peer-ca-cert", required_argument, NULL, OPT_PEER_CA_CERT},
> >          {"bootstrap-ca-cert", required_argument, NULL, OPT_BOOTSTRAP_CA_CERT},
> > @@ -2301,7 +2301,7 @@ parse_options(int argc, char *argv[])
> >              exit(EXIT_SUCCESS);
> >
> >          VLOG_OPTION_HANDLERS
> > -        DAEMON_OPTION_HANDLERS
> > +        OVN_DAEMON_OPTION_HANDLERS
> >          STREAM_SSL_OPTION_HANDLERS
> >
> >          case OPT_PEER_CA_CERT:
> > diff --git a/lib/ovn-util.c b/lib/ovn-util.c
> > index 514e2489f..353e8ab2d 100644
> > --- a/lib/ovn-util.c
> > +++ b/lib/ovn-util.c
> > @@ -15,6 +15,7 @@
> >  #include <config.h>
> >  #include <unistd.h>
> >
> > +#include "daemon.h"
> >  #include "ovn-util.h"
> >  #include "ovn-dirs.h"
> >  #include "openvswitch/vlog.h"
> > @@ -393,6 +394,31 @@ get_abs_unix_ctl_path(const char *path)
> >      return abs_path;
> >  }
> >
> > +void
> > +ovn_set_pidfile(const char *name)
> > +{
> > +    char *pidfile_name = NULL;
> > +
> > +#ifndef _WIN32
> > +    pidfile_name = name ? abs_file_name(ovn_rundir(), name)
> > +                        : xasprintf("%s/%s.pid", ovn_rundir(), program_name);
> > +#else
> > +    if (name) {
> > +        if (strchr(name, ':')) {
> > +            pidfile_name = xstrdup(name);
> > +        } else {
> > +            pidfile_name = xasprintf("%s/%s", ovn_rundir(), name);
> > +        }
> > +    } else {
> > +        pidfile_name = xasprintf("%s/%s.pid", ovn_rundir(), program_name);
> > +    }
> > +#endif
> > +
> > +    /* Call openvswitch lib function. */
> > +    set_pidfile(pidfile_name);
> > +    free(pidfile_name);
> > +}
> > +
> >  /* l3gateway, chassisredirect, and patch
> >   * are not in this list since they are
> >   * only set in the SB DB by northd
> > diff --git a/lib/ovn-util.h b/lib/ovn-util.h
> > index 11238f61c..400457618 100644
> > --- a/lib/ovn-util.h
> > +++ b/lib/ovn-util.h
> > @@ -112,6 +112,7 @@ uint32_t ovn_allocate_tnlid(struct hmap *set, const char *name, uint32_t min,
> >                              uint32_t max, uint32_t *hint);
> >
> >  char *ovn_chassis_redirect_name(const char *port_name);
> > +void ovn_set_pidfile(const char *name);
> >
> >  /* An IPv4 or IPv6 address */
> >  struct v46_ip {
> > @@ -124,4 +125,64 @@ struct v46_ip {
> >  bool ip46_parse_cidr(const char *str, struct v46_ip *prefix,
> >                       unsigned int *plen);
> >  bool ip46_equals(const struct v46_ip *addr1, const struct v46_ip *addr2);
> > +
> > +
> > +/* OVN daemon options. Taken from ovs/lib/daemon.h. */
> > +#define OVN_DAEMON_OPTION_ENUMS                     \
> > +    OVN_OPT_DETACH,                                 \
> > +    OVN_OPT_NO_SELF_CONFINEMENT,                    \
> > +    OVN_OPT_NO_CHDIR,                               \
> > +    OVN_OPT_OVERWRITE_PIDFILE,                      \
> > +    OVN_OPT_PIDFILE,                                \
> > +    OVN_OPT_MONITOR,                                \
> > +    OVN_OPT_USER_GROUP
> > +
> > +#define OVN_DAEMON_LONG_OPTIONS                                              \
> > +        {"detach",            no_argument, NULL, OVN_OPT_DETACH},            \
> > +        {"no-self-confinement", no_argument, NULL, \
> > +         OVN_OPT_NO_SELF_CONFINEMENT}, \
>
> Nit: I'd align the "\" with the rest of the macro definition.
>
> > +        {"no-chdir",          no_argument, NULL, OVN_OPT_NO_CHDIR},          \
> > +        {"pidfile",           optional_argument, NULL, OVN_OPT_PIDFILE},     \
> > +        {"overwrite-pidfile", no_argument, NULL, OVN_OPT_OVERWRITE_PIDFILE}, \
> > +        {"monitor",           no_argument, NULL, OVN_OPT_MONITOR},           \
> > +        {"user",              required_argument, NULL, OVN_OPT_USER_GROUP}
> > +
> > +#define OVN_DAEMON_OPTION_HANDLERS                  \
> > +        case OVN_OPT_DETACH:                        \
> > +            set_detach();                       \
> > +            break;                              \
> > +                                                \
> > +        case OVN_OPT_NO_SELF_CONFINEMENT:           \
> > +            daemon_disable_self_confinement();  \
> > +            break;                              \
> > +                                                \
> > +        case OVN_OPT_NO_CHDIR:                      \
> > +            set_no_chdir();                     \
> > +            break;                              \
> > +                                                \
> > +        case OVN_OPT_PIDFILE:                       \
> > +            ovn_set_pidfile(optarg);                \
> > +            break;                              \
> > +                                                \
> > +        case OVN_OPT_OVERWRITE_PIDFILE:             \
> > +            ignore_existing_pidfile();          \
> > +            break;                              \
> > +                                                \
> > +        case OVN_OPT_MONITOR:                       \
> > +            daemon_set_monitor();               \
> > +            break;                              \
> > +                                                \
> > +        case OVN_OPT_USER_GROUP:                    \
> > +            daemon_set_new_user(optarg);        \
> > +            break;
> > +> +#define OVN_DAEMON_OPTION_CASES                     \
> > +        case OVN_OPT_DETACH:                        \
> > +        case OVN_OPT_NO_SELF_CONFINEMENT:           \
> > +        case OVN_OPT_NO_CHDIR:                      \
> > +        case OVN_OPT_PIDFILE:                       \
> > +        case OVN_OPT_OVERWRITE_PIDFILE:             \
> > +        case OVN_OPT_MONITOR:                       \
> > +        case OVN_OPT_USER_GROUP:
> > +
>
> Nit: Same here, I'd align the "\" with the rest of the macro definition.
>
> >  #endif
> > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> > index 324835080..cab4db7a5 100644
> > --- a/northd/ovn-northd.c
> > +++ b/northd/ovn-northd.c
> > @@ -11627,7 +11627,7 @@ static void
> >  parse_options(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
> >  {
> >      enum {
> > -        DAEMON_OPTION_ENUMS,
> > +        OVN_DAEMON_OPTION_ENUMS,
> >          VLOG_OPTION_ENUMS,
> >          SSL_OPTION_ENUMS,
> >      };
> > @@ -11638,7 +11638,7 @@ parse_options(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
> >          {"help", no_argument, NULL, 'h'},
> >          {"options", no_argument, NULL, 'o'},
> >          {"version", no_argument, NULL, 'V'},
> > -        DAEMON_LONG_OPTIONS,
> > +        OVN_DAEMON_LONG_OPTIONS,
> >          VLOG_LONG_OPTIONS,
> >          STREAM_SSL_LONG_OPTIONS,
> >          {NULL, 0, NULL, 0},
> > @@ -11654,7 +11654,7 @@ parse_options(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
> >          }
> >
> >          switch (c) {
> > -        DAEMON_OPTION_HANDLERS;
> > +        OVN_DAEMON_OPTION_HANDLERS;
> >          VLOG_OPTION_HANDLERS;
> >          STREAM_SSL_OPTION_HANDLERS;
> >
> > diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
> > index 18bf91e32..db460d04f 100644
> > --- a/utilities/ovn-nbctl.c
> > +++ b/utilities/ovn-nbctl.c
> > @@ -324,7 +324,7 @@ enum {
> >      OPT_NO_SHUFFLE_REMOTES,
> >      OPT_BOOTSTRAP_CA_CERT,
> >      MAIN_LOOP_OPTION_ENUMS,
> > -    DAEMON_OPTION_ENUMS,
> > +    OVN_DAEMON_OPTION_ENUMS,
> >      VLOG_OPTION_ENUMS,
> >      TABLE_OPTION_ENUMS,
> >      SSL_OPTION_ENUMS,
> > @@ -428,7 +428,7 @@ get_all_options(void)
> >          {"version", no_argument, NULL, 'V'},
> >          {"unixctl", required_argument, NULL, 'u'},
> >          MAIN_LOOP_LONG_OPTIONS,
> > -        DAEMON_LONG_OPTIONS,
> > +        OVN_DAEMON_LONG_OPTIONS,
> >          VLOG_LONG_OPTIONS,
> >          STREAM_SSL_LONG_OPTIONS,
> >          {"bootstrap-ca-cert", required_argument, NULL, OPT_BOOTSTRAP_CA_CERT},
> > @@ -460,7 +460,7 @@ has_option(const struct ovs_cmdl_parsed_option *parsed_options, size_t n,
> >  static bool
> >  will_detach(const struct ovs_cmdl_parsed_option *parsed_options, size_t n)
> >  {
> > -    return has_option(parsed_options, n, OPT_DETACH);
> > +    return has_option(parsed_options, n, OVN_OPT_DETACH);
> >  }
> >
> >  static char * OVS_WARN_UNUSED_RESULT
> > @@ -547,7 +547,7 @@ apply_options_direct(const struct ovs_cmdl_parsed_option *parsed_options,
> >              printf("DB Schema %s\n", nbrec_get_db_version());
> >              exit(EXIT_SUCCESS);
> >
> > -        DAEMON_OPTION_HANDLERS
> > +        OVN_DAEMON_OPTION_HANDLERS
> >          VLOG_OPTION_HANDLERS
> >          TABLE_OPTION_HANDLERS(&table_style)
> >          STREAM_SSL_OPTION_HANDLERS
> > @@ -6598,7 +6598,7 @@ nbctl_client(const char *socket_name,
> >          case OPT_NO_SHUFFLE_REMOTES:
> >          case OPT_BOOTSTRAP_CA_CERT:
> >          STREAM_SSL_CASES
> > -        DAEMON_OPTION_CASES
> > +        OVN_DAEMON_OPTION_CASES
> >              VLOG_INFO("using ovn-nbctl daemon, ignoring %s option",
> >                        po->o->name);
> >              break;
> >
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
diff mbox series

Patch

diff --git a/controller-vtep/ovn-controller-vtep.c b/controller-vtep/ovn-controller-vtep.c
index 253a709ab..c13280bc0 100644
--- a/controller-vtep/ovn-controller-vtep.c
+++ b/controller-vtep/ovn-controller-vtep.c
@@ -169,7 +169,7 @@  parse_options(int argc, char *argv[])
         OPT_PEER_CA_CERT = UCHAR_MAX + 1,
         OPT_BOOTSTRAP_CA_CERT,
         VLOG_OPTION_ENUMS,
-        DAEMON_OPTION_ENUMS,
+        OVN_DAEMON_OPTION_ENUMS,
         SSL_OPTION_ENUMS,
     };
 
@@ -179,7 +179,7 @@  parse_options(int argc, char *argv[])
         {"help", no_argument, NULL, 'h'},
         {"version", no_argument, NULL, 'V'},
         VLOG_LONG_OPTIONS,
-        DAEMON_LONG_OPTIONS,
+        OVN_DAEMON_LONG_OPTIONS,
         STREAM_SSL_LONG_OPTIONS,
         {"peer-ca-cert", required_argument, NULL, OPT_PEER_CA_CERT},
         {"bootstrap-ca-cert", required_argument, NULL, OPT_BOOTSTRAP_CA_CERT},
@@ -212,7 +212,7 @@  parse_options(int argc, char *argv[])
             exit(EXIT_SUCCESS);
 
         VLOG_OPTION_HANDLERS
-        DAEMON_OPTION_HANDLERS
+        OVN_DAEMON_OPTION_HANDLERS
         STREAM_SSL_OPTION_HANDLERS
 
         case OPT_PEER_CA_CERT:
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index 4d21ba0fd..6ff897325 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -2268,7 +2268,7 @@  parse_options(int argc, char *argv[])
         OPT_PEER_CA_CERT = UCHAR_MAX + 1,
         OPT_BOOTSTRAP_CA_CERT,
         VLOG_OPTION_ENUMS,
-        DAEMON_OPTION_ENUMS,
+        OVN_DAEMON_OPTION_ENUMS,
         SSL_OPTION_ENUMS,
     };
 
@@ -2276,7 +2276,7 @@  parse_options(int argc, char *argv[])
         {"help", no_argument, NULL, 'h'},
         {"version", no_argument, NULL, 'V'},
         VLOG_LONG_OPTIONS,
-        DAEMON_LONG_OPTIONS,
+        OVN_DAEMON_LONG_OPTIONS,
         STREAM_SSL_LONG_OPTIONS,
         {"peer-ca-cert", required_argument, NULL, OPT_PEER_CA_CERT},
         {"bootstrap-ca-cert", required_argument, NULL, OPT_BOOTSTRAP_CA_CERT},
@@ -2301,7 +2301,7 @@  parse_options(int argc, char *argv[])
             exit(EXIT_SUCCESS);
 
         VLOG_OPTION_HANDLERS
-        DAEMON_OPTION_HANDLERS
+        OVN_DAEMON_OPTION_HANDLERS
         STREAM_SSL_OPTION_HANDLERS
 
         case OPT_PEER_CA_CERT:
diff --git a/lib/ovn-util.c b/lib/ovn-util.c
index 514e2489f..353e8ab2d 100644
--- a/lib/ovn-util.c
+++ b/lib/ovn-util.c
@@ -15,6 +15,7 @@ 
 #include <config.h>
 #include <unistd.h>
 
+#include "daemon.h"
 #include "ovn-util.h"
 #include "ovn-dirs.h"
 #include "openvswitch/vlog.h"
@@ -393,6 +394,31 @@  get_abs_unix_ctl_path(const char *path)
     return abs_path;
 }
 
+void
+ovn_set_pidfile(const char *name)
+{
+    char *pidfile_name = NULL;
+
+#ifndef _WIN32
+    pidfile_name = name ? abs_file_name(ovn_rundir(), name)
+                        : xasprintf("%s/%s.pid", ovn_rundir(), program_name);
+#else
+    if (name) {
+        if (strchr(name, ':')) {
+            pidfile_name = xstrdup(name);
+        } else {
+            pidfile_name = xasprintf("%s/%s", ovn_rundir(), name);
+        }
+    } else {
+        pidfile_name = xasprintf("%s/%s.pid", ovn_rundir(), program_name);
+    }
+#endif
+
+    /* Call openvswitch lib function. */
+    set_pidfile(pidfile_name);
+    free(pidfile_name);
+}
+
 /* l3gateway, chassisredirect, and patch
  * are not in this list since they are
  * only set in the SB DB by northd
diff --git a/lib/ovn-util.h b/lib/ovn-util.h
index 11238f61c..400457618 100644
--- a/lib/ovn-util.h
+++ b/lib/ovn-util.h
@@ -112,6 +112,7 @@  uint32_t ovn_allocate_tnlid(struct hmap *set, const char *name, uint32_t min,
                             uint32_t max, uint32_t *hint);
 
 char *ovn_chassis_redirect_name(const char *port_name);
+void ovn_set_pidfile(const char *name);
 
 /* An IPv4 or IPv6 address */
 struct v46_ip {
@@ -124,4 +125,64 @@  struct v46_ip {
 bool ip46_parse_cidr(const char *str, struct v46_ip *prefix,
                      unsigned int *plen);
 bool ip46_equals(const struct v46_ip *addr1, const struct v46_ip *addr2);
+
+
+/* OVN daemon options. Taken from ovs/lib/daemon.h. */
+#define OVN_DAEMON_OPTION_ENUMS                     \
+    OVN_OPT_DETACH,                                 \
+    OVN_OPT_NO_SELF_CONFINEMENT,                    \
+    OVN_OPT_NO_CHDIR,                               \
+    OVN_OPT_OVERWRITE_PIDFILE,                      \
+    OVN_OPT_PIDFILE,                                \
+    OVN_OPT_MONITOR,                                \
+    OVN_OPT_USER_GROUP
+
+#define OVN_DAEMON_LONG_OPTIONS                                              \
+        {"detach",            no_argument, NULL, OVN_OPT_DETACH},            \
+        {"no-self-confinement", no_argument, NULL, \
+         OVN_OPT_NO_SELF_CONFINEMENT}, \
+        {"no-chdir",          no_argument, NULL, OVN_OPT_NO_CHDIR},          \
+        {"pidfile",           optional_argument, NULL, OVN_OPT_PIDFILE},     \
+        {"overwrite-pidfile", no_argument, NULL, OVN_OPT_OVERWRITE_PIDFILE}, \
+        {"monitor",           no_argument, NULL, OVN_OPT_MONITOR},           \
+        {"user",              required_argument, NULL, OVN_OPT_USER_GROUP}
+
+#define OVN_DAEMON_OPTION_HANDLERS                  \
+        case OVN_OPT_DETACH:                        \
+            set_detach();                       \
+            break;                              \
+                                                \
+        case OVN_OPT_NO_SELF_CONFINEMENT:           \
+            daemon_disable_self_confinement();  \
+            break;                              \
+                                                \
+        case OVN_OPT_NO_CHDIR:                      \
+            set_no_chdir();                     \
+            break;                              \
+                                                \
+        case OVN_OPT_PIDFILE:                       \
+            ovn_set_pidfile(optarg);                \
+            break;                              \
+                                                \
+        case OVN_OPT_OVERWRITE_PIDFILE:             \
+            ignore_existing_pidfile();          \
+            break;                              \
+                                                \
+        case OVN_OPT_MONITOR:                       \
+            daemon_set_monitor();               \
+            break;                              \
+                                                \
+        case OVN_OPT_USER_GROUP:                    \
+            daemon_set_new_user(optarg);        \
+            break;
+
+#define OVN_DAEMON_OPTION_CASES                     \
+        case OVN_OPT_DETACH:                        \
+        case OVN_OPT_NO_SELF_CONFINEMENT:           \
+        case OVN_OPT_NO_CHDIR:                      \
+        case OVN_OPT_PIDFILE:                       \
+        case OVN_OPT_OVERWRITE_PIDFILE:             \
+        case OVN_OPT_MONITOR:                       \
+        case OVN_OPT_USER_GROUP:
+
 #endif
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 324835080..cab4db7a5 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -11627,7 +11627,7 @@  static void
 parse_options(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
 {
     enum {
-        DAEMON_OPTION_ENUMS,
+        OVN_DAEMON_OPTION_ENUMS,
         VLOG_OPTION_ENUMS,
         SSL_OPTION_ENUMS,
     };
@@ -11638,7 +11638,7 @@  parse_options(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
         {"help", no_argument, NULL, 'h'},
         {"options", no_argument, NULL, 'o'},
         {"version", no_argument, NULL, 'V'},
-        DAEMON_LONG_OPTIONS,
+        OVN_DAEMON_LONG_OPTIONS,
         VLOG_LONG_OPTIONS,
         STREAM_SSL_LONG_OPTIONS,
         {NULL, 0, NULL, 0},
@@ -11654,7 +11654,7 @@  parse_options(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
         }
 
         switch (c) {
-        DAEMON_OPTION_HANDLERS;
+        OVN_DAEMON_OPTION_HANDLERS;
         VLOG_OPTION_HANDLERS;
         STREAM_SSL_OPTION_HANDLERS;
 
diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
index 18bf91e32..db460d04f 100644
--- a/utilities/ovn-nbctl.c
+++ b/utilities/ovn-nbctl.c
@@ -324,7 +324,7 @@  enum {
     OPT_NO_SHUFFLE_REMOTES,
     OPT_BOOTSTRAP_CA_CERT,
     MAIN_LOOP_OPTION_ENUMS,
-    DAEMON_OPTION_ENUMS,
+    OVN_DAEMON_OPTION_ENUMS,
     VLOG_OPTION_ENUMS,
     TABLE_OPTION_ENUMS,
     SSL_OPTION_ENUMS,
@@ -428,7 +428,7 @@  get_all_options(void)
         {"version", no_argument, NULL, 'V'},
         {"unixctl", required_argument, NULL, 'u'},
         MAIN_LOOP_LONG_OPTIONS,
-        DAEMON_LONG_OPTIONS,
+        OVN_DAEMON_LONG_OPTIONS,
         VLOG_LONG_OPTIONS,
         STREAM_SSL_LONG_OPTIONS,
         {"bootstrap-ca-cert", required_argument, NULL, OPT_BOOTSTRAP_CA_CERT},
@@ -460,7 +460,7 @@  has_option(const struct ovs_cmdl_parsed_option *parsed_options, size_t n,
 static bool
 will_detach(const struct ovs_cmdl_parsed_option *parsed_options, size_t n)
 {
-    return has_option(parsed_options, n, OPT_DETACH);
+    return has_option(parsed_options, n, OVN_OPT_DETACH);
 }
 
 static char * OVS_WARN_UNUSED_RESULT
@@ -547,7 +547,7 @@  apply_options_direct(const struct ovs_cmdl_parsed_option *parsed_options,
             printf("DB Schema %s\n", nbrec_get_db_version());
             exit(EXIT_SUCCESS);
 
-        DAEMON_OPTION_HANDLERS
+        OVN_DAEMON_OPTION_HANDLERS
         VLOG_OPTION_HANDLERS
         TABLE_OPTION_HANDLERS(&table_style)
         STREAM_SSL_OPTION_HANDLERS
@@ -6598,7 +6598,7 @@  nbctl_client(const char *socket_name,
         case OPT_NO_SHUFFLE_REMOTES:
         case OPT_BOOTSTRAP_CA_CERT:
         STREAM_SSL_CASES
-        DAEMON_OPTION_CASES
+        OVN_DAEMON_OPTION_CASES
             VLOG_INFO("using ovn-nbctl daemon, ignoring %s option",
                       po->o->name);
             break;