[PATCH 0/3] xfsprogs: xfs_quota error message and state reporting improvement

[PATCH 0/3] xfsprogs: xfs_quota error message and state reporting improvement

[Bill O'Donnell]

This patchset improves xfs_quota command error message output as well as
adding reporting for grace times and warning limits for state (u,g,p).

Note that patches to xfstests to handle these changes will be forthcoming.
Also, patch 1 was originally submitted separately:
  (xfsprogs: xfs_quota command error message improvement)

patch 1 contains the command error message improvements.
patch 2 contains the warning limit reporting (originally SoB Darrick Wong).
patch 3 contains the additional state reporting of grace times for u,g,p.

Comments appreciated. Thanks-
Bill

[PATCH 1/3] xfsprogs: xfs_quota command error message improvement

[Bill O'Donnell]

Make the error messages for rudimentary xfs_quota commands
(off, enable, disable) more user friendly, instead of the
terse sys error outputs.

Signed-off-by: Bill O'Donnell <billodo@redhat.com>
---
 quota/state.c | 32 ++++++++++++++++++++++++++------
 1 file changed, 26 insertions(+), 6 deletions(-)

diff --git a/quota/state.c b/quota/state.c
index 8f9718f1..7a595fc6 100644
--- a/quota/state.c
+++ b/quota/state.c
@@ -306,8 +306,16 @@ enable_enforcement(
 		return;
 	}
 	dir = mount->fs_name;
-	if (xfsquotactl(XFS_QUOTAON, dir, type, 0, (void *)&qflags) < 0)
-		perror("XFS_QUOTAON");
+	if (xfsquotactl(XFS_QUOTAON, dir, type, 0, (void *)&qflags) < 0) {
+		if (errno == EEXIST)
+			fprintf(stderr,
+				_("Quota enforcement already enabled.\n"));
+		else if (errno == EINVAL || errno == ENOSYS)
+			fprintf(stderr,
+				_("Can't enable enforcement when quota off.\n"));
+		else
+			perror("XFS_QUOTAON");
+	}
 	else if (flags & VERBOSE_FLAG)
 		state_quotafile_mount(stdout, type, mount, flags);
 }
@@ -328,8 +336,16 @@ disable_enforcement(
 		return;
 	}
 	dir = mount->fs_name;
-	if (xfsquotactl(XFS_QUOTAOFF, dir, type, 0, (void *)&qflags) < 0)
-		perror("XFS_QUOTAOFF");
+	if (xfsquotactl(XFS_QUOTAOFF, dir, type, 0, (void *)&qflags) < 0) {
+		if (errno == EEXIST)
+			fprintf(stderr,
+				_("Quota enforcement already disabled.\n"));
+		else if (errno == EINVAL || errno == ENOSYS)
+			fprintf(stderr,
+				_("Can't disable enforcement when quota off.\n"));
+		else
+			perror("XFS_QUOTAOFF");
+	}
 	else if (flags & VERBOSE_FLAG)
 		state_quotafile_mount(stdout, type, mount, flags);
 }
@@ -350,8 +366,12 @@ quotaoff(
 		return;
 	}
 	dir = mount->fs_name;
-	if (xfsquotactl(XFS_QUOTAOFF, dir, type, 0, (void *)&qflags) < 0)
-		perror("XFS_QUOTAOFF");
+	if (xfsquotactl(XFS_QUOTAOFF, dir, type, 0, (void *)&qflags) < 0) {
+		if (errno == EEXIST || errno == ENOSYS)
+			fprintf(stderr, _("Quota already off.\n"));
+		else
+			perror("XFS_QUOTAOFF");
+	}
 	else if (flags & VERBOSE_FLAG)
 		state_quotafile_mount(stdout, type, mount, flags);
 }
-- 
2.26.2

Re: [PATCH 1/3] xfsprogs: xfs_quota command error message improvement

[Darrick J. Wong]

On Wed, Jul 15, 2020 at 03:12:51PM -0500, Bill O'Donnell wrote:
> Make the error messages for rudimentary xfs_quota commands
> (off, enable, disable) more user friendly, instead of the
> terse sys error outputs.
> 
> Signed-off-by: Bill O'Donnell <billodo@redhat.com>

Hmmm... where are the underlying ioctls documented, anyways?
Can we please get that done?

