[PATCH] generic/681, generic/682: add debugging information to $seqreq.full

[PATCH] generic/681, generic/682: add debugging information to $seqreq.full

[Theodore Ts'o]

These two tests are checking whether a non-privileged user causing a
block allocation while expanding a directory block when over quota
will fail with an EDQUOT error.  There are three reasons why this can
fail.

* Aa test bug, where if the file system is using cluster allocation
  (for example, ext4 bigalloc) the test doesn't add enough directory
  entries to actually force directory grwoth.

* A file system bug, where the file system allocates blocks but for
  some reason isn't charging the space quota correctly (which
  currently seems to be the case in ext4 with fscrypt).

* A file system bug, where the file system is correctly charging the
  space quota to the unprivileged user, but isn't failing the system
  call with EDQUOT.

By adding some additional debugging information about whether
directory has grown or not (in addition to the existing repquota
output) to the the $seqres.full, it makes easier for the file system
developer to disambiguate between these possibilities.  It's cheap to
do this, and it could save developer time when trying to root cause
the failure.

Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Cc: Darrick J. Wong <djwong@kernel.org>
---
 tests/generic/681 | 4 ++++
 tests/generic/682 | 4 ++++
 2 files changed, 8 insertions(+)

diff --git a/tests/generic/681 b/tests/generic/681
index 2baae9f4..090da795 100755
--- a/tests/generic/681
+++ b/tests/generic/681
@@ -50,6 +50,8 @@ echo "set up quota" >> $seqres.full
 setquota -u $qa_user 0 "$((blocksize / 1024))" 0 0 $SCRATCH_MNT
 chown $qa_user $scratchdir $scratchfile
 repquota -upn $SCRATCH_MNT >> $seqres.full
+echo $(ls $scratchdir | wc -l) files in $scratchdir  >> $seqres.full
+ls -sld $scratchdir  >> $seqres.full
 
 # Fail at appending the directory as qa_user to ensure quota enforcement works
 echo "fail quota" >> $seqres.full
@@ -60,6 +62,8 @@ for ((i = 0; i < dirents; i++)); do
 	test "${PIPESTATUS[0]}" -ne 0 && break
 done
 repquota -upn $SCRATCH_MNT >> $seqres.full
+echo $(ls $scratchdir | wc -l) files in $scratchdir  >> $seqres.full
+ls -sld $scratchdir  >> $seqres.full
 
 # success, all done
 echo Silence is golden
diff --git a/tests/generic/682 b/tests/generic/682
index 31629a56..b4cd0cd9 100755
--- a/tests/generic/682
+++ b/tests/generic/682
@@ -51,6 +51,8 @@ echo "set up quota" >> $seqres.full
 setquota -u $qa_user 0 "$((blocksize / 1024))" 0 0 $SCRATCH_MNT
 chown $qa_user $scratchdir $scratchfile
 repquota -upn $SCRATCH_MNT >> $seqres.full
+echo $(ls $scratchdir | wc -l) files in $scratchdir  >> $seqres.full
+ls -sld $scratchdir  >> $seqres.full
 
 # Fail at renaming into the directory as qa_user to ensure quota enforcement
 # works
@@ -64,6 +66,8 @@ for ((i = 0; i < dirents; i++)); do
 	test "${PIPESTATUS[0]}" -ne 0 && break
 done
 repquota -upn $SCRATCH_MNT >> $seqres.full
+echo $(ls $scratchdir | wc -l) files in $scratchdir  >> $seqres.full
+ls -sld $scratchdir  >> $seqres.full
 
 # success, all done
 echo Silence is golden
-- 
2.31.0

Re: [PATCH] generic/681, generic/682: add debugging information to $seqreq.full

[Darrick J. Wong]

On Mon, Apr 25, 2022 at 05:14:50PM -0400, Theodore Ts'o wrote:
> These two tests are checking whether a non-privileged user causing a
> block allocation while expanding a directory block when over quota
> will fail with an EDQUOT error.  There are three reasons why this can
> fail.
> 
> * Aa test bug, where if the file system is using cluster allocation
>   (for example, ext4 bigalloc) the test doesn't add enough directory
>   entries to actually force directory grwoth.
> 
> * A file system bug, where the file system allocates blocks but for
>   some reason isn't charging the space quota correctly (which
>   currently seems to be the case in ext4 with fscrypt).
> 
> * A file system bug, where the file system is correctly charging the
>   space quota to the unprivileged user, but isn't failing the system
>   call with EDQUOT.
> 
> By adding some additional debugging information about whether
> directory has grown or not (in addition to the existing repquota
> output) to the the $seqres.full, it makes easier for the file system
> developer to disambiguate between these possibilities.  It's cheap to
> do this, and it could save developer time when trying to root cause
> the failure.
> 
> Signed-off-by: Theodore Ts'o <tytso@mit.edu>
> Cc: Darrick J. Wong <djwong@kernel.org>

Sure, why not? :)
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> ---
>  tests/generic/681 | 4 ++++
>  tests/generic/682 | 4 ++++
>  2 files changed, 8 insertions(+)
> 
> diff --git a/tests/generic/681 b/tests/generic/681
> index 2baae9f4..090da795 100755
> --- a/tests/generic/681
> +++ b/tests/generic/681
> @@ -50,6 +50,8 @@ echo "set up quota" >> $seqres.full
>  setquota -u $qa_user 0 "$((blocksize / 1024))" 0 0 $SCRATCH_MNT
>  chown $qa_user $scratchdir $scratchfile
>  repquota -upn $SCRATCH_MNT >> $seqres.full
> +echo $(ls $scratchdir | wc -l) files in $scratchdir  >> $seqres.full
> +ls -sld $scratchdir  >> $seqres.full
>  
>  # Fail at appending the directory as qa_user to ensure quota enforcement works
>  echo "fail quota" >> $seqres.full
> @@ -60,6 +62,8 @@ for ((i = 0; i < dirents; i++)); do
>  	test "${PIPESTATUS[0]}" -ne 0 && break
>  done
>  repquota -upn $SCRATCH_MNT >> $seqres.full
> +echo $(ls $scratchdir | wc -l) files in $scratchdir  >> $seqres.full
> +ls -sld $scratchdir  >> $seqres.full
>  
>  # success, all done
>  echo Silence is golden
> diff --git a/tests/generic/682 b/tests/generic/682
> index 31629a56..b4cd0cd9 100755
> --- a/tests/generic/682
> +++ b/tests/generic/682
> @@ -51,6 +51,8 @@ echo "set up quota" >> $seqres.full
>  setquota -u $qa_user 0 "$((blocksize / 1024))" 0 0 $SCRATCH_MNT
>  chown $qa_user $scratchdir $scratchfile
>  repquota -upn $SCRATCH_MNT >> $seqres.full
> +echo $(ls $scratchdir | wc -l) files in $scratchdir  >> $seqres.full
> +ls -sld $scratchdir  >> $seqres.full
>  
>  # Fail at renaming into the directory as qa_user to ensure quota enforcement
>  # works
> @@ -64,6 +66,8 @@ for ((i = 0; i < dirents; i++)); do
>  	test "${PIPESTATUS[0]}" -ne 0 && break
>  done
>  repquota -upn $SCRATCH_MNT >> $seqres.full
> +echo $(ls $scratchdir | wc -l) files in $scratchdir  >> $seqres.full
> +ls -sld $scratchdir  >> $seqres.full
>  
>  # success, all done
>  echo Silence is golden
> -- 
> 2.31.0
>