diff mbox series

[2/4] generic/537: disable quota mount options for pre-metadir rt filesystems

Message ID 174259233999.743619.6582695769493412159.stgit@frogsfrogsfrogs (mailing list archive)
State New
Headers show
Series [1/4] xfs/614: determine the sector size of the fs image by doing a test format | expand

Commit Message

Darrick J. Wong March 21, 2025, 9:27 p.m. UTC 
From: Darrick J. Wong <djwong@kernel.org>

Fix this regression in generic/537:

mount: /opt: permission denied.
       dmesg(1) may have more information after failed mount system call.
mount -o uquota,gquota,pquota, -o ro,norecovery -ortdev=/dev/sdb4 /dev/sda4 /opt failed
mount -o uquota,gquota,pquota, -o ro,norecovery -ortdev=/dev/sdb4 /dev/sda4 /opt failed
(see /var/tmp/fstests/generic/537.full for details)

for reasons explained in the giant comment.  TLDR: quota and rt aren't
compatible on older xfs filesystems so we have to work around that.

Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
 tests/generic/537 |   17 +++++++++++++++++
 1 file changed, 17 insertions(+)

Comments

Zorro Lang March 22, 2025, 2:37 p.m. UTC | #1 
On Fri, Mar 21, 2025 at 02:27:54PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Fix this regression in generic/537:
> 
> mount: /opt: permission denied.
>        dmesg(1) may have more information after failed mount system call.
> mount -o uquota,gquota,pquota, -o ro,norecovery -ortdev=/dev/sdb4 /dev/sda4 /opt failed
> mount -o uquota,gquota,pquota, -o ro,norecovery -ortdev=/dev/sdb4 /dev/sda4 /opt failed
> (see /var/tmp/fstests/generic/537.full for details)
> 
> for reasons explained in the giant comment.  TLDR: quota and rt aren't
> compatible on older xfs filesystems so we have to work around that.
> 
> Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
> ---
>  tests/generic/537 |   17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> 
> diff --git a/tests/generic/537 b/tests/generic/537
> index f57bc1561dd57e..3be743c4133f4f 100755
> --- a/tests/generic/537
> +++ b/tests/generic/537
> @@ -18,6 +18,7 @@ _begin_fstest auto quick trim
>  
>  # Import common functions.
>  . ./common/filter
> +. ./common/quota
>  
>  _require_scratch
>  _require_fstrim
> @@ -36,6 +37,22 @@ _scratch_mount -o ro >> $seqres.full 2>&1
>  $FSTRIM_PROG -v $SCRATCH_MNT >> $seqres.full 2>&1
>  _scratch_unmount
>  
> +# As of kernel commit 9f0902091c332b ("xfs: Do not allow norecovery mount with
> +# quotacheck"), it is no longer possible to mount with "norecovery" and any
> +# quota mount option if the quota mount options would require a metadata update
> +# such as quotacheck.  For a pre-metadir XFS filesystem with a realtime volume
> +# and quota-enabling options, the first two mount attempts will have succeeded
> +# but with quotas disabled.  The mount option parsing for this next mount
> +# attempt will see the same quota-enabling options and a lack of qflags in the
> +# ondisk metadata and reject the mount because it thinks that will require
> +# quotacheck.  Edit out the quota mount options for this specific
> +# configuration.
> +if [ "$FSTYP" = "xfs" ]; then
> +	if [ "$USE_EXTERNAL" = "yes" ] && [ -n "$SCRATCH_RTDEV" ]; then
> +		_qmount_option ""
> +	fi
> +fi

I don't know if there's a better way, maybe a _require_no_quota, or _disable_qmount?

Anyway, for this single case, it's fine for me.

Reviewed-by: Zorro Lang <zlang@redhat.com>

> +
>  echo "fstrim on ro mount with no log replay"
>  norecovery="norecovery"
>  test $FSTYP = "btrfs" && norecovery=nologreplay
>
Darrick J. Wong March 25, 2025, 12:51 a.m. UTC | #2 
On Sat, Mar 22, 2025 at 10:37:54PM +0800, Zorro Lang wrote:
> On Fri, Mar 21, 2025 at 02:27:54PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > Fix this regression in generic/537:
> > 
> > mount: /opt: permission denied.
> >        dmesg(1) may have more information after failed mount system call.
> > mount -o uquota,gquota,pquota, -o ro,norecovery -ortdev=/dev/sdb4 /dev/sda4 /opt failed
> > mount -o uquota,gquota,pquota, -o ro,norecovery -ortdev=/dev/sdb4 /dev/sda4 /opt failed
> > (see /var/tmp/fstests/generic/537.full for details)
> > 
> > for reasons explained in the giant comment.  TLDR: quota and rt aren't
> > compatible on older xfs filesystems so we have to work around that.
> > 
> > Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
> > ---
> >  tests/generic/537 |   17 +++++++++++++++++
> >  1 file changed, 17 insertions(+)
> > 
> > 
> > diff --git a/tests/generic/537 b/tests/generic/537
> > index f57bc1561dd57e..3be743c4133f4f 100755
> > --- a/tests/generic/537
> > +++ b/tests/generic/537
> > @@ -18,6 +18,7 @@ _begin_fstest auto quick trim
> >  
> >  # Import common functions.
> >  . ./common/filter
> > +. ./common/quota
> >  
> >  _require_scratch
> >  _require_fstrim
> > @@ -36,6 +37,22 @@ _scratch_mount -o ro >> $seqres.full 2>&1
> >  $FSTRIM_PROG -v $SCRATCH_MNT >> $seqres.full 2>&1
> >  _scratch_unmount
> >  
> > +# As of kernel commit 9f0902091c332b ("xfs: Do not allow norecovery mount with
> > +# quotacheck"), it is no longer possible to mount with "norecovery" and any
> > +# quota mount option if the quota mount options would require a metadata update
> > +# such as quotacheck.  For a pre-metadir XFS filesystem with a realtime volume
> > +# and quota-enabling options, the first two mount attempts will have succeeded
> > +# but with quotas disabled.  The mount option parsing for this next mount
> > +# attempt will see the same quota-enabling options and a lack of qflags in the
> > +# ondisk metadata and reject the mount because it thinks that will require
> > +# quotacheck.  Edit out the quota mount options for this specific
> > +# configuration.
> > +if [ "$FSTYP" = "xfs" ]; then
> > +	if [ "$USE_EXTERNAL" = "yes" ] && [ -n "$SCRATCH_RTDEV" ]; then
> > +		_qmount_option ""
> > +	fi
> > +fi
> 
> I don't know if there's a better way, maybe a _require_no_quota, or _disable_qmount?

_require_no_quota would decrease test coverage.

There is no such helper as _disable_qmount...?

> Anyway, for this single case, it's fine for me.
> 
> Reviewed-by: Zorro Lang <zlang@redhat.com>

Thanks!

--D

> > +
> >  echo "fstrim on ro mount with no log replay"
> >  norecovery="norecovery"
> >  test $FSTYP = "btrfs" && norecovery=nologreplay
> > 
> 
>
diff mbox series

Patch

diff --git a/tests/generic/537 b/tests/generic/537
index f57bc1561dd57e..3be743c4133f4f 100755
--- a/tests/generic/537
+++ b/tests/generic/537
@@ -18,6 +18,7 @@  _begin_fstest auto quick trim
 
 # Import common functions.
 . ./common/filter
+. ./common/quota
 
 _require_scratch
 _require_fstrim
@@ -36,6 +37,22 @@  _scratch_mount -o ro >> $seqres.full 2>&1
 $FSTRIM_PROG -v $SCRATCH_MNT >> $seqres.full 2>&1
 _scratch_unmount
 
+# As of kernel commit 9f0902091c332b ("xfs: Do not allow norecovery mount with
+# quotacheck"), it is no longer possible to mount with "norecovery" and any
+# quota mount option if the quota mount options would require a metadata update
+# such as quotacheck.  For a pre-metadir XFS filesystem with a realtime volume
+# and quota-enabling options, the first two mount attempts will have succeeded
+# but with quotas disabled.  The mount option parsing for this next mount
+# attempt will see the same quota-enabling options and a lack of qflags in the
+# ondisk metadata and reject the mount because it thinks that will require
+# quotacheck.  Edit out the quota mount options for this specific
+# configuration.
+if [ "$FSTYP" = "xfs" ]; then
+	if [ "$USE_EXTERNAL" = "yes" ] && [ -n "$SCRATCH_RTDEV" ]; then
+		_qmount_option ""
+	fi
+fi
+
 echo "fstrim on ro mount with no log replay"
 norecovery="norecovery"
 test $FSTYP = "btrfs" && norecovery=nologreplay