Re: [RFC v6 PATCH 1/5] xfs: Remove incore use of XFS_OQUOTA_ENFD and XFS_OQUOTA_CHKD

Dave Chinner
2012-08-14T22:46:50Z

On Fri, Jul 20, 2012 at 06:02:08PM -0500, Chandra Seetharaman wrote:
> 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....

> @@ -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:

>
> 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;

> + /*
> + * 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 & ....

> + ~(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:

....

> @@ -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))) {

> 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 <dchinner@redhat.com>

--
Dave Chinner
david@fromorbit.com