Message ID | 20190826210046.23185-1-blp@ovn.org |
---|---|
State | RFC |
Headers | show |
Series | [ovs-dev,RFC] ovsdb-server: Allow OVSDB clients to specify the UUID for inserted rows. | expand |
> On Aug 26, 2019, at 5:00 PM, Ben Pfaff <blp@ovn.org> wrote: > > Requested-by: Leonid Ryzhyk <lryzhyk@vmware.com> > Signed-off-by: Ben Pfaff <blp@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@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
On Thu, Jan 09, 2020 at 02:21:18PM -0500, Flavio Fernandes wrote: > 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? That is a good idea. We can do it within the same test. Here is an incremental. I will post a v2. diff --git a/tests/ovsdb-execution.at b/tests/ovsdb-execution.at index 10472d76fcbe..e72bf0606978 100644 --- a/tests/ovsdb-execution.at +++ b/tests/ovsdb-execution.at @@ -251,17 +251,26 @@ OVSDB_CHECK_EXECUTION([insert row, query table], OVSDB_CHECK_EXECUTION([insert row with uuid, query table], [ordinal_schema], +dnl Insert initial row. [[[["ordinals", {"op": "insert", "uuid": "ffffffff-971b-4cba-bf42-520515973b7e", "table": "ordinals", "row": {"number": 0, "name": "zero"}}]]], +dnl Query row back. [[["ordinals", {"op": "select", "table": "ordinals", - "where": []}]]]], + "where": []}]]], +dnl Attempt to insert second row with same UUID (fails). + [[["ordinals", + {"op": "insert", + "uuid": "ffffffff-971b-4cba-bf42-520515973b7e", + "table": "ordinals", + "row": {"number": 0, "name": "zero"}}]]]], [[[{"uuid":["uuid","ffffffff-971b-4cba-bf42-520515973b7e"]}] [{"rows":[{"_uuid":["uuid","ffffffff-971b-4cba-bf42-520515973b7e"],"_version":["uuid","<0>"],"name":"zero","number":0}]}] +[{"details":"This UUID would duplicate a UUID already present within the table or deleted within the same transaction.","error":"duplicate uuid","syntax":"\"ffffffff-971b-4cba-bf42-520515973b7e\""}] ]]) OVSDB_CHECK_EXECUTION([insert rows, query by value],
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}]}] +]]) + 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:
Requested-by: Leonid Ryzhyk <lryzhyk@vmware.com> Signed-off-by: Ben Pfaff <blp@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(-)