linux-xfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] xfs_quota: wire up XFS_GETQSTATV
@ 2016-08-12 23:07 Eric Sandeen
  2016-08-15 13:44 ` Bill O'Donnell
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Eric Sandeen @ 2016-08-12 23:07 UTC (permalink / raw)
  To: xfs-oss

The new XFS_GETQSTATV quotactl, available since kernel v3.12,
was never implemented in xfs_quota, and the "state" command
continues to use XFS_GETQSTAT, which cannot report both
group & project quota on newer formats.

The new call has room for all 3 quota types (user, group, and
quota), vs just two, where previously project and quota
overlapped.

So:

First, try XFS_GETQSTATV.
If it passes, we have all the information we need, and we print
it. state_qfilestat() is modified to take the newer structure.

If it fails, try XFS_GETQSTAT.  If that passes, we are on an
older kernel with neither XFS_GETQSTATV nor the on-disk project
quota inode.  We copy the available information into the newer
statv structure, carefully determining wither group or project
(or neither) is actually active, and print it with the same
state_qfilestat routine.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

I probably could have done some memcpy()'s in 
state_stat_to_statv(), but opted for the explicit copy-out;
the structures aren't identical, although the newer one 
only differs by padding on the end.  If memcpy() is preferable
I could send a V2...


diff --git a/include/xqm.h b/include/xqm.h
index c084b2d..78262c3 100644
--- a/include/xqm.h
+++ b/include/xqm.h
@@ -32,6 +32,7 @@
 #define Q_XGETQSTAT	XQM_CMD(5)	/* get quota subsystem status */
 #define Q_XQUOTARM	XQM_CMD(6)	/* free disk space used by dquots */
 #define Q_XQUOTASYNC	XQM_CMD(7)	/* delalloc flush, updates dquots */
+#define Q_XGETQSTATV	XQM_CMD(8)	/* newer version of get quota */
 #define Q_XGETNEXTQUOTA	XQM_CMD(9)	/* get disk limits and usage */
 
 /*
@@ -149,4 +150,30 @@ typedef struct fs_quota_stat {
 	__u16		qs_iwarnlimit;	/* limit for num warnings */
 } fs_quota_stat_t;
 
+/*
+ * Some basic information about 'quota files' for Q_XGETQSTATV command
+ */
+struct fs_qfilestatv {
+	__u64		qfs_ino;	/* inode number */
+	__u64		qfs_nblks;	/* number of BBs 512-byte-blks */
+	__u32		qfs_nextents;	/* number of extents */
+	__u32		qfs_pad;	/* pad for 8-byte alignment */
+};
+
+struct fs_quota_statv {
+	__s8			qs_version;	/* version for future changes */
+	__u8			qs_pad1;	/* pad for 16bit alignment */
+	__u16			qs_flags;	/* FS_QUOTA_.* flags */
+	__u32			qs_incoredqs;	/* number of dquots incore */
+	struct fs_qfilestatv	qs_uquota;	/* user quota information */
+	struct fs_qfilestatv	qs_gquota;	/* group quota information */
+	struct fs_qfilestatv	qs_pquota;	/* project quota information */
+	__s32			qs_btimelimit;	/* limit for blks timer */
+	__s32			qs_itimelimit;	/* limit for inodes timer */
+	__s32			qs_rtbtimelimit;/* limit for rt blks timer */
+	__u16			qs_bwarnlimit;	/* limit for num warnings */
+	__u16			qs_iwarnlimit;	/* limit for num warnings */
+	__u64			qs_pad2[8];	/* for future proofing */
+};
+
 #endif	/* __XQM_H__ */
diff --git a/quota/linux.c b/quota/linux.c
index 74dba01..4f1f3c4 100644
--- a/quota/linux.c
+++ b/quota/linux.c
@@ -55,6 +55,8 @@ xcommand_to_qcommand(
 		return Q_XSETQLIM;
 	case XFS_GETQSTAT:
 		return Q_XGETQSTAT;
+	case XFS_GETQSTATV:
+		return Q_XGETQSTATV;
 	case XFS_QUOTARM:
 		return Q_XQUOTARM;
 	case XFS_QSYNC:
diff --git a/quota/state.c b/quota/state.c
index 8186762..5d63579 100644
--- a/quota/state.c
+++ b/quota/state.c
@@ -111,12 +111,12 @@ remove_help(void)
 
 static void
 state_qfilestat(
-	FILE		*fp,
-	fs_path_t	*mount,
-	uint		type,
-	fs_qfilestat_t	*qfs,
-	int		accounting,
-	int		enforcing)
+	FILE			*fp,
+	struct fs_path		*mount,
+	uint			type,
+	struct fs_qfilestatv	*qfs,
+	int			accounting,
+	int			enforcing)
 {
 	fprintf(fp, _("%s quota state on %s (%s)\n"), type_to_string(type),
 		mount->fs_dir, mount->fs_name);
@@ -142,39 +142,94 @@ state_timelimit(
 		time_to_string(timelimit, VERBOSE_FLAG | ABSOLUTE_FLAG));
 }
 
+/*
+ * fs_quota_stat holds a subset of fs_quota_statv; this copies
+ * the smaller into the larger, leaving any not-present fields
+ * empty.  This is so the same reporting function can be used
+ * for both XFS_GETQSTAT and XFS_GETQSTATV results.
+ */
 static void
-state_quotafile_mount(
-	FILE		*fp,
-	uint		type,
-	fs_path_t	*mount,
-	uint		flags)
+state_stat_to_statv(
+	struct fs_quota_stat	*s,
+	struct fs_quota_statv	*sv)
 {
-	fs_quota_stat_t	s;
-	char		*dev = mount->fs_name;
+	memset(sv, 0, sizeof(struct fs_quota_statv));
+
+	/* shared information */
+	sv->qs_version = s->qs_version;
+	sv->qs_flags = s->qs_flags;
+	sv->qs_incoredqs = s->qs_incoredqs;
+	sv->qs_btimelimit = s->qs_btimelimit;
+	sv->qs_itimelimit = s->qs_itimelimit;
+	sv->qs_rtbtimelimit = s->qs_rtbtimelimit;
+	sv->qs_bwarnlimit = s->qs_bwarnlimit;
+	sv->qs_iwarnlimit = s->qs_iwarnlimit;
+
+	/* Always room for uquota */
+	sv->qs_uquota.qfs_ino = s->qs_uquota.qfs_ino;
+	sv->qs_uquota.qfs_nblks = s->qs_uquota.qfs_nblks;
+	sv->qs_uquota.qfs_nextents = s->qs_uquota.qfs_nextents;
+
+	/*
+	 * If we are here, XFS_GETQSTATV failed and XFS_GETQSTAT passed;
+	 * that is a very strong hint that we're on a kernel which predates
+	 * the on-disk pquota inode; both were added in v3.12.  So, we do
+	 * some tricksy determination here.
+	 * gs_gquota may hold either group quota inode info, or project
+	 * quota if that is used instead; which one it actually holds depends
+	 * on the quota flags.  (If neither is set, neither is used)
+	 */
+	if (s->qs_flags & XFS_QUOTA_GDQ_ACCT) {
+		/* gs_gquota holds group quota info */
+		sv->qs_gquota.qfs_ino = s->qs_gquota.qfs_ino;
+		sv->qs_gquota.qfs_nblks = s->qs_gquota.qfs_nblks;
+		sv->qs_gquota.qfs_nextents = s->qs_gquota.qfs_nextents;
+	} else if (s->qs_flags & XFS_QUOTA_PDQ_ACCT) {
+		/* gs_gquota actually holds project quota info */
+		sv->qs_pquota.qfs_ino = s->qs_gquota.qfs_ino;
+		sv->qs_pquota.qfs_nblks = s->qs_gquota.qfs_nblks;
+		sv->qs_pquota.qfs_nextents = s->qs_gquota.qfs_nextents;
+	}
+}
 
-	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;
+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;
+	char			*dev = mount->fs_name;
+
+	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);
 	}
 
 	if (type & XFS_USER_QUOTA)
-		state_qfilestat(fp, mount, XFS_USER_QUOTA, &s.qs_uquota,
-				s.qs_flags & XFS_QUOTA_UDQ_ACCT,
-				s.qs_flags & XFS_QUOTA_UDQ_ENFD);
+		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, &s.qs_gquota,
-				s.qs_flags & XFS_QUOTA_GDQ_ACCT,
-				s.qs_flags & XFS_QUOTA_GDQ_ENFD);
+		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, &s.qs_gquota,
-				s.qs_flags & XFS_QUOTA_PDQ_ACCT,
-				s.qs_flags & XFS_QUOTA_PDQ_ENFD);
+		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, s.qs_btimelimit);
-	state_timelimit(fp, XFS_INODE_QUOTA, s.qs_itimelimit);
-	state_timelimit(fp, XFS_RTBLOCK_QUOTA, s.qs_rtbtimelimit);
+	state_timelimit(fp, XFS_BLOCK_QUOTA, sv.qs_btimelimit);
+	state_timelimit(fp, XFS_INODE_QUOTA, sv.qs_itimelimit);
+	state_timelimit(fp, XFS_RTBLOCK_QUOTA, sv.qs_rtbtimelimit);
 }
 
 static void


_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] xfs_quota: wire up XFS_GETQSTATV
  2016-08-12 23:07 [PATCH] xfs_quota: wire up XFS_GETQSTATV Eric Sandeen
@ 2016-08-15 13:44 ` Bill O'Donnell
  2016-08-15 16:38 ` Zorro Lang
  2016-08-16  3:17 ` [PATCH V2] " Eric Sandeen
  2 siblings, 0 replies; 9+ messages in thread
From: Bill O'Donnell @ 2016-08-15 13:44 UTC (permalink / raw)
  To: Eric Sandeen; +Cc: xfs-oss

On Fri, Aug 12, 2016 at 06:07:00PM -0500, Eric Sandeen wrote:
> The new XFS_GETQSTATV quotactl, available since kernel v3.12,
> was never implemented in xfs_quota, and the "state" command
> continues to use XFS_GETQSTAT, which cannot report both
> group & project quota on newer formats.
> 
> The new call has room for all 3 quota types (user, group, and
> quota), vs just two, where previously project and quota
> overlapped.
> 
> So:
> 
> First, try XFS_GETQSTATV.
> If it passes, we have all the information we need, and we print
> it. state_qfilestat() is modified to take the newer structure.
> 
> If it fails, try XFS_GETQSTAT.  If that passes, we are on an
> older kernel with neither XFS_GETQSTATV nor the on-disk project
> quota inode.  We copy the available information into the newer
> statv structure, carefully determining wither group or project
> (or neither) is actually active, and print it with the same
> state_qfilestat routine.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Reviewed-by: Bill O'Donnell <billodo@redhat.com>

> ---
> 
> I probably could have done some memcpy()'s in 
> state_stat_to_statv(), but opted for the explicit copy-out;
> the structures aren't identical, although the newer one 
> only differs by padding on the end.  If memcpy() is preferable
> I could send a V2...
> 
> 
> diff --git a/include/xqm.h b/include/xqm.h
> index c084b2d..78262c3 100644
> --- a/include/xqm.h
> +++ b/include/xqm.h
> @@ -32,6 +32,7 @@
>  #define Q_XGETQSTAT	XQM_CMD(5)	/* get quota subsystem status */
>  #define Q_XQUOTARM	XQM_CMD(6)	/* free disk space used by dquots */
>  #define Q_XQUOTASYNC	XQM_CMD(7)	/* delalloc flush, updates dquots */
> +#define Q_XGETQSTATV	XQM_CMD(8)	/* newer version of get quota */
>  #define Q_XGETNEXTQUOTA	XQM_CMD(9)	/* get disk limits and usage */
>  
>  /*
> @@ -149,4 +150,30 @@ typedef struct fs_quota_stat {
>  	__u16		qs_iwarnlimit;	/* limit for num warnings */
>  } fs_quota_stat_t;
>  
> +/*
> + * Some basic information about 'quota files' for Q_XGETQSTATV command
> + */
> +struct fs_qfilestatv {
> +	__u64		qfs_ino;	/* inode number */
> +	__u64		qfs_nblks;	/* number of BBs 512-byte-blks */
> +	__u32		qfs_nextents;	/* number of extents */
> +	__u32		qfs_pad;	/* pad for 8-byte alignment */
> +};
> +
> +struct fs_quota_statv {
> +	__s8			qs_version;	/* version for future changes */
> +	__u8			qs_pad1;	/* pad for 16bit alignment */
> +	__u16			qs_flags;	/* FS_QUOTA_.* flags */
> +	__u32			qs_incoredqs;	/* number of dquots incore */
> +	struct fs_qfilestatv	qs_uquota;	/* user quota information */
> +	struct fs_qfilestatv	qs_gquota;	/* group quota information */
> +	struct fs_qfilestatv	qs_pquota;	/* project quota information */
> +	__s32			qs_btimelimit;	/* limit for blks timer */
> +	__s32			qs_itimelimit;	/* limit for inodes timer */
> +	__s32			qs_rtbtimelimit;/* limit for rt blks timer */
> +	__u16			qs_bwarnlimit;	/* limit for num warnings */
> +	__u16			qs_iwarnlimit;	/* limit for num warnings */
> +	__u64			qs_pad2[8];	/* for future proofing */
> +};
> +
>  #endif	/* __XQM_H__ */
> diff --git a/quota/linux.c b/quota/linux.c
> index 74dba01..4f1f3c4 100644
> --- a/quota/linux.c
> +++ b/quota/linux.c
> @@ -55,6 +55,8 @@ xcommand_to_qcommand(
>  		return Q_XSETQLIM;
>  	case XFS_GETQSTAT:
>  		return Q_XGETQSTAT;
> +	case XFS_GETQSTATV:
> +		return Q_XGETQSTATV;
>  	case XFS_QUOTARM:
>  		return Q_XQUOTARM;
>  	case XFS_QSYNC:
> diff --git a/quota/state.c b/quota/state.c
> index 8186762..5d63579 100644
> --- a/quota/state.c
> +++ b/quota/state.c
> @@ -111,12 +111,12 @@ remove_help(void)
>  
>  static void
>  state_qfilestat(
> -	FILE		*fp,
> -	fs_path_t	*mount,
> -	uint		type,
> -	fs_qfilestat_t	*qfs,
> -	int		accounting,
> -	int		enforcing)
> +	FILE			*fp,
> +	struct fs_path		*mount,
> +	uint			type,
> +	struct fs_qfilestatv	*qfs,
> +	int			accounting,
> +	int			enforcing)
>  {
>  	fprintf(fp, _("%s quota state on %s (%s)\n"), type_to_string(type),
>  		mount->fs_dir, mount->fs_name);
> @@ -142,39 +142,94 @@ state_timelimit(
>  		time_to_string(timelimit, VERBOSE_FLAG | ABSOLUTE_FLAG));
>  }
>  
> +/*
> + * fs_quota_stat holds a subset of fs_quota_statv; this copies
> + * the smaller into the larger, leaving any not-present fields
> + * empty.  This is so the same reporting function can be used
> + * for both XFS_GETQSTAT and XFS_GETQSTATV results.
> + */
>  static void
> -state_quotafile_mount(
> -	FILE		*fp,
> -	uint		type,
> -	fs_path_t	*mount,
> -	uint		flags)
> +state_stat_to_statv(
> +	struct fs_quota_stat	*s,
> +	struct fs_quota_statv	*sv)
>  {
> -	fs_quota_stat_t	s;
> -	char		*dev = mount->fs_name;
> +	memset(sv, 0, sizeof(struct fs_quota_statv));
> +
> +	/* shared information */
> +	sv->qs_version = s->qs_version;
> +	sv->qs_flags = s->qs_flags;
> +	sv->qs_incoredqs = s->qs_incoredqs;
> +	sv->qs_btimelimit = s->qs_btimelimit;
> +	sv->qs_itimelimit = s->qs_itimelimit;
> +	sv->qs_rtbtimelimit = s->qs_rtbtimelimit;
> +	sv->qs_bwarnlimit = s->qs_bwarnlimit;
> +	sv->qs_iwarnlimit = s->qs_iwarnlimit;
> +
> +	/* Always room for uquota */
> +	sv->qs_uquota.qfs_ino = s->qs_uquota.qfs_ino;
> +	sv->qs_uquota.qfs_nblks = s->qs_uquota.qfs_nblks;
> +	sv->qs_uquota.qfs_nextents = s->qs_uquota.qfs_nextents;
> +
> +	/*
> +	 * If we are here, XFS_GETQSTATV failed and XFS_GETQSTAT passed;
> +	 * that is a very strong hint that we're on a kernel which predates
> +	 * the on-disk pquota inode; both were added in v3.12.  So, we do
> +	 * some tricksy determination here.
> +	 * gs_gquota may hold either group quota inode info, or project
> +	 * quota if that is used instead; which one it actually holds depends
> +	 * on the quota flags.  (If neither is set, neither is used)
> +	 */
> +	if (s->qs_flags & XFS_QUOTA_GDQ_ACCT) {
> +		/* gs_gquota holds group quota info */
> +		sv->qs_gquota.qfs_ino = s->qs_gquota.qfs_ino;
> +		sv->qs_gquota.qfs_nblks = s->qs_gquota.qfs_nblks;
> +		sv->qs_gquota.qfs_nextents = s->qs_gquota.qfs_nextents;
> +	} else if (s->qs_flags & XFS_QUOTA_PDQ_ACCT) {
> +		/* gs_gquota actually holds project quota info */
> +		sv->qs_pquota.qfs_ino = s->qs_gquota.qfs_ino;
> +		sv->qs_pquota.qfs_nblks = s->qs_gquota.qfs_nblks;
> +		sv->qs_pquota.qfs_nextents = s->qs_gquota.qfs_nextents;
> +	}
> +}
>  
> -	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;
> +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;
> +	char			*dev = mount->fs_name;
> +
> +	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);
>  	}
>  
>  	if (type & XFS_USER_QUOTA)
> -		state_qfilestat(fp, mount, XFS_USER_QUOTA, &s.qs_uquota,
> -				s.qs_flags & XFS_QUOTA_UDQ_ACCT,
> -				s.qs_flags & XFS_QUOTA_UDQ_ENFD);
> +		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, &s.qs_gquota,
> -				s.qs_flags & XFS_QUOTA_GDQ_ACCT,
> -				s.qs_flags & XFS_QUOTA_GDQ_ENFD);
> +		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, &s.qs_gquota,
> -				s.qs_flags & XFS_QUOTA_PDQ_ACCT,
> -				s.qs_flags & XFS_QUOTA_PDQ_ENFD);
> +		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, s.qs_btimelimit);
> -	state_timelimit(fp, XFS_INODE_QUOTA, s.qs_itimelimit);
> -	state_timelimit(fp, XFS_RTBLOCK_QUOTA, s.qs_rtbtimelimit);
> +	state_timelimit(fp, XFS_BLOCK_QUOTA, sv.qs_btimelimit);
> +	state_timelimit(fp, XFS_INODE_QUOTA, sv.qs_itimelimit);
> +	state_timelimit(fp, XFS_RTBLOCK_QUOTA, sv.qs_rtbtimelimit);
>  }
>  
>  static void
> 
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] xfs_quota: wire up XFS_GETQSTATV
  2016-08-12 23:07 [PATCH] xfs_quota: wire up XFS_GETQSTATV Eric Sandeen
  2016-08-15 13:44 ` Bill O'Donnell
@ 2016-08-15 16:38 ` Zorro Lang
  2016-08-15 16:43   ` Eric Sandeen
  2016-08-16  3:17 ` [PATCH V2] " Eric Sandeen
  2 siblings, 1 reply; 9+ messages in thread
From: Zorro Lang @ 2016-08-15 16:38 UTC (permalink / raw)
  To: Eric Sandeen; +Cc: xfs-oss

On Fri, Aug 12, 2016 at 06:07:00PM -0500, Eric Sandeen wrote:
> The new XFS_GETQSTATV quotactl, available since kernel v3.12,
> was never implemented in xfs_quota, and the "state" command
> continues to use XFS_GETQSTAT, which cannot report both
> group & project quota on newer formats.
> 
> The new call has room for all 3 quota types (user, group, and
> quota), vs just two, where previously project and quota
> overlapped.
> 
> So:
> 
> First, try XFS_GETQSTATV.
> If it passes, we have all the information we need, and we print
> it. state_qfilestat() is modified to take the newer structure.
> 
> If it fails, try XFS_GETQSTAT.  If that passes, we are on an
> older kernel with neither XFS_GETQSTATV nor the on-disk project
> quota inode.  We copy the available information into the newer
> statv structure, carefully determining wither group or project
> (or neither) is actually active, and print it with the same
> state_qfilestat routine.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> 
> I probably could have done some memcpy()'s in 
> state_stat_to_statv(), but opted for the explicit copy-out;
> the structures aren't identical, although the newer one 
> only differs by padding on the end.  If memcpy() is preferable
> I could send a V2...
> 
> 
> diff --git a/include/xqm.h b/include/xqm.h
> index c084b2d..78262c3 100644
> --- a/include/xqm.h
> +++ b/include/xqm.h
> @@ -32,6 +32,7 @@
>  #define Q_XGETQSTAT	XQM_CMD(5)	/* get quota subsystem status */
>  #define Q_XQUOTARM	XQM_CMD(6)	/* free disk space used by dquots */
>  #define Q_XQUOTASYNC	XQM_CMD(7)	/* delalloc flush, updates dquots */
> +#define Q_XGETQSTATV	XQM_CMD(8)	/* newer version of get quota */
>  #define Q_XGETNEXTQUOTA	XQM_CMD(9)	/* get disk limits and usage */
>  
>  /*
> @@ -149,4 +150,30 @@ typedef struct fs_quota_stat {
>  	__u16		qs_iwarnlimit;	/* limit for num warnings */
>  } fs_quota_stat_t;
>  
> +/*
> + * Some basic information about 'quota files' for Q_XGETQSTATV command
> + */
> +struct fs_qfilestatv {
> +	__u64		qfs_ino;	/* inode number */
> +	__u64		qfs_nblks;	/* number of BBs 512-byte-blks */
> +	__u32		qfs_nextents;	/* number of extents */
> +	__u32		qfs_pad;	/* pad for 8-byte alignment */
> +};
> +
> +struct fs_quota_statv {
> +	__s8			qs_version;	/* version for future changes */
> +	__u8			qs_pad1;	/* pad for 16bit alignment */
> +	__u16			qs_flags;	/* FS_QUOTA_.* flags */
> +	__u32			qs_incoredqs;	/* number of dquots incore */
> +	struct fs_qfilestatv	qs_uquota;	/* user quota information */
> +	struct fs_qfilestatv	qs_gquota;	/* group quota information */
> +	struct fs_qfilestatv	qs_pquota;	/* project quota information */
> +	__s32			qs_btimelimit;	/* limit for blks timer */
> +	__s32			qs_itimelimit;	/* limit for inodes timer */
> +	__s32			qs_rtbtimelimit;/* limit for rt blks timer */
> +	__u16			qs_bwarnlimit;	/* limit for num warnings */
> +	__u16			qs_iwarnlimit;	/* limit for num warnings */
> +	__u64			qs_pad2[8];	/* for future proofing */
> +};
> +
>  #endif	/* __XQM_H__ */
> diff --git a/quota/linux.c b/quota/linux.c
> index 74dba01..4f1f3c4 100644
> --- a/quota/linux.c
> +++ b/quota/linux.c
> @@ -55,6 +55,8 @@ xcommand_to_qcommand(
>  		return Q_XSETQLIM;
>  	case XFS_GETQSTAT:
>  		return Q_XGETQSTAT;
> +	case XFS_GETQSTATV:
> +		return Q_XGETQSTATV;
>  	case XFS_QUOTARM:
>  		return Q_XQUOTARM;
>  	case XFS_QSYNC:
> diff --git a/quota/state.c b/quota/state.c
> index 8186762..5d63579 100644
> --- a/quota/state.c
> +++ b/quota/state.c
> @@ -111,12 +111,12 @@ remove_help(void)
>  
>  static void
>  state_qfilestat(
> -	FILE		*fp,
> -	fs_path_t	*mount,
> -	uint		type,
> -	fs_qfilestat_t	*qfs,
> -	int		accounting,
> -	int		enforcing)
> +	FILE			*fp,
> +	struct fs_path		*mount,
> +	uint			type,
> +	struct fs_qfilestatv	*qfs,
> +	int			accounting,
> +	int			enforcing)
>  {
>  	fprintf(fp, _("%s quota state on %s (%s)\n"), type_to_string(type),
>  		mount->fs_dir, mount->fs_name);
> @@ -142,39 +142,94 @@ state_timelimit(
>  		time_to_string(timelimit, VERBOSE_FLAG | ABSOLUTE_FLAG));
>  }
>  
> +/*
> + * fs_quota_stat holds a subset of fs_quota_statv; this copies
> + * the smaller into the larger, leaving any not-present fields
> + * empty.  This is so the same reporting function can be used
> + * for both XFS_GETQSTAT and XFS_GETQSTATV results.
> + */
>  static void
> -state_quotafile_mount(
> -	FILE		*fp,
> -	uint		type,
> -	fs_path_t	*mount,
> -	uint		flags)
> +state_stat_to_statv(
> +	struct fs_quota_stat	*s,
> +	struct fs_quota_statv	*sv)
>  {
> -	fs_quota_stat_t	s;
> -	char		*dev = mount->fs_name;
> +	memset(sv, 0, sizeof(struct fs_quota_statv));
> +
> +	/* shared information */
> +	sv->qs_version = s->qs_version;
> +	sv->qs_flags = s->qs_flags;
> +	sv->qs_incoredqs = s->qs_incoredqs;
> +	sv->qs_btimelimit = s->qs_btimelimit;
> +	sv->qs_itimelimit = s->qs_itimelimit;
> +	sv->qs_rtbtimelimit = s->qs_rtbtimelimit;
> +	sv->qs_bwarnlimit = s->qs_bwarnlimit;
> +	sv->qs_iwarnlimit = s->qs_iwarnlimit;
> +
> +	/* Always room for uquota */
> +	sv->qs_uquota.qfs_ino = s->qs_uquota.qfs_ino;
> +	sv->qs_uquota.qfs_nblks = s->qs_uquota.qfs_nblks;
> +	sv->qs_uquota.qfs_nextents = s->qs_uquota.qfs_nextents;
> +
> +	/*
> +	 * If we are here, XFS_GETQSTATV failed and XFS_GETQSTAT passed;
> +	 * that is a very strong hint that we're on a kernel which predates
> +	 * the on-disk pquota inode; both were added in v3.12.  So, we do
> +	 * some tricksy determination here.
> +	 * gs_gquota may hold either group quota inode info, or project
> +	 * quota if that is used instead; which one it actually holds depends
> +	 * on the quota flags.  (If neither is set, neither is used)
> +	 */
> +	if (s->qs_flags & XFS_QUOTA_GDQ_ACCT) {
> +		/* gs_gquota holds group quota info */
> +		sv->qs_gquota.qfs_ino = s->qs_gquota.qfs_ino;
> +		sv->qs_gquota.qfs_nblks = s->qs_gquota.qfs_nblks;
> +		sv->qs_gquota.qfs_nextents = s->qs_gquota.qfs_nextents;
> +	} else if (s->qs_flags & XFS_QUOTA_PDQ_ACCT) {
> +		/* gs_gquota actually holds project quota info */
> +		sv->qs_pquota.qfs_ino = s->qs_gquota.qfs_ino;
> +		sv->qs_pquota.qfs_nblks = s->qs_gquota.qfs_nblks;
> +		sv->qs_pquota.qfs_nextents = s->qs_gquota.qfs_nextents;
> +	}
> +}
>  
> -	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;
> +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;
> +	char			*dev = mount->fs_name;
> +

When I tested on XFS with crc=1, I got below result:

  [root@dhcp-13-149 xfsprogs-dev]# mount /dev/mapper/testvg-scratchdev /mnt/scratch -o gquota,pquota
  [root@dhcp-13-149 xfsprogs-dev]# xfs_quota -xc "state" /mnt/scratch
    User quota state on /mnt/scratch (/dev/mapper/testvg-scratchdev)
      Accounting: OFF
      Enforcement: OFF
      Inode: #101 (1 blocks, 1 extents)
    Group quota state on /mnt/scratch (/dev/mapper/testvg-scratchdev)
      Accounting: ON
      Enforcement: ON
      Inode: #99 (1 blocks, 1 extents)
    Project quota state on /mnt/scratch (/dev/mapper/testvg-scratchdev)
      Accounting: ON
      Enforcement: ON
      Inode: #0 (0 blocks, 0 extents)
    Blocks grace time: [7 days]
    Inodes grace time: [7 days]
    Realtime Blocks grace time: [7 days]

I got project quota inode number is #0, when I mount gquota and pquota together.
That's incorrect, because:

  [root@dhcp-13-149 xfsprogs-dev]# xfs_db -r -c "sb 0" -c "p" /dev/mapper/testvg-scratchdev|grep quot
  uquotino = 101
  gquotino = 99
  pquotino = 100

>From the result of strace, we can see:
  quotactl(0x5808 /* Q_??? */|USRQUOTA, "/dev/mapper/testvg-scratchdev", 0, 0x7ffdbbda94c0) = -1 EINVAL (Invalid argument)
  quotactl(Q_XGETQSTAT|USRQUOTA, "/dev/mapper/testvg-scratchdev", 0, {version=1, ...}) = 0

So the first call for XFS_GETQSTATV return EINVAL.

In linux kernel quota_getxstatev() function, the logic is:
  /* If this kernel doesn't support user specified version, fail */
  switch (fqs.qs_version) {
  case FS_QSTATV_VERSION1:
        break;
  default:
        return -EINVAL;
  }

So we need to set qa_version to FS_QSTATV_VERSION1, before
we call XFS_GETQSTATV. And as we talked on IRC, it test passed if I set

  sv.qs_version = 1;

at here.

So maybe a V2 patch is needed to fix this problem:)

Thanks,
Zorro

> +	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);
>  	}
>  
>  	if (type & XFS_USER_QUOTA)
> -		state_qfilestat(fp, mount, XFS_USER_QUOTA, &s.qs_uquota,
> -				s.qs_flags & XFS_QUOTA_UDQ_ACCT,
> -				s.qs_flags & XFS_QUOTA_UDQ_ENFD);
> +		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, &s.qs_gquota,
> -				s.qs_flags & XFS_QUOTA_GDQ_ACCT,
> -				s.qs_flags & XFS_QUOTA_GDQ_ENFD);
> +		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, &s.qs_gquota,
> -				s.qs_flags & XFS_QUOTA_PDQ_ACCT,
> -				s.qs_flags & XFS_QUOTA_PDQ_ENFD);
> +		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, s.qs_btimelimit);
> -	state_timelimit(fp, XFS_INODE_QUOTA, s.qs_itimelimit);
> -	state_timelimit(fp, XFS_RTBLOCK_QUOTA, s.qs_rtbtimelimit);
> +	state_timelimit(fp, XFS_BLOCK_QUOTA, sv.qs_btimelimit);
> +	state_timelimit(fp, XFS_INODE_QUOTA, sv.qs_itimelimit);
> +	state_timelimit(fp, XFS_RTBLOCK_QUOTA, sv.qs_rtbtimelimit);
>  }
>  
>  static void
> 
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] xfs_quota: wire up XFS_GETQSTATV
  2016-08-15 16:38 ` Zorro Lang
@ 2016-08-15 16:43   ` Eric Sandeen
  0 siblings, 0 replies; 9+ messages in thread
From: Eric Sandeen @ 2016-08-15 16:43 UTC (permalink / raw)
  To: Zorro Lang; +Cc: xfs-oss

On 8/15/16 11:38 AM, Zorro Lang wrote:
> From the result of strace, we can see:
>   quotactl(0x5808 /* Q_??? */|USRQUOTA, "/dev/mapper/testvg-scratchdev", 0, 0x7ffdbbda94c0) = -1 EINVAL (Invalid argument)
>   quotactl(Q_XGETQSTAT|USRQUOTA, "/dev/mapper/testvg-scratchdev", 0, {version=1, ...}) = 0
> 
> So the first call for XFS_GETQSTATV return EINVAL.
> 
> In linux kernel quota_getxstatev() function, the logic is:
>   /* If this kernel doesn't support user specified version, fail */
>   switch (fqs.qs_version) {
>   case FS_QSTATV_VERSION1:
>         break;
>   default:
>         return -EINVAL;
>   }
> 
> So we need to set qa_version to FS_QSTATV_VERSION1, before
> we call XFS_GETQSTATV. And as we talked on IRC, it test passed if I set
> 
>   sv.qs_version = 1;
> 
> at here.
> 
> So maybe a V2 patch is needed to fix this problem:)

Yep, kernel commit said:

| Callers of the new quotactl command have to set the version of the data
| structure being passed, and kernel will fill as much data as requested.
| If the kernel does not support the user-space provided version, EINVAL
| will be returned. User-space can reduce the version number and call the same
| quotactl again.

but this isn't documented anywhere else, AFAICT; this call still isn't
documented in the quotactl manpage.  o_O

I'll fix it up, thanks for catching it!

-Eric

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH V2] xfs_quota: wire up XFS_GETQSTATV
  2016-08-12 23:07 [PATCH] xfs_quota: wire up XFS_GETQSTATV Eric Sandeen
  2016-08-15 13:44 ` Bill O'Donnell
  2016-08-15 16:38 ` Zorro Lang
@ 2016-08-16  3:17 ` Eric Sandeen
  2016-08-17  8:33   ` Zorro Lang
  2016-08-17 15:02   ` Zorro Lang
  2 siblings, 2 replies; 9+ messages in thread
From: Eric Sandeen @ 2016-08-16  3:17 UTC (permalink / raw)
  To: xfs

The new XFS_GETQSTATV quotactl, available since kernel v3.12,
was never implemented in xfs_quota, and the "state" command
continues to use XFS_GETQSTAT, which cannot report both
group & project quota on newer formats.

The new call has room for all 3 quota types (user, group, and
quota), vs just two, where previously project and quota
overlapped.

So:

First, try XFS_GETQSTATV.
If it passes, we have all the information we need, and we print
it. state_qfilestat() is modified to take the newer structure.

If it fails, try XFS_GETQSTAT.  If that passes, we are on an
older kernel with neither XFS_GETQSTATV nor the on-disk project
quota inode.  We copy the available information into the newer
statv structure, carefully determining wither group or project
(or neither) is actually active, and print it with the same
state_qfilestat routine.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

I probably could have done some memcpy()'s in 
state_stat_to_statv(), but opted for the explicit copy-out;
the structures aren't identical, although the newer one 
only differs by padding on the end.  If memcpy() is preferable
I could send a V2...

V2: set sv.qs_version = FS_QSTATV_VERSION1; before calling
the quotactl (thanks Zorro!)

diff --git a/include/xqm.h b/include/xqm.h
index c084b2d..5b6934a 100644
--- a/include/xqm.h
+++ b/include/xqm.h
@@ -32,6 +32,7 @@
 #define Q_XGETQSTAT	XQM_CMD(5)	/* get quota subsystem status */
 #define Q_XQUOTARM	XQM_CMD(6)	/* free disk space used by dquots */
 #define Q_XQUOTASYNC	XQM_CMD(7)	/* delalloc flush, updates dquots */
+#define Q_XGETQSTATV	XQM_CMD(8)	/* newer version of get quota */
 #define Q_XGETNEXTQUOTA	XQM_CMD(9)	/* get disk limits and usage */
 
 /*
@@ -149,4 +150,35 @@ typedef struct fs_quota_stat {
 	__u16		qs_iwarnlimit;	/* limit for num warnings */
 } fs_quota_stat_t;
 
+
+#ifndef FS_QSTATV_VERSION1
+#define FS_QSTATV_VERSION1	1	/* fs_quota_statv.qs_version */
+#endif
+
+/*
+ * Some basic information about 'quota files' for Q_XGETQSTATV command
+ */
+struct fs_qfilestatv {
+	__u64		qfs_ino;	/* inode number */
+	__u64		qfs_nblks;	/* number of BBs 512-byte-blks */
+	__u32		qfs_nextents;	/* number of extents */
+	__u32		qfs_pad;	/* pad for 8-byte alignment */
+};
+
+struct fs_quota_statv {
+	__s8			qs_version;	/* version for future changes */
+	__u8			qs_pad1;	/* pad for 16bit alignment */
+	__u16			qs_flags;	/* FS_QUOTA_.* flags */
+	__u32			qs_incoredqs;	/* number of dquots incore */
+	struct fs_qfilestatv	qs_uquota;	/* user quota information */
+	struct fs_qfilestatv	qs_gquota;	/* group quota information */
+	struct fs_qfilestatv	qs_pquota;	/* project quota information */
+	__s32			qs_btimelimit;	/* limit for blks timer */
+	__s32			qs_itimelimit;	/* limit for inodes timer */
+	__s32			qs_rtbtimelimit;/* limit for rt blks timer */
+	__u16			qs_bwarnlimit;	/* limit for num warnings */
+	__u16			qs_iwarnlimit;	/* limit for num warnings */
+	__u64			qs_pad2[8];	/* for future proofing */
+};
+
 #endif	/* __XQM_H__ */
diff --git a/quota/linux.c b/quota/linux.c
index 74dba01..4f1f3c4 100644
--- a/quota/linux.c
+++ b/quota/linux.c
@@ -55,6 +55,8 @@ xcommand_to_qcommand(
 		return Q_XSETQLIM;
 	case XFS_GETQSTAT:
 		return Q_XGETQSTAT;
+	case XFS_GETQSTATV:
+		return Q_XGETQSTATV;
 	case XFS_QUOTARM:
 		return Q_XQUOTARM;
 	case XFS_QSYNC:
diff --git a/quota/state.c b/quota/state.c
index 8186762..9f6616e 100644
--- a/quota/state.c
+++ b/quota/state.c
@@ -111,12 +111,12 @@ remove_help(void)
 
 static void
 state_qfilestat(
-	FILE		*fp,
-	fs_path_t	*mount,
-	uint		type,
-	fs_qfilestat_t	*qfs,
-	int		accounting,
-	int		enforcing)
+	FILE			*fp,
+	struct fs_path		*mount,
+	uint			type,
+	struct fs_qfilestatv	*qfs,
+	int			accounting,
+	int			enforcing)
 {
 	fprintf(fp, _("%s quota state on %s (%s)\n"), type_to_string(type),
 		mount->fs_dir, mount->fs_name);
@@ -142,39 +142,96 @@ state_timelimit(
 		time_to_string(timelimit, VERBOSE_FLAG | ABSOLUTE_FLAG));
 }
 
+/*
+ * fs_quota_stat holds a subset of fs_quota_statv; this copies
+ * the smaller into the larger, leaving any not-present fields
+ * empty.  This is so the same reporting function can be used
+ * for both XFS_GETQSTAT and XFS_GETQSTATV results.
+ */
 static void
-state_quotafile_mount(
-	FILE		*fp,
-	uint		type,
-	fs_path_t	*mount,
-	uint		flags)
+state_stat_to_statv(
+	struct fs_quota_stat	*s,
+	struct fs_quota_statv	*sv)
 {
-	fs_quota_stat_t	s;
-	char		*dev = mount->fs_name;
+	memset(sv, 0, sizeof(struct fs_quota_statv));
+
+	/* shared information */
+	sv->qs_version = s->qs_version;
+	sv->qs_flags = s->qs_flags;
+	sv->qs_incoredqs = s->qs_incoredqs;
+	sv->qs_btimelimit = s->qs_btimelimit;
+	sv->qs_itimelimit = s->qs_itimelimit;
+	sv->qs_rtbtimelimit = s->qs_rtbtimelimit;
+	sv->qs_bwarnlimit = s->qs_bwarnlimit;
+	sv->qs_iwarnlimit = s->qs_iwarnlimit;
+
+	/* Always room for uquota */
+	sv->qs_uquota.qfs_ino = s->qs_uquota.qfs_ino;
+	sv->qs_uquota.qfs_nblks = s->qs_uquota.qfs_nblks;
+	sv->qs_uquota.qfs_nextents = s->qs_uquota.qfs_nextents;
+
+	/*
+	 * If we are here, XFS_GETQSTATV failed and XFS_GETQSTAT passed;
+	 * that is a very strong hint that we're on a kernel which predates
+	 * the on-disk pquota inode; both were added in v3.12.  So, we do
+	 * some tricksy determination here.
+	 * gs_gquota may hold either group quota inode info, or project
+	 * quota if that is used instead; which one it actually holds depends
+	 * on the quota flags.  (If neither is set, neither is used)
+	 */
+	if (s->qs_flags & XFS_QUOTA_GDQ_ACCT) {
+		/* gs_gquota holds group quota info */
+		sv->qs_gquota.qfs_ino = s->qs_gquota.qfs_ino;
+		sv->qs_gquota.qfs_nblks = s->qs_gquota.qfs_nblks;
+		sv->qs_gquota.qfs_nextents = s->qs_gquota.qfs_nextents;
+	} else if (s->qs_flags & XFS_QUOTA_PDQ_ACCT) {
+		/* gs_gquota actually holds project quota info */
+		sv->qs_pquota.qfs_ino = s->qs_gquota.qfs_ino;
+		sv->qs_pquota.qfs_nblks = s->qs_gquota.qfs_nblks;
+		sv->qs_pquota.qfs_nextents = s->qs_gquota.qfs_nextents;
+	}
+}
 
-	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;
+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;
+	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 (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);
 	}
 
 	if (type & XFS_USER_QUOTA)
-		state_qfilestat(fp, mount, XFS_USER_QUOTA, &s.qs_uquota,
-				s.qs_flags & XFS_QUOTA_UDQ_ACCT,
-				s.qs_flags & XFS_QUOTA_UDQ_ENFD);
+		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, &s.qs_gquota,
-				s.qs_flags & XFS_QUOTA_GDQ_ACCT,
-				s.qs_flags & XFS_QUOTA_GDQ_ENFD);
+		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, &s.qs_gquota,
-				s.qs_flags & XFS_QUOTA_PDQ_ACCT,
-				s.qs_flags & XFS_QUOTA_PDQ_ENFD);
+		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, s.qs_btimelimit);
-	state_timelimit(fp, XFS_INODE_QUOTA, s.qs_itimelimit);
-	state_timelimit(fp, XFS_RTBLOCK_QUOTA, s.qs_rtbtimelimit);
+	state_timelimit(fp, XFS_BLOCK_QUOTA, sv.qs_btimelimit);
+	state_timelimit(fp, XFS_INODE_QUOTA, sv.qs_itimelimit);
+	state_timelimit(fp, XFS_RTBLOCK_QUOTA, sv.qs_rtbtimelimit);
 }
 
 static void

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH V2] xfs_quota: wire up XFS_GETQSTATV
  2016-08-16  3:17 ` [PATCH V2] " Eric Sandeen
