ovsdb: Allow ovsdb_log_open()'s caller to choose whether to lock.

The current callers of ovsdb_log_open() always want to lock the file if
they are accessing it for read/write access.  An upcoming commit will add
a new caller that does not fit this model (it wants to lock the file
across a wider region) and so the caller should be able to choose whether
to do locking.  This commit adds that ability.

Also, get rid of the use of <fcntl.h> flags to choose the open mode, which
has always seemed somewhat crude and which this change would make even
cruder.

ovsdb/file.c

@@ -43,13 +43,15 @@ static void ovsdb_file_replica_create(struct ovsdb *, struct ovsdb_log *);
 struct ovsdb_error *
 ovsdb_file_open(const char *file_name, bool read_only, struct ovsdb **dbp)
 {
+    enum ovsdb_log_open_mode open_mode;
     struct ovsdb_schema *schema;
     struct ovsdb_error *error;
     struct ovsdb_log *log;
     struct json *json;
     struct ovsdb *db;
 
-    error = ovsdb_log_open(file_name, read_only ? O_RDONLY : O_RDWR, &log);
+    open_mode = read_only ? OVSDB_LOG_READ_ONLY : OVSDB_LOG_READ_WRITE;
+    error = ovsdb_log_open(file_name, open_mode, -1, &log);
     if (error) {
         return error;
     }

ovsdb/log.c

@@ -50,21 +50,33 @@ struct ovsdb_log {
     enum ovsdb_log_mode mode;
 };
 
+/* 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'.
+ *
+ * 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, int flags, struct ovsdb_log **filep)
+ovsdb_log_open(const char *name, enum ovsdb_log_open_mode open_mode,
+               int locking, struct ovsdb_log **filep)
 {
     struct lockfile *lockfile;
     struct ovsdb_error *error;
     struct ovsdb_log *file;
     struct stat s;
     FILE *stream;
-    int accmode;
+    int flags;
     int fd;
 
     *filep = NULL;
 
-    accmode = flags & O_ACCMODE;
-    if (accmode == O_RDWR || accmode == O_WRONLY) {
+    assert(locking == -1 || locking == false || locking == true);
+    if (locking < 0) {
+        locking = open_mode != OVSDB_LOG_READ_ONLY;
+    }
+    if (locking) {
         int retval = lockfile_lock(name, 0, &lockfile);
         if (retval) {
             error = ovsdb_io_error(retval, "%s: failed to lock lockfile",
@@ -75,9 +87,18 @@ ovsdb_log_open(const char *name, int flags, struct ovsdb_log **filep)
         lockfile = NULL;
     }
 
+    if (open_mode == OVSDB_LOG_READ_ONLY) {
+        flags = O_RDONLY;
+    } else if (open_mode == OVSDB_LOG_READ_WRITE) {
+        flags = O_RDWR;
+    } else if (open_mode == OVSDB_LOG_CREATE) {
+        flags = O_RDWR | O_CREAT | O_EXCL;
+    } else {
+        NOT_REACHED();
+    }
     fd = open(name, flags, 0666);
     if (fd < 0) {
-        const char *op = flags & O_CREAT && flags & O_EXCL ? "create" : "open";
+        const char *op = open_mode == OVSDB_LOG_CREATE ? "create" : "open";
         error = ovsdb_io_error(errno, "%s: %s failed", op, name);
         goto error_unlock;
     }
@@ -98,9 +119,7 @@ ovsdb_log_open(const char *name, int flags, struct ovsdb_log **filep)
         free(dir);
     }
 
-    stream = fdopen(fd, (accmode == O_RDONLY ? "rb"
-                         : accmode == O_WRONLY ? "wb"
-                         : "w+b"));
+    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;

ovsdb/log.h

@@ -1,4 +1,4 @@
-/* Copyright (c) 2009 Nicira Networks
+/* Copyright (c) 2009, 2010 Nicira Networks
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -22,8 +22,16 @@
 struct json;
 struct ovsdb_log;
 
-struct ovsdb_error *ovsdb_log_open(const char *name, int flags,
-                                   struct ovsdb_log **) WARN_UNUSED_RESULT;
+/* Access mode for opening an OVSDB log. */
+enum ovsdb_log_open_mode {
+    OVSDB_LOG_READ_ONLY,        /* Open existing file, read-only. */
+    OVSDB_LOG_READ_WRITE,       /* Open existing file, read/write. */
+    OVSDB_LOG_CREATE            /* Create new file, read/write. */
+};
+
+struct ovsdb_error *ovsdb_log_open(const char *name, enum ovsdb_log_open_mode,
+                                   int locking, struct ovsdb_log **)
+    WARN_UNUSED_RESULT;
 void ovsdb_log_close(struct ovsdb_log *);
 
 struct ovsdb_error *ovsdb_log_read(struct ovsdb_log *, struct json **)

ovsdb/ovsdb-tool.c

@@ -164,8 +164,8 @@ do_create(int argc OVS_UNUSED, char *argv[])
     ovsdb_schema_destroy(schema);
 
     /* Create database file. */
-    check_ovsdb_error(ovsdb_log_open(db_file_name, O_RDWR | O_CREAT | O_EXCL,
-                                     &log));
+    check_ovsdb_error(ovsdb_log_open(db_file_name, OVSDB_LOG_CREATE,
+                                     -1, &log));
     check_ovsdb_error(ovsdb_log_write(log, json));
     check_ovsdb_error(ovsdb_log_commit(log));
     ovsdb_log_close(log);
@@ -288,7 +288,8 @@ do_show_log(int argc OVS_UNUSED, char *argv[])
     struct ovsdb_log *log;
     unsigned int i;
 
-    check_ovsdb_error(ovsdb_log_open(db_file_name, O_RDONLY, &log));
+    check_ovsdb_error(ovsdb_log_open(db_file_name, OVSDB_LOG_READ_ONLY,
+                                     -1, &log));
     shash_init(&names);
     for (i = 0; ; i++) {
         struct json *json;

tests/ovsdb-log.at

@@ -4,11 +4,11 @@ AT_SETUP([create empty, reread])
 AT_KEYWORDS([ovsdb log])
 AT_CAPTURE_FILE([log])
 AT_CHECK(
-  [test-ovsdb log-io file 'O_CREAT|O_RDWR'], [0], 
+  [test-ovsdb log-io file create], [0], 
   [file: open successful
 ], [ignore])
 AT_CHECK(
-  [test-ovsdb log-io file 'O_RDONLY' read], [0], 
+  [test-ovsdb log-io file read-only read], [0], 
   [file: open successful
 file: read: end of log
 ], [ignore])
@@ -19,34 +19,34 @@ AT_SETUP([write one, reread])
 AT_KEYWORDS([ovsdb log])
 AT_CAPTURE_FILE([file])
 AT_CHECK(
-  [[test-ovsdb log-io file 'O_CREAT|O_RDWR' 'write:[0]']], [0], 
+  [[test-ovsdb log-io file create 'write:[0]']], [0], 
   [[file: open successful
 file: write:[0] successful
 ]], [ignore])
 AT_CHECK(
-  [test-ovsdb log-io file 'O_RDONLY' read read], [0], 
+  [test-ovsdb log-io file read-only read read], [0], 
   [[file: open successful
 file: read: [0]
 file: read: end of log
 ]], [ignore])
 AT_CHECK([test -f .file.~lock~])
 AT_CLEANUP
 
-AT_SETUP([check that O_EXCL works])
+AT_SETUP([check that create fails if file exists])
 AT_KEYWORDS([ovsdb log])
 AT_CAPTURE_FILE([file])
 AT_CHECK(
-  [[test-ovsdb log-io file 'O_CREAT|O_RDWR' 'write:[1]']], [0], 
+  [[test-ovsdb log-io file create 'write:[1]']], [0], 
   [[file: open successful
 file: write:[1] successful
 ]], [ignore])
 AT_CHECK(
-  [test-ovsdb log-io file 'O_RDONLY' read], [0], 
+  [test-ovsdb log-io file read-only read], [0], 
   [[file: open successful
 file: read: [1]
 ]], [ignore])
 AT_CHECK(
-  [test-ovsdb -vlockfile:console:emer log-io file 'O_CREAT|O_RDWR|O_EXCL' read], [1], 
+  [test-ovsdb -vlockfile:console:emer log-io file create read], [1], 
   [], [test-ovsdb: I/O error: create: file failed (File exists)
 ])
 AT_CHECK([test -f .file.~lock~])
@@ -56,14 +56,14 @@ AT_SETUP([write one, reread])
 AT_KEYWORDS([ovsdb log])
 AT_CAPTURE_FILE([file])
 AT_CHECK(
-  [[test-ovsdb log-io file 'O_CREAT|O_RDWR' 'write:[0]' 'write:[1]' 'write:[2]']], [0], 
+  [[test-ovsdb 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 log-io file 'O_RDONLY' read read read read], [0], 
+  [test-ovsdb log-io file read-only read read read read], [0], 
   [[file: open successful
 file: read: [0]
 file: read: [1]
@@ -77,22 +77,22 @@ AT_SETUP([write one, reread, append])
 AT_KEYWORDS([ovsdb log])
 AT_CAPTURE_FILE([file])
 AT_CHECK(
-  [[test-ovsdb log-io file 'O_CREAT|O_RDWR' 'write:[0]' 'write:[1]' 'write:[2]']], [0], 
+  [[test-ovsdb 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 log-io file 'O_RDWR' read read read 'write:["append"]']], [0], 
+  [[test-ovsdb log-io file read/write read read read 'write:["append"]']], [0], 
   [[file: open successful
 file: read: [0]
 file: read: [1]
 file: read: [2]
 file: write:["append"] successful
 ]], [ignore])
 AT_CHECK(
-  [test-ovsdb log-io file 'O_RDONLY' read read read read read], [0], 
+  [test-ovsdb log-io file read-only read read read read read], [0], 
   [[file: open successful
 file: read: [0]
 file: read: [1]
@@ -107,20 +107,20 @@ AT_SETUP([write, reread one, overwrite])
 AT_KEYWORDS([ovsdb log])
 AT_CAPTURE_FILE([file])
 AT_CHECK(
-  [[test-ovsdb log-io file 'O_CREAT|O_RDWR' 'write:[0]' 'write:[1]' 'write:[2]']], [0], 
+  [[test-ovsdb 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 log-io file 'O_RDWR' read 'write:["more data"]']], [0], 
+  [[test-ovsdb log-io file read/write read 'write:["more data"]']], [0], 
   [[file: open successful
 file: read: [0]
 file: write:["more data"] successful
 ]], [ignore])
 AT_CHECK(
-  [test-ovsdb log-io file 'O_RDONLY' read read read], [0], 
+  [test-ovsdb log-io file read-only read read read], [0], 
   [[file: open successful
 file: read: [0]
 file: read: ["more data"]
@@ -133,15 +133,15 @@ AT_SETUP([write, add corrupted data, read])
 AT_KEYWORDS([ovsdb log])
 AT_CAPTURE_FILE([file])
 AT_CHECK(
-  [[test-ovsdb log-io file 'O_CREAT|O_RDWR' 'write:[0]' 'write:[1]' 'write:[2]']], [0], 
+  [[test-ovsdb 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([echo 'xxx' >> file])
 AT_CHECK(
-  [test-ovsdb log-io file 'O_RDONLY' read read read read], [0], 
+  [test-ovsdb log-io file read-only read read read read], [0], 
   [[file: open successful
 file: read: [0]
 file: read: [1]
@@ -155,15 +155,15 @@ AT_SETUP([write, add corrupted data, read, overwrite])
 AT_KEYWORDS([ovsdb log])
 AT_CAPTURE_FILE([file])
 AT_CHECK(
-  [[test-ovsdb log-io file 'O_CREAT|O_RDWR' 'write:[0]' 'write:[1]' 'write:[2]']], [0], 
+  [[test-ovsdb 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([echo 'xxx' >> file])
 AT_CHECK(
-  [[test-ovsdb log-io file 'O_RDWR' read read read read 'write:[3]']], [0], 
+  [[test-ovsdb log-io file read/write read read read read 'write:[3]']], [0], 
   [[file: open successful
 file: read: [0]
 file: read: [1]
@@ -172,7 +172,7 @@ file: read failed: syntax error: file: parse error at offset 174 in header line
 file: write:[3] successful
 ]], [ignore])
 AT_CHECK(
-  [test-ovsdb log-io file 'O_RDONLY' read read read read read], [0], 
+  [test-ovsdb log-io file read-only read read read read read], [0], 
   [[file: open successful
 file: read: [0]
 file: read: [1]
@@ -187,7 +187,7 @@ AT_SETUP([write, corrupt some data, read, overwrite])
 AT_KEYWORDS([ovsdb log])
 AT_CAPTURE_FILE([file])
 AT_CHECK(
-  [[test-ovsdb log-io file 'O_CREAT|O_RDWR' 'write:[0]' 'write:[1]' 'write:[2]']], [0], 
+  [[test-ovsdb log-io file create 'write:[0]' 'write:[1]' 'write:[2]']], [0], 
   [[file: open successful
 file: write:[0] successful
 file: write:[1] successful
@@ -198,15 +198,15 @@ AT_CHECK([mv file.tmp file])
 AT_CHECK([[grep -c '\[3]' file]], [0], [1
 ])
 AT_CHECK(
-  [[test-ovsdb log-io file 'O_RDWR' read read read 'write:["longer data"]']], [0], 
+  [[test-ovsdb log-io file read/write read read read 'write:["longer data"]']], [0], 
   [[file: open successful
 file: read: [0]
 file: read: [1]
 file: read failed: syntax error: file: 4 bytes starting at offset 170 have SHA-1 hash 5c031e5c0d3a9338cc127ebe40bb2748b6a67e78 but should have hash 98f55556e7ffd432381b56a19bd485b3e6446442
 file: write:["longer data"] successful
 ]], [ignore])
 AT_CHECK(
-  [test-ovsdb log-io file 'O_RDONLY' read read read read], [0], 
+  [test-ovsdb log-io file read-only read read read read], [0], 
   [[file: open successful
 file: read: [0]
 file: read: [1]
@@ -220,7 +220,7 @@ AT_SETUP([write, truncate file, read, overwrite])
 AT_KEYWORDS([ovsdb log])
 AT_CAPTURE_FILE([file])
 AT_CHECK(
-  [[test-ovsdb log-io file 'O_CREAT|O_RDWR' 'write:[0]' 'write:[1]' 'write:[2]']], [0], 
+  [[test-ovsdb log-io file create 'write:[0]' 'write:[1]' 'write:[2]']], [0], 
   [[file: open successful
 file: write:[0] successful
 file: write:[1] successful
@@ -231,15 +231,15 @@ AT_CHECK([mv file.tmp file])
 AT_CHECK([[grep -c '^2$' file]], [0], [1
 ])
 AT_CHECK(
-  [[test-ovsdb log-io file 'O_RDWR' read read read 'write:["longer data"]']], [0], 
+  [[test-ovsdb log-io file read/write read read read 'write:["longer data"]']], [0], 
   [[file: open successful
 file: read: [0]
 file: read: [1]
 file: read failed: I/O error: file: error reading 4 bytes starting at offset 170 (unexpected end of file)
 file: write:["longer data"] successful
 ]], [ignore])
 AT_CHECK(
-  [test-ovsdb log-io file 'O_RDONLY' read read read read], [0], 
+  [test-ovsdb log-io file read-only read read read read], [0], 
   [[file: open successful
 file: read: [0]
 file: read: [1]
@@ -253,15 +253,15 @@ AT_SETUP([write bad JSON, read, overwrite])
 AT_KEYWORDS([ovsdb log])
 AT_CAPTURE_FILE([file])
 AT_CHECK(
-  [[test-ovsdb log-io file 'O_CREAT|O_RDWR' 'write:[0]' 'write:[1]' 'write:[2]']], [0], 
+  [[test-ovsdb 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([[printf '%s\n%s\n' 'OVSDB JSON 5 d910b02871075d3156ec8675dfc95b7d5d640aa6' 'null' >> file]])
 AT_CHECK(
-  [[test-ovsdb log-io file 'O_RDWR' read read read read 'write:["replacement data"]']], [0], 
+  [[test-ovsdb log-io file read/write read read read read 'write:["replacement data"]']], [0], 
   [[file: open successful
 file: read: [0]
 file: read: [1]
@@ -270,7 +270,7 @@ file: read failed: syntax error: file: 5 bytes starting at offset 228 are not va
 file: write:["replacement data"] successful
 ]], [ignore])
 AT_CHECK(
-  [test-ovsdb log-io file 'O_RDONLY' read read read read read], [0], 
+  [test-ovsdb log-io file read-only read read read read read], [0], 
   [[file: open successful
 file: read: [0]
 file: read: [1]