diff mbox series

xfs: test quota's project ID on special files

Message ID 20240520170004.669254-2-aalbersh@redhat.com (mailing list archive)
State New, archived
Headers show
Series xfs: test quota's project ID on special files | expand

Commit Message

Andrey Albershteyn May 20, 2024, 5 p.m. UTC 
With addition of FS_IOC_FSSETXATTRAT xfs_quota now can set project
ID on filesystem inodes behind special files. Previously, quota
reporting didn't count inodes of special files created before
project initialization. Only new inodes had project ID set.

Signed-off-by: Andrey Albershteyn <aalbersh@redhat.com>
---

Notes:
    This is part of the patchset which introduces
    FS_IOC_FS[GET|SET]XATTRAT:
    https://lore.kernel.org/linux-xfs/20240520164624.665269-2-aalbersh@redhat.com/T/#t
    https://lore.kernel.org/linux-xfs/20240520165200.667150-2-aalbersh@redhat.com/T/#u

 tests/xfs/608     | 73 +++++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/608.out | 10 +++++++
 2 files changed, 83 insertions(+)
 create mode 100755 tests/xfs/608
 create mode 100644 tests/xfs/608.out

Comments

Darrick J. Wong May 20, 2024, 6:02 p.m. UTC | #1 
On Mon, May 20, 2024 at 07:00:05PM +0200, Andrey Albershteyn wrote:
> With addition of FS_IOC_FSSETXATTRAT xfs_quota now can set project
> ID on filesystem inodes behind special files. Previously, quota
> reporting didn't count inodes of special files created before
> project initialization. Only new inodes had project ID set.
> 
> Signed-off-by: Andrey Albershteyn <aalbersh@redhat.com>
> ---
> 
> Notes:
>     This is part of the patchset which introduces
>     FS_IOC_FS[GET|SET]XATTRAT:
>     https://lore.kernel.org/linux-xfs/20240520164624.665269-2-aalbersh@redhat.com/T/#t
>     https://lore.kernel.org/linux-xfs/20240520165200.667150-2-aalbersh@redhat.com/T/#u
> 
>  tests/xfs/608     | 73 +++++++++++++++++++++++++++++++++++++++++++++++
>  tests/xfs/608.out | 10 +++++++
>  2 files changed, 83 insertions(+)
>  create mode 100755 tests/xfs/608
>  create mode 100644 tests/xfs/608.out
> 
> diff --git a/tests/xfs/608 b/tests/xfs/608
> new file mode 100755
> index 000000000000..3573c764c5f4
> --- /dev/null
> +++ b/tests/xfs/608
> @@ -0,0 +1,73 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2024 Red Hat.  All Rights Reserved.
> +#
> +# FS QA Test 608
> +#
> +# Test that XFS can set quota project ID on special files
> +#
> +. ./common/preamble
> +_begin_fstest auto quota
> +
> +# Override the default cleanup function.
> +_cleanup()
> +{
> +	cd /
> +	rm -r -f $tmp.*
> +	rm -f $tmp.proects $tmp.projid

Not sure why this is "proects" (is that a misspelling of 'projects'?)
but the rm above should take care of these, and then you don't need to
redefine _cleanup.

> +}
> +
> +
> +# Import common functions.
> +. ./common/quota
> +. ./common/filter
> +
> +# Modify as appropriate.
> +_supported_fs xfs
> +_require_scratch
> +_require_xfs_quota
> +_require_user
> +
> +_scratch_mkfs >/dev/null 2>&1

Perhaps dump the output to $seqres.full?

The rest looks ok to me; thank you for including a test with the new
functionality!

--D

