diff options
author | Daniel Alvarez Sanchez <dalvarez@redhat.com> | 2023-03-29 16:26:38 -0400 |
---|---|---|
committer | Ilya Maximets <i.maximets@ovn.org> | 2023-03-30 22:10:40 +0200 |
commit | daeab9548acc07fec839a46fd6bfd2a392bb91a0 (patch) | |
tree | d250ee8794b3e1115db019e54ceb7749bdfb5cab | |
parent | 0f34ecbd5a73b00f1dd8c6675c7be6fec6e094d4 (diff) | |
download | openvswitch-daeab9548acc07fec839a46fd6bfd2a392bb91a0.tar.gz |
db-ctl-base: Partially revert b8bf410a5.
The commit b8bf410a5 [0] broke the `ovs-vsctl add` command
which now overwrites the value if it existed already.
This patch reverts the code around the `cmd_add` function
to restore the previous behavior. It also adds testing coverage
for this functionality.
[0] https://github.com/openvswitch/ovs/commit/b8bf410a5c94173da02279b369d75875c4035959
Fixes: b8bf410a5c94 ("db-ctl-base: Use partial map/set updates for last add/set commands.")
Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2182767
Acked-by: Dumitru Ceara <dceara@redhat.com>
Signed-off-by: Daniel Alvarez Sanchez <dalvarez@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
-rw-r--r-- | lib/db-ctl-base.c | 42 | ||||
-rw-r--r-- | tests/ovs-vsctl.at | 8 |
2 files changed, 12 insertions, 38 deletions
diff --git a/lib/db-ctl-base.c b/lib/db-ctl-base.c index 134496ef3..5d2635946 100644 --- a/lib/db-ctl-base.c +++ b/lib/db-ctl-base.c @@ -1492,7 +1492,7 @@ cmd_add(struct ctl_context *ctx) const struct ovsdb_idl_column *column; const struct ovsdb_idl_row *row; const struct ovsdb_type *type; - struct ovsdb_datum new; + struct ovsdb_datum old; int i; ctx->error = get_table(table_name, &table); @@ -1516,13 +1516,7 @@ cmd_add(struct ctl_context *ctx) } type = &column->type; - - if (ctx->last_command) { - ovsdb_datum_init_empty(&new); - } else { - ovsdb_datum_clone(&new, ovsdb_idl_read(row, column)); - } - + ovsdb_datum_clone(&old, ovsdb_idl_read(row, column)); for (i = 4; i < ctx->argc; i++) { struct ovsdb_type add_type; struct ovsdb_datum add; @@ -1533,41 +1527,23 @@ cmd_add(struct ctl_context *ctx) ctx->error = ovsdb_datum_from_string(&add, &add_type, ctx->argv[i], ctx->symtab); if (ctx->error) { - ovsdb_datum_destroy(&new, &column->type); + ovsdb_datum_destroy(&old, &column->type); return; } - ovsdb_datum_union(&new, &add, type); + ovsdb_datum_union(&old, &add, type); ovsdb_datum_destroy(&add, type); } - - if (!ctx->last_command && new.n > type->n_max) { + if (old.n > type->n_max) { ctl_error(ctx, "\"add\" operation would put %u %s in column %s of " "table %s but the maximum number is %u", - new.n, + old.n, type->value.type == OVSDB_TYPE_VOID ? "values" : "pairs", column->name, table->name, type->n_max); - ovsdb_datum_destroy(&new, &column->type); + ovsdb_datum_destroy(&old, &column->type); return; } - - if (ctx->last_command) { - /* Partial updates can only be made one by one. */ - for (i = 0; i < new.n; i++) { - struct ovsdb_datum *datum = xmalloc(sizeof *datum); - - ovsdb_datum_init_empty(datum); - ovsdb_datum_add_from_index_unsafe(datum, &new, i, type); - if (ovsdb_type_is_map(type)) { - ovsdb_idl_txn_write_partial_map(row, column, datum); - } else { - ovsdb_idl_txn_write_partial_set(row, column, datum); - } - } - ovsdb_datum_destroy(&new, &column->type); - } else { - ovsdb_idl_txn_verify(row, column); - ovsdb_idl_txn_write(row, column, &new); - } + ovsdb_idl_txn_verify(row, column); + ovsdb_idl_txn_write(row, column, &old); invalidate_cache(ctx); } diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at index a92156f00..a368bff6e 100644 --- a/tests/ovs-vsctl.at +++ b/tests/ovs-vsctl.at @@ -425,6 +425,7 @@ AT_CHECK([RUN_OVS_VSCTL_ONELINE( [add-port a a1], [add-bond a bond0 a2 a3], [br-set-external-id a key0 value0], + [add Bridge a external_ids key0=value1], [set port a1 external-ids:key1=value1], [set interface a2 external-ids:key2=value2], [set interface a2 external-ids:key3=value3], @@ -446,6 +447,7 @@ AT_CHECK([RUN_OVS_VSCTL_ONELINE( + key0=value0 value0 @@ -1071,13 +1073,9 @@ AT_CHECK([RUN_OVS_VSCTL([set controller br1 'connection-mode=xyz'])], AT_CHECK([RUN_OVS_VSCTL([set controller br1 connection-mode:x=y])], [1], [], [ovs-vsctl: cannot specify key to set for non-map column connection_mode ]) -AT_CHECK([RUN_OVS_VSCTL([add bridge br1 datapath_id x y -- show])], +AT_CHECK([RUN_OVS_VSCTL([add bridge br1 datapath_id x y])], [1], [], [ovs-vsctl: "add" operation would put 2 values in column datapath_id of table Bridge but the maximum number is 1 ]) -AT_CHECK([RUN_OVS_VSCTL([add bridge br1 datapath_id x y])], [1], [], [stderr]) -AT_CHECK([sed "/^.*|WARN|.*/d" < stderr], [0], [dnl -ovs-vsctl: transaction error: {"details":"set must have 0 to 1 members but 2 are present","error":"syntax error","syntax":"[[\"set\",[\"x\",\"y\"]]]"} -]) AT_CHECK([RUN_OVS_VSCTL([remove netflow `cat netflow-uuid` targets '"1.2.3.4:567"'])], [1], [], [ovs-vsctl: "remove" operation would put 0 values in column targets of table NetFlow but the minimum number is 1 ]) |