Will you please capitalize Rahim's name in the Reported-By? Should he
be added to authors? Looks like a win, thanks.

Acked-by: Ethan Jackson <[email protected]>
Ethan


On Thu, Aug 30, 2012 at 3:29 PM, Ben Pfaff <[email protected]> wrote:
> Reported-by: rahim entezari <[email protected]>
> Signed-off-by: Ben Pfaff <[email protected]>
> ---
>  lib/lockfile.c    |   21 ++++++++++++++++-----
>  tests/lockfile.at |    7 ++++---
>  2 files changed, 20 insertions(+), 8 deletions(-)
>
> diff --git a/lib/lockfile.c b/lib/lockfile.c
> index db84aeb..3708aec 100644
> --- a/lib/lockfile.c
> +++ b/lib/lockfile.c
> @@ -55,7 +55,8 @@ struct lockfile {
>  static struct hmap lock_table = HMAP_INITIALIZER(&lock_table);
>
>  static void lockfile_unhash(struct lockfile *);
> -static int lockfile_try_lock(const char *name, struct lockfile **lockfilep);
> +static int lockfile_try_lock(const char *name, pid_t *pidp,
> +                             struct lockfile **lockfilep);
>
>  /* Returns the name of the lockfile that would be created for locking a file
>   * named 'filename_'.  The caller is responsible for freeing the returned 
> name,
> @@ -98,21 +99,27 @@ lockfile_lock(const char *file, struct lockfile 
> **lockfilep)
>       * stylized ways such that any number of readers may access a file while 
> it
>       * is being written. */
>      char *lock_name;
> +    pid_t pid;
>      int error;
>
>      COVERAGE_INC(lockfile_lock);
>
>      lock_name = lockfile_name(file);
>
> -    error = lockfile_try_lock(lock_name, lockfilep);
> +    error = lockfile_try_lock(lock_name, &pid, lockfilep);
>
>      if (error) {
>          COVERAGE_INC(lockfile_error);
>          if (error == EACCES) {
>              error = EAGAIN;
>          }
> -        VLOG_WARN("%s: failed to lock file: %s",
> -                  lock_name, strerror(error));
> +        if (pid) {
> +            VLOG_WARN("%s: cannot lock file because it is already locked by "
> +                      "pid %ld", lock_name, (long int) pid);
> +        } else {
> +            VLOG_WARN("%s: failed to lock file: %s",
> +                      lock_name, strerror(error));
> +        }
>      }
>
>      free(lock_name);
> @@ -201,7 +208,7 @@ lockfile_register(const char *name, dev_t device, ino_t 
> inode, int fd)
>  }
>
>  static int
> -lockfile_try_lock(const char *name, struct lockfile **lockfilep)
> +lockfile_try_lock(const char *name, pid_t *pidp, struct lockfile **lockfilep)
>  {
>      struct flock l;
>      struct stat s;
> @@ -209,6 +216,7 @@ lockfile_try_lock(const char *name, struct lockfile 
> **lockfilep)
>      int fd;
>
>      *lockfilep = NULL;
> +    *pidp = 0;
>
>      /* Check whether we've already got a lock on that file. */
>      if (!stat(name, &s)) {
> @@ -250,6 +258,9 @@ lockfile_try_lock(const char *name, struct lockfile 
> **lockfilep)
>      if (!error) {
>          *lockfilep = lockfile_register(name, s.st_dev, s.st_ino, fd);
>      } else {
> +        if (!fcntl(fd, F_GETLK, &l) && l.l_type != F_UNLCK) {
> +            *pidp = l.l_pid;
> +        }
>          close(fd);
>      }
>      return error;
> diff --git a/tests/lockfile.at b/tests/lockfile.at
> index 50999e4..2644d3f 100644
> --- a/tests/lockfile.at
> +++ b/tests/lockfile.at
> @@ -5,7 +5,8 @@ m4_define([CHECK_LOCKFILE],
>     AT_KEYWORDS([lockfile])
>     AT_CHECK([test-lockfile $1], [0], [$1: success (m4_if(
>       [$2], [1], [$2 child], [$2 children]))
> -], [$3])
> +], [stderr])
> +   AT_CHECK([sed 's/pid [[0-9]]*/pid <pid>/' stderr], [0], [$3])
>     AT_CLEANUP])
>
>  CHECK_LOCKFILE([lock_and_unlock], [0])
> @@ -23,13 +24,13 @@ lockfile|WARN|.file.~lock~: failed to lock file: Resource 
> deadlock avoided
>
>  CHECK_LOCKFILE([lock_blocks_other_process], [1],
>    [lockfile|WARN|.file.~lock~: child does not inherit lock
> -lockfile|WARN|.file.~lock~: failed to lock file: Resource temporarily 
> unavailable
> +lockfile|WARN|.file.~lock~: cannot lock file because it is already locked by 
> pid <pid>
>  ])
>
>  CHECK_LOCKFILE([lock_twice_blocks_other_process], [1],
>    [lockfile|WARN|.file.~lock~: failed to lock file: Resource deadlock avoided
>  lockfile|WARN|.file.~lock~: child does not inherit lock
> -lockfile|WARN|.file.~lock~: failed to lock file: Resource temporarily 
> unavailable
> +lockfile|WARN|.file.~lock~: cannot lock file because it is already locked by 
> pid <pid>
>  ])
>
>  CHECK_LOCKFILE([lock_and_unlock_allows_other_process], [1])
> --
> 1.7.2.5
>
> _______________________________________________
> dev mailing list
> [email protected]
> http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev

Reply via email to