ovsdb: Remove "comment" support from OVSDB schemas.

Using a separate XML file to document a schema is much more flexible.
You end up with two files (a schema and documentation for it), each of
which is readable and maintainable, instead of a single schema file that
is almost illegible.

15 files changed, 57 insertions, 200 deletions

diff --git a/ovsdb/OVSDB.py b/ovsdb/OVSDB.py
index f40af1e9d..5297229ff 100644
--- a/ovsdb/OVSDB.py
+++ b/ovsdb/OVSDB.py
@@ -21,25 +21,23 @@ def mustGetMember(json, name, expectedType, description):
     return member
 
 class DbSchema:
-    def __init__(self, name, comment, tables):
+    def __init__(self, name, tables):
         self.name = name
-        self.comment = comment
         self.tables = tables
 
     @staticmethod
     def fromJson(json):
         name = mustGetMember(json, 'name', [unicode], 'database')
-        comment = getMember(json, 'comment', [unicode], 'database')
         tablesJson = mustGetMember(json, 'tables', [dict], 'database')
         tables = {}
         for tableName, tableJson in tablesJson.iteritems():
             tables[tableName] = TableSchema.fromJson(tableJson,
                                                      "%s table" % tableName)
-        return DbSchema(name, comment, tables)
+        return DbSchema(name, tables)
 
 class IdlSchema(DbSchema):
-    def __init__(self, name, comment, tables, idlPrefix, idlHeader):
-        DbSchema.__init__(self, name, comment, tables)
+    def __init__(self, name, tables, idlPrefix, idlHeader):
+        DbSchema.__init__(self, name, tables)
         self.idlPrefix = idlPrefix
         self.idlHeader = idlHeader
 
@@ -48,39 +46,34 @@ class IdlSchema(DbSchema):
         schema = DbSchema.fromJson(json)
         idlPrefix = mustGetMember(json, 'idlPrefix', [unicode], 'database')
         idlHeader = mustGetMember(json, 'idlHeader', [unicode], 'database')
-        return IdlSchema(schema.name, schema.comment, schema.tables,
-                         idlPrefix, idlHeader)
+        return IdlSchema(schema.name, schema.tables, idlPrefix, idlHeader)
 
 class TableSchema:
-    def __init__(self, comment, columns):
-        self.comment = comment
+    def __init__(self, columns):
         self.columns = columns
 
     @staticmethod
     def fromJson(json, description):
-        comment = getMember(json, 'comment', [unicode], description)
         columnsJson = mustGetMember(json, 'columns', [dict], description)
         columns = {}
         for name, json in columnsJson.iteritems():
             columns[name] = ColumnSchema.fromJson(
                 json, "column %s in %s" % (name, description))
-        return TableSchema(comment, columns)
+        return TableSchema(columns)
 
 class ColumnSchema:
-    def __init__(self, comment, type, persistent):
-        self.comment = comment
+    def __init__(self, type, persistent):
         self.type = type
         self.persistent = persistent
 
     @staticmethod
     def fromJson(json, description):
-        comment = getMember(json, 'comment', [unicode], description)
         type = Type.fromJson(mustGetMember(json, 'type', [dict, unicode],
                                            description),
                              'type of %s' % description)
         ephemeral = getMember(json, 'ephemeral', [bool], description)
         persistent = ephemeral != True
-        return ColumnSchema(comment, type, persistent)
+        return ColumnSchema(type, persistent)
 
 def escapeCString(src):
     dst = ""
diff --git a/ovsdb/SPECS b/ovsdb/SPECS
index e5cc21b92..db504172d 100644
--- a/ovsdb/SPECS
+++ b/ovsdb/SPECS
@@ -89,25 +89,21 @@ is represented by <database-schema>, as described below.
     A JSON object with the following members:
 
         "name": <id>                            required
-        "comment": <string>                     optional
         "tables": {<id>: <table-schema>, ...}   required
 
     The "name" identifies the database as a whole.  It must be
     provided to most JSON-RPC requests to identify the database being
-    operated on.  The "comment" optionally provides more information
-    about the database.  The value of "tables" is a JSON object whose
-    names are table names and whose values are <table-schema>s.
+    operated on.  The value of "tables" is a JSON object whose names
+    are table names and whose values are <table-schema>s.
 
 <table-schema>
 
     A JSON object with the following members:
 
-        "comment": <string>                       optional
         "columns": {<id>: <column-schema>, ...}   required
 
-    The "comment" optionally provides information about this table for
-    a human reader.  The value of "columns" is a JSON object whose
-    names are column names and whose values are <column-schema>s.
+    The value of "columns" is a JSON object whose names are column
+    names and whose values are <column-schema>s.
 
     Every table has the following columns whose definitions are not
     included in the schema:
@@ -130,15 +126,13 @@ is represented by <database-schema>, as described below.
 
     A JSON object with the following members:
 
-        "comment": <string>                       optional
         "type": <type>                            required
         "ephemeral": <boolean>                    optional
 
-    The "comment" optionally provides information about this column
-    for a human reader.  The "type" specifies the type of data stored
-    in this column.  If "ephemeral" is specified as true, then this
-    column's values are not guaranteed to be durable; they may be lost
-    when the database restarts.
+    The "type" specifies the type of data stored in this column.  If
+    "ephemeral" is specified as true, then this column's values are
+    not guaranteed to be durable; they may be lost when the database
+    restarts.
 
 <type>
 
diff --git a/ovsdb/column.c b/ovsdb/column.c
index 58fff1084..a22e1a237 100644
--- a/ovsdb/column.c
+++ b/ovsdb/column.c
@@ -27,7 +27,7 @@
 #include "util.h"
 
 struct ovsdb_column *
