andy/glusterfs
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

features/snapview-server: validate the fs instance before doing fop there

Browse Source 
PROBLEM:
    ========

    USS design depends on snapview-server translator communicating with each
    individual snapshot via gfapi. So, the snapview-server xlator maintains
    the glfs instance (thus the snapshot) to which a inode belongs to by
    storing it inside the inode context.

    Suppose, a file from a snapshot is opened by a application, and the fd
    is still valid from application's point of view (i.e. application has
    not yet closed fd). Now, if the snapshot to which the opened file
    belongs to is deleted, then the glfs_t instance corresponding to the
    snapshot is destroyed by snapview-server as part of snap deletion.
    But now, if the application does IO on the fd it has kept open, then
    snapview server tries to send that request to the corresponding snap
    via glfs instance for that snapshot stored in the inode context for
    the file on which the application is sending the fop. And this results
    in freed up glfs_t pointer being accessed and causes a segfault.

    FIX:
    ===

    For fd based operations, check whether the glfs instance that the inode
    contains in its context, is still valid or not.

    For non fd based operations, usually lookup should guarantee that. But
    if the file was already looked up, and the client accessing the snap data
    (either NFS, or native glusterfs fuse) does not bother to send a lookup
    and directly sends a path based fop, then that path based fop should
    ensure that the fs instance is valid.

Change-Id: I881be15ec46ecb51aa844d7fd41d5630f0d644fb
updates: bz#1602070
Signed-off-by: Raghavendra Bhat <raghavendra@redhat.com>
This commit is contained in:
Raghavendra Bhat 2018-06-08 09:54:00 -04:00 committed by Amar Tumballi
parent 11a9f9f0dc
commit f191bb7bc1
4 changed files with 194 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

74
tests/basic/open-fd-snap-delete.t Normal file
View File

