diff mbox

feature request: consider rw subvols ro for send when volume is mounted ro

Message ID 20140723204736.GD17798@lenny.home.zabbo.net (mailing list archive)
State New, archived
Headers show

Commit Message

Zach Brown July 23, 2014, 8:47 p.m. UTC
On Wed, Jul 23, 2014 at 02:10:29PM -0600, Chris Murphy wrote:
> The use case is when it's possible to mount a Btrfs volume ro, but not rw. Example, a situation where
> 
> # mount -o degraded /dev/sdb /mnt
> [   71.064352] BTRFS info (device sdb): allowing degraded mounts
> [   71.064812] BTRFS info (device sdb): enabling auto recovery
> [   71.065210] BTRFS info (device sdb): disk space caching is enabled
> [   71.072068] BTRFS warning (device sdb): devid 2 missing
> [   71.097320] BTRFS: too many missing devices, writeable mount is not allowed
> [   71.116616] BTRFS: open_ctree failed
> 
> Yet this works:
> # mount -o degraded,ro /dev/sdb /mnt
> 
> It would be great if it were possible to send/receive subvolumes to a
> different btrfs volume. Currently it's not possible because those
> subvols aren't ro, and because the mount is ro I can't make ro
> snapshots first.

I wonder if that's as easy as the following totally untested hack.  I
have no idea if a read-only mount would still allow background
modification that might violate the send code's assumptions.

- z

$ git diff fs/btrfs/send.c
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Duncan July 24, 2014, 1:38 a.m. UTC | #1
Zach Brown posted on Wed, 23 Jul 2014 13:47:36 -0700 as excerpted:

> On Wed, Jul 23, 2014 at 02:10:29PM -0600, Chris Murphy wrote:
>> The use case is when it's possible to mount a Btrfs volume ro, but not
>> rw. Example, a situation where
>> 
>> # mount -o degraded /dev/sdb /mnt
>> [...] BTRFS: too many missing devices, writeable mount is not allowed 
>> 
>> Yet this works:
>> # mount -o degraded,ro /dev/sdb /mnt
>> 
>> It would be great if it were possible to send/receive subvolumes to a
>> different btrfs volume. Currently it's not possible because those
>> subvols aren't ro, and because the mount is ro I can't make ro
>> snapshots first.

In general, btrfs send/receive is great when it works, but because 
there's still corner-cases like this as well as simply broken send/
receive cases popping up from time to time, I strongly recommend not 
relying on it working, and keeping a more conventional backup option 
(like rsync) tested-working and usable as well.
 
> I wonder if that's as easy as the following totally untested hack.  I
> have no idea if a read-only mount would still allow background
> modification that might violate the send code's assumptions.

Hopefully that hack works.

Meanwhile, AFAIK, yes, there's still cases where a read-only mount can 
still allow background mods that would violate send's assumptions.  Tho I 
don't believe they apply in this case.  But certainly, there has been 
recent discussion on the subvolume mount situation, since it's possible 
to access child subvolumes from writable-mount parent (including root/
id5) subvolumes, and currently nothing stops writing into the read-only-
child's mount from the parent's writable mount.

Even without that situation, however, there's bind-mounts, which start 
out with the same mount options as the original, but with a remount allow 
one of the views to be read-only while the other is writable, regardless 
of the filesystem.  Obviously that allows changing the view on the read-
only side from the writable side.

So while having the ability to do a send from a read-only mount is indeed 
a good thing to have in emergency cases such as this, I'd suggest 
requiring a --force option or the like to enable it, since the full 
immutable-read-only guarantees simply aren't there.
David Sterba July 24, 2014, 10:47 a.m. UTC | #2
On Wed, Jul 23, 2014 at 01:47:36PM -0700, Zach Brown wrote:
> On Wed, Jul 23, 2014 at 02:10:29PM -0600, Chris Murphy wrote:
> > The use case is when it's possible to mount a Btrfs volume ro, but not rw. Example, a situation where
> > 
> > # mount -o degraded /dev/sdb /mnt
> > [   71.064352] BTRFS info (device sdb): allowing degraded mounts
> > [   71.064812] BTRFS info (device sdb): enabling auto recovery
> > [   71.065210] BTRFS info (device sdb): disk space caching is enabled
> > [   71.072068] BTRFS warning (device sdb): devid 2 missing
> > [   71.097320] BTRFS: too many missing devices, writeable mount is not allowed
> > [   71.116616] BTRFS: open_ctree failed
> > 
> > Yet this works:
> > # mount -o degraded,ro /dev/sdb /mnt
> > 
> > It would be great if it were possible to send/receive subvolumes to a
> > different btrfs volume. Currently it's not possible because those
> > subvols aren't ro, and because the mount is ro I can't make ro
> > snapshots first.
> 
> I wonder if that's as easy as the following totally untested hack.  I
> have no idea if a read-only mount would still allow background
> modification that might violate the send code's assumptions.

RO mount tries hard not to do any writes (eg. the from the background
threads), however a remount to RW during send would succeed and any
writes to the sent subvolume may (and most probably will) cause lots of
fun.

This could use similar protection as the subvolumes, the usecase 'allow
to send any subvolume on a RO mount' seems valid to me. The failure of
remount,rw is not silent and the user is able to decide what to do next
(stop send, or postpone remount). Remount may fail for other reasons so
I think we're not adding some unexpected surprises.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
index 6528aa6..3528210 100644
--- a/fs/btrfs/send.c
+++ b/fs/btrfs/send.c
@@ -5536,7 +5536,7 @@  long btrfs_ioctl_send(struct file *mnt_file, void __user *arg_)
 	 * Userspace tools do the checks and warn the user if it's
 	 * not RO.
 	 */
-	if (!btrfs_root_readonly(send_root)) {
+	if (!(btrfs_root_readonly(send_root) || (sb->s_flags & MS_RDONLY))) {
 		ret = -EPERM;
 		goto out;
 	}