Re: Incorrect fallocate behaviour in cifs or samba?

[Jeremy Allison <jra@samba.org>]

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.