ovs-vsctl: Allow modifying "immutable" columns if we just created the…

… row.

OVSDB has the concept of "immutable" columns, which are columns whose
values are fixed once a row is inserted.  Until now, ovs-vsctl has not
allowed these columns to be modified at all.  However, this is a little too
strict, because these columns can be set to any value at the time that the
row is inserted.  This commit relaxes the ovs-vsctl requirement, then, to
allow an immutable column's value to be modified if its row has been
inserted within this transaction.

Requested-by: Mukesh Hira <mhira@vmware.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>

NEWS

@@ -25,6 +25,8 @@ Post-v2.3.0
      it in OpenSSL.
    - ovsdb-server: New OVSDB protocol extension allows inequality tests on
      "optional scalar" columns.  See ovsdb-server(1) for details.
+   - ovs-vsctl now permits immutable columns in a new row to be modified in
+     the same transaction that creates the row.
    - test-controller has been renamed ovs-testcontroller at request of users
      who find it useful for testing basic OpenFlow setups.  It is still not
      a necessary or desirable part of most Open vSwitch deployments.

lib/ovsdb-idl.c

@@ -1,4 +1,4 @@
-/* Copyright (c) 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
+/* Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -1196,6 +1196,21 @@ ovsdb_idl_get(const struct ovsdb_idl_row *row,
     return ovsdb_idl_read(row, column);
 }
 
+/* Returns true if the field represented by 'column' in 'row' may be modified,
+ * false if it is immutable.
+ *
+ * Normally, whether a field is mutable is controlled by its column's schema.
+ * However, an immutable column can be set to any initial value at the time of
+ * insertion, so if 'row' is a new row (one that is being added as part of the
+ * current transaction, supposing that a transaction is in progress) then even
+ * its "immutable" fields are actually mutable. */
+bool
+ovsdb_idl_is_mutable(const struct ovsdb_idl_row *row,
+                     const struct ovsdb_idl_column *column)
+{
+    return column->mutable || (row->new && !row->old);
+}
+
 /* Returns false if 'row' was obtained from the IDL, true if it was initialized
  * to all-zero-bits by some other entity.  If 'row' was set up some other way
  * then the return value is indeterminate. */

lib/ovsdb-idl.h

