From: Numan Siddique <num...@ovn.org>
ovsdb-server allows the OVSDB clients to specify the uuid for
the row inserts [1]. The C IDL client library is 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.
ovs-vsctl and other derivatives of ctl now supports the same
in the generic 'create' command with the option "--id=<UUID>".
[1] - a529e3cd1f("ovsdb-server: Allow OVSDB clients to specify the UUID for
inserted rows.:)
Signed-off-by: Numan Siddique <num...@ovn.org>
Acked-by: Adrian Moreno <amore...@redhat.com>
Acked-by: Han Zhou <hz...@ovn.org>
---
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.
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 +++++++
python/TODO.rst | 2 +
tests/ovs-vsctl.at | 25 ++++++++++++
tests/ovsdb-idl.at | 27 +++++++++++++
tests/test-ovsdb.c | 59 +++++++++++++++++++++++++++
11 files changed, 229 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..4dc165f94 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 \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 3b8741408..9ddb49273 100644
--- a/lib/ovsdb-idl.c
+++ b/lib/ovsdb-idl.c
@@ -2849,11 +2849,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)));
+ }
}
}
@@ -3278,9 +3281,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;
@@ -3765,6 +3778,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'.
*
@@ -3782,22 +3821,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/python/TODO.rst b/python/TODO.rst
index 3a53489f1..a0b3e807a 100644
--- a/python/TODO.rst
+++ b/python/TODO.rst
@@ -32,3 +32,5 @@ Python Bindings To-do List
* Support write-only-changed monitor mode (equivalent of
OVSDB_IDL_WRITE_CHANGED_ONLY).
+
+ * Support accepting the uuid for row inserts.
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 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 },
};
--
2.36.1
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