> +_qmount_option "pquota"
> +_scratch_mount
> +_require_test_program "af_unix"
> +_require_symlinks
> +_require_mknod
> +
> +function create_af_unix () {
> +	$here/src/af_unix $* || echo af_unix failed
> +}
> +
> +function filter_quota() {
> +	_filter_quota | sed "s~$tmp.proects~PROJECTS_FILE~"
> +}
> +
> +projectdir=$SCRATCH_MNT/prj
> +id=42
> +
> +mkdir $projectdir
> +mkfifo $projectdir/fifo
> +mknod $projectdir/chardev c 1 1
> +mknod $projectdir/blockdev b 1 1
> +create_af_unix $projectdir/socket
> +touch $projectdir/foo
> +ln -s $projectdir/foo $projectdir/symlink
> +touch $projectdir/bar
> +ln -s $projectdir/bar $projectdir/broken-symlink
> +rm -f $projectdir/bar
> +
> +$XFS_QUOTA_PROG -D $tmp.proects -P $tmp.projid -x \
> +	-c "project -sp $projectdir $id" $SCRATCH_DEV | filter_quota
> +$XFS_QUOTA_PROG -D $tmp.proects -P $tmp.projid -x \
> +	-c "limit -p isoft=20 ihard=20 $id " $SCRATCH_DEV | filter_quota
> +$XFS_QUOTA_PROG -D $tmp.proects -P $tmp.projid -x \
> +	-c "project -cp $projectdir $id" $SCRATCH_DEV | filter_quota
> +$XFS_QUOTA_PROG -D $tmp.proects -P $tmp.projid -x \
> +	-c "report -inN -p" $SCRATCH_DEV
> +$XFS_QUOTA_PROG -D $tmp.proects -P $tmp.projid -x \
> +	-c "project -Cp $projectdir $id" $SCRATCH_DEV | filter_quota
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/xfs/608.out b/tests/xfs/608.out
> new file mode 100644
> index 000000000000..c3d56c3c7682
> --- /dev/null
> +++ b/tests/xfs/608.out
> @@ -0,0 +1,10 @@
> +QA output created by 608
> +Setting up project 42 (path SCRATCH_MNT/prj)...
> +Processed 1 (PROJECTS_FILE and cmdline) paths for project 42 with recursion depth infinite (-1).
> +Checking project 42 (path SCRATCH_MNT/prj)...
> +Processed 1 (PROJECTS_FILE and cmdline) paths for project 42 with recursion depth infinite (-1).
> +#0                   3          0          0     00 [--------]
> +#42                  8         20         20     00 [--------]
> +
> +Clearing project 42 (path SCRATCH_MNT/prj)...
> +Processed 1 (PROJECTS_FILE and cmdline) paths for project 42 with recursion depth infinite (-1).
> -- 
> 2.42.0
> 
>
Zorro Lang May 25, 2024, 5:42 a.m. UTC | #2 
On Mon, May 20, 2024 at 07:00:05PM +0200, Andrey Albershteyn wrote:
> With addition of FS_IOC_FSSETXATTRAT xfs_quota now can set project
> ID on filesystem inodes behind special files. Previously, quota
> reporting didn't count inodes of special files created before
> project initialization. Only new inodes had project ID set.
> 
> Signed-off-by: Andrey Albershteyn <aalbersh@redhat.com>
> ---
> 
> Notes:
>     This is part of the patchset which introduces
>     FS_IOC_FS[GET|SET]XATTRAT:
>     https://lore.kernel.org/linux-xfs/20240520164624.665269-2-aalbersh@redhat.com/T/#t
>     https://lore.kernel.org/linux-xfs/20240520165200.667150-2-aalbersh@redhat.com/T/#u

So this test fails on old xfsprogs and kernel which doesn't support
above feature? Do we need a _require_xxxx helper to skip this test?
Or you hope to fail on old kernel to clarify this feature missing?

As this test requires some new patches, better to point out:
  _wants_kernel_commit xxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxx
  _wants_git_commit xfsprogs xxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxx

> 
>  tests/xfs/608     | 73 +++++++++++++++++++++++++++++++++++++++++++++++
>  tests/xfs/608.out | 10 +++++++
>  2 files changed, 83 insertions(+)
>  create mode 100755 tests/xfs/608
>  create mode 100644 tests/xfs/608.out
> 
> diff --git a/tests/xfs/608 b/tests/xfs/608
> new file mode 100755
> index 000000000000..3573c764c5f4
> --- /dev/null
> +++ b/tests/xfs/608
> @@ -0,0 +1,73 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2024 Red Hat.  All Rights Reserved.
> +#
> +# FS QA Test 608
> +#
> +# Test that XFS can set quota project ID on special files
> +#
> +. ./common/preamble
> +_begin_fstest auto quota
> +
> +# Override the default cleanup function.
> +_cleanup()
> +{
> +	cd /
> +	rm -r -f $tmp.*
> +	rm -f $tmp.proects $tmp.projid
                      ^^^^
                   projects? (same below)

And won't "rm -f $tmp.*" help to remove $tmp.proects and $tmp.projid ?
If it does, we can remove this _cleanup, just use the default one.

> +}
> +
> +
> +# Import common functions.
> +. ./common/quota
> +. ./common/filter
> +
> +# Modify as appropriate.
> +_supported_fs xfs
> +_require_scratch
> +_require_xfs_quota
> +_require_user

Does this patch use "fsgqa" user/group?

> +
> +_scratch_mkfs >/dev/null 2>&1
> +_qmount_option "pquota"
> +_scratch_mount

If there's not special reason, we generally do all _require_xxx checking
at first, then mkfs & mount.

> +_require_test_program "af_unix"
> +_require_symlinks
> +_require_mknod

So you might can move above 3 lines over the _scratch_mkfs, looks like
they don't need a SCRATCH_DEV with $FSTYP.

> +
> +function create_af_unix () {

We generally don't use "function", but that's fine if you intend that :)

Thanks,
Zorro

> +	$here/src/af_unix $* || echo af_unix failed
> +}
> +
> +function filter_quota() {
> +	_filter_quota | sed "s~$tmp.proects~PROJECTS_FILE~"
> +}
> +
> +projectdir=$SCRATCH_MNT/prj
> +id=42
> +
> +mkdir $projectdir
> +mkfifo $projectdir/fifo
> +mknod $projectdir/chardev c 1 1
> +mknod $projectdir/blockdev b 1 1
> +create_af_unix $projectdir/socket
> +touch $projectdir/foo
> +ln -s $projectdir/foo $projectdir/symlink
> +touch $projectdir/bar
> +ln -s $projectdir/bar $projectdir/broken-symlink
> +rm -f $projectdir/bar
> +
> +$XFS_QUOTA_PROG -D $tmp.proects -P $tmp.projid -x \
> +	-c "project -sp $projectdir $id" $SCRATCH_DEV | filter_quota
> +$XFS_QUOTA_PROG -D $tmp.proects -P $tmp.projid -x \
> +	-c "limit -p isoft=20 ihard=20 $id " $SCRATCH_DEV | filter_quota
> +$XFS_QUOTA_PROG -D $tmp.proects -P $tmp.projid -x \
> +	-c "project -cp $projectdir $id" $SCRATCH_DEV | filter_quota
> +$XFS_QUOTA_PROG -D $tmp.proects -P $tmp.projid -x \
> +	-c "report -inN -p" $SCRATCH_DEV
> +$XFS_QUOTA_PROG -D $tmp.proects -P $tmp.projid -x \
> +	-c "project -Cp $projectdir $id" $SCRATCH_DEV | filter_quota
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/xfs/608.out b/tests/xfs/608.out
> new file mode 100644
> index 000000000000..c3d56c3c7682
> --- /dev/null
> +++ b/tests/xfs/608.out
> @@ -0,0 +1,10 @@
> +QA output created by 608
> +Setting up project 42 (path SCRATCH_MNT/prj)...
> +Processed 1 (PROJECTS_FILE and cmdline) paths for project 42 with recursion depth infinite (-1).
> +Checking project 42 (path SCRATCH_MNT/prj)...
> +Processed 1 (PROJECTS_FILE and cmdline) paths for project 42 with recursion depth infinite (-1).
> +#0                   3          0          0     00 [--------]
> +#42                  8         20         20     00 [--------]
> +
> +Clearing project 42 (path SCRATCH_MNT/prj)...
> +Processed 1 (PROJECTS_FILE and cmdline) paths for project 42 with recursion depth infinite (-1).
> -- 
> 2.42.0
> 
>
Andrey Albershteyn May 27, 2024, 9:43 a.m. UTC | #3 
On 2024-05-25 13:42:52, Zorro Lang wrote:
> On Mon, May 20, 2024 at 07:00:05PM +0200, Andrey Albershteyn wrote:
> > With addition of FS_IOC_FSSETXATTRAT xfs_quota now can set project
> > ID on filesystem inodes behind special files. Previously, quota
> > reporting didn't count inodes of special files created before
> > project initialization. Only new inodes had project ID set.
> > 
> > Signed-off-by: Andrey Albershteyn <aalbersh@redhat.com>
> > ---
> > 
> > Notes:
> >     This is part of the patchset which introduces
> >     FS_IOC_FS[GET|SET]XATTRAT:
> >     https://lore.kernel.org/linux-xfs/20240520164624.665269-2-aalbersh@redhat.com/T/#t
> >     https://lore.kernel.org/linux-xfs/20240520165200.667150-2-aalbersh@redhat.com/T/#u
> 
> So this test fails on old xfsprogs and kernel which doesn't support
> above feature? Do we need a _require_xxxx helper to skip this test?
> Or you hope to fail on old kernel to clarify this feature missing?
> 
> As this test requires some new patches, better to point out:
>   _wants_kernel_commit xxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxx
>   _wants_git_commit xfsprogs xxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxx

Sure, thanks, will add it.

> 
> > 
> >  tests/xfs/608     | 73 +++++++++++++++++++++++++++++++++++++++++++++++
> >  tests/xfs/608.out | 10 +++++++
> >  2 files changed, 83 insertions(+)
> >  create mode 100755 tests/xfs/608
> >  create mode 100644 tests/xfs/608.out
> > 
> > diff --git a/tests/xfs/608 b/tests/xfs/608
> > new file mode 100755
> > index 000000000000..3573c764c5f4
> > --- /dev/null
> > +++ b/tests/xfs/608
> > @@ -0,0 +1,73 @@
> > +#! /bin/bash
> > +# SPDX-License-Identifier: GPL-2.0
> > +# Copyright (c) 2024 Red Hat.  All Rights Reserved.
> > +#
> > +# FS QA Test 608
> > +#
> > +# Test that XFS can set quota project ID on special files
> > +#
> > +. ./common/preamble
> > +_begin_fstest auto quota
> > +
> > +# Override the default cleanup function.
> > +_cleanup()
> > +{
> > +	cd /
> > +	rm -r -f $tmp.*
> > +	rm -f $tmp.proects $tmp.projid
>                       ^^^^
>                    projects? (same below)
> 
> And won't "rm -f $tmp.*" help to remove $tmp.proects and $tmp.projid ?
> If it does, we can remove this _cleanup, just use the default one.
> 
> > +}
> > +
> > +
> > +# Import common functions.
> > +. ./common/quota
> > +. ./common/filter
> > +
> > +# Modify as appropriate.
> > +_supported_fs xfs
> > +_require_scratch
> > +_require_xfs_quota
> > +_require_user
> 
> Does this patch use "fsgqa" user/group?
> 
> > +
> > +_scratch_mkfs >/dev/null 2>&1
> > +_qmount_option "pquota"
> > +_scratch_mount
> 
> If there's not special reason, we generally do all _require_xxx checking
> at first, then mkfs & mount.
> 
> > +_require_test_program "af_unix"
> > +_require_symlinks
> > +_require_mknod
> 
> So you might can move above 3 lines over the _scratch_mkfs, looks like
> they don't need a SCRATCH_DEV with $FSTYP.
> 
> > +
> > +function create_af_unix () {
> 
> We generally don't use "function", but that's fine if you intend that :)
> 
> Thanks,
> Zorro
> 

Thanks, will apply all your suggestions above.
diff mbox series

Patch

diff --git a/tests/xfs/608 b/tests/xfs/608
new file mode 100755
index 000000000000..3573c764c5f4
--- /dev/null
+++ b/tests/xfs/608
@@ -0,0 +1,73 @@ 
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2024 Red Hat.  All Rights Reserved.
+#
+# FS QA Test 608
+#
+# Test that XFS can set quota project ID on special files
+#
+. ./common/preamble
+_begin_fstest auto quota
+
+# Override the default cleanup function.
+_cleanup()
+{
+	cd /
+	rm -r -f $tmp.*
+	rm -f $tmp.proects $tmp.projid
+}
+
+
+# Import common functions.
+. ./common/quota
+. ./common/filter
+
+# Modify as appropriate.
+_supported_fs xfs
+_require_scratch
+_require_xfs_quota
+_require_user
+
+_scratch_mkfs >/dev/null 2>&1
+_qmount_option "pquota"
+_scratch_mount
+_require_test_program "af_unix"
+_require_symlinks
+_require_mknod
+
+function create_af_unix () {
+	$here/src/af_unix $* || echo af_unix failed
+}
+
+function filter_quota() {
+	_filter_quota | sed "s~$tmp.proects~PROJECTS_FILE~"
+}
+
+projectdir=$SCRATCH_MNT/prj
+id=42
+
+mkdir $projectdir
+mkfifo $projectdir/fifo
+mknod $projectdir/chardev c 1 1
+mknod $projectdir/blockdev b 1 1
+create_af_unix $projectdir/socket
+touch $projectdir/foo
+ln -s $projectdir/foo $projectdir/symlink
+touch $projectdir/bar
+ln -s $projectdir/bar $projectdir/broken-symlink
+rm -f $projectdir/bar
+
+$XFS_QUOTA_PROG -D $tmp.proects -P $tmp.projid -x \
+	-c "project -sp $projectdir $id" $SCRATCH_DEV | filter_quota
+$XFS_QUOTA_PROG -D $tmp.proects -P $tmp.projid -x \
+	-c "limit -p isoft=20 ihard=20 $id " $SCRATCH_DEV | filter_quota
+$XFS_QUOTA_PROG -D $tmp.proects -P $tmp.projid -x \
+	-c "project -cp $projectdir $id" $SCRATCH_DEV | filter_quota
+$XFS_QUOTA_PROG -D $tmp.proects -P $tmp.projid -x \
+	-c "report -inN -p" $SCRATCH_DEV
+$XFS_QUOTA_PROG -D $tmp.proects -P $tmp.projid -x \
+	-c "project -Cp $projectdir $id" $SCRATCH_DEV | filter_quota
+
+# success, all done
+status=0
+exit
diff --git a/tests/xfs/608.out b/tests/xfs/608.out
new file mode 100644
index 000000000000..c3d56c3c7682
--- /dev/null
+++ b/tests/xfs/608.out
@@ -0,0 +1,10 @@ 
+QA output created by 608
+Setting up project 42 (path SCRATCH_MNT/prj)...
+Processed 1 (PROJECTS_FILE and cmdline) paths for project 42 with recursion depth infinite (-1).
+Checking project 42 (path SCRATCH_MNT/prj)...
+Processed 1 (PROJECTS_FILE and cmdline) paths for project 42 with recursion depth infinite (-1).
+#0                   3          0          0     00 [--------]
+#42                  8         20         20     00 [--------]
+
+Clearing project 42 (path SCRATCH_MNT/prj)...
+Processed 1 (PROJECTS_FILE and cmdline) paths for project 42 with recursion depth infinite (-1).