Bug 11311 - vfs_glusterfs readdir returns incorrect file sizes
vfs_glusterfs readdir returns incorrect file sizes
Status: NEW
Product: Samba 4.1 and newer
Classification: Unclassified
Component: VFS Modules
4.2.2
x64 Linux
: P5 normal
: ---
Assigned To: Michael Adam
Samba QA Contact
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2015-06-03 18:44 UTC by Jason Lanclos
Modified: 2015-06-15 16:42 UTC (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jason Lanclos 2015-06-03 18:44:12 UTC
When using the glusterfs vfs module, the file sizes in a directory are incorrect. GlusterFS is version 3.6.3

Once the stat on the individual file is done, the subsequent directory listings show the correct size for that file.

smb: \> cd TestFolder1
smb: \TestFolder1\> dir
  .                                   D        0  Fri May 29 16:35:16 2015
  ..                                  D        0  Wed Jun  3 08:57:26 2015
  t                                   D        0  Fri May 29 20:08:30 2015
  CentOS-6.5-x86_64-bin-DVD1.iso      N 1489371136  Thu Jul 31 16:08:38 2014
  bright6.0-rhel6.iso                 N 1015414784  Tue Feb 19 13:34:30 2013
  bright6.0-centos6.iso               N 1048313856  Wed Dec 19 16:18:42 2012
  bright6.0-rhel5.iso                 N 1058406400  Thu Feb 21 09:30:20 2013

                43255710264 blocks of size 1024. 12345596064 blocks available

smb: \TestFolder1\> stat CentOS-6.5-x86_64-bin-DVD1.iso
File: \TestFolder1\CentOS-6.5-x86_64-bin-DVD1.iso
Size: 4467982336        Blocks: 8726528 regular file
Inode: 13275876202674536448     Links: 1
Access: (0744/-rwxr--r--)       Uid: 500        Gid: 500
Access: 2015-05-29 20:11:38 -0500
Modify: 2014-07-31 16:08:38 -0500
Change: 2015-04-06 10:02:49 -0500

smb: \TestFolder1\> dir
  .                                   D        0  Fri May 29 16:35:16 2015
  ..                                  D        0  Wed Jun  3 08:57:26 2015
  t                                   D        0  Fri May 29 20:08:30 2015
  CentOS-6.5-x86_64-bin-DVD1.iso      N 4467982336  Thu Jul 31 16:08:38 2014
  bright6.0-rhel6.iso                 N 1015414784  Tue Feb 19 13:34:30 2013
  bright6.0-centos6.iso               N 1048313856  Wed Dec 19 16:18:42 2012
  bright6.0-rhel5.iso                 N 1058406400  Thu Feb 21 09:30:20 2013

                43255710264 blocks of size 1024. 12345596064 blocks available

smb: \TestFolder1\> stat bright6.0-rhel6.iso
File: \TestFolder1\bright6.0-rhel6.iso
Size: 3046113280        Blocks: 5949440 regular file
Inode: 13022912189175015424     Links: 1
Access: (0744/-rwxr--r--)       Uid: 500        Gid: 500
Access: 2014-08-01 14:29:14 -0500
Modify: 2013-02-19 13:34:30 -0600
Change: 2015-04-06 10:02:49 -0500

smb: \TestFolder1\> dir
  .                                   D        0  Fri May 29 16:35:16 2015
  ..                                  D        0  Wed Jun  3 08:57:26 2015
  t                                   D        0  Fri May 29 20:08:30 2015
  CentOS-6.5-x86_64-bin-DVD1.iso      N 4467982336  Thu Jul 31 16:08:38 2014
  bright6.0-rhel6.iso                 N 3046113280  Tue Feb 19 13:34:30 2013
  bright6.0-centos6.iso               N 1048313856  Wed Dec 19 16:18:42 2012
  bright6.0-rhel5.iso                 N 1058406400  Thu Feb 21 09:30:20 2013

                43255710264 blocks of size 1024. 12345596064 blocks available

When the share is configured without the VFS module (using FUSE), this issue does not exist.

Any suggestions on fixes or possible workarounds will be appreciated..

Regards,

Jason Lanclos
Comment 1 Jason Lanclos 2015-06-04 00:18:24 UTC
I was able to work around the issue with the following change to *vfs_gluster_readdir

Seems like the glfs_readdirplus_r function populates sbuf with the incorrect filesize information.  When only using glfs_readdir_r, file sizes are correct.

--- /root/rpmbuild/BUILD/samba-4.2.2/source3/modules/vfs_glusterfs.c    2015-06-03 15:10:25.161869318 -0500
+++ vfs_glusterfs.c     2015-02-24 12:59:51.000000000 -0600
@@ -389,21 +389,19 @@
        struct dirent *dirent = 0;

        if (sbuf != NULL) {
-//             ret = glfs_readdirplus_r((void *)dirp, &stat, (void *)direntbuf,
-//                                      &dirent);
-           SET_STAT_INVALID(*sbuf);
-       }
-//     } else {
+               ret = glfs_readdirplus_r((void *)dirp, &stat, (void *)direntbuf,
+                                        &dirent);
+       } else {
                ret = glfs_readdir_r((void *)dirp, (void *)direntbuf, &dirent);
-//     }
+       }

        if ((ret < 0) || (dirent == NULL)) {
                return NULL;
        }

