[ovs-dev] [PATCH RFC 11/52] log: Allow client to specify magic.

Ben Pfaff blp at ovn.org
Tue Sep 19 22:00:44 UTC 2017


Until now, the logging code in ovsdb has only supported a single file
format, for OVSDB standalone database files.  Upcoming commits will add
support for another, incompatible format, which uses a different magic
string for identification.  This commit allows the logging code to
support both formats.

Signed-off-by: Ben Pfaff <blp at ovn.org>
---
 ovsdb/file.c       |  5 +++--
 ovsdb/log.c        | 54 ++++++++++++++++++++++++++++++++++++++++--------------
 ovsdb/log.h        |  5 ++++-
 ovsdb/ovsdb-tool.c | 10 +++++-----
 tests/ovsdb-log.at | 27 +++++++++++++++++++++++++++
 tests/test-ovsdb.c | 17 ++++++++++++++---
 6 files changed, 93 insertions(+), 25 deletions(-)

diff --git a/ovsdb/file.c b/ovsdb/file.c
index 6a406da2a838..16461a75bfe5 100644
--- a/ovsdb/file.c
+++ b/ovsdb/file.c
@@ -129,7 +129,7 @@ ovsdb_file_open_log(const char *file_name, enum ovsdb_log_open_mode open_mode,
 
     ovs_assert(logp || schemap);
 
-    error = ovsdb_log_open(file_name, open_mode, -1, &log);
+    error = ovsdb_log_open(file_name, OVSDB_MAGIC, open_mode, -1, &log);
     if (error) {
         goto error;
     }
@@ -440,7 +440,8 @@ ovsdb_file_save_copy__(const char *file_name, int locking,
     struct ovsdb_log *log;
     struct json *json;
 
-    error = ovsdb_log_open(file_name, OVSDB_LOG_CREATE, locking, &log);
+    error = ovsdb_log_open(file_name, OVSDB_MAGIC,
+                           OVSDB_LOG_CREATE, locking, &log);
     if (error) {
         return error;
     }
diff --git a/ovsdb/log.c b/ovsdb/log.c
index 380f5e93d464..e6f66e668fe5 100644
--- a/ovsdb/log.c
+++ b/ovsdb/log.c
@@ -1,4 +1,4 @@
-/* Copyright (c) 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
+/* Copyright (c) 2009, 2010, 2011, 2012, 2013, 2017 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -42,6 +42,7 @@ struct ovsdb_log {
     off_t prev_offset;
     off_t offset;
     char *name;
+    char *magic;
     struct lockfile *lockfile;
     FILE *stream;
     struct ovsdb_error *read_error;
@@ -53,12 +54,17 @@ struct ovsdb_log {
  * the new log into '*filep' and returns NULL; otherwise returns NULL and
  * stores NULL into '*filep'.
  *
+ * 'magic' is a short text string put at the beginning of every record and used
+ * to distinguish one kind of log file from another.  For a conventional OVSDB
+ * log file, use OVSDB_MAGIC.
+ *
  * Whether the file will be locked using lockfile_lock() depends on 'locking':
  * use true to lock it, false not to lock it, or -1 to lock it only if
  * 'open_mode' is a mode that allows writing.
  */
 struct ovsdb_error *
-ovsdb_log_open(const char *name, enum ovsdb_log_open_mode open_mode,
+ovsdb_log_open(const char *name, const char *magic,
+               enum ovsdb_log_open_mode open_mode,
                int locking, struct ovsdb_log **filep)
 {
     struct lockfile *lockfile;
@@ -118,10 +124,30 @@ ovsdb_log_open(const char *name, enum ovsdb_log_open_mode open_mode,
         goto error_unlock;
     }
 
-    if (!fstat(fd, &s) && s.st_size == 0) {
-        /* It's (probably) a new file so fsync() its parent directory to ensure
-         * that its directory entry is committed to disk. */
-        fsync_parent_dir(name);
+    if (!fstat(fd, &s)) {
+        if (s.st_size == 0) {
+            /* It's (probably) a new file so fsync() its parent directory to
+             * ensure that its directory entry is committed to disk. */
+            fsync_parent_dir(name);
+        } else if (s.st_size >= strlen(magic) && S_ISREG(s.st_mode)) {
+            /* Try to read the magic from the first log record.  If it's not
+             * the magic we expect, this is the wrong kind of file, so reject
+             * it immediately. */
+            size_t magic_len = strlen(magic);
+            char *buf = xzalloc(magic_len + 1);
+            bool err = (read(fd, buf, magic_len) == magic_len
+                        && strcmp(buf, magic));
+            free(buf);
+            if (err) {
+                error = ovsdb_error(NULL, "%s: bad magic (unexpected "
+                                    "kind of file)", name);
+                goto error_close;
+            }
+            if (lseek(fd, 0, SEEK_SET)) {
+                error = ovsdb_io_error(errno, "%s: seek failed", name);
+                goto error_close;
+            }
+        }
     }
 
     stream = fdopen(fd, open_mode == OVSDB_LOG_READ_ONLY ? "rb" : "w+b");
@@ -132,6 +158,7 @@ ovsdb_log_open(const char *name, enum ovsdb_log_open_mode open_mode,
 
     file = xmalloc(sizeof *file);
     file->name = xstrdup(name);
+    file->magic = xstrdup(magic);
     file->lockfile = lockfile;
     file->stream = stream;
     file->prev_offset = 0;
@@ -162,21 +189,20 @@ ovsdb_log_close(struct ovsdb_log *file)
     }
 }
 
-static const char magic[] = "OVSDB JSON ";
-
 static bool
-parse_header(char *header, unsigned long int *length,
+parse_header(const char *magic, char *header, unsigned long int *length,
              uint8_t sha1[SHA1_DIGEST_SIZE])
 {
     char *p;
 
     /* 'header' must consist of a magic string... */
-    if (strncmp(header, magic, strlen(magic))) {
+    size_t magic_len = strlen(magic);
+    if (strncmp(header, magic, magic_len) || header[magic_len] != ' ') {
         return false;
     }
 
     /* ...followed by a length in bytes... */
-    *length = strtoul(header + strlen(magic), &p, 10);
+    *length = strtoul(header + magic_len + 1, &p, 10);
     if (!*length || *length == ULONG_MAX || *p != ' ') {
         return false;
     }
@@ -256,7 +282,7 @@ ovsdb_log_read(struct ovsdb_log *file, struct json **jsonp)
         goto error;
     }
 
-    if (!parse_header(header, &data_length, expected_sha1)) {
+    if (!parse_header(file->magic, header, &data_length, expected_sha1)) {
         error = ovsdb_syntax_error(NULL, NULL, "%s: parse error at offset "
                                    "%lld in header line \"%.*s\"",
                                    file->name, (long long int) file->offset,
@@ -356,8 +382,8 @@ ovsdb_log_write(struct ovsdb_log *file, struct json *json)
 
     /* Compose header. */
     sha1_bytes(json_string, length, sha1);
-    snprintf(header, sizeof header, "%s%"PRIuSIZE" "SHA1_FMT"\n",
-             magic, length, SHA1_ARGS(sha1));
+    snprintf(header, sizeof header, "%s %"PRIuSIZE" "SHA1_FMT"\n",
+             file->magic, length, SHA1_ARGS(sha1));
 
     /* Write. */
     if (fwrite(header, strlen(header), 1, file->stream) != 1
diff --git a/ovsdb/log.h b/ovsdb/log.h
index 5fe636b4c387..fbcea1f2b991 100644
--- a/ovsdb/log.h
+++ b/ovsdb/log.h
@@ -29,7 +29,10 @@ enum ovsdb_log_open_mode {
     OVSDB_LOG_CREATE            /* Create new file, read/write. */
 };
 
-struct ovsdb_error *ovsdb_log_open(const char *name, enum ovsdb_log_open_mode,
+#define OVSDB_MAGIC "OVSDB JSON"
+
+struct ovsdb_error *ovsdb_log_open(const char *name, const char *magic,
+                                   enum ovsdb_log_open_mode,
                                    int locking, struct ovsdb_log **)
     OVS_WARN_UNUSED_RESULT;
 void ovsdb_log_close(struct ovsdb_log *);
diff --git a/ovsdb/ovsdb-tool.c b/ovsdb/ovsdb-tool.c
index 8908bae3628a..45b3f7348c3d 100644
--- a/ovsdb/ovsdb-tool.c
+++ b/ovsdb/ovsdb-tool.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
+ * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2016, 2017 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -217,8 +217,8 @@ do_create(struct ovs_cmdl_context *ctx)
     ovsdb_schema_destroy(schema);
 
     /* Create database file. */
-    check_ovsdb_error(ovsdb_log_open(db_file_name, OVSDB_LOG_CREATE,
-                                     -1, &log));
+    check_ovsdb_error(ovsdb_log_open(db_file_name, OVSDB_MAGIC,
+                                     OVSDB_LOG_CREATE, -1, &log));
     check_ovsdb_error(ovsdb_log_write(log, json));
     check_ovsdb_error(ovsdb_log_commit(log));
     ovsdb_log_close(log);
@@ -519,8 +519,8 @@ do_show_log(struct ovs_cmdl_context *ctx)
     struct ovsdb_schema *schema;
     unsigned int i;
 
-    check_ovsdb_error(ovsdb_log_open(db_file_name, OVSDB_LOG_READ_ONLY,
-                                     -1, &log));
+    check_ovsdb_error(ovsdb_log_open(db_file_name, OVSDB_MAGIC,
+                                     OVSDB_LOG_READ_ONLY, -1, &log));
     shash_init(&names);
     schema = NULL;
     for (i = 0; ; i++) {
diff --git a/tests/ovsdb-log.at b/tests/ovsdb-log.at
index 3e7cdf828466..c8efaaec1a50 100644
--- a/tests/ovsdb-log.at
+++ b/tests/ovsdb-log.at
@@ -73,6 +73,33 @@ file: read: end of log
 AT_CHECK([test -f .file.~lock~])
 AT_CLEANUP
 
+AT_SETUP([write one, reread - alternative magic])
+AT_KEYWORDS([ovsdb log])
+AT_CAPTURE_FILE([file])
+# Sometimes you just need more magic:
+# http://www.catb.org/jargon/html/magic-story.html
+AT_CHECK(
+  [[test-ovsdb --magic="MORE MAGIC" log-io file create 'write:[0]' 'write:[1]' 'write:[2]']], [0],
+  [[file: open successful
+file: write:[0] successful
+file: write:[1] successful
+file: write:[2] successful
+]], [ignore])
+AT_CHECK(
+  [test-ovsdb --magic="MORE MAGIC" log-io file read-only read read read read], [0],
+  [[file: open successful
+file: read: [0]
+file: read: [1]
+file: read: [2]
+file: read: end of log
+]], [ignore])
+AT_CHECK(
+  [test-ovsdb log-io file read-only], [1], [],
+  [test-ovsdb: ovsdb error: file: bad magic (unexpected kind of file)
+])
+AT_CHECK([test -f .file.~lock~])
+AT_CLEANUP
+
 AT_SETUP([write one, reread, append])
 AT_KEYWORDS([ovsdb log])
 AT_CAPTURE_FILE([file])
diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c
index 1760268b625c..36b23effff81 100644
--- a/tests/test-ovsdb.c
+++ b/tests/test-ovsdb.c
@@ -55,6 +55,9 @@ struct test_ovsdb_pvt_context {
     bool track;
 };
 
+/* Magic to pass to ovsdb_log_open(). */
+static const char *magic = OVSDB_MAGIC;
+
 OVS_NO_RETURN static void usage(void);
 static void parse_options(int argc, char *argv[],
     struct test_ovsdb_pvt_context *pvt);
@@ -76,10 +79,14 @@ main(int argc, char *argv[])
 static void
 parse_options(int argc, char *argv[], struct test_ovsdb_pvt_context *pvt)
 {
+    enum {
+        OPT_MAGIC = CHAR_MAX + 1,
+    };
     static const struct option long_options[] = {
         {"timeout", required_argument, NULL, 't'},
         {"verbose", optional_argument, NULL, 'v'},
         {"change-track", optional_argument, NULL, 'c'},
+        {"magic", required_argument, NULL, OPT_MAGIC},
         {"help", no_argument, NULL, 'h'},
         {NULL, 0, NULL, 0},
     };
@@ -116,6 +123,10 @@ parse_options(int argc, char *argv[], struct test_ovsdb_pvt_context *pvt)
             pvt->track = true;
             break;
 
+        case OPT_MAGIC:
+            magic = optarg;
+            break;
+
         case '?':
             exit(EXIT_FAILURE);
 
@@ -131,8 +142,8 @@ usage(void)
 {
     printf("%s: Open vSwitch database test utility\n"
            "usage: %s [OPTIONS] COMMAND [ARG...]\n\n"
-           "  log-io FILE FLAGS COMMAND...\n"
-           "    open FILE with FLAGS, run COMMANDs\n"
+           "  [--magic=MAGIC] log-io FILE FLAGS COMMAND...\n"
+           "    open FILE with FLAGS (and MAGIC), run COMMANDs\n"
            "  default-atoms\n"
            "    test ovsdb_atom_default()\n"
            "  default-data\n"
@@ -316,7 +327,7 @@ do_log_io(struct ovs_cmdl_context *ctx)
         ovs_fatal(0, "unknown log-io open mode \"%s\"", mode_string);
     }
 
-    check_ovsdb_error(ovsdb_log_open(name, mode, -1, &log));
+    check_ovsdb_error(ovsdb_log_open(name, magic, mode, -1, &log));
     printf("%s: open successful\n", name);
 
     for (i = 3; i < ctx->argc; i++) {
-- 
2.10.2



More information about the dev mailing list