[PATCH v2] generic: test for failure to unlock inode after chgrp fails with EDQUOT

[PATCH v2] generic: test for failure to unlock inode after chgrp fails with EDQUOT

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

This is a regression test that checks for xfs drivers that fail to
unlock the inode after changing the group id fails with EDQUOT.  It
pairs with "xfs: fix missing ILOCK unlock when xfs_setattr_nonsize fails
due to EDQUOT".

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
v2: add commit id
---
 tests/generic/719     |   59 +++++++++++++++++++++++++++++++++++++++++++++++++
 tests/generic/719.out |    2 ++
 tests/generic/group   |    1 +
 3 files changed, 62 insertions(+)
 create mode 100755 tests/generic/719
 create mode 100644 tests/generic/719.out

diff --git a/tests/generic/719 b/tests/generic/719
new file mode 100755
index 00000000..3da2539c
--- /dev/null
+++ b/tests/generic/719
@@ -0,0 +1,59 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0-or-newer
+# Copyright (c) 2019, Oracle and/or its affiliates.  All Rights Reserved.
+#
+# FS QA Test No. 719
+#
+# Regression test for chgrp returning to userspace with ILOCK held after a
+# hard quota error.  This causes the filesystem to hang, so it is (for now)
+# a dangerous test.
+#
+# This test goes with commit 1fb254aa983bf ("xfs: fix missing ILOCK unlock when
+# xfs_setattr_nonsize fails due to EDQUOT")
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1    # failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	cd /
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/quota
+. ./common/filter
+
+# real QA test starts here
+_supported_os Linux
+_supported_fs generic
+_require_scratch
+_require_quota
+_require_user
+
+rm -f $seqres.full
+
+_qmount_option "grpquota"
+_scratch_mkfs > $seqres.full
+_qmount
+
+dir="$SCRATCH_MNT/dummy"
+mkdir -p $dir
+chown $qa_user $dir
+$XFS_QUOTA_PROG -x -f -c "limit -g bsoft=100k bhard=100k $qa_user" $SCRATCH_MNT
+
+$XFS_IO_PROG -f -c 'pwrite -S 0x58 0 1m' $dir/foo >> $seqres.full
+chown $qa_user "${dir}/foo"
+su $qa_user -c "chgrp $qa_user ${dir}/foo" 2>&1 | _filter_scratch
+ls -la ${dir} >> $seqres.full
+$XFS_QUOTA_PROG -x -f -c 'report -hag' $SCRATCH_MNT >> $seqres.full
+
+# success, all done
+status=0
+exit
diff --git a/tests/generic/719.out b/tests/generic/719.out
new file mode 100644
index 00000000..8f9d51b5
--- /dev/null
+++ b/tests/generic/719.out
@@ -0,0 +1,2 @@
+QA output created by 719
+chgrp: changing group of 'SCRATCH_MNT/dummy/foo': Disk quota exceeded
diff --git a/tests/generic/group b/tests/generic/group
index 2e4a6f79..cd418106 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -568,3 +568,4 @@
 563 auto quick
 564 auto quick copy_range
 565 auto quick copy_range
+719 auto quick quota metadata

Re: [PATCH v2] generic: test for failure to unlock inode after chgrp fails with EDQUOT

[Thomas Gleixner]

On Mon, 26 Aug 2019, Darrick J. Wong wrote:
> +++ b/tests/generic/719
> @@ -0,0 +1,59 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0-or-newer

Please run scripts/spdxcheck.py on that file and consult the licensing
documentation.

Thanks,

	tglx

Re: [PATCH v2] generic: test for failure to unlock inode after chgrp fails with EDQUOT

[Darrick J. Wong]

On Tue, Aug 27, 2019 at 08:13:19AM +0200, Thomas Gleixner wrote:
> On Mon, 26 Aug 2019, Darrick J. Wong wrote:
> > +++ b/tests/generic/719
> > @@ -0,0 +1,59 @@
> > +#! /bin/bash
> > +# SPDX-License-Identifier: GPL-2.0-or-newer
> 
> Please run scripts/spdxcheck.py on that file and consult the licensing
> documentation.

-or-later, sorry.

So .... now that everyone who wanted these SPDX identifiers have spread
"GPL-2.0+" around the kernel and related projects (xfsprogs, xfstests)
just in time for SPDX 3.0 to deprecate the "+" syntax, what are we
supposed to do?  Another treewide change to fiddle with SPDX syntax?
Can we just put:

Valid-License-Identifier: GPL-2.0+
Valid-License-Identifier: GPL-2.0-or-later

in the LICENSES/GPL-2.0 file like the kernel does?

Is that even going to stay that way?  I thought I heard that Greg was
working on phasing out the "2.0+" tags since SPDX deprecated that?

I think xfsprogs and xfstests just follow whatever the kernel does, but
AFAICT this whole initiative /continues/ to communicate poorly with the
maintainers about (1) how this is supposed to benefit us and (2) what we
are supposed to do to maintain all of it.

Do we have to get lawyers involved to roll to a new SPDX version?  Will
LF do that for (at least) the projects hosted on kernel.org?  Should we
just do it and hope for the best since IANAFL?  I know how to review
code.  I don't know how to review licensing and all the tiny
implications that go along with things like this.  I don't even feel
confident that the two identifiers above are exactly the same, because
all I know is that I read it on a webpage somewhere.

I for one still have heard abso-f*cking-lutely nothing about what is
this SPDX change other than Greg shoving treewide changes in the kernel.
That sufficed to get the mechanical work done (at the cost of a lot of
frustration for Greg) but this doesn't help me sustain our community.

Guidance needed.  Apologies all around if this rant is misdirected, but
I have no idea who (if anyone) is maintaining SPDX tags.  There's no
entry for LICENSES/ in MAINTAINERS, which is where I looked first.

--D

> Thanks,
> 
> 	tglx

Re: [PATCH v2] generic: test for failure to unlock inode after chgrp fails with EDQUOT

[Thomas Gleixner]

Darrick,

On Tue, 27 Aug 2019, Darrick J. Wong wrote:

> On Tue, Aug 27, 2019 at 08:13:19AM +0200, Thomas Gleixner wrote:
> > On Mon, 26 Aug 2019, Darrick J. Wong wrote:
> > > +++ b/tests/generic/719
> > > @@ -0,0 +1,59 @@
> > > +#! /bin/bash
> > > +# SPDX-License-Identifier: GPL-2.0-or-newer
> > 
> > Please run scripts/spdxcheck.py on that file and consult the licensing
> > documentation.
> 
> -or-later, sorry.
> 
> So .... now that everyone who wanted these SPDX identifiers have spread
> "GPL-2.0+" around the kernel and related projects (xfsprogs, xfstests)
> just in time for SPDX 3.0 to deprecate the "+" syntax, what are we
> supposed to do?  Another treewide change to fiddle with SPDX syntax?
> Can we just put:
> 
> Valid-License-Identifier: GPL-2.0+
> Valid-License-Identifier: GPL-2.0-or-later
> 
> in the LICENSES/GPL-2.0 file like the kernel does?

The kernel is not going to change that because we have started with this
before the s/+/-or-later/ happened. Tools need to read both.
 
> Is that even going to stay that way?  I thought I heard that Greg was
> working on phasing out the "2.0+" tags since SPDX deprecated that?

For new stuff we should use -or-later methinks.

> I think xfsprogs and xfstests just follow whatever the kernel does, but
> AFAICT this whole initiative /continues/ to communicate poorly with the
> maintainers about (1) how this is supposed to benefit us and (2) what we
> are supposed to do to maintain all of it.

We wrote lengthy documentation and a long explanation on LKML. So what's
missing?

Maintaining it is easy. All new files need a valid SPDX identifier which is
documented in one of the licenses in the LICENSES directory. Preferrably
the SPDX identifier comes without all that boiler plate nonsense which got
copied and pasted, fatfingered and broken in the past.
 
> Do we have to get lawyers involved to roll to a new SPDX version?  Will
> LF do that for (at least) the projects hosted on kernel.org?  Should we
> just do it and hope for the best since IANAFL?  I know how to review
> code.  I don't know how to review licensing and all the tiny
> implications that go along with things like this.  I don't even feel
> confident that the two identifiers above are exactly the same, because
> all I know is that I read it on a webpage somewhere.

We have layers helping with that. 
 
> I for one still have heard abso-f*cking-lutely nothing about what is
> this SPDX change other than Greg shoving treewide changes in the kernel.
> That sufficed to get the mechanical work done (at the cost of a lot of
> frustration for Greg) but this doesn't help me sustain our community.
> 
> Guidance needed.  Apologies all around if this rant is misdirected, but
> I have no idea who (if anyone) is maintaining SPDX tags.  There's no
> entry for LICENSES/ in MAINTAINERS, which is where I looked first.

The SPDX identifiers are maintained by the SPDX group which is independent
of us. We use them and we document how to use them proper so tooling can do
proper license identification.

Yeah, we should add a MAINTAINERS entry for LICENSES. Greg and myself are
going to be volunteered I fear.

Thanks,

	tglx

Re: [PATCH v2] generic: test for failure to unlock inode after chgrp fails with EDQUOT

[Greg KH]

On Tue, Aug 27, 2019 at 05:19:27PM +0200, Thomas Gleixner wrote:
> Darrick,
> 
> On Tue, 27 Aug 2019, Darrick J. Wong wrote:
> 
> > On Tue, Aug 27, 2019 at 08:13:19AM +0200, Thomas Gleixner wrote:
> > > On Mon, 26 Aug 2019, Darrick J. Wong wrote:
> > > > +++ b/tests/generic/719
> > > > @@ -0,0 +1,59 @@
> > > > +#! /bin/bash
> > > > +# SPDX-License-Identifier: GPL-2.0-or-newer
> > > 
> > > Please run scripts/spdxcheck.py on that file and consult the licensing
> > > documentation.
> > 
> > -or-later, sorry.
> > 
> > So .... now that everyone who wanted these SPDX identifiers have spread
> > "GPL-2.0+" around the kernel and related projects (xfsprogs, xfstests)
> > just in time for SPDX 3.0 to deprecate the "+" syntax, what are we
> > supposed to do?  Another treewide change to fiddle with SPDX syntax?
> > Can we just put:
> > 
> > Valid-License-Identifier: GPL-2.0+
> > Valid-License-Identifier: GPL-2.0-or-later
> > 
> > in the LICENSES/GPL-2.0 file like the kernel does?
> 
> The kernel is not going to change that because we have started with this
> before the s/+/-or-later/ happened. Tools need to read both.
>  
> > Is that even going to stay that way?  I thought I heard that Greg was
> > working on phasing out the "2.0+" tags since SPDX deprecated that?
> 
> For new stuff we should use -or-later methinks.

For new stuff, if you wish to be "kind" to some community members, we
should use "-or-later" and "-only".  But as you say, both are fine.

And no, I am NOT working on phasing out any SPDX tags for the older
stuff.  Personally, I like the older ones.

> Yeah, we should add a MAINTAINERS entry for LICENSES. Greg and myself are
> going to be volunteered I fear.

Yeah, I figured it was only a matter of time.  Let me go create an entry
given that we already have git tree for it in linux-next for a while
now...

thanks,

greg k-h

Re: [PATCH v2] generic: test for failure to unlock inode after chgrp fails with EDQUOT

[Greg KH]

On Tue, Aug 27, 2019 at 05:26:48PM +0200, Greg KH wrote:
> On Tue, Aug 27, 2019 at 05:19:27PM +0200, Thomas Gleixner wrote:
> > Darrick,
> > 
> > On Tue, 27 Aug 2019, Darrick J. Wong wrote:
> > 
> > > On Tue, Aug 27, 2019 at 08:13:19AM +0200, Thomas Gleixner wrote:
> > > > On Mon, 26 Aug 2019, Darrick J. Wong wrote:
> > > > > +++ b/tests/generic/719
> > > > > @@ -0,0 +1,59 @@
> > > > > +#! /bin/bash
> > > > > +# SPDX-License-Identifier: GPL-2.0-or-newer
> > > > 
> > > > Please run scripts/spdxcheck.py on that file and consult the licensing
> > > > documentation.
> > > 
> > > -or-later, sorry.
> > > 
> > > So .... now that everyone who wanted these SPDX identifiers have spread
> > > "GPL-2.0+" around the kernel and related projects (xfsprogs, xfstests)
> > > just in time for SPDX 3.0 to deprecate the "+" syntax, what are we
> > > supposed to do?  Another treewide change to fiddle with SPDX syntax?
> > > Can we just put:
> > > 
> > > Valid-License-Identifier: GPL-2.0+
> > > Valid-License-Identifier: GPL-2.0-or-later
> > > 
> > > in the LICENSES/GPL-2.0 file like the kernel does?
> > 
> > The kernel is not going to change that because we have started with this
> > before the s/+/-or-later/ happened. Tools need to read both.
> >  
> > > Is that even going to stay that way?  I thought I heard that Greg was
> > > working on phasing out the "2.0+" tags since SPDX deprecated that?
> > 
> > For new stuff we should use -or-later methinks.
> 
> For new stuff, if you wish to be "kind" to some community members, we
> should use "-or-later" and "-only".  But as you say, both are fine.
> 
> And no, I am NOT working on phasing out any SPDX tags for the older
> stuff.  Personally, I like the older ones.
> 
> > Yeah, we should add a MAINTAINERS entry for LICENSES. Greg and myself are
> > going to be volunteered I fear.
> 
> Yeah, I figured it was only a matter of time.  Let me go create an entry
> given that we already have git tree for it in linux-next for a while
> now...

Now submitted:
	https://lore.kernel.org/lkml/20190827172519.GA28849@kroah.com/T/#u

Re: [PATCH v2] generic: test for failure to unlock inode after chgrp fails with EDQUOT

[Greg KH]

On Tue, Aug 27, 2019 at 08:04:51AM -0700, Darrick J. Wong wrote:
> On Tue, Aug 27, 2019 at 08:13:19AM +0200, Thomas Gleixner wrote:
> > On Mon, 26 Aug 2019, Darrick J. Wong wrote:
> > > +++ b/tests/generic/719
> > > @@ -0,0 +1,59 @@
> > > +#! /bin/bash
> > > +# SPDX-License-Identifier: GPL-2.0-or-newer
> > 
> > Please run scripts/spdxcheck.py on that file and consult the licensing
> > documentation.
> 
> -or-later, sorry.
> 
> So .... now that everyone who wanted these SPDX identifiers have spread
> "GPL-2.0+" around the kernel and related projects (xfsprogs, xfstests)
> just in time for SPDX 3.0 to deprecate the "+" syntax, what are we
> supposed to do?  Another treewide change to fiddle with SPDX syntax?
> Can we just put:
> 
> Valid-License-Identifier: GPL-2.0+
> Valid-License-Identifier: GPL-2.0-or-later
> 
> in the LICENSES/GPL-2.0 file like the kernel does?
> 
> Is that even going to stay that way?  I thought I heard that Greg was
> working on phasing out the "2.0+" tags since SPDX deprecated that?
> 
> I think xfsprogs and xfstests just follow whatever the kernel does, but
> AFAICT this whole initiative /continues/ to communicate poorly with the
> maintainers about (1) how this is supposed to benefit us and (2) what we
> are supposed to do to maintain all of it.
> 
> Do we have to get lawyers involved to roll to a new SPDX version?  Will
> LF do that for (at least) the projects hosted on kernel.org?  Should we
> just do it and hope for the best since IANAFL?  I know how to review
> code.  I don't know how to review licensing and all the tiny
> implications that go along with things like this.  I don't even feel
> confident that the two identifiers above are exactly the same, because
> all I know is that I read it on a webpage somewhere.
> 
> I for one still have heard abso-f*cking-lutely nothing about what is
> this SPDX change other than Greg shoving treewide changes in the kernel.
> That sufficed to get the mechanical work done (at the cost of a lot of
> frustration for Greg) but this doesn't help me sustain our community.

It takes a lot more to get me frustrated :)

And I am _VERY_ supprised to hear you have not heard anything about this
given that Oracle has some lawyers who have been very involved in the
SPDX process.  One would have thought they had discussed it with their
developers.  They sure seem to come to me with questions that start,
"Our developers had a question about...", so I know they must talk to
someone...

> Guidance needed.  Apologies all around if this rant is misdirected, but
> I have no idea who (if anyone) is maintaining SPDX tags.  There's no
> entry for LICENSES/ in MAINTAINERS, which is where I looked first.

Thomas seems to have answered all of your questions, hopefully.

And yeah, Thomas and I should probably be listed in MAINTAINERS for
LICENSES as we somehow got tagged for all of this work.

thanks,

greg k-h