ovsdb: Do not replace symlinks by regular files during compaction.

Signed-off-by: Ben Pfaff <blp@nicira.com>

diff --git a/ovsdb/file.c b/ovsdb/file.c
index c51f752..9e2dd50 100644 (file)
--- a/ovsdb/file.c
+++ b/ovsdb/file.c
@@ -1,4 +1,4 @@
-/* Copyright (c) 2009, 2010, 2011 Nicira, Inc.
+/* Copyright (c) 2009, 2010, 2011, 2012 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -531,11 +531,14 @@ ovsdb_file_create(struct ovsdb *db, struct ovsdb_log *log,
 {
     long long int now = time_msec();
     struct ovsdb_file *file;
+    char *deref_name;
     char *abs_name;
 
     /* Use the absolute name of the file because ovsdb-server opens its
      * database before daemonize() chdirs to "/". */
-    abs_name = abs_file_name(NULL, file_name);
+    deref_name = follow_symlinks(file_name);
+    abs_name = abs_file_name(NULL, deref_name);
+    free(deref_name);
     if (!abs_name) {
         *filep = NULL;
         return ovsdb_io_error(0, "could not determine current "

diff --git a/ovsdb/ovsdb-tool.c b/ovsdb/ovsdb-tool.c
index 4959478..f31bdd1 100644 (file)
--- a/ovsdb/ovsdb-tool.c
+++ b/ovsdb/ovsdb-tool.c
@@ -207,16 +207,26 @@ do_create(int argc, char *argv[])
 }
 
 static void
-compact_or_convert(const char *src_name, const char *dst_name,
+compact_or_convert(const char *src_name_, const char *dst_name_,
                    const struct ovsdb_schema *new_schema,
                    const char *comment)
 {
+    char *src_name, *dst_name;
     struct lockfile *src_lock;
     struct lockfile *dst_lock;
-    bool in_place = dst_name == NULL;
+    bool in_place = dst_name_ == NULL;
     struct ovsdb *db;
     int retval;
 
+    /* Dereference symlinks for source and destination names.  In the in-place
+     * case this ensures that, if the source name is a symlink, we replace its
+     * target instead of replacing the symlink by a regular file.  In the
+     * non-in-place, this has the same effect for the destination name. */
+    src_name = follow_symlinks(src_name_);
+    dst_name = (in_place
+                ? xasprintf("%s.tmp", src_name)
+                : follow_symlinks(dst_name_));
+
     /* Lock the source, if we will be replacing it. */
     if (in_place) {
         retval = lockfile_lock(src_name, 0, &src_lock);
@@ -226,9 +236,6 @@ compact_or_convert(const char *src_name, const char *dst_name,
     }
 
     /* Get (temporary) destination and lock it. */
-    if (in_place) {
-        dst_name = xasprintf("%s.tmp", src_name);
-    }
     retval = lockfile_lock(dst_name, 0, &dst_lock);
     if (retval) {
         ovs_fatal(retval, "%s: failed to lock lockfile", dst_name);
@@ -253,9 +260,8 @@ compact_or_convert(const char *src_name, const char *dst_name,
 
     lockfile_unlock(dst_lock);
 
-    if (in_place) {
-        free((char *) dst_name);
-    }
+    free(src_name);
+    free(dst_name);
 }
 
 static void

diff --git a/tests/ovsdb-server.at b/tests/ovsdb-server.at
index b81878c..9d64ef7 100644 (file)
--- a/tests/ovsdb-server.at
+++ b/tests/ovsdb-server.at
@@ -248,8 +248,15 @@ AT_CLEANUP
 AT_SETUP([compacting online])
 AT_KEYWORDS([ovsdb server compact])
 ordinal_schema > schema
-touch .db.~lock~
+dnl Make sure that "ovsdb-tool create" works with a dangling symlink for
+dnl the database and the lockfile, creating the target of each symlink rather
+dnl than replacing the symlinks with regular files.
+mkdir dir
+ln -s dir/db db
+ln -s dir/.db.~lock~ .db.~lock~
+AT_SKIP_IF([test ! -h db || test ! -h .db.~lock~])
 AT_CHECK([ovsdb-tool create db schema], [0], [ignore], [ignore])
+dnl Start ovsdb-server.
 AT_CHECK([ovsdb-server --detach --pidfile="`pwd`"/pid --unixctl="`pwd`"/unixctl --remote=punix:socket --log-file="`pwd`"/ovsdb-server.log db], [0], [ignore], [ignore])
 AT_CAPTURE_FILE([ovsdb-server.log])
 dnl Do a bunch of random transactions that put crap in the database log.
@@ -318,6 +325,12 @@ _uuid                                name  number
 dnl Now compact the database in-place.
 AT_CHECK([[ovs-appctl -t "`pwd`"/unixctl ovsdb-server/compact]],
   [0], [], [ignore], [test ! -e pid || kill `cat pid`])
+dnl Make sure that "db" is still a symlink to dir/db instead of getting
+dnl replaced by a regular file, ditto for .db.~lock~.
+AT_CHECK([test -h db])
+AT_CHECK([test -h .db.~lock~])
+AT_CHECK([test -f dir/db])
+AT_CHECK([test -f dir/.db.~lock~])
 dnl We can't fully re-check the contents of the database log, because the
 dnl order of the records is not predictable, but there should only be 4 lines
 dnl in it now.

diff --git a/tests/ovsdb-tool.at b/tests/ovsdb-tool.at
index 87949bb..e4f4a29 100644 (file)
--- a/tests/ovsdb-tool.at
+++ b/tests/ovsdb-tool.at
@@ -49,8 +49,18 @@ AT_CLEANUP
 AT_SETUP([ovsdb-tool compact])
 AT_KEYWORDS([ovsdb file positive])
 ordinal_schema > schema
-touch .db.~lock~
+dnl Make sure that "ovsdb-tool create" works with a dangling symlink,
+dnl creating the target of the symlink rather than replacing the symlink
+dnl with a regular file, and that the lockfile gets created relative to
+dnl the symlink's target.
+mkdir dir
+: > dir/.db.~lock~
+ln -s dir/db db
+AT_SKIP_IF([test ! -h db])
 AT_CHECK([ovsdb-tool create db schema], [0], [], [ignore])
+AT_CHECK([test ! -e .db.~lock])
+AT_CHECK([test -h db])
+AT_CHECK([test -f dir/db])
 dnl Do a bunch of random transactions that put crap in the database log.
 AT_CHECK(
   [[for pair in 'zero 0' 'one 1' 'two 2' 'three 3' 'four 4' 'five 5'; do
@@ -117,6 +127,11 @@ _uuid                                name  number
 dnl Now compact the database in-place.
 touch .db.tmp.~lock~
 AT_CHECK([[ovsdb-tool compact db]], [0], [], [ignore])
+dnl Make sure that "db" is still a symlink to dir/db instead of getting
+dnl replaced by a regular file.
+AT_CHECK([test ! -e .db.~lock])
+AT_CHECK([test -h db])
+AT_CHECK([test -f dir/db])
 dnl We can't fully re-check the contents of the database log, because the
 dnl order of the records is not predictable, but there should only be 4 lines
 dnl in it now.