Re: [RFC PATCH] Revert "btrfs: allow mounting btrfs subvolumes with different ro/rw options"

[Chris Mason <clm@fb.com>]

On 7/2/14, 3:59 AM, "Harald Hoyer" <harald@redhat.com> wrote:

>On 01.07.2014 18:36, Chris Mason wrote:
>> On 07/01/2014 11:32 AM, David Sterba wrote:
>>> (adding Harald to CC)
>>>
>>> On Tue, Jul 01, 2014 at 05:30:01PM +0800, Qu Wenruo wrote:
>>>> This reverts commit 0723a0473fb48a1c93b113a28665b64ce5faf35a.
>>>> This commit has the following problem:
>>>> 1) Break the ro mount rule.
>>>> When users mount the whole btrfs ro, it is still possible to mount
>>>> subvol rw and change the contents. Which make the whole fs ro mount
>>>> non-sense.
>>>
>>> The proposed usecase was to allow mounting subvolumes with different
>>> ro/rw flags, and this makes sense to me (provided that the whole
>>> filesystem is mounted rw).
>>>
>>> Anything else seems to lead to all the internal problems you point
>>> below. I'm not even sure if mounting the first subvolume 'ro' should
>>> imply that the whole filesystem is ro or not.
>>>
>>>> 2) Cause whole btrfs ro/rw mount change fails.
>>>> When mount a subvol ro first, when you can't mount the whole fs
>>>>mounted
>>>> rw. This is due to the check in btrfs_mount() which returns -EBUSY,
>>>> which is OK for single fs to prevent mount fs ro in one mount point
>>>>and
>>>> mount the same fs rw in other mount point.
>>>> Step to reproduce:
>>>> mount -o subvol=subv,ro /dev/sda6 /mnt/btrfs
>>>> mount -o rw /dev/sda6 /mnt/btrfs	<-this will fail
>>>
>>> Yeah, so first ro means whole filesystem is ro.
>>>
>>>> 3) Kernel warn in vfs.
>>>> When mount the whole fs ro, and mount a subvol ro, kernel warning will
>>>> show in fs/sync.c complaining s_umount rwsem is not locked.
>>>> Since this remount is not called by VFS, so s_mounts rwsem is not
>>>> correctly locked.
>>>
>>> That's serious.
>> 
>> Agreed, we'll pull this out until we get a better handle on things.
>> Thanks for spending time on it.
>> 
>> -chris
>> 
>
>My patch was initially only a request for comments:
>- patch pointed out the problem
>- provided a possible solution/workaround
>- even has "FIXME" in the code :)
>
>So, what I was hoping, that somebody else with more VFS knowledge than me
>would
>step up and come up with a sane solution.
>
>Pull it out, if the patch causes problems, but _please_ think about the
>problem
>and come up with a solution, so that "mount -a" works with a normal fstab.


Definitely, we don¹t want to drop this functionality over the long term.
The biggest problem here is the vfs warning.  Let me take another look and
see if I can get the locking right.

-chris