[ovs-dev] [bug5144 5/5] ovsdb: Truncate bad transactions from database log.

Ben Pfaff blp at nicira.com
Mon Mar 28 20:07:56 UTC 2011


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.
---
 ovsdb/file.c          |    7 ++++++-
 ovsdb/log.c           |   21 ++++++++++++++++++++-
 ovsdb/log.h           |    4 +++-
 tests/ovsdb-server.at |   48 ++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 77 insertions(+), 3 deletions(-)

diff --git a/ovsdb/file.c b/ovsdb/file.c
index 1425beb..605e9cb 100644
--- 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 c0be5f5..6704307 100644
--- 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 0593fd8..f48dc76 100644
--- 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 dff19ec..88499d0 100644
--- 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
-- 
1.7.1




More information about the dev mailing list