features/snapview-server: check if the reference to the snapshot world is

correct before doing any fop

The following operations might lead to problems:
* Create a file on the glusterfs mount point
* Create a snapshot (say "snap1")
* Access the contents of the snapshot
* Delete the file from the mount point
* Delete the snapshot "snap1"
* Create a new snapshot "snap1"

Now accessing the new snapshot "snap1" gives problems. Because the inode and
dentry created for snap1 would not be deleted upon the deletion of the snapshot
(as deletion of snapshot is a gluster cli operation, not a fop). So next time
upon creation of a new snap with same name, the previous inode and dentry itself
will be used. But the inode context contains old information about the glfs_t
instance and the handle in the gfapi world. Directly accessing them without
proper check leads to ENOTCONN errors. Thus the glfs_t instance should be
checked before accessing. If its wrong, then right instance should be obtained
by doing the lookup.

Change-Id: Idca0c8015ff632447cea206a4807d8ef968424fa
BUG: 1151004
Signed-off-by: Raghavendra Bhat <raghavendra@redhat.com>
Reviewed-on: http://review.gluster.org/8917
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Reviewed-by: Vijay Bellur <vbellur@redhat.com>
This commit is contained in:
Raghavendra Bhat 2014-10-09 17:32:48 +05:30 committed by Vijay Bellur
parent 2855dff243
commit 1fa3e87db7
3 changed files with 75 additions and 8 deletions

View File

@ -274,4 +274,40 @@ function count_snaps
EXPECT_WITHIN 30 "5" count_snaps $M0;
# deletion of a snapshot and creation of a new snapshot with same name
# should not create problems. The data that was supposed to be present
# in the deleted snapshot need not be present in the new snapshot just
# because the name is same. Ex:
# 1) Create a file "aaa"
# 2) Create a snapshot snap6
# 3) stat the file "aaa" in snap6 and it should succeed
# 4) delete the file "aaa"
# 5) Delete the snapshot snap6
# 6) Create a snapshot snap6
# 7) stat the file "aaa" in snap6 and it should fail now
echo "aaa" > $M0/aaa;
TEST $CLI snapshot create snap6 $V0
TEST ls $M0/.history;
EXPECT_WITHIN 30 "6" count_snaps $M0;
TEST stat $M0/.history/snap6/aaa
TEST rm -f $M0/aaa;
TEST $CLI snapshot delete snap6;
TEST $CLI snapshot create snap6 $V0
TEST ls $M0/.history;
EXPECT_WITHIN 30 "6" count_snaps $M0;
TEST ls $M0/.history/snap6/;
TEST ! stat $M0/.history/snap6/aaa;
cleanup;

View File

