From d171b5846f4988e66c45a6ed5998bbaef82571f9 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Wed, 16 Dec 2009 13:30:53 -0800 Subject: [PATCH] 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. --- lib/ovsdb-idl.c | 20 ++++++++++++++++++++ lib/ovsdb-idl.h | 1 + ovsdb/SPECS | 19 +++++++++++++++++++ ovsdb/execution.c | 17 +++++++++++++++++ ovsdb/file.c | 14 ++++++++++++++ ovsdb/transaction.c | 19 +++++++++++++++++++ ovsdb/transaction.h | 3 +++ tests/ovsdb-file.at | 18 ++++++++++++++++++ utilities/ovs-vsctl.c | 10 +++++++++- 9 files changed, 120 insertions(+), 1 deletion(-) diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c index 8c1dcd8f..f72e1876 100644 --- a/lib/ovsdb-idl.c +++ b/lib/ovsdb-idl.c @@ -23,6 +23,7 @@ #include #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 fd6aa8b9..0a1fbc6b 100644 --- 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 e6e1ca19..019eb5b4 100644 --- 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": 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 67f6b8d0..4cb8b14c 100644 --- 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 377ca280..97359d03 100644 --- 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 02cfeebf..45b20839 100644 --- a/ovsdb/transaction.c +++ b/ovsdb/transaction.c @@ -19,6 +19,7 @@ #include +#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 048bf74f..1c54ec3a 100644 --- 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 c1f0bfb1..579fcb60 100644 --- 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 d24844bf..bc6e4dbb 100644 --- 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 */ -- 2.30.2