* [PATCH 0/2] generic/681 and generic/682 fixes @ 2024-02-01 16:17 Anthony Iliopoulos 2024-02-01 16:17 ` [PATCH 1/2] generic/68[12]: use the dir blocksize for xfs filesystems Anthony Iliopoulos 2024-02-01 16:17 ` [PATCH 2/2] generic/682: update and fix-up golden output Anthony Iliopoulos 0 siblings, 2 replies; 7+ messages in thread From: Anthony Iliopoulos @ 2024-02-01 16:17 UTC (permalink / raw) To: fstests A couple of fixes on g/68[12]. Anthony Iliopoulos (2): generic/68[12]: use the dir blocksize for xfs filesystems generic/682: update and fix-up golden output tests/generic/681 | 6 +++++- tests/generic/682 | 14 ++++++++++++-- tests/generic/682.out | 2 +- 3 files changed, 18 insertions(+), 4 deletions(-) -- 2.41.0 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] generic/68[12]: use the dir blocksize for xfs filesystems 2024-02-01 16:17 [PATCH 0/2] generic/681 and generic/682 fixes Anthony Iliopoulos @ 2024-02-01 16:17 ` Anthony Iliopoulos 2024-02-02 10:56 ` Andrey Albershteyn 2024-02-01 16:17 ` [PATCH 2/2] generic/682: update and fix-up golden output Anthony Iliopoulos 1 sibling, 1 reply; 7+ messages in thread From: Anthony Iliopoulos @ 2024-02-01 16:17 UTC (permalink / raw) To: fstests The tests are using the filesystem block size for calculating the number of dirents required to fill a 2-block directory. For v4 xfs filesystems formatted with fs blocksize of 512 bytes this is failing, as the tests do not take into account that the directory block size is not always equal to the filesystem block size. As such, the tests never go over quota, and even if they did there is no hard block limit being set (due to 512 / 1024 = 0 calculation in setquota). Use the directory blocksize instead of the filesystem blocksize, when the fstype under test is xfs. Signed-off-by: Anthony Iliopoulos <ailiop@suse.com> --- common/rc | 11 +++++++++++ tests/generic/681 | 2 +- tests/generic/682 | 2 +- 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/common/rc b/common/rc index a9e0ba7e22f1..8be375965aaa 100644 --- a/common/rc +++ b/common/rc @@ -4719,6 +4719,17 @@ _get_block_size() stat -f -c %S $1 } +# Obtain the directory block size of an fs. +_get_dir_block_size() +{ + case "$FSTYP" in + xfs) + _xfs_get_dir_blocksize $1 ;; + *) + _get_block_size $1 ;; + esac +} + # Require that the fundamental allocation unit of a file is the same as the # filesystem block size. The sole parameter must be the root dir of a # filesystem. diff --git a/tests/generic/681 b/tests/generic/681 index 090da795ce64..f41647741a7d 100755 --- a/tests/generic/681 +++ b/tests/generic/681 @@ -30,7 +30,7 @@ _scratch_mkfs > "$seqres.full" 2>&1 _qmount_option usrquota _qmount -blocksize=$(_get_block_size $SCRATCH_MNT) +blocksize=$(_get_dir_block_size $SCRATCH_MNT) scratchdir=$SCRATCH_MNT/dir scratchfile=$SCRATCH_MNT/file mkdir $scratchdir diff --git a/tests/generic/682 b/tests/generic/682 index b4cd0cd9dd2e..a51d18338742 100755 --- a/tests/generic/682 +++ b/tests/generic/682 @@ -30,7 +30,7 @@ _scratch_mkfs > "$seqres.full" 2>&1 _qmount_option usrquota _qmount -blocksize=$(_get_block_size $SCRATCH_MNT) +blocksize=$(_get_dir_block_size $SCRATCH_MNT) scratchdir=$SCRATCH_MNT/dir scratchfile=$SCRATCH_MNT/file stagedir=$SCRATCH_MNT/staging -- 2.41.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] generic/68[12]: use the dir blocksize for xfs filesystems 2024-02-01 16:17 ` [PATCH 1/2] generic/68[12]: use the dir blocksize for xfs filesystems Anthony Iliopoulos @ 2024-02-02 10:56 ` Andrey Albershteyn 0 siblings, 0 replies; 7+ messages in thread From: Andrey Albershteyn @ 2024-02-02 10:56 UTC (permalink / raw) To: Anthony Iliopoulos; +Cc: fstests On 2024-02-01 17:17:31, Anthony Iliopoulos wrote: > The tests are using the filesystem block size for calculating the number > of dirents required to fill a 2-block directory. For v4 xfs filesystems > formatted with fs blocksize of 512 bytes this is failing, as the tests > do not take into account that the directory block size is not always > equal to the filesystem block size. As such, the tests never go over > quota, and even if they did there is no hard block limit being set (due > to 512 / 1024 = 0 calculation in setquota). > > Use the directory blocksize instead of the filesystem blocksize, when > the fstype under test is xfs. > > Signed-off-by: Anthony Iliopoulos <ailiop@suse.com> > --- LGTM Reviewed-by: Andrey Albershteyn <aalbersh@redhat.com> -- - Andrey ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2] generic/682: update and fix-up golden output 2024-02-01 16:17 [PATCH 0/2] generic/681 and generic/682 fixes Anthony Iliopoulos 2024-02-01 16:17 ` [PATCH 1/2] generic/68[12]: use the dir blocksize for xfs filesystems Anthony Iliopoulos @ 2024-02-01 16:17 ` Anthony Iliopoulos 2024-02-02 11:00 ` Andrey Albershteyn 2024-02-02 11:56 ` Zorro Lang 1 sibling, 2 replies; 7+ messages in thread From: Anthony Iliopoulos @ 2024-02-01 16:17 UTC (permalink / raw) To: fstests coreutils v9.4 introduced a change in the error output of mv under certain errno values via commit 3cb862ce5f10 ("mv: better diagnostic for 'mv dir x' failure"), which broke the golden output. Update golden output to match the change, and further add an output filter to avoid having the test fail on environments that ran with an older coreutils release, taken from commit d9323ad7a05e ("generic/245: Filter mv error message"). Signed-off-by: Anthony Iliopoulos <ailiop@suse.com> --- tests/generic/682 | 8 +++++++- tests/generic/682.out | 2 +- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/tests/generic/682 b/tests/generic/682 index a51d18338742..417598804bde 100755 --- a/tests/generic/682 +++ b/tests/generic/682 @@ -54,6 +54,12 @@ repquota -upn $SCRATCH_MNT >> $seqres.full echo $(ls $scratchdir | wc -l) files in $scratchdir >> $seqres.full ls -sld $scratchdir >> $seqres.full +_filter_mv_output() +{ + sed -e "s,cannot move .* to \(.*\):\(.*\),cannot overwrite \1:\2,g" \ + -e 's/y[0-9]*/yXXX/g' +} + # Fail at renaming into the directory as qa_user to ensure quota enforcement # works chmod a+rwx $stagedir @@ -62,7 +68,7 @@ for ((i = 0; i < dirents; i++)); do name=$(printf "y%0254d" $i) ln $scratchfile $stagedir/$name su - "$qa_user" -c "mv $stagedir/$name $scratchdir/$name" 2>&1 | \ - _filter_scratch | sed -e 's/y[0-9]*/yXXX/g' + _filter_scratch | _filter_mv_output test "${PIPESTATUS[0]}" -ne 0 && break done repquota -upn $SCRATCH_MNT >> $seqres.full diff --git a/tests/generic/682.out b/tests/generic/682.out index b74708117237..db22d5f60487 100644 --- a/tests/generic/682.out +++ b/tests/generic/682.out @@ -1,3 +1,3 @@ QA output created by 682 -mv: cannot move 'SCRATCH_MNT/staging/yXXX' to 'SCRATCH_MNT/dir/yXXX': Disk quota exceeded +mv: cannot overwrite 'SCRATCH_MNT/dir/yXXX': Disk quota exceeded Silence is golden -- 2.41.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] generic/682: update and fix-up golden output 2024-02-01 16:17 ` [PATCH 2/2] generic/682: update and fix-up golden output Anthony Iliopoulos @ 2024-02-02 11:00 ` Andrey Albershteyn 2024-02-02 11:56 ` Zorro Lang 1 sibling, 0 replies; 7+ messages in thread From: Andrey Albershteyn @ 2024-02-02 11:00 UTC (permalink / raw) To: Anthony Iliopoulos; +Cc: fstests On 2024-02-01 17:17:32, Anthony Iliopoulos wrote: > coreutils v9.4 introduced a change in the error output of mv under > certain errno values via commit 3cb862ce5f10 ("mv: better diagnostic for > 'mv dir x' failure"), which broke the golden output. > > Update golden output to match the change, and further add an output > filter to avoid having the test fail on environments that ran with an > older coreutils release, taken from commit d9323ad7a05e ("generic/245: > Filter mv error message"). > > Signed-off-by: Anthony Iliopoulos <ailiop@suse.com> > --- LGTM Reviewed-by: Andrey Albershteyn <aalbersh@redhat.com> -- - Andrey ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] generic/682: update and fix-up golden output 2024-02-01 16:17 ` [PATCH 2/2] generic/682: update and fix-up golden output Anthony Iliopoulos 2024-02-02 11:00 ` Andrey Albershteyn @ 2024-02-02 11:56 ` Zorro Lang 2024-02-06 17:31 ` Anthony Iliopoulos 1 sibling, 1 reply; 7+ messages in thread From: Zorro Lang @ 2024-02-02 11:56 UTC (permalink / raw) To: Anthony Iliopoulos; +Cc: fstests On Thu, Feb 01, 2024 at 05:17:32PM +0100, Anthony Iliopoulos wrote: > coreutils v9.4 introduced a change in the error output of mv under > certain errno values via commit 3cb862ce5f10 ("mv: better diagnostic for > 'mv dir x' failure"), which broke the golden output. > > Update golden output to match the change, and further add an output > filter to avoid having the test fail on environments that ran with an > older coreutils release, taken from commit d9323ad7a05e ("generic/245: > Filter mv error message"). > > Signed-off-by: Anthony Iliopoulos <ailiop@suse.com> > --- > tests/generic/682 | 8 +++++++- > tests/generic/682.out | 2 +- > 2 files changed, 8 insertions(+), 2 deletions(-) > > diff --git a/tests/generic/682 b/tests/generic/682 > index a51d18338742..417598804bde 100755 > --- a/tests/generic/682 > +++ b/tests/generic/682 > @@ -54,6 +54,12 @@ repquota -upn $SCRATCH_MNT >> $seqres.full > echo $(ls $scratchdir | wc -l) files in $scratchdir >> $seqres.full > ls -sld $scratchdir >> $seqres.full > > +_filter_mv_output() > +{ > + sed -e "s,cannot move .* to \(.*\):\(.*\),cannot overwrite \1:\2,g" \ Hi Anthony, Thanks for this fix, I remember there's a mv filter in generic/245, we just merged a patch same with above line: d9323ad7a05e ("generic/245: Filter mv error message") I think you can merge the _filter_directory_not_empty() of g/245 with this patch into one _filter_mv_output() function, and call it in g/245 and g/682 (check if there're more cases need mv output). Thanks, Zorro > + -e 's/y[0-9]*/yXXX/g' > +} > + > # Fail at renaming into the directory as qa_user to ensure quota enforcement > # works > chmod a+rwx $stagedir > @@ -62,7 +68,7 @@ for ((i = 0; i < dirents; i++)); do > name=$(printf "y%0254d" $i) > ln $scratchfile $stagedir/$name > su - "$qa_user" -c "mv $stagedir/$name $scratchdir/$name" 2>&1 | \ > - _filter_scratch | sed -e 's/y[0-9]*/yXXX/g' > + _filter_scratch | _filter_mv_output > test "${PIPESTATUS[0]}" -ne 0 && break > done > repquota -upn $SCRATCH_MNT >> $seqres.full > diff --git a/tests/generic/682.out b/tests/generic/682.out > index b74708117237..db22d5f60487 100644 > --- a/tests/generic/682.out > +++ b/tests/generic/682.out > @@ -1,3 +1,3 @@ > QA output created by 682 > -mv: cannot move 'SCRATCH_MNT/staging/yXXX' to 'SCRATCH_MNT/dir/yXXX': Disk quota exceeded > +mv: cannot overwrite 'SCRATCH_MNT/dir/yXXX': Disk quota exceeded > Silence is golden > -- > 2.41.0 > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] generic/682: update and fix-up golden output 2024-02-02 11:56 ` Zorro Lang @ 2024-02-06 17:31 ` Anthony Iliopoulos 0 siblings, 0 replies; 7+ messages in thread From: Anthony Iliopoulos @ 2024-02-06 17:31 UTC (permalink / raw) To: Zorro Lang; +Cc: fstests On Fri, Feb 02, 2024 at 07:56:05PM +0800, Zorro Lang wrote: > On Thu, Feb 01, 2024 at 05:17:32PM +0100, Anthony Iliopoulos wrote: > > coreutils v9.4 introduced a change in the error output of mv under > > certain errno values via commit 3cb862ce5f10 ("mv: better diagnostic for > > 'mv dir x' failure"), which broke the golden output. > > > > Update golden output to match the change, and further add an output > > filter to avoid having the test fail on environments that ran with an > > older coreutils release, taken from commit d9323ad7a05e ("generic/245: > > Filter mv error message"). > > > > Signed-off-by: Anthony Iliopoulos <ailiop@suse.com> > > --- > > tests/generic/682 | 8 +++++++- > > tests/generic/682.out | 2 +- > > 2 files changed, 8 insertions(+), 2 deletions(-) > > > > diff --git a/tests/generic/682 b/tests/generic/682 > > index a51d18338742..417598804bde 100755 > > --- a/tests/generic/682 > > +++ b/tests/generic/682 > > @@ -54,6 +54,12 @@ repquota -upn $SCRATCH_MNT >> $seqres.full > > echo $(ls $scratchdir | wc -l) files in $scratchdir >> $seqres.full > > ls -sld $scratchdir >> $seqres.full > > > > +_filter_mv_output() > > +{ > > + sed -e "s,cannot move .* to \(.*\):\(.*\),cannot overwrite \1:\2,g" \ > > Hi Anthony, > > Thanks for this fix, I remember there's a mv filter in generic/245, we > just merged a patch same with above line: > d9323ad7a05e ("generic/245: Filter mv error message") > > I think you can merge the _filter_directory_not_empty() of g/245 with > this patch into one _filter_mv_output() function, and call it in g/245 > and g/682 (check if there're more cases need mv output). Make sense, I'll send out a v2 shortly with the filters consolidated. AFAICT only g/245 and g/682 are affected by the particular coreutils mv error output change, due to their golden output. The only other tests are g/{369,419}: $ git grep 'cannot move' tests/ tests/generic/369.out:mv: cannot move 'h' to 'd7/h': Permission denied tests/generic/369.out:mv: cannot move 'h' to 's7/h': Permission denied tests/generic/419.out:mv: cannot move 'SCRATCH_MNT/edir/NOKEY_NAME' to 'SCRATCH_MNT/edir/NOKEY_NAME': Required key not available but those aren't affected as coreutils commit 3cb862ce5f10 only changed the error output for specific err codes (EDQUOT, EEXIST, EISDIR, EMLINK, ENOSPC, ENOTEMPTY, ETXTBSY), while g/369 is expecting EPERM and g/419 is ENOKEY. Regards, Anthony ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2024-02-06 17:31 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-02-01 16:17 [PATCH 0/2] generic/681 and generic/682 fixes Anthony Iliopoulos 2024-02-01 16:17 ` [PATCH 1/2] generic/68[12]: use the dir blocksize for xfs filesystems Anthony Iliopoulos 2024-02-02 10:56 ` Andrey Albershteyn 2024-02-01 16:17 ` [PATCH 2/2] generic/682: update and fix-up golden output Anthony Iliopoulos 2024-02-02 11:00 ` Andrey Albershteyn 2024-02-02 11:56 ` Zorro Lang 2024-02-06 17:31 ` Anthony Iliopoulos
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox