On Thu, Aug 27, 2020 at 9:51 AM Christoph Hellwig <hch@xxxxxxxxxxxxx> wrote:
>
> > + */
> > +static inline xfs_ictimestamp_t
> > +xfs_inode_to_log_dinode_ts(
> > + const struct timespec64 tv)
> > +{
> > + uint64_t t;
> > +
> > +#ifdef __LITTLE_ENDIAN
> > + t = ((uint64_t)tv.tv_nsec << 32) | ((uint64_t)tv.tv_sec & 0xffffffff);
> > +#elif __BIG_ENDIAN
> > + t = ((int64_t)tv.tv_sec << 32) | ((uint64_t)tv.tv_nsec & 0xffffffff);
> > +#else
> > +# error System is neither little nor big endian?
> > +#endif
> > + return t;
>
> Looking at this I wonder if we should just keep the struct and cast
> to it locally in the conversion functions, as that should take
> care of everything. Or just keep the union from the previous version,
> sorry..
Looking at this my eyes pop out.
I realize that maintaining on-disk format of the log is challenging,
so if there is no other technical solution that will be easier for humans
to review and maintain going forward, I will step back and let others
review this code.
But it bears the question: do we have to support replaying on BE a
log that was recorded on LE? Especially with so little BE machines
around these days, this sounds like over design to me.
Wouldn't it be better just to keep a bit in the log if it is LE or BE and refuse
to replay it on the wrong architecture?
Sure, we need to support whatever we supported up to now, but "bigtime"
can require a new incompat log feature "host order" (or something).
Anyway, I am probably mumbling utter garbage, do feel free to ignore
or set me straight.
Thanks,
Amir.
