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

[Javier Martinez Canillas]

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

> 
> 
>>
>>> +          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.

Best regards,
-- 
Javier Martinez Canillas
Software Engineer - Desktop Hardware Enablement
Red Hat