[ovs-dev] [debian 8/9] lockfile: Be more forgiving about lockfiles for symlinks.

Ansis Atteka aatteka at nicira.com
Tue Jul 31 19:22:33 UTC 2012


On Mon, Jul 30, 2012 at 3:18 PM, Ben Pfaff <blp at nicira.com> wrote:

> As the database is being transitioned from /etc to /var, there is a symlink
> from the old to the new location for the database and a symlink for its
> lockfile.  This works OK, but it would be more user-friendly to still work
> correctly in case the symlink for the lockfile isn't there (since its
> existence is non-obvious), so this commit implements that behavior.
>
> Signed-off-by: Ben Pfaff <blp at nicira.com>
> ---
>  lib/lockfile.c        |   29 +++++++++++++++++++++--------
>  tests/lockfile.at     |    1 +
>  tests/test-lockfile.c |   30 ++++++++++++++++++++++++++++++
>  3 files changed, 52 insertions(+), 8 deletions(-)
>
> diff --git a/lib/lockfile.c b/lib/lockfile.c
> index c55be66..7ac3465 100644
> --- a/lib/lockfile.c
> +++ b/lib/lockfile.c
> @@ -59,16 +59,29 @@ static int lockfile_try_lock(const char *name, bool
> block,
>                               struct lockfile **lockfilep);
>
>  /* Returns the name of the lockfile that would be created for locking a
> file
> - * named 'file_name'.  The caller is responsible for freeing the returned
> - * name, with free(), when it is no longer needed. */
> + * named 'filename_'.  The caller is responsible for freeing the returned
> name,
> + * with free(), when it is no longer needed. */
>  char *
> -lockfile_name(const char *file_name)
> +lockfile_name(const char *filename_)
>  {
> -    const char *slash = strrchr(file_name, '/');
> -    return (slash
> -            ? xasprintf("%.*s/.%s.~lock~",
> -                        (int) (slash - file_name), file_name, slash + 1)
> -            : xasprintf(".%s.~lock~", file_name));
> +    char *filename;
> +    const char *slash;
> +    char *lockname;
> +
> +    /* If 'filename_' is a symlink, base the name of the lockfile on the
> +     * symlink's target rather than the name of the symlink.  That way,
> if a
> +     * file is symlinked, but there is no symlink for its lockfile, then
> there
> +     * is only a single lockfile for both the source and the target of the
> +     * symlink, not one for each. */
> +    filename = follow_symlinks(filename_);
> +    slash = strrchr(filename, '/');
> +    lockname = (slash
> +                ? xasprintf("%.*s/.%s.~lock~",
> +                            (int) (slash - filename), filename, slash + 1)
> +                : xasprintf(".%s.~lock~", filename));
> +    free(filename);
> +
> +    return lockname;
>  }
>
>  /* Locks the configuration file against modification by other processes
> and
> diff --git a/tests/lockfile.at b/tests/lockfile.at
> index 1fa0342..602cfab 100644
> --- a/tests/lockfile.at
> +++ b/tests/lockfile.at
> @@ -19,3 +19,4 @@ CHECK_LOCKFILE([lock_timeout_gets_the_lock], [1])
>  CHECK_LOCKFILE([lock_timeout_runs_out], [1])
>  CHECK_LOCKFILE([lock_multiple], [0])
>  CHECK_LOCKFILE([lock_symlink], [0])
> +CHECK_LOCKFILE([lock_symlink_to_dir], [0])
> diff --git a/tests/test-lockfile.c b/tests/test-lockfile.c
> index 808ed1e..412010b 100644
> --- a/tests/test-lockfile.c
> +++ b/tests/test-lockfile.c
> @@ -249,6 +249,35 @@ run_lock_symlink(void)
>      CHECK(S_ISREG(s.st_mode) != 0, 1);
>  }
>
> +/* Checks that locking a file that is itself a symlink yields a lockfile
> in the
> + * directory that the symlink points to, named for the target of the
> + * symlink.
> + *
> + * (That is, if "a" is a symlink to "dir/b", then "a"'s lockfile is named
> + * "dir/.b.~lock".) */
> +static void
> +run_lock_symlink_to_dir(void)
> +{
> +    struct lockfile *a, *dummy;
> +    struct stat s;
> +
> +    /* Create a symlink "a" pointing to "dir/b". */
> +    CHECK(mkdir("dir", 0700), 0);
> +    CHECK(symlink("dir/b", "a"), 0);
> +    CHECK(lstat("a", &s), 0);
> +    CHECK(S_ISLNK(s.st_mode) != 0, 1);
> +
> +    /* Lock 'a'. */
> +    CHECK(lockfile_lock("a", 0, &a), 0);
> +    CHECK(lstat("dir/.b.~lock~", &s), 0);
> +    CHECK(S_ISREG(s.st_mode) != 0, 1);
> +    CHECK(lstat(".a.~lock~", &s), -1);
> +    CHECK(errno, ENOENT);
> +    CHECK(lockfile_lock("dir/b", 0, &dummy), EDEADLK);
> +
> +    lockfile_unlock(a);
> +}
> +
>  static void
>  run_help(void)
>  {
> @@ -275,6 +304,7 @@ static const struct test tests[] = {
>      TEST(lock_timeout_runs_out),
>      TEST(lock_multiple),
>      TEST(lock_symlink),
> +    TEST(lock_symlink_to_dir),
>      TEST(help),
>      { NULL, NULL }
>  #undef TEST
> --
> 1.7.2.5
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
Looks good to me.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openvswitch.org/pipermail/ovs-dev/attachments/20120731/89ab9309/attachment-0003.html>


More information about the dev mailing list