fix some lockfile issues

5 files changed, 54 insertions, 35 deletions

diff --git a/lib/lockfile.c b/lib/lockfile.c
index 864d3eff0..75e0d1a4c 100644
--- a/lib/lockfile.c
+++ b/lib/lockfile.c
@@ -1,4 +1,4 @@
- /* Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc.
+ /* Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2016 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -121,6 +121,8 @@ lockfile_lock(const char *file, struct lockfile **lockfilep)
 
     lock_name = lockfile_name(file);
 
+    VLOG_INFO("locking %s", file);
+
     ovs_mutex_lock(&lock_table_mutex);
     error = lockfile_try_lock(lock_name, &pid, lockfilep);
     ovs_mutex_unlock(&lock_table_mutex);
@@ -152,6 +154,8 @@ void
 lockfile_unlock(struct lockfile *lockfile)
 {
     if (lockfile) {
+        VLOG_INFO("unlocking %s", lockfile->name);
+
         ovs_mutex_lock(&lock_table_mutex);
         lockfile_do_unlock(lockfile);
         ovs_mutex_unlock(&lock_table_mutex);
diff --git a/ovsdb/log.c b/ovsdb/log.c
index b7c03154f..e320c2490 100644
--- a/ovsdb/log.c
+++ b/ovsdb/log.c
@@ -352,6 +352,23 @@ ovsdb_log_unread(struct ovsdb_log *file)
 }
 
 static struct ovsdb_error *
+ovsdb_log_truncate__(struct ovsdb_log *file)
+{
+    file->mode = OVSDB_LOG_WRITE;
+
+    struct ovsdb_error *error = NULL;
+    if (fseeko(file->stream, file->offset, SEEK_SET)) {
+        error = ovsdb_io_error(errno, "%s: cannot seek to offset %lld",
+                               file->name, (long long int) file->offset);
+    } else if (ftruncate(fileno(file->stream), file->offset)) {
+        error = ovsdb_io_error(errno, "%s: cannot truncate to length %lld",
+                               file->name, (long long int) file->offset);
+    }
+    file->write_error = error != NULL;
+    return error;
+}
+
+static struct ovsdb_error *
 ovsdb_log_write__(struct ovsdb_log *file, const char *data, size_t length)
 {
     uint8_t sha1[SHA1_DIGEST_SIZE];
@@ -359,16 +376,8 @@ ovsdb_log_write__(struct ovsdb_log *file, const char *data, size_t length)
     char header[128];
 
     if (file->mode == OVSDB_LOG_READ || file->write_error) {
-        file->mode = OVSDB_LOG_WRITE;
-        file->write_error = false;
-        if (fseeko(file->stream, file->offset, SEEK_SET)) {
-            error = ovsdb_io_error(errno, "%s: cannot seek to offset %lld",
-                                   file->name, (long long int) file->offset);
-            goto error;
-        }
-        if (ftruncate(fileno(file->stream), file->offset)) {
-            error = ovsdb_io_error(errno, "%s: cannot truncate to length %lld",
-                                   file->name, (long long int) file->offset);
+        error = ovsdb_log_truncate__(file);
+        if (error) {
             goto error;
         }
     }
@@ -434,6 +443,13 @@ ovsdb_log_commit(struct ovsdb_log *file)
     return NULL;
 }
 
+struct ovsdb_error *
+ovsdb_log_truncate(struct ovsdb_log *file)
+{
+    file->offset = 0;
+    return ovsdb_log_truncate__(file);
+}
+
 /* Returns the current offset into the file backing 'log', in bytes.  This
  * reflects the number of bytes that have been read or written in the file.  If
  * the whole file has been read, this is the file size. */
@@ -442,3 +458,4 @@ ovsdb_log_get_offset(const struct ovsdb_log *log)
 {
     return log->offset;
 }
