From a35ae81c6f79ab24e621a9d155538f5b88c5c2ac Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Mon, 30 Jul 2012 14:55:10 -0700 Subject: [PATCH 5/7] ovsdb: Do not replace symlinks by regular files during compaction. Signed-off-by: Ben Pfaff --- ovsdb/file.c | 5 ++++- ovsdb/ovsdb-tool.c | 22 ++++++++++++++++------ tests/ovsdb-server.at | 15 ++++++++++++++- tests/ovsdb-tool.at | 14 +++++++++++++- 4 files changed, 47 insertions(+), 9 deletions(-) Index: b/ovsdb/file.c =================================================================== --- a/ovsdb/file.c +++ b/ovsdb/file.c @@ -531,11 +531,14 @@ ovsdb_file_create(struct ovsdb *db, stru { 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 " Index: b/ovsdb/ovsdb-tool.c =================================================================== --- a/ovsdb/ovsdb-tool.c +++ b/ovsdb/ovsdb-tool.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2009, 2010, 2011 Nicira Networks. + * Copyright (c) 2009, 2010, 2011, 2012 Nicira Networks. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -181,16 +181,26 @@ do_create(int argc OVS_UNUSED, char *arg } 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); @@ -200,9 +210,6 @@ compact_or_convert(const char *src_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); @@ -226,6 +233,9 @@ compact_or_convert(const char *src_name, } lockfile_unlock(dst_lock); + + free(src_name); + free(dst_name); } static void Index: b/tests/ovsdb-server.at =================================================================== --- a/tests/ovsdb-server.at +++ b/tests/ovsdb-server.at @@ -254,8 +254,15 @@ AT_SETUP([compacting online]) AT_KEYWORDS([ovsdb server compact]) AT_DATA([schema], [ORDINAL_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. @@ -324,6 +331,12 @@ _uuid nam 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. Index: b/tests/ovsdb-tool.at =================================================================== --- a/tests/ovsdb-tool.at +++ b/tests/ovsdb-tool.at @@ -52,7 +52,14 @@ AT_SETUP([ovsdb-tool compact]) AT_KEYWORDS([ovsdb file positive]) AT_DATA([schema], [ORDINAL_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]) dnl Do a bunch of random transactions that put crap in the database log. AT_CHECK( @@ -120,6 +127,11 @@ _uuid nam 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.