[PATCH] vfs_glusterfs: Add fallocate support for vfs_glusterfs

[Sachin Prabhu]

On Tue, 2018-01-09 at 23:56 +0100, David Disseldorp via samba-technical 
wrote:
> Hi Sachin,
> 
> A couple of comments inline...
Hello David,

Replies inline as well.


> 
> On Tue, 09 Jan 2018 13:02:13 +0530, Sachin Prabhu via samba-technical 
> wrote:
> 
> > From 05f47588b2641242d219564e6eb8468f9484a3d4 Mon Sep 17 00:00:00
> > 2001
> > From: Sachin Prabhu <sprabhu at redhat.com>
> > Date: Tue, 14 Nov 2017 15:51:44 +0530
> > Subject: [PATCH] vfs_glusterfs: Add fallocate support for
> > vfs_glusterfs
> > 
> > Adds fallocate support to the vfs glusterfs plugin.
> > 
> > RHBZ: 1478875
> > 
> > Signed-off-by: Sachin Prabhu <sprabhu at redhat.com>
> > ---
> >  source3/modules/vfs_glusterfs.c | 26 ++++++++++++++++++++++----
> >  1 file changed, 22 insertions(+), 4 deletions(-)
> > 
> > diff --git a/source3/modules/vfs_glusterfs.c
> > b/source3/modules/vfs_glusterfs.c
> > index 953c46af4cd..c0ea722c1df 100644
> > --- a/source3/modules/vfs_glusterfs.c
> > +++ b/source3/modules/vfs_glusterfs.c
> > @@ -462,7 +462,8 @@ static int vfs_gluster_statvfs(struct
> > vfs_handle_struct *handle,
> >  static uint32_t vfs_gluster_fs_capabilities(struct
> > vfs_handle_struct *handle,
> >  					    enum
> > timestamp_set_resolution *p_ts_res)
> >  {
> > -	uint32_t caps = FILE_CASE_SENSITIVE_SEARCH |
> > FILE_CASE_PRESERVED_NAMES;
> > +	uint32_t caps = FILE_CASE_SENSITIVE_SEARCH |
> > FILE_CASE_PRESERVED_NAMES
> > +			| FILE_SUPPORTS_SPARSE_FILES;
> 
> Does glusterfs support SEEK_DATA / SEEK_HOLE? Keep in mind that
> sparse
> aware clients may issue Query Allocated Ranges requests, which can be
> mapped by the SMB2 server into VFS lseek(SEEK_DATA / SEEK_HOLE)
> calls.

Yes they do.

I have used the smbtorture test smb2.ioctl.sparse_punch to test this
patch. This test makes the ioctl call - Query allocated ranges.

I have also run tests outside the samba environment to confirm that
glfs_lseek() does support SEEK_DATA and SEEK_HOLE.

> 
> >  #ifdef STAT_HAVE_NSEC
> >  	*p_ts_res = TIMESTAMP_SET_NT_OR_BETTER;
> > @@ -1148,9 +1149,26 @@ static int vfs_gluster_fallocate(struct
> > vfs_handle_struct *handle,
> >  				 uint32_t mode,
> >  				 off_t offset, off_t len)
> >  {
> > -	/* TODO: add support using glfs_fallocate() and
> > glfs_zerofill() */
> > -	errno = ENOTSUP;
> > -	return -1;
> > +	int keep_size, punch_hole;
> > +
> > +	keep_size = mode & VFS_FALLOCATE_FL_KEEP_SIZE;
> > +	punch_hole = mode & VFS_FALLOCATE_FL_PUNCH_HOLE;
> > +
> > +	mode &=
> > ~(VFS_FALLOCATE_FL_KEEP_SIZE|VFS_FALLOCATE_FL_PUNCH_HOLE);
> > +	if (mode) {
> > +		errno = ENOTSUP;
> > +		return -1;
> > +	}
> > +
> > +	if (punch_hole) {
> > +		return glfs_discard(*(glfs_fd_t **)
> > +				    VFS_FETCH_FSP_EXTENSION(handle
> > , fsp),
> > +				    offset, len);
> 
> I assume glfs_discard() will never change the file size here, i.e. it
> matches fallocate(mode = VFS_FALLOCATE_FL_PUNCH_HOLE
> 			| VFS_FALLOCATE_FL_KEEP_SIZE) behaviour?

Yes. The glfs_discard() on the client eventually ends up calling the
fallocate() syscall on the server with the flags set to
FALLOC_FL_KEEP_SIZE|FALLOC_FL_PUNCH_HOLE.

ie. on the server, the posix translator calls the syscall fallocate()
with the flags as mentioned.
static int32_t
posix_discard(call_frame_t *frame, xlator_t *this, fd_t *fd, off_t
offset,
              size_t len, dict_t *xdata)
{
..
        int32_t flags = FALLOC_FL_KEEP_SIZE|FALLOC_FL_PUNCH_HOLE;
..
        ret = posix_do_fallocate (frame, this, fd, flags, offset, len,
                                  &statpre, &statpost, xdata);
..
}


> > +	}
> > +
> > +	return glfs_fallocate(*(glfs_fd_t **)
> > +			      VFS_FETCH_FSP_EXTENSION(handle,
> > fsp),
> > +			      keep_size, offset, len);
> >  }
> >  
> >  static struct smb_filename *vfs_gluster_realpath(struct
> > vfs_handle_struct *handle,
> 
> Cheers, David
> 

Thanks for reviewing the patch.

Sachin Prabhu