-ovsdb_column_create(const char *name, const char *comment,
+ovsdb_column_create(const char *name,
                     bool mutable, bool persistent,
                     const struct ovsdb_type *type)
 {
@@ -36,7 +36,6 @@ ovsdb_column_create(const char *name, const char *comment,
 
     column = xzalloc(sizeof *column);
     column->name = xstrdup(name);
-    column->comment = comment ? xstrdup(comment) : NULL;
     column->mutable = mutable;
     column->persistent = persistent;
     ovsdb_type_clone(&column->type, type);
@@ -48,7 +47,7 @@ struct ovsdb_column *
 ovsdb_column_clone(const struct ovsdb_column *old)
 {
     /* Doesn't copy the column's 'index': the caller must do that. */
-    return ovsdb_column_create(old->name, old->comment,
+    return ovsdb_column_create(old->name,
                                old->mutable, old->persistent,
                                &old->type);
 }
@@ -58,7 +57,6 @@ ovsdb_column_destroy(struct ovsdb_column *column)
 {
     ovsdb_type_destroy(&column->type);
     free(column->name);
-    free(column->comment);
     free(column);
 }
 
@@ -66,7 +64,7 @@ struct ovsdb_error *
 ovsdb_column_from_json(const struct json *json, const char *name,
                        struct ovsdb_column **columnp)
 {
-    const struct json *comment, *mutable, *ephemeral, *type_json;
+    const struct json *mutable, *ephemeral, *type_json;
     struct ovsdb_error *error;
     struct ovsdb_type type;
     struct ovsdb_parser parser;
@@ -75,7 +73,6 @@ ovsdb_column_from_json(const struct json *json, const char *name,
     *columnp = NULL;
 
     ovsdb_parser_init(&parser, json, "schema for column %s", name);
-    comment = ovsdb_parser_member(&parser, "comment", OP_STRING | OP_OPTIONAL);
     mutable = ovsdb_parser_member(&parser, "mutable",
                                 OP_TRUE | OP_FALSE | OP_OPTIONAL);
     ephemeral = ovsdb_parser_member(&parser, "ephemeral",
@@ -93,7 +90,6 @@ ovsdb_column_from_json(const struct json *json, const char *name,
 
     persistent = ephemeral ? !json_boolean(ephemeral) : true;
     *columnp = ovsdb_column_create(name,
-                                   comment ? json_string(comment) : NULL,
                                    mutable ? json_boolean(mutable) : true,
                                    persistent, &type);
 
@@ -106,9 +102,6 @@ struct json *
 ovsdb_column_to_json(const struct ovsdb_column *column)
 {
     struct json *json = json_object_create();
-    if (column->comment) {
-        json_object_put_string(json, "comment", column->comment);
-    }
     if (!column->mutable) {
         json_object_put(json, "mutable", json_boolean_create(false));
     }
diff --git a/ovsdb/column.h b/ovsdb/column.h
index eb1a3834a..b6f324c0f 100644
--- a/ovsdb/column.h
+++ b/ovsdb/column.h
@@ -27,7 +27,6 @@ struct ovsdb_column {
     unsigned int index;
     char *name;
 
-    char *comment;
     bool mutable;
     bool persistent;
     struct ovsdb_type type;
@@ -45,7 +44,7 @@ enum {
 };
 
 struct ovsdb_column *ovsdb_column_create(
-    const char *name, const char *comment, bool mutable, bool persistent,
+    const char *name, bool mutable, bool persistent,
     const struct ovsdb_type *);
 struct ovsdb_column *ovsdb_column_clone(const struct ovsdb_column *);
 void ovsdb_column_destroy(struct ovsdb_column *);
diff --git a/ovsdb/ovsdb-client.1.in b/ovsdb/ovsdb-client.1.in
index 87f5926f4..338faebd3 100644
--- a/ovsdb/ovsdb-client.1.in
+++ b/ovsdb/ovsdb-client.1.in
@@ -61,12 +61,12 @@ prints it in JSON format.
 .
 .IP "\fBlist-tables\fI server database\fR"
 Connects to \fIserver\fR, retrieves the schema for \fIdatabase\fR, and
-prints a table listing the names and comments (if any) on each table
+prints a table listing the name of each table
 within the database.
 .
 .IP "\fBlist-columns\fI server database \fR[\fItable\fR]"
 Connects to \fIserver\fR, retrieves the schema for \fIdatabase\fR, and
-prints a table listing the names, type, and comment (if any) on each
+prints a table listing the name and type of each
 column.  If \fItable\fR is specified, only columns in that table are
 listed; otherwise, the tables include columns in all tables.
 .
diff --git a/ovsdb/ovsdb-client.c b/ovsdb/ovsdb-client.c
index 9dd809435..da7faf61a 100644
--- a/ovsdb/ovsdb-client.c
+++ b/ovsdb/ovsdb-client.c
@@ -695,15 +695,11 @@ do_list_tables(int argc OVS_UNUSED, char *argv[])
     schema = fetch_schema(argv[1], argv[2]);
     table_init(&t);
     table_add_column(&t, "Table");
-    table_add_column(&t, "Comment");
     SHASH_FOR_EACH (node, &schema->tables) {
         struct ovsdb_table_schema *ts = node->data;
 
         table_add_row(&t);
         table_add_cell(&t, ts->name);
-        if (ts->comment) {
-            table_add_cell(&t, ts->comment);
-        }
     }
     ovsdb_schema_destroy(schema);
     table_print(&t);
@@ -724,7 +720,6 @@ do_list_columns(int argc OVS_UNUSED, char *argv[])
     }
     table_add_column(&t, "Column");
     table_add_column(&t, "Type");
-    table_add_column(&t, "Comment");
     SHASH_FOR_EACH (table_node, &schema->tables) {
         struct ovsdb_table_schema *ts = table_node->data;
 
@@ -741,9 +736,6 @@ do_list_columns(int argc OVS_UNUSED, char *argv[])
                 }
                 table_add_cell(&t, column->name);
                 table_add_cell_nocopy(&t, json_to_string(type, JSSF_SORT));
-                if (column->comment) {
-                    table_add_cell(&t, column->comment);
-                }
 
                 json_destroy(type);
             }
diff --git a/ovsdb/ovsdb-idlc.in b/ovsdb/ovsdb-idlc.in
index e99917a89..6a0303dbb 100755
--- a/ovsdb/ovsdb-idlc.in
+++ b/ovsdb/ovsdb-idlc.in
@@ -231,10 +231,7 @@ static struct %(s)s *
     for tableName, table in sorted(schema.tables.iteritems()):
         structName = "%s%s" % (prefix, tableName.lower())
         print ""
-        if table.comment != None:
-            print "/* %s table (%s). */" % (tableName, table.comment)
-        else:
-            print "/* %s table. */" % (tableName)
+        print "/* %s table. */" % (tableName)
 
         # Parse functions.
         for columnName, column in sorted(table.columns.iteritems()):
diff --git a/ovsdb/ovsdb.c b/ovsdb/ovsdb.c
index b5f6edb12..5bbaddbd3 100644
--- a/ovsdb/ovsdb.c
+++ b/ovsdb/ovsdb.c
@@ -26,13 +26,12 @@
 #include "transaction.h"
 
 struct ovsdb_schema *
-ovsdb_schema_create(const char *name, const char *comment)
+ovsdb_schema_create(const char *name)
 {
     struct ovsdb_schema *schema;
 
     schema = xzalloc(sizeof *schema);
     schema->name = xstrdup(name);
-    schema->comment = comment ? xstrdup(comment) : NULL;
     shash_init(&schema->tables);
 
     return schema;
@@ -44,7 +43,7 @@ ovsdb_schema_clone(const struct ovsdb_schema *old)
     struct ovsdb_schema *new;
     struct shash_node *node;
 
-    new = ovsdb_schema_create(old->name, old->comment);
+    new = ovsdb_schema_create(old->name);
     SHASH_FOR_EACH (node, &old->tables) {
         const struct ovsdb_table_schema *ts = node->data;
 
@@ -63,7 +62,6 @@ ovsdb_schema_destroy(struct ovsdb_schema *schema)
         ovsdb_table_schema_destroy(node->data);
     }
     shash_destroy(&schema->tables);
-    free(schema->comment);
     free(schema->name);
     free(schema);
 }
@@ -118,7 +116,7 @@ struct ovsdb_error *
 ovsdb_schema_from_json(struct json *json, struct ovsdb_schema **schemap)
 {
     struct ovsdb_schema *schema;
-    const struct json *name, *comment, *tables;
+    const struct json *name, *tables;
     struct ovsdb_error *error;
     struct shash_node *node;
     struct ovsdb_parser parser;
@@ -127,15 +125,13 @@ ovsdb_schema_from_json(struct json *json, struct ovsdb_schema **schemap)
 
     ovsdb_parser_init(&parser, json, "database schema");
     name = ovsdb_parser_member(&parser, "name", OP_ID);
-    comment = ovsdb_parser_member(&parser, "comment", OP_STRING | OP_OPTIONAL);
     tables = ovsdb_parser_member(&parser, "tables", OP_OBJECT);
     error = ovsdb_parser_finish(&parser);
     if (error) {
         return error;
     }
 
-    schema = ovsdb_schema_create(json_string(name),
-                                 comment ? json_string(comment) : NULL);
+    schema = ovsdb_schema_create(json_string(name));
     SHASH_FOR_EACH (node, json_object(tables)) {
         struct ovsdb_table_schema *table;
 
@@ -190,9 +186,6 @@ ovsdb_schema_to_json(const struct ovsdb_schema *schema)
 
     json = json_object_create();
     json_object_put_string(json, "name", schema->name);
-    if (schema->comment) {
-        json_object_put_string(json, "comment", schema->comment);
-    }
 
     tables = json_object_create();
 
diff --git a/ovsdb/ovsdb.h b/ovsdb/ovsdb.h
index 9961620ac..a83412dcd 100644
--- a/ovsdb/ovsdb.h
+++ b/ovsdb/ovsdb.h
@@ -29,12 +29,10 @@ struct uuid;
 /* Database schema. */
 struct ovsdb_schema {
     char *name;
-    char *comment;
     struct shash tables;        /* Contains "struct ovsdb_table_schema *"s. */
 };
 
-struct ovsdb_schema *ovsdb_schema_create(const char *name,
-                                         const char *comment);
+struct ovsdb_schema *ovsdb_schema_create(const char *name);
 struct ovsdb_schema *ovsdb_schema_clone(const struct ovsdb_schema *);
 void ovsdb_schema_destroy(struct ovsdb_schema *);
 
diff --git a/ovsdb/table.c b/ovsdb/table.c
index 7ba47eb01..3f35b86e5 100644
--- a/ovsdb/table.c
+++ b/ovsdb/table.c
@@ -35,26 +35,21 @@ add_column(struct ovsdb_table_schema *ts, struct ovsdb_column *column)
 }
 
 struct ovsdb_table_schema *
-ovsdb_table_schema_create(const char *name, const char *comment, bool mutable)
+ovsdb_table_schema_create(const char *name, bool mutable)
 {
     struct ovsdb_column *uuid, *version;
     struct ovsdb_table_schema *ts;
 
     ts = xzalloc(sizeof *ts);
     ts->name = xstrdup(name);
-    ts->comment = comment ? xstrdup(comment) : NULL;
     ts->mutable = mutable;
     shash_init(&ts->columns);
 
-    uuid = ovsdb_column_create(
-        "_uuid", "Unique identifier for this row.",
-        false, true, &ovsdb_type_uuid);
+    uuid = ovsdb_column_create("_uuid", false, true, &ovsdb_type_uuid);
     add_column(ts, uuid);
     assert(uuid->index == OVSDB_COL_UUID);
 
-    version = ovsdb_column_create(
-        "_version", "Unique identifier for this version of this row.",
-        false, false, &ovsdb_type_uuid);
+    version = ovsdb_column_create("_version", false, false, &ovsdb_type_uuid);
     add_column(ts, version);
     assert(version->index == OVSDB_COL_VERSION);
 
@@ -67,7 +62,7 @@ ovsdb_table_schema_clone(const struct ovsdb_table_schema *old)
     struct ovsdb_table_schema *new;
     struct shash_node *node;
 
-    new = ovsdb_table_schema_create(old->name, old->comment, old->mutable);
+    new = ovsdb_table_schema_create(old->name, old->mutable);
     SHASH_FOR_EACH (node, &old->columns) {
         const struct ovsdb_column *column = node->data;
 
@@ -90,7 +85,6 @@ ovsdb_table_schema_destroy(struct ovsdb_table_schema *ts)
         ovsdb_column_destroy(node->data);
     }
     shash_destroy(&ts->columns);
-    free(ts->comment);
     free(ts->name);
     free(ts);
 }
@@ -100,7 +94,7 @@ ovsdb_table_schema_from_json(const struct json *json, const char *name,
                              struct ovsdb_table_schema **tsp)
 {
     struct ovsdb_table_schema *ts;
-    const struct json *comment, *columns, *mutable;
+    const struct json *columns, *mutable;
     struct shash_node *node;
     struct ovsdb_parser parser;
     struct ovsdb_error *error;
@@ -108,7 +102,6 @@ ovsdb_table_schema_from_json(const struct json *json, const char *name,
     *tsp = NULL;
 
     ovsdb_parser_init(&parser, json, "table schema for table %s", name);
-    comment = ovsdb_parser_member(&parser, "comment", OP_STRING | OP_OPTIONAL);
     columns = ovsdb_parser_member(&parser, "columns", OP_OBJECT);
     mutable = ovsdb_parser_member(&parser, "mutable",
                                   OP_TRUE | OP_FALSE | OP_OPTIONAL);
@@ -123,7 +116,6 @@ ovsdb_table_schema_from_json(const struct json *json, const char *name,
     }
 
     ts = ovsdb_table_schema_create(name,
-                                   comment ? json_string(comment) : NULL,
                                    mutable ? json_boolean(mutable) : true);
     SHASH_FOR_EACH (node, json_object(columns)) {
         struct ovsdb_column *column;
@@ -154,9 +146,6 @@ ovsdb_table_schema_to_json(const struct ovsdb_table_schema *ts)
     struct shash_node *node;
 
     json = json_object_create();
-    if (ts->comment) {
-        json_object_put_string(json, "comment", ts->comment);
-    }
     if (!ts->mutable) {
         json_object_put(json, "mutable", json_boolean_create(false));
     }
diff --git a/ovsdb/table.h b/ovsdb/table.h
index 67784e93c..ff99cf17c 100644
--- a/ovsdb/table.h
+++ b/ovsdb/table.h
@@ -27,13 +27,11 @@ struct uuid;
 /* Schema for a database table. */
 struct ovsdb_table_schema {
     char *name;
-    char *comment;
     bool mutable;
     struct shash columns;       /* Contains "struct ovsdb_column *"s. */
 };
 
 struct ovsdb_table_schema *ovsdb_table_schema_create(const char *name,
-                                                     const char *comment,
                                                      bool mutable);
 struct ovsdb_table_schema *ovsdb_table_schema_clone(
     const struct ovsdb_table_schema *);
diff --git a/tests/ovsdb-column.at b/tests/ovsdb-column.at
index 03cd8dc69..7dd55e413 100644
--- a/tests/ovsdb-column.at
+++ b/tests/ovsdb-column.at
@@ -11,8 +11,3 @@ OVSDB_CHECK_POSITIVE([immutable column],
 OVSDB_CHECK_POSITIVE([ephemeral column],
   [[parse-column mycol '{"type": "uuid", "ephemeral": true}']],
   [[{"ephemeral":true,"type":"uuid"}]])
-
-OVSDB_CHECK_POSITIVE([column with comment],
-  [[parse-column mycol '{"type": "boolean",
-                         "comment": "extra information about this column"}']],
-  [[{"comment":"extra information about this column","type":"boolean"}]])
diff --git a/tests/ovsdb-table.at b/tests/ovsdb-table.at
index ebc5992d7..25f5dddf3 100644
--- a/tests/ovsdb-table.at
+++ b/tests/ovsdb-table.at
@@ -10,12 +10,6 @@ OVSDB_CHECK_POSITIVE([immutable table with one column],
       "mutable": false}']],
   [[{"columns":{"name":{"type":"string"}},"mutable":false}]])
 
-OVSDB_CHECK_POSITIVE([table with comment],
-  [[parse-table mytable \
-    '{"columns": {"name": {"type": "string"}},
-      "comment": "description of table"}']],
-  [[{"columns":{"name":{"type":"string"}},"comment":"description of table"}]])
-
 OVSDB_CHECK_NEGATIVE([column names may not begin with _],
   [[parse-table mytable \
     '{"columns": {"_column": {"type": "integer"}}}']],
diff --git a/vswitchd/vswitch.ovsschema b/vswitchd/vswitch.ovsschema
index ac42edfcb..3a2038540 100644
--- a/vswitchd/vswitch.ovsschema
+++ b/vswitchd/vswitch.ovsschema
@@ -1,298 +1,220 @@
 {"name": "Open_vSwitch",
- "comment": "Configuration for one Open vSwitch daemon.",
  "tables": {
    "Open_vSwitch": {
-     "comment": "Configuration for an Open vSwitch daemon.",
      "columns": {
        "bridges": {
-         "comment": "Set of bridges managed by the daemon.",
          "type": {"key": {"type": "uuid",
                           "refTable": "Bridge"},
                   "min": 0, "max": "unlimited"}},
        "controller": {
-         "comment": "Default Controller used by bridges.",
          "type": {"key": {"type": "uuid",
                           "refTable": "Controller"},
                    "min": 0, "max": 1}},
        "managers": {
-         "comment": "Remote database clients to which the Open vSwitch's database server should connect or to which it should listen.",
          "type": {"key": "string", "min": 0, "max": "unlimited"}},
        "ssl": {
-         "comment": "SSL used globally by the daemon.",
          "type": {"key": {"type": "uuid",
                           "refTable": "SSL"},
                   "min": 0, "max": 1}},
        "next_cfg": {
-         "comment": "Sequence number for client to increment when it modifies the configuration and wishes to wait for Open vSwitch to finish applying the changes.",
          "type": "integer"},
        "cur_cfg": {
-         "comment": "Sequence number that Open vSwitch sets to the current value of 'next_cfg' after it finishing applying a set of configuration changes.",
          "type": "integer"}}},
    "Bridge": {
-     "comment": "Configuration for a bridge within an Open_vSwitch.",
      "columns": {
        "name": {
-         "comment": "Bridge identifier.  Should be alphanumeric and no more than about 8 bytes long.  Must be unique among the names of ports, interfaces, and bridges on a host.",
          "type": "string"},
        "datapath_type": {
-         "comment": "Name of datapath provider.  The kernel datapath has type \"system\".  The userspace datapath has type \"netdev\".",
          "type": "string"},
        "datapath_id": {
-         "comment": "Reports the OpenFlow datapath ID in use.  Exactly 12 hex digits.",
          "type": {"key": "string", "min": 0, "max": 1},
          "ephemeral": true},
        "ports": {
-         "comment": "Ports included in the bridge.",
          "type": {"key": {"type": "uuid",
                           "refTable": "Port"},
                   "min": 0, "max": "unlimited"}},
        "mirrors": {
-         "comment": "Port mirroring configuration.",
          "type": {"key": {"type": "uuid",
                           "refTable": "Mirror"},
                   "min": 0, "max": "unlimited"}},
        "netflow": {
-         "comment": "NetFlow configuration.",
          "type": {"key": {"type": "uuid",
                           "refTable": "NetFlow"},
                   "min": 0, "max": 1}},
        "sflow": {
-         "comment": "sFlow configuration.",
          "type": {"key": {"type": "uuid",
                           "refTable": "sFlow"},
                   "min": 0, "max": 1}},
        "controller": {
-         "comment": "OpenFlow controller.  If unset, defaults to that specified by the parent Open_vSwitch.",
          "type": {"key": {"type": "uuid",
                           "refTable": "Controller"},
                   "min": 0, "max": 1}},
        "other_config": {
-         "comment": "Key-value pairs for configuring rarely used bridge features.  The currently defined key-value pairs are: \"datapath-id\", exactly 12 hex digits to set the OpenFlow datapath ID to a specific value; \"hwaddr\", exactly 12 hex digits in the form \"XX:XX:XX:XX:XX:XX\" to set the hardware address of the local port and influence the datapath ID.",
          "type": {"key": "string", "value": "string", "min": 0, "max": "unlimited"}},
        "external_ids": {
-         "comment": "Key-value pairs that identify this bridge's role in external systems.  The currently defined key-value pairs are: \"xs-network-uuids\", a space-delimited set of the Citrix XenServer network UUIDs with which this bridge is associated; \"xs-network-names\", a semicolon-delimited set of Citrix XenServer network names with which this bridge is associated.",
          "type": {"key": "string", "value": "string", "min": 0, "max": "unlimited"}},
        "flood_vlans": {
-         "comment": "VLAN IDs of VLANs on which MAC address learning should be disabled, so that packets are flooded instead of being sent to specific ports that are believed to contain packets' destination MACs.  This should ordinarily be used to disable MAC learning on VLANs used for mirroring (RSPAN VLANs).  It may also be useful for debugging.",
          "type": {"key": {"type": "integer",
                           "minInteger": 0, 
                           "maxInteger": 4095},
                   "min": 0, "max": 4096}}}},
    "Port": {
-     "comment": "A port within a Bridge.  May contain a single Interface or multiple (bonded) Interfaces.",
      "columns": {
        "name": {
-         "comment": "Port name.  Should be alphanumeric and no more than about 8 bytes long.    May be the same as the interface name, for non-bonded ports.  Must otherwise be unique among the names of ports, interfaces, and bridges on a host.",
          "type": "string"},
        "interfaces": {
-         "comment": "The Port's Interfaces.  If there is more than one, this is a bonded Port.",
          "type": {"key": {"type": "uuid",
                           "refTable": "Interface"},
                   "min": 1, "max": "unlimited"}},
        "trunks": {
-         "comment": "The 802.1Q VLAN(s) that this port trunks.  Should be empty if this port trunks all VLAN(s) or if this is not a trunk port.",
          "type": {"key": {"type": "integer",
                           "minInteger": 0,
                           "maxInteger": 4095},
                   "min": 0, "max": 4096}},
        "tag": {
-         "comment": "This port's implicitly tagged VLAN.  Should be empty if this is a trunk port.",
          "type": {"key": {"type": "integer",
                           "minInteger": 0,
                           "maxInteger": 4095},
                   "min": 0, "max": 1}},
        "mac": {
-         "comment": "The MAC address to use for this port for the purpose of choosing the bridge's MAC address.  This column does not necessarily reflect the port's actual MAC address, nor will setting it change the port's actual MAC address.",
          "type": {"key": {"type": "string"},
                   "min": 0, "max": 1}},
        "bond_updelay": {
-         "comment": "For a bonded port, the number of milliseconds for which carrier must stay up on an interface before the interface is considered to be up.  Ignored for non-bonded ports.",
          "type": "integer"},
        "bond_downdelay": {
-         "comment": "For a bonded port, the number of milliseconds for which carrier must stay down on an interface before the interface is considered to be down.  Ignored for non-bonded ports.",
          "type": "integer"},
        "bond_fake_iface": {
-         "comment": "For a bonded port, whether to create a fake interface with the name of the port.  Use only for compatibility with legacy software that requires this.",
          "type": "boolean"},
        "fake_bridge": {
-         "comment": "Does this port represent a sub-bridge for its tagged VLAN within the Bridge?  See ovs-vsctl(8) for more information.",
          "type": "boolean"},
        "other_config": {
-         "comment": "Key-value pairs for configuring rarely used port features.  The currently defined key-value pairs are: \"hwaddr\", exactly 12 hex digits in the form \"XX:XX:XX:XX:XX:XX\".",
          "type": {"key": "string", "value": "string", "min": 0, "max": "unlimited"}},
        "external_ids": {
-         "comment": "Key-value pairs that identify this port's role in external systems.  No key-value pairs native to Port are currently defined.  For fake bridges (see the \"fake-bridge\" column), external IDs for the fake bridge are defined here by prefixing their keys with \"fake-bridge\", e.g. \"fake-bridge-xs-network-uuids\".",
          "type": {"key": "string", "value": "string", "min": 0, "max": "unlimited"}}}},
    "Interface": {
-     "comment": "An interface within a Port.",
      "columns": {
        "name": {
-         "comment": "Interface name.  Should be alphanumeric and no more than about 8 bytes long.  May be the same as the port name, for non-bonded ports.  Must otherwise be unique among the names of ports, interfaces, and bridges on a host.",
          "type": "string"},
        "type": {
-         "comment": "The interface type.  Normal network devices, e.g.  eth0, have type \"system\" or \"\" (which are synonyms).  Internal ports have type \"internal\".  TUN/TAP devices have type \"tap\".  GRE devices have type \"gre\".",
          "type": "string"},
        "options": {
-         "comment": "Configuration options whose interpretation varies based on \"type\".",
          "type": {"key": "string", "value": "string", "min": 0, "max": "unlimited"}},
        "ingress_policing_rate": {
-         "comment": "Maximum rate for data received on this interface, in kbps.  Set to 0 to disable policing.",
          "type": {"key": {"type": "integer",
                           "minInteger": 0}}},
        "ingress_policing_burst": {
-         "comment": "Maximum burst size for data received on this interface, in kb.  The default burst size if set to 0 is 1000 kb.",
          "type": {"key": {"type": "integer",
                           "minInteger": 0}}},
        "mac": {
-         "comment": "Ethernet address to set for this interface.  If unset then the default MAC address is used.  May not be supported on all interfaces.",
          "type": {"key": {"type": "string"},
                   "min": 0, "max": 1}},
        "external_ids": {
-         "comment": "Key-value pairs that identify this interface's role in external systems.  The currently defined key-value pairs are: \"xs-vif-uuid\", the UUID of the Citrix XenServer VIF associated with this interface; \"xs-network-uuid\", the UUID of the Citrix XenServer network to which this interface is attached; \"xs-vif-vm-uuid\", the UUID of the Citrix XenServer VM to which this interface belongs; \"xs-vif-mac\", the value of the \"MAC\" field in the Citrix XenServer VIF record for this interface.",
          "type": {"key": "string", "value": "string", "min": 0, "max": "unlimited"}},
        "ofport": {
-         "comment": "OpenFlow port number for this interface.  This is populated when the port number becomes known.  Before it is populated its value will be missing.  If the interface cannot be added then this is indicated by a value of -1.",
          "type": {"key": "integer", "min": 0, "max": 1},
          "ephemeral": true}}},
    "Mirror": {
-     "comment": "A port mirror within a Bridge.",
      "columns": {
        "name": {
-         "comment": "Arbitrary identifier for the Mirror.",
          "type": "string"},
        "select_src_port": {
-         "comment": "Ports on which arriving packets are selected for mirroring.  If this column and select_dst_port are both empty, then all packets on all ports are selected for mirroring.",
          "type": {"key": {"type": "uuid",
                           "refTable": "Port"},
                    "min": 0, "max": "unlimited"}},
        "select_dst_port": {
-         "comment": "Ports on which departing packets are selected for mirroring.",
          "type": {"key": {"type": "uuid",
                           "refTable": "Port"}, "min": 0, "max": "unlimited"}},
        "select_vlan": {
-         "comment": "VLANs on which packets are selected for mirroring.  An empty set selects packets on all VLANs.",
          "type": {"key": {"type": "integer",
                           "minInteger": 0,
                           "maxInteger": 4095},
                   "min": 0, "max": 4096}},
        "output_port": {
-         "comment": "Output port for selected packets.  Mutually exclusive with output_vlan.",
          "type": {"key": {"type": "uuid",
                           "refTable": "Port"}, "min": 0, "max": 1}},
        "output_vlan": {
-         "comment": "Output VLAN for selected packets.  Mutually exclusive with output_port.",
          "type": {"key": {"type": "integer",
                           "minInteger": 1,
                           "maxInteger": 4095},
                   "min": 0, "max": 1}}}},
    "NetFlow": {
-     "comment": "A NetFlow target.",
      "columns": {
        "targets": {
-         "comment": "NetFlow targets in the form \"IP:PORT\".",
          "type": {"key": {"type": "string"},
                   "min": 1, "max": "unlimited"}},
        "engine_type": {
-         "comment": "Engine type to use in NetFlow messages.  Defaults to datapath index if not specified.",
          "type": {"key": {"type": "integer",
                           "minInteger": 0,
                           "maxInteger": 255},
                   "min": 0, "max": 1}},
        "engine_id": {
-         "comment": "Engine ID to use in NetFlow messages.  Defaults to datapath index if not specified.",
          "type": {"key": {"type": "integer",
                           "minInteger": 0,
                           "maxInteger": 255},
                   "min": 0, "max": 1}},
        "add_id_to_interface": {
-         "comment": "Place least-significant 7 bits of engine ID into most significant bits of ingress and egress interface fields of NetFlow records?",
          "type": "boolean"},
        "active_timeout": {
-         "comment": "Active timeout interval, in seconds.  A value of 0 requests the default timeout; a negative value disables active timeouts.",
          "type": {"key": {"type": "integer",
                           "minInteger": -1}}}}},
    "sFlow": {
-     "comment": "A sFlow target.",
      "columns": {
        "targets": {
-         "comment": "sFlow targets in the form \"IP:PORT\".",
          "type": {"key": "string", "min": 1, "max": "unlimited"}},
        "sampling": {
-         "comment": "Rate at which packets should be sampled and sent to the collector.  If not specified, defaults to 400, which means one out of 400, on average, will be sent to the collector.",
          "type": {"key": "integer", "min": 0, "max": 1}},
        "polling": {
-         "comment": "Polling rate in seconds to send port statistics to the collector.  If not specified, defaults to 30 seconds.",
          "type": {"key": "integer", "min": 0, "max": 1}},
        "header": {
-         "comment": "Number of bytes of a sampled packet to send to the collector.  If not specified, defaults is 128 bytes.",
          "type": {"key": "integer", "min": 0, "max": 1}},
        "agent": {
-         "comment": "IP address to report as \"agent address\" to collectors.  If not specified, defaults to collector's \"local_ip\" value.  If neither is specified, sFlow is disabled.",
          "type": {"key": "string", "min": 0, "max": 1}}}},
    "Controller": {
-     "comment": "An OpenFlow controller.",
      "columns": {
        "target": {
-         "comment": "Connection method for controller, e.g. \"ssl:...\", \"tcp:...\".  The special string \"discover\" enables controller discovery.  The special string \"none\" disables the controller.",
          "type": "string"},
        "max_backoff": {
-         "comment": "Maximum number of milliseconds to wait between connection attempts.  Default is implementation-specific.",
          "type": {"key": {"type": "integer",
                           "minInteger": 1000},
                   "min": 0, "max": 1}},
        "inactivity_probe": {
-         "comment": "Maximum number of milliseconds of idle time on connection to controller before sending an inactivity probe message.  Default is implementation-specific.",
          "type": {"key": "integer", "min": 0, "max": 1}},
        "fail_mode": {
-         "comment": "Either \"standalone\" or \"secure\", or empty to use the implementation's default.",
          "type": {"key": {"type": "string",
                           "enum": ["set", ["standalone", "secure"]]},
                   "min": 0, "max": 1}},
        "discover_accept_regex": {
-         "comment": "If \"target\" is \"discover\", a POSIX extended regular expression against which the discovered controller location is validated.  If not specified, the default is implementation-specific.",
          "type": {"key": "string", "min": 0, "max": 1}},
        "discover_update_resolv_conf": {
-         "comment": "If \"target\" is \"discover\", whether to update /etc/resolv.conf when the controller is discovered.  If not specified, the default is implementation-specific.",
          "type": {"key": "boolean", "min": 0, "max": 1}},
        "connection_mode": {
-         "comment": "Either \"in-band\" or \"out-of-band\".  If not specified, the default is implementation-specific.",
          "type": {"key": {"type": "string",
                   "enum": ["set", ["in-band", "out-of-band"]]},
                   "min": 0, "max": 1}},
        "local_ip": {
-         "comment": "If \"target\" is not \"discover\", the IP address to configure on the local port.",
          "type": {"key": {"type": "string"},
                   "min": 0, "max": 1}},
        "local_netmask": {
-         "comment": "If \"target\" is not \"discover\", the IP netmask to configure on the local port.",
          "type": {"key": {"type": "string"},
                   "min": 0, "max": 1}},
        "local_gateway": {
-         "comment": "If \"target\" is not \"discover\", the IP gateway to configure on the local port.",
          "type": {"key": {"type": "string"},
                   "min": 0, "max": 1}},
        "controller_rate_limit": {
-         "comment": "The maximum rate at which packets will be forwarded to the OpenFlow controller, in packets per second.  If not specified, the default is implementation-specific.",
          "type": {"key": {"type": "integer",
                           "minInteger": 100},
                   "min": 0, "max": 1}},
        "controller_burst_limit": {
-         "comment": "The maximum number of unused packet credits that the bridge will allow to accumulate, in packets.  If not specified, the default is implementation-specific.",
          "type": {"key": {"type": "integer", 
                           "minInteger": 25},
                   "min": 0, "max": 1}}}},
    "SSL": {
-     "comment": "SSL configuration for an Open_vSwitch.",
      "columns": {
        "private_key": {
-         "comment": "Name of a PEM file containing the private key used as the switch's identity for SSL connections to the controller.",
          "type": "string"},
        "certificate": {
-         "comment": "Name of a PEM file containing a certificate, signed by the certificate authority (CA) used by the controller and manager, that certifies the switch's private key, identifying a trustworthy switch.",
          "type": "string"},
        "ca_cert": {
-         "comment": "Name of a PEM file containing the CA certificate used to verify that the switch is connected to a trustworthy controller.",
          "type": "string"},
        "bootstrap_ca_cert": {
-         "comment": "If set to true, then Open vSwitch will attempt to obtain the CA certificate from the controller on its first SSL connection and save it to the named PEM file. If it is successful, it will immediately drop the connection and reconnect, and from then on all SSL connections must be authenticated by a certificate signed by the CA certificate thus obtained.  This option exposes the SSL connection to a man-in-the-middle attack obtaining the initial CA certificate, but it may be useful for bootstrapping.",
          "type": "boolean"}}}}}
diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index b21b56004..9b1d76ae1 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -16,10 +16,9 @@
       </column>
 
       <column name="controller">
-        Default <ref table="Controller"/> used by bridges.  If a
-        <ref table="Bridge"/> record has a <ref table="Bridge"
-        column="controller"/> column, then this
-        <ref table="Controller"/> is used instead.
+        Default <ref table="Controller"/> used by bridges.  May be
+        overridden on a per-bridge basis by the <ref table="Bridge"
+        column="controller"/> column in <ref table="Bridge"/>.
       </column>
 
       <column name="managers">
@@ -42,7 +41,7 @@
 
       <column name="cur_cfg">
         Sequence number that Open vSwitch sets to the current value of
-        <ref column="next_cfg"/> after it finishing applying a set of
+        <ref column="next_cfg"/> after it finishes applying a set of
         configuration changes.
       </column>
     </group>
@@ -100,7 +99,7 @@
       </column>
 
       <column name="datapath_id">
-        Reports the OpenFlow datapath ID in use.  Exactly 12 hex digits.
+        Reports the OpenFlow datapath ID in use.  Exactly 16 hex digits.
       </column>
     </group>
 
@@ -129,11 +128,11 @@
         features.  The currently defined key-value pairs are:
         <dl>
           <dt><code>datapath-id</code></dt>
-          <dd>Exactly 12 hex
+          <dd>Exactly 16 hex
             digits to set the OpenFlow datapath ID to a specific
             value.</dd>
           <dt><code>hwaddr</code></dt>
-          <dd>Exactly 12 hex digits in the form
+          <dd>An Ethernet address in the form
             <var>xx</var>:<var>xx</var>:<var>xx</var>:<var>xx</var>:<var>xx</var>:<var>xx</var>
             to set the hardware address of the local port and influence the
             datapath ID.</dd>
@@ -145,7 +144,7 @@
   <table name="Port" table="Port or bond configuration.">
     <p>A port within a <ref table="Bridge"/>.</p>
     <p>Most commonly, a port has exactly one ``interface,'' pointed to by its
-      <ref column="interface"/> column.  Such a port logically
+      <ref column="interfaces"/> column.  Such a port logically
       corresponds to a port on a physical Ethernet switch.  A port
       with more than one interface is a ``bonded port'' (see
       <ref group="Bonding Configuration"/>).</p>
@@ -227,12 +226,12 @@
       <column name="bond_downdelay">
         For a bonded port, the number of milliseconds for which carrier must
         stay down on an interface before the interface is considered to be
-        down.  Specify <code>0</code> to enable the interface immediately.
+        down.  Specify <code>0</code> to disable the interface immediately.
       </column>
 
       <column name="bond_fake_iface">
-        For a bonded port, whether to create a fake interface with the name of
-        the port.  Use only for compatibility with legacy software that
+        For a bonded port, whether to create a fake internal interface with the
+        name of the port.  Use only for compatibility with legacy software that
         requires this.
       </column>
     </group>
@@ -254,7 +253,8 @@
         Key-value pairs that identify this port's role in external systems.  No
         key-value pairs native to <ref table="Port"/> are currently defined.
         For fake bridges (see the <ref column="fake_bridge"/> column), external
-        IDs for the fake bridge are defined here by prefixing their keys
+        IDs for the fake bridge are defined here by prefixing a
+        <ref table="Bridge"/> <ref table="Bridge" column="external_ids"/> key
         with <code>fake-bridge-</code>,
         e.g. <code>fake-bridge-xs-network-uuids</code>.
       </column>
@@ -264,7 +264,7 @@
         currently defined key-value pairs are:
         <dl>
           <dt><code>hwaddr</code></dt>
-          <dd>Exactly 12 hex digits in the form
+          <dd>An Ethernet address in the form
             <code><var>xx</var>:<var>xx</var>:<var>xx</var>:<var>xx</var>:<var>xx</var>:<var>xx</var></code>.</dd>
         </dl>
       </column>
@@ -293,7 +293,7 @@
             whose name is first in alphabetical order).  Internal ports and
             bridge ports that are used as port mirroring destinations (see the
             <ref table="Mirror"/> table) are ignored.</li>
-          <li>For other internal interface, the default MAC is randomly
+          <li>For other internal interfaces, the default MAC is randomly
             generated.</li>
           <li>External interfaces typically have a MAC address associated with
             their hardware.</li>
@@ -310,10 +310,10 @@
         <p>Open vSwitch populates this column when the port number becomes
           known.  If the interface is successfully added,
           <ref column="ofport"/> will be set to a number between 1 and 65535
-          (generally either in the range 1 to <code>0xff00</code>, exclusive,
-          or <code>0xfffe</code>, the port number for the OpenFlow ``local
-          port'').  If the interface cannot be added then Open vSwitch sets
-          this column to <code>-1</code>.</p>
+          (generally either in the range 1 to 65280, exclusive, or 65534, the
+          port number for the OpenFlow ``local port'').  If the interface
+          cannot be added then Open vSwitch sets this column
+          to -1.</p>
       </column>
     </group>
 
@@ -328,7 +328,7 @@
             vSwitch is running.  The empty string is a synonym for
             <code>system</code>.</dd>
           <dt><code>internal</code></dt>
-          <dd>A simulated network devices that sent and receive traffic.  An
+          <dd>A simulated network device that sends and receives traffic.  An
             internal interface whose <ref column="name"/> is the same as its
             bridge's <ref table="Open_vSwitch" column="name"/> is called the
             ``local interface.''  It does not make sense to bond an internal
@@ -376,13 +376,13 @@
         <dl>
           <dt><code>xs-vif-uuid</code></dt>
           <dd>UUID of the Citrix XenServer VIF associated with this
-            interface</dd>
+            interface.</dd>
           <dt><code>xs-network-uuid</code></dt>
           <dd>UUID of the Citrix XenServer network to which this interface is
-            attached</dd>
+            attached.</dd>
           <dt><code>xs-vif-vm-uuid</code></dt>
           <dd>UUID of the Citrix XenServer VM to which this interface
-            belongs</dd>
+            belongs.</dd>
           <dt><code>xs-vif-mac</code></dt>
           <dd>The value of the "MAC" field in the Citrix XenServer VIF record
             for this interface.</dd>