On 10/13/2014 02:27 PM, Emmanuel Dreyfus wrote:
On Mon, Oct 13, 2014 at 01:42:38PM +0530, Pranith Kumar Karampuri wrote:
No bug here, just suboptimal behavior, both in glusterfs and NetBSD FUSE.
oh!, but shouldn't it get op_ret = 0 instead of op_ret -1, op_errno EINVAL?
It happens because of thei change I introduced:
seekdir (dir, (long)off);
#ifndef GF_LINUX_HOST_OS
if (telldir(dir) != off) {
gf_log (THIS->name, GF_LOG_ERROR,
"seekdir(%ld) failed on dir=%p: "
"Invalid argument (offset reused from "
"another DIR * structure?)", (long)off, dir);
errno = EINVAL;
count = -1;
goto out;
}
#endif /* GF_LINUX_HOST_OS */
The idea is to catch wrong opendir/closedir usage, and the errno = EINVAL
is critical in such a situation to avoid an infinite loop. On the other
hand it also fires for last entry. I did not notice it before because
this EINVAL will not reach calling process, but it may cause trouble
for internal glusterfs usage.
I see an easy way yo fix it (patch below)
- when posix xlator detects EOF, return an offset of -1 in last entry.
- When called with an offset of -1, immediatly return op_ret = 0 with no entry
What do you think?
I am not aware of backend filesystems that much, may be someone with
that knowledge can comment here, what happens when new entries are
created in the directory after this readdir is responded with '-1'?
diff --git a/xlators/storage/posix/src/posix.c b/xlators/storage/posix/src/posix
index 6b12d39..3e539bb 100644
--- a/xlators/storage/posix/src/posix.c
+++ b/xlators/storage/posix/src/posix.c
@@ -79,6 +79,10 @@ extern char *marker_xattrs[];
#define SET_TO_OLD_FS_ID()
#endif
+
+/* offset < sizeof(struct dirent) cannot be valid */
+#define INVALID_DIRENT_OFFSET sizeof (void *)
+
int
posix_forget (xlator_t *this, inode_t *inode)
{
@@ -4851,6 +4855,10 @@ posix_fill_readdir (fd_t *fd, DIR *dir, off_t off, size_t
int len = 0;
int ret = 0;
+ /* EOF was previously detected */
+ if (off == INVALID_DIRENT_OFFSET)
+ goto out;
+
if (skip_dirs) {
len = posix_handle_path (this, fd->inode->gfid, NULL, NULL, 0);
hpath = alloca (len + 256); /* NAME_MAX */
@@ -4969,9 +4977,12 @@ posix_fill_readdir (fd_t *fd, DIR *dir, off_t off, size_t
count ++;
}
- if ((!readdir (dir) && (errno == 0)))
+ if ((!readdir (dir) && (errno == 0))) {
/* Indicate EOF */
errno = ENOENT;
Now I understand why you kept saying ENOENT ;-). This may not be a good
idea IMO. Code all through gluster doesn't care what errno it unwinds
upwards.
+ /* Set invalid offset for later detection */
+ this_entry->d_off = INVALID_DIRENT_OFFSET;
+ }
out:
return count;
}
_______________________________________________
Gluster-devel mailing list
Gluster-devel@xxxxxxxxxxx
http://supercolony.gluster.org/mailman/listinfo/gluster-devel
