From a35ae81c6f79ab24e621a9d155538f5b88c5c2ac Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Mon, 30 Jul 2012 14:55:10 -0700 Subject: [PATCH] ovsdb: Do not replace symlinks by regular files during compaction. Signed-off-by: Ben Pfaff --- ovsdb/file.c | 7 +++++-- ovsdb/ovsdb-tool.c | 22 ++++++++++++++-------- tests/ovsdb-server.at | 15 ++++++++++++++- tests/ovsdb-tool.at | 17 ++++++++++++++++- 4 files changed, 49 insertions(+), 12 deletions(-) diff --git a/ovsdb/file.c b/ovsdb/file.c index c51f75290..9e2dd5079 100644 --- 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 49594781d..f31bdd106 100644 --- 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 b81878c46..9d64ef7d1 100644 --- 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 87949bbb5..e4f4a29e9 100644 --- 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. -- 2.20.1