@ -0,0 +1,74 @@
#!/bin/bash
. $(dirname $0)/../include.rc
. $(dirname $0)/../volume.rc
. $(dirname $0)/../snapshot.rc
. $(dirname $0)/../fileio.rc
cleanup;
TEST init_n_bricks 3;
TEST setup_lvm 3;
# start glusterd
TEST glusterd;
TEST pidof glusterd;
TEST $CLI volume create $V0 $H0:$L1 $H0:$L2 $H0:$L3;
TEST $CLI volume set $V0 nfs.disable false
TEST $CLI volume start $V0;
TEST $GFS --volfile-server=$H0 --volfile-id=$V0 $M0;
for i in {1..10} ; do echo "file" > $M0/file$i ; done
# Create file and directory
TEST touch $M0/f1
TEST mkdir $M0/dir
TEST $CLI snapshot config activate-on-create enable
TEST $CLI volume set $V0 features.uss enable;
for i in {1..10} ; do echo "file" > $M0/dir/file$i ; done
TEST $CLI snapshot create snap1 $V0 no-timestamp;
for i in {11..20} ; do echo "file" > $M0/file$i ; done
for i in {11..20} ; do echo "file" > $M0/dir/file$i ; done
TEST $CLI snapshot create snap2 $V0 no-timestamp;
TEST fd1=`fd_available`
TEST fd_open $fd1 'r' $M0/.snaps/snap2/dir/file11;
TEST fd_cat $fd1
TEST $CLI snapshot delete snap2;
TEST ! fd_cat $fd1;
# the return value of this command (i.e. fd_close) depetends
# mainly on how the release operation on a file descriptor is
# handled in snapview-server process. As of now snapview-server
# returns 0 for the release operation. And it is similar to how
# posix xlator does. So, as of now the expectation is to receive
# success for the close operation.
TEST fd_close $fd1;
# This check is mainly to ensure that the snapshot daemon
# (snapd) is up and running. If it is not running, the following
# stat would receive ENOTCONN.
TEST stat $M0/.snaps/snap1/dir/file1
TEST $CLI snapshot delete snap1;
TEST rm -rf $M0/*;
TEST $CLI volume stop $V0;
TEST $CLI volume delete $V0;
cleanup

35
xlators/features/snapview-server/src/snapview-server-helpers.c
View File

@ -595,3 +595,38 @@ svs_get_latest_snapshot (xlator_t *this)
out:
return fs;
}
glfs_t *
svs_inode_ctx_glfs_mapping (xlator_t *this, svs_inode_t *inode_ctx)
{
glfs_t *fs = NULL;
GF_VALIDATE_OR_GOTO ("svs", this, out);
GF_VALIDATE_OR_GOTO (this->name, inode_ctx, out);
fs = inode_ctx->fs;
SVS_CHECK_VALID_SNAPSHOT_HANDLE (fs, this);
out:
return fs;
}
glfs_t *
svs_inode_glfs_mapping (xlator_t *this, inode_t *inode)
{
svs_inode_t *inode_ctx = NULL;
glfs_t *fs = NULL;
inode_ctx = svs_inode_ctx_get (this, inode);
if (!inode_ctx) {
gf_log (this->name, GF_LOG_ERROR, "inode context not found for"
" the inode %s", uuid_utoa (inode->gfid));
goto out;
}
fs = svs_inode_ctx_glfs_mapping (this, inode_ctx);
out:
return fs;
}

109
xlators/features/snapview-server/src/snapview-server.c
View File

@ -543,7 +543,6 @@ svs_lookup (call_frame_t *frame, xlator_t *this, loc_t *loc, dict_t *xdata)
svs_inode_t *parent_ctx = NULL;
int32_t ret = -1;
inode_t *parent = NULL;
snap_dirent_t *dirent = NULL;
gf_boolean_t entry_point_key = _gf_false;
gf_boolean_t entry_point = _gf_false;
call_stack_t *root = NULL;
@ -592,13 +591,6 @@ svs_lookup (call_frame_t *frame, xlator_t *this, loc_t *loc, dict_t *xdata)
inode_ctx = svs_inode_ctx_get (this, loc->inode);
/* Initialize latest snapshot, which is used for nameless lookups */
dirent = svs_get_latest_snap_entry (this);
if (dirent && !dirent->fs) {
svs_initialise_snapshot_volume (this, dirent->name, NULL);
}
if (xdata && !inode_ctx) {
ret = dict_get_str_boolean (xdata, "entry-point", _gf_false);
if (ret == -1) {
@ -996,6 +988,17 @@ svs_fgetxattr (call_frame_t *frame, xlator_t *this, fd_t *fd, const char *name,
goto out;
}
if (!(svs_inode_ctx_glfs_mapping (this, inode_ctx))) {
gf_log (this->name, GF_LOG_ERROR, "glfs instance "
"instance %p to which the inode %s belongs"
"to does not exist. That snapshot might have"
"been deleted or deactivated", inode_ctx->fs,
uuid_utoa (fd->inode->gfid));
op_ret = -1;
op_errno = EBADF;
goto out;
}
sfd = svs_fd_ctx_get_or_new (this, fd);
if (!sfd) {
gf_log (this->name, GF_LOG_ERROR, "failed to get the fd "
@ -1130,9 +1133,12 @@ out:
int32_t
svs_releasedir (xlator_t *this, fd_t *fd)
{
svs_fd_t *sfd = NULL;
uint64_t tmp_pfd = 0;
int ret = 0;
svs_fd_t *sfd = NULL;
uint64_t tmp_pfd = 0;
int ret = 0;
svs_inode_t *svs_inode = NULL;
glfs_t *fs = NULL;
inode_t *inode = NULL;
GF_VALIDATE_OR_GOTO ("snapview-server", this, out);
GF_VALIDATE_OR_GOTO (this->name, fd, out);
@ -1144,13 +1150,24 @@ svs_releasedir (xlator_t *this, fd_t *fd)
goto out;
}
sfd = (svs_fd_t *)(long)tmp_pfd;
if (sfd->fd) {
ret = glfs_closedir (sfd->fd);
if (ret)
gf_log (this->name, GF_LOG_WARNING, "failed to close "
"the glfd for directory %s",
uuid_utoa (fd->inode->gfid));
inode = fd->inode;
svs_inode = svs_inode_ctx_get (this, inode);
if (svs_inode) {
fs = svs_inode->fs; /* should inode->lock be held for this? */
SVS_CHECK_VALID_SNAPSHOT_HANDLE (fs, this);
if (fs) {
sfd = (svs_fd_t *)(long)tmp_pfd;
if (sfd->fd) {
ret = glfs_closedir (sfd->fd);
if (ret)
gf_log (this->name, GF_LOG_WARNING,
"failed to close the glfd"
"for directory %s",
uuid_utoa (fd->inode->gfid));
}
}
}
GF_FREE (sfd);
@ -1209,9 +1226,12 @@ out:
int32_t
svs_release (xlator_t *this, fd_t *fd)
{
svs_fd_t *sfd = NULL;
uint64_t tmp_pfd = 0;
int ret = 0;
svs_fd_t *sfd = NULL;
uint64_t tmp_pfd = 0;
int ret = 0;
inode_t *inode = NULL;
svs_inode_t *svs_inode = NULL;
glfs_t *fs = NULL;
GF_VALIDATE_OR_GOTO ("snapview-server", this, out);
GF_VALIDATE_OR_GOTO (this->name, fd, out);
@ -1223,13 +1243,22 @@ svs_release (xlator_t *this, fd_t *fd)
goto out;
}
sfd = (svs_fd_t *)(long)tmp_pfd;
if (sfd->fd) {
ret = glfs_close (sfd->fd);
if (ret) {
gf_log (this->name, GF_LOG_ERROR, "failed to close "
"the glfd for %s",
uuid_utoa (fd->inode->gfid));
inode = fd->inode;
svs_inode = svs_inode_ctx_get (this, inode);
if (svs_inode) {
fs = svs_inode->fs; /* should inode->lock be held for this? */
SVS_CHECK_VALID_SNAPSHOT_HANDLE (fs, this);
if (fs) {
sfd = (svs_fd_t *)(long)tmp_pfd;
if (sfd->fd) {
ret = glfs_close (sfd->fd);
if (ret)
gf_log (this->name, GF_LOG_ERROR,
"failed to close "
"the glfd for %s",
uuid_utoa (fd->inode->gfid));
}
}
}
@ -1893,8 +1922,18 @@ 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 {
} else {
if (!(svs_inode_ctx_glfs_mapping (this, inode_ctx))) {
gf_log (this->name, GF_LOG_ERROR, "glfs instance "
"instance %p to which the inode %s belongs "
"to does not exist. That snapshot might have "
"been deleted or deactivated", inode_ctx->fs,
uuid_utoa (fd->inode->gfid));
op_ret = -1;
op_errno = EBADF;
goto out;
}
sfd = svs_fd_ctx_get_or_new (this, fd);
if (!sfd) {
gf_log (this->name, GF_LOG_ERROR, "failed to get the "
@ -2084,6 +2123,16 @@ svs_readv (call_frame_t *frame, xlator_t *this,
goto out;
}
if (!svs_inode_glfs_mapping (this, fd->inode)) {
gf_log (this->name, GF_LOG_ERROR, "glfs instance to "
"which the inode %s receiving read request belongs, "
"does not exist anymore",
uuid_utoa (fd->inode->gfid));
op_ret = -1;
op_errno = EBADF; /* should this be some other error? */
goto out;
}
sfd = svs_fd_ctx_get_or_new (this, fd);
if (!sfd) {
gf_log (this->name, GF_LOG_ERROR, "failed to get the fd "

6
xlators/features/snapview-server/src/snapview-server.h
View File

@ -242,4 +242,10 @@ int32_t
svs_get_handle (xlator_t *this, loc_t *loc, svs_inode_t *inode_ctx,
int32_t *op_errno);
glfs_t *
svs_inode_glfs_mapping (xlator_t *this, inode_t *inode);
glfs_t *
svs_inode_ctx_glfs_mapping (xlator_t *this, svs_inode_t *inode_ctx);
#endif /* __SNAP_VIEW_H__ */