ovsdb: Add "comment" feature to transactions and make ovs-vsctl use them.

The idea here is that transaction comments get copied to the ovsdb-server's
transaction log, which can then make it clear later why a particular change
was made to the database, to ease debugging.

diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
index 8c1dcd8..f72e187 100644 (file)
--- a/lib/ovsdb-idl.c
+++ b/lib/ovsdb-idl.c
@@ -23,6 +23,7 @@
 #include <stdlib.h>
 
 #include "bitmap.h"
+#include "dynamic-string.h"
 #include "json.h"
 #include "jsonrpc.h"
 #include "ovsdb-data.h"
@@ -80,6 +81,7 @@ struct ovsdb_idl_txn {
     struct hmap txn_rows;
     enum ovsdb_idl_txn_status status;
     bool dry_run;
+    struct ds comment;
 };
 
 static struct vlog_rate_limit syntax_rl = VLOG_RATE_LIMIT_INIT(1, 5);
@@ -793,9 +795,19 @@ ovsdb_idl_txn_create(struct ovsdb_idl *idl)
     txn->status = TXN_INCOMPLETE;
     hmap_init(&txn->txn_rows);
     txn->dry_run = false;
+    ds_init(&txn->comment);
     return txn;
 }
 
+void
+ovsdb_idl_txn_add_comment(struct ovsdb_idl_txn *txn, const char *s)
+{
+    if (txn->comment.length) {
+        ds_put_char(&txn->comment, '\n');
+    }
+    ds_put_cstr(&txn->comment, s);
+}
+
 void
 ovsdb_idl_txn_set_dry_run(struct ovsdb_idl_txn *txn)
 {
@@ -809,6 +821,7 @@ ovsdb_idl_txn_destroy(struct ovsdb_idl_txn *txn)
         hmap_remove(&txn->idl->outstanding_txns, &txn->hmap_node);
     }
     ovsdb_idl_txn_abort(txn);
+    ds_destroy(&txn->comment);
     free(txn);
 }
 
@@ -1039,6 +1052,13 @@ ovsdb_idl_txn_commit(struct ovsdb_idl_txn *txn)
         }
     }
 
+    if (txn->comment.length) {
+        struct json *op = json_object_create();
+        json_object_put_string(op, "op", "comment");
+        json_object_put_string(op, "comment", ds_cstr(&txn->comment));
+        json_array_add(operations, op);
+    }
+
     if (txn->dry_run) {
         struct json *op = json_object_create();
         json_object_put_string(op, "op", "abort");

diff --git a/lib/ovsdb-idl.h b/lib/ovsdb-idl.h
index fd6aa8b..0a1fbc6 100644 (file)
--- a/lib/ovsdb-idl.h
+++ b/lib/ovsdb-idl.h
@@ -41,6 +41,7 @@ enum ovsdb_idl_txn_status {
 const char *ovsdb_idl_txn_status_to_string(enum ovsdb_idl_txn_status);
 
 struct ovsdb_idl_txn *ovsdb_idl_txn_create(struct ovsdb_idl *);
+void ovsdb_idl_txn_add_comment(struct ovsdb_idl_txn *, const char *);
 void ovsdb_idl_txn_set_dry_run(struct ovsdb_idl_txn *);
 void ovsdb_idl_txn_destroy(struct ovsdb_idl_txn *);
 void ovsdb_idl_txn_wait(const struct ovsdb_idl_txn *);

diff --git a/ovsdb/SPECS b/ovsdb/SPECS
index e6e1ca1..019eb5b 100644 (file)
--- a/ovsdb/SPECS
+++ b/ovsdb/SPECS
@@ -972,3 +972,22 @@ Errors:
 
         The same "uuid-name" appeared on an earlier "insert" or
         "declare" operation within this transaction.
+
+comment
+.......
+
+
+Request object members:
+
+    "op": "comment"                    required
+    "comment": <string>                required
+
+Result object members:
+
+    none
+
+Semantics:
+
+    Provides information to a database administrator on the purpose of
+    a transaction.  The OVSDB server, for example, adds comments in
+    transactions that modify the database to the database journal.

diff --git a/ovsdb/execution.c b/ovsdb/execution.c
index 67f6b8d..4cb8b14 100644 (file)
--- a/ovsdb/execution.c
+++ b/ovsdb/execution.c
@@ -57,6 +57,7 @@ static ovsdb_operation_executor ovsdb_execute_wait;
 static ovsdb_operation_executor ovsdb_execute_commit;
 static ovsdb_operation_executor ovsdb_execute_abort;
 static ovsdb_operation_executor ovsdb_execute_declare;
+static ovsdb_operation_executor ovsdb_execute_comment;
 
 static ovsdb_operation_executor *
 lookup_executor(const char *name)
@@ -76,6 +77,7 @@ lookup_executor(const char *name)
         { "commit", ovsdb_execute_commit },
         { "abort", ovsdb_execute_abort },
         { "declare", ovsdb_execute_declare },
+        { "comment", ovsdb_execute_comment },
     };
 
     size_t i;
@@ -685,3 +687,18 @@ ovsdb_execute_declare(struct ovsdb_execution *x, struct ovsdb_parser *parser,
                         xasprintf(UUID_FMT, UUID_ARGS(&uuid))));
     return NULL;
 }
