| Message ID | 20221128035613.36207-1-numans@ovn.org |
|---|---|
| State | Accepted |
| Commit | 55b9507e6824b935ffa0205fc7c7bebfe4e54279 |
| Headers | show |
| Series | [ovs-dev,v8] ovsdb-idl: Add the support to specify the uuid for row insert. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | warning | apply and check: warning |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/intel-ovs-compilation | success | test: success |
Bleep bloop. Greetings Numan Siddique, 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 108 characters long (recommended limit is 79)
#126 FILE: lib/db-ctl-base.man:206:
.IP "[\fB\-\-id=(@\fIname\fR | \fIuuid\fR] \fBcreate\fR \fItable column\fR[\fB:\fIkey\fR]\fB=\fIvalue\fR..."
WARNING: Line is 174 characters long (recommended limit is 79)
#149 FILE: lib/db-ctl-base.xml:313:
<dt>[<code>--id=(@</code><var>name</var>|<var>uuid</var>)] <code>create</code> <var>table column</var>[<code>:</code><var>key</var>]<code>=</code><var>value</var>...</dt>
Lines checked: 628, Warnings: 2, Errors: 0
Please check this out. If you feel there has been an error, please email aconole@redhat.com
Thanks,
0-day Robot
On Sun, Nov 27, 2022 at 9:56 PM <numans@ovn.org> wrote: > From: Numan Siddique <numans@ovn.org> > > ovsdb-server allows the OVSDB clients to specify the uuid for > the row inserts [1]. Both the C IDL client library and Python > IDL are missing this feature. This patch adds this support. > > In C IDL, for each schema table, a new function is generated - > <schema_table>insert_persistent_uuid(txn, uuid) which can > be used the clients to persist the uuid. > > ovs-vsctl and other derivatives of ctl now supports the same > in the generic 'create' command with the option "--id=<UUID>". > > In Python IDL, the uuid to persist can be specified in > the Transaction.insert() function. > > [1] - a529e3cd1f("ovsdb-server: Allow OVSDB clients to specify the UUID > for inserted rows.:) > > Signed-off-by: Numan Siddique <numans@ovn.org> > Acked-by: Adrian Moreno <amorenoz@redhat.com> > Acked-by: Han Zhou <hzhou@ovn.org> > CC: twilson@redhat.com > CC: i.maximets@ovn.org > > --- > v7 -> v8 > --- > * Addressed the review comments from Ilya. > - Added the same support in Python IDL. > - Reworked the test cases to make them > generic - for both C and Python. > > v6 -> v7 > --- > * Rebased to resolve conflicts. > > v5 -> v6 > --- > * Rebased to resolve conflicts. > > v4 -> v5 > --- > * Addressed review comments from Ilya. > - Added NEWS item entry. > > v3 -> v4 > --- > * Added an entry in python/TODO.rst. > > v2 -> v3 > ---- > * Addressed review comments from Han > - Added test case for --id ctl option > > v1 -> v2 > ----- > * Addressed review comments from Adrian Moreno > * Added the support in generic 'create' command to specify the uuid in > --id option. > > > NEWS | 3 ++ > lib/db-ctl-base.c | 38 ++++++++++++------ > lib/db-ctl-base.man | 5 ++- > lib/db-ctl-base.xml | 6 ++- > lib/ovsdb-idl-provider.h | 1 + > lib/ovsdb-idl.c | 85 +++++++++++++++++++++++++++++----------- > lib/ovsdb-idl.h | 3 ++ > ovsdb/ovsdb-idlc.in | 15 +++++++ > python/ovs/db/idl.py | 26 ++++++++---- > tests/ovs-vsctl.at | 25 ++++++++++++ > tests/ovsdb-idl.at | 58 +++++++++++++++++++++++++++ > tests/test-ovsdb.c | 28 +++++++++++-- > tests/test-ovsdb.py | 20 +++++++++- > 13 files changed, 263 insertions(+), 50 deletions(-) > > diff --git a/NEWS b/NEWS > index ff77ee404..b827a64ab 100644 > --- a/NEWS > +++ b/NEWS > @@ -24,6 +24,9 @@ Post-v3.0.0 > If a user wishes to benefit from these fixes it is recommended to > use > DPDK 21.11.2. > > + - OVSDB-IDL: > + * Add the support to specify the persistent uuid for row insert in > both > + C and Python IDLs. > > v3.0.0 - 15 Aug 2022 > -------------------- > diff --git a/lib/db-ctl-base.c b/lib/db-ctl-base.c > index bc85e9921..856832a04 100644 > --- a/lib/db-ctl-base.c > +++ b/lib/db-ctl-base.c > @@ -1731,29 +1731,43 @@ cmd_create(struct ctl_context *ctx) > const struct ovsdb_idl_table_class *table; > const struct ovsdb_idl_row *row; > const struct uuid *uuid = NULL; > + bool persist_uuid = false; > + struct uuid uuid_; > int i; > > ctx->error = get_table(table_name, &table); > if (ctx->error) { > return; > } > + > if (id) { > - struct ovsdb_symbol *symbol = NULL; > + if (uuid_from_string(&uuid_, id)) { > + uuid = &uuid_; > + persist_uuid = true; > + } else { > + struct ovsdb_symbol *symbol = NULL; > > - ctx->error = create_symbol(ctx->symtab, id, &symbol, NULL); > - if (ctx->error) { > - return; > - } > - if (table->is_root) { > - /* This table is in the root set, meaning that rows created > in it > - * won't disappear even if they are unreferenced, so disable > - * warnings about that by pretending that there is a > reference. */ > - symbol->strong_ref = true; > + ctx->error = create_symbol(ctx->symtab, id, &symbol, NULL); > + if (ctx->error) { > + return; > + } > + if (table->is_root) { > + /* This table is in the root set, meaning that rows > created in > + * it won't disappear even if they are unreferenced, so > disable > + * warnings about that by pretending that there is a > + * reference. */ > + symbol->strong_ref = true; > + } > + uuid = &symbol->uuid; > } > - uuid = &symbol->uuid; > } > > - row = ovsdb_idl_txn_insert(ctx->txn, table, uuid); > + if (persist_uuid) { > + row = ovsdb_idl_txn_insert_persist_uuid(ctx->txn, table, uuid); > + } else { > + row = ovsdb_idl_txn_insert(ctx->txn, table, uuid); > + } > + > for (i = 2; i < ctx->argc; i++) { > ctx->error = set_column(table, row, ctx->argv[i], ctx->symtab); > if (ctx->error) { > diff --git a/lib/db-ctl-base.man b/lib/db-ctl-base.man > index a529d8b4d..c8111c9ef 100644 > --- a/lib/db-ctl-base.man > +++ b/lib/db-ctl-base.man > @@ -203,7 +203,7 @@ Without \fB\-\-if-exists\fR, it is an error if > \fIrecord\fR does not > exist. With \fB\-\-if-exists\fR, this command does nothing if > \fIrecord\fR does not exist. > . > -.IP "[\fB\-\-id=@\fIname\fR] \fBcreate\fR \fItable > column\fR[\fB:\fIkey\fR]\fB=\fIvalue\fR..." > +.IP "[\fB\-\-id=(@\fIname\fR | \fIuuid\fR] \fBcreate\fR \fItable > column\fR[\fB:\fIkey\fR]\fB=\fIvalue\fR..." > Creates a new record in \fItable\fR and sets the initial values of > each \fIcolumn\fR. Columns not explicitly set will receive their > default values. Outputs the UUID of the new row. > @@ -212,6 +212,9 @@ If \fB@\fIname\fR is specified, then the UUID for the > new row may be > referred to by that name elsewhere in the same \fB\*(PN\fR > invocation in contexts where a UUID is expected. Such references may > precede or follow the \fBcreate\fR command. > +.IP > +If a valid \fIuuid\fR is specified, then it is used as the UUID > +of the new row. > . > .RS > .IP "Caution (ovs-vsctl as example)" > diff --git a/lib/db-ctl-base.xml b/lib/db-ctl-base.xml > index f6efe98ea..27c999fe7 100644 > --- a/lib/db-ctl-base.xml > +++ b/lib/db-ctl-base.xml > @@ -310,7 +310,7 @@ > </p> > </dd> > > - <dt>[<code>--id=@</code><var>name</var>] <code>create</code> > <var>table > column</var>[<code>:</code><var>key</var>]<code>=</code><var>value</var>...</dt> > + <dt>[<code>--id=(@</code><var>name</var>|<var>uuid</var>)] > <code>create</code> <var>table > column</var>[<code>:</code><var>key</var>]<code>=</code><var>value</var>...</dt> > <dd> > <p> > Creates a new record in <var>table</var> and sets the initial > values of > @@ -323,6 +323,10 @@ > invocation in contexts where a UUID is expected. Such references > may > precede or follow the <code>create</code> command. > </p> > + <p> > + If a valid <var>uuid</var> is specified, then it is used as the > + UUID of the new row. > + </p> > <dl> > <dt>Caution (ovs-vsctl as example)</dt> > <dd> > diff --git a/lib/ovsdb-idl-provider.h b/lib/ovsdb-idl-provider.h > index 8797686f9..8d2b7d6b9 100644 > --- a/lib/ovsdb-idl-provider.h > +++ b/lib/ovsdb-idl-provider.h > @@ -74,6 +74,7 @@ struct ovsdb_idl_row { > struct ovs_list dst_arcs; /* Backward arcs > (ovsdb_idl_arc.dst_node). */ > struct ovsdb_idl_table *table; /* Containing table. */ > struct ovsdb_datum *old_datum; /* Committed data (null if orphaned). > */ > + bool persist_uuid; /* Persist 'uuid' during insert txn if > set. */ > bool parsed; /* Whether the row is parsed. */ > struct ovs_list reparse_node; /* Rows that needs to be re-parsed due > to > * insertion of a referenced row. */ > diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c > index 99b58422e..dbdfe45d8 100644 > --- a/lib/ovsdb-idl.c > +++ b/lib/ovsdb-idl.c > @@ -2855,11 +2855,14 @@ substitute_uuids(struct json *json, const struct > ovsdb_idl_txn *txn) > > row = ovsdb_idl_txn_get_row(txn, &uuid); > if (row && !row->old_datum && row->new_datum) { > - json_destroy(json); > - > - return json_array_create_2( > - json_string_create("named-uuid"), > - > json_string_create_nocopy(ovsdb_data_row_name(&uuid))); > + if (row->persist_uuid) { > + return json; > + } else { > + json_destroy(json); > + return json_array_create_2( > + json_string_create("named-uuid"), > + > json_string_create_nocopy(ovsdb_data_row_name(&uuid))); > + } > } > } > > @@ -3284,9 +3287,19 @@ ovsdb_idl_txn_commit(struct ovsdb_idl_txn *txn) > > any_updates = true; > > - json_object_put(op, "uuid-name", > - json_string_create_nocopy( > - ovsdb_data_row_name(&row->uuid))); > + char *uuid_json; > + struct json *value; > + if (row->persist_uuid) { > + uuid_json = "uuid"; > + value = json_string_create_nocopy( > + xasprintf(UUID_FMT, UUID_ARGS(&row->uuid))); > + } else { > + uuid_json = "uuid-name"; > + value = json_string_create_nocopy( > + ovsdb_data_row_name(&row->uuid)); > + } > + > + json_object_put(op, uuid_json, value); > > insert = xmalloc(sizeof *insert); > insert->dummy = row->uuid; > @@ -3770,6 +3783,31 @@ ovsdb_idl_txn_delete(const struct ovsdb_idl_row > *row_) > row->new_datum = NULL; > } > > +static const struct ovsdb_idl_row * > +ovsdb_idl_txn_insert__(struct ovsdb_idl_txn *txn, > + const struct ovsdb_idl_table_class *class, > + const struct uuid *uuid, > + bool persist_uuid) > +{ > + struct ovsdb_idl_row *row = ovsdb_idl_row_create__(class); > + > + ovs_assert(uuid || !persist_uuid); > + if (uuid) { > + ovs_assert(!ovsdb_idl_txn_get_row(txn, uuid)); > + row->uuid = *uuid; > + } else { > + uuid_generate(&row->uuid); > + } > + row->persist_uuid = persist_uuid; > + row->table = ovsdb_idl_table_from_class(txn->idl, class); > + row->new_datum = xmalloc(class->n_columns * sizeof *row->new_datum); > + hmap_insert(&row->table->rows, &row->hmap_node, > uuid_hash(&row->uuid)); > + hmap_insert(&txn->txn_rows, &row->txn_node, uuid_hash(&row->uuid)); > + ovsdb_idl_add_to_indexes(row); > + > + return row; > +} > + > /* Inserts and returns a new row in the table with the specified 'class' > in the > * database with open transaction 'txn'. > * > @@ -3787,22 +3825,23 @@ ovsdb_idl_txn_insert(struct ovsdb_idl_txn *txn, > const struct ovsdb_idl_table_class *class, > const struct uuid *uuid) > { > - struct ovsdb_idl_row *row = ovsdb_idl_row_create__(class); > - > - if (uuid) { > - ovs_assert(!ovsdb_idl_txn_get_row(txn, uuid)); > - row->uuid = *uuid; > - } else { > - uuid_generate(&row->uuid); > - } > - > - row->table = ovsdb_idl_table_from_class(txn->idl, class); > - row->new_datum = xmalloc(class->n_columns * sizeof *row->new_datum); > - hmap_insert(&row->table->rows, &row->hmap_node, > uuid_hash(&row->uuid)); > - hmap_insert(&txn->txn_rows, &row->txn_node, uuid_hash(&row->uuid)); > - ovsdb_idl_add_to_indexes(row); > + return ovsdb_idl_txn_insert__(txn, class, uuid, false); > +} > > - return row; > +/* Inserts and returns a new row in the table with the specified 'class' > in the > + * database with open transaction 'txn'. > + * > + * The new row is assigned the specified UUID (which cannot be null). > + * > + * Usually this function is used indirectly through one of the > + * "insert_persist_uuid" functions generated by ovsdb-idlc. */ > +const struct ovsdb_idl_row * > +ovsdb_idl_txn_insert_persist_uuid(struct ovsdb_idl_txn *txn, > + const struct ovsdb_idl_table_class > *class, > + const struct uuid *uuid) > +{ > + ovs_assert(uuid); > + return ovsdb_idl_txn_insert__(txn, class, uuid, true); > } > > static void > diff --git a/lib/ovsdb-idl.h b/lib/ovsdb-idl.h > index fbd9f671a..9a3e19f20 100644 > --- a/lib/ovsdb-idl.h > +++ b/lib/ovsdb-idl.h > @@ -375,6 +375,9 @@ void ovsdb_idl_txn_delete(const struct ovsdb_idl_row > *); > const struct ovsdb_idl_row *ovsdb_idl_txn_insert( > struct ovsdb_idl_txn *, const struct ovsdb_idl_table_class *, > const struct uuid *); > +const struct ovsdb_idl_row *ovsdb_idl_txn_insert_persist_uuid( > + struct ovsdb_idl_txn *txn, const struct ovsdb_idl_table_class *class, > + const struct uuid *uuid); > > struct ovsdb_idl *ovsdb_idl_txn_get_idl (struct ovsdb_idl_txn *); > void ovsdb_idl_get_initial_snapshot(struct ovsdb_idl *); > diff --git a/ovsdb/ovsdb-idlc.in b/ovsdb/ovsdb-idlc.in > index 5a97a8ea3..9a54f06a1 100755 > --- a/ovsdb/ovsdb-idlc.in > +++ b/ovsdb/ovsdb-idlc.in > @@ -362,6 +362,8 @@ struct %(s)s *%(s)s_cursor_data(struct > ovsdb_idl_cursor *); > void %(s)s_init(struct %(s)s *); > void %(s)s_delete(const struct %(s)s *); > struct %(s)s *%(s)s_insert(struct ovsdb_idl_txn *); > +struct %(s)s *%(s)s_insert_persist_uuid( > + struct ovsdb_idl_txn *txn, const struct uuid *uuid); > > /* Returns true if the tracked column referenced by 'enum > %(s)s_column_id' of > * the row referenced by 'struct %(s)s *' was updated since the last > change > @@ -809,6 +811,19 @@ struct %(s)s * > return %(s)s_cast(ovsdb_idl_txn_insert(txn, &%(p)stable_%(tl)s, > NULL)); > } > > +/* Inserts and returns a new row in the table "%(t)s" in the database > + * with open transaction 'txn'. > + * > + * The new row is assigned the UUID specified in the 'uuid' parameter > + * (which cannot be null). ovsdb-server will try to assign the same > + * UUID when 'txn' is committed. */ > +struct %(s)s * > +%(s)s_insert_persist_uuid(struct ovsdb_idl_txn *txn, const struct uuid > *uuid) > +{ > + return %(s)s_cast(ovsdb_idl_txn_insert_persist_uuid( > + txn, &%(p)stable_%(tl)s, uuid)); > +} > + > bool > %(s)s_is_updated(const struct %(s)s *row, enum %(s)s_column_id column) > { > diff --git a/python/ovs/db/idl.py b/python/ovs/db/idl.py > index 8e31e02d7..fe66402cf 100644 > --- a/python/ovs/db/idl.py > +++ b/python/ovs/db/idl.py > @@ -1223,7 +1223,7 @@ class Row(object): > d["a"] = "b" > row.mycolumn = d > """ > - def __init__(self, idl, table, uuid, data): > + def __init__(self, idl, table, uuid, data, persist_uuid=False): > # All of the explicit references to self.__dict__ below are > required > # to set real attributes with invoking self.__getattr__(). > self.__dict__["uuid"] = uuid > @@ -1278,6 +1278,10 @@ class Row(object): > # in the dictionary are all None. > self.__dict__["_prereqs"] = {} > > + # Indicates if the specified 'uuid' should be used as the row uuid > + # or let the server generate it. > + self.__dict__["_persist_uuid"] = persist_uuid > + > def __lt__(self, other): > if not isinstance(other, Row): > return NotImplemented > @@ -1816,7 +1820,11 @@ class Transaction(object): > op = {"table": row._table.name} > if row._data is None: > op["op"] = "insert" > - op["uuid-name"] = _uuid_name_from_uuid(row.uuid) > + if row._persist_uuid: > + op["uuid"] = row.uuid > + else: > + op["uuid-name"] = _uuid_name_from_uuid(row.uuid) > + > any_updates = True > > op_index = len(operations) - 1 > @@ -2056,20 +2064,22 @@ class Transaction(object): > row._mutations['_removes'].pop(column.name, None) > row._changes[column.name] = datum.copy() > > - def insert(self, table, new_uuid=None): > + def insert(self, table, new_uuid=None, persist_uuid=False): > """Inserts and returns a new row in 'table', which must be one of > the > ovs.db.schema.TableSchema objects in the Idl's 'tables' dict. > > The new row is assigned a provisional UUID. If 'uuid' is None > then one > is randomly generated; otherwise 'uuid' should specify a randomly > - generated uuid.UUID not otherwise in use. ovsdb-server will > assign a > - different UUID when 'txn' is committed, but the IDL will replace > any > - uses of the provisional UUID in the data to be to be committed by > the > - UUID assigned by ovsdb-server.""" > + generated uuid.UUID not otherwise in use. If 'persist_uuid' is > true > + and 'new_uuid' is specified, IDL requests the ovsdb-server to > assign > + the same UUID, otherwise ovsdb-server will assign a different > UUID when > + 'txn' is committed and the IDL will replace any uses of the > provisional > + UUID in the data to be committed by the UUID assigned by > + ovsdb-server.""" > assert self._status == Transaction.UNCOMMITTED > if new_uuid is None: > new_uuid = uuid.uuid4() > - row = Row(self.idl, table, new_uuid, None) > + row = Row(self.idl, table, new_uuid, None, > persist_uuid=persist_uuid) > table.rows[row.uuid] = row > self._txn_rows[row.uuid] = row > return row > diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at > index d6cd2c084..abf4fb9cf 100644 > --- a/tests/ovs-vsctl.at > +++ b/tests/ovs-vsctl.at > @@ -1710,3 +1710,28 @@ ingress_policing_kpkts_rate: 100 > ]) > OVS_VSCTL_CLEANUP > AT_CLEANUP > + > +AT_SETUP([ovs-vsctl create bridge with uuid]) > +AT_KEYWORDS([create bridge with uuid]) > +OVS_VSCTL_SETUP > + > +AT_CHECK([ovs-vsctl --no-wait --id=c5cc12f8-eaa1-43a7-8a73-bccd18df1111 > create bridge \ > +name=tst0 -- add open . bridges c5cc12f8-eaa1-43a7-8a73-bccd18df1111], > [0],[dnl > +c5cc12f8-eaa1-43a7-8a73-bccd18df1111 > +]) > + > +AT_CHECK([ovs-vsctl --no-wait --id=c5cc12f8-eaa1-43a7-8a73-bccd18df1111 > create bridge \ > +name=tst1 -- add open . bridges c5cc12f8-eaa1-43a7-8a73-bccd18df1111], > [1], [ignore], [ignore]) > + > +AT_CHECK([ovs-vsctl --no-wait --bare --columns _uuid,name list bridge], > [0], [dnl > +c5cc12f8-eaa1-43a7-8a73-bccd18df1111 > +tst0 > +]) > + > +ovs-vsctl --no-wait --id=@a create bridge \ > +name=tst1 -- add open . bridges @a > + > +AT_CHECK([ovs-vsctl --no-wait --bare --columns _uuid,name list bridge > tst1], [0], [ignore]) > + > +OVS_VSCTL_CLEANUP > +AT_CLEANUP > diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at > index 8e75d00d7..c2970984b 100644 > --- a/tests/ovsdb-idl.at > +++ b/tests/ovsdb-idl.at > @@ -2555,3 +2555,61 @@ OVSDB_CHECK_IDL_TRACK([track, insert and delete, > refs to link2], > 005: table link2: i=1 l1= uuid=<1> > 006: done > ]]) > + > +m4_define([OVSDB_CHECK_IDL_PERS_UUID_INSERT_C], > + [AT_SETUP([$1 - C]) > + AT_KEYWORDS([idl persistent uuid insert]) > + AT_CHECK([ovsdb_start_idltest "" "$abs_srcdir/idltest.ovsschema"]) > + AT_CHECK([test-ovsdb '-vPATTERN:console:test-ovsdb|%c|%m' -vjsonrpc > -t10 idl unix:socket $2], > + [0], [stdout], [stderr]) > + AT_CHECK([sort stdout], > + [0], [$3]) > + AT_CHECK([grep $4 stderr], [0], [ignore]) > + OVSDB_SERVER_SHUTDOWN > + AT_CLEANUP]) > + > +m4_define([OVSDB_CHECK_IDL_PERS_UUID_INSERT_PY], > + [AT_SETUP([$1 - Python3]) > + AT_KEYWORDS([idl persistent uuid insert]) > + AT_CHECK([ovsdb_start_idltest "" "$abs_srcdir/idltest.ovsschema"]) > + AT_CHECK([$PYTHON3 $srcdir/test-ovsdb.py -t10 idl > $srcdir/idltest.ovsschema unix:socket $2], > + [0], [stdout], [stderr]) > + AT_CHECK([sort stdout], > + [0], [$3]) > + AT_CHECK([grep $4 stderr], [0], [ignore]) > + OVSDB_SERVER_SHUTDOWN > + AT_CLEANUP]) > + > + > +m4_define([OVSDB_CHECK_IDL_PERS_UUID_INSERT], > + [OVSDB_CHECK_IDL_PERS_UUID_INSERT_C($@) > + OVSDB_CHECK_IDL_PERS_UUID_INSERT_PY($@)]) > + > +OVSDB_CHECK_IDL_PERS_UUID_INSERT([simple idl, persistent uuid insert], > + [['insert_uuid c5cc12f8-eaa1-43a7-8a73-bccd18df2222 2, insert_uuid > c5cc12f8-eaa1-43a7-8a73-bccd18df3333 3' \ > + 'insert_uuid c5cc12f8-eaa1-43a7-8a73-bccd18df4444 4, insert_uuid > c5cc12f8-eaa1-43a7-8a73-bccd18df2222 5' \ > + 'insert_uuid c5cc12f8-eaa1-43a7-8a73-bccd18df4444 4' \ > + 'delete 2' \ > + 'insert_uuid c5cc12f8-eaa1-43a7-8a73-bccd18df2222 5' > + ]], > + [[000: empty > +001: commit, status=success > +002: table simple: i=2 r=0 b=false s= > u=00000000-0000-0000-0000-000000000000 ia=[] ra=[] ba=[] sa=[] ua=[] > uuid=c5cc12f8-eaa1-43a7-8a73-bccd18df2222 > +002: table simple: i=3 r=0 b=false s= > u=00000000-0000-0000-0000-000000000000 ia=[] ra=[] ba=[] sa=[] ua=[] > uuid=c5cc12f8-eaa1-43a7-8a73-bccd18df3333 > +003: commit, status=error > +004: table simple: i=2 r=0 b=false s= > u=00000000-0000-0000-0000-000000000000 ia=[] ra=[] ba=[] sa=[] ua=[] > uuid=c5cc12f8-eaa1-43a7-8a73-bccd18df2222 > +004: table simple: i=3 r=0 b=false s= > u=00000000-0000-0000-0000-000000000000 ia=[] ra=[] ba=[] sa=[] ua=[] > uuid=c5cc12f8-eaa1-43a7-8a73-bccd18df3333 > +005: commit, status=success > +006: table simple: i=2 r=0 b=false s= > u=00000000-0000-0000-0000-000000000000 ia=[] ra=[] ba=[] sa=[] ua=[] > uuid=c5cc12f8-eaa1-43a7-8a73-bccd18df2222 > +006: table simple: i=3 r=0 b=false s= > u=00000000-0000-0000-0000-000000000000 ia=[] ra=[] ba=[] sa=[] ua=[] > uuid=c5cc12f8-eaa1-43a7-8a73-bccd18df3333 > +006: table simple: i=4 r=0 b=false s= > u=00000000-0000-0000-0000-000000000000 ia=[] ra=[] ba=[] sa=[] ua=[] > uuid=c5cc12f8-eaa1-43a7-8a73-bccd18df4444 > +007: commit, status=success > +008: table simple: i=3 r=0 b=false s= > u=00000000-0000-0000-0000-000000000000 ia=[] ra=[] ba=[] sa=[] ua=[] > uuid=c5cc12f8-eaa1-43a7-8a73-bccd18df3333 > +008: table simple: i=4 r=0 b=false s= > u=00000000-0000-0000-0000-000000000000 ia=[] ra=[] ba=[] sa=[] ua=[] > uuid=c5cc12f8-eaa1-43a7-8a73-bccd18df4444 > +009: commit, status=success > +010: table simple: i=3 r=0 b=false s= > u=00000000-0000-0000-0000-000000000000 ia=[] ra=[] ba=[] sa=[] ua=[] > uuid=c5cc12f8-eaa1-43a7-8a73-bccd18df3333 > +010: table simple: i=4 r=0 b=false s= > u=00000000-0000-0000-0000-000000000000 ia=[] ra=[] ba=[] sa=[] ua=[] > uuid=c5cc12f8-eaa1-43a7-8a73-bccd18df4444 > +010: table simple: i=5 r=0 b=false s= > u=00000000-0000-0000-0000-000000000000 ia=[] ra=[] ba=[] sa=[] ua=[] > uuid=c5cc12f8-eaa1-43a7-8a73-bccd18df2222 > +011: done > +]], > + [['This UUID would duplicate a UUID already present within the table or > deleted within the same transaction']]) > diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c > index 5f7110f41..84fe23276 100644 > --- a/tests/test-ovsdb.c > +++ b/tests/test-ovsdb.c > @@ -2400,7 +2400,7 @@ idltest_find_simple(struct ovsdb_idl *idl, int i) > return NULL; > } > > -static void > +static bool > idl_set(struct ovsdb_idl *idl, char *commands, int step) > { > char *cmd, *save_ptr1 = NULL; > @@ -2458,6 +2458,19 @@ idl_set(struct ovsdb_idl *idl, char *commands, int > step) > > s = idltest_simple_insert(txn); > idltest_simple_set_i(s, atoi(arg1)); > + } else if (!strcmp(name, "insert_uuid")) { > + struct idltest_simple *s; > + > + if (!arg1 || !arg2) { > + ovs_fatal(0, "\"insert\" command requires 2 arguments"); > + } > + > + struct uuid s_uuid; > + if (!uuid_from_string(&s_uuid, arg1)) { > + ovs_fatal(0, "\"insert_uuid\" command requires valid > uuid"); > + } > + s = idltest_simple_insert_persist_uuid(txn, &s_uuid); > + idltest_simple_set_i(s, atoi(arg2)); > } else if (!strcmp(name, "delete")) { > const struct idltest_simple *s; > > @@ -2522,7 +2535,7 @@ idl_set(struct ovsdb_idl *idl, char *commands, int > step) > print_and_log("%03d: destroy", step); > ovsdb_idl_txn_destroy(txn); > ovsdb_idl_check_consistency(idl); > - return; > + return true; > } else { > ovs_fatal(0, "unknown command %s", name); > } > @@ -2543,6 +2556,8 @@ idl_set(struct ovsdb_idl *idl, char *commands, int > step) > > ovsdb_idl_txn_destroy(txn); > ovsdb_idl_check_consistency(idl); > + > + return (status != TXN_ERROR); > } > > static const struct ovsdb_idl_table_class * > @@ -2777,7 +2792,14 @@ do_idl(struct ovs_cmdl_context *ctx) > update_conditions(idl, arg + strlen(cond_s)); > print_and_log("%03d: change conditions", step++); > } else if (arg[0] != '[') { > - idl_set(idl, arg, step++); > + if (!idl_set(idl, arg, step++)) { > + /* If idl_set() returns false, then no transaction > + * was sent to the server and most likely 'seqno' > + * would remain the same. And the above 'Wait for update' > + * for loop poll_block() would never return. > + * So set seqno to 0. */ > + seqno = 0; > + } > } else { > struct json *json = parse_json(arg); > substitute_uuids(json, symtab); > diff --git a/tests/test-ovsdb.py b/tests/test-ovsdb.py > index 402cacbe9..cca1818ea 100644 > --- a/tests/test-ovsdb.py > +++ b/tests/test-ovsdb.py > @@ -429,6 +429,14 @@ def idl_set(idl, commands, step): > > s = txn.insert(idl.tables["simple"]) > s.i = int(args[0]) > + elif name == "insert_uuid": > + if len(args) != 2: > + sys.stderr.write('"set" command requires 2 argument\n') > + sys.exit(1) > + > + s = txn.insert(idl.tables["simple"], new_uuid=args[0], > + persist_uuid=True) > + s.i = int(args[1]) > elif name == "delete": > if len(args) != 1: > sys.stderr.write('"delete" command requires 1 argument\n') > @@ -491,7 +499,7 @@ def idl_set(idl, commands, step): > print("%03d: destroy" % step) > sys.stdout.flush() > txn.abort() > - return > + return True > elif name == "linktest": > l1_0 = txn.insert(idl.tables["link1"]) > l1_0.i = 1 > @@ -615,6 +623,8 @@ def idl_set(idl, commands, step): > sys.stdout.write("\n") > sys.stdout.flush() > > + return status != ovs.db.idl.Transaction.ERROR > + > > def update_condition(idl, commands): > commands = commands[len("condition "):].split(";") > @@ -748,7 +758,13 @@ def do_idl(schema_file, remote, *commands): > sys.stdout.flush() > step += 1 > elif not command.startswith("["): > - idl_set(idl, command, step) > + if not idl_set(idl, command, step): > + # If idl_set() returns false, then no transaction > + # was sent to the server and most likely seqno > + # would remain the same. And the above 'Wait for update' > + # for loop poller.block() would never return. > + # So set seqno to 0. > + seqno = 0 > step += 1 > else: > json = ovs.json.from_string(command) > -- > 2.38.1 > > Looks good to me, I'll work on adding some code to ovsdbapp that uses this as well. Thanks! Acked-by: Terry Wilson <twilson@redhat.com>
On 11/29/22 01:32, Terry Wilson wrote: > On Sun, Nov 27, 2022 at 9:56 PM <numans@ovn.org <mailto:numans@ovn.org>> wrote: > > From: Numan Siddique <numans@ovn.org <mailto:numans@ovn.org>> > > ovsdb-server allows the OVSDB clients to specify the uuid for > the row inserts [1]. Both the C IDL client library and Python > IDL are missing this feature. This patch adds this support. > > In C IDL, for each schema table, a new function is generated - > <schema_table>insert_persistent_uuid(txn, uuid) which can > be used the clients to persist the uuid. > > ovs-vsctl and other derivatives of ctl now supports the same > in the generic 'create' command with the option "--id=<UUID>". > > In Python IDL, the uuid to persist can be specified in > the Transaction.insert() function. > > [1] - a529e3cd1f("ovsdb-server: Allow OVSDB clients to specify the UUID for inserted rows.:) > > Signed-off-by: Numan Siddique <numans@ovn.org <mailto:numans@ovn.org>> > Acked-by: Adrian Moreno <amorenoz@redhat.com <mailto:amorenoz@redhat.com>> > Acked-by: Han Zhou <hzhou@ovn.org <mailto:hzhou@ovn.org>> > CC: twilson@redhat.com <mailto:twilson@redhat.com> > CC: i.maximets@ovn.org <mailto:i.maximets@ovn.org> > > Looks good to me, I'll work on adding some code to ovsdbapp that uses this as well. Thanks! > > Acked-by: Terry Wilson <twilson@redhat.com <mailto:twilson@redhat.com>> Applied. Thanks! Best regards, Ilya Maximets.
diff --git a/NEWS b/NEWS index ff77ee404..b827a64ab 100644 --- a/NEWS +++ b/NEWS @@ -24,6 +24,9 @@ Post-v3.0.0 If a user wishes to benefit from these fixes it is recommended to use DPDK 21.11.2. + - OVSDB-IDL: + * Add the support to specify the persistent uuid for row insert in both + C and Python IDLs. v3.0.0 - 15 Aug 2022 -------------------- diff --git a/lib/db-ctl-base.c b/lib/db-ctl-base.c index bc85e9921..856832a04 100644 --- a/lib/db-ctl-base.c +++ b/lib/db-ctl-base.c @@ -1731,29 +1731,43 @@ cmd_create(struct ctl_context *ctx) const struct ovsdb_idl_table_class *table; const struct ovsdb_idl_row *row; const struct uuid *uuid = NULL; + bool persist_uuid = false; + struct uuid uuid_; int i; ctx->error = get_table(table_name, &table); if (ctx->error) { return; } + if (id) { - struct ovsdb_symbol *symbol = NULL; + if (uuid_from_string(&uuid_, id)) { + uuid = &uuid_; + persist_uuid = true; + } else { + struct ovsdb_symbol *symbol = NULL; - ctx->error = create_symbol(ctx->symtab, id, &symbol, NULL); - if (ctx->error) { - return; - } - if (table->is_root) { - /* This table is in the root set, meaning that rows created in it - * won't disappear even if they are unreferenced, so disable - * warnings about that by pretending that there is a reference. */ - symbol->strong_ref = true; + ctx->error = create_symbol(ctx->symtab, id, &symbol, NULL); + if (ctx->error) { + return; + } + if (table->is_root) { + /* This table is in the root set, meaning that rows created in + * it won't disappear even if they are unreferenced, so disable + * warnings about that by pretending that there is a + * reference. */ + symbol->strong_ref = true; + } + uuid = &symbol->uuid; } - uuid = &symbol->uuid; } - row = ovsdb_idl_txn_insert(ctx->txn, table, uuid); + if (persist_uuid) { + row = ovsdb_idl_txn_insert_persist_uuid(ctx->txn, table, uuid); + } else { + row = ovsdb_idl_txn_insert(ctx->txn, table, uuid); + } + for (i = 2; i < ctx->argc; i++) { ctx->error = set_column(table, row, ctx->argv[i], ctx->symtab); if (ctx->error) { diff --git a/lib/db-ctl-base.man b/lib/db-ctl-base.man index a529d8b4d..c8111c9ef 100644 --- a/lib/db-ctl-base.man +++ b/lib/db-ctl-base.man @@ -203,7 +203,7 @@ Without \fB\-\-if-exists\fR, it is an error if \fIrecord\fR does not exist. With \fB\-\-if-exists\fR, this command does nothing if \fIrecord\fR does not exist. . -.IP "[\fB\-\-id=@\fIname\fR] \fBcreate\fR \fItable column\fR[\fB:\fIkey\fR]\fB=\fIvalue\fR..." +.IP "[\fB\-\-id=(@\fIname\fR | \fIuuid\fR] \fBcreate\fR \fItable column\fR[\fB:\fIkey\fR]\fB=\fIvalue\fR..." Creates a new record in \fItable\fR and sets the initial values of each \fIcolumn\fR. Columns not explicitly set will receive their default values. Outputs the UUID of the new row. @@ -212,6 +212,9 @@ If \fB@\fIname\fR is specified, then the UUID for the new row may be referred to by that name elsewhere in the same \fB\*(PN\fR invocation in contexts where a UUID is expected. Such references may precede or follow the \fBcreate\fR command. +.IP +If a valid \fIuuid\fR is specified, then it is used as the UUID +of the new row. . .RS .IP "Caution (ovs-vsctl as example)" diff --git a/lib/db-ctl-base.xml b/lib/db-ctl-base.xml index f6efe98ea..27c999fe7 100644 --- a/lib/db-ctl-base.xml +++ b/lib/db-ctl-base.xml @@ -310,7 +310,7 @@ </p> </dd> - <dt>[<code>--id=@</code><var>name</var>] <code>create</code> <var>table column</var>[<code>:</code><var>key</var>]<code>=</code><var>value</var>...</dt> + <dt>[<code>--id=(@</code><var>name</var>|<var>uuid</var>)] <code>create</code> <var>table column</var>[<code>:</code><var>key</var>]<code>=</code><var>value</var>...</dt> <dd> <p> Creates a new record in <var>table</var> and sets the initial values of @@ -323,6 +323,10 @@ invocation in contexts where a UUID is expected. Such references may precede or follow the <code>create</code> command. </p> + <p> + If a valid <var>uuid</var> is specified, then it is used as the + UUID of the new row. + </p> <dl> <dt>Caution (ovs-vsctl as example)</dt> <dd> diff --git a/lib/ovsdb-idl-provider.h b/lib/ovsdb-idl-provider.h index 8797686f9..8d2b7d6b9 100644 --- a/lib/ovsdb-idl-provider.h +++ b/lib/ovsdb-idl-provider.h @@ -74,6 +74,7 @@ struct ovsdb_idl_row { struct ovs_list dst_arcs; /* Backward arcs (ovsdb_idl_arc.dst_node). */ struct ovsdb_idl_table *table; /* Containing table. */ struct ovsdb_datum *old_datum; /* Committed data (null if orphaned). */ + bool persist_uuid; /* Persist 'uuid' during insert txn if set. */ bool parsed; /* Whether the row is parsed. */ struct ovs_list reparse_node; /* Rows that needs to be re-parsed due to * insertion of a referenced row. */ diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c index 99b58422e..dbdfe45d8 100644 --- a/lib/ovsdb-idl.c +++ b/lib/ovsdb-idl.c @@ -2855,11 +2855,14 @@ substitute_uuids(struct json *json, const struct ovsdb_idl_txn *txn) row = ovsdb_idl_txn_get_row(txn, &uuid); if (row && !row->old_datum && row->new_datum) { - json_destroy(json); - - return json_array_create_2( - json_string_create("named-uuid"), - json_string_create_nocopy(ovsdb_data_row_name(&uuid))); + if (row->persist_uuid) { + return json; + } else { + json_destroy(json); + return json_array_create_2( + json_string_create("named-uuid"), + json_string_create_nocopy(ovsdb_data_row_name(&uuid))); + } } } @@ -3284,9 +3287,19 @@ ovsdb_idl_txn_commit(struct ovsdb_idl_txn *txn) any_updates = true; - json_object_put(op, "uuid-name", - json_string_create_nocopy( - ovsdb_data_row_name(&row->uuid))); + char *uuid_json; + struct json *value; + if (row->persist_uuid) { + uuid_json = "uuid"; + value = json_string_create_nocopy( + xasprintf(UUID_FMT, UUID_ARGS(&row->uuid))); + } else { + uuid_json = "uuid-name"; + value = json_string_create_nocopy( + ovsdb_data_row_name(&row->uuid)); + } + + json_object_put(op, uuid_json, value); insert = xmalloc(sizeof *insert); insert->dummy = row->uuid; @@ -3770,6 +3783,31 @@ ovsdb_idl_txn_delete(const struct ovsdb_idl_row *row_) row->new_datum = NULL; } +static const struct ovsdb_idl_row * +ovsdb_idl_txn_insert__(struct ovsdb_idl_txn *txn, + const struct ovsdb_idl_table_class *class, + const struct uuid *uuid, + bool persist_uuid) +{ + struct ovsdb_idl_row *row = ovsdb_idl_row_create__(class); + + ovs_assert(uuid || !persist_uuid); + if (uuid) { + ovs_assert(!ovsdb_idl_txn_get_row(txn, uuid)); + row->uuid = *uuid; + } else { + uuid_generate(&row->uuid); + } + row->persist_uuid = persist_uuid; + row->table = ovsdb_idl_table_from_class(txn->idl, class); + row->new_datum = xmalloc(class->n_columns * sizeof *row->new_datum); + hmap_insert(&row->table->rows, &row->hmap_node, uuid_hash(&row->uuid)); + hmap_insert(&txn->txn_rows, &row->txn_node, uuid_hash(&row->uuid)); + ovsdb_idl_add_to_indexes(row); + + return row; +} + /* Inserts and returns a new row in the table with the specified 'class' in the * database with open transaction 'txn'. * @@ -3787,22 +3825,23 @@ ovsdb_idl_txn_insert(struct ovsdb_idl_txn *txn, const struct ovsdb_idl_table_class *class, const struct uuid *uuid) { - struct ovsdb_idl_row *row = ovsdb_idl_row_create__(class); - - if (uuid) { - ovs_assert(!ovsdb_idl_txn_get_row(txn, uuid)); - row->uuid = *uuid; - } else { - uuid_generate(&row->uuid); - } - - row->table = ovsdb_idl_table_from_class(txn->idl, class); - row->new_datum = xmalloc(class->n_columns * sizeof *row->new_datum); - hmap_insert(&row->table->rows, &row->hmap_node, uuid_hash(&row->uuid)); - hmap_insert(&txn->txn_rows, &row->txn_node, uuid_hash(&row->uuid)); - ovsdb_idl_add_to_indexes(row); + return ovsdb_idl_txn_insert__(txn, class, uuid, false); +} - return row; +/* Inserts and returns a new row in the table with the specified 'class' in the + * database with open transaction 'txn'. + * + * The new row is assigned the specified UUID (which cannot be null). + * + * Usually this function is used indirectly through one of the + * "insert_persist_uuid" functions generated by ovsdb-idlc. */ +const struct ovsdb_idl_row * +ovsdb_idl_txn_insert_persist_uuid(struct ovsdb_idl_txn *txn, + const struct ovsdb_idl_table_class *class, + const struct uuid *uuid) +{ + ovs_assert(uuid); + return ovsdb_idl_txn_insert__(txn, class, uuid, true); } static void diff --git a/lib/ovsdb-idl.h b/lib/ovsdb-idl.h index fbd9f671a..9a3e19f20 100644 --- a/lib/ovsdb-idl.h +++ b/lib/ovsdb-idl.h @@ -375,6 +375,9 @@ void ovsdb_idl_txn_delete(const struct ovsdb_idl_row *); const struct ovsdb_idl_row *ovsdb_idl_txn_insert( struct ovsdb_idl_txn *, const struct ovsdb_idl_table_class *, const struct uuid *); +const struct ovsdb_idl_row *ovsdb_idl_txn_insert_persist_uuid( + struct ovsdb_idl_txn *txn, const struct ovsdb_idl_table_class *class, + const struct uuid *uuid); struct ovsdb_idl *ovsdb_idl_txn_get_idl (struct ovsdb_idl_txn *); void ovsdb_idl_get_initial_snapshot(struct ovsdb_idl *); diff --git a/ovsdb/ovsdb-idlc.in b/ovsdb/ovsdb-idlc.in index 5a97a8ea3..9a54f06a1 100755 --- a/ovsdb/ovsdb-idlc.in +++ b/ovsdb/ovsdb-idlc.in @@ -362,6 +362,8 @@ struct %(s)s *%(s)s_cursor_data(struct ovsdb_idl_cursor *); void %(s)s_init(struct %(s)s *); void %(s)s_delete(const struct %(s)s *); struct %(s)s *%(s)s_insert(struct ovsdb_idl_txn *); +struct %(s)s *%(s)s_insert_persist_uuid( + struct ovsdb_idl_txn *txn, const struct uuid *uuid); /* Returns true if the tracked column referenced by 'enum %(s)s_column_id' of * the row referenced by 'struct %(s)s *' was updated since the last change @@ -809,6 +811,19 @@ struct %(s)s * return %(s)s_cast(ovsdb_idl_txn_insert(txn, &%(p)stable_%(tl)s, NULL)); } +/* Inserts and returns a new row in the table "%(t)s" in the database + * with open transaction 'txn'. + * + * The new row is assigned the UUID specified in the 'uuid' parameter + * (which cannot be null). ovsdb-server will try to assign the same + * UUID when 'txn' is committed. */ +struct %(s)s * +%(s)s_insert_persist_uuid(struct ovsdb_idl_txn *txn, const struct uuid *uuid) +{ + return %(s)s_cast(ovsdb_idl_txn_insert_persist_uuid( + txn, &%(p)stable_%(tl)s, uuid)); +} + bool %(s)s_is_updated(const struct %(s)s *row, enum %(s)s_column_id column) { diff --git a/python/ovs/db/idl.py b/python/ovs/db/idl.py index 8e31e02d7..fe66402cf 100644 --- a/python/ovs/db/idl.py +++ b/python/ovs/db/idl.py @@ -1223,7 +1223,7 @@ class Row(object): d["a"] = "b" row.mycolumn = d """ - def __init__(self, idl, table, uuid, data): + def __init__(self, idl, table, uuid, data, persist_uuid=False): # All of the explicit references to self.__dict__ below are required # to set real attributes with invoking self.__getattr__(). self.__dict__["uuid"] = uuid @@ -1278,6 +1278,10 @@ class Row(object): # in the dictionary are all None. self.__dict__["_prereqs"] = {} + # Indicates if the specified 'uuid' should be used as the row uuid + # or let the server generate it. + self.__dict__["_persist_uuid"] = persist_uuid + def __lt__(self, other): if not isinstance(other, Row): return NotImplemented @@ -1816,7 +1820,11 @@ class Transaction(object): op = {"table": row._table.name} if row._data is None: op["op"] = "insert" - op["uuid-name"] = _uuid_name_from_uuid(row.uuid) + if row._persist_uuid: + op["uuid"] = row.uuid + else: + op["uuid-name"] = _uuid_name_from_uuid(row.uuid) + any_updates = True op_index = len(operations) - 1 @@ -2056,20 +2064,22 @@ class Transaction(object): row._mutations['_removes'].pop(column.name, None) row._changes[column.name] = datum.copy() - def insert(self, table, new_uuid=None): + def insert(self, table, new_uuid=None, persist_uuid=False): """Inserts and returns a new row in 'table', which must be one of the ovs.db.schema.TableSchema objects in the Idl's 'tables' dict. The new row is assigned a provisional UUID. If 'uuid' is None then one is randomly generated; otherwise 'uuid' should specify a randomly - generated uuid.UUID not otherwise in use. ovsdb-server will assign a - different UUID when 'txn' is committed, but the IDL will replace any - uses of the provisional UUID in the data to be to be committed by the - UUID assigned by ovsdb-server.""" + generated uuid.UUID not otherwise in use. If 'persist_uuid' is true + and 'new_uuid' is specified, IDL requests the ovsdb-server to assign + the same UUID, otherwise ovsdb-server will assign a different UUID when + 'txn' is committed and the IDL will replace any uses of the provisional + UUID in the data to be committed by the UUID assigned by + ovsdb-server.""" assert self._status == Transaction.UNCOMMITTED if new_uuid is None: new_uuid = uuid.uuid4() - row = Row(self.idl, table, new_uuid, None) + row = Row(self.idl, table, new_uuid, None, persist_uuid=persist_uuid) table.rows[row.uuid] = row self._txn_rows[row.uuid] = row return row diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at index d6cd2c084..abf4fb9cf 100644 --- a/tests/ovs-vsctl.at +++ b/tests/ovs-vsctl.at @@ -1710,3 +1710,28 @@ ingress_policing_kpkts_rate: 100 ]) OVS_VSCTL_CLEANUP AT_CLEANUP + +AT_SETUP([ovs-vsctl create bridge with uuid]) +AT_KEYWORDS([create bridge with uuid]) +OVS_VSCTL_SETUP + +AT_CHECK([ovs-vsctl --no-wait --id=c5cc12f8-eaa1-43a7-8a73-bccd18df1111 create bridge \ +name=tst0 -- add open . bridges c5cc12f8-eaa1-43a7-8a73-bccd18df1111], [0],[dnl +c5cc12f8-eaa1-43a7-8a73-bccd18df1111 +]) + +AT_CHECK([ovs-vsctl --no-wait --id=c5cc12f8-eaa1-43a7-8a73-bccd18df1111 create bridge \ +name=tst1 -- add open . bridges c5cc12f8-eaa1-43a7-8a73-bccd18df1111], [1], [ignore], [ignore]) + +AT_CHECK([ovs-vsctl --no-wait --bare --columns _uuid,name list bridge], [0], [dnl +c5cc12f8-eaa1-43a7-8a73-bccd18df1111 +tst0 +]) + +ovs-vsctl --no-wait --id=@a create bridge \ +name=tst1 -- add open . bridges @a + +AT_CHECK([ovs-vsctl --no-wait --bare --columns _uuid,name list bridge tst1], [0], [ignore]) + +OVS_VSCTL_CLEANUP +AT_CLEANUP diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at index 8e75d00d7..c2970984b 100644 --- a/tests/ovsdb-idl.at +++ b/tests/ovsdb-idl.at @@ -2555,3 +2555,61 @@ OVSDB_CHECK_IDL_TRACK([track, insert and delete, refs to link2], 005: table link2: i=1 l1= uuid=<1> 006: done ]]) + +m4_define([OVSDB_CHECK_IDL_PERS_UUID_INSERT_C], + [AT_SETUP([$1 - C]) + AT_KEYWORDS([idl persistent uuid insert]) + AT_CHECK([ovsdb_start_idltest "" "$abs_srcdir/idltest.ovsschema"]) + AT_CHECK([test-ovsdb '-vPATTERN:console:test-ovsdb|%c|%m' -vjsonrpc -t10 idl unix:socket $2], + [0], [stdout], [stderr]) + AT_CHECK([sort stdout], + [0], [$3]) + AT_CHECK([grep $4 stderr], [0], [ignore]) + OVSDB_SERVER_SHUTDOWN + AT_CLEANUP]) + +m4_define([OVSDB_CHECK_IDL_PERS_UUID_INSERT_PY], + [AT_SETUP([$1 - Python3]) + AT_KEYWORDS([idl persistent uuid insert]) + AT_CHECK([ovsdb_start_idltest "" "$abs_srcdir/idltest.ovsschema"]) + AT_CHECK([$PYTHON3 $srcdir/test-ovsdb.py -t10 idl $srcdir/idltest.ovsschema unix:socket $2], + [0], [stdout], [stderr]) + AT_CHECK([sort stdout], + [0], [$3]) + AT_CHECK([grep $4 stderr], [0], [ignore]) + OVSDB_SERVER_SHUTDOWN + AT_CLEANUP]) + + +m4_define([OVSDB_CHECK_IDL_PERS_UUID_INSERT], + [OVSDB_CHECK_IDL_PERS_UUID_INSERT_C($@) + OVSDB_CHECK_IDL_PERS_UUID_INSERT_PY($@)]) + +OVSDB_CHECK_IDL_PERS_UUID_INSERT([simple idl, persistent uuid insert], + [['insert_uuid c5cc12f8-eaa1-43a7-8a73-bccd18df2222 2, insert_uuid c5cc12f8-eaa1-43a7-8a73-bccd18df3333 3' \ + 'insert_uuid c5cc12f8-eaa1-43a7-8a73-bccd18df4444 4, insert_uuid c5cc12f8-eaa1-43a7-8a73-bccd18df2222 5' \ + 'insert_uuid c5cc12f8-eaa1-43a7-8a73-bccd18df4444 4' \ + 'delete 2' \ + 'insert_uuid c5cc12f8-eaa1-43a7-8a73-bccd18df2222 5' + ]], + [[000: empty +001: commit, status=success +002: table simple: i=2 r=0 b=false s= u=00000000-0000-0000-0000-000000000000 ia=[] ra=[] ba=[] sa=[] ua=[] uuid=c5cc12f8-eaa1-43a7-8a73-bccd18df2222 +002: table simple: i=3 r=0 b=false s= u=00000000-0000-0000-0000-000000000000 ia=[] ra=[] ba=[] sa=[] ua=[] uuid=c5cc12f8-eaa1-43a7-8a73-bccd18df3333 +003: commit, status=error +004: table simple: i=2 r=0 b=false s= u=00000000-0000-0000-0000-000000000000 ia=[] ra=[] ba=[] sa=[] ua=[] uuid=c5cc12f8-eaa1-43a7-8a73-bccd18df2222 +004: table simple: i=3 r=0 b=false s= u=00000000-0000-0000-0000-000000000000 ia=[] ra=[] ba=[] sa=[] ua=[] uuid=c5cc12f8-eaa1-43a7-8a73-bccd18df3333 +005: commit, status=success +006: table simple: i=2 r=0 b=false s= u=00000000-0000-0000-0000-000000000000 ia=[] ra=[] ba=[] sa=[] ua=[] uuid=c5cc12f8-eaa1-43a7-8a73-bccd18df2222 +006: table simple: i=3 r=0 b=false s= u=00000000-0000-0000-0000-000000000000 ia=[] ra=[] ba=[] sa=[] ua=[] uuid=c5cc12f8-eaa1-43a7-8a73-bccd18df3333 +006: table simple: i=4 r=0 b=false s= u=00000000-0000-0000-0000-000000000000 ia=[] ra=[] ba=[] sa=[] ua=[] uuid=c5cc12f8-eaa1-43a7-8a73-bccd18df4444 +007: commit, status=success +008: table simple: i=3 r=0 b=false s= u=00000000-0000-0000-0000-000000000000 ia=[] ra=[] ba=[] sa=[] ua=[] uuid=c5cc12f8-eaa1-43a7-8a73-bccd18df3333 +008: table simple: i=4 r=0 b=false s= u=00000000-0000-0000-0000-000000000000 ia=[] ra=[] ba=[] sa=[] ua=[] uuid=c5cc12f8-eaa1-43a7-8a73-bccd18df4444 +009: commit, status=success +010: table simple: i=3 r=0 b=false s= u=00000000-0000-0000-0000-000000000000 ia=[] ra=[] ba=[] sa=[] ua=[] uuid=c5cc12f8-eaa1-43a7-8a73-bccd18df3333 +010: table simple: i=4 r=0 b=false s= u=00000000-0000-0000-0000-000000000000 ia=[] ra=[] ba=[] sa=[] ua=[] uuid=c5cc12f8-eaa1-43a7-8a73-bccd18df4444 +010: table simple: i=5 r=0 b=false s= u=00000000-0000-0000-0000-000000000000 ia=[] ra=[] ba=[] sa=[] ua=[] uuid=c5cc12f8-eaa1-43a7-8a73-bccd18df2222 +011: done +]], + [['This UUID would duplicate a UUID already present within the table or deleted within the same transaction']]) diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c index 5f7110f41..84fe23276 100644 --- a/tests/test-ovsdb.c +++ b/tests/test-ovsdb.c @@ -2400,7 +2400,7 @@ idltest_find_simple(struct ovsdb_idl *idl, int i) return NULL; } -static void +static bool idl_set(struct ovsdb_idl *idl, char *commands, int step) { char *cmd, *save_ptr1 = NULL; @@ -2458,6 +2458,19 @@ idl_set(struct ovsdb_idl *idl, char *commands, int step) s = idltest_simple_insert(txn); idltest_simple_set_i(s, atoi(arg1)); + } else if (!strcmp(name, "insert_uuid")) { + struct idltest_simple *s; + + if (!arg1 || !arg2) { + ovs_fatal(0, "\"insert\" command requires 2 arguments"); + } + + struct uuid s_uuid; + if (!uuid_from_string(&s_uuid, arg1)) { + ovs_fatal(0, "\"insert_uuid\" command requires valid uuid"); + } + s = idltest_simple_insert_persist_uuid(txn, &s_uuid); + idltest_simple_set_i(s, atoi(arg2)); } else if (!strcmp(name, "delete")) { const struct idltest_simple *s; @@ -2522,7 +2535,7 @@ idl_set(struct ovsdb_idl *idl, char *commands, int step) print_and_log("%03d: destroy", step); ovsdb_idl_txn_destroy(txn); ovsdb_idl_check_consistency(idl); - return; + return true; } else { ovs_fatal(0, "unknown command %s", name); } @@ -2543,6 +2556,8 @@ idl_set(struct ovsdb_idl *idl, char *commands, int step) ovsdb_idl_txn_destroy(txn); ovsdb_idl_check_consistency(idl); + + return (status != TXN_ERROR); } static const struct ovsdb_idl_table_class * @@ -2777,7 +2792,14 @@ do_idl(struct ovs_cmdl_context *ctx) update_conditions(idl, arg + strlen(cond_s)); print_and_log("%03d: change conditions", step++); } else if (arg[0] != '[') { - idl_set(idl, arg, step++); + if (!idl_set(idl, arg, step++)) { + /* If idl_set() returns false, then no transaction + * was sent to the server and most likely 'seqno' + * would remain the same. And the above 'Wait for update' + * for loop poll_block() would never return. + * So set seqno to 0. */ + seqno = 0; + } } else { struct json *json = parse_json(arg); substitute_uuids(json, symtab); diff --git a/tests/test-ovsdb.py b/tests/test-ovsdb.py index 402cacbe9..cca1818ea 100644 --- a/tests/test-ovsdb.py +++ b/tests/test-ovsdb.py @@ -429,6 +429,14 @@ def idl_set(idl, commands, step): s = txn.insert(idl.tables["simple"]) s.i = int(args[0]) + elif name == "insert_uuid": + if len(args) != 2: + sys.stderr.write('"set" command requires 2 argument\n') + sys.exit(1) + + s = txn.insert(idl.tables["simple"], new_uuid=args[0], + persist_uuid=True) + s.i = int(args[1]) elif name == "delete": if len(args) != 1: sys.stderr.write('"delete" command requires 1 argument\n') @@ -491,7 +499,7 @@ def idl_set(idl, commands, step): print("%03d: destroy" % step) sys.stdout.flush() txn.abort() - return + return True elif name == "linktest": l1_0 = txn.insert(idl.tables["link1"]) l1_0.i = 1 @@ -615,6 +623,8 @@ def idl_set(idl, commands, step): sys.stdout.write("\n") sys.stdout.flush() + return status != ovs.db.idl.Transaction.ERROR + def update_condition(idl, commands): commands = commands[len("condition "):].split(";") @@ -748,7 +758,13 @@ def do_idl(schema_file, remote, *commands): sys.stdout.flush() step += 1 elif not command.startswith("["): - idl_set(idl, command, step) + if not idl_set(idl, command, step): + # If idl_set() returns false, then no transaction + # was sent to the server and most likely seqno + # would remain the same. And the above 'Wait for update' + # for loop poller.block() would never return. + # So set seqno to 0. + seqno = 0 step += 1 else: json = ovs.json.from_string(command)