From: Chris Mason <clm@fb.com>
To: <dsterba@suse.cz>, Qu Wenruo <quwenruo@cn.fujitsu.com>,
<linux-btrfs@vger.kernel.org>, <harald@redhat.com>
Subject: Re: [RFC PATCH] Revert "btrfs: allow mounting btrfs subvolumes with different ro/rw options"
Date: Tue, 1 Jul 2014 12:36:01 -0400 [thread overview]
Message-ID: <53B2E371.6030504@fb.com> (raw)
In-Reply-To: <20140701153240.GL1553@twin.jikos.cz>
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
next prev parent reply other threads:[~2014-07-01 16:36 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-07-01 9:30 [RFC PATCH] Revert "btrfs: allow mounting btrfs subvolumes with different ro/rw options" Qu Wenruo
2014-07-01 15:32 ` David Sterba
2014-07-01 16:36 ` Chris Mason [this message]
2014-07-02 7:59 ` Harald Hoyer
2014-07-02 8:28 ` Duncan
2014-07-02 8:58 ` Qu Wenruo
2014-07-03 1:26 ` Chris Mason
2014-07-02 17:48 ` Goffredo Baroncelli
2014-07-03 0:28 ` Qu Wenruo
2014-07-03 8:06 ` Tobias Geerinckx-Rice
2014-07-03 8:33 ` Qu Wenruo
2014-07-03 11:26 ` Tobias Geerinckx-Rice
2014-07-03 17:37 ` Goffredo Baroncelli
2014-07-04 1:28 ` Qu Wenruo
2014-07-04 17:41 ` Goffredo Baroncelli
2014-07-07 1:46 ` Qu Wenruo
2014-07-07 17:37 ` Goffredo Baroncelli
2014-07-08 2:43 ` Duncan
2014-07-08 4:07 ` Goffredo Baroncelli
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=53B2E371.6030504@fb.com \
--to=clm@fb.com \
--cc=dsterba@suse.cz \
--cc=harald@redhat.com \
--cc=linux-btrfs@vger.kernel.org \
--cc=quwenruo@cn.fujitsu.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).