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

From: "Darrick J. Wong" <djwong@kernel.org>
To: Theodore Ts'o <tytso@mit.edu>
Cc: fstests@vger.kernel.org
Subject: Re: [PATCH] generic/681, generic/682: add debugging information to $seqreq.full
Date: Mon, 25 Apr 2022 19:54:43 -0700	[thread overview]
Message-ID: <20220426025443.GH17014@magnolia> (raw)
In-Reply-To: <20220425211450.3842888-1-tytso@mit.edu>

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
> 