On 7/30/24 13:52, Dumitru Ceara wrote: > On 7/29/24 22:47, Rosemarie O'Riorden wrote: >> All other OVN daemons have the --unixctl option to specify the control >> socket file. It was missing from ovn-controller and ovn-controller-vtep, >> which this patch fixes. >> >> The ovn-controller-vtep documentation was also missing the already >> supported --help and --version options, which are now reflected in the >> docs. >> >> Reported-at: https://issues.redhat.com/browse/FDP-714 >> Signed-off-by: Rosemarie O'Riorden <[email protected]> >> --- > > Thanks for the patch, Rosemarie! It looks good overall, I only have one > minor comment. > >> controller-vtep/ovn-controller-vtep.8.xml | 5 +++++ >> controller-vtep/ovn-controller-vtep.c | 11 ++++++++++- >> controller/ovn-controller.8.xml | 3 ++- >> controller/ovn-controller.c | 11 ++++++++++- >> 4 files changed, 27 insertions(+), 3 deletions(-) >> >> diff --git a/controller-vtep/ovn-controller-vtep.8.xml >> b/controller-vtep/ovn-controller-vtep.8.xml >> index 89acae7ed..6aec43fa7 100644 >> --- a/controller-vtep/ovn-controller-vtep.8.xml >> +++ b/controller-vtep/ovn-controller-vtep.8.xml >> @@ -28,6 +28,11 @@ >> <xi:include href="lib/ssl-bootstrap.xml" >> xmlns:xi="http://www.w3.org/2003/XInclude"/> >> <xi:include href="lib/ssl-peer-ca-cert.xml" >> xmlns:xi="http://www.w3.org/2003/XInclude"/> >> >> + <h2>Other Options</h2> >> + <xi:include href="lib/unixctl.xml" >> xmlns:xi="http://www.w3.org/2003/XInclude"/> >> + <h3></h3> > > This <h3></h3> seems weird.. should it actually be part of common.xml?
It's a separator between sections, so it shouldn't be part of the section. Our xml-to-nroff translation is not ideal. Without such a separator common options become subsection of unixctl options, unfortunately. Best regards, Ilya Maximets. > In any case, we probably don't need it until we actually include common.xml. > >> + <xi:include href="lib/common.xml" >> xmlns:xi="http://www.w3.org/2003/XInclude"/> >> + > > This should probably be in a separate patch, it doesn't really have > anything to do with the unixctl man page. > >> <h1>Configuration</h1> >> <p> >> <code>ovn-controller-vtep</code> retrieves its configuration >> diff --git a/controller-vtep/ovn-controller-vtep.c >> b/controller-vtep/ovn-controller-vtep.c >> index 4472e5285..698511482 100644 >> --- a/controller-vtep/ovn-controller-vtep.c >> +++ b/controller-vtep/ovn-controller-vtep.c >> @@ -58,6 +58,9 @@ static char *vtep_remote; >> static char *ovnsb_remote; >> static char *default_db_; >> >> +/* --unixctl-path: Path to use for unixctl server socket. */ >> +static char *unixctl_path; >> + >> /* Returns true if the northd internal version stored in SB_Global >> * and ovn-controller-vtep internal version match. >> */ >> @@ -118,7 +121,7 @@ main(int argc, char *argv[]) >> >> daemonize_start(false, false); >> >> - char *abs_unixctl_path = get_abs_unix_ctl_path(NULL); >> + char *abs_unixctl_path = get_abs_unix_ctl_path(unixctl_path); >> retval = unixctl_server_create(abs_unixctl_path, &unixctl); >> free(abs_unixctl_path); >> >> @@ -287,6 +290,7 @@ parse_options(int argc, char *argv[]) >> {"vtep-db", required_argument, NULL, 'D'}, >> {"help", no_argument, NULL, 'h'}, >> {"version", no_argument, NULL, 'V'}, >> + {"unixctl", required_argument, NULL, 'u'}, >> VLOG_LONG_OPTIONS, >> OVN_DAEMON_LONG_OPTIONS, >> STREAM_SSL_LONG_OPTIONS, >> @@ -322,6 +326,10 @@ parse_options(int argc, char *argv[]) >> ovn_print_version(OFP13_VERSION, OFP13_VERSION); >> exit(EXIT_SUCCESS); >> >> + case 'u': >> + unixctl_path = optarg; >> + break; >> + >> VLOG_OPTION_HANDLERS >> OVN_DAEMON_OPTION_HANDLERS >> STREAM_SSL_OPTION_HANDLERS >> @@ -364,6 +372,7 @@ Options:\n\ >> (default: %s)\n\ >> --ovnsb-db=DATABASE connect to ovn-sb database at DATABASE\n\ >> (default: %s)\n\ >> + -u, --unixctl=SOCKET set control socket name\n\ >> -h, --help display this help message\n\ >> -o, --options list available options\n\ >> -V, --version display version information\n\ >> diff --git a/controller/ovn-controller.8.xml >> b/controller/ovn-controller.8.xml >> index faefa77b9..bcae4617e 100644 >> --- a/controller/ovn-controller.8.xml >> +++ b/controller/ovn-controller.8.xml >> @@ -47,7 +47,8 @@ >> <xi:include href="lib/ssl-peer-ca-cert.xml" >> xmlns:xi="http://www.w3.org/2003/XInclude"/> >> >> <h2>Other Options</h2> >> - >> + <xi:include href="lib/unixctl.xml" >> xmlns:xi="http://www.w3.org/2003/XInclude"/> >> + <h3></h3> >> <xi:include href="lib/common.xml" >> xmlns:xi="http://www.w3.org/2003/XInclude"/> >> >> >> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c >> index f5674184a..936b42636 100644 >> --- a/controller/ovn-controller.c >> +++ b/controller/ovn-controller.c >> @@ -137,6 +137,9 @@ static const char *ssl_private_key_file; >> static const char *ssl_certificate_file; >> static const char *ssl_ca_cert_file; >> >> +/* --unixctl-path: Path to use for unixctl server socket. */ >> +static char *unixctl_path; >> + >> /* By default don't set an upper bound for the lflow cache and enable auto >> * trimming above 10K logical flows when reducing cache size by 50%. >> */ >> @@ -4835,7 +4838,7 @@ main(int argc, char *argv[]) >> >> daemonize_start(true, false); >> >> - char *abs_unixctl_path = get_abs_unix_ctl_path(NULL); >> + 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) { >> @@ -5957,6 +5960,7 @@ parse_options(int argc, char *argv[]) >> static struct option long_options[] = { >> {"help", no_argument, NULL, 'h'}, >> {"version", no_argument, NULL, 'V'}, >> + {"unixctl", required_argument, NULL, 'u'}, >> VLOG_LONG_OPTIONS, >> OVN_DAEMON_LONG_OPTIONS, >> STREAM_SSL_LONG_OPTIONS, >> @@ -5986,6 +5990,10 @@ parse_options(int argc, char *argv[]) >> printf("SB DB Schema %s\n", sbrec_get_db_version()); >> exit(EXIT_SUCCESS); >> >> + case 'u': >> + unixctl_path = optarg; >> + break; >> + >> VLOG_OPTION_HANDLERS >> OVN_DAEMON_OPTION_HANDLERS >> >> @@ -6061,6 +6069,7 @@ usage(void) >> daemon_usage(); >> vlog_usage(); >> printf("\nOther options:\n" >> + " -u, --unixctl=SOCKET set control socket name\n" >> " -n custom chassis name\n" >> " -h, --help display this help message\n" >> " -V, --version display version information\n"); > > Regards, > Dumitru > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
