summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMichael Adam <obnox@samba.org>2019-06-20 13:09:37 +0200
committerhari gowtham <hari.gowtham005@gmail.com>2019-07-04 05:53:23 +0000
commitfb93da37914458996987665d8e10f7ef87772133 (patch)
treebabd86cd71ea1fd75026e16e625bb8d0b145f74b
parentc6391433b67f7ab4eab7f85e080aec11019acf2c (diff)
[RFC] change get_real_filename implementation to use ENOATTR instead of ENOENT
get_real_filename is implemented as a virtual extended attribute to help Samba implement the case-insensitive but case preserving SMB protocol more efficiently. It is implemented as a getxattr call on the parent directory with the virtual key of "get_real_filename:<entryname>" by looking for a spelling with different case for the provided file/dir name (<entryname>) and returning this correct spelling as a result if the entry is found. Originally (05aaec645a6262d431486eb5ac7cd702646cfcfb), the implementation used the ENOENT errno to return the authoritative answer that <entryname> does not exist in any case folding. Now this implementation is actually a violation or misuse of the defined API for the getxattr call which returns ENOENT for the case that the dir that the call is made against does not exist and ENOATTR (or the synonym ENODATA) for the case that the xattr does not exist. This was not a problem until the gluster fuse-bridge was changed to do map ENOENT to ESTALE in 59629f1da9dca670d5dcc6425f7f89b3e96b46bf, after which we the getxattr call for get_real_filename returned an ESTALE instead of ENOENT breaking the expectation in Samba. It is an independent problem that ESTALE should not leak out to user space but is intended to trigger retries between fuse and gluster. But nevertheless, the semantics seem to be incorrect here and should be changed. This patch changes the implementation of the get_real_filename virtual xattr to correctly return ENOATTR instead of ENOENT if the file/directory being looked up is not found. The Samba glusterfs_fuse vfs module which takes advantage of the get_real_filename over a fuse mount will receive a corresponding change to map ENOATTR to ENOENT. Without this change, it will still work correctly, but the performance optimization for nonexisting files is lost. On the other hand side, this change removes the distinction between the old not-implemented case and the implemented case. So Samba changed to treat ENOATTR like ENOENT will not work correctly any more against old servers that don't implement get_real_filename. I.e. existing files will be reported as non-existing Backport of: > Change-Id: I971b427ab8410636d5d201157d9af70e0d075b67 > fixes: bz#1722977 > Signed-off-by: Michael Adam <obnox@samba.org> Change-Id: I971b427ab8410636d5d201157d9af70e0d075b67 fixes: bz#1723659 Signed-off-by: Michael Adam <obnox@samba.org> (cherry picked from commit dc1b87fcfef08c9497b0c02b2410c9d18bbc2dba)
-rw-r--r--xlators/cluster/dht/src/dht-common.c8
-rw-r--r--xlators/storage/posix/src/posix-inode-fd-ops.c4
2 files changed, 6 insertions, 6 deletions
diff --git a/xlators/cluster/dht/src/dht-common.c b/xlators/cluster/dht/src/dht-common.c
index bc8de94a7ca..a052d826408 100644
--- a/xlators/cluster/dht/src/dht-common.c
+++ b/xlators/cluster/dht/src/dht-common.c
@@ -4615,7 +4615,7 @@ dht_getxattr_get_real_filename_cbk(call_frame_t *frame, void *cookie,
LOCK(&frame->lock);
{
- if (local->op_errno == ENODATA || local->op_errno == EOPNOTSUPP) {
+ if (local->op_errno == EOPNOTSUPP) {
/* Nothing to do here, we have already found
* a subvol which does not have the get_real_filename
* optimization. If condition is for simple logic.
@@ -4624,7 +4624,7 @@ dht_getxattr_get_real_filename_cbk(call_frame_t *frame, void *cookie,
}
if (op_ret == -1) {
- if (op_errno == ENODATA || op_errno == EOPNOTSUPP) {
+ if (op_errno == EOPNOTSUPP) {
/* This subvol does not have the optimization.
* Better let the user know we don't support it.
* Remove previous results if any.
@@ -4652,7 +4652,7 @@ dht_getxattr_get_real_filename_cbk(call_frame_t *frame, void *cookie,
goto post_unlock;
}
- if (op_errno == ENOENT) {
+ if (op_errno == ENOATTR) {
/* Do nothing, our defaults are set to this.
*/
goto unlock;
@@ -4720,7 +4720,7 @@ dht_getxattr_get_real_filename(call_frame_t *frame, xlator_t *this, loc_t *loc,
cnt = local->call_cnt = layout->cnt;
local->op_ret = -1;
- local->op_errno = ENOENT;
+ local->op_errno = ENOATTR;
for (i = 0; i < cnt; i++) {
subvol = layout->list[i].xlator;
diff --git a/xlators/storage/posix/src/posix-inode-fd-ops.c b/xlators/storage/posix/src/posix-inode-fd-ops.c
index 157df8972f2..9a9467f8870 100644
--- a/xlators/storage/posix/src/posix-inode-fd-ops.c
+++ b/xlators/storage/posix/src/posix-inode-fd-ops.c
@@ -2840,7 +2840,7 @@ posix_xattr_get_real_filename(call_frame_t *frame, xlator_t *this, loc_t *loc,
(void)sys_closedir(fd);
if (!found)
- return -ENOENT;
+ return -ENOATTR;
ret = dict_set_dynstr(dict, (char *)key, found);
if (ret) {
@@ -3308,7 +3308,7 @@ posix_getxattr(call_frame_t *frame, xlator_t *this, loc_t *loc,
if (ret < 0) {
op_ret = -1;
op_errno = -ret;
- if (op_errno == ENOENT) {
+ if (op_errno == ENOATTR) {
gf_msg_debug(this->name, 0,
"Failed to get "
"real filename (%s, %s)",