On Sun, Nov 27, 2022 at 9:56 PM <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]. 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 <num...@ovn.org>
> Acked-by: Adrian Moreno <amore...@redhat.com>
> Acked-by: Han Zhou <hz...@ovn.org>
> CC: twil...@redhat.com
> CC: i.maxim...@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 <twil...@redhat.com>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
