[PATCH] btrfs: Introduce new mount option to disable tree log replay

[PATCH] btrfs: Introduce new mount option to disable tree log replay

[Qu Wenruo]

Introduce a new mount option "nologreplay" to co-operate with "ro" mount
option to get real readonly mount, like "norecovery" in ext* and xfs.

Since the new parse_options() need to check new flags at remount time,
so add a new parameter for parse_options().

Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
 Documentation/filesystems/btrfs.txt |  5 +++++
 fs/btrfs/ctree.h                    |  4 +++-
 fs/btrfs/disk-io.c                  |  7 ++++---
 fs/btrfs/super.c                    | 20 +++++++++++++++++---
 4 files changed, 29 insertions(+), 7 deletions(-)

diff --git a/Documentation/filesystems/btrfs.txt b/Documentation/filesystems/btrfs.txt
index c772b47..ac4ed68 100644
--- a/Documentation/filesystems/btrfs.txt
+++ b/Documentation/filesystems/btrfs.txt
@@ -168,6 +168,11 @@ Options with (*) are default options and will not show in the mount options.
   notreelog
 	Enable/disable the tree logging used for fsync and O_SYNC writes.
 
+  nologreplay
+	Disable the log tree replay at mount time for real read-only mount.
+	Must be use with "ro" mount option and can't be disabled by mount
+	option.
+
   recovery
 	Enable autorecovery attempts if a bad tree root is found at mount time.
 	Currently this scans a list of several previous tree roots and tries to
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index a0165c6..c54ff25 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -2184,6 +2184,7 @@ struct btrfs_ioctl_defrag_range_args {
 #define BTRFS_MOUNT_RESCAN_UUID_TREE	(1 << 23)
 #define BTRFS_MOUNT_FRAGMENT_DATA	(1 << 24)
 #define BTRFS_MOUNT_FRAGMENT_METADATA	(1 << 25)
+#define BTRFS_MOUNT_NOLOGREPLAY		(1 << 26)
 
 #define BTRFS_DEFAULT_COMMIT_INTERVAL	(30)
 #define BTRFS_DEFAULT_MAX_INLINE	(8192)
@@ -4070,7 +4071,8 @@ void btrfs_sysfs_remove_mounted(struct btrfs_fs_info *fs_info);
 ssize_t btrfs_listxattr(struct dentry *dentry, char *buffer, size_t size);
 
 /* super.c */
-int btrfs_parse_options(struct btrfs_root *root, char *options);
+int btrfs_parse_options(struct btrfs_root *root, char *options,
+			unsigned long new_flags);
 int btrfs_sync_fs(struct super_block *sb, int wait);
 
 #ifdef CONFIG_PRINTK
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 1eb0839..617bf4f 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -2711,7 +2711,7 @@ int open_ctree(struct super_block *sb,
 	 */
 	fs_info->compress_type = BTRFS_COMPRESS_ZLIB;
 
-	ret = btrfs_parse_options(tree_root, options);
+	ret = btrfs_parse_options(tree_root, options, sb->s_flags);
 	if (ret) {
 		err = ret;
 		goto fail_alloc;
@@ -3009,8 +3009,9 @@ retry_root_backup:
 	if (ret)
 		goto fail_trans_kthread;
 
-	/* do not make disk changes in broken FS */
-	if (btrfs_super_log_root(disk_super) != 0) {
+	/* do not make disk changes in broken FS or nologreplay is given */
+	if (btrfs_super_log_root(disk_super) != 0 &&
+	    !btrfs_test_opt(tree_root, NOLOGREPLAY)) {
 		ret = btrfs_replay_log(fs_info, fs_devices);
 		if (ret) {
 			err = ret;
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 24154e4..3b1dcc1 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -302,7 +302,7 @@ enum {
 	Opt_check_integrity_print_mask, Opt_fatal_errors, Opt_rescan_uuid_tree,
 	Opt_commit_interval, Opt_barrier, Opt_nodefrag, Opt_nodiscard,
 	Opt_noenospc_debug, Opt_noflushoncommit, Opt_acl, Opt_datacow,
-	Opt_datasum, Opt_treelog, Opt_noinode_cache,
+	Opt_datasum, Opt_treelog, Opt_noinode_cache, Opt_nologreplay,
 #ifdef CONFIG_BTRFS_DEBUG
 	Opt_fragment_data, Opt_fragment_metadata, Opt_fragment_all,
 #endif
@@ -334,6 +334,7 @@ static match_table_t tokens = {
 	{Opt_noacl, "noacl"},
 	{Opt_notreelog, "notreelog"},
 	{Opt_treelog, "treelog"},
+	{Opt_nologreplay, "nologreplay"},
 	{Opt_flushoncommit, "flushoncommit"},
 	{Opt_noflushoncommit, "noflushoncommit"},
 	{Opt_ratio, "metadata_ratio=%d"},
@@ -371,7 +372,8 @@ static match_table_t tokens = {
  * reading in a new superblock is parsed here.
  * XXX JDM: This needs to be cleaned up for remount.
  */
-int btrfs_parse_options(struct btrfs_root *root, char *options)
+int btrfs_parse_options(struct btrfs_root *root, char *options,
+			unsigned long new_flags)
 {
 	struct btrfs_fs_info *info = root->fs_info;
 	substring_t args[MAX_OPT_ARGS];
@@ -587,6 +589,10 @@ int btrfs_parse_options(struct btrfs_root *root, char *options)
 			btrfs_clear_and_info(root, NOTREELOG,
 					     "enabling tree log");
 			break;
+		case Opt_nologreplay:
+			btrfs_set_and_info(root, NOLOGREPLAY,
+					   "disabling log replay at mount time");
+			break;
 		case Opt_flushoncommit:
 			btrfs_set_and_info(root, FLUSHONCOMMIT,
 					   "turning on flush-on-commit");
@@ -753,6 +759,14 @@ int btrfs_parse_options(struct btrfs_root *root, char *options)
 			break;
 		}
 	}
+	/*
+	 * Extra check for current option against current flag
+	 */
+	if (btrfs_test_opt(root, NOLOGREPLAY) && !(new_flags & MS_RDONLY)) {
+		btrfs_err(root->fs_info,
+			  "nologreplay must be used with ro mount option");
+		ret = -EINVAL;
+	}
 out:
 	if (!ret && btrfs_test_opt(root, SPACE_CACHE))
 		btrfs_info(root->fs_info, "disk space caching is enabled");
@@ -1637,7 +1651,7 @@ static int btrfs_remount(struct super_block *sb, int *flags, char *data)
 		}
 	}
 
-	ret = btrfs_parse_options(root, data);
+	ret = btrfs_parse_options(root, data, *flags);
 	if (ret) {
 		ret = -EINVAL;
 		goto restore;
-- 
2.6.3

Re: [PATCH] btrfs: Introduce new mount option to disable tree log replay

[Chandan Rajendra]

On Monday 07 Dec 2015 14:06:42 Qu Wenruo wrote:
> Introduce a new mount option "nologreplay" to co-operate with "ro" mount
> option to get real readonly mount, like "norecovery" in ext* and xfs.
> 
> Since the new parse_options() need to check new flags at remount time,
> so add a new parameter for parse_options().
>

Hello Qu,
The patch should add an entry into btrfs_show_options() for 'mount' command to
be able to display an entry corresponding to 'nologreplay' mount option.

Even after adding such an entry to btrfs_show_options(),
the following occurs,
# mount -o nologreplay,ro /dev/loop6 /mnt/
# mount | grep loop6
/dev/loop6 on /mnt type btrfs (ro,relatime,seclabel,nologreplay,space_cache,subvolid=5,subvol=/)
# mount -o remount,rw /dev/loop6 /mnt/
# mount | grep loop6
/dev/loop6 on /mnt type btrfs (rw,relatime,seclabel,nologreplay,space_cache,subvolid=5,subvol=/)

As can be seen from the last line, both 'rw' and 'nologreplay' options are set.

-- 
chandan

Re: [PATCH] btrfs: Introduce new mount option to disable tree log replay

[Eric Sandeen]

On 12/7/15 12:06 AM, Qu Wenruo wrote:
> Introduce a new mount option "nologreplay" to co-operate with "ro" mount
> option to get real readonly mount, like "norecovery" in ext* and xfs.
> 
> Since the new parse_options() need to check new flags at remount time,
> so add a new parameter for parse_options().
> 
> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
> ---
>  Documentation/filesystems/btrfs.txt |  5 +++++
>  fs/btrfs/ctree.h                    |  4 +++-
>  fs/btrfs/disk-io.c                  |  7 ++++---
>  fs/btrfs/super.c                    | 20 +++++++++++++++++---
>  4 files changed, 29 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/filesystems/btrfs.txt b/Documentation/filesystems/btrfs.txt
> index c772b47..ac4ed68 100644
> --- a/Documentation/filesystems/btrfs.txt
> +++ b/Documentation/filesystems/btrfs.txt
> @@ -168,6 +168,11 @@ Options with (*) are default options and will not show in the mount options.
>    notreelog
>  	Enable/disable the tree logging used for fsync and O_SYNC writes.
>  
> +  nologreplay
> +	Disable the log tree replay at mount time for real read-only mount.
> +	Must be use with "ro" mount option and can't be disabled by mount
> +	option.

This documentation is not clear to me - "can't be disabled by mount option?"

I think you mean to talk about remount here?  Perhaps something like:

"... Must be used with 'ro' mount option.  A filesystem mounted with the
'nologreplay' option cannot transition to a read-write mount via 
remount,rw - the filesystem must be unmounted and remounted if read-write
access is desired."

Thanks,
-Eric

Re: [PATCH] btrfs: Introduce new mount option to disable tree log replay

[Austin S Hemmelgarn]

[-- Attachment #1: Type: text/plain, Size: 496 bytes --]

On 2015-12-07 01:06, Qu Wenruo wrote:
> Introduce a new mount option "nologreplay" to co-operate with "ro" mount
> option to get real readonly mount, like "norecovery" in ext* and xfs.
>
> Since the new parse_options() need to check new flags at remount time,
> so add a new parameter for parse_options().
>

Passes xfstests and a handful of other things that I really should just 
take the time to integrate into xfstests, so:
Tested-by: Austin S. Hemmelgarn <ahferroin7@gmail.com>


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 3019 bytes --]

Re: [PATCH] btrfs: Introduce new mount option to disable tree log replay

[Chandan Rajendra]

On Monday 07 Dec 2015 10:27:05 Eric Sandeen wrote:
> On 12/7/15 12:06 AM, Qu Wenruo wrote:
> > Introduce a new mount option "nologreplay" to co-operate with "ro" mount
> > option to get real readonly mount, like "norecovery" in ext* and xfs.
> > 
> > Since the new parse_options() need to check new flags at remount time,
> > so add a new parameter for parse_options().
> > 
> > Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
> > ---
> > 
> >  Documentation/filesystems/btrfs.txt |  5 +++++
> >  fs/btrfs/ctree.h                    |  4 +++-
> >  fs/btrfs/disk-io.c                  |  7 ++++---
> >  fs/btrfs/super.c                    | 20 +++++++++++++++++---
> >  4 files changed, 29 insertions(+), 7 deletions(-)
> > 
> > diff --git a/Documentation/filesystems/btrfs.txt
> > b/Documentation/filesystems/btrfs.txt index c772b47..ac4ed68 100644
> > --- a/Documentation/filesystems/btrfs.txt
> > +++ b/Documentation/filesystems/btrfs.txt
> > @@ -168,6 +168,11 @@ Options with (*) are default options and will not
> > show in the mount options.> 
> >    notreelog
> >  	
> >  	Enable/disable the tree logging used for fsync and O_SYNC writes.
> > 
> > +  nologreplay
> > +	Disable the log tree replay at mount time for real read-only mount.
> > +	Must be use with "ro" mount option and can't be disabled by mount
> > +	option.
> 
> This documentation is not clear to me - "can't be disabled by mount option?"
> 
> I think you mean to talk about remount here?  Perhaps something like:
> 
> "... Must be used with 'ro' mount option.  A filesystem mounted with the
> 'nologreplay' option cannot transition to a read-write mount via
> remount,rw - the filesystem must be unmounted and remounted if read-write
> access is desired."
> 
Eric, I had assumed the same logic with respect to the transition from 'ro' to
'rw' via remount. But when doing so, btrfs_remount() flags an error only when
a valid 'tree log' tree is present in the filesystem
i.e. btrfs_super_block->log_root has a non-zero value. Otherwise,
btrfs_remount() does not seem to have any problem with the transition from
'ro' to 'rw'.

-- 
chandan

Re: [PATCH] btrfs: Introduce new mount option to disable tree log replay

[Eric Sandeen]

On 12/7/15 10:52 AM, Chandan Rajendra wrote:
> On Monday 07 Dec 2015 10:27:05 Eric Sandeen wrote:
>> On 12/7/15 12:06 AM, Qu Wenruo wrote:
>>> Introduce a new mount option "nologreplay" to co-operate with "ro" mount
>>> option to get real readonly mount, like "norecovery" in ext* and xfs.
>>>
>>> Since the new parse_options() need to check new flags at remount time,
>>> so add a new parameter for parse_options().
>>>
>>> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
>>> ---
>>>
>>>  Documentation/filesystems/btrfs.txt |  5 +++++
>>>  fs/btrfs/ctree.h                    |  4 +++-
>>>  fs/btrfs/disk-io.c                  |  7 ++++---
>>>  fs/btrfs/super.c                    | 20 +++++++++++++++++---
>>>  4 files changed, 29 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/Documentation/filesystems/btrfs.txt
>>> b/Documentation/filesystems/btrfs.txt index c772b47..ac4ed68 100644
>>> --- a/Documentation/filesystems/btrfs.txt
>>> +++ b/Documentation/filesystems/btrfs.txt
>>> @@ -168,6 +168,11 @@ Options with (*) are default options and will not
>>> show in the mount options.> 
>>>    notreelog
>>>  	
>>>  	Enable/disable the tree logging used for fsync and O_SYNC writes.
>>>
>>> +  nologreplay
>>> +	Disable the log tree replay at mount time for real read-only mount.
>>> +	Must be use with "ro" mount option and can't be disabled by mount
>>> +	option.
>>
>> This documentation is not clear to me - "can't be disabled by mount option?"
>>
>> I think you mean to talk about remount here?  Perhaps something like:
>>
>> "... Must be used with 'ro' mount option.  A filesystem mounted with the
>> 'nologreplay' option cannot transition to a read-write mount via
>> remount,rw - the filesystem must be unmounted and remounted if read-write
>> access is desired."
>>
> Eric, I had assumed the same logic with respect to the transition from 'ro' to
> 'rw' via remount. But when doing so, btrfs_remount() flags an error only when
> a valid 'tree log' tree is present in the filesystem
> i.e. btrfs_super_block->log_root has a non-zero value. Otherwise,
> btrfs_remount() does not seem to have any problem with the transition from
> 'ro' to 'rw'.

Ok, I don't know if that's intended - but I think the docs should be clarified
to explicitly state the expected behavior in any case.

FWIW, new mount options and their descriptions should be added to BTRFS-MOUNT(5) 
as well.

-Eric

Re: [PATCH] btrfs: Introduce new mount option to disable tree log replay

[Christoph Anton Mitterer]

[-- Attachment #1: Type: text/plain, Size: 1363 bytes --]

On Mon, 2015-12-07 at 11:29 -0600, Eric Sandeen wrote:
> FWIW, new mount options and their descriptions should be added to
> BTRFS-MOUNT(5) 
> as well.
Also, from the end-user perspective, there should be:
1) another option like (hard-ro) which is defined to imply any other
options that are required to make a mount truly read-only (i.e. right
now: ro and nologreplay... but in the future any other features that
may be required to make read-only really read-only).

and/or

2) a section that describes "ro" in btrfs-mount(5) which describes that
normal "ro" alone may cause changes on the device and which then refers
to hard-ro and/or the list of options (currently nologreplay) which are
required right now to make it truly ro.


I think this is important as an end-user probably expects "ro" to be
truly ro, so if he looks it up in the documentation (2) he should find
enough information that this isn't the case and what to do instead.
Further, as one might expect that in the future, other places (than
just the log) may cause changes to a device, even though mounted ro...
I think it's better for the end user to also have a real "hard-ro" like
option, than a possibly growing list of "noXYZ" where the end-user may
have no clue that something else is now also required to get the truly
read-only behaviour.


Cheers,
Chris.

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5313 bytes --]

Re: [PATCH] btrfs: Introduce new mount option to disable tree log replay

[Eric Sandeen]

On 12/7/15 2:54 PM, Christoph Anton Mitterer wrote:

...
 
> 2) a section that describes "ro" in btrfs-mount(5) which describes that
> normal "ro" alone may cause changes on the device and which then refers
> to hard-ro and/or the list of options (currently nologreplay) which are
> required right now to make it truly ro.
> 
> 
> I think this is important as an end-user probably expects "ro" to be
> truly ro,

Yeah, I don't know that this is true.  It hasn't been true for over a
decade (2?), with the most widely-used filesystem in linux history, i.e.
ext3.  So if btrfs wants to go on this re-education crusade, more power
to you, but I don't know that it's really a fight worth fighting.  ;)

> so if he looks it up in the documentation (2) he should find
> enough information that this isn't the case and what to do instead.
> Further, as one might expect that in the future, other places (than
> just the log) may cause changes to a device, even though mounted ro...
> I think it's better for the end user to also have a real "hard-ro" like
> option, than a possibly growing list of "noXYZ" where the end-user may
> have no clue that something else is now also required to get the truly
> read-only behaviour.

# blockdev --setro ...

-Eric

Re: [PATCH] btrfs: Introduce new mount option to disable tree log replay

[Qu Wenruo]

On 12/07/2015 11:38 PM, Chandan Rajendra wrote:
> On Monday 07 Dec 2015 14:06:42 Qu Wenruo wrote:
>> Introduce a new mount option "nologreplay" to co-operate with "ro" mount
>> option to get real readonly mount, like "norecovery" in ext* and xfs.
>>
>> Since the new parse_options() need to check new flags at remount time,
>> so add a new parameter for parse_options().
>>
>
> Hello Qu,
> The patch should add an entry into btrfs_show_options() for 'mount' command to
> be able to display an entry corresponding to 'nologreplay' mount option.

Thanks for the advice.
I just forgot show_mount_options().

And for option to disable "nologreplay", I follow the behavior of xfs 
and ext4 "norecovery" behavior.
Which means no option to disable "norecovery", as it is only used for 
debugging case.
So the only method is to umount it and mount it again.

>
> Even after adding such an entry to btrfs_show_options(),
> the following occurs,
> # mount -o nologreplay,ro /dev/loop6 /mnt/
> # mount | grep loop6
> /dev/loop6 on /mnt type btrfs (ro,relatime,seclabel,nologreplay,space_cache,subvolid=5,subvol=/)
> # mount -o remount,rw /dev/loop6 /mnt/
> # mount | grep loop6
> /dev/loop6 on /mnt type btrfs (rw,relatime,seclabel,nologreplay,space_cache,subvolid=5,subvol=/)
>
> As can be seen from the last line, both 'rw' and 'nologreplay' options are set.
>

I'll double check it, maybe it bypassed btrfs_parse_options(), and 
bypassed the check in it.

Thanks,
Qu

Re: [PATCH] btrfs: Introduce new mount option to disable tree log replay

[Christoph Anton Mitterer]

[-- Attachment #1: Type: text/plain, Size: 885 bytes --]

On Mon, 2015-12-07 at 17:06 -0600, Eric Sandeen wrote:
> Yeah, I don't know that this is true.  It hasn't been true for over a
> decade (2?), with the most widely-used filesystem in linux history,
> i.e.
> ext3.
Based on what? I'd now many sysadmins who don't expect that e.g. the
journal is replayed when ext* is mounted ro.

>   So if btrfs wants to go on this re-education crusade, more power
> to you, but I don't know that it's really a fight worth fighting.  ;)
I don't think there's a bight fight necessary, is it?
Just properly document all options and not only from the developers or
expert-users PoV.

Using blockdev --setro is of course an alternative to a dedicated
"hard-ro" option (or something similar that goes by a better name),...
OTOH I don't think that having such an option would cause big problems
or "religion wars" ;)


Cheers,
Chris.

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5313 bytes --]

Re: [PATCH] btrfs: Introduce new mount option to disable tree log replay

[Qu Wenruo]

Austin S Hemmelgarn wrote on 2015/12/07 11:36 -0500:
> On 2015-12-07 01:06, Qu Wenruo wrote:
>> Introduce a new mount option "nologreplay" to co-operate with "ro" mount
>> option to get real readonly mount, like "norecovery" in ext* and xfs.
>>
>> Since the new parse_options() need to check new flags at remount time,
>> so add a new parameter for parse_options().
>>
>
> Passes xfstests and a handful of other things that I really should just
> take the time to integrate into xfstests, so:
> Tested-by: Austin S. Hemmelgarn <ahferroin7@gmail.com>
>
Thanks for the test.

But I'm afraid you may need to test v2 patch again, as the v2 changed 
some behavior.

Thanks,
Qu

Re: [PATCH] btrfs: Introduce new mount option to disable tree log replay

[Austin S Hemmelgarn]

[-- Attachment #1: Type: text/plain, Size: 1559 bytes --]

On 2015-12-07 18:06, Eric Sandeen wrote:
> On 12/7/15 2:54 PM, Christoph Anton Mitterer wrote:
>
> ...
>
>> 2) a section that describes "ro" in btrfs-mount(5) which describes that
>> normal "ro" alone may cause changes on the device and which then refers
>> to hard-ro and/or the list of options (currently nologreplay) which are
>> required right now to make it truly ro.
>>
>>
>> I think this is important as an end-user probably expects "ro" to be
>> truly ro,
>
> Yeah, I don't know that this is true.  It hasn't been true for over a
> decade (2?), with the most widely-used filesystem in linux history, i.e.
> ext3.  So if btrfs wants to go on this re-education crusade, more power
> to you, but I don't know that it's really a fight worth fighting.  ;)
>
Actually, AFAICT, it's been at least 4.5 decades.  Last I checked, this 
dates back to the original UNIX filesystems, which still updated atimes 
even when mounted RO.

