> On Dec 31, 2017, at 9:16 PM, Ben Pfaff <b...@ovn.org> wrote:
> 
> --- a/NEWS
> +++ b/NEWS
> @@ -5,6 +5,8 @@ Post-v2.8.0
>      * New high-level documentation in ovsdb(7).
>      * New file format documentation for developers in ovsdb(5).
>      * Protocol documentation moved from ovsdb-server(1) to ovsdb-server(7).
> +     * ovsdb-server now supports online schema conversion via
> +       "ovsdb-client convert".

Do you think it's worth mentioning the "needs-conversion" and "convert" 
commands?

> diff --git a/ovsdb/file.c b/ovsdb/file.c
> index 4aafb3be8ab4..dadb988d3088 100644
> --- a/ovsdb/file.c
> +++ b/ovsdb/file.c
> @@ -566,22 +566,31 @@ ovsdb_file_txn_annotate(struct json *json, const char 
> *comment)
>     return json;
> }
> 
> -struct ovsdb_error *
> -ovsdb_file_commit(struct ovsdb_file *file,
> -                  const struct ovsdb_txn *txn, bool durable)
> +/* Returns 'txn' transformed into the JSON format that is used in OVSDB 
> files.
> + * (But the caller must use ovsdb_file_txn_annotate() to add the _comment the
> + * _date members.)  If 'txn' doesn't actually change anything, returns NULL 
> */

I think that should be "and _date members".

> diff --git a/ovsdb/monitor.c b/ovsdb/monitor.c
> index 3e58c3fbd274..97706932614c 100644
> --- a/ovsdb/monitor.c
> +++ b/ovsdb/monitor.c
> ...
> +void
> +ovsdb_monitor_prereplace_db(struct ovsdb *db)
> +{
> +    struct ovsdb_monitor *m, *next_m;
> +
> +    LIST_FOR_EACH_SAFE (m, next_m, list_node, &db->monitors) {
> +        struct jsonrpc_monitor_node *jm, *next_jm;
> +
> +        /* Delete all front end monitors. Removing the last front
> +         * end monitor will also destroy the corresponding 'ovsdb_monitor'.
> +         * ovsdb monitor will also be destroied.  */

I found the last sentence confusing in relation with the sentence before it.  
If that sentence should stay, it should be "destroyed".

> diff --git a/ovsdb/ovsdb-client.c b/ovsdb/ovsdb-client.c
> index 600c5070db78..a7cab600c98b 100644
> --- a/ovsdb/ovsdb-client.c
> +++ b/ovsdb/ovsdb-client.c
> ...
> @@ -303,6 +308,8 @@ usage(void)
>            "    DATABASE on SERVER.\n"
>            "    COLUMNs may include !initial, !insert, !delete, !modify\n"
>            "    to avoid seeing the specified kinds of changes.\n"
> +           "\n  convert [SERVER] SCHEMA\n"
> +           "    convert database on SERVER named in SCHEMA to SCHEMA.\n"

The man page discusses "need-conversion", but it's not mentioned in the usage.  
Is this intentional?

> +static void
> +do_convert(struct jsonrpc *rpc, const char *database OVS_UNUSED,
> +           int argc OVS_UNUSED, char *argv[])
> +{
> +    struct ovsdb_schema *new_schema;
> +    check_ovsdb_error(ovsdb_schema_from_file(argv[0], &new_schema));
> +
> +    struct jsonrpc_msg *request, *reply;
> +    request = jsonrpc_create_request(
> +        "convert",
> +        json_array_create_2(json_string_create(new_schema->name),
> +                            ovsdb_schema_to_json(new_schema)), NULL);

I"m probably misunderstanding something, but I thought the only parameter to 
"convert" was "<database-schema>" based on the documentation in 
"ovsdb-server.7.rst", but this seems to be an array that consists of the name 
and the schema.  My reading of the RFC is that "<ovsdb-schema>" only contains 
the schema.

> diff --git a/ovsdb/trigger.h b/ovsdb/trigger.h
> index 90246a4a42bd..d9df97f31222 100644
> --- a/ovsdb/trigger.h
> +++ b/ovsdb/trigger.h
> @@ -25,8 +25,8 @@ struct ovsdb_trigger {
>     struct ovsdb *db;           /* Database on which trigger acts. */
>     struct ovs_list node;       /* !result: in db->triggers;
>                                  * result: in session->completions. */
> -    struct json *request;       /* Database request. */
> -    struct json *result;        /* Result (null if none yet). */
> +    struct jsonrpc_msg *request; /* Database request. */
> +    struct jsonrpc_msg *reply;   /* Result (null if none yet).. */

There appears to be an extra period at the end of this comment.

Acked-by: Justin Pettit <jpet...@ovn.org>

--Justin


_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to