[ovs-dev] [PATCH] [RFC] ovsdb-server: Allow OVSDB clients to specify the UUID for inserted rows.
Flavio Fernandes
flavio at flaviof.com
Thu Jan 9 19:21:18 UTC 2020
> On Aug 26, 2019, at 5:00 PM, Ben Pfaff <blp at ovn.org> wrote:
>
> Requested-by: Leonid Ryzhyk <lryzhyk at vmware.com>
> Signed-off-by: Ben Pfaff <blp at ovn.org>
> ---
> Documentation/ref/ovsdb-server.7.rst | 9 +++++++++
> NEWS | 3 ++-
> ovsdb/execution.c | 26 ++++++++++++++++++++++----
> ovsdb/transaction.c | 22 +++++++++++++++++++++-
> ovsdb/transaction.h | 5 ++++-
> tests/ovsdb-execution.at | 15 +++++++++++++++
> tests/uuidfilt.py | 18 ++++++++++++++++--
> 7 files changed, 89 insertions(+), 9 deletions(-)
>
> diff --git a/Documentation/ref/ovsdb-server.7.rst b/Documentation/ref/ovsdb-server.7.rst
> index bc533611cb4a..967761bdfb84 100644
> --- a/Documentation/ref/ovsdb-server.7.rst
> +++ b/Documentation/ref/ovsdb-server.7.rst
> @@ -546,6 +546,15 @@ condition can be either a 3-element JSON array as described in the RFC or a
> boolean value. In case of an empty array an implicit true boolean value will be
> considered.
>
> +5.2.1 Insert
> +------------
> +
> +As an extension, Open vSwitch 2.13 and later allow an optional ``uuid`` member
> +to specify the UUID for the new row. The specified UUID must be unique within
> +the table when it is inserted and not the UUID of a row previously deleted
> +within the transaction. If the UUID violates these rules, then the operation
> +fails with a ``duplicate uuid`` error.
> +
> 5.2.6 Wait, 5.2.7 Commit, 5.2.9 Comment
> ---------------------------------------
>
> diff --git a/NEWS b/NEWS
> index c5caa13d6374..7cda1e91a138 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -1,6 +1,7 @@
> Post-v2.12.0
> ---------------------
> -
> + - ovsdb-server: New OVSDB extension to allow clients to specify row UUIDs.
> +
>
> v2.12.0 - xx xxx xxxx
> ---------------------
> diff --git a/ovsdb/execution.c b/ovsdb/execution.c
> index c55a0b771032..a8083da126dd 100644
> --- a/ovsdb/execution.c
> +++ b/ovsdb/execution.c
> @@ -1,4 +1,4 @@
> -/* Copyright (c) 2009, 2010, 2011, 2012, 2013, 2017 Nicira, Inc.
> +/* Copyright (c) 2009, 2010, 2011, 2012, 2013, 2017, 2019 Nicira, Inc.
> *
> * Licensed under the Apache License, Version 2.0 (the "License");
> * you may not use this file except in compliance with the License.
> @@ -329,11 +329,12 @@ ovsdb_execute_insert(struct ovsdb_execution *x, struct ovsdb_parser *parser,
> {
> struct ovsdb_table *table;
> struct ovsdb_row *row = NULL;
> - const struct json *uuid_name, *row_json;
> + const struct json *uuid_json, *uuid_name, *row_json;
> struct ovsdb_error *error;
> struct uuid row_uuid;
>
> table = parse_table(x, parser, "table");
> + uuid_json = ovsdb_parser_member(parser, "uuid", OP_STRING | OP_OPTIONAL);
> uuid_name = ovsdb_parser_member(parser, "uuid-name", OP_ID | OP_OPTIONAL);
> row_json = ovsdb_parser_member(parser, "row", OP_OBJECT);
> error = ovsdb_parser_get_error(parser);
> @@ -341,6 +342,19 @@ ovsdb_execute_insert(struct ovsdb_execution *x, struct ovsdb_parser *parser,
> return error;
> }
>
> + if (uuid_json) {
> + if (!uuid_from_string(&row_uuid, json_string(uuid_json))) {
> + return ovsdb_syntax_error(uuid_json, NULL, "bad uuid");
> + }
> +
> + if (!ovsdb_txn_may_create_row(table, &row_uuid)) {
> + return ovsdb_syntax_error(uuid_json, "duplicate uuid",
> + "This UUID would duplicate a UUID "
> + "already present within the table or "
> + "deleted within the same transaction.");
> + }
> + }
> +
> if (uuid_name) {
> struct ovsdb_symbol *symbol;
>
> @@ -350,9 +364,13 @@ ovsdb_execute_insert(struct ovsdb_execution *x, struct ovsdb_parser *parser,
> "This \"uuid-name\" appeared on an "
> "earlier \"insert\" operation.");
> }
> - row_uuid = symbol->uuid;
> + if (uuid_json) {
> + symbol->uuid = row_uuid;
> + } else {
> + row_uuid = symbol->uuid;
> + }
> symbol->created = true;
> - } else {
> + } else if (!uuid_json) {
> uuid_generate(&row_uuid);
> }
>
> diff --git a/ovsdb/transaction.c b/ovsdb/transaction.c
> index 866fabe8df57..369436bffbf5 100644
> --- a/ovsdb/transaction.c
> +++ b/ovsdb/transaction.c
> @@ -1,4 +1,4 @@
> -/* Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2017 Nicira, Inc.
> +/* Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2017, 2019 Nicira, Inc.
> *
> * Licensed under the Apache License, Version 2.0 (the "License");
> * you may not use this file except in compliance with the License.
> @@ -1287,6 +1287,26 @@ ovsdb_txn_row_delete(struct ovsdb_txn *txn, const struct ovsdb_row *row_)
> }
> }
>
> +/* Returns true if 'row_uuid' may be used as the UUID for a newly created row
> + * in 'table' (that is, that it is unique within 'table'), false otherwise. */
> +bool
> +ovsdb_txn_may_create_row(const struct ovsdb_table *table,
> + const struct uuid *row_uuid)
> +{
> + /* If a row 'row_uuid' currently exists, disallow creating a duplicate. */
> + if (ovsdb_table_get_row(table, row_uuid)) {
> + return false;
> + }
> +
> + /* If a row 'row_uuid' previously existed in this transaction, disallow
> + * creating a new row with the same UUID. */
> + if (find_txn_row(table, row_uuid)) {
> + return false;
> + }
> +
> + return true;
> +}
> +
> void
> ovsdb_txn_add_comment(struct ovsdb_txn *txn, const char *s)
> {
> diff --git a/ovsdb/transaction.h b/ovsdb/transaction.h
> index ea6b53d3c2a3..6b5bb7f24b29 100644
> --- a/ovsdb/transaction.h
> +++ b/ovsdb/transaction.h
> @@ -1,4 +1,4 @@
> -/* Copyright (c) 2009, 2010, 2017 Nicira, Inc.
> +/* Copyright (c) 2009, 2010, 2017, 2019 Nicira, Inc.
> *
> * Licensed under the Apache License, Version 2.0 (the "License");
> * you may not use this file except in compliance with the License.
> @@ -53,6 +53,9 @@ struct ovsdb_row *ovsdb_txn_row_modify(struct ovsdb_txn *,
> void ovsdb_txn_row_insert(struct ovsdb_txn *, struct ovsdb_row *);
> void ovsdb_txn_row_delete(struct ovsdb_txn *, const struct ovsdb_row *);
>
> +bool ovsdb_txn_may_create_row(const struct ovsdb_table *,
> + const struct uuid *row_uuid);
> +
> typedef bool ovsdb_txn_row_cb_func(const struct ovsdb_row *old,
> const struct ovsdb_row *new,
> const unsigned long int *changed,
> diff --git a/tests/ovsdb-execution.at b/tests/ovsdb-execution.at
> index 3129e73f4a09..10472d76fcbe 100644
> --- a/tests/ovsdb-execution.at
> +++ b/tests/ovsdb-execution.at
> @@ -249,6 +249,21 @@ OVSDB_CHECK_EXECUTION([insert row, query table],
> [{"rows":[{"_uuid":["uuid","<0>"],"_version":["uuid","<1>"],"name":"zero","number":0}]}]
> ]])
>
> +OVSDB_CHECK_EXECUTION([insert row with uuid, query table],
> + [ordinal_schema],
> + [[[["ordinals",
> + {"op": "insert",
> + "uuid": "ffffffff-971b-4cba-bf42-520515973b7e",
> + "table": "ordinals",
> + "row": {"number": 0, "name": "zero"}}]]],
> + [[["ordinals",
> + {"op": "select",
> + "table": "ordinals",
> + "where": []}]]]],
> + [[[{"uuid":["uuid","ffffffff-971b-4cba-bf42-520515973b7e"]}]
> +[{"rows":[{"_uuid":["uuid","ffffffff-971b-4cba-bf42-520515973b7e"],"_version":["uuid","<0>"],"name":"zero","number":0}]}]
> +]])
> +
This looks good to me, but I wonder if it would be a good idea to also add a test where we
exercise the failure expected when trying to insert with a duplicate uuid. What do you think?
-- flaviof
> OVSDB_CHECK_EXECUTION([insert rows, query by value],
> [ordinal_schema],
> [[[["ordinals",
> diff --git a/tests/uuidfilt.py b/tests/uuidfilt.py
> index ea7281296e22..bc49aa480e9e 100755
> --- a/tests/uuidfilt.py
> +++ b/tests/uuidfilt.py
> @@ -18,7 +18,8 @@ def sort_set(match):
>
>
> u = '[0-9a-fA-F]'
> -uuid_re = re.compile(r'%s{8}-%s{4}-%s{4}-%s{4}-%s{12}' % ((u,) * 5))
> +uuid_re = re.compile(r'%s{8}(?<!ffffffff)-%s{4}-%s{4}-%s{4}-%s{12}'
> + % ((u,) * 5))
> set_re = re.compile(r'(\["set",\[(,?\["uuid","<\d+>"\])+\]\])')
>
>
> @@ -43,7 +44,20 @@ def filter_uuids(src, dst):
>
>
> if __name__ == '__main__':
> - if len(sys.argv) > 1:
> + if '--help' in sys.argv:
> + print("""\
> +uuidfilt, for filtering UUIDs into numbered markers
> +usage: %s [INPUT..] > OUTPUT
> + or %s < INPUT > OUTPUT
> +
> +Reads each INPUT, locates UUIDs in the standard textual form, and
> +converts them into numbered markers <0>, <1>, ..., <n>, where <0>
> +stands in for each instance of the first unique UUID found, <1> for
> +each instance of the second, and so on.
> +
> +UUIDs that begin with ffffffff are not converted to markers.
> +""")
> + elif len(sys.argv) > 1:
> for src in sys.argv[1:]:
> filter_uuids(open(src), sys.stdout)
> else:
> --
> 2.20.1
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
More information about the dev
mailing list