@ 2016-08-17  8:33   ` Zorro Lang
  2016-08-17 14:43     ` Eric Sandeen
  2016-08-17 15:02   ` Zorro Lang
  1 sibling, 1 reply; 9+ messages in thread
From: Zorro Lang @ 2016-08-17  8:33 UTC (permalink / raw)
  To: Eric Sandeen; +Cc: xfs

On Mon, Aug 15, 2016 at 10:17:19PM -0500, Eric Sandeen wrote:
> The new XFS_GETQSTATV quotactl, available since kernel v3.12,
> was never implemented in xfs_quota, and the "state" command
> continues to use XFS_GETQSTAT, which cannot report both
> group & project quota on newer formats.
> 
> The new call has room for all 3 quota types (user, group, and
> quota), vs just two, where previously project and quota
> overlapped.
> 
> So:
> 
> First, try XFS_GETQSTATV.
> If it passes, we have all the information we need, and we print
> it. state_qfilestat() is modified to take the newer structure.
> 
> If it fails, try XFS_GETQSTAT.  If that passes, we are on an
> older kernel with neither XFS_GETQSTATV nor the on-disk project
> quota inode.  We copy the available information into the newer
> statv structure, carefully determining wither group or project
> (or neither) is actually active, and print it with the same
> state_qfilestat routine.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---

Hi Eric,

This's what I tried to explain to you:

  [root@dhcp-13-149 ~]# mount /dev/mapper/testvg-scratchdev /mnt/scratch -o pquota,gquota,uquota
  [root@dhcp-13-149 xfsprogs-dev]# xfs_quota -xc "limit bsoft=100m bhard=200m fsgqa" /mnt/scratch
  [root@dhcp-13-149 xfsprogs-dev]# xfs_quota -xc "limit -g bsoft=100m bhard=200m fsgqa" /mnt/scratch
  [root@dhcp-13-149 xfsprogs-dev]# xfs_quota -xc "state" /mnt/scratch
  User quota state on /mnt/scratch (/dev/mapper/testvg-scratchdev)
    Accounting: ON
    Enforcement: ON
    Inode: #101 (2 blocks, 2 extents)
  Group quota state on /mnt/scratch (/dev/mapper/testvg-scratchdev)
    Accounting: ON
    Enforcement: ON
    Inode: #99 (2 blocks, 2 extents)
  Project quota state on /mnt/scratch (/dev/mapper/testvg-scratchdev)
    Accounting: ON
    Enforcement: ON
    Inode: #100 (1 blocks, 1 extents)
  Blocks grace time: [7 days]
  Inodes grace time: [7 days]
  Realtime Blocks grace time: [7 days]
  [root@dhcp-13-149 xfsprogs-dev]# umount /mnt/scratch
  [root@dhcp-13-149 xfsprogs-dev]# mount /dev/mapper/testvg-scratchdev /mnt/scratch -o pquota
  [root@dhcp-13-149 xfsprogs-dev]# xfs_quota -xc "state" /mnt/scratch
  User quota state on /mnt/scratch (/dev/mapper/testvg-scratchdev)
    Accounting: OFF
    Enforcement: OFF
    Inode: #0 (0 blocks, 0 extents)
  Group quota state on /mnt/scratch (/dev/mapper/testvg-scratchdev)
    Accounting: OFF
    Enforcement: OFF
    Inode: #0 (0 blocks, 0 extents)
  Project quota state on /mnt/scratch (/dev/mapper/testvg-scratchdev)
    Accounting: ON
    Enforcement: ON
    Inode: #100 (1 blocks, 1 extents)
  Blocks grace time: [7 days]
  Inodes grace time: [7 days]
  Realtime Blocks grace time: [7 days]
  [root@dhcp-13-149 ~]# xfs_db -r -c "sb 0" -c "p" /dev/mapper/testvg-scratchdev|grep quot
  uquotino = 101
  gquotino = 99
  pquotino = 100


If I someone quota isn't enable, "state" command will print that inode
number, blocks and extents as 0.

Do you think this's a problem should be fixed?
If it's a bug, maybe we can fix it on another patch later, because
it's another bug?

Thanks,
Zorro

> 
> I probably could have done some memcpy()'s in 
> state_stat_to_statv(), but opted for the explicit copy-out;
> the structures aren't identical, although the newer one 
> only differs by padding on the end.  If memcpy() is preferable
> I could send a V2...
> 
> V2: set sv.qs_version = FS_QSTATV_VERSION1; before calling
> the quotactl (thanks Zorro!)
> 
> diff --git a/include/xqm.h b/include/xqm.h
> index c084b2d..5b6934a 100644
> --- a/include/xqm.h
> +++ b/include/xqm.h
> @@ -32,6 +32,7 @@
>  #define Q_XGETQSTAT	XQM_CMD(5)	/* get quota subsystem status */
>  #define Q_XQUOTARM	XQM_CMD(6)	/* free disk space used by dquots */
>  #define Q_XQUOTASYNC	XQM_CMD(7)	/* delalloc flush, updates dquots */
> +#define Q_XGETQSTATV	XQM_CMD(8)	/* newer version of get quota */
>  #define Q_XGETNEXTQUOTA	XQM_CMD(9)	/* get disk limits and usage */
>  
>  /*
> @@ -149,4 +150,35 @@ typedef struct fs_quota_stat {
>  	__u16		qs_iwarnlimit;	/* limit for num warnings */
>  } fs_quota_stat_t;
>  
> +
> +#ifndef FS_QSTATV_VERSION1
> +#define FS_QSTATV_VERSION1	1	/* fs_quota_statv.qs_version */
> +#endif
> +
> +/*
> + * Some basic information about 'quota files' for Q_XGETQSTATV command
> + */
> +struct fs_qfilestatv {
> +	__u64		qfs_ino;	/* inode number */
> +	__u64		qfs_nblks;	/* number of BBs 512-byte-blks */
> +	__u32		qfs_nextents;	/* number of extents */
> +	__u32		qfs_pad;	/* pad for 8-byte alignment */
> +};
> +
> +struct fs_quota_statv {
> +	__s8			qs_version;	/* version for future changes */
> +	__u8			qs_pad1;	/* pad for 16bit alignment */
> +	__u16			qs_flags;	/* FS_QUOTA_.* flags */
> +	__u32			qs_incoredqs;	/* number of dquots incore */
> +	struct fs_qfilestatv	qs_uquota;	/* user quota information */
> +	struct fs_qfilestatv	qs_gquota;	/* group quota information */
> +	struct fs_qfilestatv	qs_pquota;	/* project quota information */
> +	__s32			qs_btimelimit;	/* limit for blks timer */
> +	__s32			qs_itimelimit;	/* limit for inodes timer */
> +	__s32			qs_rtbtimelimit;/* limit for rt blks timer */
> +	__u16			qs_bwarnlimit;	/* limit for num warnings */
> +	__u16			qs_iwarnlimit;	/* limit for num warnings */
> +	__u64			qs_pad2[8];	/* for future proofing */
> +};
> +
>  #endif	/* __XQM_H__ */
> diff --git a/quota/linux.c b/quota/linux.c
> index 74dba01..4f1f3c4 100644
> --- a/quota/linux.c
> +++ b/quota/linux.c
> @@ -55,6 +55,8 @@ xcommand_to_qcommand(
>  		return Q_XSETQLIM;
>  	case XFS_GETQSTAT:
>  		return Q_XGETQSTAT;
> +	case XFS_GETQSTATV:
> +		return Q_XGETQSTATV;
>  	case XFS_QUOTARM:
>  		return Q_XQUOTARM;
>  	case XFS_QSYNC:
> diff --git a/quota/state.c b/quota/state.c
> index 8186762..9f6616e 100644
> --- a/quota/state.c
> +++ b/quota/state.c
> @@ -111,12 +111,12 @@ remove_help(void)
>  
>  static void
>  state_qfilestat(
> -	FILE		*fp,
> -	fs_path_t	*mount,
> -	uint		type,
> -	fs_qfilestat_t	*qfs,
> -	int		accounting,
> -	int		enforcing)
> +	FILE			*fp,
> +	struct fs_path		*mount,
> +	uint			type,
> +	struct fs_qfilestatv	*qfs,
> +	int			accounting,
> +	int			enforcing)
>  {
>  	fprintf(fp, _("%s quota state on %s (%s)\n"), type_to_string(type),
>  		mount->fs_dir, mount->fs_name);
> @@ -142,39 +142,96 @@ state_timelimit(
>  		time_to_string(timelimit, VERBOSE_FLAG | ABSOLUTE_FLAG));
>  }
>  
> +/*
> + * fs_quota_stat holds a subset of fs_quota_statv; this copies
> + * the smaller into the larger, leaving any not-present fields
> + * empty.  This is so the same reporting function can be used
> + * for both XFS_GETQSTAT and XFS_GETQSTATV results.
> + */
>  static void
> -state_quotafile_mount(
> -	FILE		*fp,
> -	uint		type,
> -	fs_path_t	*mount,
> -	uint		flags)
> +state_stat_to_statv(
> +	struct fs_quota_stat	*s,
> +	struct fs_quota_statv	*sv)
>  {
> -	fs_quota_stat_t	s;
> -	char		*dev = mount->fs_name;
> +	memset(sv, 0, sizeof(struct fs_quota_statv));
> +
> +	/* shared information */
> +	sv->qs_version = s->qs_version;
> +	sv->qs_flags = s->qs_flags;
> +	sv->qs_incoredqs = s->qs_incoredqs;
> +	sv->qs_btimelimit = s->qs_btimelimit;
> +	sv->qs_itimelimit = s->qs_itimelimit;
> +	sv->qs_rtbtimelimit = s->qs_rtbtimelimit;
> +	sv->qs_bwarnlimit = s->qs_bwarnlimit;
> +	sv->qs_iwarnlimit = s->qs_iwarnlimit;
> +
> +	/* Always room for uquota */
> +	sv->qs_uquota.qfs_ino = s->qs_uquota.qfs_ino;
> +	sv->qs_uquota.qfs_nblks = s->qs_uquota.qfs_nblks;
> +	sv->qs_uquota.qfs_nextents = s->qs_uquota.qfs_nextents;
> +
> +	/*
> +	 * If we are here, XFS_GETQSTATV failed and XFS_GETQSTAT passed;
> +	 * that is a very strong hint that we're on a kernel which predates
> +	 * the on-disk pquota inode; both were added in v3.12.  So, we do
> +	 * some tricksy determination here.
> +	 * gs_gquota may hold either group quota inode info, or project
> +	 * quota if that is used instead; which one it actually holds depends
> +	 * on the quota flags.  (If neither is set, neither is used)
> +	 */
> +	if (s->qs_flags & XFS_QUOTA_GDQ_ACCT) {
> +		/* gs_gquota holds group quota info */
> +		sv->qs_gquota.qfs_ino = s->qs_gquota.qfs_ino;
> +		sv->qs_gquota.qfs_nblks = s->qs_gquota.qfs_nblks;
> +		sv->qs_gquota.qfs_nextents = s->qs_gquota.qfs_nextents;
> +	} else if (s->qs_flags & XFS_QUOTA_PDQ_ACCT) {
> +		/* gs_gquota actually holds project quota info */
> +		sv->qs_pquota.qfs_ino = s->qs_gquota.qfs_ino;
> +		sv->qs_pquota.qfs_nblks = s->qs_gquota.qfs_nblks;
> +		sv->qs_pquota.qfs_nextents = s->qs_gquota.qfs_nextents;
> +	}
> +}
>  
> -	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;
> +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;
> +	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 (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);
>  	}
>  
>  	if (type & XFS_USER_QUOTA)
> -		state_qfilestat(fp, mount, XFS_USER_QUOTA, &s.qs_uquota,
> -				s.qs_flags & XFS_QUOTA_UDQ_ACCT,
> -				s.qs_flags & XFS_QUOTA_UDQ_ENFD);
> +		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, &s.qs_gquota,
> -				s.qs_flags & XFS_QUOTA_GDQ_ACCT,
> -				s.qs_flags & XFS_QUOTA_GDQ_ENFD);
> +		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, &s.qs_gquota,
> -				s.qs_flags & XFS_QUOTA_PDQ_ACCT,
> -				s.qs_flags & XFS_QUOTA_PDQ_ENFD);
> +		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, s.qs_btimelimit);
> -	state_timelimit(fp, XFS_INODE_QUOTA, s.qs_itimelimit);
> -	state_timelimit(fp, XFS_RTBLOCK_QUOTA, s.qs_rtbtimelimit);
> +	state_timelimit(fp, XFS_BLOCK_QUOTA, sv.qs_btimelimit);
> +	state_timelimit(fp, XFS_INODE_QUOTA, sv.qs_itimelimit);
> +	state_timelimit(fp, XFS_RTBLOCK_QUOTA, sv.qs_rtbtimelimit);
>  }
>  
>  static void
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH V2] xfs_quota: wire up XFS_GETQSTATV
  2016-08-17  8:33   ` Zorro Lang
@ 2016-08-17 14:43     ` Eric Sandeen
  2016-08-17 14:58       ` Zorro Lang
  0 siblings, 1 reply; 9+ messages in thread
From: Eric Sandeen @ 2016-08-17 14:43 UTC (permalink / raw)
  To: xfs

On 8/17/16 3:33 AM, Zorro Lang wrote:

> Hi Eric,
> 
> This's what I tried to explain to you:
> 
>   [root@dhcp-13-149 ~]# mount /dev/mapper/testvg-scratchdev /mnt/scratch -o pquota,gquota,uquota
>   [root@dhcp-13-149 xfsprogs-dev]# xfs_quota -xc "limit bsoft=100m bhard=200m fsgqa" /mnt/scratch
>   [root@dhcp-13-149 xfsprogs-dev]# xfs_quota -xc "limit -g bsoft=100m bhard=200m fsgqa" /mnt/scratch
>   [root@dhcp-13-149 xfsprogs-dev]# xfs_quota -xc "state" /mnt/scratch
>   User quota state on /mnt/scratch (/dev/mapper/testvg-scratchdev)
>     Accounting: ON
>     Enforcement: ON
>     Inode: #101 (2 blocks, 2 extents)
>   Group quota state on /mnt/scratch (/dev/mapper/testvg-scratchdev)
>     Accounting: ON
>     Enforcement: ON
>     Inode: #99 (2 blocks, 2 extents)
>   Project quota state on /mnt/scratch (/dev/mapper/testvg-scratchdev)
>     Accounting: ON
>     Enforcement: ON
>     Inode: #100 (1 blocks, 1 extents)
>   Blocks grace time: [7 days]
>   Inodes grace time: [7 days]
>   Realtime Blocks grace time: [7 days]

And that's all correct, right.

>   [root@dhcp-13-149 xfsprogs-dev]# umount /mnt/scratch
>   [root@dhcp-13-149 xfsprogs-dev]# mount /dev/mapper/testvg-scratchdev /mnt/scratch -o pquota

Now you have only pquota...

>   [root@dhcp-13-149 xfsprogs-dev]# xfs_quota -xc "state" /mnt/scratch
>   User quota state on /mnt/scratch (/dev/mapper/testvg-scratchdev)
>     Accounting: OFF
>     Enforcement: OFF
>     Inode: #0 (0 blocks, 0 extents)
>   Group quota state on /mnt/scratch (/dev/mapper/testvg-scratchdev)
>     Accounting: OFF
>     Enforcement: OFF
>     Inode: #0 (0 blocks, 0 extents)
>   Project quota state on /mnt/scratch (/dev/mapper/testvg-scratchdev)
>     Accounting: ON
>     Enforcement: ON
>     Inode: #100 (1 blocks, 1 extents)
>   Blocks grace time: [7 days]
>   Inodes grace time: [7 days]
>   Realtime Blocks grace time: [7 days]

And pquota is properly shown, but user and group show inode 0.

>   [root@dhcp-13-149 ~]# xfs_db -r -c "sb 0" -c "p" /dev/mapper/testvg-scratchdev|grep quot
>   uquotino = 101
>   gquotino = 99
>   pquotino = 100
> 
> 
> If I someone quota isn't enable, "state" command will print that inode
> number, blocks and extents as 0.

*nod*

> Do you think this's a problem should be fixed?
> If it's a bug, maybe we can fix it on another patch later, because
> it's another bug?

This is a bug in the kernel, because it does not fill in inode information
for inactive quota types, and fills in 0 instead (so we don't even
get NULLFSINO).

I did send a patch for this,

[PATCH] quota: fill in Q_XGETQSTAT inode information for inactive quotas

to the linux-fsdevel list (it is in common vfs quota code, but I probably
should have cc'd the xfs list as well).  Jan has indicated that he has
merged it.

With that patch and this one, it should all work as expected.

Thanks,
-Eric

> Thanks,
> Zorro
> 
>>
>> I probably could have done some memcpy()'s in 
>> state_stat_to_statv(), but opted for the explicit copy-out;
>> the structures aren't identical, although the newer one 
>> only differs by padding on the end.  If memcpy() is preferable
>> I could send a V2...
>>
>> V2: set sv.qs_version = FS_QSTATV_VERSION1; before calling
>> the quotactl (thanks Zorro!)
>>
>> diff --git a/include/xqm.h b/include/xqm.h
>> index c084b2d..5b6934a 100644
>> --- a/include/xqm.h
>> +++ b/include/xqm.h
>> @@ -32,6 +32,7 @@
>>  #define Q_XGETQSTAT	XQM_CMD(5)	/* get quota subsystem status */
>>  #define Q_XQUOTARM	XQM_CMD(6)	/* free disk space used by dquots */
>>  #define Q_XQUOTASYNC	XQM_CMD(7)	/* delalloc flush, updates dquots */
>> +#define Q_XGETQSTATV	XQM_CMD(8)	/* newer version of get quota */
>>  #define Q_XGETNEXTQUOTA	XQM_CMD(9)	/* get disk limits and usage */
>>  
>>  /*
>> @@ -149,4 +150,35 @@ typedef struct fs_quota_stat {
>>  	__u16		qs_iwarnlimit;	/* limit for num warnings */
>>  } fs_quota_stat_t;
>>  
>> +
>> +#ifndef FS_QSTATV_VERSION1
>> +#define FS_QSTATV_VERSION1	1	/* fs_quota_statv.qs_version */
>> +#endif
>> +
>> +/*
>> + * Some basic information about 'quota files' for Q_XGETQSTATV command
>> + */
>> +struct fs_qfilestatv {
>> +	__u64		qfs_ino;	/* inode number */
>> +	__u64		qfs_nblks;	/* number of BBs 512-byte-blks */
>> +	__u32		qfs_nextents;	/* number of extents */
>> +	__u32		qfs_pad;	/* pad for 8-byte alignment */
>> +};
>> +
>> +struct fs_quota_statv {
>> +	__s8			qs_version;	/* version for future changes */
>> +	__u8			qs_pad1;	/* pad for 16bit alignment */
>> +	__u16			qs_flags;	/* FS_QUOTA_.* flags */
>> +	__u32			qs_incoredqs;	/* number of dquots incore */
>> +	struct fs_qfilestatv	qs_uquota;	/* user quota information */
>> +	struct fs_qfilestatv	qs_gquota;	/* group quota information */
>> +	struct fs_qfilestatv	qs_pquota;	/* project quota information */
>> +	__s32			qs_btimelimit;	/* limit for blks timer */
>> +	__s32			qs_itimelimit;	/* limit for inodes timer */
>> +	__s32			qs_rtbtimelimit;/* limit for rt blks timer */
>> +	__u16			qs_bwarnlimit;	/* limit for num warnings */
>> +	__u16			qs_iwarnlimit;	/* limit for num warnings */
>> +	__u64			qs_pad2[8];	/* for future proofing */
>> +};
>> +
>>  #endif	/* __XQM_H__ */
>> diff --git a/quota/linux.c b/quota/linux.c
>> index 74dba01..4f1f3c4 100644
>> --- a/quota/linux.c
>> +++ b/quota/linux.c
>> @@ -55,6 +55,8 @@ xcommand_to_qcommand(
>>  		return Q_XSETQLIM;
>>  	case XFS_GETQSTAT:
>>  		return Q_XGETQSTAT;
>> +	case XFS_GETQSTATV:
>> +		return Q_XGETQSTATV;
>>  	case XFS_QUOTARM:
>>  		return Q_XQUOTARM;
>>  	case XFS_QSYNC:
>> diff --git a/quota/state.c b/quota/state.c
>> index 8186762..9f6616e 100644
>> --- a/quota/state.c
>> +++ b/quota/state.c
>> @@ -111,12 +111,12 @@ remove_help(void)
>>  
>>  static void
>>  state_qfilestat(
>> -	FILE		*fp,
>> -	fs_path_t	*mount,
>> -	uint		type,
>> -	fs_qfilestat_t	*qfs,
>> -	int		accounting,
>> -	int		enforcing)
>> +	FILE			*fp,
>> +	struct fs_path		*mount,
>> +	uint			type,
>> +	struct fs_qfilestatv	*qfs,
>> +	int			accounting,
>> +	int			enforcing)
>>  {
>>  	fprintf(fp, _("%s quota state on %s (%s)\n"), type_to_string(type),
>>  		mount->fs_dir, mount->fs_name);
>> @@ -142,39 +142,96 @@ state_timelimit(
>>  		time_to_string(timelimit, VERBOSE_FLAG | ABSOLUTE_FLAG));
>>  }
>>  
>> +/*
>> + * fs_quota_stat holds a subset of fs_quota_statv; this copies
>> + * the smaller into the larger, leaving any not-present fields
>> + * empty.  This is so the same reporting function can be used
>> + * for both XFS_GETQSTAT and XFS_GETQSTATV results.
>> + */
>>  static void
>> -state_quotafile_mount(
>> -	FILE		*fp,
>> -	uint		type,
>> -	fs_path_t	*mount,
>> -	uint		flags)
>> +state_stat_to_statv(
>> +	struct fs_quota_stat	*s,
>> +	struct fs_quota_statv	*sv)
>>  {
>> -	fs_quota_stat_t	s;
>> -	char		*dev = mount->fs_name;
>> +	memset(sv, 0, sizeof(struct fs_quota_statv));
>> +
>> +	/* shared information */
>> +	sv->qs_version = s->qs_version;
>> +	sv->qs_flags = s->qs_flags;
>> +	sv->qs_incoredqs = s->qs_incoredqs;
>> +	sv->qs_btimelimit = s->qs_btimelimit;
>> +	sv->qs_itimelimit = s->qs_itimelimit;
>> +	sv->qs_rtbtimelimit = s->qs_rtbtimelimit;
>> +	sv->qs_bwarnlimit = s->qs_bwarnlimit;
>> +	sv->qs_iwarnlimit = s->qs_iwarnlimit;
>> +
>> +	/* Always room for uquota */
>> +	sv->qs_uquota.qfs_ino = s->qs_uquota.qfs_ino;
>> +	sv->qs_uquota.qfs_nblks = s->qs_uquota.qfs_nblks;
>> +	sv->qs_uquota.qfs_nextents = s->qs_uquota.qfs_nextents;
>> +
>> +	/*
>> +	 * If we are here, XFS_GETQSTATV failed and XFS_GETQSTAT passed;
>> +	 * that is a very strong hint that we're on a kernel which predates
>> +	 * the on-disk pquota inode; both were added in v3.12.  So, we do
>> +	 * some tricksy determination here.
>> +	 * gs_gquota may hold either group quota inode info, or project
>> +	 * quota if that is used instead; which one it actually holds depends
>> +	 * on the quota flags.  (If neither is set, neither is used)
>> +	 */
>> +	if (s->qs_flags & XFS_QUOTA_GDQ_ACCT) {
>> +		/* gs_gquota holds group quota info */
>> +		sv->qs_gquota.qfs_ino = s->qs_gquota.qfs_ino;
>> +		sv->qs_gquota.qfs_nblks = s->qs_gquota.qfs_nblks;
>> +		sv->qs_gquota.qfs_nextents = s->qs_gquota.qfs_nextents;
>> +	} else if (s->qs_flags & XFS_QUOTA_PDQ_ACCT) {
>> +		/* gs_gquota actually holds project quota info */
>> +		sv->qs_pquota.qfs_ino = s->qs_gquota.qfs_ino;
>> +		sv->qs_pquota.qfs_nblks = s->qs_gquota.qfs_nblks;
>> +		sv->qs_pquota.qfs_nextents = s->qs_gquota.qfs_nextents;
>> +	}
>> +}
>>  
>> -	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;
>> +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;
>> +	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 (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);
>>  	}
>>  
>>  	if (type & XFS_USER_QUOTA)
>> -		state_qfilestat(fp, mount, XFS_USER_QUOTA, &s.qs_uquota,
>> -				s.qs_flags & XFS_QUOTA_UDQ_ACCT,
>> -				s.qs_flags & XFS_QUOTA_UDQ_ENFD);
>> +		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, &s.qs_gquota,
>> -				s.qs_flags & XFS_QUOTA_GDQ_ACCT,
>> -				s.qs_flags & XFS_QUOTA_GDQ_ENFD);
>> +		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, &s.qs_gquota,
>> -				s.qs_flags & XFS_QUOTA_PDQ_ACCT,
>> -				s.qs_flags & XFS_QUOTA_PDQ_ENFD);
>> +		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, s.qs_btimelimit);
>> -	state_timelimit(fp, XFS_INODE_QUOTA, s.qs_itimelimit);
>> -	state_timelimit(fp, XFS_RTBLOCK_QUOTA, s.qs_rtbtimelimit);
>> +	state_timelimit(fp, XFS_BLOCK_QUOTA, sv.qs_btimelimit);
>> +	state_timelimit(fp, XFS_INODE_QUOTA, sv.qs_itimelimit);
>> +	state_timelimit(fp, XFS_RTBLOCK_QUOTA, sv.qs_rtbtimelimit);
>>  }
>>  
>>  static void
>>
>> _______________________________________________
>> xfs mailing list
>> xfs@oss.sgi.com
>> http://oss.sgi.com/mailman/listinfo/xfs
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs
> 

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH V2] xfs_quota: wire up XFS_GETQSTATV
  2016-08-17 14:43     ` Eric Sandeen
@ 2016-08-17 14:58       ` Zorro Lang
  0 siblings, 0 replies; 9+ messages in thread
From: Zorro Lang @ 2016-08-17 14:58 UTC (permalink / raw)
  To: Eric Sandeen; +Cc: xfs

On Wed, Aug 17, 2016 at 09:43:36AM -0500, Eric Sandeen wrote:
> On 8/17/16 3:33 AM, Zorro Lang wrote:
> 
> > Hi Eric,
> > 
> > This's what I tried to explain to you:
> > 
> >   [root@dhcp-13-149 ~]# mount /dev/mapper/testvg-scratchdev /mnt/scratch -o pquota,gquota,uquota
> >   [root@dhcp-13-149 xfsprogs-dev]# xfs_quota -xc "limit bsoft=100m bhard=200m fsgqa" /mnt/scratch
> >   [root@dhcp-13-149 xfsprogs-dev]# xfs_quota -xc "limit -g bsoft=100m bhard=200m fsgqa" /mnt/scratch
> >   [root@dhcp-13-149 xfsprogs-dev]# xfs_quota -xc "state" /mnt/scratch
> >   User quota state on /mnt/scratch (/dev/mapper/testvg-scratchdev)
> >     Accounting: ON
> >     Enforcement: ON
> >     Inode: #101 (2 blocks, 2 extents)
> >   Group quota state on /mnt/scratch (/dev/mapper/testvg-scratchdev)
> >     Accounting: ON
> >     Enforcement: ON
> >     Inode: #99 (2 blocks, 2 extents)
> >   Project quota state on /mnt/scratch (/dev/mapper/testvg-scratchdev)
> >     Accounting: ON
> >     Enforcement: ON
> >     Inode: #100 (1 blocks, 1 extents)
> >   Blocks grace time: [7 days]
> >   Inodes grace time: [7 days]
> >   Realtime Blocks grace time: [7 days]
> 
> And that's all correct, right.
> 
> >   [root@dhcp-13-149 xfsprogs-dev]# umount /mnt/scratch
> >   [root@dhcp-13-149 xfsprogs-dev]# mount /dev/mapper/testvg-scratchdev /mnt/scratch -o pquota
> 
> Now you have only pquota...
> 
> >   [root@dhcp-13-149 xfsprogs-dev]# xfs_quota -xc "state" /mnt/scratch
> >   User quota state on /mnt/scratch (/dev/mapper/testvg-scratchdev)
> >     Accounting: OFF
> >     Enforcement: OFF
> >     Inode: #0 (0 blocks, 0 extents)
> >   Group quota state on /mnt/scratch (/dev/mapper/testvg-scratchdev)
> >     Accounting: OFF
> >     Enforcement: OFF
> >     Inode: #0 (0 blocks, 0 extents)
> >   Project quota state on /mnt/scratch (/dev/mapper/testvg-scratchdev)
> >     Accounting: ON
> >     Enforcement: ON
> >     Inode: #100 (1 blocks, 1 extents)
> >   Blocks grace time: [7 days]
> >   Inodes grace time: [7 days]
> >   Realtime Blocks grace time: [7 days]
> 
> And pquota is properly shown, but user and group show inode 0.
> 
> >   [root@dhcp-13-149 ~]# xfs_db -r -c "sb 0" -c "p" /dev/mapper/testvg-scratchdev|grep quot
> >   uquotino = 101
> >   gquotino = 99
> >   pquotino = 100
> > 
> > 
> > If I someone quota isn't enable, "state" command will print that inode
> > number, blocks and extents as 0.
> 
> *nod*
> 
> > Do you think this's a problem should be fixed?
> > If it's a bug, maybe we can fix it on another patch later, because
> > it's another bug?
> 
> This is a bug in the kernel, because it does not fill in inode information
> for inactive quota types, and fills in 0 instead (so we don't even
> get NULLFSINO).
> 
> I did send a patch for this,
> 
> [PATCH] quota: fill in Q_XGETQSTAT inode information for inactive quotas
> 
> to the linux-fsdevel list (it is in common vfs quota code, but I probably
> should have cc'd the xfs list as well).  Jan has indicated that he has
> merged it.
> 
> With that patch and this one, it should all work as expected.

Wow, sorry I didn't notice that patch(It's not in xfs cc list). So now
everything works well, I have no objection with this patch now. Please
allow me to ack this patch:)

I'll test both patches together later.

Thanks,
Zorro

> 
> Thanks,
> -Eric
> 
> > Thanks,
> > Zorro
> > 
> >>
> >> I probably could have done some memcpy()'s in 
> >> state_stat_to_statv(), but opted for the explicit copy-out;
> >> the structures aren't identical, although the newer one 
> >> only differs by padding on the end.  If memcpy() is preferable
> >> I could send a V2...
> >>
> >> V2: set sv.qs_version = FS_QSTATV_VERSION1; before calling
> >> the quotactl (thanks Zorro!)
> >>
> >> diff --git a/include/xqm.h b/include/xqm.h
> >> index c084b2d..5b6934a 100644
> >> --- a/include/xqm.h
> >> +++ b/include/xqm.h
> >> @@ -32,6 +32,7 @@
> >>  #define Q_XGETQSTAT	XQM_CMD(5)	/* get quota subsystem status */
> >>  #define Q_XQUOTARM	XQM_CMD(6)	/* free disk space used by dquots */
> >>  #define Q_XQUOTASYNC	XQM_CMD(7)	/* delalloc flush, updates dquots */
> >> +#define Q_XGETQSTATV	XQM_CMD(8)	/* newer version of get quota */
> >>  #define Q_XGETNEXTQUOTA	XQM_CMD(9)	/* get disk limits and usage */
> >>  
> >>  /*
> >> @@ -149,4 +150,35 @@ typedef struct fs_quota_stat {
> >>  	__u16		qs_iwarnlimit;	/* limit for num warnings */
> >>  } fs_quota_stat_t;
> >>  
> >> +
> >> +#ifndef FS_QSTATV_VERSION1
> >> +#define FS_QSTATV_VERSION1	1	/* fs_quota_statv.qs_version */
> >> +#endif
> >> +
> >> +/*
> >> + * Some basic information about 'quota files' for Q_XGETQSTATV command
> >> + */
> >> +struct fs_qfilestatv {
> >> +	__u64		qfs_ino;	/* inode number */
> >> +	__u64		qfs_nblks;	/* number of BBs 512-byte-blks */
> >> +	__u32		qfs_nextents;	/* number of extents */
> >> +	__u32		qfs_pad;	/* pad for 8-byte alignment */
> >> +};
> >> +
> >> +struct fs_quota_statv {
> >> +	__s8			qs_version;	/* version for future changes */
> >> +	__u8			qs_pad1;	/* pad for 16bit alignment */
> >> +	__u16			qs_flags;	/* FS_QUOTA_.* flags */
> >> +	__u32			qs_incoredqs;	/* number of dquots incore */
> >> +	struct fs_qfilestatv	qs_uquota;	/* user quota information */
> >> +	struct fs_qfilestatv	qs_gquota;	/* group quota information */
> >> +	struct fs_qfilestatv	qs_pquota;	/* project quota information */
> >> +	__s32			qs_btimelimit;	/* limit for blks timer */
> >> +	__s32			qs_itimelimit;	/* limit for inodes timer */
> >> +	__s32			qs_rtbtimelimit;/* limit for rt blks timer */
> >> +	__u16			qs_bwarnlimit;	/* limit for num warnings */
> >> +	__u16			qs_iwarnlimit;	/* limit for num warnings */
> >> +	__u64			qs_pad2[8];	/* for future proofing */
> >> +};
> >> +
> >>  #endif	/* __XQM_H__ */
> >> diff --git a/quota/linux.c b/quota/linux.c
> >> index 74dba01..4f1f3c4 100644
> >> --- a/quota/linux.c
> >> +++ b/quota/linux.c
> >> @@ -55,6 +55,8 @@ xcommand_to_qcommand(
> >>  		return Q_XSETQLIM;
> >>  	case XFS_GETQSTAT:
> >>  		return Q_XGETQSTAT;
> >> +	case XFS_GETQSTATV:
> >> +		return Q_XGETQSTATV;
> >>  	case XFS_QUOTARM:
> >>  		return Q_XQUOTARM;
> >>  	case XFS_QSYNC:
> >> diff --git a/quota/state.c b/quota/state.c
> >> index 8186762..9f6616e 100644
> >> --- a/quota/state.c
> >> +++ b/quota/state.c
> >> @@ -111,12 +111,12 @@ remove_help(void)
> >>  
> >>  static void
> >>  state_qfilestat(
> >> -	FILE		*fp,
> >> -	fs_path_t	*mount,
> >> -	uint		type,
> >> -	fs_qfilestat_t	*qfs,
> >> -	int		accounting,
> >> -	int		enforcing)
> >> +	FILE			*fp,
> >> +	struct fs_path		*mount,
> >> +	uint			type,
> >> +	struct fs_qfilestatv	*qfs,
> >> +	int			accounting,
> >> +	int			enforcing)
> >>  {
> >>  	fprintf(fp, _("%s quota state on %s (%s)\n"), type_to_string(type),
> >>  		mount->fs_dir, mount->fs_name);
> >> @@ -142,39 +142,96 @@ state_timelimit(
> >>  		time_to_string(timelimit, VERBOSE_FLAG | ABSOLUTE_FLAG));
> >>  }
> >>  
> >> +/*
> >> + * fs_quota_stat holds a subset of fs_quota_statv; this copies
> >> + * the smaller into the larger, leaving any not-present fields
> >> + * empty.  This is so the same reporting function can be used
> >> + * for both XFS_GETQSTAT and XFS_GETQSTATV results.
> >> + */
> >>  static void
> >> -state_quotafile_mount(
> >> -	FILE		*fp,
> >> -	uint		type,
> >> -	fs_path_t	*mount,
> >> -	uint		flags)
> >> +state_stat_to_statv(
> >> +	struct fs_quota_stat	*s,
> >> +	struct fs_quota_statv	*sv)
> >>  {
> >> -	fs_quota_stat_t	s;
> >> -	char		*dev = mount->fs_name;
> >> +	memset(sv, 0, sizeof(struct fs_quota_statv));
> >> +
> >> +	/* shared information */
> >> +	sv->qs_version = s->qs_version;
> >> +	sv->qs_flags = s->qs_flags;
> >> +	sv->qs_incoredqs = s->qs_incoredqs;
> >> +	sv->qs_btimelimit = s->qs_btimelimit;
> >> +	sv->qs_itimelimit = s->qs_itimelimit;
> >> +	sv->qs_rtbtimelimit = s->qs_rtbtimelimit;
> >> +	sv->qs_bwarnlimit = s->qs_bwarnlimit;
> >> +	sv->qs_iwarnlimit = s->qs_iwarnlimit;
> >> +
> >> +	/* Always room for uquota */
> >> +	sv->qs_uquota.qfs_ino = s->qs_uquota.qfs_ino;
> >> +	sv->qs_uquota.qfs_nblks = s->qs_uquota.qfs_nblks;
> >> +	sv->qs_uquota.qfs_nextents = s->qs_uquota.qfs_nextents;
> >> +
> >> +	/*
> >> +	 * If we are here, XFS_GETQSTATV failed and XFS_GETQSTAT passed;
> >> +	 * that is a very strong hint that we're on a kernel which predates
> >> +	 * the on-disk pquota inode; both were added in v3.12.  So, we do
> >> +	 * some tricksy determination here.
> >> +	 * gs_gquota may hold either group quota inode info, or project
> >> +	 * quota if that is used instead; which one it actually holds depends
> >> +	 * on the quota flags.  (If neither is set, neither is used)
> >> +	 */
> >> +	if (s->qs_flags & XFS_QUOTA_GDQ_ACCT) {
> >> +		/* gs_gquota holds group quota info */
> >> +		sv->qs_gquota.qfs_ino = s->qs_gquota.qfs_ino;
> >> +		sv->qs_gquota.qfs_nblks = s->qs_gquota.qfs_nblks;
> >> +		sv->qs_gquota.qfs_nextents = s->qs_gquota.qfs_nextents;
> >> +	} else if (s->qs_flags & XFS_QUOTA_PDQ_ACCT) {
> >> +		/* gs_gquota actually holds project quota info */
> >> +		sv->qs_pquota.qfs_ino = s->qs_gquota.qfs_ino;
> >> +		sv->qs_pquota.qfs_nblks = s->qs_gquota.qfs_nblks;
> >> +		sv->qs_pquota.qfs_nextents = s->qs_gquota.qfs_nextents;
> >> +	}
> >> +}
> >>  
> >> -	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;
> >> +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;
> >> +	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 (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);
> >>  	}
> >>  
> >>  	if (type & XFS_USER_QUOTA)
> >> -		state_qfilestat(fp, mount, XFS_USER_QUOTA, &s.qs_uquota,
> >> -				s.qs_flags & XFS_QUOTA_UDQ_ACCT,
> >> -				s.qs_flags & XFS_QUOTA_UDQ_ENFD);
> >> +		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, &s.qs_gquota,
> >> -				s.qs_flags & XFS_QUOTA_GDQ_ACCT,
> >> -				s.qs_flags & XFS_QUOTA_GDQ_ENFD);
> >> +		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, &s.qs_gquota,
> >> -				s.qs_flags & XFS_QUOTA_PDQ_ACCT,
> >> -				s.qs_flags & XFS_QUOTA_PDQ_ENFD);
> >> +		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, s.qs_btimelimit);
> >> -	state_timelimit(fp, XFS_INODE_QUOTA, s.qs_itimelimit);
> >> -	state_timelimit(fp, XFS_RTBLOCK_QUOTA, s.qs_rtbtimelimit);
> >> +	state_timelimit(fp, XFS_BLOCK_QUOTA, sv.qs_btimelimit);
> >> +	state_timelimit(fp, XFS_INODE_QUOTA, sv.qs_itimelimit);
> >> +	state_timelimit(fp, XFS_RTBLOCK_QUOTA, sv.qs_rtbtimelimit);
> >>  }
> >>  
> >>  static void
> >>
> >> _______________________________________________
> >> xfs mailing list
> >> xfs@oss.sgi.com
> >> http://oss.sgi.com/mailman/listinfo/xfs
> > 
> > _______________________________________________
> > xfs mailing list
> > xfs@oss.sgi.com
> > http://oss.sgi.com/mailman/listinfo/xfs
> > 
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH V2] xfs_quota: wire up XFS_GETQSTATV
  2016-08-16  3:17 ` [PATCH V2] " Eric Sandeen
  2016-08-17  8:33   ` Zorro Lang
