| Message ID | 20180712134016.14336-13-jkbs@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Daemon mode for ovn-nbctl | expand |
On 07/12/2018 09:40 AM, Jakub Sitnicki wrote: > Make ovn-nbctl act as a unixctl server if we were asked to detach. This > turns ovn-nbctl into a long-lived process that acts a proxy for > interacting with NB DB. The main difference to regular mode of ovn-nbctl > is that in the daemon mode, a local copy of database contents has to be > obtained only once. > > Just two unixctl commands are supported 'run' and 'exit'. The former can > be used to run any ovn-nbctl command or a batch of them as so: > > ovs-appctl -t ovn-nbctl run [OPTIONS] COMMAND [-- [OPTIONS] COMMAND] ... > > Running commands that have not yet been converted to not use ctl_fatal() > will result in death of the daemon process. However, --monitor option > can be used to keep the daemon running. > > Signed-off-by: Jakub Sitnicki <jkbs@redhat.com> > --- > ovn/utilities/ovn-nbctl.8.xml | 40 ++++++++ > ovn/utilities/ovn-nbctl.c | 213 ++++++++++++++++++++++++++++++++++++------ > 2 files changed, 227 insertions(+), 26 deletions(-) > > diff --git a/ovn/utilities/ovn-nbctl.8.xml b/ovn/utilities/ovn-nbctl.8.xml > index abba4ecdb..2cd2fab30 100644 > --- a/ovn/utilities/ovn-nbctl.8.xml > +++ b/ovn/utilities/ovn-nbctl.8.xml > @@ -913,6 +913,43 @@ > </dd> > </dl> > > + <h1>Daemon Mode</h1> > + > + <p> > + If <code>ovn-nbctl</code> is invoked with the <code>--detach</code> > + option (see <code>Daemon Options</code>, below), it runs in the > + background as a daemon and accepts commands from <code>ovs-appctl</code> > + (or another JSON-RPC client) indefinitely. The currently supported > + commands are described below. > + </p> > + > + <p> > + > + </p> > + > + <dl> > + <dt> > + <code>run</code> [<var>options</var>] <var>command</var> > + [<var>arg</var>...] [<code>--</code> [<var>options</var>] > + <var>command</var> [<var>arg</var>...] ...] > + </dt> > + <dd> > + Instructs the daemon process to run one or more <code>ovn-nbctl</code> > + commands described above and reply with the results of running these > + commands. Accepts the <code>--no-wait</code>, <code>--wait</code>, > + <code>--timeout</code>, <code>--dry-run</code>, <code>--oneline</code>, > + and the options described under <code>Table Formatting Options</code> > + in addition to the the command-specific options. > + </dd> > + > + <dt><code>exit</code></dt> > + <dd>Causes <code>ovn-nbctl</code> to gracefully terminate.</dd> > + </dl> > + > + <p> > + Daemon mode is considered experimental. > + </p> > + > <h1>Options</h1> > > <dl> > @@ -982,6 +1019,9 @@ > </dd> > </dl> > > + <h2>Daemon Options</h2> > + <xi:include href="lib/daemon.xml" xmlns:xi="http://www.w3.org/2003/XInclude"/> > + > <h1>Logging options</h1> > <xi:include href="lib/vlog.xml" xmlns:xi="http://www.w3.org/2003/XInclude"/> > > diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c > index 3dd24d193..ba9b7ca49 100644 > --- a/ovn/utilities/ovn-nbctl.c > +++ b/ovn/utilities/ovn-nbctl.c > @@ -20,6 +20,7 @@ > #include <stdio.h> > > #include "command-line.h" > +#include "daemon.h" > #include "db-ctl-base.h" > #include "dirs.h" > #include "fatal-signal.h" > @@ -38,6 +39,7 @@ > #include "table.h" > #include "timeval.h" > #include "timer.h" > +#include "unixctl.h" > #include "util.h" > #include "openvswitch/vlog.h" > > @@ -80,6 +82,13 @@ OVS_NO_RETURN static void nbctl_exit(int status); > /* --leader-only, --no-leader-only: Only accept the leader in a cluster. */ > static int leader_only = true; > > +/* --unixctl-path: Path to use for unixctl server, for "monitor" and "snoop" > + commands. */ > +static char *unixctl_path; > + > +static unixctl_cb_func server_cmd_exit; > +static unixctl_cb_func server_cmd_run; > + > static void nbctl_cmd_init(void); > OVS_NO_RETURN static void usage(void); > static void parse_options(int argc, char *argv[], struct shash *local_options); > @@ -98,15 +107,13 @@ static char * OVS_WARN_UNUSED_RESULT main_loop(const char *args, > size_t n_commands, > struct ovsdb_idl *idl, > const struct timer *); > +static void server_loop(struct ovsdb_idl *idl); > > int > main(int argc, char *argv[]) > { > struct ovsdb_idl *idl; > - struct ctl_command *commands; > struct shash local_options; > - size_t n_commands; > - char *error; > > set_program_name(argv[0]); > fatal_ignore_sigpipe(); > @@ -119,38 +126,55 @@ main(int argc, char *argv[]) > char *args = process_escape_args(argv); > shash_init(&local_options); > parse_options(argc, argv, &local_options); > - commands = ctl_parse_commands(argc - optind, argv + optind, &local_options, > - &n_commands); > - VLOG(ctl_might_write_to_db(commands, n_commands) ? VLL_INFO : VLL_DBG, > - "Called as %s", args); > - > - if (timeout) { > - time_alarm(timeout); > - } > + argc -= optind; > + argv += optind; > > /* Initialize IDL. */ > idl = the_idl = ovsdb_idl_create(db, &nbrec_idl_class, true, false); > ovsdb_idl_set_leader_only(idl, leader_only); > - error = run_prerequisites(commands, n_commands, idl); > - if (error) { > - ctl_fatal("%s", error); > - } > > - error = main_loop(args, commands, n_commands, idl, NULL); > - if (error) { > - ctl_fatal("%s", error); > + if (get_detach()) { > + if (argc != 0) { > + ctl_fatal("non-option arguments not supported with --detach " > + "(use --help for help)"); > + } > + server_loop(idl); > + } else { > + struct ctl_command *commands; > + size_t n_commands; > + char *error; > + > + commands = ctl_parse_commands(argc, argv, &local_options, &n_commands); > + VLOG(ctl_might_write_to_db(commands, n_commands) ? VLL_INFO : VLL_DBG, > + "Called as %s", args); > + > + if (timeout) { > + time_alarm(timeout); > + } > + > + error = run_prerequisites(commands, n_commands, idl); > + if (error) { > + ctl_fatal("%s", error); > + } > + > + error = main_loop(args, commands, n_commands, idl, NULL); > + if (error) { > + ctl_fatal("%s", error); > + } > + > + struct ctl_command *c; > + for (c = commands; c < &commands[n_commands]; c++) { > + ds_destroy(&c->output); > + table_destroy(c->table); > + free(c->table); > + shash_destroy_free_data(&c->options); > + } > + free(commands); > } > > ovsdb_idl_destroy(idl); > idl = the_idl = NULL; > > - for (struct ctl_command *c = commands; c < &commands[n_commands]; c++) { > - ds_destroy(&c->output); > - table_destroy(c->table); > - free(c->table); > - shash_destroy_free_data(&c->options); > - } > - free(commands); > free(args); > exit(EXIT_SUCCESS); > } > @@ -160,6 +184,7 @@ main_loop(const char *args, struct ctl_command *commands, size_t n_commands, > struct ovsdb_idl *idl, const struct timer *wait_timeout) > { > unsigned int seqno; > + bool idl_ready; > > /* Execute the commands. > * > @@ -169,6 +194,11 @@ main_loop(const char *args, struct ctl_command *commands, size_t n_commands, > * it's because the database changed and we need to obtain an up-to-date > * view of the database before we try the transaction again. */ > seqno = ovsdb_idl_get_seqno(idl); > + > + /* IDL might have already obtained the database copy during previous > + * invocation. If so, we can't expect the sequence number to change before > + * we issue any new requests. */ > + idl_ready = ovsdb_idl_has_ever_connected(idl); > for (;;) { > ovsdb_idl_run(idl); > if (!ovsdb_idl_is_alive(idl)) { > @@ -177,7 +207,8 @@ main_loop(const char *args, struct ctl_command *commands, size_t n_commands, > db, ovs_retval_to_string(retval)); > } > > - if (seqno != ovsdb_idl_get_seqno(idl)) { > + if (idl_ready || seqno != ovsdb_idl_get_seqno(idl)) { > + idl_ready = false; > seqno = ovsdb_idl_get_seqno(idl); > > bool retry; > @@ -214,6 +245,7 @@ parse_options(int argc, char *argv[], struct shash *local_options) > OPT_COMMANDS, > OPT_OPTIONS, > OPT_BOOTSTRAP_CA_CERT, > + DAEMON_OPTION_ENUMS, > VLOG_OPTION_ENUMS, > TABLE_OPTION_ENUMS, > SSL_OPTION_ENUMS, > @@ -232,6 +264,7 @@ parse_options(int argc, char *argv[], struct shash *local_options) > {"leader-only", no_argument, &leader_only, true}, > {"no-leader-only", no_argument, &leader_only, false}, > {"version", no_argument, NULL, 'V'}, > + DAEMON_LONG_OPTIONS, > VLOG_LONG_OPTIONS, > STREAM_SSL_LONG_OPTIONS, > {"bootstrap-ca-cert", required_argument, NULL, OPT_BOOTSTRAP_CA_CERT}, > @@ -336,6 +369,7 @@ parse_options(int argc, char *argv[], struct shash *local_options) > } > break; > > + DAEMON_OPTION_HANDLERS > VLOG_OPTION_HANDLERS > TABLE_OPTION_HANDLERS(&table_style) > STREAM_SSL_OPTION_HANDLERS > @@ -529,6 +563,7 @@ Options:\n\ > program_name, program_name, ctl_get_db_cmd_usage(), > ctl_list_db_tables_usage(), default_nb_db()); > table_usage(); > + daemon_usage(); > vlog_usage(); > printf("\ > --no-syslog equivalent to --verbose=nbctl:syslog:warn\n"); > @@ -4562,3 +4597,129 @@ nbctl_cmd_init(void) > ctl_init(&nbrec_idl_class, nbrec_table_classes, tables, NULL, nbctl_exit); > ctl_register_commands(nbctl_commands); > } > + > +static void > +server_cmd_exit(struct unixctl_conn *conn, int argc OVS_UNUSED, > + const char *argv[] OVS_UNUSED, void *exiting_) > +{ > + bool *exiting = exiting_; > + *exiting = true; > + unixctl_command_reply(conn, NULL); > +} > + > +static void > +server_cmd_run(struct unixctl_conn *conn, int argc, const char **argv_, > + void *idl_) > +{ > + struct ovsdb_idl *idl = idl_; > + struct ctl_command *commands = NULL; > + struct shash local_options; > + size_t n_commands = 0; > + char *error = NULL; > + > + /* Copy args so that getopt() can permute them. Leave last entry NULL. */ > + char **argv = xcalloc(argc + 1, sizeof *argv); > + for (int i = 0; i < argc; i++) { > + argv[i] = xstrdup(argv_[i]); > + } > + > + /* Reset global state. */ > + oneline = false; > + dry_run = false; > + wait_type = NBCTL_WAIT_NONE; > + force_wait = false; > + timeout = 0; > + table_style = table_style_default; Not all global state is being reset here. The biggest thing I spotted was that the vlog level is not reset. > + > + /* Parse commands & options. */ > + char *args = process_escape_args(argv); > + shash_init(&local_options); > + optind = 0; > + parse_options(argc, argv, &local_options); Calling parse_options() here is interesting. There are some options that are relevant, some options that are irrelevant and ignored, and there are some options that we really don't want to parse here. For instance, "commands", "help", "version", and "options" will result in the server process exiting. I know in your cover letter, you mentioned that there were still some places where ctl_fatal() is called. I'm not sure if you had noticed these places where exit() is called as well. > + commands = ctl_parse_commands(argc - optind, argv + optind, > + &local_options, &n_commands); > + VLOG(ctl_might_write_to_db(commands, n_commands) ? VLL_INFO : VLL_DBG, > + "Running command %s", args); > + > + struct timer *wait_timeout = NULL; > + struct timer wait_timeout_; > + if (timeout) { > + wait_timeout = &wait_timeout_; > + timer_set_duration(wait_timeout, timeout * 1000); > + } > + > + error = run_prerequisites(commands, n_commands, idl); > + if (error) { > + unixctl_command_reply_error(conn, error); > + goto out; > + } > + error = main_loop(args, commands, n_commands, idl, wait_timeout); > + if (error) { > + unixctl_command_reply_error(conn, error); > + goto out; > + } > + > + struct ds output = DS_EMPTY_INITIALIZER; > + for (struct ctl_command *c = commands; c < &commands[n_commands]; c++) { > + if (c->table) { > + table_format(c->table, &table_style, &output); > + } else if (oneline) { > + oneline_format(&c->output, &output); > + } else { > + ds_put_cstr(&output, ds_cstr_ro(&c->output)); > + } > + > + ds_destroy(&c->output); > + table_destroy(c->table); > + free(c->table); > + } > + unixctl_command_reply(conn, ds_cstr_ro(&output)); > + ds_destroy(&output); > + > +out: > + free(error); > + for (struct ctl_command *c = commands; c < &commands[n_commands]; c++) { > + shash_destroy_free_data(&c->options); > + } > + free(commands); > + shash_destroy_free_data(&local_options); > + free(args); > + for (int i = 0; i < argc; i++) { > + free(argv[i]); > + } > + free(argv); > +} > + > +static void > +server_cmd_init(struct ovsdb_idl *idl, bool *exiting) > +{ > + unixctl_command_register("exit", "", 0, 0, server_cmd_exit, exiting); > + unixctl_command_register("run", "", 1, INT_MAX, server_cmd_run, idl); > +} > + > +static void > +server_loop(struct ovsdb_idl *idl) > +{ > + struct unixctl_server *server = NULL; > + bool exiting = false; > + > + daemonize_start(false); > + int error = unixctl_server_create(unixctl_path, &server); > + if (error) { > + ctl_fatal("failed to create unixctl server (%s)", > + ovs_retval_to_string(error)); > + } > + server_cmd_init(idl, &exiting); > + > + for (;;) { > + unixctl_server_run(server); > + daemonize_complete(); You should move the call to daemonize_complete() to outside the for loop. It is a no-op after the first time you call it. > + unixctl_server_wait(server); > + if (exiting) { > + break; > + } > + poll_block(); > + } > + > + unixctl_server_destroy(server); > +} >
On Thu, 12 Jul 2018 17:10:04 -0400 Mark Michelson <mmichels@redhat.com> wrote: > On 07/12/2018 09:40 AM, Jakub Sitnicki wrote: > > Make ovn-nbctl act as a unixctl server if we were asked to detach. This > > turns ovn-nbctl into a long-lived process that acts a proxy for > > interacting with NB DB. The main difference to regular mode of ovn-nbctl > > is that in the daemon mode, a local copy of database contents has to be > > obtained only once. > > > > Just two unixctl commands are supported 'run' and 'exit'. The former can > > be used to run any ovn-nbctl command or a batch of them as so: > > > > ovs-appctl -t ovn-nbctl run [OPTIONS] COMMAND [-- [OPTIONS] COMMAND] ... > > > > Running commands that have not yet been converted to not use ctl_fatal() > > will result in death of the daemon process. However, --monitor option > > can be used to keep the daemon running. > > > > Signed-off-by: Jakub Sitnicki <jkbs@redhat.com> > > --- > > ovn/utilities/ovn-nbctl.8.xml | 40 ++++++++ > > ovn/utilities/ovn-nbctl.c | 213 ++++++++++++++++++++++++++++++++++++------ > > 2 files changed, 227 insertions(+), 26 deletions(-) > > (...) > > +static void > > +server_cmd_run(struct unixctl_conn *conn, int argc, const char **argv_, > > + void *idl_) > > +{ > > + struct ovsdb_idl *idl = idl_; > > + struct ctl_command *commands = NULL; > > + struct shash local_options; > > + size_t n_commands = 0; > > + char *error = NULL; > > + > > + /* Copy args so that getopt() can permute them. Leave last entry NULL. */ > > + char **argv = xcalloc(argc + 1, sizeof *argv); > > + for (int i = 0; i < argc; i++) { > > + argv[i] = xstrdup(argv_[i]); > > + } > > + > > + /* Reset global state. */ > > + oneline = false; > > + dry_run = false; > > + wait_type = NBCTL_WAIT_NONE; > > + force_wait = false; > > + timeout = 0; > > + table_style = table_style_default; > > Not all global state is being reset here. The biggest thing I spotted > was that the vlog level is not reset. Let me fix that. > > > + > > + /* Parse commands & options. */ > > + char *args = process_escape_args(argv); > > + shash_init(&local_options); > > + optind = 0; > > + parse_options(argc, argv, &local_options); > > Calling parse_options() here is interesting. There are some options that > are relevant, some options that are irrelevant and ignored, and there > are some options that we really don't want to parse here. > > For instance, "commands", "help", "version", and "options" will result > in the server process exiting. I know in your cover letter, you > mentioned that there were still some places where ctl_fatal() is called. > I'm not sure if you had noticed these places where exit() is called as well. Perhaps I need to revisit the idea of having an options parses tailored to server mode needs. Other option would be to ignore options that don't make sense for the server. > > > + commands = ctl_parse_commands(argc - optind, argv + optind, > > + &local_options, &n_commands); > > + VLOG(ctl_might_write_to_db(commands, n_commands) ? VLL_INFO : VLL_DBG, > > + "Running command %s", args); > > + > > + struct timer *wait_timeout = NULL; > > + struct timer wait_timeout_; > > + if (timeout) { > > + wait_timeout = &wait_timeout_; > > + timer_set_duration(wait_timeout, timeout * 1000); > > + } > > + > > + error = run_prerequisites(commands, n_commands, idl); > > + if (error) { > > + unixctl_command_reply_error(conn, error); > > + goto out; > > + } > > + error = main_loop(args, commands, n_commands, idl, wait_timeout); > > + if (error) { > > + unixctl_command_reply_error(conn, error); > > + goto out; > > + } > > + > > + struct ds output = DS_EMPTY_INITIALIZER; > > + for (struct ctl_command *c = commands; c < &commands[n_commands]; c++) { > > + if (c->table) { > > + table_format(c->table, &table_style, &output); > > + } else if (oneline) { > > + oneline_format(&c->output, &output); > > + } else { > > + ds_put_cstr(&output, ds_cstr_ro(&c->output)); > > + } > > + > > + ds_destroy(&c->output); > > + table_destroy(c->table); > > + free(c->table); > > + } > > + unixctl_command_reply(conn, ds_cstr_ro(&output)); > > + ds_destroy(&output); > > + > > +out: > > + free(error); > > + for (struct ctl_command *c = commands; c < &commands[n_commands]; c++) { > > + shash_destroy_free_data(&c->options); > > + } > > + free(commands); > > + shash_destroy_free_data(&local_options); > > + free(args); > > + for (int i = 0; i < argc; i++) { > > + free(argv[i]); > > + } > > + free(argv); > > +} > > + > > +static void > > +server_cmd_init(struct ovsdb_idl *idl, bool *exiting) > > +{ > > + unixctl_command_register("exit", "", 0, 0, server_cmd_exit, exiting); > > + unixctl_command_register("run", "", 1, INT_MAX, server_cmd_run, idl); > > +} > > + > > +static void > > +server_loop(struct ovsdb_idl *idl) > > +{ > > + struct unixctl_server *server = NULL; > > + bool exiting = false; > > + > > + daemonize_start(false); > > + int error = unixctl_server_create(unixctl_path, &server); > > + if (error) { > > + ctl_fatal("failed to create unixctl server (%s)", > > + ovs_retval_to_string(error)); > > + } > > + server_cmd_init(idl, &exiting); > > + > > + for (;;) { > > + unixctl_server_run(server); > > + daemonize_complete(); > > You should move the call to daemonize_complete() to outside the for > loop. It is a no-op after the first time you call it. You're right. The server loop is modeled after ovn-trace main loop. So now I'm also wondering if I should check the IDL connection state before calling daemonize_complete(). Let me do some testing to figure it out. > > > + unixctl_server_wait(server); > > + if (exiting) { > > + break; > > + } > > + poll_block(); > > + } > > + > > + unixctl_server_destroy(server); > > +} > > >
diff --git a/ovn/utilities/ovn-nbctl.8.xml b/ovn/utilities/ovn-nbctl.8.xml index abba4ecdb..2cd2fab30 100644 --- a/ovn/utilities/ovn-nbctl.8.xml +++ b/ovn/utilities/ovn-nbctl.8.xml @@ -913,6 +913,43 @@ </dd> </dl> + <h1>Daemon Mode</h1> + + <p> + If <code>ovn-nbctl</code> is invoked with the <code>--detach</code> + option (see <code>Daemon Options</code>, below), it runs in the + background as a daemon and accepts commands from <code>ovs-appctl</code> + (or another JSON-RPC client) indefinitely. The currently supported + commands are described below. + </p> + + <p> + + </p> + + <dl> + <dt> + <code>run</code> [<var>options</var>] <var>command</var> + [<var>arg</var>...] [<code>--</code> [<var>options</var>] + <var>command</var> [<var>arg</var>...] ...] + </dt> + <dd> + Instructs the daemon process to run one or more <code>ovn-nbctl</code> + commands described above and reply with the results of running these + commands. Accepts the <code>--no-wait</code>, <code>--wait</code>, + <code>--timeout</code>, <code>--dry-run</code>, <code>--oneline</code>, + and the options described under <code>Table Formatting Options</code> + in addition to the the command-specific options. + </dd> + + <dt><code>exit</code></dt> + <dd>Causes <code>ovn-nbctl</code> to gracefully terminate.</dd> + </dl> + + <p> + Daemon mode is considered experimental. + </p> + <h1>Options</h1> <dl> @@ -982,6 +1019,9 @@ </dd> </dl> + <h2>Daemon Options</h2> + <xi:include href="lib/daemon.xml" xmlns:xi="http://www.w3.org/2003/XInclude"/> + <h1>Logging options</h1> <xi:include href="lib/vlog.xml" xmlns:xi="http://www.w3.org/2003/XInclude"/> diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c index 3dd24d193..ba9b7ca49 100644 --- a/ovn/utilities/ovn-nbctl.c +++ b/ovn/utilities/ovn-nbctl.c @@ -20,6 +20,7 @@ #include <stdio.h> #include "command-line.h" +#include "daemon.h" #include "db-ctl-base.h" #include "dirs.h" #include "fatal-signal.h" @@ -38,6 +39,7 @@ #include "table.h" #include "timeval.h" #include "timer.h" +#include "unixctl.h" #include "util.h" #include "openvswitch/vlog.h" @@ -80,6 +82,13 @@ OVS_NO_RETURN static void nbctl_exit(int status); /* --leader-only, --no-leader-only: Only accept the leader in a cluster. */ static int leader_only = true; +/* --unixctl-path: Path to use for unixctl server, for "monitor" and "snoop" + commands. */ +static char *unixctl_path; + +static unixctl_cb_func server_cmd_exit; +static unixctl_cb_func server_cmd_run; + static void nbctl_cmd_init(void); OVS_NO_RETURN static void usage(void); static void parse_options(int argc, char *argv[], struct shash *local_options); @@ -98,15 +107,13 @@ static char * OVS_WARN_UNUSED_RESULT main_loop(const char *args, size_t n_commands, struct ovsdb_idl *idl, const struct timer *); +static void server_loop(struct ovsdb_idl *idl); int main(int argc, char *argv[]) { struct ovsdb_idl *idl; - struct ctl_command *commands; struct shash local_options; - size_t n_commands; - char *error; set_program_name(argv[0]); fatal_ignore_sigpipe(); @@ -119,38 +126,55 @@ main(int argc, char *argv[]) char *args = process_escape_args(argv); shash_init(&local_options); parse_options(argc, argv, &local_options); - commands = ctl_parse_commands(argc - optind, argv + optind, &local_options, - &n_commands); - VLOG(ctl_might_write_to_db(commands, n_commands) ? VLL_INFO : VLL_DBG, - "Called as %s", args); - - if (timeout) { - time_alarm(timeout); - } + argc -= optind; + argv += optind; /* Initialize IDL. */ idl = the_idl = ovsdb_idl_create(db, &nbrec_idl_class, true, false); ovsdb_idl_set_leader_only(idl, leader_only); - error = run_prerequisites(commands, n_commands, idl); - if (error) { - ctl_fatal("%s", error); - } - error = main_loop(args, commands, n_commands, idl, NULL); - if (error) { - ctl_fatal("%s", error); + if (get_detach()) { + if (argc != 0) { + ctl_fatal("non-option arguments not supported with --detach " + "(use --help for help)"); + } + server_loop(idl); + } else { + struct ctl_command *commands; + size_t n_commands; + char *error; + + commands = ctl_parse_commands(argc, argv, &local_options, &n_commands); + VLOG(ctl_might_write_to_db(commands, n_commands) ? VLL_INFO : VLL_DBG, + "Called as %s", args); + + if (timeout) { + time_alarm(timeout); + } + + error = run_prerequisites(commands, n_commands, idl); + if (error) { + ctl_fatal("%s", error); + } + + error = main_loop(args, commands, n_commands, idl, NULL); + if (error) { + ctl_fatal("%s", error); + } + + struct ctl_command *c; + for (c = commands; c < &commands[n_commands]; c++) { + ds_destroy(&c->output); + table_destroy(c->table); + free(c->table); + shash_destroy_free_data(&c->options); + } + free(commands); } ovsdb_idl_destroy(idl); idl = the_idl = NULL; - for (struct ctl_command *c = commands; c < &commands[n_commands]; c++) { - ds_destroy(&c->output); - table_destroy(c->table); - free(c->table); - shash_destroy_free_data(&c->options); - } - free(commands); free(args); exit(EXIT_SUCCESS); } @@ -160,6 +184,7 @@ main_loop(const char *args, struct ctl_command *commands, size_t n_commands, struct ovsdb_idl *idl, const struct timer *wait_timeout) { unsigned int seqno; + bool idl_ready; /* Execute the commands. * @@ -169,6 +194,11 @@ main_loop(const char *args, struct ctl_command *commands, size_t n_commands, * it's because the database changed and we need to obtain an up-to-date * view of the database before we try the transaction again. */ seqno = ovsdb_idl_get_seqno(idl); + + /* IDL might have already obtained the database copy during previous + * invocation. If so, we can't expect the sequence number to change before + * we issue any new requests. */ + idl_ready = ovsdb_idl_has_ever_connected(idl); for (;;) { ovsdb_idl_run(idl); if (!ovsdb_idl_is_alive(idl)) { @@ -177,7 +207,8 @@ main_loop(const char *args, struct ctl_command *commands, size_t n_commands, db, ovs_retval_to_string(retval)); } - if (seqno != ovsdb_idl_get_seqno(idl)) { + if (idl_ready || seqno != ovsdb_idl_get_seqno(idl)) { + idl_ready = false; seqno = ovsdb_idl_get_seqno(idl); bool retry; @@ -214,6 +245,7 @@ parse_options(int argc, char *argv[], struct shash *local_options) OPT_COMMANDS, OPT_OPTIONS, OPT_BOOTSTRAP_CA_CERT, + DAEMON_OPTION_ENUMS, VLOG_OPTION_ENUMS, TABLE_OPTION_ENUMS, SSL_OPTION_ENUMS, @@ -232,6 +264,7 @@ parse_options(int argc, char *argv[], struct shash *local_options) {"leader-only", no_argument, &leader_only, true}, {"no-leader-only", no_argument, &leader_only, false}, {"version", no_argument, NULL, 'V'}, + DAEMON_LONG_OPTIONS, VLOG_LONG_OPTIONS, STREAM_SSL_LONG_OPTIONS, {"bootstrap-ca-cert", required_argument, NULL, OPT_BOOTSTRAP_CA_CERT}, @@ -336,6 +369,7 @@ parse_options(int argc, char *argv[], struct shash *local_options) } break; + DAEMON_OPTION_HANDLERS VLOG_OPTION_HANDLERS TABLE_OPTION_HANDLERS(&table_style) STREAM_SSL_OPTION_HANDLERS @@ -529,6 +563,7 @@ Options:\n\ program_name, program_name, ctl_get_db_cmd_usage(), ctl_list_db_tables_usage(), default_nb_db()); table_usage(); + daemon_usage(); vlog_usage(); printf("\ --no-syslog equivalent to --verbose=nbctl:syslog:warn\n"); @@ -4562,3 +4597,129 @@ nbctl_cmd_init(void) ctl_init(&nbrec_idl_class, nbrec_table_classes, tables, NULL, nbctl_exit); ctl_register_commands(nbctl_commands); } + +static void +server_cmd_exit(struct unixctl_conn *conn, int argc OVS_UNUSED, + const char *argv[] OVS_UNUSED, void *exiting_) +{ + bool *exiting = exiting_; + *exiting = true; + unixctl_command_reply(conn, NULL); +} + +static void +server_cmd_run(struct unixctl_conn *conn, int argc, const char **argv_, + void *idl_) +{ + struct ovsdb_idl *idl = idl_; + struct ctl_command *commands = NULL; + struct shash local_options; + size_t n_commands = 0; + char *error = NULL; + + /* Copy args so that getopt() can permute them. Leave last entry NULL. */ + char **argv = xcalloc(argc + 1, sizeof *argv); + for (int i = 0; i < argc; i++) { + argv[i] = xstrdup(argv_[i]); + } + + /* Reset global state. */ + oneline = false; + dry_run = false; + wait_type = NBCTL_WAIT_NONE; + force_wait = false; + timeout = 0; + table_style = table_style_default; + + /* Parse commands & options. */ + char *args = process_escape_args(argv); + shash_init(&local_options); + optind = 0; + parse_options(argc, argv, &local_options); + commands = ctl_parse_commands(argc - optind, argv + optind, + &local_options, &n_commands); + VLOG(ctl_might_write_to_db(commands, n_commands) ? VLL_INFO : VLL_DBG, + "Running command %s", args); + + struct timer *wait_timeout = NULL; + struct timer wait_timeout_; + if (timeout) { + wait_timeout = &wait_timeout_; + timer_set_duration(wait_timeout, timeout * 1000); + } + + error = run_prerequisites(commands, n_commands, idl); + if (error) { + unixctl_command_reply_error(conn, error); + goto out; + } + error = main_loop(args, commands, n_commands, idl, wait_timeout); + if (error) { + unixctl_command_reply_error(conn, error); + goto out; + } + + struct ds output = DS_EMPTY_INITIALIZER; + for (struct ctl_command *c = commands; c < &commands[n_commands]; c++) { + if (c->table) { + table_format(c->table, &table_style, &output); + } else if (oneline) { + oneline_format(&c->output, &output); + } else { + ds_put_cstr(&output, ds_cstr_ro(&c->output)); + } + + ds_destroy(&c->output); + table_destroy(c->table); + free(c->table); + } + unixctl_command_reply(conn, ds_cstr_ro(&output)); + ds_destroy(&output); + +out: + free(error); + for (struct ctl_command *c = commands; c < &commands[n_commands]; c++) { + shash_destroy_free_data(&c->options); + } + free(commands); + shash_destroy_free_data(&local_options); + free(args); + for (int i = 0; i < argc; i++) { + free(argv[i]); + } + free(argv); +} + +static void +server_cmd_init(struct ovsdb_idl *idl, bool *exiting) +{ + unixctl_command_register("exit", "", 0, 0, server_cmd_exit, exiting); + unixctl_command_register("run", "", 1, INT_MAX, server_cmd_run, idl); +} + +static void +server_loop(struct ovsdb_idl *idl) +{ + struct unixctl_server *server = NULL; + bool exiting = false; + + daemonize_start(false); + int error = unixctl_server_create(unixctl_path, &server); + if (error) { + ctl_fatal("failed to create unixctl server (%s)", + ovs_retval_to_string(error)); + } + server_cmd_init(idl, &exiting); + + for (;;) { + unixctl_server_run(server); + daemonize_complete(); + unixctl_server_wait(server); + if (exiting) { + break; + } + poll_block(); + } + + unixctl_server_destroy(server); +}
Make ovn-nbctl act as a unixctl server if we were asked to detach. This turns ovn-nbctl into a long-lived process that acts a proxy for interacting with NB DB. The main difference to regular mode of ovn-nbctl is that in the daemon mode, a local copy of database contents has to be obtained only once. Just two unixctl commands are supported 'run' and 'exit'. The former can be used to run any ovn-nbctl command or a batch of them as so: ovs-appctl -t ovn-nbctl run [OPTIONS] COMMAND [-- [OPTIONS] COMMAND] ... Running commands that have not yet been converted to not use ctl_fatal() will result in death of the daemon process. However, --monitor option can be used to keep the daemon running. Signed-off-by: Jakub Sitnicki <jkbs@redhat.com> --- ovn/utilities/ovn-nbctl.8.xml | 40 ++++++++ ovn/utilities/ovn-nbctl.c | 213 ++++++++++++++++++++++++++++++++++++------ 2 files changed, 227 insertions(+), 26 deletions(-)