(Note that ENOSYS can also mean that the fs didn't register any quota
operations, but xfs always does so meh.)

Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> ---
>  quota/state.c | 32 ++++++++++++++++++++++++++------
>  1 file changed, 26 insertions(+), 6 deletions(-)
> 
> diff --git a/quota/state.c b/quota/state.c
> index 8f9718f1..7a595fc6 100644
> --- a/quota/state.c
> +++ b/quota/state.c
> @@ -306,8 +306,16 @@ enable_enforcement(
>  		return;
>  	}
>  	dir = mount->fs_name;
> -	if (xfsquotactl(XFS_QUOTAON, dir, type, 0, (void *)&qflags) < 0)
> -		perror("XFS_QUOTAON");
> +	if (xfsquotactl(XFS_QUOTAON, dir, type, 0, (void *)&qflags) < 0) {
> +		if (errno == EEXIST)
> +			fprintf(stderr,
> +				_("Quota enforcement already enabled.\n"));
> +		else if (errno == EINVAL || errno == ENOSYS)
> +			fprintf(stderr,
> +				_("Can't enable enforcement when quota off.\n"));
> +		else
> +			perror("XFS_QUOTAON");
> +	}
>  	else if (flags & VERBOSE_FLAG)
>  		state_quotafile_mount(stdout, type, mount, flags);
>  }
> @@ -328,8 +336,16 @@ disable_enforcement(
>  		return;
>  	}
>  	dir = mount->fs_name;
> -	if (xfsquotactl(XFS_QUOTAOFF, dir, type, 0, (void *)&qflags) < 0)
> -		perror("XFS_QUOTAOFF");
> +	if (xfsquotactl(XFS_QUOTAOFF, dir, type, 0, (void *)&qflags) < 0) {
> +		if (errno == EEXIST)
> +			fprintf(stderr,
> +				_("Quota enforcement already disabled.\n"));
> +		else if (errno == EINVAL || errno == ENOSYS)
> +			fprintf(stderr,
> +				_("Can't disable enforcement when quota off.\n"));
> +		else
> +			perror("XFS_QUOTAOFF");
> +	}
>  	else if (flags & VERBOSE_FLAG)
>  		state_quotafile_mount(stdout, type, mount, flags);
>  }
> @@ -350,8 +366,12 @@ quotaoff(
>  		return;
>  	}
>  	dir = mount->fs_name;
> -	if (xfsquotactl(XFS_QUOTAOFF, dir, type, 0, (void *)&qflags) < 0)
> -		perror("XFS_QUOTAOFF");
> +	if (xfsquotactl(XFS_QUOTAOFF, dir, type, 0, (void *)&qflags) < 0) {
> +		if (errno == EEXIST || errno == ENOSYS)
> +			fprintf(stderr, _("Quota already off.\n"));
> +		else
> +			perror("XFS_QUOTAOFF");
> +	}
>  	else if (flags & VERBOSE_FLAG)
>  		state_quotafile_mount(stdout, type, mount, flags);
>  }
> -- 
> 2.26.2
> 

Re: [PATCH 1/3] xfsprogs: xfs_quota command error message improvement

[Christoph Hellwig]

On Wed, Jul 15, 2020 at 03:12:51PM -0500, Bill O'Donnell wrote:
> Make the error messages for rudimentary xfs_quota commands
> (off, enable, disable) more user friendly, instead of the
> terse sys error outputs.
> 
> Signed-off-by: Bill O'Donnell <billodo@redhat.com>

I think we should have one helper with the error message
instead of duplicating them three times.

Re: [PATCH 1/3] xfsprogs: xfs_quota command error message improvement

[Bill O'Donnell]

On Tue, Jul 21, 2020 at 04:04:04PM +0100, Christoph Hellwig wrote:
> On Wed, Jul 15, 2020 at 03:12:51PM -0500, Bill O'Donnell wrote:
> > Make the error messages for rudimentary xfs_quota commands
> > (off, enable, disable) more user friendly, instead of the
> > terse sys error outputs.
> > 
> > Signed-off-by: Bill O'Donnell <billodo@redhat.com>
> 
> I think we should have one helper with the error message
> instead of duplicating them three times.
> 
Except that the error messages are different depending on the context,
so crafting a helper function that recognizes the context seems to
offer diminishing return AFAICT.

Thanks-
Bill

[PATCH 2/3] xfs_quota: display warning limits when printing quota type information

[Bill O'Donnell]

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

We should dump the default warning limits when we're printing quota
information.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 quota/state.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/quota/state.c b/quota/state.c
index 7a595fc6..1627181d 100644
--- a/quota/state.c
+++ b/quota/state.c
@@ -130,6 +130,16 @@ state_timelimit(
 		time_to_string(timelimit, VERBOSE_FLAG | ABSOLUTE_FLAG));
 }
 
+static void
+state_warnlimit(
+	FILE		*fp,
+	uint		form,
+	uint16_t	warnlimit)
+{
+	fprintf(fp, _("%s max warnings: %u\n"),
+		form_to_string(form), warnlimit);
+}
+
 /*
  * fs_quota_stat holds a subset of fs_quota_statv; this copies
  * the smaller into the larger, leaving any not-present fields
@@ -218,7 +228,11 @@ state_quotafile_mount(
 				sv.qs_flags & XFS_QUOTA_PDQ_ENFD);
 
 	state_timelimit(fp, XFS_BLOCK_QUOTA, sv.qs_btimelimit);
+	state_warnlimit(fp, XFS_BLOCK_QUOTA, sv.qs_bwarnlimit);
+
 	state_timelimit(fp, XFS_INODE_QUOTA, sv.qs_itimelimit);
+	state_warnlimit(fp, XFS_INODE_QUOTA, sv.qs_iwarnlimit);
+
 	state_timelimit(fp, XFS_RTBLOCK_QUOTA, sv.qs_rtbtimelimit);
 }
 
-- 
2.26.2

[PATCH 3/3] xfsprogs: xfs_quota state command should report ugp grace times

[Bill O'Donnell]

Since grace periods are now supported for three quota types (ugp),
modify xfs_quota state command to report times for all three.
Add a helper function for stat reporting.

Signed-off-by: Bill O'Donnell <billodo@redhat.com>
---
 quota/state.c | 108 +++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 80 insertions(+), 28 deletions(-)

diff --git a/quota/state.c b/quota/state.c
index 1627181d..5aadcca2 100644
--- a/quota/state.c
+++ b/quota/state.c
@@ -190,6 +190,39 @@ state_stat_to_statv(
 	}
 }
 
+static void
+state_quotafile_stat(
+	FILE			*fp,
+	uint			type,
+	struct fs_path		*mount,
+	struct fs_quota_statv	*sv)
+{
+	switch(type) {
+	case XFS_USER_QUOTA:
+		state_qfilestat(fp, mount, XFS_USER_QUOTA, &sv->qs_uquota,
+				sv->qs_flags & XFS_QUOTA_UDQ_ACCT,
+				sv->qs_flags & XFS_QUOTA_UDQ_ENFD);
+		break;
+	case XFS_GROUP_QUOTA:
+		state_qfilestat(fp, mount, XFS_GROUP_QUOTA, &sv->qs_gquota,
+				sv->qs_flags & XFS_QUOTA_GDQ_ACCT,
+				sv->qs_flags & XFS_QUOTA_GDQ_ENFD);
+		break;
+	case XFS_PROJ_QUOTA:
+		state_qfilestat(fp, mount, XFS_PROJ_QUOTA, &sv->qs_pquota,
+				sv->qs_flags & XFS_QUOTA_PDQ_ACCT,
+				sv->qs_flags & XFS_QUOTA_PDQ_ENFD);
+		break;
+	}
+	state_timelimit(fp, XFS_BLOCK_QUOTA, sv->qs_btimelimit);
+	state_warnlimit(fp, XFS_BLOCK_QUOTA, sv->qs_bwarnlimit);
+
+	state_timelimit(fp, XFS_INODE_QUOTA, sv->qs_itimelimit);
+	state_warnlimit(fp, XFS_INODE_QUOTA, sv->qs_iwarnlimit);
+
+	state_timelimit(fp, XFS_RTBLOCK_QUOTA, sv->qs_rtbtimelimit);
+}
+
 static void
 state_quotafile_mount(
 	FILE			*fp,
@@ -203,37 +236,56 @@ state_quotafile_mount(
 
 	sv.qs_version = FS_QSTATV_VERSION1;
 
-	if (xfsquotactl(XFS_GETQSTATV, dev, type, 0, (void *)&sv) < 0) {
-		if (xfsquotactl(XFS_GETQSTAT, dev, type, 0, (void *)&s) < 0) {
-			if (flags & VERBOSE_FLAG)
-				fprintf(fp,
-					_("%s quota are not enabled on %s\n"),
-					type_to_string(type), dev);
-			return;
+	if (type & XFS_USER_QUOTA) {
+		if (xfsquotactl(XFS_GETQSTATV, dev, XFS_USER_QUOTA,
+				0, (void *)&sv) < 0) {
+			if (xfsquotactl(XFS_GETQSTAT, dev, XFS_USER_QUOTA,
+					0, (void *)&s) < 0) {
+				if (flags & VERBOSE_FLAG)
+					fprintf(fp,
+						_("%s quota are not enabled on %s\n"),
+						type_to_string(XFS_USER_QUOTA),
+						dev);
+				return;
+			}
+			state_stat_to_statv(&s, &sv);
 		}
-		state_stat_to_statv(&s, &sv);
+		state_quotafile_stat(fp, XFS_USER_QUOTA, mount, &sv);
 	}
 
-	if (type & XFS_USER_QUOTA)
-		state_qfilestat(fp, mount, XFS_USER_QUOTA, &sv.qs_uquota,
-				sv.qs_flags & XFS_QUOTA_UDQ_ACCT,
-				sv.qs_flags & XFS_QUOTA_UDQ_ENFD);
-	if (type & XFS_GROUP_QUOTA)
-		state_qfilestat(fp, mount, XFS_GROUP_QUOTA, &sv.qs_gquota,
-				sv.qs_flags & XFS_QUOTA_GDQ_ACCT,
-				sv.qs_flags & XFS_QUOTA_GDQ_ENFD);
-	if (type & XFS_PROJ_QUOTA)
-		state_qfilestat(fp, mount, XFS_PROJ_QUOTA, &sv.qs_pquota,
-				sv.qs_flags & XFS_QUOTA_PDQ_ACCT,
-				sv.qs_flags & XFS_QUOTA_PDQ_ENFD);
-
-	state_timelimit(fp, XFS_BLOCK_QUOTA, sv.qs_btimelimit);
-	state_warnlimit(fp, XFS_BLOCK_QUOTA, sv.qs_bwarnlimit);
-
-	state_timelimit(fp, XFS_INODE_QUOTA, sv.qs_itimelimit);
-	state_warnlimit(fp, XFS_INODE_QUOTA, sv.qs_iwarnlimit);
-
-	state_timelimit(fp, XFS_RTBLOCK_QUOTA, sv.qs_rtbtimelimit);
+	if (type & XFS_GROUP_QUOTA) {
+		if (xfsquotactl(XFS_GETQSTATV, dev, XFS_GROUP_QUOTA,
+				0, (void *)&sv) < 0) {
+			if (xfsquotactl(XFS_GETQSTAT, dev, XFS_GROUP_QUOTA,
+					0, (void *)&s) < 0) {
+				if (flags & VERBOSE_FLAG)
+					fprintf(fp,
+						_("%s quota are not enabled on %s\n"),
+						type_to_string(XFS_GROUP_QUOTA),
+						dev);
+				return;
+			}
+			state_stat_to_statv(&s, &sv);
+		}
+		state_quotafile_stat(fp, XFS_GROUP_QUOTA, mount, &sv);
+	}
+
+	if (type & XFS_PROJ_QUOTA) {
+		if (xfsquotactl(XFS_GETQSTATV, dev, XFS_PROJ_QUOTA,
+				0, (void *)&sv) < 0) {
+			if (xfsquotactl(XFS_GETQSTAT, dev, XFS_PROJ_QUOTA,
+					0, (void *)&s) < 0) {
+				if (flags & VERBOSE_FLAG)
+					fprintf(fp,
+						_("%s quota are not enabled on %s\n"),
+						type_to_string(XFS_PROJ_QUOTA),
+						dev);
+				return;
+			}
+			state_stat_to_statv(&s, &sv);
+		}
+		state_quotafile_stat(fp, XFS_PROJ_QUOTA, mount, &sv);
+	}
 }
 
 static void
-- 
2.26.2

Re: [PATCH 3/3] xfsprogs: xfs_quota state command should report ugp grace times

[Eric Sandeen]

On 7/15/20 1:12 PM, Bill O'Donnell wrote:
> Since grace periods are now supported for three quota types (ugp),
> modify xfs_quota state command to report times for all three.
> Add a helper function for stat reporting.
> 
> Signed-off-by: Bill O'Donnell <billodo@redhat.com>

...

> +	if (type & XFS_GROUP_QUOTA) {
> +		if (xfsquotactl(XFS_GETQSTATV, dev, XFS_GROUP_QUOTA,
> +				0, (void *)&sv) < 0) {
> +			if (xfsquotactl(XFS_GETQSTAT, dev, XFS_GROUP_QUOTA,
> +					0, (void *)&s) < 0) {
> +				if (flags & VERBOSE_FLAG)
> +					fprintf(fp,
> +						_("%s quota are not enabled on %s\n"),
> +						type_to_string(XFS_GROUP_QUOTA),
> +						dev);
> +				return;
> +			}
> +			state_stat_to_statv(&s, &sv);
> +		}

At first glance, can't all of the above be moved into the helper as well?

Maybe something like this (needs fixing up for sure)

static void
state_quotafile_stat(
	FILE			*fp,
	uint			type,
	struct fs_path		*mount,
	struct fs_quota_statv	*sv)
{
	bool 			accounting, enforcing;
	struct fs_qfilestat	*qsv;

	switch(type) {
	case XFS_USER_QUOTA:
		qsv = &sv->qs_uquota;
		accounting = sv->qs_flags & XFS_QUOTA_UDQ_ACCT;
		enforcing = sv->qs_flags & XFS_QUOTA_UDQ_ENFD;		
		break;
	case XFS_GROUP_QUOTA:
		...
		break;
...
	default:
		/* defensive check goes here */
	}

	if (xfsquotactl(XFS_GETQSTATV, dev, type, 0, (void *)&sv) < 0) {
		if (xfsquotactl(XFS_GETQSTAT, dev, type, 0, (void *)&s) < 0) {
			if (flags & VERBOSE_FLAG)
				fprintf(fp,
					_("%s quota are not enabled on %s\n"),
					type_to_string(type), dev);
			return;
		}
		state_stat_to_statv(&s, &sv)
	}	

	state_qfilestat(fp, mount, type, qsv, accounting, enforcing);

	state_timelimit(fp, XFS_BLOCK_QUOTA, sv->qs_btimelimit);
	state_warnlimit(fp, XFS_BLOCK_QUOTA, sv->qs_bwarnlimit);

	state_timelimit(fp, XFS_INODE_QUOTA, sv->qs_itimelimit);
	state_warnlimit(fp, XFS_INODE_QUOTA, sv->qs_iwarnlimit);

	state_timelimit(fp, XFS_RTBLOCK_QUOTA, sv->qs_rtbtimelimit);
}


