* [PATCH 0/3] bcachefs support
@ 2021-04-27 16:44 Kent Overstreet
2021-04-27 16:44 ` [PATCH 1/3] Initial " Kent Overstreet
` (3 more replies)
0 siblings, 4 replies; 19+ messages in thread
From: Kent Overstreet @ 2021-04-27 16:44 UTC (permalink / raw)
To: fstests, linux-fsdevel, linux-bcachefs; +Cc: Kent Overstreet
A small patch adding bcachefs support, and two other patches for consideration:
Kent Overstreet (3):
Initial bcachefs support
Improved .gitignore
Use --yes option to lvcreate
.gitignore | 3 +++
common/attr | 6 ++++++
common/config | 3 +++
common/dmlogwrites | 7 +++++++
common/quota | 4 ++--
common/rc | 31 +++++++++++++++++++++++++++++++
tests/generic/042 | 3 ++-
tests/generic/081 | 2 +-
tests/generic/108 | 2 +-
tests/generic/425 | 3 +++
tests/generic/441 | 2 +-
tests/generic/482 | 27 ++++++++++++++++++++-------
tests/generic/558 | 2 ++
13 files changed, 82 insertions(+), 13 deletions(-)
--
2.31.1
^ permalink raw reply [flat|nested] 19+ messages in thread* [PATCH 1/3] Initial bcachefs support 2021-04-27 16:44 [PATCH 0/3] bcachefs support Kent Overstreet @ 2021-04-27 16:44 ` Kent Overstreet 2021-05-09 14:36 ` Eryu Guan 2021-04-27 16:44 ` [PATCH 2/3] Improved .gitignore Kent Overstreet ` (2 subsequent siblings) 3 siblings, 1 reply; 19+ messages in thread From: Kent Overstreet @ 2021-04-27 16:44 UTC (permalink / raw) To: fstests, linux-fsdevel, linux-bcachefs; +Cc: Kent Overstreet, Kent Overstreet From: Kent Overstreet <kmo@daterainc.com> Signed-off-by: Kent Overstreet <kent.overstreet@gmail.com> --- common/attr | 6 ++++++ common/config | 3 +++ common/dmlogwrites | 7 +++++++ common/quota | 4 ++-- common/rc | 31 +++++++++++++++++++++++++++++++ tests/generic/042 | 3 ++- tests/generic/425 | 3 +++ tests/generic/441 | 2 +- tests/generic/482 | 27 ++++++++++++++++++++------- tests/generic/558 | 2 ++ 10 files changed, 77 insertions(+), 11 deletions(-) diff --git a/common/attr b/common/attr index 669909d600..42ceab9233 100644 --- a/common/attr +++ b/common/attr @@ -33,6 +33,9 @@ _acl_get_max() echo 506 fi ;; + bcachefs) + echo 251 + ;; *) echo 0 ;; @@ -273,6 +276,9 @@ pvfs2) 9p|ceph|nfs) MAX_ATTRVAL_SIZE=65536 ;; +bcachefs) + MAX_ATTRVAL_SIZE=1024 + ;; *) # Assume max ~1 block of attrs BLOCK_SIZE=`_get_block_size $TEST_DIR` diff --git a/common/config b/common/config index a47e462c77..8153301483 100644 --- a/common/config +++ b/common/config @@ -415,6 +415,9 @@ _mkfs_opts() btrfs) export MKFS_OPTIONS="$BTRFS_MKFS_OPTIONS" ;; + bcachefs) + export MKFS_OPTIONS="--errors=panic" + ;; *) ;; esac diff --git a/common/dmlogwrites b/common/dmlogwrites index 573f4b8a56..668d49e995 100644 --- a/common/dmlogwrites +++ b/common/dmlogwrites @@ -111,6 +111,13 @@ _log_writes_replay_log() [ -z "$_blkdev" ] && _fail \ "block dev must be specified for _log_writes_replay_log" + if [ "$FSTYP" = "bcachefs" ]; then + # bcachefs gets confused if we're replaying the history out of + # order, and we see writes on the device from a newer point in + # time than what the superblock points to: + dd if=/dev/zero of=$SCRATCH_DEV bs=1M oflag=direct >& /dev/null + fi + $here/src/log-writes/replay-log --log $LOGWRITES_DEV --find \ --end-mark $_mark >> $seqres.full 2>&1 [ $? -ne 0 ] && _fail "mark '$_mark' does not exist" diff --git a/common/quota b/common/quota index 32a9a55593..883a28a20d 100644 --- a/common/quota +++ b/common/quota @@ -17,7 +17,7 @@ _require_quota() _notrun "Installed kernel does not support quotas" fi ;; - gfs2|ocfs2) + gfs2|ocfs2|bcachefs) ;; xfs) if [ ! -f /proc/fs/xfs/xqmstat ]; then @@ -278,7 +278,7 @@ _check_quota_usage() VFS_QUOTA=0 case $FSTYP in - ext2|ext3|ext4|ext4dev|f2fs|reiserfs|gfs2) + ext2|ext3|ext4|ext4dev|f2fs|reiserfs|gfs2|bcachefs) VFS_QUOTA=1 quotaon -f -u -g $SCRATCH_MNT 2>/dev/null ;; diff --git a/common/rc b/common/rc index 2cf550ec68..0e03846aeb 100644 --- a/common/rc +++ b/common/rc @@ -334,6 +334,7 @@ _try_scratch_mount() return $? fi _mount -t $FSTYP `_scratch_mount_options $*` + return } # mount scratch device with given options and _fail if mount fails @@ -667,6 +668,9 @@ _test_mkfs() ext2|ext3|ext4) $MKFS_PROG -t $FSTYP -- -F $MKFS_OPTIONS $* $TEST_DEV ;; + bcachefs) + $MKFS_PROG -t $FSTYP -- $MKFS_OPTIONS $* $TEST_DEV + ;; *) yes | $MKFS_PROG -t $FSTYP -- $MKFS_OPTIONS $* $TEST_DEV ;; @@ -706,6 +710,10 @@ _mkfs_dev() $MKFS_PROG -t $FSTYP -- -f $MKFS_OPTIONS $* \ 2>$tmp.mkfserr 1>$tmp.mkfsstd ;; + bcachefs) + $MKFS_PROG -t $FSTYP -- $MKFS_OPTIONS $* \ + 2>$tmp_dir.mkfserr 1>$tmp_dir.mkfsstd + ;; *) yes | $MKFS_PROG -t $FSTYP -- $MKFS_OPTIONS $* \ 2>$tmp.mkfserr 1>$tmp.mkfsstd @@ -803,6 +811,10 @@ _scratch_mkfs() mkfs_cmd="yes | $MKFS_PROG -t $FSTYP --" mkfs_filter="grep -v -e ^mkfs\.ocfs2" ;; + bcachefs) + mkfs_cmd="$MKFS_PROG -t $FSTYP --" + mkfs_filter="cat" + ;; *) mkfs_cmd="yes | $MKFS_PROG -t $FSTYP --" mkfs_filter="cat" @@ -1065,6 +1077,9 @@ _scratch_mkfs_sized() fi export MOUNT_OPTIONS="-o size=$fssize $TMPFS_MOUNT_OPTIONS" ;; + bcachefs) + $MKFS_PROG -t $FSTYP -- $MKFS_OPTIONS --fs_size=$fssize --block_size=$blocksize $SCRATCH_DEV + ;; *) _notrun "Filesystem $FSTYP not supported in _scratch_mkfs_sized" ;; @@ -1133,6 +1148,9 @@ _scratch_mkfs_blocksized() ocfs2) yes | ${MKFS_PROG} -t $FSTYP -F $MKFS_OPTIONS -b $blocksize -C $blocksize $SCRATCH_DEV ;; + bcachefs) + ${MKFS_PROG}.$FSTYP $MKFS_OPTIONS --block_size=$blocksize $SCRATCH_DEV + ;; *) _notrun "Filesystem $FSTYP not supported in _scratch_mkfs_blocksized" ;; @@ -1179,6 +1197,19 @@ _repair_scratch_fs() fi return $res ;; + bcachefs) + fsck -t $FSTYP -n $SCRATCH_DEV 2>&1 + res=$? + case $res in + 0) + res=0 + ;; + *) + _dump_err2 "fsck.$FSTYP failed, err=$res" + ;; + esac + return $res + ;; *) local dev=$SCRATCH_DEV local fstyp=$FSTYP diff --git a/tests/generic/042 b/tests/generic/042 index 35727bcbc6..42919e2313 100755 --- a/tests/generic/042 +++ b/tests/generic/042 @@ -63,7 +63,8 @@ _crashtest() # We should /never/ see 0xCD in the file, because we wrote that pattern # to the filesystem image to expose stale data. - if hexdump -v -e '/1 "%02X "' $file | grep -q "CD"; then + # The file is not required to exist since we didn't sync before going down: + if [[ -f $file ]] && hexdump -v -e '/1 "%02X "' $file | grep -q "CD"; then echo "Saw stale data!!!" hexdump $file fi diff --git a/tests/generic/425 b/tests/generic/425 index 51cbe1c67d..be2bc1b02e 100755 --- a/tests/generic/425 +++ b/tests/generic/425 @@ -30,6 +30,9 @@ _cleanup() # real QA test starts here _supported_fs generic + +[ $FSTYP = bcachefs ] && _notrun "bcachefs does not store xattrs in blocks" + _require_scratch _require_attrs _require_xfs_io_command "fiemap" "-a" diff --git a/tests/generic/441 b/tests/generic/441 index bedbcb0817..814387b2a9 100755 --- a/tests/generic/441 +++ b/tests/generic/441 @@ -40,7 +40,7 @@ case $FSTYP in btrfs) _notrun "btrfs has a specialized test for this" ;; - ext3|ext4|xfs) + ext3|ext4|xfs|bcachefs) # Do the more thorough test if we have a logdev _has_logdev && sflag='' ;; diff --git a/tests/generic/482 b/tests/generic/482 index 86941e8468..3cbe187f2e 100755 --- a/tests/generic/482 +++ b/tests/generic/482 @@ -77,16 +77,29 @@ prev=$(_log_writes_mark_to_entry_number mkfs) cur=$(_log_writes_find_next_fua $prev) [ -z "$cur" ] && _fail "failed to locate next FUA write" +if [ "$FSTYP" = "bcachefs" ]; then + _dmthin_cleanup + _dmthin_init $devsize $devsize $csize $lowspace +fi + while [ ! -z "$cur" ]; do _log_writes_replay_log_range $cur $DMTHIN_VOL_DEV >> $seqres.full - # Here we need extra mount to replay the log, mainly for journal based - # fs, as their fsck will report dirty log as error. - # We don't care to preserve any data on the replay dev, as we can replay - # back to the point we need, and in fact sometimes creating/deleting - # snapshots repeatedly can be slower than replaying the log. - _dmthin_mount - _dmthin_check_fs + if [ "$FSTYP" = "bcachefs" ]; then + # bcachefs will get confused if fsck does writes to replay the log, + # but then we replay writes from an earlier point in time on the + # same fs - but fsck in -n mode won't do any writes: + _check_scratch_fs -n $DMTHIN_VOL_DEV + else + # Here we need extra mount to replay the log, mainly for journal based + # fs, as their fsck will report dirty log as error. + # We don't care to preserve any data on the replay dev, as we can replay + # back to the point we need, and in fact sometimes creating/deleting + # snapshots repeatedly can be slower than replaying the log. + + _dmthin_mount + _dmthin_check_fs + fi prev=$cur cur=$(_log_writes_find_next_fua $(($cur + 1))) diff --git a/tests/generic/558 b/tests/generic/558 index 4bed90e2a7..2eaa8f7686 100755 --- a/tests/generic/558 +++ b/tests/generic/558 @@ -55,6 +55,8 @@ _scratch_mount i=0 free_inode=`_get_free_inode $SCRATCH_MNT` file_per_dir=1000 +[ $FSTYP = bcachefs ] && file_per_dir=10000 + loop=$((free_inode / file_per_dir + 1)) mkdir -p $SCRATCH_MNT/testdir -- 2.31.1 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 1/3] Initial bcachefs support 2021-04-27 16:44 ` [PATCH 1/3] Initial " Kent Overstreet @ 2021-05-09 14:36 ` Eryu Guan 2021-05-23 22:51 ` Kent Overstreet 0 siblings, 1 reply; 19+ messages in thread From: Eryu Guan @ 2021-05-09 14:36 UTC (permalink / raw) To: Kent Overstreet; +Cc: fstests, linux-fsdevel, linux-bcachefs, Kent Overstreet On Tue, Apr 27, 2021 at 12:44:17PM -0400, Kent Overstreet wrote: > From: Kent Overstreet <kmo@daterainc.com> Better to add commit logs at least to give an example about how to setup fstests to test bcachefs. > > Signed-off-by: Kent Overstreet <kent.overstreet@gmail.com> > --- > common/attr | 6 ++++++ > common/config | 3 +++ > common/dmlogwrites | 7 +++++++ > common/quota | 4 ++-- > common/rc | 31 +++++++++++++++++++++++++++++++ > tests/generic/042 | 3 ++- > tests/generic/425 | 3 +++ > tests/generic/441 | 2 +- > tests/generic/482 | 27 ++++++++++++++++++++------- > tests/generic/558 | 2 ++ > 10 files changed, 77 insertions(+), 11 deletions(-) > > diff --git a/common/attr b/common/attr > index 669909d600..42ceab9233 100644 > --- a/common/attr > +++ b/common/attr > @@ -33,6 +33,9 @@ _acl_get_max() > echo 506 > fi > ;; > + bcachefs) > + echo 251 > + ;; > *) > echo 0 > ;; > @@ -273,6 +276,9 @@ pvfs2) > 9p|ceph|nfs) > MAX_ATTRVAL_SIZE=65536 > ;; > +bcachefs) > + MAX_ATTRVAL_SIZE=1024 > + ;; > *) > # Assume max ~1 block of attrs > BLOCK_SIZE=`_get_block_size $TEST_DIR` > diff --git a/common/config b/common/config > index a47e462c77..8153301483 100644 > --- a/common/config > +++ b/common/config > @@ -415,6 +415,9 @@ _mkfs_opts() > btrfs) > export MKFS_OPTIONS="$BTRFS_MKFS_OPTIONS" > ;; > + bcachefs) > + export MKFS_OPTIONS="--errors=panic" I think this might be useful for bcachefs developers to find bugs earlier, but not suitable for people like QA, as this may crash their test machines and stop the whole test. You could always set MKFS_OPTIONS to "--errors=panic" explicitly when needed. > + ;; > *) > ;; > esac > diff --git a/common/dmlogwrites b/common/dmlogwrites > index 573f4b8a56..668d49e995 100644 > --- a/common/dmlogwrites > +++ b/common/dmlogwrites > @@ -111,6 +111,13 @@ _log_writes_replay_log() > [ -z "$_blkdev" ] && _fail \ > "block dev must be specified for _log_writes_replay_log" > > + if [ "$FSTYP" = "bcachefs" ]; then > + # bcachefs gets confused if we're replaying the history out of > + # order, and we see writes on the device from a newer point in > + # time than what the superblock points to: > + dd if=/dev/zero of=$SCRATCH_DEV bs=1M oflag=direct >& /dev/null I don't know bcachefs internals, I'm not sure I understand this, clearing the first 1M of SCRATCH_DEV seems to clear superblock, but I'm still not sure why it's needed. Does wipefs work? > + fi > + > $here/src/log-writes/replay-log --log $LOGWRITES_DEV --find \ > --end-mark $_mark >> $seqres.full 2>&1 > [ $? -ne 0 ] && _fail "mark '$_mark' does not exist" > diff --git a/common/quota b/common/quota > index 32a9a55593..883a28a20d 100644 > --- a/common/quota > +++ b/common/quota > @@ -17,7 +17,7 @@ _require_quota() > _notrun "Installed kernel does not support quotas" > fi > ;; > - gfs2|ocfs2) > + gfs2|ocfs2|bcachefs) > ;; > xfs) > if [ ! -f /proc/fs/xfs/xqmstat ]; then > @@ -278,7 +278,7 @@ _check_quota_usage() > > VFS_QUOTA=0 > case $FSTYP in > - ext2|ext3|ext4|ext4dev|f2fs|reiserfs|gfs2) > + ext2|ext3|ext4|ext4dev|f2fs|reiserfs|gfs2|bcachefs) > VFS_QUOTA=1 > quotaon -f -u -g $SCRATCH_MNT 2>/dev/null > ;; > diff --git a/common/rc b/common/rc > index 2cf550ec68..0e03846aeb 100644 > --- a/common/rc > +++ b/common/rc > @@ -334,6 +334,7 @@ _try_scratch_mount() > return $? > fi > _mount -t $FSTYP `_scratch_mount_options $*` > + return Seems not necessary. > } > > # mount scratch device with given options and _fail if mount fails > @@ -667,6 +668,9 @@ _test_mkfs() > ext2|ext3|ext4) > $MKFS_PROG -t $FSTYP -- -F $MKFS_OPTIONS $* $TEST_DEV > ;; > + bcachefs) > + $MKFS_PROG -t $FSTYP -- $MKFS_OPTIONS $* $TEST_DEV > + ;; I think we could just use the default mkfs command below. The only difference is dropping the "yes | " part, but that does nothing if mkfs doesn't read "yes" or "no" from stdin. > *) > yes | $MKFS_PROG -t $FSTYP -- $MKFS_OPTIONS $* $TEST_DEV > ;; > @@ -706,6 +710,10 @@ _mkfs_dev() > $MKFS_PROG -t $FSTYP -- -f $MKFS_OPTIONS $* \ > 2>$tmp.mkfserr 1>$tmp.mkfsstd > ;; > + bcachefs) > + $MKFS_PROG -t $FSTYP -- $MKFS_OPTIONS $* \ > + 2>$tmp_dir.mkfserr 1>$tmp_dir.mkfsstd > + ;; Same as above. > *) > yes | $MKFS_PROG -t $FSTYP -- $MKFS_OPTIONS $* \ > 2>$tmp.mkfserr 1>$tmp.mkfsstd > @@ -803,6 +811,10 @@ _scratch_mkfs() > mkfs_cmd="yes | $MKFS_PROG -t $FSTYP --" > mkfs_filter="grep -v -e ^mkfs\.ocfs2" > ;; > + bcachefs) > + mkfs_cmd="$MKFS_PROG -t $FSTYP --" > + mkfs_filter="cat" > + ;; Same here. > *) > mkfs_cmd="yes | $MKFS_PROG -t $FSTYP --" > mkfs_filter="cat" > @@ -1065,6 +1077,9 @@ _scratch_mkfs_sized() > fi > export MOUNT_OPTIONS="-o size=$fssize $TMPFS_MOUNT_OPTIONS" > ;; > + bcachefs) > + $MKFS_PROG -t $FSTYP -- $MKFS_OPTIONS --fs_size=$fssize --block_size=$blocksize $SCRATCH_DEV > + ;; > *) > _notrun "Filesystem $FSTYP not supported in _scratch_mkfs_sized" > ;; > @@ -1133,6 +1148,9 @@ _scratch_mkfs_blocksized() > ocfs2) > yes | ${MKFS_PROG} -t $FSTYP -F $MKFS_OPTIONS -b $blocksize -C $blocksize $SCRATCH_DEV > ;; > + bcachefs) > + ${MKFS_PROG}.$FSTYP $MKFS_OPTIONS --block_size=$blocksize $SCRATCH_DEV > + ;; Better to be consistent and use "${MKFS_PROG} -t $FSTYP ..." instead of ${MKFS_PROG}.$FSTYP > *) > _notrun "Filesystem $FSTYP not supported in _scratch_mkfs_blocksized" > ;; > @@ -1179,6 +1197,19 @@ _repair_scratch_fs() > fi > return $res > ;; > + bcachefs) > + fsck -t $FSTYP -n $SCRATCH_DEV 2>&1 _repair_scratch_fs() is supposed to actually fix the errors, does "fsck -n" fix errors for bcachefs? > + res=$? > + case $res in > + 0) > + res=0 > + ;; > + *) > + _dump_err2 "fsck.$FSTYP failed, err=$res" > + ;; > + esac > + return $res > + ;; > *) > local dev=$SCRATCH_DEV > local fstyp=$FSTYP > diff --git a/tests/generic/042 b/tests/generic/042 > index 35727bcbc6..42919e2313 100755 > --- a/tests/generic/042 > +++ b/tests/generic/042 > @@ -63,7 +63,8 @@ _crashtest() > > # We should /never/ see 0xCD in the file, because we wrote that pattern > # to the filesystem image to expose stale data. > - if hexdump -v -e '/1 "%02X "' $file | grep -q "CD"; then > + # The file is not required to exist since we didn't sync before going down: > + if [[ -f $file ]] && hexdump -v -e '/1 "%02X "' $file | grep -q "CD"; then > echo "Saw stale data!!!" > hexdump $file > fi Updates for individual test should be in a separate patch. > diff --git a/tests/generic/425 b/tests/generic/425 > index 51cbe1c67d..be2bc1b02e 100755 > --- a/tests/generic/425 > +++ b/tests/generic/425 > @@ -30,6 +30,9 @@ _cleanup() > > # real QA test starts here > _supported_fs generic > + > +[ $FSTYP = bcachefs ] && _notrun "bcachefs does not store xattrs in blocks" > + > _require_scratch > _require_attrs > _require_xfs_io_command "fiemap" "-a" > diff --git a/tests/generic/441 b/tests/generic/441 > index bedbcb0817..814387b2a9 100755 > --- a/tests/generic/441 > +++ b/tests/generic/441 > @@ -40,7 +40,7 @@ case $FSTYP in > btrfs) > _notrun "btrfs has a specialized test for this" > ;; > - ext3|ext4|xfs) > + ext3|ext4|xfs|bcachefs) > # Do the more thorough test if we have a logdev > _has_logdev && sflag='' > ;; > diff --git a/tests/generic/482 b/tests/generic/482 > index 86941e8468..3cbe187f2e 100755 > --- a/tests/generic/482 > +++ b/tests/generic/482 > @@ -77,16 +77,29 @@ prev=$(_log_writes_mark_to_entry_number mkfs) > cur=$(_log_writes_find_next_fua $prev) > [ -z "$cur" ] && _fail "failed to locate next FUA write" > > +if [ "$FSTYP" = "bcachefs" ]; then > + _dmthin_cleanup > + _dmthin_init $devsize $devsize $csize $lowspace > +fi > + > while [ ! -z "$cur" ]; do > _log_writes_replay_log_range $cur $DMTHIN_VOL_DEV >> $seqres.full > > - # Here we need extra mount to replay the log, mainly for journal based > - # fs, as their fsck will report dirty log as error. > - # We don't care to preserve any data on the replay dev, as we can replay > - # back to the point we need, and in fact sometimes creating/deleting > - # snapshots repeatedly can be slower than replaying the log. > - _dmthin_mount > - _dmthin_check_fs > + if [ "$FSTYP" = "bcachefs" ]; then > + # bcachefs will get confused if fsck does writes to replay the log, > + # but then we replay writes from an earlier point in time on the > + # same fs - but fsck in -n mode won't do any writes: > + _check_scratch_fs -n $DMTHIN_VOL_DEV > + else > + # Here we need extra mount to replay the log, mainly for journal based > + # fs, as their fsck will report dirty log as error. > + # We don't care to preserve any data on the replay dev, as we can replay > + # back to the point we need, and in fact sometimes creating/deleting > + # snapshots repeatedly can be slower than replaying the log. > + > + _dmthin_mount > + _dmthin_check_fs > + fi > > prev=$cur > cur=$(_log_writes_find_next_fua $(($cur + 1))) > diff --git a/tests/generic/558 b/tests/generic/558 > index 4bed90e2a7..2eaa8f7686 100755 > --- a/tests/generic/558 > +++ b/tests/generic/558 > @@ -55,6 +55,8 @@ _scratch_mount > i=0 > free_inode=`_get_free_inode $SCRATCH_MNT` > file_per_dir=1000 > +[ $FSTYP = bcachefs ] && file_per_dir=10000 > + Some comments would be good. Thanks, Eryu > loop=$((free_inode / file_per_dir + 1)) > mkdir -p $SCRATCH_MNT/testdir > > -- > 2.31.1 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/3] Initial bcachefs support 2021-05-09 14:36 ` Eryu Guan @ 2021-05-23 22:51 ` Kent Overstreet 2021-05-24 3:56 ` Eryu Guan 0 siblings, 1 reply; 19+ messages in thread From: Kent Overstreet @ 2021-05-23 22:51 UTC (permalink / raw) To: Eryu Guan; +Cc: fstests, linux-fsdevel, linux-bcachefs, Kent Overstreet On Sun, May 09, 2021 at 10:36:05PM +0800, Eryu Guan wrote: > On Tue, Apr 27, 2021 at 12:44:17PM -0400, Kent Overstreet wrote: > > From: Kent Overstreet <kmo@daterainc.com> > > Better to add commit logs at least to give an example about how to setup > fstests to test bcachefs. > > You could always set MKFS_OPTIONS to "--errors=panic" explicitly when > needed. Forgot that was an option - doing that now. > > > + ;; > > *) > > ;; > > esac > > diff --git a/common/dmlogwrites b/common/dmlogwrites > > index 573f4b8a56..668d49e995 100644 > > --- a/common/dmlogwrites > > +++ b/common/dmlogwrites > > @@ -111,6 +111,13 @@ _log_writes_replay_log() > > [ -z "$_blkdev" ] && _fail \ > > "block dev must be specified for _log_writes_replay_log" > > > > + if [ "$FSTYP" = "bcachefs" ]; then > > + # bcachefs gets confused if we're replaying the history out of > > + # order, and we see writes on the device from a newer point in > > + # time than what the superblock points to: > > + dd if=/dev/zero of=$SCRATCH_DEV bs=1M oflag=direct >& /dev/null > > I don't know bcachefs internals, I'm not sure I understand this, > clearing the first 1M of SCRATCH_DEV seems to clear superblock, but I'm > still not sure why it's needed. Does wipefs work? It's not just the superblock we need to clear, it's really all metadata - the journal, and btree nodes are also log structured in bcachefs. So 1M actually isn't sufficient - the better solution would be to either - change the tests to check the markers in the log in the correct order, so we never see metadata from a future point in time, also making sure we don't do any writes to the filesystem when we're checking the different markers, or - just replay to a new dm-thin device This is basically what I did for generic/482, the 1M zerout is really just a hack for 455 and 457 and should probably be moved there, unless you've got another suggestion. > > diff --git a/common/rc b/common/rc > > index 2cf550ec68..0e03846aeb 100644 > > --- a/common/rc > > +++ b/common/rc > > @@ -334,6 +334,7 @@ _try_scratch_mount() > > return $? > > fi > > _mount -t $FSTYP `_scratch_mount_options $*` > > + return > > Seems not necessary. Not sure how that got in, dropped it. > > + bcachefs) > > + $MKFS_PROG -t $FSTYP -- $MKFS_OPTIONS $* $TEST_DEV > > + ;; > > I think we could just use the default mkfs command below. The only > difference is dropping the "yes | " part, but that does nothing if mkfs > doesn't read "yes" or "no" from stdin. That dates from when my test environment had SIGPIPE set up wrong (systemd!), it's fixed now so I've dropped these. > > *) > > _notrun "Filesystem $FSTYP not supported in _scratch_mkfs_blocksized" > > ;; > > @@ -1179,6 +1197,19 @@ _repair_scratch_fs() > > fi > > return $res > > ;; > > + bcachefs) > > + fsck -t $FSTYP -n $SCRATCH_DEV 2>&1 > > _repair_scratch_fs() is supposed to actually fix the errors, does > "fsck -n" fix errors for bcachefs? No - but with bcachefs fsck finding errors _always_ indicates a bug, so for the purposes of these tests I think this is the right thing to do - I don't want the tests to pass if fsck is finding and fixing errors. > > diff --git a/tests/generic/042 b/tests/generic/042 > > index 35727bcbc6..42919e2313 100755 > > --- a/tests/generic/042 > > +++ b/tests/generic/042 > > @@ -63,7 +63,8 @@ _crashtest() > > > > # We should /never/ see 0xCD in the file, because we wrote that pattern > > # to the filesystem image to expose stale data. > > - if hexdump -v -e '/1 "%02X "' $file | grep -q "CD"; then > > + # The file is not required to exist since we didn't sync before going down: > > + if [[ -f $file ]] && hexdump -v -e '/1 "%02X "' $file | grep -q "CD"; then > > echo "Saw stale data!!!" > > hexdump $file > > fi > > Updates for individual test should be in a separate patch. Ok, I'll split those out. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/3] Initial bcachefs support 2021-05-23 22:51 ` Kent Overstreet @ 2021-05-24 3:56 ` Eryu Guan 2021-05-24 4:04 ` Kent Overstreet 0 siblings, 1 reply; 19+ messages in thread From: Eryu Guan @ 2021-05-24 3:56 UTC (permalink / raw) To: Kent Overstreet Cc: Eryu Guan, fstests, linux-fsdevel, linux-bcachefs, Kent Overstreet On Sun, May 23, 2021 at 06:51:49PM -0400, Kent Overstreet wrote: [snip] > > > ;; > > > @@ -1179,6 +1197,19 @@ _repair_scratch_fs() > > > fi > > > return $res > > > ;; > > > + bcachefs) > > > + fsck -t $FSTYP -n $SCRATCH_DEV 2>&1 > > > > _repair_scratch_fs() is supposed to actually fix the errors, does > > "fsck -n" fix errors for bcachefs? > > No - but with bcachefs fsck finding errors _always_ indicates a bug, so for the > purposes of these tests I think this is the right thing to do - I don't want the > tests to pass if fsck is finding and fixing errors. Then _check_scratch_fs() should be used instead, which will fail the test if any fsck finds any corruptions. _repair_scratch_fs() is meant to fix errors, and only report failure when there's unfixable errors. Thanks, Eryu > > > > diff --git a/tests/generic/042 b/tests/generic/042 > > > index 35727bcbc6..42919e2313 100755 > > > --- a/tests/generic/042 > > > +++ b/tests/generic/042 > > > @@ -63,7 +63,8 @@ _crashtest() > > > > > > # We should /never/ see 0xCD in the file, because we wrote that pattern > > > # to the filesystem image to expose stale data. > > > - if hexdump -v -e '/1 "%02X "' $file | grep -q "CD"; then > > > + # The file is not required to exist since we didn't sync before going down: > > > + if [[ -f $file ]] && hexdump -v -e '/1 "%02X "' $file | grep -q "CD"; then > > > echo "Saw stale data!!!" > > > hexdump $file > > > fi > > > > Updates for individual test should be in a separate patch. > > Ok, I'll split those out. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/3] Initial bcachefs support 2021-05-24 3:56 ` Eryu Guan @ 2021-05-24 4:04 ` Kent Overstreet 2021-05-24 4:22 ` Eryu Guan 0 siblings, 1 reply; 19+ messages in thread From: Kent Overstreet @ 2021-05-24 4:04 UTC (permalink / raw) To: Eryu Guan Cc: Eryu Guan, fstests, linux-fsdevel, linux-bcachefs, Kent Overstreet On Mon, May 24, 2021 at 11:56:04AM +0800, Eryu Guan wrote: > On Sun, May 23, 2021 at 06:51:49PM -0400, Kent Overstreet wrote: > [snip] > > > > > ;; > > > > @@ -1179,6 +1197,19 @@ _repair_scratch_fs() > > > > fi > > > > return $res > > > > ;; > > > > + bcachefs) > > > > + fsck -t $FSTYP -n $SCRATCH_DEV 2>&1 > > > > > > _repair_scratch_fs() is supposed to actually fix the errors, does > > > "fsck -n" fix errors for bcachefs? > > > > No - but with bcachefs fsck finding errors _always_ indicates a bug, so for the > > purposes of these tests I think this is the right thing to do - I don't want the > > tests to pass if fsck is finding and fixing errors. > > Then _check_scratch_fs() should be used instead, which will fail the > test if any fsck finds any corruptions. _repair_scratch_fs() is meant to > fix errors, and only report failure when there's unfixable errors. I see no reason to make such a change to generic tests that were written for other filesystems, when this gets me exactly what I want. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/3] Initial bcachefs support 2021-05-24 4:04 ` Kent Overstreet @ 2021-05-24 4:22 ` Eryu Guan 2021-05-24 4:48 ` Kent Overstreet 0 siblings, 1 reply; 19+ messages in thread From: Eryu Guan @ 2021-05-24 4:22 UTC (permalink / raw) To: Kent Overstreet Cc: Eryu Guan, fstests, linux-fsdevel, linux-bcachefs, Kent Overstreet On Mon, May 24, 2021 at 12:04:32AM -0400, Kent Overstreet wrote: > On Mon, May 24, 2021 at 11:56:04AM +0800, Eryu Guan wrote: > > On Sun, May 23, 2021 at 06:51:49PM -0400, Kent Overstreet wrote: > > [snip] > > > > > > > ;; > > > > > @@ -1179,6 +1197,19 @@ _repair_scratch_fs() > > > > > fi > > > > > return $res > > > > > ;; > > > > > + bcachefs) > > > > > + fsck -t $FSTYP -n $SCRATCH_DEV 2>&1 > > > > > > > > _repair_scratch_fs() is supposed to actually fix the errors, does > > > > "fsck -n" fix errors for bcachefs? > > > > > > No - but with bcachefs fsck finding errors _always_ indicates a bug, so for the > > > purposes of these tests I think this is the right thing to do - I don't want the > > > tests to pass if fsck is finding and fixing errors. > > > > Then _check_scratch_fs() should be used instead, which will fail the > > test if any fsck finds any corruptions. _repair_scratch_fs() is meant to > > fix errors, and only report failure when there's unfixable errors. > > I see no reason to make such a change to generic tests that were written for > other filesystems, when this gets me exactly what I want. Oh, I noticed that "with bcachefs fsck finding errors _always_ indicates a bug" now.. Then I think using fsck -n is fine here, but better with comments to describe why this is ok for bcachefs. Also, we could just use _check_scratch_fs here instead of the open-coded fsck command. As _check_scratch_fs calls _check_generic_filesystem() which calls fsck -n internally, and handles mount/umount device correctly and prints pretty log if there's any errors. Thanks, Eryu ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/3] Initial bcachefs support 2021-05-24 4:22 ` Eryu Guan @ 2021-05-24 4:48 ` Kent Overstreet 0 siblings, 0 replies; 19+ messages in thread From: Kent Overstreet @ 2021-05-24 4:48 UTC (permalink / raw) To: Eryu Guan Cc: Eryu Guan, fstests, linux-fsdevel, linux-bcachefs, Kent Overstreet On Mon, May 24, 2021 at 12:22:53PM +0800, Eryu Guan wrote: > Oh, I noticed that "with bcachefs fsck finding errors _always_ indicates > a bug" now.. Then I think using fsck -n is fine here, but better with > comments to describe why this is ok for bcachefs. > > Also, we could just use _check_scratch_fs here instead of the open-coded > fsck command. As _check_scratch_fs calls _check_generic_filesystem() > which calls fsck -n internally, and handles mount/umount device > correctly and prints pretty log if there's any errors. That should work. ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 2/3] Improved .gitignore 2021-04-27 16:44 [PATCH 0/3] bcachefs support Kent Overstreet 2021-04-27 16:44 ` [PATCH 1/3] Initial " Kent Overstreet @ 2021-04-27 16:44 ` Kent Overstreet 2021-04-27 16:44 ` [PATCH 3/3] Use --yes option to lvcreate Kent Overstreet 2021-05-09 14:20 ` [PATCH 0/3] bcachefs support Eryu Guan 3 siblings, 0 replies; 19+ messages in thread From: Kent Overstreet @ 2021-04-27 16:44 UTC (permalink / raw) To: fstests, linux-fsdevel, linux-bcachefs; +Cc: Kent Overstreet Signed-off-by: Kent Overstreet <kent.overstreet@gmail.com> --- .gitignore | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.gitignore b/.gitignore index 4cc9c80724..5a0e5206da 100644 --- a/.gitignore +++ b/.gitignore @@ -4,6 +4,9 @@ .dep .libs .ltdep +.* +cscope.* +tags /local.config /results -- 2.31.1 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 3/3] Use --yes option to lvcreate 2021-04-27 16:44 [PATCH 0/3] bcachefs support Kent Overstreet 2021-04-27 16:44 ` [PATCH 1/3] Initial " Kent Overstreet 2021-04-27 16:44 ` [PATCH 2/3] Improved .gitignore Kent Overstreet @ 2021-04-27 16:44 ` Kent Overstreet 2021-04-27 17:03 ` Eryu Guan 2021-05-09 14:20 ` [PATCH 0/3] bcachefs support Eryu Guan 3 siblings, 1 reply; 19+ messages in thread From: Kent Overstreet @ 2021-04-27 16:44 UTC (permalink / raw) To: fstests, linux-fsdevel, linux-bcachefs; +Cc: Kent Overstreet This fixes spurious test failures caused by broken pipe messages. Signed-off-by: Kent Overstreet <kent.overstreet@gmail.com> --- tests/generic/081 | 2 +- tests/generic/108 | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/generic/081 b/tests/generic/081 index 5dff079852..26702007ab 100755 --- a/tests/generic/081 +++ b/tests/generic/081 @@ -70,7 +70,7 @@ _scratch_mkfs_sized $((300 * 1024 * 1024)) >>$seqres.full 2>&1 $LVM_PROG vgcreate -f $vgname $SCRATCH_DEV >>$seqres.full 2>&1 # We use yes pipe instead of 'lvcreate --yes' because old version of lvm # (like 2.02.95 in RHEL6) don't support --yes option -yes | $LVM_PROG lvcreate -L 256M -n $lvname $vgname >>$seqres.full 2>&1 +$LVM_PROG lvcreate --yes -L 256M -n $lvname $vgname >>$seqres.full 2>&1 # wait for lvcreation to fully complete $UDEV_SETTLE_PROG >>$seqres.full 2>&1 diff --git a/tests/generic/108 b/tests/generic/108 index 6fb194f43c..74945fdf3c 100755 --- a/tests/generic/108 +++ b/tests/generic/108 @@ -56,7 +56,7 @@ $LVM_PROG pvcreate -f $SCSI_DEBUG_DEV $SCRATCH_DEV >>$seqres.full 2>&1 $LVM_PROG vgcreate -f $vgname $SCSI_DEBUG_DEV $SCRATCH_DEV >>$seqres.full 2>&1 # We use yes pipe instead of 'lvcreate --yes' because old version of lvm # (like 2.02.95 in RHEL6) don't support --yes option -yes | $LVM_PROG lvcreate -i 2 -I 4m -L 275m -n $lvname $vgname \ +$LVM_PROG lvcreate --yes -i 2 -I 4m -L 275m -n $lvname $vgname \ >>$seqres.full 2>&1 # wait for lv creation to fully complete $UDEV_SETTLE_PROG >>$seqres.full 2>&1 -- 2.31.1 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 3/3] Use --yes option to lvcreate 2021-04-27 16:44 ` [PATCH 3/3] Use --yes option to lvcreate Kent Overstreet @ 2021-04-27 17:03 ` Eryu Guan 2021-04-27 20:29 ` Kent Overstreet 0 siblings, 1 reply; 19+ messages in thread From: Eryu Guan @ 2021-04-27 17:03 UTC (permalink / raw) To: Kent Overstreet; +Cc: fstests, linux-fsdevel, linux-bcachefs On Tue, Apr 27, 2021 at 12:44:19PM -0400, Kent Overstreet wrote: > This fixes spurious test failures caused by broken pipe messages. > > Signed-off-by: Kent Overstreet <kent.overstreet@gmail.com> > --- > tests/generic/081 | 2 +- > tests/generic/108 | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/tests/generic/081 b/tests/generic/081 > index 5dff079852..26702007ab 100755 > --- a/tests/generic/081 > +++ b/tests/generic/081 > @@ -70,7 +70,7 @@ _scratch_mkfs_sized $((300 * 1024 * 1024)) >>$seqres.full 2>&1 > $LVM_PROG vgcreate -f $vgname $SCRATCH_DEV >>$seqres.full 2>&1 > # We use yes pipe instead of 'lvcreate --yes' because old version of lvm > # (like 2.02.95 in RHEL6) don't support --yes option > -yes | $LVM_PROG lvcreate -L 256M -n $lvname $vgname >>$seqres.full 2>&1 > +$LVM_PROG lvcreate --yes -L 256M -n $lvname $vgname >>$seqres.full 2>&1 Please see above comments, we use yes pipe intentionally. I don't see how this would result in broken pipe. Would you please provide more details? And let's see if we could fix the broken pipe issue. Thanks, Eryu > # wait for lvcreation to fully complete > $UDEV_SETTLE_PROG >>$seqres.full 2>&1 > > diff --git a/tests/generic/108 b/tests/generic/108 > index 6fb194f43c..74945fdf3c 100755 > --- a/tests/generic/108 > +++ b/tests/generic/108 > @@ -56,7 +56,7 @@ $LVM_PROG pvcreate -f $SCSI_DEBUG_DEV $SCRATCH_DEV >>$seqres.full 2>&1 > $LVM_PROG vgcreate -f $vgname $SCSI_DEBUG_DEV $SCRATCH_DEV >>$seqres.full 2>&1 > # We use yes pipe instead of 'lvcreate --yes' because old version of lvm > # (like 2.02.95 in RHEL6) don't support --yes option > -yes | $LVM_PROG lvcreate -i 2 -I 4m -L 275m -n $lvname $vgname \ > +$LVM_PROG lvcreate --yes -i 2 -I 4m -L 275m -n $lvname $vgname \ > >>$seqres.full 2>&1 > # wait for lv creation to fully complete > $UDEV_SETTLE_PROG >>$seqres.full 2>&1 > -- > 2.31.1 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/3] Use --yes option to lvcreate 2021-04-27 17:03 ` Eryu Guan @ 2021-04-27 20:29 ` Kent Overstreet 2021-04-27 20:43 ` Matthew Wilcox 0 siblings, 1 reply; 19+ messages in thread From: Kent Overstreet @ 2021-04-27 20:29 UTC (permalink / raw) To: Eryu Guan; +Cc: fstests, linux-fsdevel, linux-bcachefs On Wed, Apr 28, 2021 at 01:03:39AM +0800, Eryu Guan wrote: > On Tue, Apr 27, 2021 at 12:44:19PM -0400, Kent Overstreet wrote: > > This fixes spurious test failures caused by broken pipe messages. > > > > Signed-off-by: Kent Overstreet <kent.overstreet@gmail.com> > > --- > > tests/generic/081 | 2 +- > > tests/generic/108 | 2 +- > > 2 files changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/tests/generic/081 b/tests/generic/081 > > index 5dff079852..26702007ab 100755 > > --- a/tests/generic/081 > > +++ b/tests/generic/081 > > @@ -70,7 +70,7 @@ _scratch_mkfs_sized $((300 * 1024 * 1024)) >>$seqres.full 2>&1 > > $LVM_PROG vgcreate -f $vgname $SCRATCH_DEV >>$seqres.full 2>&1 > > # We use yes pipe instead of 'lvcreate --yes' because old version of lvm > > # (like 2.02.95 in RHEL6) don't support --yes option > > -yes | $LVM_PROG lvcreate -L 256M -n $lvname $vgname >>$seqres.full 2>&1 > > +$LVM_PROG lvcreate --yes -L 256M -n $lvname $vgname >>$seqres.full 2>&1 > > Please see above comments, we use yes pipe intentionally. I don't see > how this would result in broken pipe. Would you please provide more > details? And let's see if we could fix the broken pipe issue. If lvcreate never ask y/n - never reads from standard input, then echo sees a broken pipe when it tries to write. That's what I get without this patch. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/3] Use --yes option to lvcreate 2021-04-27 20:29 ` Kent Overstreet @ 2021-04-27 20:43 ` Matthew Wilcox 2021-04-27 21:02 ` Eric Biggers 0 siblings, 1 reply; 19+ messages in thread From: Matthew Wilcox @ 2021-04-27 20:43 UTC (permalink / raw) To: Kent Overstreet; +Cc: Eryu Guan, fstests, linux-fsdevel, linux-bcachefs On Tue, Apr 27, 2021 at 04:29:23PM -0400, Kent Overstreet wrote: > On Wed, Apr 28, 2021 at 01:03:39AM +0800, Eryu Guan wrote: > > On Tue, Apr 27, 2021 at 12:44:19PM -0400, Kent Overstreet wrote: > > > This fixes spurious test failures caused by broken pipe messages. > > > > > > Signed-off-by: Kent Overstreet <kent.overstreet@gmail.com> > > > --- > > > tests/generic/081 | 2 +- > > > tests/generic/108 | 2 +- > > > 2 files changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/tests/generic/081 b/tests/generic/081 > > > index 5dff079852..26702007ab 100755 > > > --- a/tests/generic/081 > > > +++ b/tests/generic/081 > > > @@ -70,7 +70,7 @@ _scratch_mkfs_sized $((300 * 1024 * 1024)) >>$seqres.full 2>&1 > > > $LVM_PROG vgcreate -f $vgname $SCRATCH_DEV >>$seqres.full 2>&1 > > > # We use yes pipe instead of 'lvcreate --yes' because old version of lvm > > > # (like 2.02.95 in RHEL6) don't support --yes option > > > -yes | $LVM_PROG lvcreate -L 256M -n $lvname $vgname >>$seqres.full 2>&1 > > > +$LVM_PROG lvcreate --yes -L 256M -n $lvname $vgname >>$seqres.full 2>&1 > > > > Please see above comments, we use yes pipe intentionally. I don't see > > how this would result in broken pipe. Would you please provide more > > details? And let's see if we could fix the broken pipe issue. > > If lvcreate never ask y/n - never reads from standard input, then echo sees a > broken pipe when it tries to write. That's what I get without this patch. I think it's something in how ktest sets up the environment. I also see the SIGPIPEs when using your ktest scripts, but not when ssh'ing into the guest and running the test. What that thing is, I don't know. I'm not tall enough to understand signal handling. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/3] Use --yes option to lvcreate 2021-04-27 20:43 ` Matthew Wilcox @ 2021-04-27 21:02 ` Eric Biggers 2021-04-27 21:18 ` Kent Overstreet 0 siblings, 1 reply; 19+ messages in thread From: Eric Biggers @ 2021-04-27 21:02 UTC (permalink / raw) To: Matthew Wilcox Cc: Kent Overstreet, Eryu Guan, fstests, linux-fsdevel, linux-bcachefs On Tue, Apr 27, 2021 at 09:43:19PM +0100, Matthew Wilcox wrote: > On Tue, Apr 27, 2021 at 04:29:23PM -0400, Kent Overstreet wrote: > > On Wed, Apr 28, 2021 at 01:03:39AM +0800, Eryu Guan wrote: > > > On Tue, Apr 27, 2021 at 12:44:19PM -0400, Kent Overstreet wrote: > > > > This fixes spurious test failures caused by broken pipe messages. > > > > > > > > Signed-off-by: Kent Overstreet <kent.overstreet@gmail.com> > > > > --- > > > > tests/generic/081 | 2 +- > > > > tests/generic/108 | 2 +- > > > > 2 files changed, 2 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/tests/generic/081 b/tests/generic/081 > > > > index 5dff079852..26702007ab 100755 > > > > --- a/tests/generic/081 > > > > +++ b/tests/generic/081 > > > > @@ -70,7 +70,7 @@ _scratch_mkfs_sized $((300 * 1024 * 1024)) >>$seqres.full 2>&1 > > > > $LVM_PROG vgcreate -f $vgname $SCRATCH_DEV >>$seqres.full 2>&1 > > > > # We use yes pipe instead of 'lvcreate --yes' because old version of lvm > > > > # (like 2.02.95 in RHEL6) don't support --yes option > > > > -yes | $LVM_PROG lvcreate -L 256M -n $lvname $vgname >>$seqres.full 2>&1 > > > > +$LVM_PROG lvcreate --yes -L 256M -n $lvname $vgname >>$seqres.full 2>&1 > > > > > > Please see above comments, we use yes pipe intentionally. I don't see > > > how this would result in broken pipe. Would you please provide more > > > details? And let's see if we could fix the broken pipe issue. > > > > If lvcreate never ask y/n - never reads from standard input, then echo sees a > > broken pipe when it tries to write. That's what I get without this patch. > > I think it's something in how ktest sets up the environment. I also see > the SIGPIPEs when using your ktest scripts, but not when ssh'ing into > the guest and running the test. > > What that thing is, I don't know. I'm not tall enough to understand > signal handling. See xfstests commit 9bcb266cd778 ("generic/397: be compatible with ignored SIGPIPE") for an example of the same problem being fixed in another test, with some more explanation. But it's better to just not execute xfstests (or any shell script, for that matter) in an environment where SIGPIPE is ignored. - Eric ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/3] Use --yes option to lvcreate 2021-04-27 21:02 ` Eric Biggers @ 2021-04-27 21:18 ` Kent Overstreet 0 siblings, 0 replies; 19+ messages in thread From: Kent Overstreet @ 2021-04-27 21:18 UTC (permalink / raw) To: Eric Biggers Cc: Matthew Wilcox, Eryu Guan, fstests, linux-fsdevel, linux-bcachefs On Tue, Apr 27, 2021 at 02:02:15PM -0700, Eric Biggers wrote: > On Tue, Apr 27, 2021 at 09:43:19PM +0100, Matthew Wilcox wrote: > > On Tue, Apr 27, 2021 at 04:29:23PM -0400, Kent Overstreet wrote: > > > On Wed, Apr 28, 2021 at 01:03:39AM +0800, Eryu Guan wrote: > > > > On Tue, Apr 27, 2021 at 12:44:19PM -0400, Kent Overstreet wrote: > > > > > This fixes spurious test failures caused by broken pipe messages. > > > > > > > > > > Signed-off-by: Kent Overstreet <kent.overstreet@gmail.com> > > > > > --- > > > > > tests/generic/081 | 2 +- > > > > > tests/generic/108 | 2 +- > > > > > 2 files changed, 2 insertions(+), 2 deletions(-) > > > > > > > > > > diff --git a/tests/generic/081 b/tests/generic/081 > > > > > index 5dff079852..26702007ab 100755 > > > > > --- a/tests/generic/081 > > > > > +++ b/tests/generic/081 > > > > > @@ -70,7 +70,7 @@ _scratch_mkfs_sized $((300 * 1024 * 1024)) >>$seqres.full 2>&1 > > > > > $LVM_PROG vgcreate -f $vgname $SCRATCH_DEV >>$seqres.full 2>&1 > > > > > # We use yes pipe instead of 'lvcreate --yes' because old version of lvm > > > > > # (like 2.02.95 in RHEL6) don't support --yes option > > > > > -yes | $LVM_PROG lvcreate -L 256M -n $lvname $vgname >>$seqres.full 2>&1 > > > > > +$LVM_PROG lvcreate --yes -L 256M -n $lvname $vgname >>$seqres.full 2>&1 > > > > > > > > Please see above comments, we use yes pipe intentionally. I don't see > > > > how this would result in broken pipe. Would you please provide more > > > > details? And let's see if we could fix the broken pipe issue. > > > > > > If lvcreate never ask y/n - never reads from standard input, then echo sees a > > > broken pipe when it tries to write. That's what I get without this patch. > > > > I think it's something in how ktest sets up the environment. I also see > > the SIGPIPEs when using your ktest scripts, but not when ssh'ing into > > the guest and running the test. > > > > What that thing is, I don't know. I'm not tall enough to understand > > signal handling. > > See xfstests commit 9bcb266cd778 ("generic/397: be compatible with ignored > SIGPIPE") for an example of the same problem being fixed in another test, with > some more explanation. > > But it's better to just not execute xfstests (or any shell script, for that > matter) in an environment where SIGPIPE is ignored. Ah, thanks for the info. I'll see if I can figure out what's mucking with the signal handler (probably systemd...) ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 0/3] bcachefs support 2021-04-27 16:44 [PATCH 0/3] bcachefs support Kent Overstreet ` (2 preceding siblings ...) 2021-04-27 16:44 ` [PATCH 3/3] Use --yes option to lvcreate Kent Overstreet @ 2021-05-09 14:20 ` Eryu Guan 2021-05-10 23:14 ` Dave Chinner 3 siblings, 1 reply; 19+ messages in thread From: Eryu Guan @ 2021-05-09 14:20 UTC (permalink / raw) To: Kent Overstreet; +Cc: fstests, linux-fsdevel, linux-bcachefs On Tue, Apr 27, 2021 at 12:44:16PM -0400, Kent Overstreet wrote: > A small patch adding bcachefs support, and two other patches for consideration: As bcachefs is not upstream yet, I think we should re-visit bcachefs support after it's in upstream. OTOH, I have some minor comments go to patch 1. Thanks, Eryu > > Kent Overstreet (3): > Initial bcachefs support > Improved .gitignore > Use --yes option to lvcreate > > .gitignore | 3 +++ > common/attr | 6 ++++++ > common/config | 3 +++ > common/dmlogwrites | 7 +++++++ > common/quota | 4 ++-- > common/rc | 31 +++++++++++++++++++++++++++++++ > tests/generic/042 | 3 ++- > tests/generic/081 | 2 +- > tests/generic/108 | 2 +- > tests/generic/425 | 3 +++ > tests/generic/441 | 2 +- > tests/generic/482 | 27 ++++++++++++++++++++------- > tests/generic/558 | 2 ++ > 13 files changed, 82 insertions(+), 13 deletions(-) > > -- > 2.31.1 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 0/3] bcachefs support 2021-05-09 14:20 ` [PATCH 0/3] bcachefs support Eryu Guan @ 2021-05-10 23:14 ` Dave Chinner 2021-05-11 1:26 ` Darrick J. Wong 0 siblings, 1 reply; 19+ messages in thread From: Dave Chinner @ 2021-05-10 23:14 UTC (permalink / raw) To: Eryu Guan; +Cc: Kent Overstreet, fstests, linux-fsdevel, linux-bcachefs On Sun, May 09, 2021 at 10:20:38PM +0800, Eryu Guan wrote: > On Tue, Apr 27, 2021 at 12:44:16PM -0400, Kent Overstreet wrote: > > A small patch adding bcachefs support, and two other patches for consideration: > > As bcachefs is not upstream yet, I think we should re-visit bcachefs > support after it's in upstream. I disagree completely. I've been waiting for this to land for some time so I can actually run fstests against bcachefs easily to evaluate it's current state of stability and support. The plans are to get bcachefs merged upstream, and so having support already in fstests makes it much easier for reviewers and developers to actually run tests and find problems prior to merging. As an upstream developer and someone who will be reviewing bcachefs when it is next proposed for merge, I would much prefer to see extensive and long term fstests coverage *before* the code is even merged upstream. Given that filesystems take years to develop to the point where they are stable and ready for merge, saying "can't enable the test environment until it is merged upstream" is not very helpful. As a general principle, we want developers of new filesystems to start using fstests early in the development process of their filesystem. We should be encouraging new filesystems to be added to fstests, not saying "we only support upstream filesystems". If the filesystem plans to be merged upstream, then fstests support for that filesystem should be there long before the filesytsem is even proposed for merge. We need to help people get new filesystems upstream, not place arbitrary "not upstream so not supported" catch-22s in their way... Hence I ask that you merge bcachefs support to help the process of getting bcachefs suport upstream. Cheers, Dave. -- Dave Chinner david@fromorbit.com ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 0/3] bcachefs support 2021-05-10 23:14 ` Dave Chinner @ 2021-05-11 1:26 ` Darrick J. Wong 2021-05-16 13:54 ` Eryu Guan 0 siblings, 1 reply; 19+ messages in thread From: Darrick J. Wong @ 2021-05-11 1:26 UTC (permalink / raw) To: Dave Chinner Cc: Eryu Guan, Kent Overstreet, fstests, linux-fsdevel, linux-bcachefs On Tue, May 11, 2021 at 09:14:46AM +1000, Dave Chinner wrote: > On Sun, May 09, 2021 at 10:20:38PM +0800, Eryu Guan wrote: > > On Tue, Apr 27, 2021 at 12:44:16PM -0400, Kent Overstreet wrote: > > > A small patch adding bcachefs support, and two other patches for consideration: > > > > As bcachefs is not upstream yet, I think we should re-visit bcachefs > > support after it's in upstream. > > I disagree completely. I've been waiting for this to land for some > time so I can actually run fstests against bcachefs easily to > evaluate it's current state of stability and support. The plans are > to get bcachefs merged upstream, and so having support already in > fstests makes it much easier for reviewers and developers to > actually run tests and find problems prior to merging. > > As an upstream developer and someone who will be reviewing bcachefs > when it is next proposed for merge, I would much prefer to see > extensive and long term fstests coverage *before* the code is even > merged upstream. Given that filesystems take years to develop to the > point where they are stable and ready for merge, saying "can't > enable the test environment until it is merged upstream" is not very > helpful. > > As a general principle, we want developers of new filesystems to > start using fstests early in the development process of their > filesystem. We should be encouraging new filesystems to be added to > fstests, not saying "we only support upstream filesystems". If the > filesystem plans to be merged upstream, then fstests support for > that filesystem should be there long before the filesytsem is even > proposed for merge. We need to help people get new filesystems > upstream, not place arbitrary "not upstream so not supported" > catch-22s in their way... > > Hence I ask that you merge bcachefs support to help the process of > getting bcachefs suport upstream. /me notes that both Eryu and Dave have been willing to merge surprisingly large quantities of code for XFS reflink and online fsck long before either of those features landed in mainline, so I think it's perfectly fine to merge Kent's relatively small changes to enable 'FSTYP=bcachefs'. (With all of Eryu's review comments fixed, obviously...) --D > Cheers, > > Dave. > -- > Dave Chinner > david@fromorbit.com ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 0/3] bcachefs support 2021-05-11 1:26 ` Darrick J. Wong @ 2021-05-16 13:54 ` Eryu Guan 0 siblings, 0 replies; 19+ messages in thread From: Eryu Guan @ 2021-05-16 13:54 UTC (permalink / raw) To: Darrick J. Wong Cc: Dave Chinner, Kent Overstreet, fstests, linux-fsdevel, linux-bcachefs On Mon, May 10, 2021 at 06:26:45PM -0700, Darrick J. Wong wrote: > On Tue, May 11, 2021 at 09:14:46AM +1000, Dave Chinner wrote: > > On Sun, May 09, 2021 at 10:20:38PM +0800, Eryu Guan wrote: > > > On Tue, Apr 27, 2021 at 12:44:16PM -0400, Kent Overstreet wrote: > > > > A small patch adding bcachefs support, and two other patches for consideration: > > > > > > As bcachefs is not upstream yet, I think we should re-visit bcachefs > > > support after it's in upstream. > > > > I disagree completely. I've been waiting for this to land for some > > time so I can actually run fstests against bcachefs easily to > > evaluate it's current state of stability and support. The plans are > > to get bcachefs merged upstream, and so having support already in > > fstests makes it much easier for reviewers and developers to > > actually run tests and find problems prior to merging. > > > > As an upstream developer and someone who will be reviewing bcachefs > > when it is next proposed for merge, I would much prefer to see > > extensive and long term fstests coverage *before* the code is even > > merged upstream. Given that filesystems take years to develop to the > > point where they are stable and ready for merge, saying "can't > > enable the test environment until it is merged upstream" is not very > > helpful. > > > > As a general principle, we want developers of new filesystems to > > start using fstests early in the development process of their > > filesystem. We should be encouraging new filesystems to be added to > > fstests, not saying "we only support upstream filesystems". If the > > filesystem plans to be merged upstream, then fstests support for > > that filesystem should be there long before the filesytsem is even > > proposed for merge. We need to help people get new filesystems > > upstream, not place arbitrary "not upstream so not supported" > > catch-22s in their way... OK, that makes sense. Actually, I should have made my "upstream first" more clear. I'd love to merge non-upstream features/new fs supports if the proposed new feature/new filesystem has been developmented actively and the community has generally made the agreement that will merge the new feature/filesystem when it's in a good shape. IOW, it's not some random new features/filesystems, which are very likely to be dropped in the half way. And providing such info in the patch is very helpful to reviewers. > > > > Hence I ask that you merge bcachefs support to help the process of > > getting bcachefs suport upstream. Sure, bcachefs looks promising to me now :) > > /me notes that both Eryu and Dave have been willing to merge > surprisingly large quantities of code for XFS reflink and online fsck > long before either of those features landed in mainline, so I think it's Because I know you and the xfs commnity will work on it continuously and very unlikely make the new tests dead code :) Thanks, Eryu > perfectly fine to merge Kent's relatively small changes to enable > 'FSTYP=bcachefs'. > > (With all of Eryu's review comments fixed, obviously...) > > --D > > > Cheers, > > > > Dave. > > -- > > Dave Chinner > > david@fromorbit.com ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2021-05-24 4:48 UTC | newest] Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-04-27 16:44 [PATCH 0/3] bcachefs support Kent Overstreet 2021-04-27 16:44 ` [PATCH 1/3] Initial " Kent Overstreet 2021-05-09 14:36 ` Eryu Guan 2021-05-23 22:51 ` Kent Overstreet 2021-05-24 3:56 ` Eryu Guan 2021-05-24 4:04 ` Kent Overstreet 2021-05-24 4:22 ` Eryu Guan 2021-05-24 4:48 ` Kent Overstreet 2021-04-27 16:44 ` [PATCH 2/3] Improved .gitignore Kent Overstreet 2021-04-27 16:44 ` [PATCH 3/3] Use --yes option to lvcreate Kent Overstreet 2021-04-27 17:03 ` Eryu Guan 2021-04-27 20:29 ` Kent Overstreet 2021-04-27 20:43 ` Matthew Wilcox 2021-04-27 21:02 ` Eric Biggers 2021-04-27 21:18 ` Kent Overstreet 2021-05-09 14:20 ` [PATCH 0/3] bcachefs support Eryu Guan 2021-05-10 23:14 ` Dave Chinner 2021-05-11 1:26 ` Darrick J. Wong 2021-05-16 13:54 ` Eryu Guan
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).