Re: Incorrect fallocate behaviour in cifs or samba?

[David Disseldorp <ddiss@suse.de>]

Hi Jeremy and David,

On Thu, 13 Jan 2022 09:43:32 -0800, Jeremy Allison wrote:

> On Thu, Jan 13, 2022 at 03:20:32PM +0000, David Howells wrote:
> >David Howells <dhowells@redhat.com> wrote:
> >  
> >> If I do the following:
> >>
> >> 	mount //carina/test /xfstest.test -o user=shares,pass=foobar,noperm,vers=3.0,mfsymlinks,actimeo=0
> >> 	/usr/sbin/xfs_io -f -t \
> >> 		-c "pwrite -S 0x41 0 4096"
> >> 		-c "pwrite -S 0x42 4096 4096"
> >> 		-c "fzero 0 4096" \
> >> 		-c "pread 0 8192" \
> >> 		/xfstest.test/008.7067
> >> ...
> >>    31 0.321638749  192.168.6.2 -> 192.168.6.1  SMB2 206 Ioctl Request FSCTL_SET_ZERO_DATA File: 008.7067  
> >
> >So what I see is that Samba does:
> >
> >	fallocate(24, FALLOC_FL_KEEP_SIZE|FALLOC_FL_PUNCH_HOLE, 0, 4096) = 0
> >
> >for this... but that's not what cifs was asked to do.  Should Samba be using
> >FALLOC_FL_ZERO_RANGE instead?  
> 
> This is from fsctl_zero_data() in Samba. We have:
> 
>          /*
>           * MS-FSCC <58> Section 2.3.67
>           * This FSCTL sets the range of bytes to zero (0) without extending the
>           * file size.
>           *
>           * The VFS_FALLOCATE_FL_KEEP_SIZE flag is used to satisfy this
>           * constraint.
>           */
> 
>          mode = VFS_FALLOCATE_FL_PUNCH_HOLE | VFS_FALLOCATE_FL_KEEP_SIZE;
>          ret = SMB_VFS_FALLOCATE(fsp, mode, zdata_info.file_off, len);
>          if (ret == -1)  {
>                  status = map_nt_error_from_unix_common(errno);
>                  DEBUG(2, ("zero-data fallocate(0x%x) failed: %s\n", mode,
>                        strerror(errno)));
>                  return status;
>          }
> 
>          if (!fsp->fsp_flags.is_sparse && lp_strict_allocate(SNUM(fsp->conn))) {
>                  /*
>                   * File marked non-sparse and "strict allocate" is enabled -
>                   * allocate the range that we just punched out.
>                   * In future FALLOC_FL_ZERO_RANGE could be used exclusively for
>                   * this, but it's currently only supported on XFS and ext4.
>                   *
>                   * The newly allocated range still won't be found by SEEK_DATA
>                   * for QAR, but stat.st_blocks will reflect it.
>                   */
>                  ret = SMB_VFS_FALLOCATE(fsp, VFS_FALLOCATE_FL_KEEP_SIZE,
>                                          zdata_info.file_off, len);
> 
> Note the "currently only supported on XFS and ext4" problem
> with FALLOC_FL_ZERO_RANGE.

FWIW, Samba's fsctl_zero_data semantics are based on observed Windows
server behaviour and the MS specs, which state(d at the time):
/*
 * 2.3.57 FSCTL_SET_ZERO_DATA Request
 *
 * How an implementation zeros data within a file is implementation-dependent.
 * A file system MAY choose to deallocate regions of disk space that have been
 * zeroed.<50>
 * <50>
 * ... NTFS might deallocate disk space in the file if the file is stored on an
 * NTFS volume, and the file is sparse or compressed. It will free any allocated
 * space in chunks of 64 kilobytes that begin at an offset that is a multiple of
 * 64 kilobytes. Other bytes in the file (prior to the first freed 64-kilobyte
 * chunk and after the last freed 64-kilobyte chunk) will be zeroed but not
 * deallocated.
 */

IIRC while implementing this I observed Windows deallocation behaviour
using FSCTL_QUERY_ALLOCATED_RANGES (referred to as QAR in the previous
code snippit).

Cheers, David