diff mbox series

[ovs-dev,v3] db-ctl-base: Partially revert b8bf410a5

Message ID CAPXMuf-d0xHc_NYsrBgHdSWwPrQFAgg6DjU9qDSr7MU4bj+GNQ@mail.gmail.com
State Not Applicable
Headers show
Series [ovs-dev,v3] db-ctl-base: Partially revert b8bf410a5 | expand

Checks

Context Check Description
ovsrobot/apply-robot fail apply and check: fail
ovsrobot/intel-ovs-compilation fail test: fail

Commit Message

Daniel Alvarez Sanchez March 29, 2023, 8:07 p.m. UTC 
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: b8bf410a5 ("db-ctl-base: Use partial map/set updates for last
add/set commands")
Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2182767
Signed-off-by: Daniel Alvarez Sanchez <dalvarez@redhat.com>
---

Notes:
  * v2: fix failing test
  * v3: fix formatting and adding 'Fixes' to the commit message

 lib/db-ctl-base.c  | 42 +++++++++---------------------------------
 tests/ovs-vsctl.at |  8 +++-----
 2 files changed, 12 insertions(+), 38 deletions(-)

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
 ])
--
2.39.1
diff mbox series

Patch

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