[ovs-dev] [PATCH 10/13] log: Support multiple magic.

Ben Pfaff blp at ovn.org
Fri Dec 8 00:12:37 UTC 2017


Some OVSDB tools will want to open files that might be standalone or
clustered databases, and so it's better if ovsdb_log_open() can accept more
than one valid "magic".  This commit makes that possible.

Signed-off-by: Ben Pfaff <blp at ovn.org>
---
 ovsdb/log.c        | 147 +++++++++++++++++++++++++++++++++++++----------------
 ovsdb/log.h        |   6 ++-
 tests/ovsdb-log.at |   2 +-
 3 files changed, 108 insertions(+), 47 deletions(-)

diff --git a/ovsdb/log.c b/ovsdb/log.c
index d25f766474dc..dd641884a2ee 100644
--- a/ovsdb/log.c
+++ b/ovsdb/log.c
@@ -88,13 +88,19 @@ static bool rename_open_files = false;
 static bool rename_open_files = true;
 #endif
 
+static bool parse_header(char *header, const char **magicp,
+                         unsigned long int *length,
+                         uint8_t sha1[SHA1_DIGEST_SIZE]);
+static bool is_magic_ok(const char *needle, const char *haystack);
+
 /* Attempts to open 'name' with the specified 'open_mode'.  On success, stores
  * 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.
+ * log file, use OVSDB_MAGIC.  To accept more than one magic string, separate
+ * them with "|", e.g. "MAGIC 1|MAGIC 2".
  *
  * 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
@@ -107,12 +113,16 @@ ovsdb_log_open(const char *name, const char *magic,
 {
     struct lockfile *lockfile;
     struct ovsdb_error *error;
-    struct ovsdb_log *file;
     struct stat s;
     FILE *stream;
     int flags;
     int fd;
 
+    /* If we can create a new file, we need to know what kind of magic to
+     * use, so there must be only one kind. */
+    if (open_mode == OVSDB_LOG_CREATE_EXCL || open_mode == OVSDB_LOG_CREATE) {
+        ovs_assert(!strchr(magic, '|'));
+    }
     *filep = NULL;
 
     ovs_assert(locking == -1 || locking == false || locking == true);
@@ -181,43 +191,54 @@ ovsdb_log_open(const char *name, const char *magic,
         goto error_unlock;
     }
 
-    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");
     if (!stream) {
         error = ovsdb_io_error(errno, "%s: fdopen failed", name);
-        goto error_close;
+        close(fd);
+        goto error_unlock;
+    }
+
+    /* Read the magic from the first log record. */
+    char header[128];
+    const char *actual_magic;
+    if (!fgets(header, sizeof header, stream)) {
+        if (ferror(stream)) {
+            error = ovsdb_io_error(errno, "%s: read error", name);
+            goto error_fclose;
+        }
+
+        /* We need to be able to report what kind of file this is but we can't
+         * if it's empty and we accept more than one. */
+        if (strchr(magic, '|')) {
+            error = ovsdb_error(NULL, "%s: unexpected end of file", name);
+            goto error_fclose;
+        }
+        actual_magic = magic;
+
+        /* It's an empty file and therefore probably a new file, so fsync()
+         * its parent directory to ensure that its directory entry is
+         * committed to disk. */
+        fsync_parent_dir(name);
+    } else {
+        unsigned long int length;
+        uint8_t sha1[SHA1_DIGEST_SIZE];
+        if (!parse_header(header, &actual_magic, &length, sha1)
+            || !is_magic_ok(actual_magic, magic)) {
+            error = ovsdb_error(NULL, "%s: unexpected file format", name);
+            goto error_fclose;
+        }
+    }
+
+    if (fseek(stream, 0, SEEK_SET)) {
+        error = ovsdb_io_error(errno, "%s: seek failed", name);
+        goto error_fclose;
     }
 
-    file = xmalloc(sizeof *file);
+    struct ovsdb_log *file = xmalloc(sizeof *file);
     file->state = OVSDB_LOG_READ;
     file->error = NULL;
     file->name = xstrdup(name);
-    file->magic = xstrdup(magic);
+    file->magic = xstrdup(actual_magic);
     file->lockfile = lockfile;
     file->stream = stream;
     file->prev_offset = 0;
@@ -225,20 +246,43 @@ ovsdb_log_open(const char *name, const char *magic,
     *filep = file;
     return NULL;
 
-error_close:
-    close(fd);
+error_fclose:
+    fclose(stream);
 error_unlock:
     lockfile_unlock(lockfile);
 error:
     return error;
 }
 
+/* Returns true if 'needle' is one of the |-delimited words in 'haystack'. */
+static bool
+is_magic_ok(const char *needle, const char *haystack)
+{
+    /* 'needle' can't be multiple words. */
+    if (strchr(needle, '|')) {
+        return false;
+    }
+
+    size_t n = strlen(needle);
+    for (;;) {
+        if (!strncmp(needle, haystack, n) && strchr("|", haystack[n])) {
+            return true;
+        }
+        haystack = strchr(haystack, '|');
+        if (!haystack) {
+            return false;
+        }
+        haystack++;
+    }
+}
+
 void
 ovsdb_log_close(struct ovsdb_log *file)
 {
     if (file) {
         ovsdb_error_destroy(file->error);
         free(file->name);
+        free(file->magic);
         if (file->stream) {
             fclose(file->stream);
         }
@@ -247,20 +291,34 @@ ovsdb_log_close(struct ovsdb_log *file)
     }
 }
 
