| Message ID | 20220504165147.1796350-1-numans@ovn.org |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [ovs-dev,v2] 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/intel-ovs-compilation | success | test: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
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 118 characters long (recommended limit is 79) #95 FILE: lib/db-ctl-base.man:206: .IP "[\fB\-\-id=@\fIname\fR or \fB\-\-id=\fIuuid\fR] \fBcreate\fR \fItable column\fR[\fB:\fIkey\fR]\fB=\fIvalue\fR..." Lines checked: 417, 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 5/4/22 18:51, numans@ovn.org wrote: > From: Numan Siddique <numans@ovn.org> > > ovsdb-server already supports specifying the uuid in the insert > transaction by the client. But the C IDL client library was > missing this feature. This patch adds this support. > > For each schema table, a new function is generated - > <schema_table>insert_persistent_uuid(txn, uuid) and the users > of IDL client library can make use of this function. > > Signed-off-by: Numan Siddique <numans@ovn.org> > --- > > v1 -> v2 > ----- > * Addressed review comments from Adrian Moreno > * Added the support in generic 'create' command to specify the uuid in > --id option. > > lib/db-ctl-base.c | 38 ++++++++++++------ > lib/db-ctl-base.man | 5 ++- > lib/db-ctl-base.xml | 4 ++ > lib/ovsdb-idl-provider.h | 1 + > lib/ovsdb-idl.c | 86 +++++++++++++++++++++++++++++----------- > lib/ovsdb-idl.h | 3 ++ > ovsdb/ovsdb-idlc.in | 15 +++++++ > tests/ovsdb-idl.at | 27 +++++++++++++ > tests/test-ovsdb.c | 59 +++++++++++++++++++++++++++ > 9 files changed, 202 insertions(+), 36 deletions(-) > > diff --git a/lib/db-ctl-base.c b/lib/db-ctl-base.c > index 707456158..f39e090a0 100644 > --- a/lib/db-ctl-base.c > +++ b/lib/db-ctl-base.c > @@ -1732,28 +1732,42 @@ cmd_create(struct ctl_context *ctx) > const struct ovsdb_idl_row *row; > const struct uuid *uuid = NULL; > int i; > + struct uuid uuid_; > + bool persist_uuid = false; > > 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 9c786f298..6899a1d86 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 or \fB\-\-id=\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 \fB@\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..4f63aa7f7 100644 > --- a/lib/db-ctl-base.xml > +++ b/lib/db-ctl-base.xml > @@ -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..332c4075b 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 6fbc109cd..6646618ca 100644 > --- a/lib/ovsdb-idl.c > +++ b/lib/ovsdb-idl.c > @@ -2838,11 +2838,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))); > + } > } > } > > @@ -3267,9 +3270,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; > @@ -3754,6 +3767,32 @@ 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); > + > + if (uuid) { > + ovs_assert(!ovsdb_idl_txn_get_row(txn, uuid)); > + row->uuid = *uuid; > + row->persist_uuid = persist_uuid; > + } else { > + uuid_generate(&row->uuid); > + row->persist_uuid = false; > + } > + > + 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'. > * > @@ -3771,22 +3810,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 13c535939..7e09497ea 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/tests/ovsdb-idl.at b/tests/ovsdb-idl.at > index d1cfa59c5..28c032105 100644 > --- a/tests/ovsdb-idl.at > +++ b/tests/ovsdb-idl.at > @@ -2466,3 +2466,30 @@ unix:socket2 remote has col id in table simple7 > > OVSDB_SERVER_SHUTDOWN > AT_CLEANUP > + > +AT_SETUP([idl creating rows with persistent uuid - C]) > +AT_KEYWORDS([ovsdb client idl txn]) > + > +AT_CHECK([ovsdb_start_idltest "" "$abs_srcdir/idltest.ovsschema"]) > +AT_CHECK([test-ovsdb '-vPATTERN:console:test-ovsdb|%c|%m' -vjsonrpc -t10 idl-txn-persistent-uuid unix:socket], > + [0], [stdout], [stderr]) > +AT_CHECK([sort stdout], [0], > + [[000: After inserting simple, simple3 and simple4 > +001: table simple3: name=simple3 uset=[] uref=[c5cc12f8-eaa1-43a7-8a73-bccd18df1111] uuid=c5cc12f8-eaa1-43a7-8a73-bccd18df2222 > +001: table simple4: name=simple4 uuid=c5cc12f8-eaa1-43a7-8a73-bccd18df1111 > +001: table simple: i=0 r=0 b=false s=simple u=00000000-0000-0000-0000-000000000000 ia=[] ra=[] ba=[] sa=[] ua=[] uuid=c5cc12f8-eaa1-43a7-8a73-bccd18df3333 > +002: After inserting simple with same uuid > +003: table simple3: name=simple3 uset=[] uref=[c5cc12f8-eaa1-43a7-8a73-bccd18df1111] uuid=c5cc12f8-eaa1-43a7-8a73-bccd18df2222 > +003: table simple4: name=simple4 uuid=c5cc12f8-eaa1-43a7-8a73-bccd18df1111 > +003: table simple: i=0 r=0 b=false s=simple u=00000000-0000-0000-0000-000000000000 ia=[] ra=[] ba=[] sa=[] ua=[] uuid=c5cc12f8-eaa1-43a7-8a73-bccd18df3333 > +004: End test > +]]) > + > +AT_CHECK([grep ovsdb_idl stderr | sort], [0], [dnl > +test-ovsdb|ovsdb_idl|transaction error: {"details":"This UUID would dnl > +duplicate a UUID already present within the table or deleted within dnl > +the same transaction.","error":"duplicate uuid","syntax":"\"c5cc12f8-eaa1-43a7-8a73-bccd18df3333\""} > +]) > + > +OVSDB_SERVER_SHUTDOWN > +AT_CLEANUP > diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c > index 0fef1f978..515a3015d 100644 > --- a/tests/test-ovsdb.c > +++ b/tests/test-ovsdb.c > @@ -3389,6 +3389,63 @@ do_idl_table_column_check(struct ovs_cmdl_context *ctx) > ovsdb_idl_destroy(idl); > } > > +static void > +do_idl_txn_persistent_uuid(struct ovs_cmdl_context *ctx) > +{ > + struct ovsdb_idl *idl; > + struct ovsdb_idl_txn *myTxn; > + int step = 0; > + > + idl = ovsdb_idl_create(ctx->argv[1], &idltest_idl_class, true, true); > + ovsdb_idl_get_initial_snapshot(idl); > + ovsdb_idl_run(idl); > + > + myTxn = ovsdb_idl_txn_create(idl); > + > + struct uuid uuid1; > + uuid_from_string(&uuid1, "c5cc12f8-eaa1-43a7-8a73-bccd18df1111"); > + > + struct uuid uuid2; > + uuid_from_string(&uuid2, "c5cc12f8-eaa1-43a7-8a73-bccd18df2222"); > + > + struct idltest_simple4 *simple4_row = > + idltest_simple4_insert_persist_uuid(myTxn, &uuid1); > + idltest_simple4_set_name(simple4_row, "simple4"); > + > + struct idltest_simple3 *simple3_row = > + idltest_simple3_insert_persist_uuid(myTxn, &uuid2); > + idltest_simple3_set_name(simple3_row, "simple3"); > + idltest_simple3_set_uref(simple3_row, &simple4_row, 1); > + > + struct uuid uuid3; > + uuid_from_string(&uuid3, "c5cc12f8-eaa1-43a7-8a73-bccd18df3333"); > + > + struct idltest_simple *simple_row = > + idltest_simple_insert_persist_uuid(myTxn, &uuid3); > + idltest_simple_set_s(simple_row, "simple"); > + > + ovsdb_idl_txn_commit_block(myTxn); > + ovsdb_idl_txn_destroy(myTxn); > + ovsdb_idl_get_initial_snapshot(idl); > + printf("%03d: After inserting simple, simple3 and simple4\n", step++); > + print_idl(idl, step++, false); > + > + /* Create another txn, insert the row in simple table with the existing > + * uuid. */ > + myTxn = ovsdb_idl_txn_create(idl); > + simple_row = > + idltest_simple_insert_persist_uuid(myTxn, &uuid3); > + idltest_simple_set_s(simple_row, "simple_foo"); > + ovsdb_idl_txn_commit_block(myTxn); > + ovsdb_idl_txn_destroy(myTxn); > + ovsdb_idl_get_initial_snapshot(idl); > + printf("%03d: After inserting simple with same uuid\n", step++); > + print_idl(idl, step++, false); > + > + ovsdb_idl_destroy(idl); > + printf("%03d: End test\n", step++); > +} > + > static struct ovs_cmdl_command all_commands[] = { > { "log-io", NULL, 2, INT_MAX, do_log_io, OVS_RO }, > { "default-atoms", NULL, 0, 0, do_default_atoms, OVS_RO }, > @@ -3429,6 +3486,8 @@ static struct ovs_cmdl_command all_commands[] = { > do_idl_partial_update_set_column, OVS_RO }, > { "idl-table-column-check", NULL, 2, 2, > do_idl_table_column_check, OVS_RO }, > + { "idl-txn-persistent-uuid", NULL, 1, INT_MAX, > + do_idl_txn_persistent_uuid, OVS_RO }, > { "help", NULL, 0, INT_MAX, do_help, OVS_RO }, > { NULL, NULL, 0, 0, NULL, OVS_RO }, > }; Acked-by: Adrian Moreno <amorenoz@redhat.com>
On Wed, May 4, 2022 at 9:52 AM <numans@ovn.org> wrote: > > From: Numan Siddique <numans@ovn.org> > > ovsdb-server already supports specifying the uuid in the insert > transaction by the client. But the C IDL client library was > missing this feature. This patch adds this support. > > For each schema table, a new function is generated - > <schema_table>insert_persistent_uuid(txn, uuid) and the users > of IDL client library can make use of this function. > > Signed-off-by: Numan Siddique <numans@ovn.org> > --- > > v1 -> v2 > ----- > * Addressed review comments from Adrian Moreno > * Added the support in generic 'create' command to specify the uuid in > --id option. > > lib/db-ctl-base.c | 38 ++++++++++++------ > lib/db-ctl-base.man | 5 ++- > lib/db-ctl-base.xml | 4 ++ > lib/ovsdb-idl-provider.h | 1 + > lib/ovsdb-idl.c | 86 +++++++++++++++++++++++++++++----------- > lib/ovsdb-idl.h | 3 ++ > ovsdb/ovsdb-idlc.in | 15 +++++++ > tests/ovsdb-idl.at | 27 +++++++++++++ > tests/test-ovsdb.c | 59 +++++++++++++++++++++++++++ > 9 files changed, 202 insertions(+), 36 deletions(-) > > diff --git a/lib/db-ctl-base.c b/lib/db-ctl-base.c > index 707456158..f39e090a0 100644 > --- a/lib/db-ctl-base.c > +++ b/lib/db-ctl-base.c > @@ -1732,28 +1732,42 @@ cmd_create(struct ctl_context *ctx) > const struct ovsdb_idl_row *row; > const struct uuid *uuid = NULL; > int i; > + struct uuid uuid_; > + bool persist_uuid = false; > > 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 9c786f298..6899a1d86 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 or \fB\-\-id=\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 \fB@\fIuuid\fR is specified, then it is used as the UUID > +of the new row. A typo here: \fB@ should be removed. I didn't find any test for this option --id <uuid>. I think a simple test with ovs-vsctl should be good. With the above minor comment addressed: Acked-by: Han Zhou <hzhou@ovn.org> (sorry for the slow response)
diff --git a/lib/db-ctl-base.c b/lib/db-ctl-base.c index 707456158..f39e090a0 100644 --- a/lib/db-ctl-base.c +++ b/lib/db-ctl-base.c @@ -1732,28 +1732,42 @@ cmd_create(struct ctl_context *ctx) const struct ovsdb_idl_row *row; const struct uuid *uuid = NULL; int i; + struct uuid uuid_; + bool persist_uuid = false; 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 9c786f298..6899a1d86 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 or \fB\-\-id=\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 \fB@\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..4f63aa7f7 100644 --- a/lib/db-ctl-base.xml +++ b/lib/db-ctl-base.xml @@ -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..332c4075b 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 6fbc109cd..6646618ca 100644 --- a/lib/ovsdb-idl.c +++ b/lib/ovsdb-idl.c @@ -2838,11 +2838,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))); + } } } @@ -3267,9 +3270,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; @@ -3754,6 +3767,32 @@ 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); + + if (uuid) { + ovs_assert(!ovsdb_idl_txn_get_row(txn, uuid)); + row->uuid = *uuid; + row->persist_uuid = persist_uuid; + } else { + uuid_generate(&row->uuid); + row->persist_uuid = false; + } + + 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'. * @@ -3771,22 +3810,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 13c535939..7e09497ea 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/tests/ovsdb-idl.at b/tests/ovsdb-idl.at index d1cfa59c5..28c032105 100644 --- a/tests/ovsdb-idl.at +++ b/tests/ovsdb-idl.at @@ -2466,3 +2466,30 @@ unix:socket2 remote has col id in table simple7 OVSDB_SERVER_SHUTDOWN AT_CLEANUP + +AT_SETUP([idl creating rows with persistent uuid - C]) +AT_KEYWORDS([ovsdb client idl txn]) + +AT_CHECK([ovsdb_start_idltest "" "$abs_srcdir/idltest.ovsschema"]) +AT_CHECK([test-ovsdb '-vPATTERN:console:test-ovsdb|%c|%m' -vjsonrpc -t10 idl-txn-persistent-uuid unix:socket], + [0], [stdout], [stderr]) +AT_CHECK([sort stdout], [0], + [[000: After inserting simple, simple3 and simple4 +001: table simple3: name=simple3 uset=[] uref=[c5cc12f8-eaa1-43a7-8a73-bccd18df1111] uuid=c5cc12f8-eaa1-43a7-8a73-bccd18df2222 +001: table simple4: name=simple4 uuid=c5cc12f8-eaa1-43a7-8a73-bccd18df1111 +001: table simple: i=0 r=0 b=false s=simple u=00000000-0000-0000-0000-000000000000 ia=[] ra=[] ba=[] sa=[] ua=[] uuid=c5cc12f8-eaa1-43a7-8a73-bccd18df3333 +002: After inserting simple with same uuid +003: table simple3: name=simple3 uset=[] uref=[c5cc12f8-eaa1-43a7-8a73-bccd18df1111] uuid=c5cc12f8-eaa1-43a7-8a73-bccd18df2222 +003: table simple4: name=simple4 uuid=c5cc12f8-eaa1-43a7-8a73-bccd18df1111 +003: table simple: i=0 r=0 b=false s=simple u=00000000-0000-0000-0000-000000000000 ia=[] ra=[] ba=[] sa=[] ua=[] uuid=c5cc12f8-eaa1-43a7-8a73-bccd18df3333 +004: End test +]]) + +AT_CHECK([grep ovsdb_idl stderr | sort], [0], [dnl +test-ovsdb|ovsdb_idl|transaction error: {"details":"This UUID would dnl +duplicate a UUID already present within the table or deleted within dnl +the same transaction.","error":"duplicate uuid","syntax":"\"c5cc12f8-eaa1-43a7-8a73-bccd18df3333\""} +]) + +OVSDB_SERVER_SHUTDOWN +AT_CLEANUP diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c index 0fef1f978..515a3015d 100644 --- a/tests/test-ovsdb.c +++ b/tests/test-ovsdb.c @@ -3389,6 +3389,63 @@ do_idl_table_column_check(struct ovs_cmdl_context *ctx) ovsdb_idl_destroy(idl); } +static void +do_idl_txn_persistent_uuid(struct ovs_cmdl_context *ctx) +{ + struct ovsdb_idl *idl; + struct ovsdb_idl_txn *myTxn; + int step = 0; + + idl = ovsdb_idl_create(ctx->argv[1], &idltest_idl_class, true, true); + ovsdb_idl_get_initial_snapshot(idl); + ovsdb_idl_run(idl); + + myTxn = ovsdb_idl_txn_create(idl); + + struct uuid uuid1; + uuid_from_string(&uuid1, "c5cc12f8-eaa1-43a7-8a73-bccd18df1111"); + + struct uuid uuid2; + uuid_from_string(&uuid2, "c5cc12f8-eaa1-43a7-8a73-bccd18df2222"); + + struct idltest_simple4 *simple4_row = + idltest_simple4_insert_persist_uuid(myTxn, &uuid1); + idltest_simple4_set_name(simple4_row, "simple4"); + + struct idltest_simple3 *simple3_row = + idltest_simple3_insert_persist_uuid(myTxn, &uuid2); + idltest_simple3_set_name(simple3_row, "simple3"); + idltest_simple3_set_uref(simple3_row, &simple4_row, 1); + + struct uuid uuid3; + uuid_from_string(&uuid3, "c5cc12f8-eaa1-43a7-8a73-bccd18df3333"); + + struct idltest_simple *simple_row = + idltest_simple_insert_persist_uuid(myTxn, &uuid3); + idltest_simple_set_s(simple_row, "simple"); + + ovsdb_idl_txn_commit_block(myTxn); + ovsdb_idl_txn_destroy(myTxn); + ovsdb_idl_get_initial_snapshot(idl); + printf("%03d: After inserting simple, simple3 and simple4\n", step++); + print_idl(idl, step++, false); + + /* Create another txn, insert the row in simple table with the existing + * uuid. */ + myTxn = ovsdb_idl_txn_create(idl); + simple_row = + idltest_simple_insert_persist_uuid(myTxn, &uuid3); + idltest_simple_set_s(simple_row, "simple_foo"); + ovsdb_idl_txn_commit_block(myTxn); + ovsdb_idl_txn_destroy(myTxn); + ovsdb_idl_get_initial_snapshot(idl); + printf("%03d: After inserting simple with same uuid\n", step++); + print_idl(idl, step++, false); + + ovsdb_idl_destroy(idl); + printf("%03d: End test\n", step++); +} + static struct ovs_cmdl_command all_commands[] = { { "log-io", NULL, 2, INT_MAX, do_log_io, OVS_RO }, { "default-atoms", NULL, 0, 0, do_default_atoms, OVS_RO }, @@ -3429,6 +3486,8 @@ static struct ovs_cmdl_command all_commands[] = { do_idl_partial_update_set_column, OVS_RO }, { "idl-table-column-check", NULL, 2, 2, do_idl_table_column_check, OVS_RO }, + { "idl-txn-persistent-uuid", NULL, 1, INT_MAX, + do_idl_txn_persistent_uuid, OVS_RO }, { "help", NULL, 0, INT_MAX, do_help, OVS_RO }, { NULL, NULL, 0, 0, NULL, OVS_RO }, };