[ovs-dev] [PATCH] ovsdb-server: Preserve remotes across crash and restart.
Gurucharan Shetty
shettyg at nicira.com
Thu Jun 13 18:25:07 UTC 2013
On Thu, Jun 13, 2013 at 10:29 AM, Ben Pfaff <blp at nicira.com> wrote:
> Commit b421d2af0ab (ovsdb-server: Add commands for adding and removing
> remotes) made it possible to make ovsdb-server connect to OVS managers only
> after ovs-vswitchd has completed its initial configuration. But this
> results in an undesirable effect: whenever ovsdb-server crashes, the
> monitor restarts its, but ovsdb-server can no longer connect to the manager
> because the remotes were added during runtime and that information is lost
> during the crash.
>
Looks good to me. Couple of comments.
>
> This commit fixes the problem.
>
> Signed-off-by: Ben Pfaff <blp at nicira.com>
> ---
> The above commit message is taken almost verbatim from one written
> by Guru for a previous version of this fix.
>
> ovsdb/ovsdb-server.c | 98
> ++++++++++++++++++++++++++++++++++++++++++++++---
> tests/ovsdb-server.at | 45 ++++++++++++++++++++++
> 2 files changed, 137 insertions(+), 6 deletions(-)
>
> diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c
> index 20e1964..5aec839 100644
> --- a/ovsdb/ovsdb-server.c
> +++ b/ovsdb/ovsdb-server.c
> @@ -81,8 +81,14 @@ struct add_remote_aux {
> struct sset *remotes;
> struct db *dbs;
> size_t n_dbs;
> + FILE *config_tmpfile;
> };
> static unixctl_cb_func ovsdb_server_add_remote;
> +
> +struct remove_remote_aux {
> + struct sset *remotes;
> + FILE *config_tmpfile;
> +};
>
Do we need the extra data structure here. Looks like the add_remote_aux is
good enough (may be with a different name).
> static unixctl_cb_func ovsdb_server_remove_remote;
> static unixctl_cb_func ovsdb_server_list_remotes;
>
> @@ -99,6 +105,9 @@ static void update_remote_status(const struct
> ovsdb_jsonrpc_server *jsonrpc,
> const struct sset *remotes,
> struct db dbs[], size_t n_dbs);
>
> +static void save_config(FILE *config_file, const struct sset *);
> +static void load_config(FILE *config_file, struct sset *);
> +
> int
> main(int argc, char *argv[])
> {
> @@ -112,6 +121,8 @@ main(int argc, char *argv[])
> int retval;
> long long int status_timer = LLONG_MIN;
> struct add_remote_aux add_remote_aux;
> + struct remove_remote_aux remove_remote_aux;
> + FILE *config_tmpfile;
>
> struct db *dbs;
> int n_dbs;
> @@ -125,8 +136,22 @@ main(int argc, char *argv[])
>
> parse_options(&argc, &argv, &remotes, &unixctl_path, &run_command);
>
> + /* Create and initialize 'config_tmpfile' as a temporary file to hold
> + * ovsdb-server's most basic configuration, and then save our initial
> + * configuration to it. When --monitor is used, this preserves the
> effects
> + * of ovs-appctl commands such as ovsdb-server/add-remote (which
> saves the
> + * new configuration) across crashes. */
> + config_tmpfile = tmpfile();
> + if (!config_tmpfile) {
> + ovs_fatal(errno, "failed to create temporary file");
> + }
> + save_config(config_tmpfile, &remotes);
> +
> daemonize_start();
>
> + /* Load the saved config. */
> + load_config(config_tmpfile, &remotes);
> +
> n_dbs = MAX(1, argc);
> dbs = xcalloc(n_dbs + 1, sizeof *dbs);
> if (argc > 0) {
> @@ -195,10 +220,15 @@ main(int argc, char *argv[])
> add_remote_aux.remotes = &remotes;
> add_remote_aux.dbs = dbs;
> add_remote_aux.n_dbs = n_dbs;
> + add_remote_aux.config_tmpfile = config_tmpfile;
> unixctl_command_register("ovsdb-server/add-remote", "REMOTE", 1, 1,
> ovsdb_server_add_remote, &add_remote_aux);
> +
> + remove_remote_aux.remotes = &remotes;
> + remove_remote_aux.config_tmpfile = config_tmpfile;
> unixctl_command_register("ovsdb-server/remove-remote", "REMOTE", 1, 1,
> - ovsdb_server_remove_remote, &remotes);
> + ovsdb_server_remove_remote,
> &remove_remote_aux);
> +
> unixctl_command_register("ovsdb-server/list-remotes", "", 0, 0,
> ovsdb_server_list_remotes, &remotes);
>
> @@ -922,7 +952,9 @@ ovsdb_server_add_remote(struct unixctl_conn *conn, int
> argc OVS_UNUSED,
> : parse_db_column(aux->dbs, aux->n_dbs, remote,
> &db, &table, &column));
> if (!retval) {
> - sset_add(aux->remotes, remote);
> + if (sset_add(aux->remotes, remote)) {
> + save_config(aux->config_tmpfile, aux->remotes);
> + }
> unixctl_command_reply(conn, NULL);
> } else {
> unixctl_command_reply_error(conn, retval);
> @@ -934,14 +966,15 @@ ovsdb_server_add_remote(struct unixctl_conn *conn,
> int argc OVS_UNUSED,
> * that ovsdb-server services. */
> static void
> ovsdb_server_remove_remote(struct unixctl_conn *conn, int argc OVS_UNUSED,
> - const char *argv[], void *remotes_)
> + const char *argv[], void *aux_)
> {
> - struct sset *remotes = remotes_;
> + struct remove_remote_aux *aux = aux_;
> struct sset_node *node;
>
> - node = sset_find(remotes, argv[1]);
> + node = sset_find(aux->remotes, argv[1]);
> if (node) {
> - sset_delete(remotes, node);
> + sset_delete(aux->remotes, node);
> + save_config(aux->config_tmpfile, aux->remotes);
> unixctl_command_reply(conn, NULL);
> } else {
> unixctl_command_reply_error(conn, "no such remote");
> @@ -1092,3 +1125,56 @@ usage(void)
> leak_checker_usage();
> exit(EXIT_SUCCESS);
> }
> +
> +/* Truncates and replaces the contents of 'config_file' by a
> representatation
s/representatation/representation
> + * of 'remotes'. */
> +static void
> +save_config(FILE *config_file, const struct sset *remotes)
> +{
> + const char *remote;
> + struct json *json;
> + char *s;
> +
> + if (ftruncate(fileno(config_file), 0) == -1) {
> + VLOG_FATAL("failed to truncate temporary file (%s)",
> strerror(errno));
>
+ }
> +
> + json = json_array_create_empty();
> + SSET_FOR_EACH (remote, remotes) {
> + json_array_add(json, json_string_create(remote));
> + }
> + s = json_to_string(json, 0);
> + json_destroy(json);
> +
> + if (fseek(config_file, 0, SEEK_SET) != 0
> + || fputs(s, config_file) == EOF
> + || fflush(config_file) == EOF) {
> + VLOG_FATAL("failed to write temporary file (%s)",
> strerror(errno));
> + }
> + free(s);
> +}
> +
> +/* Clears and replaces 'remotes' by a configuration read from
> 'config_file',
> + * which must have been previously written by save_config(). */
> +static void
> +load_config(FILE *config_file, struct sset *remotes)
> +{
> + struct json *json;
> + size_t i;
> +
> + sset_clear(remotes);
> +
> + if (fseek(config_file, 0, SEEK_SET) != 0) {
> + VLOG_FATAL("seek failed in temporary file (%s)", strerror(errno));
> + }
> + json = json_from_stream(config_file);
> + if (json->type == JSON_STRING) {
> + VLOG_FATAL("reading json failed (%s)", json_string(json));
> + }
> + ovs_assert(json->type == JSON_ARRAY);
> + for (i = 0; i < json->u.array.n; i++) {
> + const struct json *remote = json->u.array.elems[i];
> + sset_add(remotes, json_string(remote));
> + }
> + json_destroy(json);
> +}
> diff --git a/tests/ovsdb-server.at b/tests/ovsdb-server.at
> index 5f73c00..acb5a44 100644
> --- a/tests/ovsdb-server.at
> +++ b/tests/ovsdb-server.at
> @@ -271,6 +271,51 @@ AT_CHECK([test ! -e socket1])
> AT_CHECK([ovs-appctl -t ovsdb-server ovsdb-server/list-remotes])
> AT_CLEANUP
>
> +AT_SETUP([ovsdb-server/add-remote and remove-remote with --monitor])
> +AT_KEYWORDS([ovsdb server positive])
> +# Start ovsdb-server, initially with no remotes.
> +OVS_RUNDIR=`pwd`; export OVS_RUNDIR
> +OVS_LOGDIR=`pwd`; export OVS_LOGDIR
> +ordinal_schema > schema
> +AT_CHECK([ovsdb-tool create db schema], [0], [ignore], [ignore])
> +ON_EXIT([kill `cat *.pid`])
> +AT_CHECK([ovsdb-server -v -vvlog:off --monitor --detach --no-chdir
> --pidfile --log-file db])
> +
> +# Add a remote.
> +AT_CHECK([test ! -e socket1])
> +AT_CHECK([ovs-appctl -t ovsdb-server ovsdb-server/add-remote
> punix:socket1])
> +OVS_WAIT_UNTIL([test -S socket1])
> +AT_CHECK([ovs-appctl -t ovsdb-server ovsdb-server/list-remotes],
> + [0], [punix:socket1
> +])
> +
> +# Kill the daemon process, making it look like a segfault,
> +# and wait for a new daemon process to get spawned and for it to
> +# start listening on 'socket1'.
> +cp ovsdb-server.pid old.pid
> +rm socket1
> +AT_CHECK([kill -SEGV `cat ovsdb-server.pid`])
> +OVS_WAIT_WHILE([kill -0 `cat old.pid`])
> +OVS_WAIT_UNTIL(
> + [test -s ovsdb-server.pid && test `cat ovsdb-server.pid` != `cat
> old.pid`])
> +OVS_WAIT_UNTIL([test -S socket1])
> +
> +# Remove the remote.
> +AT_CHECK([ovs-appctl -t ovsdb-server ovsdb-server/remove-remote
> punix:socket1])
> +OVS_WAIT_UNTIL([test ! -e socket1])
> +AT_CHECK([ovs-appctl -t ovsdb-server ovsdb-server/list-remotes])
> +
> +# Kill the daemon process, making it look like a segfault,
> +# and wait for a new daemon process to get spawned and make sure that it
> +# does not listen on 'socket1'.
> +cp ovsdb-server.pid old.pid
> +AT_CHECK([kill -SEGV `cat ovsdb-server.pid`])
> +OVS_WAIT_WHILE([kill -0 `cat old.pid`])
> +OVS_WAIT_UNTIL(
> + [test -s ovsdb-server.pid && test `cat ovsdb-server.pid` != `cat
> old.pid`])
> +AT_CHECK([test ! -e socket1])
> +AT_CLEANUP
> +
> AT_SETUP([SSL db: implementation])
> AT_KEYWORDS([ovsdb server positive ssl $5])
> AT_SKIP_IF([test "$HAVE_OPENSSL" = no])
> --
> 1.7.2.5
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openvswitch.org/pipermail/ovs-dev/attachments/20130613/c8658cf3/attachment-0003.html>
More information about the dev
mailing list