+const char *
+ovsdb_log_get_magic(const struct ovsdb_log *log)
+{
+    return log->magic;
+}
+
 static bool
-parse_header(const char *magic, char *header, unsigned long int *length,
-             uint8_t sha1[SHA1_DIGEST_SIZE])
+parse_header(char *header, const char **magicp,
+             unsigned long int *length, uint8_t sha1[SHA1_DIGEST_SIZE])
 {
-    char *p;
+    /* 'header' must consist of "OVSDB "... */
+    const char lead[] = "OVSDB ";
+    if (strncmp(lead, header, strlen(lead))) {
+        return false;
+    }
 
-    /* 'header' must consist of a magic string... */
-    size_t magic_len = strlen(magic);
-    if (strncmp(header, magic, magic_len) || header[magic_len] != ' ') {
+    /* ...followed by a magic string... */
+    char *magic = header + strlen(lead);
+    size_t magic_len = strcspn(magic, " ");
+    if (magic[magic_len] != ' ') {
         return false;
     }
+    magic[magic_len] = '\0';
+    *magicp = magic;
 
     /* ...followed by a length in bytes... */
-    *length = strtoul(header + magic_len + 1, &p, 10);
+    char *p;
+    *length = strtoul(magic + magic_len + 1, &p, 10);
     if (!*length || *length == ULONG_MAX || *p != ' ') {
         return false;
     }
@@ -342,7 +400,6 @@ ovsdb_log_read(struct ovsdb_log *file, struct json **jsonp)
     uint8_t expected_sha1[SHA1_DIGEST_SIZE];
     uint8_t actual_sha1[SHA1_DIGEST_SIZE];
     struct ovsdb_error *error;
-    off_t data_offset;
     unsigned long data_length;
     struct json *json;
     char header[128];
@@ -356,8 +413,11 @@ ovsdb_log_read(struct ovsdb_log *file, struct json **jsonp)
         error = ovsdb_io_error(errno, "%s: read failed", file->name);
         goto error;
     }
+    off_t data_offset = file->offset + strlen(header);
 
-    if (!parse_header(file->magic, header, &data_length, expected_sha1)) {
+    const char *magic;
+    if (!parse_header(header, &magic, &data_length, expected_sha1)
+        || strcmp(magic, file->magic)) {
         error = ovsdb_syntax_error(NULL, NULL, "%s: parse error at offset "
                                    "%lld in header line \"%.*s\"",
                                    file->name, (long long int) file->offset,
@@ -365,7 +425,6 @@ ovsdb_log_read(struct ovsdb_log *file, struct json **jsonp)
         goto error;
     }
 
-    data_offset = file->offset + strlen(header);
     error = parse_body(file, data_offset, data_length, actual_sha1, &json);
     if (error) {
         goto error;
@@ -442,7 +501,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, "OVSDB %s %"PRIuSIZE" "SHA1_FMT"\n",
                   magic, data->length, SHA1_ARGS(sha1));
 }
 
diff --git a/ovsdb/log.h b/ovsdb/log.h
index cc8469c01e57..4b74ca11ce6a 100644
--- a/ovsdb/log.h
+++ b/ovsdb/log.h
@@ -1,4 +1,4 @@
-/* Copyright (c) 2009, 2010, 2011 Nicira, Inc.
+/* Copyright (c) 2009, 2010, 2011, 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.
@@ -31,7 +31,7 @@ enum ovsdb_log_open_mode {
     OVSDB_LOG_CREATE            /* Create or open file, read/write. */
 };
 
-#define OVSDB_MAGIC "OVSDB JSON"
+#define OVSDB_MAGIC "JSON"
 
 struct ovsdb_error *ovsdb_log_open(const char *name, const char *magic,
                                    enum ovsdb_log_open_mode,
@@ -39,6 +39,8 @@ struct ovsdb_error *ovsdb_log_open(const char *name, const char *magic,
     OVS_WARN_UNUSED_RESULT;
 void ovsdb_log_close(struct ovsdb_log *);
 
+const char *ovsdb_log_get_magic(const struct ovsdb_log *);
+
 struct ovsdb_error *ovsdb_log_read(struct ovsdb_log *, struct json **)
     OVS_WARN_UNUSED_RESULT;
 void ovsdb_log_unread(struct ovsdb_log *);
diff --git a/tests/ovsdb-log.at b/tests/ovsdb-log.at
index 125ddd187551..9aa1e870039c 100644
--- a/tests/ovsdb-log.at
+++ b/tests/ovsdb-log.at
@@ -174,7 +174,7 @@ 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)
+  [test-ovsdb: ovsdb error: file: unexpected file format
 ])
 AT_CHECK([test -f .file.~lock~])
 AT_CLEANUP
-- 
2.10.2



More information about the dev mailing list