On Mon, May 07, 2018 at 09:20:28AM -0500, Eric Sandeen wrote:
> Today, quota inodes are not checked at all in xfs_repair. (This is
> a little odd, because xfs_check used to do it in process_quota()).
>
> The kernel has quota inode validation and repair routines, but it is
> out of the ordinary for the kernel to be doing metadata repair. And
> now that we have metadata verifiers, this also yields a surprisingly
> noisy mount if quota inodes are corrupted, even immediately after an
> xfs_repair.
>
> So this patch allows xfs_repair to fix the quota inode metadata.
>
> Quotacheck is still left for the kernel. After a few more releases,
> I'll propose removing the repair calls from the kernel, and move the
> repair functions from libxfs/ into the repair code.
>
> Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxx>
> ---
>
> This will need a bit of adjustment when my other quota patches land
> in 4.18, but that should be no big deal.
>
> diff --git a/libxfs/libxfs_priv.h b/libxfs/libxfs_priv.h
> index ab195f5f..e5eb3de1 100644
> --- a/libxfs/libxfs_priv.h
> +++ b/libxfs/libxfs_priv.h
> @@ -196,8 +196,6 @@ enum ce { CE_DEBUG, CE_CONT, CE_NOTE, CE_WARN, CE_ALERT, CE_PANIC };
> # define barrier() __memory_barrier()
> #endif
>
> -#define XFS_DQUOT_CLUSTER_SIZE_FSB (xfs_filblks_t)1
> -
> /* miscellaneous kernel routines not in user space */
> #define down_read(a) ((void) 0)
> #define up_read(a) ((void) 0)
> diff --git a/libxfs/xfs_quota_defs.h b/libxfs/xfs_quota_defs.h
> index bb1b13a9..067475e2 100644
> --- a/libxfs/xfs_quota_defs.h
> +++ b/libxfs/xfs_quota_defs.h
> @@ -30,6 +30,8 @@
> typedef uint64_t xfs_qcnt_t;
> typedef uint16_t xfs_qwarncnt_t;
>
> +#define XFS_DQUOT_CLUSTER_SIZE_FSB (xfs_filblks_t)1
Requires a kernel patch, right? :)
> +
> /*
> * flags for q_flags field in the dquot.
> */
> diff --git a/repair/dinode.c b/repair/dinode.c
> index 9af4f058..a4887f8c 100644
> --- a/repair/dinode.c
> +++ b/repair/dinode.c
> @@ -1277,6 +1277,118 @@ null_check(char *name, int length)
> return(0);
> }
>
> +/*
> + * This does /not/ do quotacheck, it validates the basic quota
> + * inode metadata, checksums, etc.
> + */
> +#define uuid_equal(s,d) (platform_uuid_compare((s),(d)) == 0)
> +static int
> +process_quota_inode(
> + struct xfs_mount *mp,
> + xfs_ino_t lino,
> + struct xfs_dinode *dino,
> + uint ino_type,
> + struct blkmap *blkmap)
> +{
> + xfs_fsblock_t fsbno;
> + struct xfs_buf *bp;
> + xfs_filblks_t dqchunklen;
> + uint dqperchunk;
> + int quota_type;
> + char *quota_string;
> + xfs_dqid_t dqid;
> + xfs_fileoff_t qbno;
> + int i;
> + int t = 0;
> +
> + switch (ino_type) {
> + case XR_INO_UQUOTA:
> + quota_type = XFS_DQ_USER;
> + quota_string = _("User quota");
> + break;
> + case XR_INO_GQUOTA:
> + quota_type = XFS_DQ_GROUP;
> + quota_string = _("Group quota");
> + break;
> + case XR_INO_PQUOTA:
> + quota_type = XFS_DQ_PROJ;
> + quota_string = _("Project quota");
> + break;
> + default:
> + ASSERT(0);
> + }
> +
> + dqchunklen = XFS_FSB_TO_BB(mp, XFS_DQUOT_CLUSTER_SIZE_FSB);
> + dqperchunk = xfs_calc_dquots_per_chunk(dqchunklen);
> + dqid = 0;
> + qbno = NULLFILEOFF;
> +
> + while ((qbno = blkmap_next_off(blkmap, qbno, &t)) != NULLFILEOFF) {
> + xfs_dqblk_t *dqb;
> + int writebuf = 0;
> +
> + fsbno = blkmap_get(blkmap, qbno);
> + dqid = (xfs_dqid_t)qbno * dqperchunk;
> +
> + bp = libxfs_readbuf(mp->m_dev, XFS_FSB_TO_DADDR(mp, fsbno),
> + dqchunklen, 0, &xfs_dquot_buf_ops);
> + if (!bp) {
> + do_warn(
> +_("cannot read inode %" PRIu64 ", file block %" PRIu64 ", disk block %" PRIu64 "\n"),
> + lino, qbno, fsbno);
> + return 1;
> + }
> +
> + dqb = bp->b_addr;
> + for (i = 0; i < dqperchunk; i++, dqid++, dqb++) {
> + xfs_failaddr_t fa;
> + int bad_dqb = 0;
> +
> + /* We only print the first problem we find */
> + if (xfs_sb_version_hascrc(&mp->m_sb)) {
> + if (!xfs_verify_cksum((char *)dqb, sizeof(*dqb),
> + XFS_DQUOT_CRC_OFF)) {
> + do_warn(_("%s: bad CRC for id %u. "),
> + quota_string, dqid);
> + bad_dqb = 1;
> + goto bad;
> + }
> +
> + if (!uuid_equal(&dqb->dd_uuid,
> + &mp->m_sb.sb_meta_uuid)) {
> + do_warn(_("%s: bad UUID for id %u. "),
> + quota_string, dqid);
> + bad_dqb = 1;
> + goto bad;
> + }
> + }
> + fa = xfs_dquot_verify(mp, &dqb->dd_diskdq, dqid, quota_type, 0);
> + if (fa) {
> + do_warn(_("%s: Corrupt quota for id %u. "),
> + quota_string, dqid);
> + bad_dqb = 1;
> + }
> +
> +bad:
> + if (bad_dqb) {
> + if (no_modify)
> + do_warn(_("Would correct.\n"));
> + else {
> + do_warn(_("Corrected.\n"));
> + xfs_dquot_repair(mp, &dqb->dd_diskdq, dqid, quota_type);
> + writebuf = 1;
> + }
> + }
> + }
> +
> + if (writebuf && !no_modify)
> + libxfs_writebuf(bp, 0);
> + else
> + libxfs_putbuf(bp);
> + }
> + return 0;
> +}
> +
> static int
> process_symlink_remote(
> struct xfs_mount *mp,
> @@ -1479,6 +1591,13 @@ _("size of socket inode %" PRIu64 " != 0 (%" PRId64 " bytes)\n"), lino,
> _("size of fifo inode %" PRIu64 " != 0 (%" PRId64 " bytes)\n"), lino,
> (int64_t)be64_to_cpu(dino->di_size));
> break;
> + case XR_INO_UQUOTA:
> + case XR_INO_GQUOTA:
> + case XR_INO_PQUOTA:
> + do_warn(
> +_("size of quota inode %" PRIu64 " != 0 (%" PRId64 " bytes)\n"), lino,
> + (int64_t)be64_to_cpu(dino->di_size));
> + break;
> default:
> do_warn(_("Internal error - process_misc_ino_types, "
> "illegal type %d\n"), type);
> @@ -1599,7 +1718,7 @@ process_check_sb_inodes(
> int *dirty)
> {
> if (lino == mp->m_sb.sb_rootino) {
> - if (*type != XR_INO_DIR) {
> + if (*type != XR_INO_DIR) {
> do_warn(_("root inode %" PRIu64 " has bad type 0x%x\n"),
> lino, dinode_fmt(dinoc));
> *type = XR_INO_DIR;
> @@ -1613,7 +1732,7 @@ process_check_sb_inodes(
> return 0;
> }
> if (lino == mp->m_sb.sb_uquotino) {
> - if (*type != XR_INO_DATA) {
> + if (*type != XR_INO_UQUOTA) {
> do_warn(_("user quota inode %" PRIu64 " has bad type 0x%x\n"),
> lino, dinode_fmt(dinoc));
> mp->m_sb.sb_uquotino = NULLFSINO;
> @@ -1622,7 +1741,7 @@ process_check_sb_inodes(
> return 0;
> }
> if (lino == mp->m_sb.sb_gquotino) {
> - if (*type != XR_INO_DATA) {
> + if (*type != XR_INO_GQUOTA) {
> do_warn(_("group quota inode %" PRIu64 " has bad type 0x%x\n"),
> lino, dinode_fmt(dinoc));
> mp->m_sb.sb_gquotino = NULLFSINO;
> @@ -1631,7 +1750,7 @@ process_check_sb_inodes(
> return 0;
> }
> if (lino == mp->m_sb.sb_pquotino) {
> - if (*type != XR_INO_DATA) {
> + if (*type != XR_INO_PQUOTA) {
> do_warn(_("project quota inode %" PRIu64 " has bad type 0x%x\n"),
> lino, dinode_fmt(dinoc));
> mp->m_sb.sb_pquotino = NULLFSINO;
> @@ -1738,6 +1857,14 @@ _("directory inode %" PRIu64 " has bad size %" PRId64 "\n"),
> return 1;
> break;
>
> + case XR_INO_UQUOTA:
> + case XR_INO_GQUOTA:
> + case XR_INO_PQUOTA:
> + /* Quota inodes have same restrictions as above types */
> + if (process_misc_ino_types(mp, dino, lino, type))
> + return 1;
> + break;
> +
> case XR_INO_RTDATA:
> /*
> * if we have no realtime blocks, any inode claiming
> @@ -2620,6 +2747,12 @@ _("bad (negative) size %" PRId64 " on inode %" PRIu64 "\n"),
> type = XR_INO_RTBITMAP;
> else if (lino == mp->m_sb.sb_rsumino)
> type = XR_INO_RTSUM;
> + else if (lino == mp->m_sb.sb_uquotino)
> + type = XR_INO_UQUOTA;
> + else if (lino == mp->m_sb.sb_gquotino)
> + type = XR_INO_GQUOTA;
> + else if (lino == mp->m_sb.sb_pquotino)
> + type = XR_INO_PQUOTA;
> else
> type = XR_INO_DATA;
> break;
> @@ -2784,6 +2917,15 @@ _("Cannot have CoW extent size of zero on cowextsize inode %" PRIu64 ", "),
> goto clear_bad_out;
> }
> break;
> + case XR_INO_UQUOTA:
> + case XR_INO_GQUOTA:
> + case XR_INO_PQUOTA:
> + if (process_quota_inode(mp, lino, dino, type, dblkmap) != 0) {
> + do_warn(
> + _("problem with quota inode %" PRIu64 "\n"), lino);
> + goto clear_bad_out;
> + }
> + break;
> default:
> break;
> }
> diff --git a/repair/incore.h b/repair/incore.h
> index fd66084f..78a0ed30 100644
> --- a/repair/incore.h
> +++ b/repair/incore.h
> @@ -222,13 +222,16 @@ int count_bcnt_extents(xfs_agnumber_t);
> #define XR_INO_RTDATA 2 /* realtime file */
> #define XR_INO_RTBITMAP 3 /* realtime bitmap inode */
> #define XR_INO_RTSUM 4 /* realtime summary inode */
> -#define XR_INO_DATA 5 /* regular file */
> -#define XR_INO_SYMLINK 6 /* symlink */
> -#define XR_INO_CHRDEV 7 /* character device */
> -#define XR_INO_BLKDEV 8 /* block device */
> -#define XR_INO_SOCK 9 /* socket */
> -#define XR_INO_FIFO 10 /* fifo */
> -#define XR_INO_MOUNTPOINT 11 /* mountpoint */
> +#define XR_INO_UQUOTA 5 /* user quota inode */
> +#define XR_INO_GQUOTA 6 /* group quota inode */
> +#define XR_INO_PQUOTA 7 /* project quota inode */
/me wonders, is there an advantage/requirement to insert the three quota
inode types in the middle of the range instead of tacking them on the
end?
Anyway the rest mostly looks ok to me.
--D
> +#define XR_INO_DATA 8 /* regular file */
> +#define XR_INO_SYMLINK 9 /* symlink */
> +#define XR_INO_CHRDEV 10 /* character device */
> +#define XR_INO_BLKDEV 11 /* block device */
> +#define XR_INO_SOCK 12 /* socket */
> +#define XR_INO_FIFO 13 /* fifo */
> +#define XR_INO_MOUNTPOINT 14 /* mountpoint */
>
> /* inode allocation tree */
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
