On Mon, Jul 30, 2012 at 3:18 PM, Ben Pfaff <b...@nicira.com> wrote:
> Signed-off-by: Ben Pfaff <b...@nicira.com>
> ---
> 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 c51f752..9e2dd50 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 4959478..f31bdd1 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 f5db1a8..614e8f4 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 2d19b32..556c1fa 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.
> --
> 1.7.2.5
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
looks good to me.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
