Bug 1581345 - posix unwinds readdirp calls with readdir signature
Summary: posix unwinds readdirp calls with readdir signature
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: GlusterFS
Classification: Community
Component: posix
Version: mainline
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: bugs@gluster.org
QA Contact:
URL:
Whiteboard:
Depends On:
Blocks: 1582199
TreeView+ depends on / blocked
 
Reported: 2018-05-22 14:34 UTC by Raghavendra Bhat
Modified: 2018-10-23 15:09 UTC (History)
1 user (show)

Fixed In Version: glusterfs-5.0
Clone Of:
: 1582199 (view as bug list)
Environment:
Last Closed: 2018-10-23 15:09:43 UTC
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Embargoed:


Attachments (Terms of Use)

Description Raghavendra Bhat 2018-05-22 14:34:58 UTC
Description of problem:

As of now, posix xlator unwinds the readdirp calls with readdir signature.

This from functionality point of view might not impact the application or any feature. But if a xlator sitting above posix in the xlator stack (graph) expects the metadata and/or dictionary contents (for some of the internal xattrs used by other xlators in the graph) along with each directory entry, then such xlators might not be able to get those information. And they have to rely on a explicit lookup call to come OR they have to send a explicit lookup or getxattr operation themselves for getting the required metadata and the xattr values respectively.

This is what the current implementation of the function posix_do_readdir does (used by both posix_readdir and posix_readdirp)


"
int32_t
posix_do_readdir (call_frame_t *frame, xlator_t *this,
                  fd_t *fd, size_t size, off_t off, int whichop, dict_t *dict)
{
        struct posix_fd *pfd       = NULL;
        DIR             *dir       = NULL;
        int              ret       = -1;
	int              count     = 0;
        int32_t          op_ret    = -1;
        int32_t          op_errno  = 0;
	gf_dirent_t      entries;
        int32_t          skip_dirs = 0;

        VALIDATE_OR_GOTO (frame, out);
        VALIDATE_OR_GOTO (this, out);
	VALIDATE_OR_GOTO (fd, out);

	INIT_LIST_HEAD (&entries.list);

        ret = posix_fd_ctx_get (fd, this, &pfd, &op_errno);
	if (ret < 0) {
		gf_msg (this->name, GF_LOG_WARNING, op_errno, P_MSG_PFD_NULL,
                        "pfd is NULL, fd=%p", fd);
                goto out;
        }

        dir = pfd->dir;

        if (!dir) {
                gf_msg (this->name, GF_LOG_WARNING, EINVAL, P_MSG_PFD_NULL,
                        "dir is NULL for fd=%p", fd);
                op_errno = EINVAL;
                goto out;
        }

        /* When READDIR_FILTER option is set to on, we can filter out                                                                                                                                              
         * directory's entry from the entry->list.                                                                                                                                                                 
         */
        ret = dict_get_int32 (dict, GF_READDIR_SKIP_DIRS, &skip_dirs);

        LOCK (&fd->lock);
        {
                /* posix_fill_readdir performs multiple separate individual                                                                                                                                        
                   readdir() calls to fill up the buffer.                                                                                                                                                          
                                                                                                                                                                                                                   
                   In case of NFS where the same anonymous FD is shared between                                                                                                                                    
                   different applications, reading a common directory can                                                                                                                                          
                   result in the anonymous fd getting re-used unsafely between                                                                                                                                     
                   the two readdir requests (in two different io-threads).                                                                                                                                         
                                                                                                                                                                                                                   
                   It would also help, in the future, to replace the loop                                                                                                                                          
                   around readdir() with a single large getdents() call.                                                                                                                                           
                */
                count = posix_fill_readdir (fd, dir, off, size, &entries, this,
                                            skip_dirs);
        }
        UNLOCK (&fd->lock);

        /* pick ENOENT to indicate EOF */
        op_errno = errno;
        op_ret = count;

        if (whichop != GF_FOP_READDIRP)
                goto out;

        posix_readdirp_fill (this, fd, &entries, dict);

out:
        STACK_UNWIND_STRICT (readdir, frame, op_ret, op_errno, &entries, NULL); ======> unwinding with readdir signature without checking whether the incoming
        fop was readdir or readdirp.

        gf_dirent_free (&entries);

        return 0;
}


The similar behavior is there in the function posix_readdirp as well.

"
int32_t
posix_readdirp (call_frame_t *frame, xlator_t *this,
		fd_t *fd, size_t size, off_t off, dict_t *dict)
{
        gf_dirent_t entries;
        int32_t     op_ret = -1, op_errno = 0;
	gf_dirent_t     *entry     = NULL;


	if ((dict != NULL) && (dict_get (dict, GET_ANCESTRY_DENTRY_KEY))) {
                INIT_LIST_HEAD (&entries.list);

                op_ret = posix_get_ancestry (this, fd->inode, &entries, NULL,
                                             POSIX_ANCESTRY_DENTRY,
                                             &op_errno, dict);
                if (op_ret >= 0) {
			op_ret = 0;

                        list_for_each_entry (entry, &entries.list, list) {
				op_ret++;
			}
                }

                STACK_UNWIND_STRICT (readdir, frame, op_ret, op_errno,&entries,
                                     NULL);
                 ===> posix_readdirp is unwinding with readdir signature.

                gf_dirent_free (&entries);
                return 0;
        }

        posix_do_readdir (frame, this, fd, size, off, GF_FOP_READDIRP, dict);
        return 0;
}
"
Version-Release number of selected component (if applicable):


How reproducible:


Steps to Reproduce:
1.
2.
3.

Actual results:


Expected results:


Additional info:

Comment 1 Worker Ant 2018-05-22 14:41:17 UTC
REVIEW: https://review.gluster.org/20054 (storage/posix: use proper FOP for unwinding readdir(p)) posted (#2) for review on master by Raghavendra Bhat

Comment 2 Worker Ant 2018-05-24 06:14:00 UTC
COMMIT: https://review.gluster.org/20054 committed in master by "Amar Tumballi" <amarts> with a commit message- storage/posix: use proper FOP for unwinding readdir(p)

As of now, even for readdirp, posix is unwinding with readdir
signature.

Change-Id: I6440c8a253c5d78bbcc97043e4e6e208e3d47cd1
fixes: bz#1581345
Signed-off-by: Raghavendra Bhat <raghavendra>

Comment 3 Worker Ant 2018-05-24 13:53:26 UTC
REVIEW: https://review.gluster.org/20081 (storage/posix: use proper FOP for unwinding readdir(p)) posted (#1) for review on release-4.1 by Raghavendra Bhat

Comment 4 Shyamsundar 2018-10-23 15:09:43 UTC
This bug is getting closed because a release has been made available that should address the reported issue. In case the problem is still not fixed with glusterfs-5.0, please open a new bug report.

glusterfs-5.0 has been announced on the Gluster mailinglists [1], packages for several distributions should become available in the near future. Keep an eye on the Gluster Users mailinglist [2] and the update infrastructure for your distribution.

[1] https://lists.gluster.org/pipermail/announce/2018-October/000115.html
[2] https://www.gluster.org/pipermail/gluster-users/


Note You need to log in before you can comment on or make changes to this bug.