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?
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;
+ /* Set invalid offset for later detection */
+ this_entry->d_off = INVALID_DIRENT_OFFSET;
+ }
out:
return count;
}
--
Emmanuel Dreyfus
manu@xxxxxxxxxx
_______________________________________________
Gluster-devel mailing list
Gluster-devel@xxxxxxxxxxx
http://supercolony.gluster.org/mailman/listinfo/gluster-devel
