> On Dec 31, 2017, at 9:16 PM, Ben Pfaff <[email protected]> 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 <[email protected]>
--Justin
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev