Re: [PATCH] xfs: Print a warning if the filesystem needs to be repaired

[Daniel Kiper]

On Thu, Apr 22, 2021 at 12:15:18PM +0200, Javier Martinez Canillas wrote:
> On 4/22/21 11:20 AM, Carlos Maiolino wrote:
> > Hi Daniel
> >
> >>> --- 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/
> >
> > I believe Javier got this from kernel, I wonder if it doesn't make sense to 
> > keep
> > the flags names consistent with kernel?
>
> That's correct, I copied the macro name from the kernel and agree
> with Carlos that's better to keep it the same. It makes much easier
> to grep the kernel source code when looking for something:
>
> #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_BIGTIME  (1 << 3)        /* large timestamps */
> #define XFS_SB_FEAT_INCOMPAT_NEEDSREPAIR (1 << 4)     /* needs xfs_repair */
>
> https://elixir.bootlin.com/linux/latest/source/fs/xfs/libxfs/xfs_format.h#L471

If this comes from the kernel then I am OK with it.

> >>> +          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?
> >
> > No, only xfs V5 superblock supports needsrepair flag.
>
> Indeed, and there's no plan for a XFS v5+ in the near future as was
> explained to me by Carlos. I also copied the logic to check from the
> Linux kernel which uses '==' for all the flags supported only in v5.

OK, then just repost the patch with minor fixes...

Daniel