Re: [PATCH] fs: add the FIGETFROZEN ioctl call

[Dave Chinner]

On Mon, Apr 18, 2016 at 11:20:22AM -0400, Eric Sandeen wrote:
> 
> 
> On 4/14/16 10:17 PM, Dave Chinner wrote:
> > On Thu, Apr 14, 2016 at 09:57:07AM +0200, Florian Margaine wrote:
> >> This lets userland get the filesystem freezing status, aka whether the
> >> filesystem is frozen or not. This is so that an application can know if
> >> it should freeze the filesystem or if it isn't necessary when taking a
> >> snapshot.
> > 
> > freezing nests, so there is no reason for avoiding a freeze when
> > doing a snapshot. 
> 
> Sadly, no:
> 
> # xfs_freeze -f /mnt/test
> # xfs_freeze -f /mnt/test
> xfs_freeze: cannot freeze filesystem at /mnt/test: Device or resource busy
> 
> It used to, but it was broken^Wchanged quite some time ago.

Ugh. Block device freeze nesting still works (i.e. freeze_bdev, as
snapshots from DM would use), but I didn't realise (or had
forgetten) that superblock level freeze nesting had been removed...

> > Indeed, if you don't wrap freeze/thaw around a
> > snapshot, then if the fs is thawed while the snapshot is in progress
> > then you are going to get a corrupt snapshot....
> 
> Yep.
> 
> IMHO what really needs to happen is to fix freeze to allow nesting
> again.

Probably. I quick dig shows nesting was intentionally broken more
than 5 years ago in making the freeze ioctl work on btrfs. 

commit 18e9e5104fcd9a973ffe3eed3816c87f2a1b6cd2
Author: Josef Bacik <josef@xxxxxxxxxx>
Date:   Tue Mar 23 10:34:56 2010 -0400

    Introduce freeze_super and thaw_super for the fsfreeze ioctl
.....
    The only new gotcha is multiple calls to the fsfreeze ioctl will return EBUSY if
    the fs is already frozen.  I thought this was a better solution than adding a
    freeze counter to the super_block, but if everybody hates this idea I'm open to
    suggestions.  Thanks,
....

Not sure many people noticed that at the time....

> A way to query freeze state might be nice, I think, but yeah, it's
> racy, so you can't depend on it - but it might be useful in the "huh,
> IO is failing, what's going on?  Oh, it's frozen, ok" scenario...

So maybe we should just add the frozen state to /proc/self/mountinfo
or something similar, then people who think it matters can shoot
themselves in the foot all they want without us needing to care
about it.

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx