ovsdb: Force columns that contain weak references to be immutable.

An immutable weak reference is a hole in the constraint system: if referenced rows are deleted, then the weak reference needs to change. Therefore, force columsn that contain weak references to be mutable. Reported-by: "Elluru, Krishna Mohan" <[email protected]> Signed-off-by: Ben Pfaff <[email protected]> Acked-by: Ryan Moats <[email protected]>
grizax · Apr 12, 2016 · 3df3584 · 3df3584
1 parent c775160
commit 3df3584
Show file tree

Hide file tree

Showing 5 changed files with 47 additions and 11 deletions.
diff --git a/AUTHORS b/AUTHORS
@@ -346,6 +346,7 @@ Koichi Yagishita        [email protected]
 Konstantin Khorenko     [email protected]
 Kris zhang              [email protected]
 Krishna Miriyala        [email protected]
+Krishna Mohan Elluru    [email protected]
 Len Gao                 [email protected]
 Logan Rosen             [email protected]
 Luca Falavigna          [email protected]

diff --git a/ovsdb/column.c b/ovsdb/column.c
@@ -1,4 +1,4 @@
-/* Copyright (c) 2009, 2010, 2011 Nicira, Inc.
+/* Copyright (c) 2009, 2010, 2011, 2016 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -65,17 +65,16 @@ struct ovsdb_error *
 ovsdb_column_from_json(const struct json *json, const char *name,
                        struct ovsdb_column **columnp)
 {
-    const struct json *mutable, *ephemeral, *type_json;
+    const struct json *mutable_json, *ephemeral, *type_json;
     struct ovsdb_error *error;
     struct ovsdb_type type;
     struct ovsdb_parser parser;
-    bool persistent;
 
     *columnp = NULL;
 
     ovsdb_parser_init(&parser, json, "schema for column %s", name);
-    mutable = ovsdb_parser_member(&parser, "mutable",
-                                OP_TRUE | OP_FALSE | OP_OPTIONAL);
+    mutable_json = ovsdb_parser_member(&parser, "mutable",
+                                       OP_TRUE | OP_FALSE | OP_OPTIONAL);
     ephemeral = ovsdb_parser_member(&parser, "ephemeral",
                                     OP_TRUE | OP_FALSE | OP_OPTIONAL);
     type_json = ovsdb_parser_member(&parser, "type", OP_STRING | OP_OBJECT);
@@ -89,10 +88,17 @@ ovsdb_column_from_json(const struct json *json, const char *name,
         return error;
     }
 
-    persistent = ephemeral ? !json_boolean(ephemeral) : true;
-    *columnp = ovsdb_column_create(name,
-                                   mutable ? json_boolean(mutable) : true,
-                                   persistent, &type);
+    bool mutable = !mutable_json || json_boolean(mutable_json);
+    if (!mutable
+        && (ovsdb_base_type_is_weak_ref(&type.key) ||
+            ovsdb_base_type_is_weak_ref(&type.value))) {
+        /* We cannot allow a weak reference to be immutable: if referenced rows
+         * are deleted, then the weak reference needs to change. */
+        mutable = true;
+    }
+
+    bool persistent = ephemeral ? !json_boolean(ephemeral) : true;
+    *columnp = ovsdb_column_create(name, mutable, persistent, &type);
 
     ovsdb_type_destroy(&type);
 

diff --git a/ovsdb/ovsdb-server.1.in b/ovsdb/ovsdb-server.1.in
@@ -219,6 +219,12 @@ A map or set contains a duplicate key.
 RFC 7047 requires the "version" field in <database-schema>.  Current
 versions of \fBovsdb\-server\fR allow it to be omitted (future
 versions are likely to require it).
+.IP
+RFC 7047 allows columns that contain weak references to be immutable.
+This raises the issue of the behavior of the weak reference when the
+rows that it references are deleted.  Since version 2.6,
+\fBovsdb\-server\fR forces columns that contain weak references to be
+mutable.
 .
 .IP "4. Wire Protocol"
 The original OVSDB specifications included the following reason,

diff --git a/python/ovs/db/schema.py b/python/ovs/db/schema.py
@@ -1,4 +1,4 @@
-# Copyright (c) 2009, 2010, 2011 Nicira, Inc.
+# Copyright (c) 2009, 2010, 2011, 2016 Nicira, Inc.
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
@@ -265,6 +265,12 @@ def from_json(json, name):
         type_ = ovs.db.types.Type.from_json(parser.get("type", _types))
         parser.finish()
 
+        if not mutable and (type_.key.is_weak_ref()
+                            or (type_.value and type_.value.is_weak_ref())):
+            # We cannot allow a weak reference to be immutable: if referenced
+            # rows are deleted, then the weak reference needs to change.
+            mutable = True
+
         return ColumnSchema(name, mutable, not ephemeral, type_)
 
     def to_json(self):

diff --git a/tests/ovsdb-schema.at b/tests/ovsdb-schema.at
@@ -55,7 +55,24 @@ OVSDB_CHECK_POSITIVE_CPY([schema with ephemeral strong references],
                     "refTable": "a"}}}},
               "isRoot": true}}}']],
   [[{"name":"mydb","tables":{"a":{"columns":{"x":{"ephemeral":true,"type":{"key":{"refTable":"b","type":"uuid"}}},"y":{"type":{"key":{"refTable":"a","type":"uuid"}}}}},"b":{"columns":{"aRef":{"type":{"key":{"refTable":"a","type":"uuid"}}}},"isRoot":true}},"version":"4.2.1"}]])
-
+
+dnl Immutable weak references are forced to be mutable.
+OVSDB_CHECK_POSITIVE_CPY([schema with immutable weak references],
+  [[parse-schema \
+      '{"name": "mydb",
+	"version": "4.2.1",
+	"tables": {
+	  "a": {
+	    "columns": {
+	      "x": {
+		"type": {
+		  "key": {
+		    "type": "uuid",
+		    "refTable": "a",
+		    "refType": "weak"}},
+		"mutable": false}}}}}']],
+  [[{"name":"mydb","tables":{"a":{"columns":{"x":{"type":{"key":{"refTable":"a","refType":"weak","type":"uuid"}}}}}},"version":"4.2.1"}]])
+
 dnl Schemas without version numbers are accepted for backward
 dnl compatibility, but this is a deprecated feature.
 OVSDB_CHECK_POSITIVE_CPY([schema without version number],