@ 2016-08-17 15:02   ` Zorro Lang
  1 sibling, 0 replies; 9+ messages in thread
From: Zorro Lang @ 2016-08-17 15:02 UTC (permalink / raw)
  To: Eric Sandeen; +Cc: xfs

On Mon, Aug 15, 2016 at 10:17:19PM -0500, Eric Sandeen wrote:
> The new XFS_GETQSTATV quotactl, available since kernel v3.12,
> was never implemented in xfs_quota, and the "state" command
> continues to use XFS_GETQSTAT, which cannot report both
> group & project quota on newer formats.
> 
> The new call has room for all 3 quota types (user, group, and
> quota), vs just two, where previously project and quota
> overlapped.
> 
> So:
> 
> First, try XFS_GETQSTATV.
> If it passes, we have all the information we need, and we print
> it. state_qfilestat() is modified to take the newer structure.
> 
> If it fails, try XFS_GETQSTAT.  If that passes, we are on an
> older kernel with neither XFS_GETQSTATV nor the on-disk project
> quota inode.  We copy the available information into the newer
> statv structure, carefully determining wither group or project
> (or neither) is actually active, and print it with the same
> state_qfilestat routine.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---

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

Everything looks good to me now, all problems I can find have
been resolved.

Thanks,
Zorro

> 
> I probably could have done some memcpy()'s in 
> state_stat_to_statv(), but opted for the explicit copy-out;
> the structures aren't identical, although the newer one 
> only differs by padding on the end.  If memcpy() is preferable
> I could send a V2...
> 
> V2: set sv.qs_version = FS_QSTATV_VERSION1; before calling
> the quotactl (thanks Zorro!)
> 
> diff --git a/include/xqm.h b/include/xqm.h
> index c084b2d..5b6934a 100644
> --- a/include/xqm.h
> +++ b/include/xqm.h
> @@ -32,6 +32,7 @@
>  #define Q_XGETQSTAT	XQM_CMD(5)	/* get quota subsystem status */
>  #define Q_XQUOTARM	XQM_CMD(6)	/* free disk space used by dquots */
>  #define Q_XQUOTASYNC	XQM_CMD(7)	/* delalloc flush, updates dquots */
> +#define Q_XGETQSTATV	XQM_CMD(8)	/* newer version of get quota */
>  #define Q_XGETNEXTQUOTA	XQM_CMD(9)	/* get disk limits and usage */
>  
>  /*
> @@ -149,4 +150,35 @@ typedef struct fs_quota_stat {
>  	__u16		qs_iwarnlimit;	/* limit for num warnings */
>  } fs_quota_stat_t;
>  
> +
> +#ifndef FS_QSTATV_VERSION1
> +#define FS_QSTATV_VERSION1	1	/* fs_quota_statv.qs_version */
> +#endif
> +
> +/*
> + * Some basic information about 'quota files' for Q_XGETQSTATV command
> + */
> +struct fs_qfilestatv {
> +	__u64		qfs_ino;	/* inode number */
> +	__u64		qfs_nblks;	/* number of BBs 512-byte-blks */
> +	__u32		qfs_nextents;	/* number of extents */
> +	__u32		qfs_pad;	/* pad for 8-byte alignment */
> +};
> +
> +struct fs_quota_statv {
> +	__s8			qs_version;	/* version for future changes */
> +	__u8			qs_pad1;	/* pad for 16bit alignment */
> +	__u16			qs_flags;	/* FS_QUOTA_.* flags */
> +	__u32			qs_incoredqs;	/* number of dquots incore */
> +	struct fs_qfilestatv	qs_uquota;	/* user quota information */
> +	struct fs_qfilestatv	qs_gquota;	/* group quota information */
> +	struct fs_qfilestatv	qs_pquota;	/* project quota information */
> +	__s32			qs_btimelimit;	/* limit for blks timer */
> +	__s32			qs_itimelimit;	/* limit for inodes timer */
> +	__s32			qs_rtbtimelimit;/* limit for rt blks timer */
> +	__u16			qs_bwarnlimit;	/* limit for num warnings */
> +	__u16			qs_iwarnlimit;	/* limit for num warnings */
> +	__u64			qs_pad2[8];	/* for future proofing */
> +};
> +
>  #endif	/* __XQM_H__ */
> diff --git a/quota/linux.c b/quota/linux.c
> index 74dba01..4f1f3c4 100644
> --- a/quota/linux.c
> +++ b/quota/linux.c
> @@ -55,6 +55,8 @@ xcommand_to_qcommand(
>  		return Q_XSETQLIM;
>  	case XFS_GETQSTAT:
>  		return Q_XGETQSTAT;
> +	case XFS_GETQSTATV:
> +		return Q_XGETQSTATV;
>  	case XFS_QUOTARM:
>  		return Q_XQUOTARM;
>  	case XFS_QSYNC:
> diff --git a/quota/state.c b/quota/state.c
> index 8186762..9f6616e 100644
> --- a/quota/state.c
> +++ b/quota/state.c
> @@ -111,12 +111,12 @@ remove_help(void)
>  
>  static void
>  state_qfilestat(
> -	FILE		*fp,
> -	fs_path_t	*mount,
> -	uint		type,
> -	fs_qfilestat_t	*qfs,
> -	int		accounting,
> -	int		enforcing)
> +	FILE			*fp,
> +	struct fs_path		*mount,
> +	uint			type,
> +	struct fs_qfilestatv	*qfs,
> +	int			accounting,
> +	int			enforcing)
>  {
>  	fprintf(fp, _("%s quota state on %s (%s)\n"), type_to_string(type),
>  		mount->fs_dir, mount->fs_name);
> @@ -142,39 +142,96 @@ state_timelimit(
>  		time_to_string(timelimit, VERBOSE_FLAG | ABSOLUTE_FLAG));
>  }
>  
> +/*
> + * fs_quota_stat holds a subset of fs_quota_statv; this copies
> + * the smaller into the larger, leaving any not-present fields
> + * empty.  This is so the same reporting function can be used
> + * for both XFS_GETQSTAT and XFS_GETQSTATV results.
> + */
>  static void
> -state_quotafile_mount(
> -	FILE		*fp,
> -	uint		type,
> -	fs_path_t	*mount,
> -	uint		flags)
> +state_stat_to_statv(
> +	struct fs_quota_stat	*s,
> +	struct fs_quota_statv	*sv)
>  {
> -	fs_quota_stat_t	s;
> -	char		*dev = mount->fs_name;
> +	memset(sv, 0, sizeof(struct fs_quota_statv));
> +
> +	/* shared information */
> +	sv->qs_version = s->qs_version;
> +	sv->qs_flags = s->qs_flags;
> +	sv->qs_incoredqs = s->qs_incoredqs;
> +	sv->qs_btimelimit = s->qs_btimelimit;
> +	sv->qs_itimelimit = s->qs_itimelimit;
> +	sv->qs_rtbtimelimit = s->qs_rtbtimelimit;
> +	sv->qs_bwarnlimit = s->qs_bwarnlimit;
> +	sv->qs_iwarnlimit = s->qs_iwarnlimit;
> +
> +	/* Always room for uquota */
> +	sv->qs_uquota.qfs_ino = s->qs_uquota.qfs_ino;
> +	sv->qs_uquota.qfs_nblks = s->qs_uquota.qfs_nblks;
> +	sv->qs_uquota.qfs_nextents = s->qs_uquota.qfs_nextents;
> +
> +	/*
> +	 * If we are here, XFS_GETQSTATV failed and XFS_GETQSTAT passed;
> +	 * that is a very strong hint that we're on a kernel which predates
> +	 * the on-disk pquota inode; both were added in v3.12.  So, we do
> +	 * some tricksy determination here.
> +	 * gs_gquota may hold either group quota inode info, or project
> +	 * quota if that is used instead; which one it actually holds depends
> +	 * on the quota flags.  (If neither is set, neither is used)
> +	 */
> +	if (s->qs_flags & XFS_QUOTA_GDQ_ACCT) {
> +		/* gs_gquota holds group quota info */
> +		sv->qs_gquota.qfs_ino = s->qs_gquota.qfs_ino;
> +		sv->qs_gquota.qfs_nblks = s->qs_gquota.qfs_nblks;
> +		sv->qs_gquota.qfs_nextents = s->qs_gquota.qfs_nextents;
> +	} else if (s->qs_flags & XFS_QUOTA_PDQ_ACCT) {
> +		/* gs_gquota actually holds project quota info */
> +		sv->qs_pquota.qfs_ino = s->qs_gquota.qfs_ino;
> +		sv->qs_pquota.qfs_nblks = s->qs_gquota.qfs_nblks;
> +		sv->qs_pquota.qfs_nextents = s->qs_gquota.qfs_nextents;
> +	}
> +}
>  
> -	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;
> +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;
> +	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 (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);
>  	}
>  
>  	if (type & XFS_USER_QUOTA)
> -		state_qfilestat(fp, mount, XFS_USER_QUOTA, &s.qs_uquota,
> -				s.qs_flags & XFS_QUOTA_UDQ_ACCT,
> -				s.qs_flags & XFS_QUOTA_UDQ_ENFD);
> +		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, &s.qs_gquota,
> -				s.qs_flags & XFS_QUOTA_GDQ_ACCT,
> -				s.qs_flags & XFS_QUOTA_GDQ_ENFD);
> +		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, &s.qs_gquota,
> -				s.qs_flags & XFS_QUOTA_PDQ_ACCT,
> -				s.qs_flags & XFS_QUOTA_PDQ_ENFD);
> +		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, s.qs_btimelimit);
> -	state_timelimit(fp, XFS_INODE_QUOTA, s.qs_itimelimit);
> -	state_timelimit(fp, XFS_RTBLOCK_QUOTA, s.qs_rtbtimelimit);
> +	state_timelimit(fp, XFS_BLOCK_QUOTA, sv.qs_btimelimit);
> +	state_timelimit(fp, XFS_INODE_QUOTA, sv.qs_itimelimit);
> +	state_timelimit(fp, XFS_RTBLOCK_QUOTA, sv.qs_rtbtimelimit);
>  }
>  
>  static void
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2016-08-17 15:02 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-12 23:07 [PATCH] xfs_quota: wire up XFS_GETQSTATV Eric Sandeen
2016-08-15 13:44 ` Bill O'Donnell
2016-08-15 16:38 ` Zorro Lang
2016-08-15 16:43   ` Eric Sandeen
2016-08-16  3:17 ` [PATCH V2] " Eric Sandeen
2016-08-17  8:33   ` Zorro Lang
2016-08-17 14:43     ` Eric Sandeen
2016-08-17 14:58       ` Zorro Lang
2016-08-17 15:02   ` Zorro Lang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).