diff mbox series

quota: allow users to truncate group and project quota files

Message ID 20210728200208.GH3601443@magnolia (mailing list archive)
State New, archived
Headers show
Series quota: allow users to truncate group and project quota files | expand

Commit Message

Darrick J. Wong July 28, 2021, 8:02 p.m. UTC 
From: Darrick J. Wong <djwong@kernel.org>

In commit 79ac1ae4, I /think/ xfsprogs gained the ability to deal with
project or group quotas.  For some reason, the quota remove command was
structured so that if the user passes both -g and -p, it will only ask
the kernel truncate the group quota file.  This is a strange behavior
since -ug results in truncation requests for both user and group quota
files, and the kernel is smart enough to return 0 if asked to truncate a
quota file that doesn't exist.

In other words, this is a seemingly arbitrary limitation of the command.
It's an unexpected behavior since we don't do any sort of parameter
validation to warn users when -p is silently ignored.  Modern V5
filesystems support both group and project quotas, so it's all the more
surprising that you can't do group and project all at once.  Remove this
pointless restriction.

Found while triaging xfs/007 regressions.

Fixes: 79ac1ae4 ("Fix xfs_quota disable, enable, off and remove commands Merge of master-melb:xfs-cmds:29395a by kenmcd.")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 quota/state.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Darrick J. Wong July 28, 2021, 9:15 p.m. UTC | #1 
Eric asked me to resend all the pending patches for 5.13, so I might as
well NAK this and tell everyone to watch for the imminent patchbomb.

--D

On Wed, Jul 28, 2021 at 01:02:08PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> In commit 79ac1ae4, I /think/ xfsprogs gained the ability to deal with
> project or group quotas.  For some reason, the quota remove command was
> structured so that if the user passes both -g and -p, it will only ask
> the kernel truncate the group quota file.  This is a strange behavior
> since -ug results in truncation requests for both user and group quota
> files, and the kernel is smart enough to return 0 if asked to truncate a
> quota file that doesn't exist.
> 
> In other words, this is a seemingly arbitrary limitation of the command.
> It's an unexpected behavior since we don't do any sort of parameter
> validation to warn users when -p is silently ignored.  Modern V5
> filesystems support both group and project quotas, so it's all the more
> surprising that you can't do group and project all at once.  Remove this
> pointless restriction.
> 
> Found while triaging xfs/007 regressions.
> 
> Fixes: 79ac1ae4 ("Fix xfs_quota disable, enable, off and remove commands Merge of master-melb:xfs-cmds:29395a by kenmcd.")
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
>  quota/state.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/quota/state.c b/quota/state.c
> index 3ffb2c88..43fb700f 100644
> --- a/quota/state.c
> +++ b/quota/state.c
> @@ -463,7 +463,8 @@ remove_extents(
>  	if (type & XFS_GROUP_QUOTA) {
>  		if (remove_qtype_extents(dir, XFS_GROUP_QUOTA) < 0)
>  			return;
> -	} else if (type & XFS_PROJ_QUOTA) {
> +	}
> +	if (type & XFS_PROJ_QUOTA) {
>  		if (remove_qtype_extents(dir, XFS_PROJ_QUOTA) < 0)
>  			return;
>  	}
Carlos Maiolino July 29, 2021, 11:21 a.m. UTC | #2 
On Wed, Jul 28, 2021 at 01:02:08PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> In commit 79ac1ae4, I /think/ xfsprogs gained the ability to deal with
> project or group quotas.  For some reason, the quota remove command was
> structured so that if the user passes both -g and -p, it will only ask
> the kernel truncate the group quota file.  This is a strange behavior
> since -ug results in truncation requests for both user and group quota
> files, and the kernel is smart enough to return 0 if asked to truncate a
> quota file that doesn't exist.
> 
> In other words, this is a seemingly arbitrary limitation of the command.
> It's an unexpected behavior since we don't do any sort of parameter
> validation to warn users when -p is silently ignored.  Modern V5
> filesystems support both group and project quotas, so it's all the more
> surprising that you can't do group and project all at once.  Remove this
> pointless restriction.
> 
> Found while triaging xfs/007 regressions.
> 
> Fixes: 79ac1ae4 ("Fix xfs_quota disable, enable, off and remove commands Merge of master-melb:xfs-cmds:29395a by kenmcd.")
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>

Looks good.
Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>

> ---
>  quota/state.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/quota/state.c b/quota/state.c
> index 3ffb2c88..43fb700f 100644
> --- a/quota/state.c
> +++ b/quota/state.c
> @@ -463,7 +463,8 @@ remove_extents(
>  	if (type & XFS_GROUP_QUOTA) {
>  		if (remove_qtype_extents(dir, XFS_GROUP_QUOTA) < 0)
>  			return;
> -	} else if (type & XFS_PROJ_QUOTA) {
> +	}
> +	if (type & XFS_PROJ_QUOTA) {
>  		if (remove_qtype_extents(dir, XFS_PROJ_QUOTA) < 0)
>  			return;
>  	}
>
Carlos Maiolino July 29, 2021, 11:23 a.m. UTC | #3 
On Wed, Jul 28, 2021 at 02:15:35PM -0700, Darrick J. Wong wrote:
> Eric asked me to resend all the pending patches for 5.13, so I might as
> well NAK this and tell everyone to watch for the imminent patchbomb.

oh, I'll read the whole thread before reviewing next time, but well, feel free
to carry my reviewed-by.

Cheers

> 
> --D
> 
> On Wed, Jul 28, 2021 at 01:02:08PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > In commit 79ac1ae4, I /think/ xfsprogs gained the ability to deal with
> > project or group quotas.  For some reason, the quota remove command was
> > structured so that if the user passes both -g and -p, it will only ask
> > the kernel truncate the group quota file.  This is a strange behavior
> > since -ug results in truncation requests for both user and group quota
> > files, and the kernel is smart enough to return 0 if asked to truncate a
> > quota file that doesn't exist.
> > 
> > In other words, this is a seemingly arbitrary limitation of the command.
> > It's an unexpected behavior since we don't do any sort of parameter
> > validation to warn users when -p is silently ignored.  Modern V5
> > filesystems support both group and project quotas, so it's all the more
> > surprising that you can't do group and project all at once.  Remove this
> > pointless restriction.
> > 
> > Found while triaging xfs/007 regressions.
> > 
> > Fixes: 79ac1ae4 ("Fix xfs_quota disable, enable, off and remove commands Merge of master-melb:xfs-cmds:29395a by kenmcd.")
> > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > ---
> >  quota/state.c |    3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/quota/state.c b/quota/state.c
> > index 3ffb2c88..43fb700f 100644
> > --- a/quota/state.c
> > +++ b/quota/state.c
> > @@ -463,7 +463,8 @@ remove_extents(
> >  	if (type & XFS_GROUP_QUOTA) {
> >  		if (remove_qtype_extents(dir, XFS_GROUP_QUOTA) < 0)
> >  			return;
> > -	} else if (type & XFS_PROJ_QUOTA) {
> > +	}
> > +	if (type & XFS_PROJ_QUOTA) {
> >  		if (remove_qtype_extents(dir, XFS_PROJ_QUOTA) < 0)
> >  			return;
> >  	}
>
diff mbox series

Patch

diff --git a/quota/state.c b/quota/state.c
index 3ffb2c88..43fb700f 100644
--- a/quota/state.c
+++ b/quota/state.c
@@ -463,7 +463,8 @@  remove_extents(
 	if (type & XFS_GROUP_QUOTA) {
 		if (remove_qtype_extents(dir, XFS_GROUP_QUOTA) < 0)
 			return;
-	} else if (type & XFS_PROJ_QUOTA) {
+	}
+	if (type & XFS_PROJ_QUOTA) {
 		if (remove_qtype_extents(dir, XFS_PROJ_QUOTA) < 0)
 			return;
 	}