diff mbox series

[ovs-dev,08/22] ovsdb: Track jsonrpc options per remote.

Message ID 20231214010431.1664005-9-i.maximets@ovn.org
State Superseded
Delegated to: Ilya Maximets
Headers show
Series [ovs-dev,01/22] ovsdb-server.at: Enbale debug logs in active-backup tests. | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/intel-ovs-compilation fail test: fail

Commit Message

Ilya Maximets Dec. 14, 2023, 1:04 a.m. UTC 
Store KSON-RPC options for each remote separately, so it will be
possible to have different configurations per remote in the future.

These are also stored to and loaded from the temporary file that
OVSDB is using to restore runtime configuration of the server
restarted by the monitor process after a crash.

Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
---
 ovsdb/jsonrpc-server.c |  11 ++++
 ovsdb/jsonrpc-server.h |   6 +-
 ovsdb/ovsdb-server.c   | 136 ++++++++++++++++++++++++++++-------------
 3 files changed, 110 insertions(+), 43 deletions(-)

Comments

Frode Nordahl Dec. 14, 2023, 7:15 a.m. UTC | #1 
On Thu, Dec 14, 2023 at 2:05 AM Ilya Maximets <i.maximets@ovn.org> wrote:
>
> Store KSON-RPC options for each remote separately, so it will be

s/KSON/JSON

> possible to have different configurations per remote in the future.
>
> These are also stored to and loaded from the temporary file that
> OVSDB is using to restore runtime configuration of the server
> restarted by the monitor process after a crash.
>
> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
> ---
>  ovsdb/jsonrpc-server.c |  11 ++++
>  ovsdb/jsonrpc-server.h |   6 +-
>  ovsdb/ovsdb-server.c   | 136 ++++++++++++++++++++++++++++-------------
>  3 files changed, 110 insertions(+), 43 deletions(-)
>
> diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c
> index 51b7db886..299afbb1d 100644
> --- a/ovsdb/jsonrpc-server.c
> +++ b/ovsdb/jsonrpc-server.c
> @@ -219,6 +219,17 @@ ovsdb_jsonrpc_default_options(const char *target)
>      return options;
>  }
>
> +struct ovsdb_jsonrpc_options *
> +ovsdb_jsonrpc_options_clone(const struct ovsdb_jsonrpc_options *options)
> +{
> +    struct ovsdb_jsonrpc_options *clone;
> +
> +    clone = xmemdup(options, sizeof *options);
> +    clone->role = nullable_xstrdup(options->role);
> +
> +    return clone;
> +}
> +
>  struct json *
>  ovsdb_jsonrpc_options_to_json(const struct ovsdb_jsonrpc_options *options)
>  {
> diff --git a/ovsdb/jsonrpc-server.h b/ovsdb/jsonrpc-server.h
> index 9c49966c1..39366ad70 100644
> --- a/ovsdb/jsonrpc-server.h
> +++ b/ovsdb/jsonrpc-server.h
> @@ -39,8 +39,10 @@ struct ovsdb_jsonrpc_options {
>      int dscp;                   /* Dscp value for manager connections */
>      char *role;                 /* Role, for role-based access controls */
>  };
> -struct ovsdb_jsonrpc_options *
> -ovsdb_jsonrpc_default_options(const char *target);
> +struct ovsdb_jsonrpc_options *ovsdb_jsonrpc_default_options(
> +    const char *target);
> +struct ovsdb_jsonrpc_options *ovsdb_jsonrpc_options_clone(
> +    const struct ovsdb_jsonrpc_options *);
>
>  struct json *ovsdb_jsonrpc_options_to_json(
>                                  const struct ovsdb_jsonrpc_options *)
> diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c
> index c294ebe67..7f65cadfe 100644
> --- a/ovsdb/ovsdb-server.c
> +++ b/ovsdb/ovsdb-server.c
> @@ -101,7 +101,7 @@ static unixctl_cb_func ovsdb_server_get_sync_status;
>  static unixctl_cb_func ovsdb_server_get_db_storage_status;
>
>  struct server_config {
> -    struct sset *remotes;
> +    struct shash *remotes;

When reading this patch I was looking for the reason for making the
change from sset to shash. I am sure you do have a good reason, would
it make sense to state it in the commit message?
Ilya Maximets Dec. 14, 2023, 10:41 a.m. UTC | #2 
On 12/14/23 08:15, Frode Nordahl wrote:
> On Thu, Dec 14, 2023 at 2:05 AM Ilya Maximets <i.maximets@ovn.org> wrote:
>>
>> Store KSON-RPC options for each remote separately, so it will be
> 
> s/KSON/JSON
> 
>> possible to have different configurations per remote in the future.
>>
>> These are also stored to and loaded from the temporary file that
>> OVSDB is using to restore runtime configuration of the server
>> restarted by the monitor process after a crash.
>>
>> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
>> ---
>>  ovsdb/jsonrpc-server.c |  11 ++++
>>  ovsdb/jsonrpc-server.h |   6 +-
>>  ovsdb/ovsdb-server.c   | 136 ++++++++++++++++++++++++++++-------------
>>  3 files changed, 110 insertions(+), 43 deletions(-)
>>
>> diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c
>> index 51b7db886..299afbb1d 100644
>> --- a/ovsdb/jsonrpc-server.c
>> +++ b/ovsdb/jsonrpc-server.c
>> @@ -219,6 +219,17 @@ ovsdb_jsonrpc_default_options(const char *target)
>>      return options;
>>  }
>>
>> +struct ovsdb_jsonrpc_options *
>> +ovsdb_jsonrpc_options_clone(const struct ovsdb_jsonrpc_options *options)
>> +{
>> +    struct ovsdb_jsonrpc_options *clone;
>> +
>> +    clone = xmemdup(options, sizeof *options);
>> +    clone->role = nullable_xstrdup(options->role);
>> +
>> +    return clone;
>> +}
>> +
>>  struct json *
>>  ovsdb_jsonrpc_options_to_json(const struct ovsdb_jsonrpc_options *options)
>>  {
>> diff --git a/ovsdb/jsonrpc-server.h b/ovsdb/jsonrpc-server.h
>> index 9c49966c1..39366ad70 100644
>> --- a/ovsdb/jsonrpc-server.h
>> +++ b/ovsdb/jsonrpc-server.h
>> @@ -39,8 +39,10 @@ struct ovsdb_jsonrpc_options {
>>      int dscp;                   /* Dscp value for manager connections */
>>      char *role;                 /* Role, for role-based access controls */
>>  };
>> -struct ovsdb_jsonrpc_options *
>> -ovsdb_jsonrpc_default_options(const char *target);
>> +struct ovsdb_jsonrpc_options *ovsdb_jsonrpc_default_options(
>> +    const char *target);
>> +struct ovsdb_jsonrpc_options *ovsdb_jsonrpc_options_clone(
>> +    const struct ovsdb_jsonrpc_options *);
>>
>>  struct json *ovsdb_jsonrpc_options_to_json(
>>                                  const struct ovsdb_jsonrpc_options *)
>> diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c
>> index c294ebe67..7f65cadfe 100644
>> --- a/ovsdb/ovsdb-server.c
>> +++ b/ovsdb/ovsdb-server.c
>> @@ -101,7 +101,7 @@ static unixctl_cb_func ovsdb_server_get_sync_status;
>>  static unixctl_cb_func ovsdb_server_get_db_storage_status;
>>
>>  struct server_config {
>> -    struct sset *remotes;
>> +    struct shash *remotes;
> 
> When reading this patch I was looking for the reason for making the
> change from sset to shash. I am sure you do have a good reason, would
> it make sense to state it in the commit message?
> 

Hi, Frode.  Thanks for looking at the patches!

We need to track JSON-RPC options per remote.  The sset is a set
of strings, i.e. just a set of remotes themselves, but we need a
way to map options to these remotes, i.e. have a structure where
a string (remote) is stored together with an options structure
(struct ovsdb_jsonrpc_options).  This is achieved by using shash,
i.e. a map from a string to a structure.  Does that make sense?

I agree though that all the options will kind of be the same for
every remote at this point in the patch set, but it is a preparation
for having different options per remote once we have a config file
at the end of a patch set.

I could add some more words to the commit message in the next
version, sure.

Best regards, Ilya Maximets.
Frode Nordahl Dec. 14, 2023, 11:34 a.m. UTC | #3 
On Thu, Dec 14, 2023 at 11:41 AM Ilya Maximets <i.maximets@ovn.org> wrote:
>
> On 12/14/23 08:15, Frode Nordahl wrote:
> > On Thu, Dec 14, 2023 at 2:05 AM Ilya Maximets <i.maximets@ovn.org> wrote:
> >>
> >> Store KSON-RPC options for each remote separately, so it will be
> >
> > s/KSON/JSON
> >
> >> possible to have different configurations per remote in the future.
> >>
> >> These are also stored to and loaded from the temporary file that
> >> OVSDB is using to restore runtime configuration of the server
> >> restarted by the monitor process after a crash.
> >>
> >> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
> >> ---
> >>  ovsdb/jsonrpc-server.c |  11 ++++
> >>  ovsdb/jsonrpc-server.h |   6 +-
> >>  ovsdb/ovsdb-server.c   | 136 ++++++++++++++++++++++++++++-------------
> >>  3 files changed, 110 insertions(+), 43 deletions(-)
> >>
> >> diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c
> >> index 51b7db886..299afbb1d 100644
> >> --- a/ovsdb/jsonrpc-server.c
> >> +++ b/ovsdb/jsonrpc-server.c
> >> @@ -219,6 +219,17 @@ ovsdb_jsonrpc_default_options(const char *target)
> >>      return options;
> >>  }
> >>
> >> +struct ovsdb_jsonrpc_options *
> >> +ovsdb_jsonrpc_options_clone(const struct ovsdb_jsonrpc_options *options)
> >> +{
> >> +    struct ovsdb_jsonrpc_options *clone;
> >> +
> >> +    clone = xmemdup(options, sizeof *options);
> >> +    clone->role = nullable_xstrdup(options->role);
> >> +
> >> +    return clone;
> >> +}
> >> +
> >>  struct json *
> >>  ovsdb_jsonrpc_options_to_json(const struct ovsdb_jsonrpc_options *options)
> >>  {
> >> diff --git a/ovsdb/jsonrpc-server.h b/ovsdb/jsonrpc-server.h
> >> index 9c49966c1..39366ad70 100644
> >> --- a/ovsdb/jsonrpc-server.h
> >> +++ b/ovsdb/jsonrpc-server.h
> >> @@ -39,8 +39,10 @@ struct ovsdb_jsonrpc_options {
> >>      int dscp;                   /* Dscp value for manager connections */
> >>      char *role;                 /* Role, for role-based access controls */
> >>  };
> >> -struct ovsdb_jsonrpc_options *
> >> -ovsdb_jsonrpc_default_options(const char *target);
> >> +struct ovsdb_jsonrpc_options *ovsdb_jsonrpc_default_options(
> >> +    const char *target);
> >> +struct ovsdb_jsonrpc_options *ovsdb_jsonrpc_options_clone(
> >> +    const struct ovsdb_jsonrpc_options *);
> >>
> >>  struct json *ovsdb_jsonrpc_options_to_json(
> >>                                  const struct ovsdb_jsonrpc_options *)
> >> diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c
> >> index c294ebe67..7f65cadfe 100644
> >> --- a/ovsdb/ovsdb-server.c
> >> +++ b/ovsdb/ovsdb-server.c
> >> @@ -101,7 +101,7 @@ static unixctl_cb_func ovsdb_server_get_sync_status;
> >>  static unixctl_cb_func ovsdb_server_get_db_storage_status;
> >>
> >>  struct server_config {
> >> -    struct sset *remotes;
> >> +    struct shash *remotes;
> >
> > When reading this patch I was looking for the reason for making the
> > change from sset to shash. I am sure you do have a good reason, would
> > it make sense to state it in the commit message?
> >
>
> Hi, Frode.  Thanks for looking at the patches!
>
> We need to track JSON-RPC options per remote.  The sset is a set
> of strings, i.e. just a set of remotes themselves, but we need a
> way to map options to these remotes, i.e. have a structure where
> a string (remote) is stored together with an options structure
> (struct ovsdb_jsonrpc_options).  This is achieved by using shash,
> i.e. a map from a string to a structure.  Does that make sense?

It does, I guess I had a momentary lapse of reason thinking sets could
contain things. The other question I had was whether we were losing
any uniqueness checks by changing this but I see there is a check
before adding new ones, so I guess that's covered. Thanks for
entertaining my question.
diff mbox series

Patch

diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c
index 51b7db886..299afbb1d 100644
--- a/ovsdb/jsonrpc-server.c
+++ b/ovsdb/jsonrpc-server.c
@@ -219,6 +219,17 @@  ovsdb_jsonrpc_default_options(const char *target)
     return options;
 }
 
+struct ovsdb_jsonrpc_options *
+ovsdb_jsonrpc_options_clone(const struct ovsdb_jsonrpc_options *options)
+{
+    struct ovsdb_jsonrpc_options *clone;
+
+    clone = xmemdup(options, sizeof *options);
+    clone->role = nullable_xstrdup(options->role);
+
+    return clone;
+}
+
 struct json *
 ovsdb_jsonrpc_options_to_json(const struct ovsdb_jsonrpc_options *options)
 {
diff --git a/ovsdb/jsonrpc-server.h b/ovsdb/jsonrpc-server.h
index 9c49966c1..39366ad70 100644
--- a/ovsdb/jsonrpc-server.h
+++ b/ovsdb/jsonrpc-server.h
@@ -39,8 +39,10 @@  struct ovsdb_jsonrpc_options {
     int dscp;                   /* Dscp value for manager connections */
     char *role;                 /* Role, for role-based access controls */
 };
-struct ovsdb_jsonrpc_options *
-ovsdb_jsonrpc_default_options(const char *target);
+struct ovsdb_jsonrpc_options *ovsdb_jsonrpc_default_options(
+    const char *target);
+struct ovsdb_jsonrpc_options *ovsdb_jsonrpc_options_clone(
+    const struct ovsdb_jsonrpc_options *);
 
 struct json *ovsdb_jsonrpc_options_to_json(
                                 const struct ovsdb_jsonrpc_options *)
diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c
index c294ebe67..7f65cadfe 100644
--- a/ovsdb/ovsdb-server.c
+++ b/ovsdb/ovsdb-server.c
@@ -101,7 +101,7 @@  static unixctl_cb_func ovsdb_server_get_sync_status;
 static unixctl_cb_func ovsdb_server_get_db_storage_status;
 
 struct server_config {
-    struct sset *remotes;
+    struct shash *remotes;
     struct shash *all_dbs;
     FILE *config_tmpfile;
     char **sync_from;
@@ -130,29 +130,34 @@  static void remove_db(struct server_config *, struct shash_node *db, char *);
 static void close_db(struct server_config *, struct db *, char *);
 
 static void parse_options(int argc, char *argvp[],
-                          struct sset *db_filenames, struct sset *remotes,
+                          struct sset *db_filenames, struct shash *remotes,
                           char **unixctl_pathp, char **run_command,
                           char **sync_from, char **sync_exclude,
                           bool *is_backup);
 OVS_NO_RETURN static void usage(void);
 
+static struct ovsdb_jsonrpc_options *add_remote(
+                            struct shash *remotes, const char *target,
+                            const struct ovsdb_jsonrpc_options *);
+static void free_remotes(struct shash *remotes);
+
 static char *reconfigure_remotes(struct ovsdb_jsonrpc_server *,
                                  const struct shash *all_dbs,
-                                 struct sset *remotes);
+                                 struct shash *remotes);
 static char *reconfigure_ssl(const struct shash *all_dbs);
 static void report_error_if_changed(char *error, char **last_errorp);
 
 static void update_remote_status(const struct ovsdb_jsonrpc_server *jsonrpc,
-                                 const struct sset *remotes,
+                                 const struct shash *remotes,
                                  struct shash *all_dbs);
 static void update_server_status(struct shash *all_dbs);
 
-static void save_config__(FILE *config_file, const struct sset *remotes,
+static void save_config__(FILE *config_file, const struct shash *remotes,
                           const struct sset *db_filenames,
                           const char *sync_from, const char *sync_exclude,
                           bool is_backup);
 static void save_config(struct server_config *);
-static void load_config(FILE *config_file, struct sset *remotes,
+static void load_config(FILE *config_file, struct shash *remotes,
                         struct sset *db_filenames, char **sync_from,
                         char **sync_exclude, bool *is_backup);
 
@@ -184,7 +189,7 @@  log_and_free_error(struct ovsdb_error *error)
 static void
 main_loop(struct server_config *config,
           struct ovsdb_jsonrpc_server *jsonrpc, struct shash *all_dbs,
-          struct unixctl_server *unixctl, struct sset *remotes,
+          struct unixctl_server *unixctl, struct shash *remotes,
           struct process *run_process, bool *exiting, bool *is_backup)
 {
     char *remotes_error, *ssl_error;
@@ -318,7 +323,8 @@  main(int argc, char *argv[])
     char *run_command = NULL;
     struct unixctl_server *unixctl;
     struct ovsdb_jsonrpc_server *jsonrpc;
-    struct sset remotes, db_filenames;
+    struct sset db_filenames;
+    struct shash remotes;
     char *sync_from, *sync_exclude;
     bool is_backup;
     const char *db_filename;
@@ -514,7 +520,7 @@  main(int argc, char *argv[])
     }
     ovsdb_jsonrpc_server_destroy(jsonrpc);
     shash_destroy(&all_dbs);
-    sset_destroy(&remotes);
+    free_remotes(&remotes);
     sset_destroy(&db_filenames);
     free(sync_from);
     free(sync_exclude);
@@ -971,13 +977,16 @@  query_db_string(const struct shash *all_dbs, const char *name,
 }
 
 static struct ovsdb_jsonrpc_options *
-add_remote(struct shash *remotes, const char *target)
+add_remote(struct shash *remotes, const char *target,
+           const struct ovsdb_jsonrpc_options *options_)
 {
     struct ovsdb_jsonrpc_options *options;
 
     options = shash_find_data(remotes, target);
     if (!options) {
-        options = ovsdb_jsonrpc_default_options(target);
+        options = options_
+                  ? ovsdb_jsonrpc_options_clone(options_)
+                  : ovsdb_jsonrpc_default_options(target);
         shash_add(remotes, target, options);
     }
 
@@ -995,6 +1004,7 @@  free_remotes(struct shash *remotes)
             free(options->role);
         }
         shash_destroy_free_data(remotes);
+        shash_init(remotes);
     }
 }
 
@@ -1015,7 +1025,7 @@  add_manager_options(struct shash *remotes, const struct ovsdb_row *row)
         return;
     }
 
-    options = add_remote(remotes, target);
+    options = add_remote(remotes, target, NULL);
     if (ovsdb_util_read_integer_column(row, "max_backoff", &max_backoff)) {
         options->max_backoff = max_backoff;
     }
@@ -1075,7 +1085,7 @@  query_db_remotes(const char *name, const struct shash *all_dbs,
 
             datum = &row->fields[column->index];
             for (i = 0; i < datum->n; i++) {
-                add_remote(remotes, json_string(datum->keys[i].s));
+                add_remote(remotes, json_string(datum->keys[i].s), NULL);
             }
         }
     } else if (column->type.key.type == OVSDB_TYPE_UUID
@@ -1223,19 +1233,24 @@  commit_txn(struct ovsdb_txn *txn, const char *name)
 
 static void
 update_remote_status(const struct ovsdb_jsonrpc_server *jsonrpc,
-                     const struct sset *remotes,
+                     const struct shash *remotes,
                      struct shash *all_dbs)
 {
-    struct shash_node *node;
-    SHASH_FOR_EACH (node, all_dbs) {
-        struct db *db = node->data;
+    struct shash_node *db_node;
+
+    SHASH_FOR_EACH (db_node, all_dbs) {
+        struct db *db = db_node->data;
+
         if (!db->db || ovsdb_storage_is_clustered(db->db->storage)) {
             continue;
         }
 
         struct ovsdb_txn *txn = ovsdb_txn_create(db->db);
-        const char *remote;
-        SSET_FOR_EACH (remote, remotes) {
+        const struct shash_node *remote_node;
+
+        SHASH_FOR_EACH (remote_node, remotes) {
+            const char *remote = remote_node->name;
+
             update_remote_rows(all_dbs, db, remote, jsonrpc, txn);
         }
         commit_txn(txn, "remote status");
@@ -1342,19 +1357,22 @@  update_server_status(struct shash *all_dbs)
 /* Reconfigures ovsdb-server's remotes based on information in the database. */
 static char *
 reconfigure_remotes(struct ovsdb_jsonrpc_server *jsonrpc,
-                    const struct shash *all_dbs, struct sset *remotes)
+                    const struct shash *all_dbs, struct shash *remotes)
 {
     struct ds errors = DS_EMPTY_INITIALIZER;
     struct shash resolved_remotes;
-    const char *name;
+    struct shash_node *node;
 
     /* Configure remotes. */
     shash_init(&resolved_remotes);
-    SSET_FOR_EACH (name, remotes) {
+    SHASH_FOR_EACH (node, remotes) {
+        const struct ovsdb_jsonrpc_options *options = node->data;
+        const char *name = node->name;
+
         if (!strncmp(name, "db:", 3)) {
             query_db_remotes(name, all_dbs, &resolved_remotes, &errors);
         } else {
-            add_remote(&resolved_remotes, name);
+            add_remote(&resolved_remotes, name, options);
         }
     }
     ovsdb_jsonrpc_server_set_remotes(jsonrpc, &resolved_remotes);
@@ -1719,7 +1737,7 @@  ovsdb_server_add_remote(struct unixctl_conn *conn, int argc OVS_UNUSED,
               : parse_db_column(config->all_dbs, remote,
                                 &db, &table, &column));
     if (!retval) {
-        if (sset_add(config->remotes, remote)) {
+        if (add_remote(config->remotes, remote, NULL)) {
             save_config(config);
         }
         unixctl_command_reply(conn, NULL);
@@ -1736,11 +1754,12 @@  ovsdb_server_remove_remote(struct unixctl_conn *conn, int argc OVS_UNUSED,
                            const char *argv[], void *config_)
 {
     struct server_config *config = config_;
-    struct sset_node *node;
+    struct ovsdb_jsonrpc_options *options;
 
-    node = sset_find(config->remotes, argv[1]);
-    if (node) {
-        sset_delete(config->remotes, node);
+    options = shash_find_and_delete(config->remotes, argv[1]);
+    if (options) {
+        free(options->role);
+        free(options);
         save_config(config);
         unixctl_command_reply(conn, NULL);
     } else {
@@ -1753,15 +1772,15 @@  static void
 ovsdb_server_list_remotes(struct unixctl_conn *conn, int argc OVS_UNUSED,
                           const char *argv[] OVS_UNUSED, void *remotes_)
 {
-    struct sset *remotes = remotes_;
-    const char **list, **p;
+    const struct shash *remotes = remotes_;
+    const struct shash_node **list;
     struct ds s;
 
     ds_init(&s);
 
-    list = sset_sort(remotes);
-    for (p = list; *p; p++) {
-        ds_put_format(&s, "%s\n", *p);
+    list = shash_sort(remotes);
+    for (size_t i = 0; i < shash_count(remotes); i++) {
+        ds_put_format(&s, "%s\n", list[i]->name);
     }
     free(list);
 
@@ -1996,7 +2015,7 @@  ovsdb_server_get_db_storage_status(struct unixctl_conn *conn,
 
 static void
 parse_options(int argc, char *argv[],
-              struct sset *db_filenames, struct sset *remotes,
+              struct sset *db_filenames, struct shash *remotes,
               char **unixctl_pathp, char **run_command,
               char **sync_from, char **sync_exclude, bool *active)
 {
@@ -2047,7 +2066,7 @@  parse_options(int argc, char *argv[],
     *sync_from = NULL;
     *sync_exclude = NULL;
     sset_init(db_filenames);
-    sset_init(remotes);
+    shash_init(remotes);
     for (;;) {
         int c;
 
@@ -2058,7 +2077,7 @@  parse_options(int argc, char *argv[],
 
         switch (c) {
         case OPT_REMOTE:
-            sset_add(remotes, optarg);
+            add_remote(remotes, optarg, NULL);
             break;
 
         case OPT_UNIXCTL:
@@ -2197,10 +2216,24 @@  sset_to_json(const struct sset *sset)
     return array;
 }
 
+static struct json *
+remotes_to_json(const struct shash *remotes)
+{
+    const struct shash_node *node;
+    struct json *json;
+
+    json = json_object_create();
+    SHASH_FOR_EACH (node, remotes) {
+        json_object_put(json, node->name,
+                        ovsdb_jsonrpc_options_to_json(node->data));
+    }
+    return json;
+}
+
 /* Truncates and replaces the contents of 'config_file' by a representation of
  * 'remotes' and 'db_filenames'. */
 static void
-save_config__(FILE *config_file, const struct sset *remotes,
+save_config__(FILE *config_file, const struct shash *remotes,
               const struct sset *db_filenames, const char *sync_from,
               const char *sync_exclude, bool is_backup)
 {
@@ -2213,7 +2246,7 @@  save_config__(FILE *config_file, const struct sset *remotes,
     }
 
     obj = json_object_create();
-    json_object_put(obj, "remotes", sset_to_json(remotes));
+    json_object_put(obj, "remotes", remotes_to_json(remotes));
     json_object_put(obj, "db_filenames", sset_to_json(db_filenames));
     if (sync_from) {
         json_object_put(obj, "sync_from", json_string_create(sync_from));
@@ -2273,11 +2306,32 @@  sset_from_json(struct sset *sset, const struct json *array)
     }
 }
 
+static void
+remotes_from_json(struct shash *remotes, const struct json *json)
+{
+    struct ovsdb_jsonrpc_options *options;
+    const struct shash_node *node;
+    const struct shash *object;
+
+    free_remotes(remotes);
+
+    ovs_assert(json);
+    ovs_assert(json->type == JSON_OBJECT);
+
+    object = json_object(json);
+    SHASH_FOR_EACH (node, object) {
+        options = ovsdb_jsonrpc_default_options(node->name);
+        ovsdb_jsonrpc_options_update_from_json(options, node->data);
+        shash_add(remotes, node->name, options);
+    }
+}
+
 /* Clears and replaces 'remotes' and 'dbnames' 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 sset *db_filenames,
-            char **sync_from, char **sync_exclude, bool *is_backup)
+load_config(FILE *config_file, struct shash *remotes,
+            struct sset *db_filenames, char **sync_from,
+            char **sync_exclude, bool *is_backup)
 {
     struct json *json;
 
@@ -2290,7 +2344,7 @@  load_config(FILE *config_file, struct sset *remotes, struct sset *db_filenames,
     }
     ovs_assert(json->type == JSON_OBJECT);
 
-    sset_from_json(remotes, shash_find_data(json_object(json), "remotes"));
+    remotes_from_json(remotes, shash_find_data(json_object(json), "remotes"));
     sset_from_json(db_filenames,
                    shash_find_data(json_object(json), "db_filenames"));