ovsdb: Truncate bad transactions from database log.

When ovsdb-server reads a database file that is corrupted at the
transaction level (that is, the transaction is valid JSON and has the
correct SHA-1 hash, but it does not describe a valid database transaction),
then ovsdb-server should truncate it and overwrite it by valid
transactions.  However, until now, it didn't.  Instead, it would keep the
invalid transaction and possibly every transaction in the database file
(depending on in what way the transaction was invalid), which would just
cause the same trouble again the next time the database was read.

This fixes the problem.  An invalid transaction will be deleted from the
database file at the first write to the database.

Bug #5144.
Bug #5149.

diff --git a/ovsdb/file.c b/ovsdb/file.c
index 1425bebec549fbc3b022d2c4d54f703673179db9..605e9cba32f3a2d5e2ae308776b51cac1a674150 100644 (file)
--- a/ovsdb/file.c
+++ b/ovsdb/file.c
@@ -215,6 +215,7 @@ ovsdb_file_open__(const char *file_name,
                                          &date, &txn);
         json_destroy(json);
         if (error) {
+            ovsdb_log_unread(log);
             break;
         }
 
@@ -223,7 +224,11 @@ ovsdb_file_open__(const char *file_name,
             oldest_commit = date;
         }
 
-        ovsdb_error_destroy(ovsdb_txn_commit(txn, false));
+        error = ovsdb_txn_commit(txn, false);
+        if (error) {
+            ovsdb_log_unread(log);
+            break;
+        }
     }
     if (error) {
         /* Log error but otherwise ignore it.  Probably the database just got

diff --git a/ovsdb/log.c b/ovsdb/log.c
index c0be5f5d6372f1ee08287c186b84cc8fef651566..67043078f73da192f52971c771d5bcb101175042 100644 (file)
--- a/ovsdb/log.c
+++ b/ovsdb/log.c
@@ -1,4 +1,4 @@
-/* Copyright (c) 2009, 2010 Nicira Networks
+/* Copyright (c) 2009, 2010, 2011 Nicira Networks
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -43,6 +43,7 @@ enum ovsdb_log_mode {
 };
 
 struct ovsdb_log {
+    off_t prev_offset;
     off_t offset;
     char *name;
     struct lockfile *lockfile;
@@ -121,6 +122,7 @@ ovsdb_log_open(const char *name, enum ovsdb_log_open_mode open_mode,
     file->name = xstrdup(name);
     file->lockfile = lockfile;
     file->stream = stream;
+    file->prev_offset = 0;
     file->offset = 0;
     file->read_error = NULL;
     file->write_error = NULL;
@@ -284,6 +286,7 @@ ovsdb_log_read(struct ovsdb_log *file, struct json **jsonp)
         goto error;
     }
 
+    file->prev_offset = file->offset;
     file->offset = data_offset + data_length;
     *jsonp = json;
     return 0;
@@ -294,6 +297,22 @@ error:
     return error;
 }
 
+/* Causes the log record read by the previous call to ovsdb_log_read() to be
+ * effectively discarded.  The next call to ovsdb_log_write() will overwrite
+ * that previously read record.
+ *
+ * Calling this function more than once has no additional effect.
+ *
+ * This function is useful when ovsdb_log_read() successfully reads a record
+ * but that record does not make sense at a higher level (e.g. it specifies an
+ * invalid transaction). */
+void
+ovsdb_log_unread(struct ovsdb_log *file)
+{
+    assert(file->mode == OVSDB_LOG_READ);
+    file->offset = file->prev_offset;
+}
+
 struct ovsdb_error *
 ovsdb_log_write(struct ovsdb_log *file, struct json *json)
 {

diff --git a/ovsdb/log.h b/ovsdb/log.h
index 0593fd87ae70065b09efd631a58f91264a2186bf..f48dc76bdad27a6c4824808808ebfddad78297b7 100644 (file)
--- a/ovsdb/log.h
+++ b/ovsdb/log.h
@@ -1,4 +1,4 @@
-/* Copyright (c) 2009, 2010 Nicira Networks
+/* Copyright (c) 2009, 2010, 2011 Nicira Networks
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -36,6 +36,8 @@ void ovsdb_log_close(struct ovsdb_log *);
 
 struct ovsdb_error *ovsdb_log_read(struct ovsdb_log *, struct json **)
     WARN_UNUSED_RESULT;
+void ovsdb_log_unread(struct ovsdb_log *);
+
 struct ovsdb_error *ovsdb_log_write(struct ovsdb_log *, struct json *)
     WARN_UNUSED_RESULT;
 struct ovsdb_error *ovsdb_log_commit(struct ovsdb_log *)

diff --git a/tests/ovsdb-server.at b/tests/ovsdb-server.at
index dff19ecc56f649c341be301093da1419647a3c7e..88499d07fa71b6269c8a9da34a811cbbf80d5ecd 100644 (file)
--- a/tests/ovsdb-server.at
+++ b/tests/ovsdb-server.at
@@ -85,6 +85,54 @@ AT_CHECK([perl $srcdir/uuidfilt.pl output], [0],
          [test ! -e pid || kill `cat pid`])
 AT_CLEANUP
 
+AT_SETUP([truncating database log with bad transaction])
+AT_KEYWORDS([ovsdb server positive unix])
+AT_DATA([schema], [ORDINAL_SCHEMA
+])
+AT_CHECK([ovsdb-tool create db schema], [0], [stdout], [ignore])
+dnl Do one transaction and save the output.
+AT_DATA([txnfile], [[ovsdb-client transact unix:socket \
+'["ordinals",
+  {"op": "insert",
+   "table": "ordinals",
+   "row": {"number": 0, "name": "zero"}}]'
+]])
+AT_CHECK([ovsdb-server -vlockfile:ANY:EMER --remote=punix:socket --unixctl=$PWD/unixctl db --run="sh txnfile"], [0], [stdout], [])
+cat stdout >> output
+dnl Add some crap to the database log and run another transaction, which should
+dnl ignore the crap and truncate it out of the log.
+echo 'OVSDB JSON 15 ffbcdae4b0386265f9ea3280dd7c8f0b72a20e56
+{"invalid":{}}' >> db
+AT_DATA([txnfile], [[ovsdb-client transact unix:socket \
+'["ordinals",
+  {"op": "insert",
+   "table": "ordinals",
+   "row": {"number": 1, "name": "one"}}]'
+]])
+AT_CHECK([ovsdb-server -vlockfile:ANY:EMER --remote=punix:socket --unixctl=$PWD/unixctl db --run="sh txnfile"], [0], [stdout], [stderr])
+AT_CHECK([grep 'syntax "{"invalid":{}}": unknown table: No table named invalid.' stderr],
+  [0], [ignore])
+cat stdout >> output
+dnl Run a final transaction to verify that both transactions succeeeded.
+dnl The crap that we added should have been truncated by the previous run,
+dnl so ovsdb-server shouldn't log a warning this time.
+AT_DATA([txnfile], [[ovsdb-client transact unix:socket \
+'["ordinals",
+  {"op": "select",
+   "table": "ordinals",
+   "where": [],
+   "sort": ["number"]}]'
+]])
+AT_CHECK([ovsdb-server -vlockfile:ANY:EMER --remote=punix:socket --unixctl=$PWD/unixctl db --run="sh txnfile"], [0], [stdout], [])
+cat stdout >> output
+AT_CHECK([perl $srcdir/uuidfilt.pl output], [0],
+  [[[{"uuid":["uuid","<0>"]}]
+[{"uuid":["uuid","<1>"]}]
+[{"rows":[{"_uuid":["uuid","<0>"],"_version":["uuid","<2>"],"name":"zero","number":0},{"_uuid":["uuid","<1>"],"_version":["uuid","<3>"],"name":"one","number":1}]}]
+]], [],
+         [test ! -e pid || kill `cat pid`])
+AT_CLEANUP
+
 AT_SETUP([ovsdb-client get-schema-version])
 AT_KEYWORDS([ovsdb server positive])
 AT_DATA([schema], [ORDINAL_SCHEMA