| Message ID | 20200109204830.4087729-1-blp@ovn.org |
|---|---|
| State | Accepted |
| Commit | a529e3cd1f410747778c3a8be1370a618ef0c861 |
| Headers | show |
| Series | [ovs-dev,v2] ovsdb-server: Allow OVSDB clients to specify the UUID for inserted rows. | expand |
Bleep bloop. Greetings Ben Pfaff, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. checkpatch: WARNING: Line is 82 characters long (recommended limit is 79) #116 FILE: ovsdb/transaction.c:1: /* Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2017, 2019 Nicira, Inc. Lines checked: 240, Warnings: 1, Errors: 0 Please check this out. If you feel there has been an error, please email aconole@redhat.com Thanks, 0-day Robot
On Thu, Jan 9, 2020 at 12:48 PM Ben Pfaff <blp@ovn.org> wrote: > > Requested-by: Leonid Ryzhyk <lryzhyk@vmware.com> > Signed-off-by: Ben Pfaff <blp@ovn.org> > --- > v1->v2: Improve test as suggested by Flavio Fernandes. > > Documentation/ref/ovsdb-server.7.rst | 9 +++++++++ > NEWS | 1 + > ovsdb/execution.c | 26 ++++++++++++++++++++++---- > ovsdb/transaction.c | 22 +++++++++++++++++++++- > ovsdb/transaction.h | 5 ++++- > tests/ovsdb-execution.at | 24 ++++++++++++++++++++++++ > tests/uuidfilt.py | 18 ++++++++++++++++-- > 7 files changed, 97 insertions(+), 8 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 965facaf852d..d8f82cd18af0 100644 > --- a/NEWS > +++ b/NEWS > @@ -34,6 +34,7 @@ Post-v2.12.0 > interval is increased to 60 seconds for the connection to the > replication server. This value is configurable with the unixctl > command - ovsdb-server/set-active-ovsdb-server-probe-interval. > + * ovsdb-server: New OVSDB extension to allow clients to specify row UUIDs. > > v2.12.0 - 03 Sep 2019 > --------------------- > diff --git a/ovsdb/execution.c b/ovsdb/execution.c > index f2cf3d72d45f..e45f3d6796a7 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..e72bf0606978 100644 > --- a/tests/ovsdb-execution.at > +++ b/tests/ovsdb-execution.at > @@ -249,6 +249,30 @@ 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], > +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": []}]]], > +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], > [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.23.0 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev Acked-by: Han Zhou <hzhou@ovn.org> (I also tested with ovsdb-client which works as expected. Maybe we can update the xxxctl tools to support specifying uuids for the "create" command as well)
Hi Ben,
It will be appreciated，if you give more explanation for the resaon to summit this patch. In my opinion, it makes the "UUID" column be almost the same with "name" column, if we need "name" column in the future?
At 2020-01-10 11:39:22, "Han Zhou" <hzhou@ovn.org> wrote:
>On Thu, Jan 9, 2020 at 12:48 PM Ben Pfaff <blp@ovn.org> wrote:
>>
>> Requested-by: Leonid Ryzhyk <lryzhyk@vmware.com>
>> Signed-off-by: Ben Pfaff <blp@ovn.org>
>> ---
>> v1->v2: Improve test as suggested by Flavio Fernandes.
>>
>> Documentation/ref/ovsdb-server.7.rst | 9 +++++++++
>> NEWS | 1 +
>> ovsdb/execution.c | 26 ++++++++++++++++++++++----
>> ovsdb/transaction.c | 22 +++++++++++++++++++++-
>> ovsdb/transaction.h | 5 ++++-
>> tests/ovsdb-execution.at | 24 ++++++++++++++++++++++++
>> tests/uuidfilt.py | 18 ++++++++++++++++--
>> 7 files changed, 97 insertions(+), 8 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 965facaf852d..d8f82cd18af0 100644
>> --- a/NEWS
>> +++ b/NEWS
>> @@ -34,6 +34,7 @@ Post-v2.12.0
>> interval is increased to 60 seconds for the connection to the
>> replication server. This value is configurable with the unixctl
>> command - ovsdb-server/set-active-ovsdb-server-probe-interval.
>> + * ovsdb-server: New OVSDB extension to allow clients to specify row
>UUIDs.
>>
>> v2.12.0 - 03 Sep 2019
>> ---------------------
>> diff --git a/ovsdb/execution.c b/ovsdb/execution.c
>> index f2cf3d72d45f..e45f3d6796a7 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..e72bf0606978 100644
>> --- a/tests/ovsdb-execution.at
>> +++ b/tests/ovsdb-execution.at
>> @@ -249,6 +249,30 @@ 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],
>> +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": []}]]],
>> +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],
>> [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.23.0
>>
>> _______________________________________________
>> dev mailing list
>> dev@openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>Acked-by: Han Zhou <hzhou@ovn.org>
>
>(I also tested with ovsdb-client which works as expected. Maybe we can
>update the xxxctl tools to support specifying uuids for the "create"
>command as well)
>_______________________________________________
>dev mailing list
>dev@openvswitch.org
>https://mail.openvswitch.org/mailman/listinfo/ovs-dev
I don't understand that claim. Which name column do you think we can delete, from which schema? On Fri, Jan 10, 2020 at 05:14:44PM +0800, taoyunupt wrote: > Hi Ben, > It will be appreciated，if you give more explanation for the resaon to summit this patch. In my opinion, it makes the "UUID" column be almost the same with "name" column, if we need "name" column in the future? > > > > > > At 2020-01-10 11:39:22, "Han Zhou" <hzhou@ovn.org> wrote: > >On Thu, Jan 9, 2020 at 12:48 PM Ben Pfaff <blp@ovn.org> wrote: > >> > >> Requested-by: Leonid Ryzhyk <lryzhyk@vmware.com> > >> Signed-off-by: Ben Pfaff <blp@ovn.org> > >> --- > >> v1->v2: Improve test as suggested by Flavio Fernandes. > >> > >> Documentation/ref/ovsdb-server.7.rst | 9 +++++++++ > >> NEWS | 1 + > >> ovsdb/execution.c | 26 ++++++++++++++++++++++---- > >> ovsdb/transaction.c | 22 +++++++++++++++++++++- > >> ovsdb/transaction.h | 5 ++++- > >> tests/ovsdb-execution.at | 24 ++++++++++++++++++++++++ > >> tests/uuidfilt.py | 18 ++++++++++++++++-- > >> 7 files changed, 97 insertions(+), 8 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 965facaf852d..d8f82cd18af0 100644 > >> --- a/NEWS > >> +++ b/NEWS > >> @@ -34,6 +34,7 @@ Post-v2.12.0 > >> interval is increased to 60 seconds for the connection to the > >> replication server. This value is configurable with the unixctl > >> command - ovsdb-server/set-active-ovsdb-server-probe-interval. > >> + * ovsdb-server: New OVSDB extension to allow clients to specify row > >UUIDs. > >> > >> v2.12.0 - 03 Sep 2019 > >> --------------------- > >> diff --git a/ovsdb/execution.c b/ovsdb/execution.c > >> index f2cf3d72d45f..e45f3d6796a7 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..e72bf0606978 100644 > >> --- a/tests/ovsdb-execution.at > >> +++ b/tests/ovsdb-execution.at > >> @@ -249,6 +249,30 @@ 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], > >> +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": []}]]], > >> +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], > >> [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.23.0 > >> > >> _______________________________________________ > >> dev mailing list > >> dev@openvswitch.org > >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > >Acked-by: Han Zhou <hzhou@ovn.org> > > > >(I also tested with ovsdb-client which works as expected. Maybe we can > >update the xxxctl tools to support specifying uuids for the "create" > >command as well) > >_______________________________________________ > >dev mailing list > >dev@openvswitch.org > >https://mail.openvswitch.org/mailman/listinfo/ovs-dev
On Thu, Jan 09, 2020 at 07:39:22PM -0800, Han Zhou wrote: > On Thu, Jan 9, 2020 at 12:48 PM Ben Pfaff <blp@ovn.org> wrote: > > > > Requested-by: Leonid Ryzhyk <lryzhyk@vmware.com> > > Signed-off-by: Ben Pfaff <blp@ovn.org> > > --- > > v1->v2: Improve test as suggested by Flavio Fernandes. > > Acked-by: Han Zhou <hzhou@ovn.org> Applied to master, thanks! > (I also tested with ovsdb-client which works as expected. Maybe we can > update the xxxctl tools to support specifying uuids for the "create" > command as well) Good idea.
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 965facaf852d..d8f82cd18af0 100644 --- a/NEWS +++ b/NEWS @@ -34,6 +34,7 @@ Post-v2.12.0 interval is increased to 60 seconds for the connection to the replication server. This value is configurable with the unixctl command - ovsdb-server/set-active-ovsdb-server-probe-interval. + * ovsdb-server: New OVSDB extension to allow clients to specify row UUIDs. v2.12.0 - 03 Sep 2019 --------------------- diff --git a/ovsdb/execution.c b/ovsdb/execution.c index f2cf3d72d45f..e45f3d6796a7 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..e72bf0606978 100644 --- a/tests/ovsdb-execution.at +++ b/tests/ovsdb-execution.at @@ -249,6 +249,30 @@ 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], +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": []}]]], +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], [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> --- v1->v2: Improve test as suggested by Flavio Fernandes. Documentation/ref/ovsdb-server.7.rst | 9 +++++++++ NEWS | 1 + ovsdb/execution.c | 26 ++++++++++++++++++++++---- ovsdb/transaction.c | 22 +++++++++++++++++++++- ovsdb/transaction.h | 5 ++++- tests/ovsdb-execution.at | 24 ++++++++++++++++++++++++ tests/uuidfilt.py | 18 ++++++++++++++++-- 7 files changed, 97 insertions(+), 8 deletions(-)