[PATCH 0/2] generic/681 and generic/682 fixes

[PATCH 0/2] generic/681 and generic/682 fixes

[Anthony Iliopoulos]

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

[PATCH 1/2] generic/68[12]: use the dir blocksize for xfs filesystems

[Anthony Iliopoulos]

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

[PATCH 2/2] generic/682: update and fix-up golden output

[Anthony Iliopoulos]

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

Re: [PATCH 1/2] generic/68[12]: use the dir blocksize for xfs filesystems

[Andrey Albershteyn]

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

Re: [PATCH 2/2] generic/682: update and fix-up golden output

[Andrey Albershteyn]

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

Re: [PATCH 2/2] generic/682: update and fix-up golden output

[Zorro Lang]

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
> 
> 

Re: [PATCH 2/2] generic/682: update and fix-up golden output

[Anthony Iliopoulos]

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