Re: [PATCH 6/8] fsfreeze: add vfs ioctl to check freeze state

[Fernando Luis Vazquez Cao <fernando_b1@lab.ntt.co.jp>]

On 2012/09/14 15:28, Dave Chinner wrote:
> On Fri, Sep 14, 2012 at 10:46:33AM +0900, Fernando Luis Vazquez Cao wrote:
>>> Indeed, what happens when the superblock freeze is driven from
>>> dm-snapshot, and the user unmounts the fs and runs the blockdev
>>> ioctl to drop the freeze reference that dm-snapshot holds?
>>> That would free the superblock out from underneath DM, so this is
>>> a can of worms I'd prefer not to open.
>> The prototype I wrote for the blockdev ioctl
>> grabs the bdev->bd_fsfreeze_mutex and checks
>> bdev->bd_fsfreeze_count. If bd_fsfreeze_count>1
>> it returns EBUSY, else it calls freeze_super so
>> that the thawed and subsequently released.
> So if I run FIFREEZE, then unmount, bdev->bd_fsfreeze_count is
> zero so BLKTHAW won't thaw the superblock. If you decide to ignore
> the bd_fsfreeze_count and thaw_super(), then you don't have any
> nesting sanity at all.

No. In such a case, since bdev_fsfreeze_count is 0 BLKTHAW
would call thaw_super and thaw the superblock. It is only
when bdev_fsfreeze_count>0 that BLKTHAW would return
EBUSY. BLKTHAW's job is to thaw a superblock sitting on top
of a block device not the block device itself and only if
the block device is not frozen.


>> I hope this addresses your concerns. That said
>> I am not married to this aproach,  if we decide
>> that filesystems should be thawed on
>> umount (which I was told is a no-go in a previous
>> LSF) I am willing to implement it.
> I don't recall it ever being discussed. If filesystems are supposed
> to maintain frozen state during mount and unmount (which is what
> this implies), then we've got a lot of work to do in every single
> filesystem to audit and ensure that the mount/unmount paths never
> write anything to disk...

I agree it is a lot of work.


> It's far simpler, and IMO much saner, simply to saw unmount == thaw.

This issue was discussed at LSF 2010 and we have a nice
report from that day written by Jon Corbet:

http://lwn.net/Articles/399148/

Al suggested that the right solution is creating a new
freeze command which is described in the article above.
I implemented this new API and posted the patches
last year:

http://www.spinics.net/lists/linux-fsdevel/msg47238.html

The consensus was that this is the best long-term solution,
but since there are tools based on the older ioctl() commands
we also need a new ioctl() command which will cause the
thawing of an unmounted filesystem.


>> However this breaks lazy umount and I was told that
>> auto-thaw on umount is not acceptable, so I came
>> with the current patch set.
> Silently corrupted snapshots due to unmount/mount when frozen is
> pretty bad, and there's no way around it right now.

I agree, we need to fix that too.


> FWIW: "I was told...": A reference to the discussion so I can find
> out the reasons why it is unacceptible would be handy....

The only thing I could find is the article linked
above. The main reason was that (from my
notes of the day): "Umount has no business thawing
a filesystem since it could interfere with a backup
process that is scheduled periodically for example.
As a different approach we could block umount
until the filesystem is thawed but the filesystem
can be left frozen for an undetermined amount
of time, so this behaviour was deemed undesirable
too."

I just implemented the behaviour people seemed
to prefer, but I am fine with either approach as
long as it is not broken and is documented properly
(and of course I am willing to do the work if
necessary).

I guess this is Al's call. Al, which solution do you want
us to implement?


>> The filesystem was frozen without the guest's users
>> being aware of it, so these users will find that writes to
>> the filesystem block and they have no way to know
>> what is going on because we do not have check ioctls.
> They should know what happened - the failure of the management
> operation or agent should have been reported to the administrator.
> It doesn't matter whether the filesystem is frozen or not - any
> failure of a automated management operation that coul daffect the
> operationof the guest VM shoul dbe raising alarms.  At that point,
> the owner ofthe guest will know what failed before they even start
> digging around in the guest VM and have some idea of what to look
> for and what to resolve.

The owner of the guest only knows that the guest agent
is dead and it has no knowledge about the state in which
the filesystem(s) were left.


>> In such cases I was told by costumers that they would
>> like to use either emergency thaw (which this patch set
>> attempts to fix) or a check ioctl.
> If they can run a check ioctl, then they don't need an emergency
> thaw - they can just run fsfreeze -u.

That is one of the reasons I want a check ioctl.
I agree that if we had check ioctls the emergency
thaw feature would be less important.


> Further, what those users want is to know if the filesystem is
> frozen, they don't *want* an ioctl.  For them, something in
> /proc/mount (e.g. add a "frozen" state to the mount options output)
> or /sys/block/<dev>/freeze_count for block level freezes. The
> information is easy to obtain and only requires cat to access.

Either API is fine with me (I even implemented both!).
The reason that I proposed the ioctl is that since
we use an ioctl to freeze/thaw the filesystem our
customers (and most of the of the people I discussed
this issue with) wanted me to implement the check
functionality as an ioctl too, for the sake of consistency.

Thanks,
Fernando