Despite this, it really isn't a widely known or well documented behavior 
outside of developers, forensic specialists, and people who have had to 
deal with the implications it has on data recovery.  There really isn't 
any way that the user would know about it without being explicitly told, 
and it's something that can have a serious impact on being able to 
recover a broken filesystem.  TBH, I really feel that _every_ 
filesystem's documentation should have something about how to make it 
mount truly read-only, even if it's just a reference to how to mark the 
block device read-only.



[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 3019 bytes --]

Re: [PATCH] btrfs: Introduce new mount option to disable tree log replay

[Austin S Hemmelgarn]

[-- Attachment #1: Type: text/plain, Size: 814 bytes --]

On 2015-12-08 01:08, Qu Wenruo wrote:
>
>
> Austin S Hemmelgarn wrote on 2015/12/07 11:36 -0500:
>> On 2015-12-07 01:06, Qu Wenruo wrote:
>>> Introduce a new mount option "nologreplay" to co-operate with "ro" mount
>>> option to get real readonly mount, like "norecovery" in ext* and xfs.
>>>
>>> Since the new parse_options() need to check new flags at remount time,
>>> so add a new parameter for parse_options().
>>>
>>
>> Passes xfstests and a handful of other things that I really should just
>> take the time to integrate into xfstests, so:
>> Tested-by: Austin S. Hemmelgarn <ahferroin7@gmail.com>
>>
> Thanks for the test.
>
> But I'm afraid you may need to test v2 patch again, as the v2 changed
> some behavior.
>
That's OK, I should have results for you some time later today.



[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 3019 bytes --]

Re: [PATCH] btrfs: Introduce new mount option to disable tree log replay

[Christoph Anton Mitterer]

[-- Attachment #1: Type: text/plain, Size: 1108 bytes --]

On Tue, 2015-12-08 at 07:15 -0500, Austin S Hemmelgarn wrote:
> Despite this, it really isn't a widely known or well documented
> behavior 
> outside of developers, forensic specialists, and people who have had
> to 
> deal with the implications it has on data recovery.  There really
> isn't 
> any way that the user would know about it without being explicitly
> told, 
> and it's something that can have a serious impact on being able to 
> recover a broken filesystem.  TBH, I really feel that _every_ 
> filesystem's documentation should have something about how to make it
> mount truly read-only, even if it's just a reference to how to mark
> the 
> block device read-only.
Exactly what I've meant.

And the developers here, should definitely consider that every normal
end-user, may easily assume the role of e.g. a forensics specialist
(especially with btrfs ;-) ), when recovery in case of corruptions is
tried.


I don't think that "it has always been improperly documented" (i.e. the
"ro" option) is a good excuse to continue doing it that way =)


Cheers,
Chris.

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5313 bytes --]

Re: [PATCH] btrfs: Introduce new mount option to disable tree log replay

[Austin S Hemmelgarn]

[-- Attachment #1: Type: text/plain, Size: 2470 bytes --]

On 2015-12-08 14:20, Christoph Anton Mitterer wrote:
> On Tue, 2015-12-08 at 07:15 -0500, Austin S Hemmelgarn wrote:
>> Despite this, it really isn't a widely known or well documented
>> behavior
>> outside of developers, forensic specialists, and people who have had
>> to
>> deal with the implications it has on data recovery.  There really
>> isn't
>> any way that the user would know about it without being explicitly
>> told,
>> and it's something that can have a serious impact on being able to
>> recover a broken filesystem.  TBH, I really feel that _every_
>> filesystem's documentation should have something about how to make it
>> mount truly read-only, even if it's just a reference to how to mark
>> the
>> block device read-only.
> Exactly what I've meant.
>
> And the developers here, should definitely consider that every normal
> end-user, may easily assume the role of e.g. a forensics specialist
> (especially with btrfs ;-) ), when recovery in case of corruptions is
> tried.
>
>
> I don't think that "it has always been improperly documented" (i.e. the
> "ro" option) is a good excuse to continue doing it that way =)
Agreed, 'but it's always been that way' is never a valid argument, and 
the fact that people who have been working on UNIX for decades know it 
doesn't mean that it's something that people will just inherently know. 
  The only reason it was that way to begin with is because it was 
assumed that everyone dealing with computers had a huge amount of domain 
specific knowledge of them (this was a valid assumption back in 1970, it 
hasn't been a valid assumption since at least 1990).

Stuff that seems obvious to people who have been working on it for years 
isn't necessarily obvious to people who have limited experience with it 
(I recently had to explain to a friend who had almost no networking 
background how IP addresses are just an abstraction for MAC addresses, 
and how it's not possible to block WiFi access based on an IP address; 
it took me three tries and eventually making the analogy of street 
addresses being an abstraction for geographical coordinates before he 
finally got it).

TBH, the only reason I knew about this rather annoying detail of 
filesystem implementation before using BTRFS is because of dealing with 
shared storage on VM's (that was an interesting week of debugging and 
restoring backups before I finally figured out what was going on).



[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 3019 bytes --]