On 9/26/22 19:52, num...@ovn.org wrote:
> 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>
> ---
> 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 | 2 +
> 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/TODO.rst | 2 +
> tests/ovs-vsctl.at | 25 ++++++++++++
> tests/ovsdb-idl.at | 27 +++++++++++++
> tests/test-ovsdb.c | 59 ++++++++++++++++++++++++++++
> 12 files changed, 231 insertions(+), 37 deletions(-)
>
> 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.
How hard it is to actually add the support?
it would be great to not increase the difference between
implementations. Sometimes we do not add features to
python idl, but that mostly happens when they depend on
some other missing features. UUID persistense seems to
not have any extra dependencies.
> 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 965b0f913..3edb2bc78 100644
> --- a/tests/test-ovsdb.c
> +++ b/tests/test-ovsdb.c
> @@ -3451,6 +3451,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++);
> +}
This test case is a bit worrying, because whenever we'll have
a python implementation, the test case will have to be
duplicated or fully re-written. Can we add a new command
to the 'transact' test instead, e.g. 'insert-with-uuid' and
write a generic test case for it?
> +
> 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 },
> @@ -3491,6 +3548,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 },
> };
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
