On Mon, Nov 18, 2019 at 10:22 AM Dave Chinner <david@xxxxxxxxxxxxx> wrote:
>
> On Mon, Nov 18, 2019 at 06:52:39AM +0200, Amir Goldstein wrote:
> > > >
> > > > I wonder if your version has struct xfs_dinode_v3 or it could avoid it.
> > > > There is a benefit in terms of code complexity and test coverage
> > > > to keep the only difference between inode versions in the on-disk
> > > > parsers, while reading into the same struct, the same way as
> > > > old inode versions are read into struct xfs_dinode.
> > > >
> > > > Oh well, I can wait for tomorrow to see the polished version :-)
> > >
> > > Well now we noticed that Arnd also changed the disk quota structure
> > > format too, so that'll slow things down as we try to figure out how to
> > > reconcile 34-bit inode seconds vs. 40-bit quota timer seconds.
> > >
> > > (Or whatever happens with that)
> > >
> >
> > Sigh. FWIW, I liked Arnd's 40-bit inode time patch because it
> > keeps the patch LoC for this conversion minimal.
>
> We can extend the quota warning range without changing the on-disk
> structures, and with much less code than changing the on-disk
> structures.
>
> We only need a ~500 year range for the warning expiry timestamp, and
> we don't really care about fine grained resolution of the timer
> expiry.
>
> We've already got a 70 year range with the signed second counter. So
> let's just redefine the timeout value on disk to use units of 10s
> instead of 1s when the bigtime superblock feature bit is set. ANd
> now we have our >500 year range requirement.
>
> That shouldn't need much more than 5-10 lines of new code
> translating the units when we read/write them from/to disk....
>
Sounds good.
What is your take on the issue of keeping struct xfs_dinode
and struct xfs_log_dinode common to v3..v4?
If we make struct xfs_timestamp_t/xfs_ictimestamp_t a union
of {{t_sec32;t_nsec32}, {t_nsec64}} then xfs_log_dinode_to_disk()
conversion code is conditional to di_version.
If we store v4 on-disk as {t_nsec32_hi;t_nsec32_lo} then the
conversion code from disk to log is unconditional to di_version.
Am I overthinking this?
Darrick,
I am assuming you are working on the patch.
If you would like me to re-post my patch with the decided
on-disk formats for inode and a quota patch let me know.
Thanks,
Amir.
