From 34bca063cd8e7721c876e0f06d42978d5ff4d835 Mon Sep 17 00:00:00 2001 From: "Kaleb S. KEITHLEY" Date: Mon, 12 Nov 2012 09:58:02 -0500 Subject: NFS is picking up geo-rep's already open (read-only) file descriptor Add anonymous member to fd_t and use it instead of over-loading pid for geo-rep and self heal Change-Id: I4d6b29a044a8ed4b8f69ff6e3f35ee227739b2af Signed-off-by: Kaleb S. KEITHLEY BUG: 874272 Reviewed-on: http://review.gluster.org/4185 Tested-by: Gluster Build System Reviewed-by: Raghavendra Bhat Reviewed-by: Vijay Bellur Reviewed-on: http://review.gluster.org/5283 --- libglusterfs/src/fd.c | 36 +++++++++++++++++------ libglusterfs/src/fd.h | 23 ++------------- tests/bugs/bug-874272.t | 48 +++++++++++++++++++++++++++++++ xlators/storage/posix/src/posix-helpers.c | 2 +- 4 files changed, 80 insertions(+), 29 deletions(-) create mode 100755 tests/bugs/bug-874272.t diff --git a/libglusterfs/src/fd.c b/libglusterfs/src/fd.c index b0b75a5ad..5ef4c0479 100644 --- a/libglusterfs/src/fd.c +++ b/libglusterfs/src/fd.c @@ -121,7 +121,7 @@ gf_fd_fdtable_alloc (void) } -fdentry_t * +static fdentry_t * __gf_fd_fdtable_get_all_fds (fdtable_t *fdtable, uint32_t *count) { fdentry_t *fdentries = NULL; @@ -159,7 +159,7 @@ gf_fd_fdtable_get_all_fds (fdtable_t *fdtable, uint32_t *count) } -fdentry_t * +static fdentry_t * __gf_fd_fdtable_copy_all_fds (fdtable_t *fdtable, uint32_t *count) { fdentry_t *fdentries = NULL; @@ -707,24 +707,44 @@ fd_lookup_uint64 (inode_t *inode, uint64_t pid) return fd; } +static fd_t * +__fd_lookup_anonymous (inode_t *inode) +{ + fd_t *iter_fd = NULL; + fd_t *fd = NULL; + + if (list_empty (&inode->fd_list)) + return NULL; -fd_t * + list_for_each_entry (iter_fd, &inode->fd_list, inode_list) { + if (iter_fd->anonymous) { + fd = __fd_ref (iter_fd); + break; + } + } + + return fd; +} + +static fd_t * __fd_anonymous (inode_t *inode) { fd_t *fd = NULL; - fd = __fd_lookup (inode, (uint64_t)-1); + fd = __fd_lookup_anonymous (inode); /* if (fd); then we already have increased the refcount in - __fd_lookup(), so no need of one more fd_ref(). + __fd_lookup_anonymous(), so no need of one more fd_ref(). if (!fd); then both create and bind wont bump up the ref count, so we have to call fd_ref() after bind. */ if (!fd) { - fd = __fd_create (inode, (uint64_t)-1); + fd = __fd_create (inode, 0); if (!fd) return NULL; + fd->anonymous = _gf_true; + __fd_bind (fd); __fd_ref (fd); @@ -752,7 +772,7 @@ fd_anonymous (inode_t *inode) gf_boolean_t fd_is_anonymous (fd_t *fd) { - return (fd && fd->pid == -1); + return (fd && fd->anonymous); } @@ -900,7 +920,7 @@ fd_ctx_get (fd_t *fd, xlator_t *xlator, uint64_t *value) } -int +static int __fd_ctx_del (fd_t *fd, xlator_t *xlator, uint64_t *value) { int index = 0; diff --git a/libglusterfs/src/fd.h b/libglusterfs/src/fd.h index 42df22b95..a70707bc1 100644 --- a/libglusterfs/src/fd.h +++ b/libglusterfs/src/fd.h @@ -22,6 +22,7 @@ #include "glusterfs.h" #include "locking.h" #include "fd-lk.h" +#include "common-utils.h" struct _inode; struct _dict; @@ -38,12 +39,8 @@ struct _fd_ctx { }; }; -/* If this structure changes, please have mercy on the booster maintainer - * and update the fd_t struct in booster/src/booster-fd.h. - * See the comment there to know why. - */ struct _fd { - uint64_t pid; + uint64_t pid; int32_t flags; int32_t refcount; struct list_head inode_list; @@ -53,6 +50,7 @@ struct _fd { struct _fd_ctx *_ctx; int xl_count; /* Number of xl referred in this fd */ struct fd_lk_ctx *lk_ctx; + gf_boolean_t anonymous; /* geo-rep anonymous fd */ }; typedef struct _fd fd_t; @@ -118,10 +116,6 @@ fd_t * fd_ref (fd_t *fd); -fd_t * -__fd_unref (fd_t *fd); - - void fd_unref (fd_t *fd); @@ -153,8 +147,6 @@ fd_list_empty (struct _inode *inode); fd_t * fd_bind (fd_t *fd); -fd_t * -__fd_bind (fd_t *fd); int fd_ctx_set (fd_t *fd, xlator_t *xlator, uint64_t value); @@ -167,7 +159,6 @@ fd_ctx_get (fd_t *fd, xlator_t *xlator, uint64_t *value); int fd_ctx_del (fd_t *fd, xlator_t *xlator, uint64_t *value); - int __fd_ctx_set (fd_t *fd, xlator_t *xlator, uint64_t value); @@ -176,20 +167,12 @@ int __fd_ctx_get (fd_t *fd, xlator_t *xlator, uint64_t *value); -int -__fd_ctx_del (fd_t *fd, xlator_t *xlator, uint64_t *value); - -fd_t * -__fd_ref (fd_t *fd); - void fd_ctx_dump (fd_t *fd, char *prefix); fdentry_t * gf_fd_fdtable_copy_all_fds (fdtable_t *fdtable, uint32_t *count); -fdentry_t * -__gf_fd_fdtable_copy_all_fds (fdtable_t *fdtable, uint32_t *count); void gf_fdptr_put (fdtable_t *fdtable, fd_t *fd); diff --git a/tests/bugs/bug-874272.t b/tests/bugs/bug-874272.t new file mode 100755 index 000000000..01793a368 --- /dev/null +++ b/tests/bugs/bug-874272.t @@ -0,0 +1,48 @@ +#!/bin/bash + +. $(dirname $0)/../include.rc + +cleanup; + +TEST glusterd +TEST pidof glusterd +TEST $CLI volume info; + +function volinfo_field() +{ + local vol=$1; + local field=$2; + + $CLI volume info $vol | grep "^$field: " | sed 's/.*: //'; +} + +TEST $CLI volume create $V0 $H0:$B0/brick1; +EXPECT 'Created' volinfo_field $V0 'Status'; + +TEST $CLI volume start $V0; +EXPECT 'Started' volinfo_field $V0 'Status'; + +## Wait for volume to register with rpc.mountd +sleep 5; + +#mount on a random dir +TEST MOUNTDIR="/tmp/$RANDOM" +TEST mkdir $MOUNTDIR +TEST mount -t nfs -o vers=3,nolock,soft,intr $H0:/$V0 $MOUNTDIR; +flag=0 + +TEST touch $MOUNTDIR/testfile + +TEST GEOREPDIR="/tmp/$RANDOM" +TEST mkdir $GEOREPDIR + +TEST $CLI volume geo-replication $V0 file:///$GEOREPDIR start + +for i in {1..500}; do cat /etc/passwd >> $MOUNTDIR/testfile; if [ $? -ne 0 ]; then flag=1; break; fi; done +TEST [ $flag -eq 0 ] +TEST rm -rf $GEOREPDIR + +TEST umount $MOUNTDIR +TEST rm -rf $MOUNTDIR + +cleanup; diff --git a/xlators/storage/posix/src/posix-helpers.c b/xlators/storage/posix/src/posix-helpers.c index c4f93100f..80d1e53a8 100644 --- a/xlators/storage/posix/src/posix-helpers.c +++ b/xlators/storage/posix/src/posix-helpers.c @@ -1014,7 +1014,7 @@ __posix_fd_ctx_get (fd_t *fd, xlator_t *this, struct posix_fd **pfd_p) goto out; } - if (fd->pid != -1) + if (!fd_is_anonymous(fd)) /* anonymous fd */ goto out; -- cgit