+
diff --git a/ovsdb/log.h b/ovsdb/log.h
index e8f6c007e..b9773c707 100644
--- a/ovsdb/log.h
+++ b/ovsdb/log.h
@@ -52,6 +52,9 @@ struct ovsdb_error *ovsdb_log_write_json(struct ovsdb_log *,
 struct ovsdb_error *ovsdb_log_commit(struct ovsdb_log *)
     OVS_WARN_UNUSED_RESULT;
 
+struct ovsdb_error *ovsdb_log_truncate(struct ovsdb_log *)
+    OVS_WARN_UNUSED_RESULT;
+
 off_t ovsdb_log_get_offset(const struct ovsdb_log *);
 
 #endif /* ovsdb/log.h */
diff --git a/ovsdb/ovsdb-tool.c b/ovsdb/ovsdb-tool.c
index cb131a981..f92e9b46b 100644
--- a/ovsdb/ovsdb-tool.c
+++ b/ovsdb/ovsdb-tool.c
@@ -60,6 +60,7 @@ main(int argc, char *argv[])
     set_program_name(argv[0]);
     parse_options(argc, argv);
     fatal_ignore_sigpipe();
+    fatal_signal_init();
     ctx.argc = argc - optind;
     ctx.argv = argv + optind;
     ovs_cmdl_run_command(&ctx, get_all_commands());
diff --git a/ovsdb/raft.c b/ovsdb/raft.c
index f79475a47..f8221495d 100644
--- a/ovsdb/raft.c
+++ b/ovsdb/raft.c
@@ -204,6 +204,7 @@ struct raft {
     long long int election_timeout;
 
     /* File synchronization. */
+    bool fsync_thread_running;
     pthread_t fsync_thread;
     struct ovs_mutex fsync_mutex;
     uint64_t fsync_next OVS_GUARDED;
@@ -1177,6 +1178,7 @@ raft_open__(const char *file_name, enum ovsdb_log_open_mode mode,
         goto error;
     }
 
+    raft->fsync_thread_running = true;
     raft->fsync_thread = ovs_thread_create("raft_fsync",
                                            raft_fsync_thread, raft);
 
@@ -1386,7 +1388,9 @@ raft_close(struct raft *raft)
     raft->fsync_next = UINT64_MAX;
     ovs_mutex_unlock(&raft->fsync_mutex);
     seq_change(raft->fsync_request);
-    xpthread_join(raft->fsync_thread, NULL);
+    if (raft->fsync_thread_running) {
+        xpthread_join(raft->fsync_thread, NULL);
+    }
 
     ovsdb_log_close(raft->storage);
 
@@ -1998,8 +2002,9 @@ static void
 raft_install_snapshot_request_from_jsonrpc(
     struct ovsdb_parser *p, struct raft_install_snapshot_request *rq)
 {
-    const struct json *servers = ovsdb_parser_member(p, "servers", OP_OBJECT);
-    struct ovsdb_error *error = raft_servers_from_json(servers,
+    const struct json *last_servers = ovsdb_parser_member(p, "last_servers",
+                                                          OP_OBJECT);
+    struct ovsdb_error *error = raft_servers_from_json(last_servers,
                                                        &rq->last_servers);
     if (error) {
         ovsdb_parser_put_error(p, error);
@@ -3120,11 +3125,13 @@ raft_write_snapshot(struct raft *raft, const char *file_name)
     /* Create log file. */
     struct ovsdb_log *storage;
     struct ovsdb_error *error;
-    error = ovsdb_log_open(file_name, RAFT_MAGIC, OVSDB_LOG_CREATE_EXCL,
+    error = ovsdb_log_open(file_name, RAFT_MAGIC, OVSDB_LOG_CREATE,
                            -1, &storage);
+    if (!error) {
+        error = ovsdb_log_truncate(storage);
+    }
     if (error) {
-        /* "goto exit" is unsafe here: it will unlink an existing file! */
-        return error;
+        goto exit;
     }
 
     /* Write header record. */
@@ -3181,7 +3188,6 @@ exit:
         raft->storage = storage;
     } else {
         ovsdb_log_close(storage);
-        unlink(file_name);
     }
     return error;
 }
@@ -3189,23 +3195,8 @@ exit:
 static struct ovsdb_error * OVS_WARN_UNUSED_RESULT
 raft_save_snapshot(struct raft *raft)
 {
-    struct lockfile *tmp_lock = NULL;
+    char *tmp_name = xasprintf("%s.tmp", raft->file_name);
     struct ovsdb_error *error = NULL;
-    char *tmp_name = NULL;
-
-    /* Lock temporary file. */
-    tmp_name = xasprintf("%s.tmp", raft->file_name);
-    int retval = lockfile_lock(tmp_name, &tmp_lock);
-    if (retval) {
-        error = ovsdb_io_error(retval, "could not get lock on %s", tmp_name);
-        goto exit;
-    }
-
-    /* Remove temporary file.  (It might not exist.) */
-    if (unlink(tmp_name) < 0 && errno != ENOENT) {
-        error = ovsdb_io_error(errno, "failed to remove %s", tmp_name);
-        goto exit;
-    }
 
     /* Save a snapshot. */
     error = raft_write_snapshot(raft, tmp_name);
@@ -3222,7 +3213,6 @@ raft_save_snapshot(struct raft *raft)
     fsync_parent_dir(raft->file_name);
 
 exit:
-    lockfile_unlock(tmp_lock);
     free(tmp_name);
     return error;
 }
@@ -3274,6 +3264,10 @@ raft_handle_install_snapshot_request__(
 
     struct ovsdb_error *error = raft_save_snapshot(raft);
     if (error) {
+        char *error_s = ovsdb_error_to_string(error);
+        VLOG_WARN("could not save snapshot: %s", error_s);
+        free(error_s);
+
         /* XXX handle error */
     }
 }