will fix them as suggested.

Thanks for the review.

Chandra
On Wed, 2012-08-15 at 08:46 +1000, Dave Chinner wrote:

On Fri, Jul 20, 2012 at 06:02:08PM -0500, Chandra Seetharaman wrote:

quoted

Remove all incore use of XFS_OQUOTA_ENFD and XFS_OQUOTA_CHKD. Instead,
start using XFS_GQUOTA_.* XFS_PQUOTA_.* counterparts for GQUOTA nd
PQUOTA respectively.

No changes are made to the on-disk version of the superblock yet. On-disk
copy still uses XFS_OQUOTA_ENFD and XFS_OQUOTA_CHKD.

Read and write of the superblock does the conversion from *OQUOTA*
to *[PG]QUOTA*.

Couple of minor style things....

quoted

@@ -622,6 +636,7 @@ xfs_sb_to_disk(
 	xfs_sb_field_t	f;
 	int		first;
 	int		size;
+	 __uint16_t	tmp16;

tmp16 is a bad name for a temporary variable. Move it to the scope
that uses it, and name it for it's purpose. i.e:

quoted

 
 	ASSERT(fields);
 	if (!fields)

@@ -636,6 +651,26 @@ xfs_sb_to_disk(
 
 		if (size == 1 || xfs_sb_info[f].type == 1) {
 			memcpy(to_ptr + first, from_ptr + first, size);
+		} else if (f == XFS_SBS_QFLAGS) {

			__uint16_t	qflags;

quoted

+			/*
+			 * The in-core version of sb_qflags do not have
+			 * XFS_OQUOTA_* flags, whereas the on-disk version
+			 * does.  Save the in-core sb_qflags temporarily,
+			 * removing the new XFS_{PG}QUOTA_* flags and re-apply
+			 * the old on-disk flags.
+			 */
+			tmp16 = from->sb_qflags &

			qflags = from->sb_qflags & ....

quoted

+					~(XFS_PQUOTA_ENFD | XFS_PQUOTA_CHKD |
+					XFS_GQUOTA_ENFD | XFS_GQUOTA_CHKD);
+
+			if (from->sb_qflags &
+					(XFS_PQUOTA_ENFD | XFS_GQUOTA_ENFD))
+				tmp16 |= XFS_OQUOTA_ENFD;
+			if (from->sb_qflags &
+					(XFS_PQUOTA_CHKD | XFS_GQUOTA_CHKD))
+				tmp16 |= XFS_OQUOTA_CHKD;
+
+			*(__be16 *)(to_ptr + first) = cpu_to_be16(tmp16);
 		} else {
 			switch (size) {
 			case 2:

....

quoted

@@ -339,9 +339,11 @@ xfs_qm_scall_quotaon(
 	    ||
 	    ((flags & XFS_PQUOTA_ACCT) == 0 &&
 	    (mp->m_sb.sb_qflags & XFS_PQUOTA_ACCT) == 0 &&
-	    (flags & XFS_GQUOTA_ACCT) == 0 &&
+	    (flags & XFS_PQUOTA_ENFD))
+	    ||
+	    ((flags & XFS_GQUOTA_ACCT) == 0 &&
 	    (mp->m_sb.sb_qflags & XFS_GQUOTA_ACCT) == 0 &&
-	    (flags & XFS_OQUOTA_ENFD))) {
+	    (flags & XFS_GQUOTA_ENFD))) {

Can you fix the flat indenting here at the same time so that the
logic is obvious at a glance and consistent with the rest of the XFS
code? i.e.

	if (((flags & XFS_UQUOTA_ACCT) == 0 &&
	     (mp->m_sb.sb_qflags & XFS_UQUOTA_ACCT) == 0 &&
	     (flags & XFS_UQUOTA_ENFD)) ||
	    ((flags & XFS_PQUOTA_ACCT) == 0 &&
	     (mp->m_sb.sb_qflags & XFS_PQUOTA_ACCT) == 0 &&
	     (flags & XFS_PQUOTA_ENFD)) ||
	    (flags & XFS_GQUOTA_ACCT) == 0 &&
	     (mp->m_sb.sb_qflags & XFS_GQUOTA_ACCT) == 0 &&
	     (flags & XFS_GQUOTA_ENFD))) {

quoted

 		xfs_debug(mp,
 			"%s: Can't enforce without acct, flags=%x sbflags=%x\n",
 			__func__, flags, mp->m_sb.sb_qflags);

@@ -771,8 +773,10 @@ xfs_qm_scall_getquota(
 	 * so return zeroes in that case.
 	 */
 	if ((!XFS_IS_UQUOTA_ENFORCED(mp) && dqp->q_core.d_flags == XFS_DQ_USER) ||
-	    (!XFS_IS_OQUOTA_ENFORCED(mp) &&
-			(dqp->q_core.d_flags & (XFS_DQ_PROJ | XFS_DQ_GROUP)))) {
+	    (!XFS_IS_PQUOTA_ENFORCED(mp)
+			&& dqp->q_core.d_flags == XFS_DQ_PROJ) ||
+	    (!XFS_IS_GQUOTA_ENFORCED(mp)
+			&& dqp->q_core.d_flags == XFS_DQ_GROUP)) {

Same here:

	if ((!XFS_IS_UQUOTA_ENFORCED(mp) &&
	     dqp->q_core.d_flags == XFS_DQ_USER) ||
	    (!XFS_IS_PQUOTA_ENFORCED(mp) &&
	     dqp->q_core.d_flags == XFS_DQ_PROJ) ||
	    (!XFS_IS_GQUOTA_ENFORCED(mp) &&
	     dqp->q_core.d_flags == XFS_DQ_GROUP)) {

Otherwise it looks good. Fix the above and you can add a

Reviewed-by: Dave Chinner <redacted>

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