| Message ID | 20240623121103.974270-6-hch@lst.de (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [1/8] remove support for ext4dev | expand |
On Sun, Jun 23, 2024 at 02:10:34PM +0200, Christoph Hellwig wrote: > Instead of limiting this test to a few file systems, opt out the > file systems supported in common/rc that don't support overwrite > checking at all, and those like extN that support it, but only when > run interactively. If script(1) is installed, can we use it to run mkfs.extX in a sub-pty? Or is that not worth the trouble? (This is really more of a question for Ted...) --D > Also remove support for really old mkfs.btrfs versions that lack > the overwrite check. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > tests/generic/740 | 15 ++++++--------- > 1 file changed, 6 insertions(+), 9 deletions(-) > > diff --git a/tests/generic/740 b/tests/generic/740 > index bac927227..903e891db 100755 > --- a/tests/generic/740 > +++ b/tests/generic/740 > @@ -12,19 +12,16 @@ _begin_fstest mkfs auto quick > # Import common functions. > . ./common/filter > > -# real QA test starts here > -_supported_fs xfs btrfs > +# a bunch of file systems don't support foreign fs detection > +# ext* do support it, but disable the feature when called non-interactively > +_supported_fs ^ext2 ^ext3 ^ext4 ^jfs ^ocfs2 ^udf > > -_require_scratch_nocheck > -_require_no_large_scratch_dev > +_require_block_device "${SCRATCH_DEV}" > # not all the FS support zoned block device > _require_non_zoned_device "${SCRATCH_DEV}" > > -# mkfs.btrfs did not have overwrite detection at first > -if [ "$FSTYP" == "btrfs" ]; then > - grep -q 'force overwrite' `echo $MKFS_BTRFS_PROG | awk '{print $1}'` || \ > - _notrun "Installed mkfs.btrfs does not support -f option" > -fi > +_require_scratch_nocheck > +_require_no_large_scratch_dev > > echo "Silence is golden." > for fs in `echo ${MKFS_PROG}.* | sed -e "s:${MKFS_PROG}.::g"` > -- > 2.43.0 > >
On Mon, Jun 24, 2024 at 09:16:05AM -0700, Darrick J. Wong wrote: > On Sun, Jun 23, 2024 at 02:10:34PM +0200, Christoph Hellwig wrote: > > Instead of limiting this test to a few file systems, opt out the > > file systems supported in common/rc that don't support overwrite > > checking at all, and those like extN that support it, but only when > > run interactively. > > If script(1) is installed, can we use it to run mkfs.extX in a sub-pty? > > Or is that not worth the trouble? > > (This is really more of a question for Ted...) It might not be worth it. One of the reasons for it is that mkfs.ext4 can be set up to try to pull in libmagic using dlopen, to minimize the package dependencies for things like the distribution's installer or minimal root setu[s for Docker, et. al. As a result, mkfs.ext4's ability a pre-existing foreign fil;e system won't always work, depending on the libmagic shared libraery is available. It will be a lot easier to add a test for this functionality functionality in e2fsprogs's regression tests, since the build system will know whether libmagic is available. So maybe it's not worth trying to teach generic/740 how to test mkfs.ext4, at least for now. Cheers, - Ted
On Mon, Jun 24, 2024 at 11:50:08PM -0400, Theodore Ts'o wrote: > It might not be worth it. One of the reasons for it is that mkfs.ext4 > can be set up to try to pull in libmagic using dlopen, to minimize the > package dependencies for things like the distribution's installer or > minimal root setu[s for Docker, et. al. So mkfs.extN doesn't actually use libblkid for foreign fs detection like most (all?) other tools?
On Tue, Jun 25, 2024 at 08:00:39AM +0200, Christoph Hellwig wrote: > On Mon, Jun 24, 2024 at 11:50:08PM -0400, Theodore Ts'o wrote: > > It might not be worth it. One of the reasons for it is that mkfs.ext4 > > can be set up to try to pull in libmagic using dlopen, to minimize the > > package dependencies for things like the distribution's installer or > > minimal root setu[s for Docker, et. al. > > So mkfs.extN doesn't actually use libblkid for foreign fs detection > like most (all?) other tools? Oh, good point. Yeah, mke2fs uses libblkid in addition to libmagic. So yes, it should work for basic detection of file systems for the purposes of generic/740. So the only issue would be the fact that mkfs.extN only does the detection if it is running with a tty. The reasoning behind this was that there might have been existing shell scripts that might try to reformat a block device over an existing file system. (For example, like file system test / performance scripts like, say, for example xfstests's "check" script. :-) What I've considered doing adding an extended option, "mke2fs -E existing_fs_test={on,off,auto}" where auto is today's behavior, and "on" would always do the pre-existing file system test and fail if it there is a pre-existing file system, and "off" would skip it entirely. This would allow generic/740 to work without having to depend on "script" being installed. Of course, this would only work if a sufficiently new version of mkfs.extN being used by fstests. - Ted
On Tue, Jun 25, 2024 at 04:05:55PM -0400, Theodore Ts'o wrote: > Oh, good point. Yeah, mke2fs uses libblkid in addition to libmagic. > So yes, it should work for basic detection of file systems for the > purposes of generic/740. So the only issue would be the fact that > mkfs.extN only does the detection if it is running with a tty. The > reasoning behind this was that there might have been existing shell > scripts that might try to reformat a block device over an existing > file system. (For example, like file system test / performance > scripts like, say, for example xfstests's "check" script. :-) > > What I've considered doing adding an extended option, "mke2fs -E > existing_fs_test={on,off,auto}" where auto is today's behavior, and > "on" would always do the pre-existing file system test and fail if it > there is a pre-existing file system, and "off" would skip it entirely. > > This would allow generic/740 to work without having to depend on > "script" being installed. Of course, this would only work if a > sufficiently new version of mkfs.extN being used by fstests. Yes, so it's probably not worth it at least for this purpose. My preference would be to go ahead with the series as-is for now. I'll see if I can come up with the script magic eventually.
diff --git a/tests/generic/740 b/tests/generic/740 index bac927227..903e891db 100755 --- a/tests/generic/740 +++ b/tests/generic/740 @@ -12,19 +12,16 @@ _begin_fstest mkfs auto quick # Import common functions. . ./common/filter -# real QA test starts here -_supported_fs xfs btrfs +# a bunch of file systems don't support foreign fs detection +# ext* do support it, but disable the feature when called non-interactively +_supported_fs ^ext2 ^ext3 ^ext4 ^jfs ^ocfs2 ^udf -_require_scratch_nocheck -_require_no_large_scratch_dev +_require_block_device "${SCRATCH_DEV}" # not all the FS support zoned block device _require_non_zoned_device "${SCRATCH_DEV}" -# mkfs.btrfs did not have overwrite detection at first -if [ "$FSTYP" == "btrfs" ]; then - grep -q 'force overwrite' `echo $MKFS_BTRFS_PROG | awk '{print $1}'` || \ - _notrun "Installed mkfs.btrfs does not support -f option" -fi +_require_scratch_nocheck +_require_no_large_scratch_dev echo "Silence is golden." for fs in `echo ${MKFS_PROG}.* | sed -e "s:${MKFS_PROG}.::g"`
Instead of limiting this test to a few file systems, opt out the file systems supported in common/rc that don't support overwrite checking at all, and those like extN that support it, but only when run interactively. Also remove support for really old mkfs.btrfs versions that lack the overwrite check. Signed-off-by: Christoph Hellwig <hch@lst.de> --- tests/generic/740 | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-)