-//     if (sbuf != NULL) {
-//             smb_stat_ex_from_stat(sbuf, &stat);
-//     }
+       if (sbuf != NULL) {
+               smb_stat_ex_from_stat(sbuf, &stat);
+       }

        return dirent;
 }
Comment 2 Michael Adam 2015-06-08 10:33:29 UTC
Thanks for reporting this, Jason, and for the good analysis.
Looking ...
Comment 3 Raghavendra Talur 2015-06-12 13:07:40 UTC
I tried to reproduce this and was not able to.

Interestingly, I don't see SET_STAT_INVALID(*sbuf); line in any of the branches of Samba code that I have. It is visible in comment 1  here.

Is there anything I am missing?
I will try to reproduce it again.
Comment 4 Michael Adam 2015-06-15 16:38:40 UTC
(In reply to Raghavendra Talur from comment #3)

> Interestingly, I don't see SET_STAT_INVALID(*sbuf); line in
> any of the branches of Samba code that I have.
> It is visible in comment 1  here.

I think the diff in comment #1 is reversed, i.e. the
lines with "-" are the new state, "+" are the old state.

Removing the commented out lines ("//"), I come up with
the following diff that I think is equivalent to the
one of comment #1:

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 source3/modules/vfs_glusterfs.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/source3/modules/vfs_glusterfs.c b/source3/modules/vfs_glusterfs.c
index cfdd195..7a10948 100644
--- a/source3/modules/vfs_glusterfs.c
+++ b/source3/modules/vfs_glusterfs.c
@@ -389,12 +389,11 @@ static struct dirent *vfs_gluster_readdir(struct vfs_handle_struct *handle,
        struct dirent *dirent = 0;
 
        if (sbuf != NULL) {
-               ret = glfs_readdirplus_r((void *)dirp, &stat, (void *)direntbuf,
-                                        &dirent);
-       } else {
-               ret = glfs_readdir_r((void *)dirp, (void *)direntbuf, &dirent);
+               SET_STAT_INVALID(*sbuf);
        }
 
+       ret = glfs_readdir_r((void *)dirp, (void *)direntbuf, &dirent);
+
        if ((ret < 0) || (dirent == NULL)) {
                return NULL;
        }
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Not quite sure what to make of it yet.
Talur: we need to dig deeper...
Comment 5 Michael Adam 2015-06-15 16:42:45 UTC
(forgot the removal of the call to  smb_stat_ex_from_stat in the diff)