Hello Daniel,
On 5/17/21 4:09 PM, Daniel Kiper wrote:
> 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/?
>
Ok.
>> 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...
>
Yes, sorry about that. Originally Carlos' series had this patch before 1/4
and I re-arranged them in the series because made more sense to first add
the 64-bit support in GRUB core and then the XFS support. I'll drop this.
[snip]
>>
>> static grub_dl_t my_mod;
>>
>> -
>> -
>
Ok.
> 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...
>
Ok.
>> /* 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...
>
Oh, I did indeed miss these comments... sorry, I'll fix them in v3.
Best regards, --
Javier Martinez Canillas
Software Engineer
New Platform Technologies Enablement team
RHEL Engineering
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
