summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorRaghavendra Bhat <raghavendra@redhat.com>2015-06-12 15:12:05 +0530
committerNiels de Vos <ndevos@redhat.com>2015-07-06 08:14:02 -0700
commit69c434432853e2ba1ee53296f05c6a54ab300d02 (patch)
tree5a72c7c4d03bd57b47ab143c2b7e114e405d9f01
parent36a5c9beb60fe34c24a6fd7bf222f16e62a679ef (diff)
libgfapi: send explicit lookups on inodes linked in readdirp
Backport of http://review.gluster.org/11236 If the inode is linked via readdirp, then the consuners of gfapi which are using handles (got either in lookup or readdirp) might not send an explicit lookup on that object again (ex: NFS, samba, USS). If there is a replicate volume where the replicas of the object are not in sync, then readdirp followed by fops might lead data being served from the subvolume which is not in sync with latest data. And since lookup is needed to trigger self-heal on that object the consumers might keep getting wrong data until an explicit lookup is not done. Fuse handles this situation by sending an explicit lookup by itself (fuse xlator) on those inodes which are linked via readdirp, whenever a fop comes on that inode. The same procedure is done in gfapi as well to address this situation. Thanks to shyam(srangana@redhat.com) for valuable inputs Change-Id: I4230fae8e0b01a95c056282b08ed30832d4804a7 BUG: 1240190 Signed-off-by: Raghavendra Bhat <raghavendra@redhat.com> Reviewed-on: http://review.gluster.org/11545 Tested-by: NetBSD Build System <jenkins@build.gluster.org> Tested-by: Gluster Build System <jenkins@build.gluster.com> Reviewed-by: Shyamsundar Ranganathan <srangana@redhat.com> Reviewed-by: Niels de Vos <ndevos@redhat.com>
-rw-r--r--api/src/glfs-fops.c16
-rw-r--r--api/src/glfs-resolve.c49
-rw-r--r--libglusterfs/src/inode.c31
-rw-r--r--libglusterfs/src/inode.h6
-rw-r--r--xlators/mount/fuse/src/fuse-bridge.c35
-rw-r--r--xlators/mount/fuse/src/fuse-resolve.c8
6 files changed, 92 insertions, 53 deletions
diff --git a/api/src/glfs-fops.c b/api/src/glfs-fops.c
index e1762ae1285..b66f336338d 100644
--- a/api/src/glfs-fops.c
+++ b/api/src/glfs-fops.c
@@ -2270,6 +2270,7 @@ glfd_entry_refresh (struct glfs_fd *glfd, int plus)
xlator_t *subvol = NULL;
gf_dirent_t entries;
gf_dirent_t old;
+ gf_dirent_t *entry = NULL;
int ret = -1;
fd_t *fd = NULL;
@@ -2304,8 +2305,20 @@ glfd_entry_refresh (struct glfs_fd *glfd, int plus)
&entries, NULL, NULL);
DECODE_SYNCOP_ERR (ret);
if (ret >= 0) {
- if (plus)
+ if (plus) {
+ /**
+ * Set inode_needs_lookup flag before linking the
+ * inode. Doing it later post linkage might lead
+ * to a race where a fop comes after inode link
+ * but before setting need_lookup flag.
+ */
+ list_for_each_entry (entry, &glfd->entries, list) {
+ if (entry->inode)
+ inode_set_need_lookup (entry->inode, THIS);
+ }
+
gf_link_inodes_from_dirent (THIS, fd->inode, &entries);
+ }
list_splice_init (&glfd->entries, &old.list);
list_splice_init (&entries.list, &glfd->entries);
@@ -2314,6 +2327,7 @@ glfd_entry_refresh (struct glfs_fd *glfd, int plus)
errno = 0;
}
+
if (ret > 0)
glfd->next = list_entry (glfd->entries.next, gf_dirent_t, list);
diff --git a/api/src/glfs-resolve.c b/api/src/glfs-resolve.c
index 2767abf1c39..b5efcbae0e7 100644
--- a/api/src/glfs-resolve.c
+++ b/api/src/glfs-resolve.c
@@ -75,27 +75,44 @@ __glfs_first_lookup (struct glfs *fs, xlator_t *subvol)
}
+/**
+ * We have to check if need_lookup flag is set in both old and the new inodes.
+ * If its set in oldinode, then directly go ahead and do an explicit lookup.
+ * But if its not set in the oldinode, then check if the newinode is linked
+ * via readdirp. If so an explicit lookup is needed on the new inode, so that
+ * below xlators can set their respective contexts.
+ */
inode_t *
-glfs_refresh_inode_safe (xlator_t *subvol, inode_t *oldinode)
+glfs_refresh_inode_safe (xlator_t *subvol, inode_t *oldinode,
+ gf_boolean_t need_lookup)
{
loc_t loc = {0, };
int ret = -1;
struct iatt iatt = {0, };
inode_t *newinode = NULL;
+ gf_boolean_t lookup_needed = _gf_false;
if (!oldinode)
return NULL;
- if (oldinode->table->xl == subvol)
+ if (!need_lookup && oldinode->table->xl == subvol)
return inode_ref (oldinode);
newinode = inode_find (subvol->itable, oldinode->gfid);
- if (newinode)
- return newinode;
+ if (!need_lookup && newinode) {
+
+ lookup_needed = inode_needs_lookup (newinode, THIS);
+ if (!lookup_needed)
+ return newinode;
+ }
gf_uuid_copy (loc.gfid, oldinode->gfid);
- loc.inode = inode_new (subvol->itable);
+ if (!newinode)
+ loc.inode = inode_new (subvol->itable);
+ else
+ loc.inode = newinode;
+
if (!loc.inode)
return NULL;
@@ -122,14 +139,15 @@ glfs_refresh_inode_safe (xlator_t *subvol, inode_t *oldinode)
inode_t *
-__glfs_refresh_inode (struct glfs *fs, xlator_t *subvol, inode_t *inode)
+__glfs_refresh_inode (struct glfs *fs, xlator_t *subvol, inode_t *inode,
+ gf_boolean_t need_lookup)
{
inode_t *newinode = NULL;
fs->migration_in_progress = 1;
pthread_mutex_unlock (&fs->mutex);
{
- newinode = glfs_refresh_inode_safe (subvol, inode);
+ newinode = glfs_refresh_inode_safe (subvol, inode, need_lookup);
}
pthread_mutex_lock (&fs->mutex);
fs->migration_in_progress = 0;
@@ -622,7 +640,7 @@ glfs_migrate_fd_safe (struct glfs *fs, xlator_t *newsubvol, fd_t *oldfd)
}
}
- newinode = glfs_refresh_inode_safe (newsubvol, oldinode);
+ newinode = glfs_refresh_inode_safe (newsubvol, oldinode, _gf_false);
if (!newinode) {
gf_msg (fs->volname, GF_LOG_WARNING, errno,
API_MSG_INODE_REFRESH_FAILED,
@@ -810,7 +828,8 @@ __glfs_active_subvol (struct glfs *fs)
}
if (fs->cwd) {
- new_cwd = __glfs_refresh_inode (fs, new_subvol, fs->cwd);
+ new_cwd = __glfs_refresh_inode (fs, new_subvol, fs->cwd,
+ _gf_false);
if (!new_cwd) {
char buf1[64];
@@ -904,7 +923,8 @@ int
__glfs_cwd_set (struct glfs *fs, inode_t *inode)
{
if (inode->table->xl != fs->active_subvol) {
- inode = __glfs_refresh_inode (fs, fs->active_subvol, inode);
+ inode = __glfs_refresh_inode (fs, fs->active_subvol, inode,
+ _gf_false);
if (!inode)
return -1;
} else {
@@ -948,7 +968,7 @@ __glfs_cwd_get (struct glfs *fs)
return cwd;
}
- cwd = __glfs_refresh_inode (fs, fs->active_subvol, fs->cwd);
+ cwd = __glfs_refresh_inode (fs, fs->active_subvol, fs->cwd, _gf_false);
return cwd;
}
@@ -972,12 +992,15 @@ __glfs_resolve_inode (struct glfs *fs, xlator_t *subvol,
struct glfs_object *object)
{
inode_t *inode = NULL;
+ gf_boolean_t lookup_needed = _gf_false;
+
+ lookup_needed = inode_needs_lookup (object->inode, THIS);
- if (object->inode->table->xl == subvol)
+ if (!lookup_needed && object->inode->table->xl == subvol)
return inode_ref (object->inode);
inode = __glfs_refresh_inode (fs, fs->active_subvol,
- object->inode);
+ object->inode, lookup_needed);
if (!inode)
return NULL;
diff --git a/libglusterfs/src/inode.c b/libglusterfs/src/inode.c
index 7d3215ed16e..579b94ca036 100644
--- a/libglusterfs/src/inode.c
+++ b/libglusterfs/src/inode.c
@@ -1775,6 +1775,37 @@ out:
return inode;
}
+void
+inode_set_need_lookup (inode_t *inode, xlator_t *this)
+{
+ uint64_t need_lookup = 1;
+
+ if (!inode | !this)
+ return;
+
+ inode_ctx_set (inode, this, &need_lookup);
+
+ return;
+}
+
+gf_boolean_t
+inode_needs_lookup (inode_t *inode, xlator_t *this)
+{
+ uint64_t need_lookup = 0;
+ gf_boolean_t ret = _gf_false;
+
+ if (!inode || !this)
+ return ret;
+
+ inode_ctx_get (inode, this, &need_lookup);
+ if (need_lookup) {
+ ret = _gf_true;
+ need_lookup = 0;
+ inode_ctx_set (inode, this, &need_lookup);
+ }
+
+ return ret;
+}
int
__inode_ctx_set2 (inode_t *inode, xlator_t *xlator, uint64_t *value1_p,
diff --git a/libglusterfs/src/inode.h b/libglusterfs/src/inode.h
index 474dc3992fc..b8bac7932c3 100644
--- a/libglusterfs/src/inode.h
+++ b/libglusterfs/src/inode.h
@@ -272,4 +272,10 @@ inode_ctx_merge (fd_t *fd, inode_t *inode, inode_t *linked_inode);
int
inode_is_linked (inode_t *inode);
+void
+inode_set_need_lookup (inode_t *inode, xlator_t *this);
+
+gf_boolean_t
+inode_needs_lookup (inode_t *inode, xlator_t *this);
+
#endif /* _INODE_H */
diff --git a/xlators/mount/fuse/src/fuse-bridge.c b/xlators/mount/fuse/src/fuse-bridge.c
index 84efd6ca510..b67a60a76b1 100644
--- a/xlators/mount/fuse/src/fuse-bridge.c
+++ b/xlators/mount/fuse/src/fuse-bridge.c
@@ -63,39 +63,6 @@ fuse_forget_cbk (xlator_t *this, inode_t *inode)
return 0;
}
-void
-fuse_inode_set_need_lookup (inode_t *inode, xlator_t *this)
-{
- uint64_t need_lookup = 1;
-
- if (!inode || !this)
- return;
-
- inode_ctx_set (inode, this, &need_lookup);
-
- return;
-}
-
-
-gf_boolean_t
-fuse_inode_needs_lookup (inode_t *inode, xlator_t *this)
-{
- uint64_t need_lookup = 0;
- gf_boolean_t ret = _gf_false;
-
- if (!inode || !this)
- return ret;
-
- inode_ctx_get (inode, this, &need_lookup);
- if (need_lookup)
- ret = _gf_true;
- need_lookup = 0;
- inode_ctx_set (inode, this, &need_lookup);
-
- return ret;
-}
-
-
fuse_fd_ctx_t *
__fuse_fd_ctx_check_n_create (xlator_t *this, fd_t *fd)
{
@@ -2787,7 +2754,7 @@ fuse_readdirp_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
feo->nodeid = inode_to_fuse_nodeid (linked_inode);
- fuse_inode_set_need_lookup (linked_inode, this);
+ inode_set_need_lookup (linked_inode, this);
inode_unref (linked_inode);
diff --git a/xlators/mount/fuse/src/fuse-resolve.c b/xlators/mount/fuse/src/fuse-resolve.c
index 2ddb31cd076..a670f4f6dac 100644
--- a/xlators/mount/fuse/src/fuse-resolve.c
+++ b/xlators/mount/fuse/src/fuse-resolve.c
@@ -26,8 +26,6 @@ int fuse_migrate_fd (xlator_t *this, fd_t *fd, xlator_t *old_subvol,
fuse_fd_ctx_t *
fuse_fd_ctx_get (xlator_t *this, fd_t *fd);
-gf_boolean_t fuse_inode_needs_lookup (inode_t *inode, xlator_t *this);
-
static int
fuse_resolve_loc_touchup (fuse_state_t *state)
{
@@ -229,7 +227,7 @@ fuse_resolve_parent_simple (fuse_state_t *state)
parent = resolve->parhint;
if (parent->table == state->itable) {
- if (fuse_inode_needs_lookup (parent, THIS))
+ if (inode_needs_lookup (parent, THIS))
return 1;
/* no graph switches since */
@@ -258,7 +256,7 @@ fuse_resolve_parent_simple (fuse_state_t *state)
/* non decisive result - parent missing */
return 1;
}
- if (fuse_inode_needs_lookup (parent, THIS)) {
+ if (inode_needs_lookup (parent, THIS)) {
inode_unref (parent);
return 1;
}
@@ -317,7 +315,7 @@ fuse_resolve_inode_simple (fuse_state_t *state)
inode = inode_find (state->itable, resolve->gfid);
if (inode) {
- if (!fuse_inode_needs_lookup (inode, THIS))
+ if (!inode_needs_lookup (inode, THIS))
goto found;
/* inode was linked through readdirplus */
inode_unref (inode);