On Wed, May 12, 2021 at 05:46:15PM +0200, Javier Martinez Canillas wrote:
> From: Carlos Maiolino <cmaiol...@redhat.com>
>
> The XFS filesystem supports a bigtime feature to overcome y2038 problem.
> This patch makes the GRUB able to support the XFS filesystems with this
> feature enabled.
>
> The XFS counter for the bigtime enabled timestamps starts on 0, which
> translates to GRUB_INT32_MIN (Dec 31 20:45:52 UTC 1901) in the legacy
> timestamps. The conversion to unix timestamps is made before passing the
> value to other GRUB functions.
>
> For this to work properly, GRUB requires to access flags2 field in the
> XFS ondisk inode. So, the grub_xfs_inode structure has been updated to
> the full ondisk inode size.
s/the full ondisk inode size/cover full ondisk inode/?
> This patch is enough to make the GRUB work properly with files that have
> timestamps with values up to GRUB_INT32_MAX (Jan 19 03:14:07 UTC 2038).
This paragraph contradicts a bit what was said in the first paragraph...
> Any file with a timestamps bigger than this will overflow the counter,
> causing GRUB to show wrong timestamps. This is not really different than
> the current situation, since the timestamps in GRUB are 32-bit values.
Err... I think this is incorrect right now because the patch before this
one fixed the issue. So, this paragraph should be dropped. There is only
one question: are the changes here sufficient to have full XFS bigtime
support in the GRUB? I think they are...
> Signed-off-by: Carlos Maiolino <cmaiol...@redhat.com>
> Signed-off-by: Javier Martinez Canillas <javi...@redhat.com>
> ---
>
> Changes in v2:
> - Improve the commit message of patch #3 (dkiper).
> - Drop Reviewed-by tag from patch #3 (dkiper).
> _ Drop unrelated changes in patch #3 (dkiper).
>
> grub-core/fs/xfs.c | 63 +++++++++++++++++++++++++++++++++++++---------
> 1 file changed, 51 insertions(+), 12 deletions(-)
>
> diff --git a/grub-core/fs/xfs.c b/grub-core/fs/xfs.c
> index 43023e03fb3..bcf281cf80d 100644
> --- a/grub-core/fs/xfs.c
> +++ b/grub-core/fs/xfs.c
> @@ -75,10 +75,15 @@ GRUB_MOD_LICENSE ("GPLv3+");
> XFS_SB_VERSION2_PROJID32BIT | \
> XFS_SB_VERSION2_FTYPE)
>
> +/* Inode flags2 flags */
> +#define XFS_DIFLAG2_BIGTIME_BIT 3
> +#define XFS_DIFLAG2_BIGTIME (1 << XFS_DIFLAG2_BIGTIME_BIT)
> +
> /* incompat feature flags */
> #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
> */
>
> /*
> * Directory entries with ftype are explicitly handled by GRUB code.
> @@ -92,7 +97,8 @@ GRUB_MOD_LICENSE ("GPLv3+");
> #define XFS_SB_FEAT_INCOMPAT_SUPPORTED \
> (XFS_SB_FEAT_INCOMPAT_FTYPE | \
> XFS_SB_FEAT_INCOMPAT_SPINODES | \
> - XFS_SB_FEAT_INCOMPAT_META_UUID)
> + XFS_SB_FEAT_INCOMPAT_META_UUID | \
> + XFS_SB_FEAT_INCOMPAT_BIGTIME)
>
> struct grub_xfs_sblock
> {
> @@ -177,7 +183,7 @@ struct grub_xfs_btree_root
> grub_uint64_t keys[1];
> } GRUB_PACKED;
>
> -struct grub_xfs_time
> +struct grub_xfs_time_legacy
> {
> grub_uint32_t sec;
> grub_uint32_t nanosec;
> @@ -190,20 +196,23 @@ struct grub_xfs_inode
> grub_uint8_t version;
> grub_uint8_t format;
> grub_uint8_t unused2[26];
> - struct grub_xfs_time atime;
> - struct grub_xfs_time mtime;
> - struct grub_xfs_time ctime;
> + grub_uint64_t atime;
> + grub_uint64_t mtime;
> + grub_uint64_t ctime;
> grub_uint64_t size;
> grub_uint64_t nblocks;
> grub_uint32_t extsize;
> grub_uint32_t nextents;
> grub_uint16_t unused3;
> grub_uint8_t fork_offset;
> - grub_uint8_t unused4[17];
> + grub_uint8_t unused4[37];
> + grub_uint64_t flags2;
> + grub_uint8_t unused5[48];
> } GRUB_PACKED;
>
> -#define XFS_V2_INODE_SIZE sizeof(struct grub_xfs_inode)
> -#define XFS_V3_INODE_SIZE (XFS_V2_INODE_SIZE + 76)
> +#define XFS_V3_INODE_SIZE sizeof(struct grub_xfs_inode)
> +/* Size of struct grub_xfs_inode until fork_offset (included)*/
> +#define XFS_V2_INODE_SIZE (XFS_V3_INODE_SIZE - 92)
>
> struct grub_xfs_dirblock_tail
> {
> @@ -233,8 +242,6 @@ struct grub_xfs_data
>
> static grub_dl_t my_mod;
>
> -
> -
Could you drop this change?
> static int grub_xfs_sb_hascrc(struct grub_xfs_data *data)
> {
> return (data->sblock.version &
> grub_cpu_to_be16_compile_time(XFS_SB_VERSION_NUMBITS)) ==
> @@ -950,7 +957,6 @@ grub_xfs_mount (grub_disk_t disk)
> return 0;
> }
>
> -
Ditto...
> /* Context for grub_xfs_dir. */
> struct grub_xfs_dir_ctx
> {
> @@ -958,6 +964,39 @@ struct grub_xfs_dir_ctx
> void *hook_data;
> };
>
> +/* Bigtime inodes helpers */
> +
> +#define NSEC_PER_SEC 1000000000L
> +#define XFS_BIGTIME_EPOCH_OFFSET (-(grub_int64_t)GRUB_INT32_MIN)
I think starting from here you forgot to take into account my comments...
> +static int grub_xfs_inode_has_bigtime(const struct grub_xfs_inode *inode)
> +{
> + return inode->version >= 3 &&
> + (inode->flags2 & grub_cpu_to_be64_compile_time(XFS_DIFLAG2_BIGTIME));
> +}
> +
> +static grub_int64_t
> +grub_xfs_bigtime_to_unix(grub_uint64_t time)
> +{
> + grub_uint64_t rem;
> + grub_int64_t nsec = NSEC_PER_SEC;
> + grub_int64_t seconds = grub_divmod64((grub_int64_t)time, nsec, &rem);
> +
> + return seconds - XFS_BIGTIME_EPOCH_OFFSET;
> +}
> +
> +static grub_int64_t
> +grub_xfs_get_inode_time(struct grub_xfs_inode *inode)
> +{
> + struct grub_xfs_time_legacy *lts;
> +
> + if (grub_xfs_inode_has_bigtime(inode))
> + return grub_xfs_bigtime_to_unix(grub_be_to_cpu64(inode->mtime));
> +
> + lts = (struct grub_xfs_time_legacy *)&inode->mtime;
> + return grub_be_to_cpu32(lts->sec);
> +}
> +
> /* Helper for grub_xfs_dir. */
> static int
> grub_xfs_dir_iter (const char *filename, enum grub_fshelp_filetype filetype,
> @@ -970,7 +1009,7 @@ grub_xfs_dir_iter (const char *filename, enum
> grub_fshelp_filetype filetype,
> if (node->inode_read)
> {
> info.mtimeset = 1;
> - info.mtime = grub_be_to_cpu32 (node->inode.mtime.sec);
> + info.mtime = grub_xfs_get_inode_time(&node->inode);
> }
> info.dir = ((filetype & GRUB_FSHELP_TYPE_MASK) == GRUB_FSHELP_DIR);
> grub_free (node);
Daniel
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