+
+static struct ovsdb_error *
+ovsdb_execute_comment(struct ovsdb_execution *x, struct ovsdb_parser *parser,
+                      struct json *result UNUSED)
+{
+    const struct json *comment;
+
+    comment = ovsdb_parser_member(parser, "comment", OP_STRING);
+    if (!comment) {
+        return NULL;
+    }
+    ovsdb_txn_add_comment(x->txn, json_string(comment));
+
+    return NULL;
+}

diff --git a/ovsdb/file.c b/ovsdb/file.c
index 377ca28..97359d0 100644 (file)
--- a/ovsdb/file.c
+++ b/ovsdb/file.c
@@ -27,6 +27,7 @@
 #include "ovsdb-error.h"
 #include "row.h"
 #include "table.h"
+#include "timeval.h"
 #include "transaction.h"
 #include "uuid.h"
 #include "util.h"
@@ -185,6 +186,11 @@ ovsdb_file_txn_from_json(struct ovsdb *db, const struct json *json,
 
         table = shash_find_data(&db->tables, table_name);
         if (!table) {
+            if (!strcmp(table_name, "_date")
+                || !strcmp(table_name, "_comment")) {
+                continue;
+            }
+
             error = ovsdb_syntax_error(json, "unknown table",
                                        "No table named %s.", table_name);
             goto error;
@@ -299,6 +305,7 @@ ovsdb_file_replica_commit(struct ovsdb_replica *r_,
     struct ovsdb_file_replica *r = ovsdb_file_replica_cast(r_);
     struct ovsdb_file_replica_aux aux;
     struct ovsdb_error *error;
+    const char *comment;
 
     aux.json = NULL;
     aux.table_json = NULL;
@@ -310,6 +317,13 @@ ovsdb_file_replica_commit(struct ovsdb_replica *r_,
         return NULL;
     }
 
+    comment = ovsdb_txn_get_comment(txn);
+    if (comment) {
+        json_object_put_string(aux.json, "_comment", comment);
+    }
+
+    json_object_put(aux.json, "_date", json_integer_create(time_now()));
+
     error = ovsdb_log_write(r->log, aux.json);
     json_destroy(aux.json);
     if (error) {

diff --git a/ovsdb/transaction.c b/ovsdb/transaction.c
index 02cfeeb..45b2083 100644 (file)
--- a/ovsdb/transaction.c
+++ b/ovsdb/transaction.c
@@ -19,6 +19,7 @@
 
 #include <assert.h>
 
+#include "dynamic-string.h"
 #include "hash.h"
 #include "hmap.h"
 #include "json.h"
@@ -31,6 +32,7 @@
 struct ovsdb_txn {
     struct ovsdb *db;
     struct hmap txn_tables;     /* Contains "struct ovsdb_txn_table"s. */
+    struct ds comment;
 };
 
 /* A table modified by a transaction. */
@@ -65,6 +67,7 @@ ovsdb_txn_create(struct ovsdb *db)
     struct ovsdb_txn *txn = xmalloc(sizeof *txn);
     txn->db = db;
     hmap_init(&txn->txn_tables);
+    ds_init(&txn->comment);
     return txn;
 }
 
@@ -96,6 +99,7 @@ ovsdb_txn_destroy(struct ovsdb_txn *txn, void (*cb)(struct ovsdb_txn_row *))
         free(txn_table);
     }
     hmap_destroy(&txn->txn_tables);
+    ds_destroy(&txn->comment);
     free(txn);
 }
 
@@ -284,3 +288,18 @@ ovsdb_txn_row_delete(struct ovsdb_txn *txn, const struct ovsdb_row *row_)
         ovsdb_row_destroy(row);
     }
 }