@@ -1,4 +1,4 @@
-/* Copyright (c) 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
+/* Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -115,6 +115,8 @@ const struct ovsdb_datum *ovsdb_idl_get(const struct ovsdb_idl_row *,
                                         const struct ovsdb_idl_column *,
                                         enum ovsdb_atomic_type key_type,
                                         enum ovsdb_atomic_type value_type);
+bool ovsdb_idl_is_mutable(const struct ovsdb_idl_row *,
+                          const struct ovsdb_idl_column *);
 
 bool ovsdb_idl_row_is_synthetic(const struct ovsdb_idl_row *);
 

tests/ovs-vsctl.at

@@ -632,7 +632,8 @@ AT_SETUP([database commands -- positive checks])
 AT_KEYWORDS([ovs-vsctl])
 OVS_VSCTL_SETUP
 AT_CHECK(
-  [RUN_OVS_VSCTL_TOGETHER([--id=@br0 create bridge name=br0],
+  [RUN_OVS_VSCTL_TOGETHER([--id=@br0 create bridge name=br123],
+                          [set b br123 name=br0],
                           [set o . bridges=@br0])],
   [0], [stdout], [], [OVS_VSCTL_CLEANUP])
 cp stdout out1
@@ -642,6 +643,7 @@ cp stdout out2
 AT_CHECK([${PERL} $srcdir/uuidfilt.pl out1 out2], [0],
   [[<0>
 
+
 _uuid               : <0>
 controller          : []
 datapath_id         : []
@@ -847,10 +849,10 @@ AT_CHECK([RUN_OVS_VSCTL([destroy bridge br2])],
 AT_CHECK([RUN_OVS_VSCTL([add in br1 name x])],
   [1], [], [ovs-vsctl: cannot modify read-only column name in table Interface
 ], [OVS_VSCTL_CLEANUP])
-AT_CHECK([RUN_OVS_VSCTL([set port br1 name br2])],
+AT_CHECK([RUN_OVS_VSCTL([set port br0 name=br2])],
   [1], [], [ovs-vsctl: cannot modify read-only column name in table Port
 ], [OVS_VSCTL_CLEANUP])
-AT_CHECK([RUN_OVS_VSCTL([remove bridge br1 name br1])],
+AT_CHECK([RUN_OVS_VSCTL([remove bridge br0 name br1])],
   [1], [], [ovs-vsctl: cannot modify read-only column name in table Bridge
 ], [OVS_VSCTL_CLEANUP])
 AT_CHECK([RUN_OVS_VSCTL([remove bridge br1 flood-vlans true])],

utilities/ovs-vsctl.c

@@ -2987,12 +2987,12 @@ pre_parse_column_key_value(struct vsctl_context *ctx,
 }
 
 static void
-check_mutable(const struct vsctl_table_class *table,
+check_mutable(const struct ovsdb_idl_row *row,
               const struct ovsdb_idl_column *column)
 {
-    if (!column->mutable) {
+    if (!ovsdb_idl_is_mutable(row, column)) {
         vsctl_fatal("cannot modify read-only column %s in table %s",
-                    column->name, table->class->name);
+                    column->name, row->table->class->name);
     }
 }
 
@@ -3339,10 +3339,7 @@ pre_cmd_set(struct vsctl_context *ctx)
 
     table = pre_get_table(ctx, table_name);
     for (i = 3; i < ctx->argc; i++) {
-        const struct ovsdb_idl_column *column;
-
-        column = pre_parse_column_key_value(ctx, ctx->argv[i], table);
-        check_mutable(table, column);
+        pre_parse_column_key_value(ctx, ctx->argv[i], table);
     }
 }
 
@@ -3361,6 +3358,7 @@ set_column(const struct vsctl_table_class *table,
     if (!value_string) {
         vsctl_fatal("%s: missing value", arg);
     }
+    check_mutable(row, column);
 
     if (key_string) {
         union ovsdb_atom key, value;
@@ -3431,7 +3429,6 @@ pre_cmd_add(struct vsctl_context *ctx)
 
     table = pre_get_table(ctx, table_name);
     pre_get_column(ctx, table, column_name, &column);
-    check_mutable(table, column);
 }
 
 static void
@@ -3454,6 +3451,7 @@ cmd_add(struct vsctl_context *ctx)
     if (!row) {
         return;
     }
+    check_mutable(row, column);
 
     type = &column->type;
     ovsdb_datum_clone(&old, ovsdb_idl_read(row, column), &column->type);
@@ -3492,7 +3490,6 @@ pre_cmd_remove(struct vsctl_context *ctx)
 
     table = pre_get_table(ctx, table_name);
     pre_get_column(ctx, table, column_name, &column);
-    check_mutable(table, column);
 }
 
 static void
@@ -3515,6 +3512,7 @@ cmd_remove(struct vsctl_context *ctx)
     if (!row) {
         return;
     }
+    check_mutable(row, column);
 
     type = &column->type;
     ovsdb_datum_clone(&old, ovsdb_idl_read(row, column), &column->type);
@@ -3566,7 +3564,6 @@ pre_cmd_clear(struct vsctl_context *ctx)
         const struct ovsdb_idl_column *column;
 
         pre_get_column(ctx, table, ctx->argv[i], &column);
-        check_mutable(table, column);
     }
 }
 
@@ -3592,6 +3589,7 @@ cmd_clear(struct vsctl_context *ctx)
         struct ovsdb_datum datum;
 
         die_if_error(get_column(table, ctx->argv[i], &column));
+        check_mutable(row, column);
 
         type = &column->type;
         if (type->n_min > 0) {