@ -254,7 +254,7 @@ svs_lookup_snapshot (xlator_t *this, loc_t *loc, struct iatt *buf,
iatt_from_stat (buf, &statbuf);
uuid_copy (buf->ia_gfid, gfid);
svs_fill_ino_from_gfid (buf);
inode_ctx->type = SNAP_VIEW_VIRTUAL_INODE;
inode_ctx->type = SNAP_VIEW_SNAPSHOT_INODE;
inode_ctx->fs = fs;
inode_ctx->object = object;
memcpy (&inode_ctx->buf, buf, sizeof (*buf));
@ -574,7 +574,8 @@ svs_opendir (call_frame_t *frame, xlator_t *this, loc_t *loc, fd_t *fd,
op_ret = 0;
op_errno = 0;
goto out;
} else if (inode_ctx->type == SNAP_VIEW_VIRTUAL_INODE) {
}
else {
SVS_GET_INODE_CTX_INFO(inode_ctx, fs, object, this, loc, op_ret,
op_errno, out);
@ -702,7 +703,8 @@ svs_getxattr (call_frame_t *frame, xlator_t *this, loc_t *loc, const char *name,
op_ret = -1;
op_errno = EINVAL;
goto out;
} else if (inode_ctx->type == SNAP_VIEW_VIRTUAL_INODE) {
}
else {
SVS_GET_INODE_CTX_INFO(inode_ctx, fs, object, this, loc, op_ret,
op_errno, out);
@ -832,8 +834,7 @@ svs_fgetxattr (call_frame_t *frame, xlator_t *this, fd_t *fd, const char *name,
op_errno = EINVAL;
goto out;
}
if (inode_ctx->type == SNAP_VIEW_VIRTUAL_INODE) {
else {
dict = dict_new ();
if (!dict) {
gf_log (this->name, GF_LOG_ERROR, "failed to "
@ -1270,16 +1271,17 @@ svs_readdirp_fill (xlator_t *this, inode_t *parent, svs_inode_t *parent_ctx,
goto out;
}
inode_ctx->type = SNAP_VIEW_VIRTUAL_INODE;
if (parent_ctx->type == SNAP_VIEW_ENTRY_POINT_INODE) {
buf.ia_type = IA_IFDIR;
inode_ctx->buf = buf;
entry->d_stat = buf;
inode_ctx->type = SNAP_VIEW_SNAPSHOT_INODE;
} else {
uuid_copy (entry->d_stat.ia_gfid, buf.ia_gfid);
entry->d_stat.ia_ino = buf.ia_ino;
inode_ctx->buf = entry->d_stat;
inode_ctx->type = SNAP_VIEW_VIRTUAL_INODE;
}
}
@ -1592,7 +1594,8 @@ svs_stat (call_frame_t *frame, xlator_t *this, loc_t *loc, dict_t *xdata)
if (inode_ctx->type == SNAP_VIEW_ENTRY_POINT_INODE) {
svs_iatt_fill (loc->inode->gfid, &buf);
op_ret = 0;
} else if (inode_ctx->type == SNAP_VIEW_VIRTUAL_INODE) {
}
else {
SVS_GET_INODE_CTX_INFO(inode_ctx, fs, object, this, loc, op_ret,
op_errno, out);
@ -1655,7 +1658,8 @@ svs_fstat (call_frame_t *frame, xlator_t *this, fd_t *fd, dict_t *xdata)
if (inode_ctx->type == SNAP_VIEW_ENTRY_POINT_INODE) {
svs_iatt_fill (fd->inode->gfid, &buf);
op_ret = 0;
} else if (inode_ctx->type == SNAP_VIEW_VIRTUAL_INODE) {
}
else {
sfd = svs_fd_ctx_get_or_new (this, fd);
if (!sfd) {
gf_log (this->name, GF_LOG_ERROR, "failed to get the "

View File

@ -53,11 +53,37 @@
STACK_DESTROY (((call_frame_t *)_frame)->root); \
} while (0)
#define SVS_CHECK_VALID_SNAPSHOT_HANDLE(fs, this) \
do { \
svs_private_t *_private = NULL; \
_private = this->private; \
int i = 0; \
gf_boolean_t found = _gf_false; \
LOCK (&_private->snaplist_lock); \
{ \
for (i = 0; i < _private->num_snaps; i++) { \
if (_private->dirents->fs && fs && \
_private->dirents->fs == fs) { \
found = _gf_true; \
break; \
} \
} \
} \
UNLOCK (&_private->snaplist_lock); \
\
if (!found) \
fs = NULL; \
} 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; \
SVS_CHECK_VALID_SNAPSHOT_HANDLE (fs, this); \
if (!fs) \
object = NULL; \
\
if (!fs || !object) { \
int32_t tmp = -1; \
char tmp_uuid[64]; \
@ -94,6 +120,7 @@ mgmt_get_snapinfo_cbk (struct rpc_req *req, struct iovec *iov,
typedef enum {
SNAP_VIEW_ENTRY_POINT_INODE = 0,
SNAP_VIEW_SNAPSHOT_INODE,
SNAP_VIEW_VIRTUAL_INODE
} inode_type_t;