* [PATCH] xfs_repair: check and repair quota metadata
@ 2018-05-07 14:20 Eric Sandeen
2018-05-07 16:32 ` Darrick J. Wong
2018-05-07 23:43 ` [PATCH V2] " Eric Sandeen
0 siblings, 2 replies; 8+ messages in thread
From: Eric Sandeen @ 2018-05-07 14:20 UTC (permalink / raw)
To: linux-xfs
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@redhat.com>
---
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
+
/*
* 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 */
+#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 */
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH] xfs_repair: check and repair quota metadata
2018-05-07 14:20 [PATCH] xfs_repair: check and repair quota metadata Eric Sandeen
@ 2018-05-07 16:32 ` Darrick J. Wong
2018-05-07 16:44 ` Eric Sandeen
2018-05-07 23:28 ` Dave Chinner
2018-05-07 23:43 ` [PATCH V2] " Eric Sandeen
1 sibling, 2 replies; 8+ messages in thread
From: Darrick J. Wong @ 2018-05-07 16:32 UTC (permalink / raw)
To: Eric Sandeen; +Cc: linux-xfs
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@redhat.com>
> ---
>
> 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@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] xfs_repair: check and repair quota metadata
2018-05-07 16:32 ` Darrick J. Wong
@ 2018-05-07 16:44 ` Eric Sandeen
2018-05-07 16:59 ` Darrick J. Wong
2018-05-07 23:28 ` Dave Chinner
1 sibling, 1 reply; 8+ messages in thread
From: Eric Sandeen @ 2018-05-07 16:44 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs
On 5/7/18 11:32 AM, Darrick J. Wong wrote:
>> 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? :)
oh derp, I'll move that elsewhere, maybe the non-priv userspace
libxfs header, I'll see.
>> 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.
Probably not, I was grouping the internal special inodes together IIRC, but
happy just move them to the end if that's preferable.
-Eric
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] xfs_repair: check and repair quota metadata
2018-05-07 16:44 ` Eric Sandeen
@ 2018-05-07 16:59 ` Darrick J. Wong
0 siblings, 0 replies; 8+ messages in thread
From: Darrick J. Wong @ 2018-05-07 16:59 UTC (permalink / raw)
To: Eric Sandeen; +Cc: linux-xfs
On Mon, May 07, 2018 at 11:44:00AM -0500, Eric Sandeen wrote:
> On 5/7/18 11:32 AM, Darrick J. Wong wrote:
>
> >> 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? :)
>
> oh derp, I'll move that elsewhere, maybe the non-priv userspace
> libxfs header, I'll see.
<shrug> Seems like a trivial mod...
>
> >> 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.
>
> Probably not, I was grouping the internal special inodes together IIRC, but
> happy just move them to the end if that's preferable.
Ok. I wasn't sure if there was some hidden ordering requirement buried
in here, but at a first glance I didn't see a reason to renumber the
whole list.
--D
> -Eric
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] xfs_repair: check and repair quota metadata
2018-05-07 16:32 ` Darrick J. Wong
2018-05-07 16:44 ` Eric Sandeen
@ 2018-05-07 23:28 ` Dave Chinner
2018-05-07 23:30 ` Eric Sandeen
1 sibling, 1 reply; 8+ messages in thread
From: Dave Chinner @ 2018-05-07 23:28 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: Eric Sandeen, linux-xfs
On Mon, May 07, 2018 at 09:32:42AM -0700, Darrick J. Wong wrote:
> 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@redhat.com>
> > ---
> >
> > 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? :)
It's already defined in the kernel in fs/xfs/xfs_qm.h.
And userspace also defines it as a private define to allow libxfs to
build:
$ git grep XFS_DQUOT_CLUSTER_SIZE_FSB
libxfs/libxfs_priv.h:#define XFS_DQUOT_CLUSTER_SIZE_FSB (xfs_filblks_t)1
libxfs/xfs_trans_resv.c: XFS_FSB_TO_B(mp, XFS_DQUOT_CLUSTER_SIZE_FSB) - 1);
libxfs/xfs_trans_space.h: XFS_DQUOT_CLUSTER_SIZE_FSB)
$
If it's going to be used outside libxfs in userspace, then it should
be lifted to a libxfs header file both in the kernel and userspace,
such as fs/xfs/xfs_quota_defs.h....
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] xfs_repair: check and repair quota metadata
2018-05-07 23:28 ` Dave Chinner
@ 2018-05-07 23:30 ` Eric Sandeen
0 siblings, 0 replies; 8+ messages in thread
From: Eric Sandeen @ 2018-05-07 23:30 UTC (permalink / raw)
To: Dave Chinner, Darrick J. Wong; +Cc: linux-xfs
On 5/7/18 6:28 PM, Dave Chinner wrote:
> On Mon, May 07, 2018 at 09:32:42AM -0700, Darrick J. Wong wrote:
>> On Mon, May 07, 2018 at 09:20:28AM -0500, Eric Sandeen wrote:
...
>>> 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? :)
>
> It's already defined in the kernel in fs/xfs/xfs_qm.h.
>
> And userspace also defines it as a private define to allow libxfs to
> build:
>
> $ git grep XFS_DQUOT_CLUSTER_SIZE_FSB
> libxfs/libxfs_priv.h:#define XFS_DQUOT_CLUSTER_SIZE_FSB (xfs_filblks_t)1
> libxfs/xfs_trans_resv.c: XFS_FSB_TO_B(mp, XFS_DQUOT_CLUSTER_SIZE_FSB) - 1);
> libxfs/xfs_trans_space.h: XFS_DQUOT_CLUSTER_SIZE_FSB)
> $
>
> If it's going to be used outside libxfs in userspace, then it should
> be lifted to a libxfs header file both in the kernel and userspace,
> such as fs/xfs/xfs_quota_defs.h....
Ok, which is what I did in the first place. :) Ok, so can change it
on the kernel side to keep up with this patch, then.
Thanks,
-Eric
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH V2] xfs_repair: check and repair quota metadata
2018-05-07 14:20 [PATCH] xfs_repair: check and repair quota metadata Eric Sandeen
2018-05-07 16:32 ` Darrick J. Wong
@ 2018-05-07 23:43 ` Eric Sandeen
2018-05-07 23:54 ` Darrick J. Wong
1 sibling, 1 reply; 8+ messages in thread
From: Eric Sandeen @ 2018-05-07 23:43 UTC (permalink / raw)
To: Eric Sandeen, linux-xfs
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 quotacheck.
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
V2: don't renumber all the XR_INO_* defines
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
+
/*
* 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..e6b71272 100644
--- a/repair/incore.h
+++ b/repair/incore.h
@@ -229,6 +229,9 @@ int count_bcnt_extents(xfs_agnumber_t);
#define XR_INO_SOCK 9 /* socket */
#define XR_INO_FIFO 10 /* fifo */
#define XR_INO_MOUNTPOINT 11 /* mountpoint */
+#define XR_INO_UQUOTA 12 /* user quota inode */
+#define XR_INO_GQUOTA 13 /* group quota inode */
+#define XR_INO_PQUOTA 14 /* project quota inode */
/* inode allocation tree */
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH V2] xfs_repair: check and repair quota metadata
2018-05-07 23:43 ` [PATCH V2] " Eric Sandeen
@ 2018-05-07 23:54 ` Darrick J. Wong
0 siblings, 0 replies; 8+ messages in thread
From: Darrick J. Wong @ 2018-05-07 23:54 UTC (permalink / raw)
To: Eric Sandeen; +Cc: Eric Sandeen, linux-xfs
On Mon, May 07, 2018 at 06:43:17PM -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 quotacheck.
>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
--D
> ---
>
> V2: don't renumber all the XR_INO_* defines
>
> 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
> +
> /*
> * 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..e6b71272 100644
> --- a/repair/incore.h
> +++ b/repair/incore.h
> @@ -229,6 +229,9 @@ int count_bcnt_extents(xfs_agnumber_t);
> #define XR_INO_SOCK 9 /* socket */
> #define XR_INO_FIFO 10 /* fifo */
> #define XR_INO_MOUNTPOINT 11 /* mountpoint */
> +#define XR_INO_UQUOTA 12 /* user quota inode */
> +#define XR_INO_GQUOTA 13 /* group quota inode */
> +#define XR_INO_PQUOTA 14 /* project quota inode */
>
> /* inode allocation tree */
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-05-07 23:54 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-07 14:20 [PATCH] xfs_repair: check and repair quota metadata Eric Sandeen
2018-05-07 16:32 ` Darrick J. Wong
2018-05-07 16:44 ` Eric Sandeen
2018-05-07 16:59 ` Darrick J. Wong
2018-05-07 23:28 ` Dave Chinner
2018-05-07 23:30 ` Eric Sandeen
2018-05-07 23:43 ` [PATCH V2] " Eric Sandeen
2018-05-07 23:54 ` Darrick J. Wong
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.