On 4/23/20 9:23 AM, [email protected] wrote: > From: Numan Siddique <[email protected]> > > 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 <[email protected]>
Looks good to me, thanks! Acked-by: Dumitru Ceara <[email protected]> > --- > > v1 -> v2 > ---- > * Addressed review comments from Dumitru > > controller-vtep/ovn-controller-vtep.c | 6 +-- > controller/ovn-controller.c | 6 +-- > ic/ovn-ic.c | 6 +-- > lib/ovn-util.c | 26 ++++++++++++ > lib/ovn-util.h | 60 +++++++++++++++++++++++++++ > northd/ovn-northd.c | 6 +-- > utilities/ovn-nbctl.c | 10 ++--- > utilities/ovn-trace.c | 6 +-- > 8 files changed, 106 insertions(+), 20 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/ic/ovn-ic.c b/ic/ovn-ic.c > index d931ca50f..a1ed25623 100644 > --- a/ic/ovn-ic.c > +++ b/ic/ovn-ic.c > @@ -1461,7 +1461,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, > }; > @@ -1474,7 +1474,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}, > @@ -1490,7 +1490,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/lib/ovn-util.c b/lib/ovn-util.c > index 1b30c2e9a..3482edb8d 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" > @@ -394,6 +395,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 4076e8b9a..ec5f2cf5a 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 { > @@ -129,4 +130,63 @@ bool ip46_equals(const struct v46_ip *addr1, const > struct v46_ip *addr2); > * Caller must free the returned string. > */ > char *str_tolower(const char *orig); > + > +/* 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 f02dc5d5e..c4675bd68 100644 > --- a/northd/ovn-northd.c > +++ b/northd/ovn-northd.c > @@ -11640,7 +11640,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, > }; > @@ -11651,7 +11651,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}, > @@ -11667,7 +11667,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 cb46d3aa5..c86fa3886 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 > @@ -6611,7 +6611,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; > diff --git a/utilities/ovn-trace.c b/utilities/ovn-trace.c > index eae9622d3..c9d72285c 100644 > --- a/utilities/ovn-trace.c > +++ b/utilities/ovn-trace.c > @@ -239,7 +239,7 @@ parse_options(int argc, char *argv[]) > OPT_CT, > OPT_FRIENDLY_NAMES, > OPT_NO_FRIENDLY_NAMES, > - DAEMON_OPTION_ENUMS, > + OVN_DAEMON_OPTION_ENUMS, > SSL_OPTION_ENUMS, > VLOG_OPTION_ENUMS, > OPT_LB_DST, > @@ -260,7 +260,7 @@ parse_options(int argc, char *argv[]) > {"version", no_argument, NULL, 'V'}, > {"lb-dst", required_argument, NULL, OPT_LB_DST}, > {"select-id", required_argument, NULL, OPT_SELECT_ID}, > - DAEMON_LONG_OPTIONS, > + OVN_DAEMON_LONG_OPTIONS, > VLOG_LONG_OPTIONS, > STREAM_SSL_LONG_OPTIONS, > {NULL, 0, NULL, 0}, > @@ -333,7 +333,7 @@ parse_options(int argc, char *argv[]) > printf("DB Schema %s\n", sbrec_get_db_version()); > exit(EXIT_SUCCESS); > > - DAEMON_OPTION_HANDLERS > + OVN_DAEMON_OPTION_HANDLERS > VLOG_OPTION_HANDLERS > STREAM_SSL_OPTION_HANDLERS > > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