+
+void
+ovsdb_txn_add_comment(struct ovsdb_txn *txn, const char *s)
+{
+    if (txn->comment.length) {
+        ds_put_char(&txn->comment, '\n');
+    }
+    ds_put_cstr(&txn->comment, s);
+}
+
+const char *
+ovsdb_txn_get_comment(const struct ovsdb_txn *txn)
+{
+    return txn->comment.length ? ds_cstr_ro(&txn->comment) : NULL;
+}

diff --git a/ovsdb/transaction.h b/ovsdb/transaction.h
index 048bf74..1c54ec3 100644 (file)
--- a/ovsdb/transaction.h
+++ b/ovsdb/transaction.h
@@ -39,4 +39,7 @@ typedef bool ovsdb_txn_row_cb_func(const struct ovsdb_row *old,
 void ovsdb_txn_for_each_change(const struct ovsdb_txn *,
                                ovsdb_txn_row_cb_func *, void *aux);
 
+void ovsdb_txn_add_comment(struct ovsdb_txn *, const char *);
+const char *ovsdb_txn_get_comment(const struct ovsdb_txn *);
+
 #endif /* ovsdb/transaction.h */

diff --git a/tests/ovsdb-file.at b/tests/ovsdb-file.at
index c1f0bfb..579fcb6 100644 (file)
--- a/tests/ovsdb-file.at
+++ b/tests/ovsdb-file.at
@@ -28,3 +28,21 @@ cat stdout >> output
    AT_CLEANUP])
 
 EXECUTION_EXAMPLES
+
+AT_SETUP([transaction comments])
+AT_KEYWORDS([ovsdb file positive])
+AT_DATA([schema], [ORDINAL_SCHEMA
+])
+touch .db.~lock~
+OVS_CHECK_LCOV([ovsdb-tool create db schema], [0], [], [ignore])
+OVS_CHECK_LCOV([[ovsdb-tool transact db '
+    [{"op": "insert",
+      "table": "ordinals",
+      "row": {"name": "five", "number": 5}},
+     {"op": "comment",
+      "comment": "add row for 5"}]']], [0], [stdout], [ignore])
+AT_CHECK([perl $srcdir/uuidfilt.pl stdout], [0],
+ [[[{"uuid":["uuid","<0>"]},{}]
+]])
+AT_CHECK([grep -q "add row for 5" db])
+AT_CLEANUP

diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c
index d24844b..bc6e4db 100644 (file)
--- a/utilities/ovs-vsctl.c
+++ b/utilities/ovs-vsctl.c
@@ -1214,7 +1214,7 @@ do_vsctl(int argc, char *argv[], struct ovsdb_idl *idl)
     struct ovsdb_idl_txn *txn;
     const struct ovsrec_open_vswitch *ovs;
     enum ovsdb_idl_txn_status status;
-    struct ds *output;
+    struct ds comment, *output;
     int n_output;
     int i, start;
 
@@ -1223,6 +1223,14 @@ do_vsctl(int argc, char *argv[], struct ovsdb_idl *idl)
         ovsdb_idl_txn_set_dry_run(txn);
     }
 
+    ds_init(&comment);
+    ds_put_cstr(&comment, "ovs-vsctl:");
+    for (i = 0; i < argc; i++) {
+        ds_put_format(&comment, " %s", argv[i]);
+    }
+    ovsdb_idl_txn_add_comment(txn, ds_cstr(&comment));
+    ds_destroy(&comment);
+
     ovs = ovsrec_open_vswitch_first(idl);
     if (!ovs) {
         /* XXX add verification that table is empty */