On Mon, Apr 19, 2021 at 05:16:50PM +0200, Javier Martinez Canillas wrote:
> XFS now has an incompat feature flag to indicate that the filesystem needs
> to be repaired. The Linux kernel refuses to mount a filesystem that has it
> set and only the xfs_repair tool is able to clear that flag.
>
> One option is to make the GRUB behaviour consistent with the Linux kernel,
> and don't allow to mount a XFS filesystem that needs to be repaired. But
> that will prevent to even boot to a rescue environment for the OS in order
> to repair the filesystem.
>
> To prevent this situation, let's GRUB attempt to mount the filesystem even
> when needs to be repaired. Since after all, it currently attempts to mount
> even if is in an inconsistent state without trying to replay the jornal.
>
> For this reason, make it a best effort but print an error message when the
> filesystem is mounted if needs to be repaired. That way, if booting fails
s/filesystem is mounted if needs to be repaired/filesystem is mounted and
should be repaired/
> later due the inconsistencies when traversing the filesystem, the user can
> understand why that and take any needed action.
s/that/that happened/
> Suggested-by: Eric Sandeen <esand...@redhat.com>
> Signed-off-by: Javier Martinez Canillas <javi...@redhat.com>
> ---
>
> grub-core/fs/xfs.c | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/grub-core/fs/xfs.c b/grub-core/fs/xfs.c
> index 43023e03fb3..22e7e61d574 100644
> --- a/grub-core/fs/xfs.c
> +++ b/grub-core/fs/xfs.c
> @@ -79,6 +79,7 @@ GRUB_MOD_LICENSE ("GPLv3+");
> #define XFS_SB_FEAT_INCOMPAT_FTYPE (1 << 0) /* filetype in
> dirent */
> #define XFS_SB_FEAT_INCOMPAT_SPINODES (1 << 1) /* sparse inode
> chunks */
> #define XFS_SB_FEAT_INCOMPAT_META_UUID (1 << 2) /* metadata UUID */
> +#define XFS_SB_FEAT_INCOMPAT_NEEDSREPAIR (1 << 4) /* needs xfs_repair
> */
s/XFS_SB_FEAT_INCOMPAT_NEEDSREPAIR/XFS_SB_FEAT_INCOMPAT_NEEDS_REPAIR/
> /*
> * Directory entries with ftype are explicitly handled by GRUB code.
> @@ -300,6 +301,16 @@ static int grub_xfs_sb_valid(struct grub_xfs_data *data)
> return 0;
> }
>
> +static int
> +grub_xfs_sb_needsrepair(struct grub_xfs_data *data)
s/grub_xfs_sb_needsrepair/xfs_sb_needs_repair/
> +{
> + return ((data->sblock.version &
> + grub_cpu_to_be16_compile_time(XFS_SB_VERSION_NUMBITS)) ==
Missing space before "(" and below...
> + grub_cpu_to_be16_compile_time(XFS_SB_VERSION_5) &&
Hmmm... Is XFS_SB_FEAT_INCOMPAT_NEEDSREPAIR flag available for
XFS_SB_VERSION_5 only? Should not we use ">=" instead of "=="?
Does not older XFS versions support this flag?
> + data->sblock.sb_features_incompat &
> + grub_cpu_to_be32_compile_time(XFS_SB_FEAT_INCOMPAT_NEEDSREPAIR));
I would add more parenthesis here...
> +}
> +
> /* Filetype information as used in inodes. */
> #define FILETYPE_INO_MASK 0170000
> #define FILETYPE_INO_REG 0100000
> @@ -915,6 +926,11 @@ grub_xfs_mount (grub_disk_t disk)
> if (!grub_xfs_sb_valid(data))
> goto fail;
>
> + if (grub_xfs_sb_needsrepair(data))
> + {
> + grub_printf (N_("Filesystem needs repair. Please run a XFS repair
> tool"));
> + }
Please drop these curly brackets here...
Daniel
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