Thanks,
-Eric

Re: [PATCH 3/3] xfsprogs: xfs_quota state command should report ugp grace times

[Bill O'Donnell]

On Wed, Jul 15, 2020 at 01:33:29PM -0700, Eric Sandeen wrote:
> On 7/15/20 1:12 PM, Bill O'Donnell wrote:
> > Since grace periods are now supported for three quota types (ugp),
> > modify xfs_quota state command to report times for all three.
> > Add a helper function for stat reporting.
> > 
> > Signed-off-by: Bill O'Donnell <billodo@redhat.com>
> 
> ...
> 
> > +	if (type & XFS_GROUP_QUOTA) {
> > +		if (xfsquotactl(XFS_GETQSTATV, dev, XFS_GROUP_QUOTA,
> > +				0, (void *)&sv) < 0) {
> > +			if (xfsquotactl(XFS_GETQSTAT, dev, XFS_GROUP_QUOTA,
> > +					0, (void *)&s) < 0) {
> > +				if (flags & VERBOSE_FLAG)
> > +					fprintf(fp,
> > +						_("%s quota are not enabled on %s\n"),
> > +						type_to_string(XFS_GROUP_QUOTA),
> > +						dev);
> > +				return;
> > +			}
> > +			state_stat_to_statv(&s, &sv);
> > +		}
> 
> At first glance, can't all of the above be moved into the helper as well?
> 
> Maybe something like this (needs fixing up for sure)

Yeah, I should further reduce the redundancy.
Thanks-
Bill


> 
> static void
> state_quotafile_stat(
> 	FILE			*fp,
> 	uint			type,
> 	struct fs_path		*mount,
> 	struct fs_quota_statv	*sv)
> {
> 	bool 			accounting, enforcing;
> 	struct fs_qfilestat	*qsv;
> 
> 	switch(type) {
> 	case XFS_USER_QUOTA:
> 		qsv = &sv->qs_uquota;
> 		accounting = sv->qs_flags & XFS_QUOTA_UDQ_ACCT;
> 		enforcing = sv->qs_flags & XFS_QUOTA_UDQ_ENFD;		
> 		break;
> 	case XFS_GROUP_QUOTA:
> 		...
> 		break;
> ...
> 	default:
> 		/* defensive check goes here */
> 	}
> 
> 	if (xfsquotactl(XFS_GETQSTATV, dev, type, 0, (void *)&sv) < 0) {
> 		if (xfsquotactl(XFS_GETQSTAT, dev, type, 0, (void *)&s) < 0) {
> 			if (flags & VERBOSE_FLAG)
> 				fprintf(fp,
> 					_("%s quota are not enabled on %s\n"),
> 					type_to_string(type), dev);
> 			return;
> 		}
> 		state_stat_to_statv(&s, &sv)
> 	}	
> 
> 	state_qfilestat(fp, mount, type, qsv, accounting, enforcing);
> 
> 	state_timelimit(fp, XFS_BLOCK_QUOTA, sv->qs_btimelimit);
> 	state_warnlimit(fp, XFS_BLOCK_QUOTA, sv->qs_bwarnlimit);
> 
> 	state_timelimit(fp, XFS_INODE_QUOTA, sv->qs_itimelimit);
> 	state_warnlimit(fp, XFS_INODE_QUOTA, sv->qs_iwarnlimit);
> 
> 	state_timelimit(fp, XFS_RTBLOCK_QUOTA, sv->qs_rtbtimelimit);
> }
> 
> 
> Thanks,
> -Eric
> 

[PATCH v2 3/3] xfsprogs: xfs_quota state command should report ugp grace times

[Bill O'Donnell]

Since grace periods are now supported for three quota types (ugp),
modify xfs_quota state command to report times for all three.
Add a helper function for stat reporting.

Signed-off-by: Bill O'Donnell <billodo@redhat.com>
---
v2: load-up helper function more, further reducing redundant LoC

 quota/state.c | 96 +++++++++++++++++++++++++++++++++++----------------
 1 file changed, 67 insertions(+), 29 deletions(-)

diff --git a/quota/state.c b/quota/state.c
index 1627181d..19d34ed0 100644
--- a/quota/state.c
+++ b/quota/state.c
@@ -191,49 +191,87 @@ state_stat_to_statv(
 }
 
 static void
-state_quotafile_mount(
+state_quotafile_stat(
 	FILE			*fp,
 	uint			type,
-	struct fs_path		*mount,
+	struct fs_path          *mount,
+	struct fs_quota_statv	*sv,
+	struct fs_quota_stat	*s,
 	uint			flags)
 {
-	struct fs_quota_stat	s;
-	struct fs_quota_statv	sv;
+	bool			accounting, enforcing;
+	struct fs_qfilestatv	*qsv;
 	char			*dev = mount->fs_name;
 
-	sv.qs_version = FS_QSTATV_VERSION1;
-
-	if (xfsquotactl(XFS_GETQSTATV, dev, type, 0, (void *)&sv) < 0) {
-		if (xfsquotactl(XFS_GETQSTAT, dev, type, 0, (void *)&s) < 0) {
+	if (xfsquotactl(XFS_GETQSTATV, dev, type, 0, (void *)sv) < 0) {
+		if (xfsquotactl(XFS_GETQSTAT, dev, type, 0, (void *)s) < 0) {
 			if (flags & VERBOSE_FLAG)
 				fprintf(fp,
 					_("%s quota are not enabled on %s\n"),
 					type_to_string(type), dev);
 			return;
 		}
-		state_stat_to_statv(&s, &sv);
+		state_stat_to_statv(s, sv);
+	}
+
+	switch(type) {
+	case XFS_USER_QUOTA:
+		qsv = &sv->qs_uquota;
+		accounting = sv->qs_flags & XFS_QUOTA_UDQ_ACCT;
+		enforcing = sv->qs_flags & XFS_QUOTA_UDQ_ENFD;
+		break;
+	case XFS_GROUP_QUOTA:
+		qsv = &sv->qs_gquota;
+		accounting = sv->qs_flags & XFS_QUOTA_GDQ_ACCT;
+		enforcing = sv->qs_flags & XFS_QUOTA_GDQ_ENFD;
+		break;
+	case XFS_PROJ_QUOTA:
+		qsv = &sv->qs_pquota;
+		accounting = sv->qs_flags & XFS_QUOTA_PDQ_ACCT;
+		enforcing = sv->qs_flags & XFS_QUOTA_PDQ_ENFD;
+		break;
+	default:
+		return;
 	}
 
-	if (type & XFS_USER_QUOTA)
-		state_qfilestat(fp, mount, XFS_USER_QUOTA, &sv.qs_uquota,
-				sv.qs_flags & XFS_QUOTA_UDQ_ACCT,
-				sv.qs_flags & XFS_QUOTA_UDQ_ENFD);
-	if (type & XFS_GROUP_QUOTA)
-		state_qfilestat(fp, mount, XFS_GROUP_QUOTA, &sv.qs_gquota,
-				sv.qs_flags & XFS_QUOTA_GDQ_ACCT,
-				sv.qs_flags & XFS_QUOTA_GDQ_ENFD);
-	if (type & XFS_PROJ_QUOTA)
-		state_qfilestat(fp, mount, XFS_PROJ_QUOTA, &sv.qs_pquota,
-				sv.qs_flags & XFS_QUOTA_PDQ_ACCT,
-				sv.qs_flags & XFS_QUOTA_PDQ_ENFD);
-
-	state_timelimit(fp, XFS_BLOCK_QUOTA, sv.qs_btimelimit);
-	state_warnlimit(fp, XFS_BLOCK_QUOTA, sv.qs_bwarnlimit);
-
-	state_timelimit(fp, XFS_INODE_QUOTA, sv.qs_itimelimit);
-	state_warnlimit(fp, XFS_INODE_QUOTA, sv.qs_iwarnlimit);
-
-	state_timelimit(fp, XFS_RTBLOCK_QUOTA, sv.qs_rtbtimelimit);
+
+	state_qfilestat(fp, mount, type, qsv, accounting, enforcing);
+
+	state_timelimit(fp, XFS_BLOCK_QUOTA, sv->qs_btimelimit);
+	state_warnlimit(fp, XFS_BLOCK_QUOTA, sv->qs_bwarnlimit);
+
+	state_timelimit(fp, XFS_INODE_QUOTA, sv->qs_itimelimit);
+	state_warnlimit(fp, XFS_INODE_QUOTA, sv->qs_iwarnlimit);
+
+	state_timelimit(fp, XFS_RTBLOCK_QUOTA, sv->qs_rtbtimelimit);
+}
+
+static void
+state_quotafile_mount(
+	FILE			*fp,
+	uint			type,
+	struct fs_path		*mount,
+	uint			flags)
+{
+	struct fs_quota_stat	s;
+	struct fs_quota_statv	sv;
+
+	sv.qs_version = FS_QSTATV_VERSION1;
+
+	if (type & XFS_USER_QUOTA) {
+		state_quotafile_stat(fp, XFS_USER_QUOTA, mount,
+				     &sv, &s, flags);
+	}
+
+	if (type & XFS_GROUP_QUOTA) {
+		state_quotafile_stat(fp, XFS_GROUP_QUOTA, mount,
+				     &sv, &s, flags);
+	}
+
+	if (type & XFS_PROJ_QUOTA) {
+		state_quotafile_stat(fp, XFS_PROJ_QUOTA, mount,
+				     &sv, &s, flags);
+	}
 }
 
 static void
-- 
2.26.2

Re: [PATCH v2 3/3] xfsprogs: xfs_quota state command should report ugp grace times

[Darrick J. Wong]

On Fri, Jul 17, 2020 at 03:43:14PM -0500, Bill O'Donnell wrote:
> Since grace periods are now supported for three quota types (ugp),
> modify xfs_quota state command to report times for all three.
> Add a helper function for stat reporting.
> 
> Signed-off-by: Bill O'Donnell <billodo@redhat.com>
> ---
> v2: load-up helper function more, further reducing redundant LoC
> 
>  quota/state.c | 96 +++++++++++++++++++++++++++++++++++----------------
>  1 file changed, 67 insertions(+), 29 deletions(-)
> 
> diff --git a/quota/state.c b/quota/state.c
> index 1627181d..19d34ed0 100644
> --- a/quota/state.c
> +++ b/quota/state.c
> @@ -191,49 +191,87 @@ state_stat_to_statv(
>  }
>  
>  static void
> -state_quotafile_mount(
> +state_quotafile_stat(
>  	FILE			*fp,
>  	uint			type,
> -	struct fs_path		*mount,
> +	struct fs_path          *mount,
> +	struct fs_quota_statv	*sv,
> +	struct fs_quota_stat	*s,
>  	uint			flags)
>  {
> -	struct fs_quota_stat	s;
> -	struct fs_quota_statv	sv;
> +	bool			accounting, enforcing;
> +	struct fs_qfilestatv	*qsv;
>  	char			*dev = mount->fs_name;
>  
> -	sv.qs_version = FS_QSTATV_VERSION1;
> -
> -	if (xfsquotactl(XFS_GETQSTATV, dev, type, 0, (void *)&sv) < 0) {
> -		if (xfsquotactl(XFS_GETQSTAT, dev, type, 0, (void *)&s) < 0) {
> +	if (xfsquotactl(XFS_GETQSTATV, dev, type, 0, (void *)sv) < 0) {

At some point I'd love to refactor all these ioctl-like xfsquotactl
calls into a set of real functions with type checking and whatnot, but
this looks fine to me on its own:

Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> +		if (xfsquotactl(XFS_GETQSTAT, dev, type, 0, (void *)s) < 0) {
>  			if (flags & VERBOSE_FLAG)
>  				fprintf(fp,
>  					_("%s quota are not enabled on %s\n"),
>  					type_to_string(type), dev);
>  			return;
>  		}
> -		state_stat_to_statv(&s, &sv);
> +		state_stat_to_statv(s, sv);
> +	}
> +
> +	switch(type) {
> +	case XFS_USER_QUOTA:
> +		qsv = &sv->qs_uquota;
> +		accounting = sv->qs_flags & XFS_QUOTA_UDQ_ACCT;
> +		enforcing = sv->qs_flags & XFS_QUOTA_UDQ_ENFD;
> +		break;
> +	case XFS_GROUP_QUOTA:
> +		qsv = &sv->qs_gquota;
> +		accounting = sv->qs_flags & XFS_QUOTA_GDQ_ACCT;
> +		enforcing = sv->qs_flags & XFS_QUOTA_GDQ_ENFD;
> +		break;
> +	case XFS_PROJ_QUOTA:
> +		qsv = &sv->qs_pquota;
> +		accounting = sv->qs_flags & XFS_QUOTA_PDQ_ACCT;
> +		enforcing = sv->qs_flags & XFS_QUOTA_PDQ_ENFD;
> +		break;
> +	default:
> +		return;
>  	}
>  
> -	if (type & XFS_USER_QUOTA)
> -		state_qfilestat(fp, mount, XFS_USER_QUOTA, &sv.qs_uquota,
> -				sv.qs_flags & XFS_QUOTA_UDQ_ACCT,
> -				sv.qs_flags & XFS_QUOTA_UDQ_ENFD);
> -	if (type & XFS_GROUP_QUOTA)
> -		state_qfilestat(fp, mount, XFS_GROUP_QUOTA, &sv.qs_gquota,
> -				sv.qs_flags & XFS_QUOTA_GDQ_ACCT,
> -				sv.qs_flags & XFS_QUOTA_GDQ_ENFD);
> -	if (type & XFS_PROJ_QUOTA)
> -		state_qfilestat(fp, mount, XFS_PROJ_QUOTA, &sv.qs_pquota,
> -				sv.qs_flags & XFS_QUOTA_PDQ_ACCT,
> -				sv.qs_flags & XFS_QUOTA_PDQ_ENFD);
> -
> -	state_timelimit(fp, XFS_BLOCK_QUOTA, sv.qs_btimelimit);
> -	state_warnlimit(fp, XFS_BLOCK_QUOTA, sv.qs_bwarnlimit);
> -
> -	state_timelimit(fp, XFS_INODE_QUOTA, sv.qs_itimelimit);
> -	state_warnlimit(fp, XFS_INODE_QUOTA, sv.qs_iwarnlimit);
> -
> -	state_timelimit(fp, XFS_RTBLOCK_QUOTA, sv.qs_rtbtimelimit);
> +
> +	state_qfilestat(fp, mount, type, qsv, accounting, enforcing);
> +
> +	state_timelimit(fp, XFS_BLOCK_QUOTA, sv->qs_btimelimit);
> +	state_warnlimit(fp, XFS_BLOCK_QUOTA, sv->qs_bwarnlimit);
> +
> +	state_timelimit(fp, XFS_INODE_QUOTA, sv->qs_itimelimit);
> +	state_warnlimit(fp, XFS_INODE_QUOTA, sv->qs_iwarnlimit);
> +
> +	state_timelimit(fp, XFS_RTBLOCK_QUOTA, sv->qs_rtbtimelimit);
> +}
> +
> +static void
> +state_quotafile_mount(
> +	FILE			*fp,
> +	uint			type,
> +	struct fs_path		*mount,
> +	uint			flags)
> +{
> +	struct fs_quota_stat	s;
> +	struct fs_quota_statv	sv;
> +
> +	sv.qs_version = FS_QSTATV_VERSION1;
> +
> +	if (type & XFS_USER_QUOTA) {
> +		state_quotafile_stat(fp, XFS_USER_QUOTA, mount,
> +				     &sv, &s, flags);
> +	}
> +
> +	if (type & XFS_GROUP_QUOTA) {
> +		state_quotafile_stat(fp, XFS_GROUP_QUOTA, mount,
> +				     &sv, &s, flags);
> +	}
> +
> +	if (type & XFS_PROJ_QUOTA) {
> +		state_quotafile_stat(fp, XFS_PROJ_QUOTA, mount,
> +				     &sv, &s, flags);
> +	}
>  }
>  
>  static void
> -- 
> 2.26.2
> 

Re: [PATCH v2 3/3] xfsprogs: xfs_quota state command should report ugp grace times

[Christoph Hellwig]

On Fri, Jul 17, 2020 at 03:43:14PM -0500, Bill O'Donnell wrote:
> Since grace periods are now supported for three quota types (ugp),
> modify xfs_quota state command to report times for all three.
> Add a helper function for stat reporting.
> 
> Signed-off-by: Bill O'Donnell <billodo@redhat.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 0/3] xfsprogs: xfs_quota error message and state reporting improvement

[Eric Sandeen]

On 7/15/20 1:12 PM, Bill O'Donnell wrote:
> 
> This patchset improves xfs_quota command error message output as well as
> adding reporting for grace times and warning limits for state (u,g,p).
> 
> Note that patches to xfstests to handle these changes will be forthcoming.
> Also, patch 1 was originally submitted separately:
>   (xfsprogs: xfs_quota command error message improvement)
> 
> patch 1 contains the command error message improvements.
> patch 2 contains the warning limit reporting (originally SoB Darrick Wong).
> patch 3 contains the additional state reporting of grace times for u,g,p.
> 
> Comments appreciated. Thanks-
> Bill

Hi Bill - 

These look good and have reviews, but we need xfstests fixed up with filters
now, multiple tests now have output that doesn't match expected, causing
failures:

xfs/106	- output mismatch (see /root/xfstests-dev/results//xfs/106.out.bad)
    --- tests/xfs/106.out	2017-08-28 12:17:34.950134054 -0400
    +++ /root/xfstests-dev/results//xfs/106.out.bad	2020-07-30 13:34:26.889836623 -0400
    @@ -102,7 +102,9 @@
      Enforcement: OFF
      Inode: #[INO] (X blocks, Y extents)
     Blocks grace time: [3 days]
    +Blocks max warnings: 5
     Inodes grace time: [3 days]
    +Inodes max warnings: 5
     Realtime Blocks grace time: [7 days]


Thanks,
-Eric

Re: [PATCH 0/3] xfsprogs: xfs_quota error message and state reporting improvement

[Bill O'Donnell]

On Thu, Jul 30, 2020 at 10:46:18AM -0700, Eric Sandeen wrote:
> On 7/15/20 1:12 PM, Bill O'Donnell wrote:
> > 
> > This patchset improves xfs_quota command error message output as well as
> > adding reporting for grace times and warning limits for state (u,g,p).
> > 
> > Note that patches to xfstests to handle these changes will be forthcoming.
> > Also, patch 1 was originally submitted separately:
> >   (xfsprogs: xfs_quota command error message improvement)
> > 
> > patch 1 contains the command error message improvements.
> > patch 2 contains the warning limit reporting (originally SoB Darrick Wong).
> > patch 3 contains the additional state reporting of grace times for u,g,p.
> > 
> > Comments appreciated. Thanks-
> > Bill
> 
> Hi Bill - 
> 
> These look good and have reviews, but we need xfstests fixed up with filters
> now, multiple tests now have output that doesn't match expected, causing
> failures:

I'll work on it. Thanks for the reminder.
-Bill

> 
> xfs/106	- output mismatch (see /root/xfstests-dev/results//xfs/106.out.bad)
>     --- tests/xfs/106.out	2017-08-28 12:17:34.950134054 -0400
>     +++ /root/xfstests-dev/results//xfs/106.out.bad	2020-07-30 13:34:26.889836623 -0400
>     @@ -102,7 +102,9 @@
>       Enforcement: OFF
>       Inode: #[INO] (X blocks, Y extents)
>      Blocks grace time: [3 days]
>     +Blocks max warnings: 5
>      Inodes grace time: [3 days]
>     +Inodes max warnings: 5
>      Realtime Blocks grace time: [7 days]
> 
> 
> Thanks,
> -Eric
> 

Re: [PATCH 0/3] xfsprogs: xfs_quota error message and state reporting improvement

[Eric Sandeen]

On 7/30/20 10:52 AM, Bill O'Donnell wrote:
> On Thu, Jul 30, 2020 at 10:46:18AM -0700, Eric Sandeen wrote:
>> On 7/15/20 1:12 PM, Bill O'Donnell wrote:
>>>
>>> This patchset improves xfs_quota command error message output as well as
>>> adding reporting for grace times and warning limits for state (u,g,p).
>>>
>>> Note that patches to xfstests to handle these changes will be forthcoming.
>>> Also, patch 1 was originally submitted separately:
>>>   (xfsprogs: xfs_quota command error message improvement)
>>>
>>> patch 1 contains the command error message improvements.
>>> patch 2 contains the warning limit reporting (originally SoB Darrick Wong).
>>> patch 3 contains the additional state reporting of grace times for u,g,p.
>>>
>>> Comments appreciated. Thanks-
>>> Bill
>>
>> Hi Bill - 
>>
>> These look good and have reviews, but we need xfstests fixed up with filters
>> now, multiple tests now have output that doesn't match expected, causing
>> failures:
> 
> I'll work on it. Thanks for the reminder.
> -Bill

Ah, sounds like Darrick has (sent?) some(?) patches for this already....

-Eric