| Message ID | 20231218205720.3498-1-patrakov@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3] _require_sparse_files: rewrite as a direct test instead of a black list | expand |
On Tue, Dec 19, 2023 at 04:57:20AM +0800, Alexander Patrakov wrote: > _require_sparse_files was implemented as a list of filesystems known not to > support sparse files, and therefore it missed some cases. > > However, if sparse files do not work as expected during a test, the risk > is that the test will write out to the disk all the zeros that would > normally be unwritten. This amounts to at least 4 TB for the generic/129 > test, and therefore there is a significant media wear-out concern here. > > Adding more filesystems to the list of exclusions would not scale and > would not work anyway because CIFS backed by SAMBA is safe, while CIFS > backed by Windows Server 2022 is not (because the specific write > patterns found in generic/014 and generic/129 cause it to ignore the > otherwise-supported request to make a file sparse). > > Mitigate this risk by rewriting the check as a small-scale test that > reliably triggers Windows misbehavior. The black list becomes unneeded > because the same test creates and detects non-sparse files on exfat and > hfsplus. > > Signed-off-by: Alexander Patrakov <patrakov@gmail.com> Looks good, thanks for replacing the FSTYP test with a functional test. Reviewed-by: Darrick J. Wong <djwong@kernel.org> --D > --- > common/rc | 29 +++++++++++++++++++++-------- > 1 file changed, 21 insertions(+), 8 deletions(-) > > diff --git a/common/rc b/common/rc > index cc92fe06..a9e0ba7e 100644 > --- a/common/rc > +++ b/common/rc > @@ -2870,17 +2870,30 @@ _require_fs_space() > # > # Check if the filesystem supports sparse files. > # > -# Unfortunately there is no better way to do this than a manual black list. > +# Filesystems (such as CIFS mounted from a Windows server) that generally > +# support sparse files but are tricked into creating a non-sparse file by one > +# of the tests are treated here as not supporting sparse files. This special > +# treatment is done due to media wear-out concerns -- e.g., generic/129 would > +# write multiple terabytes of zeros if allowed to run on a filesystem that > +# ignores the request to make a file sparse. > # > _require_sparse_files() > { > - case $FSTYP in > - hfsplus|exfat) > - _notrun "Sparse files not supported by this filesystem type: $FSTYP" > - ;; > - *) > - ;; > - esac > + local testfile="$TEST_DIR/$$.sparsefiletest" > + rm -f "$testfile" > + > + # The write size and offset are specifically chosen to trick the Windows > + # SMB server implementation into dishonoring the request to create a sparse > + # file, while still fitting into the 64 kb SMB1 maximum request size. > + # This also creates a non-sparse file on vfat, exfat, and hfsplus. > + $XFS_IO_PROG -f -c 'pwrite -b 51200 -S 0x61 1638400 51200' "$testfile" >/dev/null > + > + resulting_file_size_kb=$( du -sk "$testfile" | cut -f 1 ) > + rm -f "$testfile" > + > + # The threshold of 1 MB allows for filesystems with such large clusters. > + [ $resulting_file_size_kb -gt 1024 ] && \ > + _notrun "Sparse files are not supported or do not work as expected" > } > > _require_debugfs() > -- > 2.43.0 > >
diff --git a/common/rc b/common/rc index cc92fe06..a9e0ba7e 100644 --- a/common/rc +++ b/common/rc @@ -2870,17 +2870,30 @@ _require_fs_space() # # Check if the filesystem supports sparse files. # -# Unfortunately there is no better way to do this than a manual black list. +# Filesystems (such as CIFS mounted from a Windows server) that generally +# support sparse files but are tricked into creating a non-sparse file by one +# of the tests are treated here as not supporting sparse files. This special +# treatment is done due to media wear-out concerns -- e.g., generic/129 would +# write multiple terabytes of zeros if allowed to run on a filesystem that +# ignores the request to make a file sparse. # _require_sparse_files() { - case $FSTYP in - hfsplus|exfat) - _notrun "Sparse files not supported by this filesystem type: $FSTYP" - ;; - *) - ;; - esac + local testfile="$TEST_DIR/$$.sparsefiletest" + rm -f "$testfile" + + # The write size and offset are specifically chosen to trick the Windows + # SMB server implementation into dishonoring the request to create a sparse + # file, while still fitting into the 64 kb SMB1 maximum request size. + # This also creates a non-sparse file on vfat, exfat, and hfsplus. + $XFS_IO_PROG -f -c 'pwrite -b 51200 -S 0x61 1638400 51200' "$testfile" >/dev/null + + resulting_file_size_kb=$( du -sk "$testfile" | cut -f 1 ) + rm -f "$testfile" + + # The threshold of 1 MB allows for filesystems with such large clusters. + [ $resulting_file_size_kb -gt 1024 ] && \ + _notrun "Sparse files are not supported or do not work as expected" } _require_debugfs()
_require_sparse_files was implemented as a list of filesystems known not to support sparse files, and therefore it missed some cases. However, if sparse files do not work as expected during a test, the risk is that the test will write out to the disk all the zeros that would normally be unwritten. This amounts to at least 4 TB for the generic/129 test, and therefore there is a significant media wear-out concern here. Adding more filesystems to the list of exclusions would not scale and would not work anyway because CIFS backed by SAMBA is safe, while CIFS backed by Windows Server 2022 is not (because the specific write patterns found in generic/014 and generic/129 cause it to ignore the otherwise-supported request to make a file sparse). Mitigate this risk by rewriting the check as a small-scale test that reliably triggers Windows misbehavior. The black list becomes unneeded because the same test creates and detects non-sparse files on exfat and hfsplus. Signed-off-by: Alexander Patrakov <patrakov@gmail.com> --- common/rc | 29 +++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-)