On Thu, Jun 02, 2016 at 06:58:33PM -0700, Darrick J. Wong wrote:
> On Fri, Jun 03, 2016 at 08:54:15AM +1000, Dave Chinner wrote:
> > On Thu, Jun 02, 2016 at 04:19:07PM +0200, Christoph Hellwig wrote:
> > > I've had some vocal user requests to allow enabling reflinks at run time,
> > > which happens to be a mostly trivial feature. The only caveat is that we
> > > need a large enough log size to support the reflink requirements, but for
> > > typical large file systems that's not an issue.
> >
> > Hmmm - how does this interact with all the rmap code? I was not
> > planning on enabling reflink without rmap and vice versa simply
> > because it makes the validation and testing matrix vastly more
> > complex. Indeed, having reflink turned on after a filesystem has
> > aged for some time (i.e. from unknown initial conditions) makes
> > validation especially tricky....
>
> Well...
>
> It's not strictly impossible, but there will be some problems running
> repair and remounting.
>
> The patchset doesn't actually check that we satisfy the minimum log
> space requirement, which will result in xfs refusing to mount. As
> Christoph says, this is only an issue on small FSes, but nevertheless,
> we shouldn't trap the user like that.
xfs_log_mount always verified the required log size.
> Second, mkfs lays out all the AG btree roots at the start of the AG
> before finding an aligned inode block for the root inode. xfs_repair
> feeds the same algorithm from the on-disk feature fields to check that
> s_rootino is sane, and gets very unhappy if it doesn't find the root
> inode at the computed location. Adding the two btree root blocks is
> enough to shift the root inode from 96 to 128. This all can be fixed,
> but it /was/ convenient not to have to support weirdo upgraded XFSes
> like ext4. :)
Yes, that algorithm might be unhappy, but I don't think it's overly
smart to rely on it either.
> Furthermore, if you turn on reflink, you should enable the per-AG
> reservations so we don't crash the FS by running out of space when it
> needs a block for the refcountbt.
xfs_refcountbt_calc_reserves is switched to use xfs_mp_hasreflink,
so it will do the right thing.
|