On Sun, Jun 23, 2013 at 09:48:25PM -0500, Chandra Seetharaman wrote:
> Start using pquotino and define a macro to check if the
> superblock has pquotino.
>
> Keep backward compatibilty by alowing mount of older superblock
> with no separate pquota inode.
>
> Signed-off-by: Chandra Seetharaman <sekharan@xxxxxxxxxx>
> ---
> fs/xfs/xfs_fsops.c | 3 +-
> fs/xfs/xfs_mount.c | 51 +++++++++++++++++++++++++++++++--------
> fs/xfs/xfs_qm.c | 22 +++++++++--------
> fs/xfs/xfs_qm_syscalls.c | 24 ++++++++++++++++--
> fs/xfs/xfs_quota.h | 8 ------
> fs/xfs/xfs_sb.h | 16 +++++++++++-
> fs/xfs/xfs_super.c | 14 ++++++----
> include/uapi/linux/dqblk_xfs.h | 1 +
> 8 files changed, 99 insertions(+), 40 deletions(-)
>
> diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c
> index 614eb0c..3bf05f4 100644
> --- a/fs/xfs/xfs_fsops.c
> +++ b/fs/xfs/xfs_fsops.c
> @@ -501,7 +501,8 @@ xfs_growfs_data_private(
> error, agno);
> break;
> }
> - xfs_sb_to_disk(XFS_BUF_TO_SBP(bp), &mp->m_sb, XFS_SB_ALL_BITS);
> + xfs_sb_to_disk(XFS_BUF_TO_SBP(bp), &mp->m_sb,
> + xfs_sb_all_bits(&mp->m_sb));
I think you could still pass XFS_SB_ALL_BITS to xfs_sb_to_disk(),
and do the XFS_SB_PQUOTINO filtering inside xfs_sb_quota_to_disk().
Actually xfs_sb_all_bits() is only used here, and it seems to me
that it is not necessary as we do a pquota support check inside
xfs_sb_to_disk() and can handle this in that place...
>
> /*
> * If we get an error writing out the alternate superblocks,
> diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
> index e2e14cb..bb7b23e 100644
> --- a/fs/xfs/xfs_mount.c
> +++ b/fs/xfs/xfs_mount.c
> @@ -336,12 +336,17 @@ xfs_mount_validate_sb(
> return XFS_ERROR(EWRONGFS);
> }
>
> - if ((sbp->sb_qflags & (XFS_OQUOTA_ENFD | XFS_OQUOTA_CHKD)) &&
> - (sbp->sb_qflags & (XFS_PQUOTA_ENFD | XFS_GQUOTA_ENFD |
> - XFS_PQUOTA_CHKD | XFS_GQUOTA_CHKD))) {
> - xfs_notice(mp,
> -"Super block has XFS_OQUOTA bits along with XFS_PQUOTA and/or XFS_GQUOTA bits.\n");
> - return XFS_ERROR(EFSCORRUPTED);
> + if (xfs_sb_version_has_pquota(sbp)) {
> + if (sbp->sb_qflags & (XFS_OQUOTA_ENFD | XFS_OQUOTA_CHKD)) {
> + xfs_notice(mp,
> + "Version 5 of Super block has XFS_OQUOTA bits.\n");
> + return XFS_ERROR(EFSCORRUPTED);
> + }
> + } else if (sbp->sb_qflags & (XFS_PQUOTA_ENFD | XFS_GQUOTA_ENFD |
> + XFS_PQUOTA_CHKD | XFS_GQUOTA_CHKD)) {
> + xfs_notice(mp,
> +"Superblock earlier than Version 5 has XFS_[PQ]UOTA_{ENFD|CHKD} bits.\n");
> + return XFS_ERROR(EFSCORRUPTED);
[ xfs_alert() for those, I think. ]
> @@ -638,6 +643,13 @@ xfs_handle_quota_to_disk(
> {
> __uint16_t qflags = from->sb_qflags;
>
> + /*
> + * We need to do these manipilations only if we are working
> + * with an older version of on-disk superblock.
> + */
> + if (xfs_sb_version_has_pquota(from))
> + return;
> +
> if (*fields & XFS_SB_QFLAGS) {
> /*
> * The in-core version of sb_qflags do not have
xfs_sb_all_bits() does:
if (xfs_sb_version_has_pquota(sbp))
return XFS_SB_ALL_BITS;
return XFS_SB_ALL_BITS & ~XFS_SB_PQUOTINO;
which means that we enter xfs_handle_quota_to_disk() with
XFS_SB_ALL_BITS intact for the pquota case, but without it in the
non-pquota case. Hence if we don't have pquota set, we continue
onwards and....
> @@ -657,6 +669,10 @@ xfs_handle_quota_to_disk(
> to->sb_qflags = cpu_to_be16(qflags);
> *fields &= ~XFS_SB_QFLAGS;
> }
> + if (*fields & XFS_SB_PQUOTINO) {
> + to->sb_gquotino = cpu_to_be64(from->sb_pquotino);
> + *fields &= ~XFS_SB_PQUOTINO;
> + }
We will never do this because we've already cleared XFS_SB_PQUOTINO
before we entered xfs_sb_to_disk(). That doesn't seem right to me...
> @@ -1524,8 +1524,10 @@ xfs_qm_init_quotainos(
> flags &= ~XFS_QMOPT_SBVERSION;
> }
> if (XFS_IS_PQUOTA_ON(mp) && pip == NULL) {
> + if (!xfs_sb_version_has_pquota(&mp->m_sb))
> + sbflags &= ~XFS_SB_GQUOTINO;
That's taken me a while to work out - it needs a comment explaining
why XFS_SB_GQUOTINO is being cleared here.
> error = xfs_qm_qino_alloc(mp, &pip,
> - sbflags | XFS_SB_GQUOTINO,
> + sbflags | XFS_SB_PQUOTINO,
> flags | XFS_QMOPT_PQUOTA);
> if (error)
> goto error_rele;
> diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
> index ed7cd55..d664a2d 100644
> --- a/fs/xfs/xfs_qm_syscalls.c
> +++ b/fs/xfs/xfs_qm_syscalls.c
> @@ -201,8 +201,7 @@ xfs_qm_scall_quotaoff(
> /*
> * If quotas is completely disabled, close shop.
> */
> - if (((flags & XFS_MOUNT_QUOTA_ALL) == XFS_MOUNT_QUOTA_SET1) ||
> - ((flags & XFS_MOUNT_QUOTA_ALL) == XFS_MOUNT_QUOTA_SET2)) {
> + if ((flags & XFS_MOUNT_QUOTA_ALL) == XFS_MOUNT_QUOTA_ALL) {
> mutex_unlock(&q->qi_quotaofflock);
> xfs_qm_destroy_quotainfo(mp);
> return (0);
This makes the assumption that userspace passes all three quota
types into the kernel in a single quota off call as the only way to
turn quotas off completely. What happens if they are turned off one
at a time? Shouldn't we detect when no more quotas are actually
enabled and then do this?
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
