[Bugs] [Bug 1582199] New: posix unwinds readdirp calls with readdir signature

[bugzilla at redhat.com]

https://bugzilla.redhat.com/show_bug.cgi?id=1582199

            Bug ID: 1582199
           Summary: posix unwinds readdirp calls with readdir signature
           Product: GlusterFS
           Version: 4.1
         Component: posix
          Assignee: bugs at gluster.org
          Reporter: rabhat at redhat.com
                CC: bugs at gluster.org
        Depends On: 1581345



+++ This bug was initially created as a clone of Bug #1581345 +++

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:

--- Additional comment from Worker Ant on 2018-05-22 10:41:17 EDT ---

REVIEW: https://review.gluster.org/20054 (storage/posix: use proper FOP for
unwinding readdir(p)) posted (#2) for review on master by Raghavendra Bhat

--- Additional comment from Worker Ant on 2018-05-24 02:14:00 EDT ---

COMMIT: https://review.gluster.org/20054 committed in master by "Amar Tumballi"
<amarts at redhat.com> 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 at redhat.com>


Referenced Bugs:

https://bugzilla.redhat.com/show_bug.cgi?id=1581345
[Bug 1581345] posix unwinds readdirp calls with readdir signature
-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are the assignee for the bug.