log: Allow client to specify magic.

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 <[email protected]> Acked-by: Justin Pettit <[email protected]>
lut777 · Dec 24, 2017 · 19b276c · 19b276c
1 parent 09de859
commit 19b276c
Show file tree

Hide file tree

Showing 7 changed files with 97 additions and 28 deletions.
diff --git 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;
     }
@@ -438,7 +438,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
@@ -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.
@@ -43,6 +43,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;
@@ -54,12 +55,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 the OVSDB_MAGIC macro.
+ *
  * 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;
@@ -119,10 +125,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");
@@ -133,6 +159,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;
@@ -156,28 +183,28 @@ ovsdb_log_close(struct ovsdb_log *file)
 {
     if (file) {
         free(file->name);
+        free(file->magic);
         fclose(file->stream);
         lockfile_unlock(file->lockfile);
         ovsdb_error_destroy(file->read_error);
         free(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;
     }
@@ -257,7 +284,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,
@@ -325,7 +352,7 @@ ovsdb_log_unread(struct ovsdb_log *file)
  * The caller must initialize 'header' and 'data' to empty strings. */
 void
 ovsdb_log_compose_record(const struct json *json,
-                         struct ds *header, struct ds *data)
+                         const char *magic, struct ds *header, struct ds *data)
 {
     ovs_assert(json->type == JSON_OBJECT || json->type == JSON_ARRAY);
     ovs_assert(!header->length);
@@ -338,7 +365,7 @@ ovsdb_log_compose_record(const struct json *json,
     /* Compose header. */
     uint8_t sha1[SHA1_DIGEST_SIZE];
     sha1_bytes(data->string, data->length, sha1);
-    ds_put_format(header, "%s%"PRIuSIZE" "SHA1_FMT"\n",
+    ds_put_format(header, "%s %"PRIuSIZE" "SHA1_FMT"\n",
                   magic, data->length, SHA1_ARGS(sha1));
 }
 
@@ -369,7 +396,7 @@ ovsdb_log_write(struct ovsdb_log *file, struct json *json)
 
     struct ds header = DS_EMPTY_INITIALIZER;
     struct ds data = DS_EMPTY_INITIALIZER;
-    ovsdb_log_compose_record(json, &header, &data);
+    ovsdb_log_compose_record(json, file->magic, &header, &data);
     size_t total_length = header.length + data.length;
 
     /* Write. */

diff --git a/ovsdb/log.h b/ovsdb/log.h
@@ -30,7 +30,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 *);
@@ -39,7 +42,7 @@ struct ovsdb_error *ovsdb_log_read(struct ovsdb_log *, struct json **)
     OVS_WARN_UNUSED_RESULT;
 void ovsdb_log_unread(struct ovsdb_log *);
 
-void ovsdb_log_compose_record(const struct json *,
+void ovsdb_log_compose_record(const struct json *, const char *magic,
                               struct ds *header, struct ds *data);
 
 struct ovsdb_error *ovsdb_log_write(struct ovsdb_log *, struct json *)

diff --git a/ovsdb/ovsdb-client.c b/ovsdb/ovsdb-client.c
@@ -1455,7 +1455,7 @@ print_and_free_log_record(struct json *record)
 {
     struct ds header = DS_EMPTY_INITIALIZER;
     struct ds data = DS_EMPTY_INITIALIZER;
-    ovsdb_log_compose_record(record, &header, &data);
+    ovsdb_log_compose_record(record, OVSDB_MAGIC, &header, &data);
     fwrite(header.string, header.length, 1, stdout);
     fwrite(data.string, data.length, 1, stdout);
     ds_destroy(&data);

diff --git 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.
@@ -219,8 +219,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);
@@ -544,8 +544,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
@@ -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
@@ -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"
@@ -314,7 +325,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++) {