summaryrefslogtreecommitdiffstats
path: root/xlators/features/snapview-client
diff options
context:
space:
mode:
authorRaghavendra Bhat <raghavendra@redhat.com>2018-12-03 11:23:20 -0500
committerAmar Tumballi <amarts@redhat.com>2018-12-17 12:25:33 +0000
commitbdcb2d8497d77ff28cb031ae3992eb7ea0c90486 (patch)
tree54d309a1d7d1ad95410ebb5379612a0ad44d635d /xlators/features/snapview-client
parent403c69d35827b6cbb430e97a797c318cca81e86e (diff)
features/snapview-client: access priv->path inside lock
To handle the race condition of a fop or a function accessing priv->path and a reconfigure changing priv->path (because entry point directory changed), the private structure's path is guarded by the lock. updates bz#1650403 Change-Id: I61c539da06d68d38eafcf2155699c7702f31323e Signed-off-by: Raghavendra Bhat <raghavendra@redhat.com>
Diffstat (limited to 'xlators/features/snapview-client')
-rw-r--r--xlators/features/snapview-client/src/snapview-client-messages.h4
-rw-r--r--xlators/features/snapview-client/src/snapview-client.c426
-rw-r--r--xlators/features/snapview-client/src/snapview-client.h5
3 files changed, 360 insertions, 75 deletions
diff --git a/xlators/features/snapview-client/src/snapview-client-messages.h b/xlators/features/snapview-client/src/snapview-client-messages.h
index d5215dcecf9..f6b8f48ef72 100644
--- a/xlators/features/snapview-client/src/snapview-client-messages.h
+++ b/xlators/features/snapview-client/src/snapview-client-messages.h
@@ -31,6 +31,8 @@ GLFS_MSGID(SNAPVIEW_CLIENT, SVC_MSG_NO_MEMORY, SVC_MSG_MEM_ACNT_FAILED,
SVC_MSG_XLATOR_CHILDREN_WRONG, SVC_MSG_NORMAL_GRAPH_LOOKUP_FAIL,
SVC_MSG_SNAPVIEW_GRAPH_LOOKUP_FAIL, SVC_MSG_OPENDIR_SPECIAL_DIR,
SVC_MSG_RENAME_SNAPSHOT_ENTRY, SVC_MSG_LINK_SNAPSHOT_ENTRY,
- SVC_MSG_ENTRY_POINT_SPECIAL_DIR);
+ SVC_MSG_COPY_ENTRY_POINT_FAILED, SVC_MSG_ENTRY_POINT_SPECIAL_DIR,
+ SVC_MSG_STR_LEN, SVC_MSG_INVALID_ENTRY_POINT, SVC_MSG_NULL_PRIV,
+ SVC_MSG_PRIV_DESTROY_FAILED);
#endif /* !_SNAPVIEW_CLIENT_MESSAGES_H_ */
diff --git a/xlators/features/snapview-client/src/snapview-client.c b/xlators/features/snapview-client/src/snapview-client.c
index 0322ac82f86..5d7986c7f0f 100644
--- a/xlators/features/snapview-client/src/snapview-client.c
+++ b/xlators/features/snapview-client/src/snapview-client.c
@@ -238,6 +238,52 @@ out:
return svc_fd;
}
+/**
+ * @this: xlator
+ * @entry_point: pointer to the buffer provided by consumer
+ *
+ * This function is mainly for copying the entry point name
+ * (stored as string in priv->path) to a buffer point to by
+ * @entry_point within the lock. It is for the consumer to
+ * allocate the memory for the buffer.
+ *
+ * This function is called by all the functions (or fops)
+ * who need to use priv->path for avoiding the race.
+ * For example, either in lookup or in any other fop,
+ * while priv->path is being accessed, a reconfigure can
+ * happen to change priv->path. This ensures that, a lock
+ * is taken before accessing priv->path.
+ **/
+int
+gf_svc_get_entry_point(xlator_t *this, char *entry_point, size_t dest_size)
+{
+ int ret = -1;
+ svc_private_t *priv = NULL;
+
+ GF_VALIDATE_OR_GOTO("snapview-client", this, out);
+ GF_VALIDATE_OR_GOTO(this->name, entry_point, out);
+
+ priv = this->private;
+
+ LOCK(&priv->lock);
+ {
+ if (dest_size <= strlen(priv->path)) {
+ gf_msg(this->name, GF_LOG_ERROR, 0, SVC_MSG_STR_LEN,
+ "destination buffer "
+ "size %zu is less than the length %zu of "
+ "the entry point name %s",
+ dest_size, strlen(priv->path), priv->path);
+ } else {
+ snprintf(entry_point, dest_size, "%s", priv->path);
+ ret = 0;
+ }
+ }
+ UNLOCK(&priv->lock);
+
+out:
+ return ret;
+}
+
static int32_t
gf_svc_lookup_cbk(call_frame_t *frame, void *cookie, xlator_t *this,
int32_t op_ret, int32_t op_errno, inode_t *inode,
@@ -341,20 +387,19 @@ gf_svc_lookup(call_frame_t *frame, xlator_t *this, loc_t *loc, dict_t *xdata)
int op_ret = -1;
int op_errno = EINVAL;
inode_t *parent = NULL;
- svc_private_t *priv = NULL;
dict_t *new_xdata = NULL;
int inode_type = -1;
int parent_type = -1;
gf_boolean_t wind = _gf_false;
+ char entry_point[NAME_MAX + 1] = {
+ 0,
+ };
GF_VALIDATE_OR_GOTO("svc", this, out);
- GF_VALIDATE_OR_GOTO(this->name, this->private, out);
GF_VALIDATE_OR_GOTO(this->name, frame, out);
GF_VALIDATE_OR_GOTO(this->name, loc, out);
GF_VALIDATE_OR_GOTO(this->name, loc->inode, out);
- priv = this->private;
-
ret = svc_inode_ctx_get(this, loc->inode, &inode_type);
if (!__is_root_gfid(loc->gfid)) {
if (loc->parent) {
@@ -411,7 +456,14 @@ gf_svc_lookup(call_frame_t *frame, xlator_t *this, loc_t *loc, dict_t *xdata)
}
}
- if (strcmp(loc->name, priv->path)) {
+ if (gf_svc_get_entry_point(this, entry_point, sizeof(entry_point))) {
+ gf_msg(this->name, GF_LOG_WARNING, op_errno,
+ SVC_MSG_COPY_ENTRY_POINT_FAILED,
+ "failed to copy the entry point string");
+ goto out;
+ }
+
+ if (strcmp(loc->name, entry_point)) {
if (parent_type == VIRTUAL_INODE) {
subvolume = SECOND_CHILD(this);
} else {
@@ -429,8 +481,8 @@ gf_svc_lookup(call_frame_t *frame, xlator_t *this, loc_t *loc, dict_t *xdata)
/* Indication of whether the lookup is happening on the
entry point or not, to the snapview-server.
*/
- SVC_ENTRY_POINT_SET(this, xdata, op_ret, op_errno, new_xdata, priv,
- ret, out);
+ SVC_ENTRY_POINT_SET(this, xdata, op_ret, op_errno, new_xdata, ret,
+ out);
}
}
@@ -469,6 +521,9 @@ gf_svc_statfs(call_frame_t *frame, xlator_t *this, loc_t *loc, dict_t *xdata)
0,
};
loc_t *temp_loc = NULL;
+ char entry_point[NAME_MAX + 1] = {
+ 0,
+ };
GF_VALIDATE_OR_GOTO("svc", this, out);
GF_VALIDATE_OR_GOTO(this->name, frame, out);
@@ -484,7 +539,14 @@ gf_svc_statfs(call_frame_t *frame, xlator_t *this, loc_t *loc, dict_t *xdata)
if (path_len >= snap_len && inode_type == VIRTUAL_INODE) {
path = &loc->path[path_len - snap_len];
- if (!strcmp(path, priv->path)) {
+ if (gf_svc_get_entry_point(this, entry_point, sizeof(entry_point))) {
+ gf_msg(this->name, GF_LOG_WARNING, op_errno,
+ SVC_MSG_COPY_ENTRY_POINT_FAILED,
+ "failed to copy the entry point string ");
+ goto out;
+ }
+
+ if (!strcmp(path, entry_point)) {
/*
* statfs call for virtual snap directory.
* Sent the fops to parent volume by removing
@@ -816,6 +878,9 @@ gf_svc_getxattr(call_frame_t *frame, xlator_t *this, loc_t *loc,
char attrname[PATH_MAX] = "";
char attrval[64] = "";
dict_t *dict = NULL;
+ char entry_point[NAME_MAX + 1] = {
+ 0,
+ };
GF_VALIDATE_OR_GOTO("svc", this, out);
GF_VALIDATE_OR_GOTO(this->name, frame, out);
@@ -839,14 +904,21 @@ gf_svc_getxattr(call_frame_t *frame, xlator_t *this, loc_t *loc,
strcat(attrname, ":");
if (!strcmp(attrname, GF_XATTR_GET_REAL_FILENAME_KEY)) {
- if (!strcasecmp(attrval, priv->path)) {
+ if (gf_svc_get_entry_point(this, entry_point, sizeof(entry_point))) {
+ gf_msg(this->name, GF_LOG_WARNING, op_errno,
+ SVC_MSG_COPY_ENTRY_POINT_FAILED,
+ "failed to copy the entry point string");
+ goto out;
+ }
+
+ if (!strcasecmp(attrval, entry_point)) {
dict = dict_new();
if (NULL == dict) {
op_errno = ENOMEM;
goto out;
}
- ret = dict_set_dynstr_with_alloc(dict, (char *)name, priv->path);
+ ret = dict_set_dynstr_with_alloc(dict, (char *)name, entry_point);
if (ret) {
op_errno = ENOMEM;
@@ -854,7 +926,7 @@ gf_svc_getxattr(call_frame_t *frame, xlator_t *this, loc_t *loc,
}
op_errno = 0;
- op_ret = strlen(priv->path) + 1;
+ op_ret = strlen(entry_point) + 1;
/* We should return from here */
goto out;
}
@@ -1079,17 +1151,16 @@ gf_svc_mkdir(call_frame_t *frame, xlator_t *this, loc_t *loc, mode_t mode,
int ret = -1;
int op_ret = -1;
int op_errno = EINVAL;
- svc_private_t *priv = NULL;
gf_boolean_t wind = _gf_false;
+ char entry_point[NAME_MAX + 1] = {
+ 0,
+ };
GF_VALIDATE_OR_GOTO("svc", this, out);
- GF_VALIDATE_OR_GOTO(this->name, this->private, out);
GF_VALIDATE_OR_GOTO(this->name, frame, out);
GF_VALIDATE_OR_GOTO(this->name, loc, out);
GF_VALIDATE_OR_GOTO(this->name, loc->inode, out);
- priv = this->private;
-
ret = svc_inode_ctx_get(this, loc->parent, &parent_type);
if (ret < 0) {
op_ret = -1;
@@ -1101,7 +1172,14 @@ gf_svc_mkdir(call_frame_t *frame, xlator_t *this, loc_t *loc, mode_t mode,
goto out;
}
- if (strcmp(loc->name, priv->path) && parent_type == NORMAL_INODE) {
+ if (gf_svc_get_entry_point(this, entry_point, sizeof(entry_point))) {
+ gf_msg(this->name, GF_LOG_WARNING, op_errno,
+ SVC_MSG_COPY_ENTRY_POINT_FAILED,
+ "failed to copy the entry point string");
+ goto out;
+ }
+
+ if (strcmp(loc->name, entry_point) && parent_type == NORMAL_INODE) {
STACK_WIND(frame, gf_svc_mkdir_cbk, FIRST_CHILD(this),
FIRST_CHILD(this)->fops->mkdir, loc, mode, umask, xdata);
} else {
@@ -1151,17 +1229,16 @@ gf_svc_mknod(call_frame_t *frame, xlator_t *this, loc_t *loc, mode_t mode,
int ret = -1;
int op_ret = -1;
int op_errno = EINVAL;
- svc_private_t *priv = NULL;
gf_boolean_t wind = _gf_false;
+ char entry_point[NAME_MAX + 1] = {
+ 0,
+ };
GF_VALIDATE_OR_GOTO("svc", this, out);
- GF_VALIDATE_OR_GOTO(this->name, this->private, out);
GF_VALIDATE_OR_GOTO(this->name, frame, out);
GF_VALIDATE_OR_GOTO(this->name, loc, out);
GF_VALIDATE_OR_GOTO(this->name, loc->inode, out);
- priv = this->private;
-
ret = svc_inode_ctx_get(this, loc->parent, &parent_type);
if (ret < 0) {
op_ret = -1;
@@ -1173,7 +1250,14 @@ gf_svc_mknod(call_frame_t *frame, xlator_t *this, loc_t *loc, mode_t mode,
goto out;
}
- if (strcmp(loc->name, priv->path) && parent_type == NORMAL_INODE) {
+ if (gf_svc_get_entry_point(this, entry_point, sizeof(entry_point))) {
+ gf_msg(this->name, GF_LOG_WARNING, op_errno,
+ SVC_MSG_COPY_ENTRY_POINT_FAILED,
+ "failed to copy the entry point string");
+ goto out;
+ }
+
+ if (strcmp(loc->name, entry_point) && parent_type == NORMAL_INODE) {
STACK_WIND(frame, gf_svc_mknod_cbk, FIRST_CHILD(this),
FIRST_CHILD(this)->fops->mknod, loc, mode, rdev, umask,
xdata);
@@ -1272,18 +1356,17 @@ gf_svc_create(call_frame_t *frame, xlator_t *this, loc_t *loc, int32_t flags,
int ret = -1;
int op_ret = -1;
int op_errno = EINVAL;
- svc_private_t *priv = NULL;
gf_boolean_t wind = _gf_false;
+ char entry_point[NAME_MAX + 1] = {
+ 0,
+ };
GF_VALIDATE_OR_GOTO("svc", this, out);
- GF_VALIDATE_OR_GOTO(this->name, this->private, out);
GF_VALIDATE_OR_GOTO(this->name, frame, out);
GF_VALIDATE_OR_GOTO(this->name, loc, out);
GF_VALIDATE_OR_GOTO(this->name, loc->inode, out);
GF_VALIDATE_OR_GOTO(this->name, fd, out);
- priv = this->private;
-
ret = svc_inode_ctx_get(this, loc->parent, &parent_type);
if (ret < 0) {
op_ret = -1;
@@ -1295,7 +1378,14 @@ gf_svc_create(call_frame_t *frame, xlator_t *this, loc_t *loc, int32_t flags,
goto out;
}
- if (strcmp(loc->name, priv->path) && parent_type == NORMAL_INODE) {
+ if (gf_svc_get_entry_point(this, entry_point, sizeof(entry_point))) {
+ gf_msg(this->name, GF_LOG_WARNING, op_errno,
+ SVC_MSG_COPY_ENTRY_POINT_FAILED,
+ "failed to copy the entry point string");
+ goto out;
+ }
+
+ if (strcmp(loc->name, entry_point) && parent_type == NORMAL_INODE) {
STACK_WIND(frame, gf_svc_create_cbk, FIRST_CHILD(this),
FIRST_CHILD(this)->fops->create, loc, flags, mode, umask, fd,
xdata);
@@ -1347,17 +1437,16 @@ gf_svc_symlink(call_frame_t *frame, xlator_t *this, const char *linkpath,
int op_ret = -1;
int op_errno = EINVAL;
int ret = -1;
- svc_private_t *priv = NULL;
gf_boolean_t wind = _gf_false;
+ char entry_point[NAME_MAX + 1] = {
+ 0,
+ };
GF_VALIDATE_OR_GOTO("svc", this, out);
GF_VALIDATE_OR_GOTO(this->name, frame, out);
- GF_VALIDATE_OR_GOTO(this->name, this->private, out);
GF_VALIDATE_OR_GOTO(this->name, loc, out);
GF_VALIDATE_OR_GOTO(this->name, loc->inode, out);
- priv = this->private;
-
ret = svc_inode_ctx_get(this, loc->parent, &parent_type);
if (ret < 0) {
op_ret = -1;
@@ -1369,7 +1458,14 @@ gf_svc_symlink(call_frame_t *frame, xlator_t *this, const char *linkpath,
goto out;
}
- if (strcmp(loc->name, priv->path) && parent_type == NORMAL_INODE) {
+ if (gf_svc_get_entry_point(this, entry_point, sizeof(entry_point))) {
+ gf_msg(this->name, GF_LOG_WARNING, op_errno,
+ SVC_MSG_COPY_ENTRY_POINT_FAILED,
+ "failed to copy the entry point string");
+ goto out;
+ }
+
+ if (strcmp(loc->name, entry_point) && parent_type == NORMAL_INODE) {
STACK_WIND(frame, gf_svc_symlink_cbk, FIRST_CHILD(this),
FIRST_CHILD(this)->fops->symlink, linkpath, loc, umask,
xdata);
@@ -1533,14 +1629,13 @@ gf_svc_readdir_cbk(call_frame_t *frame, void *cookie, xlator_t *this,
gf_dirent_t *entry = NULL;
gf_dirent_t *tmpentry = NULL;
svc_local_t *local = NULL;
- svc_private_t *priv = NULL;
+ char entry_point[NAME_MAX + 1] = {
+ 0,
+ };
if (op_ret < 0)
goto out;
- GF_VALIDATE_OR_GOTO(this->name, this->private, out);
-
- priv = this->private;
local = frame->local;
/* If .snaps pre-exists, then it should not be listed
@@ -1551,9 +1646,25 @@ gf_svc_readdir_cbk(call_frame_t *frame, void *cookie, xlator_t *this,
if (local->subvolume != FIRST_CHILD(this))
goto out;
+ /*
+ * Better to goto out if getting the entry point
+ * fails. We might end up sending the directory
+ * entry for the snapview entry point in the readdir
+ * response. But, the intention is to avoid the race
+ * condition where priv->path is being changed in
+ * reconfigure while this is accessing it.
+ */
+ if (gf_svc_get_entry_point(this, entry_point, sizeof(entry_point))) {
+ gf_msg(this->name, GF_LOG_WARNING, op_errno,
+ SVC_MSG_COPY_ENTRY_POINT_FAILED,
+ "failed to copy the entry point string. "
+ "Proceeding.");
+ goto out;
+ }
+
list_for_each_entry_safe(entry, tmpentry, &entries->list, list)
{
- if (strcmp(priv->path, entry->d_name) == 0)
+ if (strcmp(entry_point, entry->d_name) == 0)
gf_dirent_entry_free(entry);
}
@@ -1655,17 +1766,16 @@ gf_svc_readdirp_lookup_cbk(call_frame_t *frame, void *cookie, xlator_t *this,
{
gf_dirent_t entries;
gf_dirent_t *entry = NULL;
- svc_private_t *private = NULL;
svc_fd_t *svc_fd = NULL;
svc_local_t *local = NULL;
int inode_type = -1;
int ret = -1;
+ char entry_point[NAME_MAX + 1] = {
+ 0,
+ };
GF_VALIDATE_OR_GOTO("snapview-client", this, out);
- GF_VALIDATE_OR_GOTO(this->name, this->private, out);
- private
- = this->private;
INIT_LIST_HEAD(&entries.list);
local = frame->local;
@@ -1693,10 +1803,18 @@ gf_svc_readdirp_lookup_cbk(call_frame_t *frame, void *cookie, xlator_t *this,
goto out;
}
- entry = gf_dirent_for_name(private->path);
+ if (gf_svc_get_entry_point(this, entry_point, sizeof(entry_point))) {
+ gf_msg(this->name, GF_LOG_WARNING, 0, SVC_MSG_COPY_ENTRY_POINT_FAILED,
+ "failed to copy the entry point string");
+ op_ret = 0;
+ op_errno = ENOENT;
+ goto out;
+ }
+
+ entry = gf_dirent_for_name(entry_point);
if (!entry) {
gf_msg(this->name, GF_LOG_ERROR, 0, SVC_MSG_NO_MEMORY,
- "failed to allocate memory for the entry %s", private->path);
+ "failed to allocate memory for the entry %s", entry_point);
op_ret = 0;
op_errno = ENOMEM;
goto out;
@@ -1732,18 +1850,16 @@ int
gf_svc_special_dir_revalidate_lookup(call_frame_t *frame, xlator_t *this,
dict_t *xdata)
{
- svc_private_t *private = NULL;
svc_local_t *local = NULL;
loc_t *loc = NULL;
dict_t *tmp_xdata = NULL;
char *path = NULL;
int ret = -1;
+ char entry_point[NAME_MAX + 1] = {
+ 0,
+ };
GF_VALIDATE_OR_GOTO("snapview-client", this, out);
- GF_VALIDATE_OR_GOTO(this->name, this->private, out);
-
- private
- = this->private;
local = frame->local;
loc = &local->loc;
@@ -1764,8 +1880,14 @@ gf_svc_special_dir_revalidate_lookup(call_frame_t *frame, xlator_t *this,
goto out;
}
+ if (gf_svc_get_entry_point(this, entry_point, sizeof(entry_point))) {
+ gf_msg(this->name, GF_LOG_WARNING, 0, SVC_MSG_COPY_ENTRY_POINT_FAILED,
+ "failed to copy the entry point string");
+ goto out;
+ }
+
gf_uuid_copy(local->loc.gfid, loc->inode->gfid);
- ret = inode_path(loc->parent, private->path, &path);
+ ret = inode_path(loc->parent, entry_point, &path);
if (ret < 0)
goto out;
@@ -1820,6 +1942,9 @@ gf_svc_readdir_on_special_dir(call_frame_t *frame, void *cookie, xlator_t *this,
int ret = -1;
gf_boolean_t unwind = _gf_true;
svc_fd_t *svc_fd = NULL;
+ char entry_point[NAME_MAX + 1] = {
+ 0,
+ };
GF_VALIDATE_OR_GOTO("snapview-client", this, out);
GF_VALIDATE_OR_GOTO(this->name, this->private, out);
@@ -1850,7 +1975,13 @@ gf_svc_readdir_on_special_dir(call_frame_t *frame, void *cookie, xlator_t *this,
if (op_ret == 0 && op_errno == ENOENT && private->special_dir &&
strcmp(private->special_dir, "") && svc_fd->special_dir &&
local->subvolume == FIRST_CHILD(this)) {
- inode = inode_grep(fd->inode->table, fd->inode, private->path);
+ if (gf_svc_get_entry_point(this, entry_point, sizeof(entry_point))) {
+ gf_msg(this->name, GF_LOG_WARNING, 0, SVC_MSG_GET_FD_CONTEXT_FAILED,
+ "failed to copy the entry point string");
+ goto out;
+ }
+
+ inode = inode_grep(fd->inode->table, fd->inode, entry_point);
if (!inode) {
inode = inode_new(fd->inode->table);
if (!inode) {
@@ -1863,7 +1994,7 @@ gf_svc_readdir_on_special_dir(call_frame_t *frame, void *cookie, xlator_t *this,
gf_uuid_copy(local->loc.pargfid, fd->inode->gfid);
gf_uuid_copy(local->loc.gfid, inode->gfid);
if (gf_uuid_is_null(inode->gfid))
- ret = inode_path(fd->inode, private->path, &path);
+ ret = inode_path(fd->inode, entry_point, &path);
else
ret = inode_path(inode, NULL, &path);
@@ -1923,14 +2054,15 @@ gf_svc_readdirp_cbk(call_frame_t *frame, void *cookie, xlator_t *this,
int ret = -1;
svc_fd_t *svc_fd = NULL;
gf_boolean_t unwind = _gf_true;
- svc_private_t *priv = NULL;
+ char entry_point[NAME_MAX + 1] = {
+ 0,
+ };
if (op_ret < 0)
goto out;
GF_VALIDATE_OR_GOTO("snapview-client", this, out);
- GF_VALIDATE_OR_GOTO(this->name, this->private, out);
- priv = this->private;
+
local = frame->local;
svc_fd = svc_fd_ctx_get(this, local->fd);
@@ -1945,6 +2077,19 @@ gf_svc_readdirp_cbk(call_frame_t *frame, void *cookie, xlator_t *this,
else
inode_type = VIRTUAL_INODE;
+ /*
+ * Better to goto out and return whatever is there in the
+ * readdirp response (even if the readdir response contains
+ * a directory entry for the snapshot entry point). Otherwise
+ * if we ignore the error, then there is a chance of race
+ * condition where, priv->path is changed in reconfigure
+ */
+ if (gf_svc_get_entry_point(this, entry_point, sizeof(entry_point))) {
+ gf_msg(this->name, GF_LOG_WARNING, 0, SVC_MSG_COPY_ENTRY_POINT_FAILED,
+ "failed to copy the entry point");
+ goto out;
+ }
+
list_for_each_entry_safe(entry, tmpentry, &entries->list, list)
{
/* If .snaps pre-exists, then it should not be listed
@@ -1952,7 +2097,7 @@ gf_svc_readdirp_cbk(call_frame_t *frame, void *cookie, xlator_t *this,
* so filter the .snaps entry if exists.
* However it is OK to list .snaps in VIRTUAL world
*/
- if (inode_type == NORMAL_INODE && !strcmp(priv->path, entry->d_name)) {
+ if (inode_type == NORMAL_INODE && !strcmp(entry_point, entry->d_name)) {
gf_dirent_entry_free(entry);
continue;
}
@@ -2349,16 +2494,114 @@ out:
return 0;
}
+static int
+gf_svc_priv_destroy(xlator_t *this, svc_private_t *priv)
+{
+ int ret = -1;
+
+ if (!priv) {
+ gf_msg(this->name, GF_LOG_WARNING, 0, SVC_MSG_NULL_PRIV, "priv NULL");
+ goto out;
+ }
+
+ GF_FREE(priv->path);
+ GF_FREE(priv->special_dir);
+
+ LOCK_DESTROY(&priv->lock);
+
+ GF_FREE(priv);
+
+ if (this->local_pool) {
+ mem_pool_destroy(this->local_pool);
+ this->local_pool = NULL;
+ }
+
+ ret = 0;
+
+out:
+ return ret;
+}
+
+/**
+ * ** NOTE **:
+ * =============
+ * The option "snapdir-entry-path" is NOT reconfigurable.
+ * That option as of now is only for the consumption of
+ * samba, where, it needs to tell glusterfs about the
+ * directory that is shared with windows client for the
+ * access. Now, in windows-explorer (GUI) interface, for
+ * the directory shared, the entry point to the snapshot
+ * world (snapshot-directory option) should be visible,
+ * atleast as a hidden entry. For that to happen, glusterfs
+ * has to send that entry in the readdir response coming on
+ * the directory used as the smb share. Therefore, samba,
+ * while initializing the gluster volume (via gfapi) sets
+ * the xlator option "snapdir-entry-path" to the directory
+ * which is to be shared with windows (check the file
+ * vfs_glusterfs.c from samba source code). So to avoid
+ * problems with smb access, not allowing snapdir-entry-path
+ * option to be configurable. That option is for those
+ * consumers who know what they are doing.
+ **/
int
reconfigure(xlator_t *this, dict_t *options)
{
svc_private_t *priv = NULL;
+ char *path = NULL;
+ gf_boolean_t show_entry_point = _gf_false;
+ char *tmp = NULL;
priv = this->private;
- GF_OPTION_RECONF("snapshot-directory", priv->path, options, str, out);
- GF_OPTION_RECONF("show-snapshot-directory", priv->show_entry_point, options,
- bool, out);
+ GF_OPTION_RECONF("snapshot-directory", path, options, str, out);
+ if (!path || (strlen(path) > NAME_MAX) || path[0] != '.') {
+ gf_msg(this->name, GF_LOG_ERROR, 0, SVC_MSG_INVALID_ENTRY_POINT,
+ "%s is not a "
+ "valid entry point",
+ path);
+ goto out;
+ }
+
+ GF_OPTION_RECONF("show-snapshot-directory", show_entry_point, options, bool,
+ out);
+
+ /*
+ * The assumption now is that priv->path is an allocated memory (either
+ * in init or in a previous reconfigure).
+ * So, the intention here is to preserve the older contents of the option
+ * until the new option's value has been completely stored in the priv.
+ * So, do this.
+ * - Store the pointer of priv->path in a temporary pointer.
+ * - Allocate new memory for the new value of the option that is just
+ * obtained from the above call to GF_OPTION_RECONF.
+ * - If the above allocation fails, again set the pointer from priv
+ * to the address stored in tmp. i.e. the previous value.
+ * - If the allocation succeeds, then free the tmp pointer.
+ * WARNING: Before changing the allocation and freeing logic of
+ * priv->path, always check the init function to see how
+ * priv->path is set. Take decisions accordingly. As of now,
+ * the assumption is that, the string elements of private
+ * structure of snapview-client are allocated (either in
+ * init or here in reconfugure).
+ */
+ LOCK(&priv->lock);
+ {
+ tmp = priv->path;
+ priv->path = NULL;
+ priv->path = gf_strdup(path);
+ if (!priv->path) {
+ gf_log(this->name, GF_LOG_ERROR,
+ "failed to reconfigure snapshot-directory option to %s",
+ path);
+ priv->path = tmp;
+ } else {
+ GF_FREE(tmp);
+ tmp = NULL;
+ }
+
+ priv->show_entry_point = show_entry_point;
+ }
+ UNLOCK(&priv->lock);
out:
return 0;
@@ -2390,6 +2633,8 @@ init(xlator_t *this)
int ret = -1;
int children = 0;
xlator_list_t *xl = NULL;
+ char *path = NULL;
+ char *special_dir = NULL;
if (!this->children) {
gf_msg(this->name, GF_LOG_ERROR, 0, SVC_MSG_NO_CHILD_FOR_XLATOR,
@@ -2424,22 +2669,51 @@ init(xlator_t *this)
if (!private)
goto out;
- GF_OPTION_INIT("snapshot-directory", private->path, str, out);
- GF_OPTION_INIT("snapdir-entry-path", private->special_dir, str, out);
- GF_OPTION_INIT("show-snapshot-directory", private->show_entry_point, bool,
- out);
+ LOCK_INIT(&private->lock);
- if (strstr(private->special_dir, private->path)) {
- gf_msg(this->name, GF_LOG_ERROR, 0, SVC_MSG_ENTRY_POINT_SPECIAL_DIR,
- "entry point "
- "directory cannot be part of the special directory");
- GF_FREE(private->special_dir);
- private
- ->special_dir = NULL;
+ GF_OPTION_INIT("snapshot-directory", path, str, out);
+ if (!path || (strlen(path) > NAME_MAX) || path[0] != '.') {
+ gf_msg(this->name, GF_LOG_ERROR, 0, SVC_MSG_INVALID_ENTRY_POINT,
+ "%s is not a valid entry point", path);
goto out;
}
- this->private = private;
+ private
+ ->path = gf_strdup(path);
+ if (!private->path) {
+ gf_msg(this->name, GF_LOG_ERROR, 0, LG_MSG_NO_MEMORY,
+ "failed to allocate memory "
+ "for the entry point path %s",
+ path);
+ goto out;
+ }
+
+ GF_OPTION_INIT("snapdir-entry-path", special_dir, str, out);
+ if (!special_dir || strstr(special_dir, path)) {
+ if (special_dir)
+ gf_msg(this->name, GF_LOG_ERROR, 0, SVC_MSG_ENTRY_POINT_SPECIAL_DIR,
+ "entry point directory %s cannot be part of "
+ "the special directory %s",
+ path, special_dir);
+ else
+ gf_msg(this->name, GF_LOG_ERROR, 0, SVC_MSG_ENTRY_POINT_SPECIAL_DIR,
+ "null special directory");
+ goto out;
+ }
+
+ private
+ ->special_dir = gf_strdup(special_dir);
+ if (!private->special_dir) {
+ gf_msg(this->name, GF_LOG_ERROR, 0, LG_MSG_NO_MEMORY,
+ "failed to allocate memory "
+ "for the special directory %s",
+ special_dir);
+ goto out;
+ }
+
+ GF_OPTION_INIT("show-snapshot-directory", private->show_entry_point, bool,
+ out);
+
this->local_pool = mem_pool_new(svc_local_t, 128);
if (!this->local_pool) {
gf_msg(this->name, GF_LOG_ERROR, 0, SVC_MSG_NO_MEMORY,
@@ -2447,11 +2721,13 @@ init(xlator_t *this)
goto out;
}
+ this->private = private;
+
ret = 0;
out:
if (ret)
- GF_FREE(private);
+ (void)gf_svc_priv_destroy(this, private);
return ret;
}
@@ -2468,9 +2744,15 @@ fini(xlator_t *this)
if (!priv)
return;
- this->private = NULL;
+ /*
+ * Just log the failure and go ahead to
+ * set this->priv to NULL.
+ */
+ if (gf_svc_priv_destroy(this, priv))
+ gf_msg(this->name, GF_LOG_WARNING, 0, SVC_MSG_PRIV_DESTROY_FAILED,
+ "failed to destroy private");
- GF_FREE(priv);
+ this->private = NULL;
return;
}
diff --git a/xlators/features/snapview-client/src/snapview-client.h b/xlators/features/snapview-client/src/snapview-client.h
index 00caa0988c7..166116a439d 100644
--- a/xlators/features/snapview-client/src/snapview-client.h
+++ b/xlators/features/snapview-client/src/snapview-client.h
@@ -39,8 +39,8 @@ typedef struct __svc_local svc_local_t;
svc_local_free(__local); \
} while (0)
-#define SVC_ENTRY_POINT_SET(this, xdata, op_ret, op_errno, new_xdata, priv, \
- ret, label) \
+#define SVC_ENTRY_POINT_SET(this, xdata, op_ret, op_errno, new_xdata, ret, \
+ label) \
do { \
if (!xdata) { \
xdata = new_xdata = dict_new(); \
@@ -81,6 +81,7 @@ struct svc_private {
char *path;
char *special_dir; /* needed for samba */
gf_boolean_t show_entry_point;
+ gf_lock_t lock; /* mainly to guard private->path */
};
typedef struct svc_private svc_private_t;