summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorRaghavendra Bhat <raghavendra@redhat.com>2014-07-17 12:15:54 +0530
committerVijay Bellur <vbellur@redhat.com>2014-09-12 10:55:06 -0700
commit5d6f55ed9f122d3aeab583bb0ad16cb0c392a339 (patch)
tree105890034c67f26a5499252023bfcd920fb8346f
parent2bce69036fdbd760f03fe55ac4954b5fa9febfeb (diff)
snapview-server: get the handle if its absent before doing any fop
* Now that NFS server does inode linking in readdirp, it can resolve the gfid (i.e. find the right inode from its inode table) present in the filehandle sent by the NFS client on which a fop came. So instead of sending the lookup on that entry, it directly sends the fop. But snapview-server does not get the handle for the entries in readdirp (because doing a lookup on each entry via gfapi would be costly. So it waits till a lookup is done on that inode, to get the handle and the fs instance and fill it in the inode context). So when NFS resoves the gfid and directly sends the fop, snapview-server will not be able to perform the fop as the inode contet would not contain the fs instance and the handle. So fops should check for the handle before doing gfapi calls. If the handle and fs instance are not present in the inode context they should get them by doing an explicit lookup on the entry. Change-Id: Idd648fbcc3ff6aadc3b63ff236561ca967b92f5d BUG: 1115949 Signed-off-by: Raghavendra Bhat <raghavendra@redhat.com> Reviewed-on: http://review.gluster.org/8324 Tested-by: Gluster Build System <jenkins@build.gluster.com> Reviewed-by: Vijay Bellur <vbellur@redhat.com>
-rw-r--r--tests/basic/uss.t22
-rw-r--r--xlators/features/snapview-server/src/snapview-server.c160
-rw-r--r--xlators/features/snapview-server/src/snapview-server.h29
3 files changed, 177 insertions, 34 deletions
diff --git a/tests/basic/uss.t b/tests/basic/uss.t
index f97cd59883e..04d4baf64a5 100644
--- a/tests/basic/uss.t
+++ b/tests/basic/uss.t
@@ -101,20 +101,31 @@ TEST fd_close $fd3
# test 44
TEST mount_nfs $H0:/$V0 $N0 nolock;
-TEST ls -l $N0/.snaps;
-
NUM_SNAPS=$(ls $N0/.snaps | wc -l);
TEST [ $NUM_SNAPS == 4 ];
+TEST stat $N0/.snaps/snap1;
+TEST stat $N0/.snaps/snap2;
+
+TEST ls -l $N0/.snaps;
+
+# readdir + lookup on each entry
TEST ls -l $N0/.snaps/snap1;
TEST ls -l $N0/.snaps/snap2;
-TEST ls -l $N0/.snaps/snap3;
-TEST ls -l $N0/.snaps/snap4;
+
+# readdir + access each entry by doing stat. If snapview-server has not
+# filled the fs instance and handle in the inode context of the entry as
+# part of readdirp, then when stat comes (i.e fop comes directly without
+# a previous lookup), snapview-server should do a lookup of the entry via
+# gfapi call and fill in the fs instance + handle information in the inode
+# context
+TEST ls $N0/.snaps/snap3/;
+TEST stat $N0/.snaps/snap3/dir1;
+TEST stat $N0/.snaps/snap3/dir2;
TEST ls -l $N0/.snaps/snap3/dir1;
TEST ls -l $N0/.snaps/snap3/dir2;
-
TEST ls -l $N0/.snaps/snap4/dir1;
TEST ls -l $N0/.snaps/snap4/dir2;
@@ -123,6 +134,7 @@ TEST ! ls -l $N0/dir2/.snaps/snap2;
TEST ls -l $N0/dir1/.snaps/snap3;
TEST ls -l $N0/dir2/.snaps/snap4;
+
TEST fd1=`fd_available`
TEST fd_open $fd1 'r' $N0/.snaps/snap1/file1;
TEST fd_cat $fd1
diff --git a/xlators/features/snapview-server/src/snapview-server.c b/xlators/features/snapview-server/src/snapview-server.c
index 21f3dbee563..a9c95850f3e 100644
--- a/xlators/features/snapview-server/src/snapview-server.c
+++ b/xlators/features/snapview-server/src/snapview-server.c
@@ -575,8 +575,10 @@ svs_opendir (call_frame_t *frame, xlator_t *this, loc_t *loc, fd_t *fd,
op_errno = 0;
goto out;
} else if (inode_ctx->type == SNAP_VIEW_VIRTUAL_INODE) {
- fs = inode_ctx->fs;
- object = inode_ctx->object;
+
+ SVS_GET_INODE_CTX_INFO(inode_ctx, fs, object, this, loc, op_ret,
+ op_errno, out);
+
glfd = glfs_h_opendir (fs, object);
if (!glfd) {
op_ret = -1;
@@ -701,8 +703,10 @@ svs_getxattr (call_frame_t *frame, xlator_t *this, loc_t *loc, const char *name,
op_errno = EINVAL;
goto out;
} else if (inode_ctx->type == SNAP_VIEW_VIRTUAL_INODE) {
- fs = inode_ctx->fs;
- object = inode_ctx->object;
+
+ SVS_GET_INODE_CTX_INFO(inode_ctx, fs, object, this, loc, op_ret,
+ op_errno, out);
+
dict = dict_new ();
if (!dict) {
gf_log (this->name, GF_LOG_ERROR, "failed to "
@@ -1439,6 +1443,118 @@ unwind:
return 0;
}
+/*
+ * This function is mainly helpful for NFS. Till now NFS server was not linking
+ * the inodes in readdirp, which caused problems when below operations were
+ * performed.
+ *
+ * 1) ls -l in one of the snaopshots (snapview-server would generate gfids for
+ * each entry on the fly and link the inodes associated with those entries)
+ * 2) NFS server upon getting readdirp reply would not link the inodes of the
+ * entries. But it used to generate filehandles for each entry and associate
+ * the gfid of that entry with the filehandle and send it as part of the
+ * reply to nfs client.
+ * 3) NFS client would send the filehandle of one of those entries when some
+ * activity is done on it.
+ * 4) NFS server would not be able to find the inode for the gfid present in the
+ * filehandle (as the inode was not linked) and would go for hard resolution
+ * by sending a lookup on the gfid by creating a new inode.
+ * 5) snapview-client will not able to identify whether the inode is a real
+ * inode existing in the main volume or a virtual inode existing in the
+ * snapshots as there would not be any inode context.
+ * 6) Since the gfid upon which lookup is sent is a virtual gfid which is not
+ * present in the disk, lookup would fail and the application would get an
+ * error.
+ *
+ * The above problem is fixed by the below commit which makes snapview server
+ * more compatible with nfs server (1dea949cb60c3814c9206df6ba8dddec8d471a94).
+ * But now because NFS server does inode linking in readdirp has introduced
+ * the below issue.
+ * In readdirp though snapview-server allocates inode contexts it does not
+ * actually perform lookup on each entry it obtained in readdirp (as doing
+ * a lookup via gfapi over the network for each entry would be costly).
+ *
+ * Till now it was not a problem with NFS server, as NFS was sending a lookup on
+ * the gfid it got from NFS client, for which it was not able to find the right
+ * inode. So snapview-server was able to get the fs instance (glfs_t) of the
+ * snapshot volume to which the entry belongs to, and the handle for the entry
+ * from the corresponding snapshot volume and fill those informations in the
+ * inode context.
+ *
+ * But now, since NFS server is able to find the inode from the inode table for
+ * the gfid it got from the NFS client, it wont send lookup. Rather it directly
+ * sends the fop it received from the client. Now this causes problems for
+ * snapview-server. Because for each fop snapview-server assumes that lookup has
+ * been performed on that entry and the entry's inode context contains the
+ * pointers for the fs instance and the handle to the entry in that fs. When NFS
+ * server sends the fop and snapview-server finds that the fs instance and the
+ * handle within the inode context are NULL it unwinds with EINVAL.
+ *
+ * So to handle this, if fs instance or handle within the inode context are
+ * NULL, then do a lookup based on parent inode context's fs instance. And
+ * unwind the results obtained as part of lookup
+ */
+
+int32_t
+svs_get_handle (xlator_t *this, loc_t *loc, svs_inode_t *inode_ctx,
+ int32_t *op_errno)
+{
+ svs_inode_t *parent_ctx = NULL;
+ int ret = -1;
+ inode_t *parent = NULL;
+ struct iatt postparent = {0, };
+ struct iatt buf = {0, };
+ char uuid1[64];
+
+ GF_VALIDATE_OR_GOTO ("snap-view-daemon", this, out);
+ GF_VALIDATE_OR_GOTO (this->name, loc, out);
+ GF_VALIDATE_OR_GOTO (this->name, loc->inode, out);
+
+ if (loc->path) {
+ if (!loc->name || (loc->name && !strcmp (loc->name, ""))) {
+ loc->name = strrchr (loc->path, '/');
+ if (loc->name)
+ loc->name++;
+ }
+ }
+
+ if (loc->parent)
+ parent = inode_ref (loc->parent);
+ else {
+ parent = inode_find (loc->inode->table, loc->pargfid);
+ if (!parent)
+ parent = inode_parent (loc->inode, NULL, NULL);
+ }
+
+ if (parent)
+ parent_ctx = svs_inode_ctx_get (this, parent);
+
+ if (!parent_ctx) {
+ gf_log (this->name, GF_LOG_WARNING, "failed to get the parent "
+ "context for %s (%s)", loc->path,
+ uuid_utoa_r (loc->inode->gfid, uuid1));
+ *op_errno = EINVAL;
+ goto out;
+ }
+
+ if (parent_ctx) {
+ if (parent_ctx->type == SNAP_VIEW_ENTRY_POINT_INODE)
+ ret = svs_lookup_snapshot (this, loc, &buf,
+ &postparent, parent,
+ parent_ctx, op_errno);
+ else
+ ret = svs_lookup_entry (this, loc, &buf,
+ &postparent, parent,
+ parent_ctx, op_errno);
+ }
+
+out:
+ if (parent)
+ inode_unref (parent);
+
+ return ret;
+}
+
int32_t
svs_stat (call_frame_t *frame, xlator_t *this, loc_t *loc, dict_t *xdata)
{
@@ -1477,8 +1593,10 @@ svs_stat (call_frame_t *frame, xlator_t *this, loc_t *loc, dict_t *xdata)
svs_iatt_fill (loc->inode->gfid, &buf);
op_ret = 0;
} else if (inode_ctx->type == SNAP_VIEW_VIRTUAL_INODE) {
- fs = inode_ctx->fs;
- object = inode_ctx->object;
+
+ SVS_GET_INODE_CTX_INFO(inode_ctx, fs, object, this, loc, op_ret,
+ op_errno, out);
+
ret = glfs_h_stat (fs, object, &stat);
if (ret) {
gf_log (this->name, GF_LOG_ERROR, "glfs_h_stat on %s "
@@ -1599,8 +1717,8 @@ svs_open (call_frame_t *frame, xlator_t *this, loc_t *loc, int32_t flags,
if (inode_ctx->type == SNAP_VIEW_ENTRY_POINT_INODE)
GF_ASSERT (0); // on entry point it should always be opendir
- fs = inode_ctx->fs;
- object = inode_ctx->object;
+ SVS_GET_INODE_CTX_INFO(inode_ctx, fs, object, this, loc, op_ret,
+ op_errno, out);
glfd = glfs_h_open (fs, object, flags);
if (!glfd) {
@@ -1749,25 +1867,8 @@ svs_readlink (call_frame_t *frame, xlator_t *this,
goto out;
}
- fs = inode_ctx->fs;
- if (!fs) {
- gf_log (this->name, GF_LOG_ERROR, "failed to get the fs "
- "instance for %s (gfid: %s)", loc->name,
- uuid_utoa (loc->inode->gfid));
- op_ret = -1;
- op_errno = ESTALE;
- goto out;
- }
-
- object = inode_ctx->object;
- if (!object) {
- gf_log (this->name, GF_LOG_ERROR, "failed to get the object "
- "for %s (gfid: %s)", loc->name,
- uuid_utoa (loc->inode->gfid));
- op_ret = -1;
- op_errno = ESTALE;
- goto out;
- }
+ SVS_GET_INODE_CTX_INFO(inode_ctx, fs, object, this, loc, op_ret,
+ op_errno, out);
ret = glfs_h_stat (fs, object, &stat);
if (ret) {
@@ -1850,8 +1951,9 @@ svs_access (call_frame_t *frame, xlator_t *this, loc_t *loc, int mask,
goto out;
}
- fs = inode_ctx->fs;
- object = inode_ctx->object;
+
+ SVS_GET_INODE_CTX_INFO(inode_ctx, fs, object, this, loc, op_ret,
+ op_errno, out);
/* The actual posix_acl xlator does acl checks differently for
fuse and nfs. So set frame->root->pid as fspid of the syncop
diff --git a/xlators/features/snapview-server/src/snapview-server.h b/xlators/features/snapview-server/src/snapview-server.h
index e689e4981a0..4b42aefcd98 100644
--- a/xlators/features/snapview-server/src/snapview-server.h
+++ b/xlators/features/snapview-server/src/snapview-server.h
@@ -53,6 +53,31 @@
STACK_DESTROY (((call_frame_t *)_frame)->root); \
} while (0)
+#define SVS_GET_INODE_CTX_INFO(inode_ctx, fs, object, this, loc, ret, \
+ op_errno, label) \
+ do { \
+ fs = inode_ctx->fs; \
+ object = inode_ctx->object; \
+ if (!fs || !object) { \
+ int32_t tmp = -1; \
+ char tmp_uuid[64]; \
+ \
+ tmp = svs_get_handle (this, loc, inode_ctx, \
+ &op_errno); \
+ if (tmp) { \
+ gf_log (this->name, GF_LOG_ERROR, \
+ "failed to get the handle for %s " \
+ "(gfid: %s)", loc->path, \
+ uuid_utoa_r (loc->inode->gfid, \
+ tmp_uuid)); \
+ ret = -1; \
+ goto label; \
+ } \
+ \
+ fs = inode_ctx->fs; \
+ object = inode_ctx->object; \
+ } \
+ } while(0);
int
svs_mgmt_submit_request (void *req, call_frame_t *frame,
@@ -168,4 +193,8 @@ svs_get_snap_dirent (xlator_t *this, const char *name);
int
svs_mgmt_init (xlator_t *this);
+int32_t
+svs_get_handle (xlator_t *this, loc_t *loc, svs_inode_t *inode_ctx,
+ int32_t *op_errno);
+
#endif /* __SNAP_VIEW_H__ */