[PATCH v4 1/2] btrfs: Introduce new mount option backuproot to replace recovery

[PATCH v4 1/2] btrfs: Introduce new mount option backuproot to replace recovery

[Qu Wenruo]

Current "recovery" mount option will only try to use backup root.
However the word "recovery" is too generic and may be confusing for some
users.

Here introduce a new and more specific mount option, "backuproot" to
replace "recovery" mount option.
"Recovery" will be kept for compatibility reason, but will be
deprecated.

Also, since "backuproot" will only affect mount behavior and after
open_ctree() it has nothing to do with the filesystem, so clear the flag
after mount succeeded.

This provides the basis for later unified "norecovery" mount option.

Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
v4:
  Newly introduced to avoid confusion with later 'norecovery' patch.
---
 Documentation/filesystems/btrfs.txt |  9 ++++++---
 fs/btrfs/ctree.h                    |  2 +-
 fs/btrfs/disk-io.c                  |  8 +++++++-
 fs/btrfs/super.c                    | 17 +++++++++++------
 4 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/Documentation/filesystems/btrfs.txt b/Documentation/filesystems/btrfs.txt
index c772b47..cc84c63 100644
--- a/Documentation/filesystems/btrfs.txt
+++ b/Documentation/filesystems/btrfs.txt
@@ -168,10 +168,13 @@ 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.
 
-  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
+  backuproot
+	Enable attempts to use backup tree roots if a bad tree root is found at
+	mount time.
+	Currently this scans a list of 4 previous tree roots and tries to
 	use the first readable.
+	And since the mount option only affect mount behavior, it won't be
+	shown in mount options.
 
   rescan_uuid_tree
 	Force check and rebuild procedure of the UUID tree. This should not
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index a0165c6..0b3bff1 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -2176,7 +2176,7 @@ struct btrfs_ioctl_defrag_range_args {
 #define BTRFS_MOUNT_ENOSPC_DEBUG	 (1 << 15)
 #define BTRFS_MOUNT_AUTO_DEFRAG		(1 << 16)
 #define BTRFS_MOUNT_INODE_MAP_CACHE	(1 << 17)
-#define BTRFS_MOUNT_RECOVERY		(1 << 18)
+#define BTRFS_MOUNT_BACKUPROOT		(1 << 18)
 #define BTRFS_MOUNT_SKIP_BALANCE	(1 << 19)
 #define BTRFS_MOUNT_CHECK_INTEGRITY	(1 << 20)
 #define BTRFS_MOUNT_CHECK_INTEGRITY_INCLUDING_EXTENT_DATA (1 << 21)
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 1eb0839..c0e953d 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -3102,6 +3102,12 @@ retry_root_backup:
 
 	fs_info->open = 1;
 
+	/*
+	 * backuproot only affect mount behavior, and if open_ctree succeeded,
+	 * no need to keep the flag
+	 */
+	btrfs_clear_opt(fs_info->mount_opt, BACKUPROOT);
+
 	return 0;
 
 fail_qgroup:
@@ -3156,7 +3162,7 @@ fail:
 	return err;
 
 recovery_tree_root:
-	if (!btrfs_test_opt(tree_root, RECOVERY))
+	if (!btrfs_test_opt(tree_root, BACKUPROOT))
 		goto fail_tree_roots;
 
 	free_root_pointers(fs_info, 0);
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 24154e4..920f0cd 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_backuproot,
 #ifdef CONFIG_BTRFS_DEBUG
 	Opt_fragment_data, Opt_fragment_metadata, Opt_fragment_all,
 #endif
@@ -350,7 +350,8 @@ static match_table_t tokens = {
 	{Opt_inode_cache, "inode_cache"},
 	{Opt_noinode_cache, "noinode_cache"},
 	{Opt_no_space_cache, "nospace_cache"},
-	{Opt_recovery, "recovery"},
+	{Opt_recovery, "recovery"}, /* deprecated */
+	{Opt_backuproot, "backuproot"},
 	{Opt_skip_balance, "skip_balance"},
 	{Opt_check_integrity, "check_int"},
 	{Opt_check_integrity_including_extent_data, "check_int_data"},
@@ -657,8 +658,12 @@ int btrfs_parse_options(struct btrfs_root *root, char *options)
 					     "disabling auto defrag");
 			break;
 		case Opt_recovery:
-			btrfs_info(root->fs_info, "enabling auto recovery");
-			btrfs_set_opt(info->mount_opt, RECOVERY);
+			btrfs_warn(root->fs_info,
+				   "recovery is deprecated, use backuproot instead");
+		case Opt_backuproot:
+			btrfs_info(root->fs_info,
+				   "enabling backup root at mount time");
+			btrfs_set_opt(info->mount_opt, BACKUPROOT);
 			break;
 		case Opt_skip_balance:
 			btrfs_set_opt(info->mount_opt, SKIP_BALANCE);
@@ -1178,8 +1183,8 @@ static int btrfs_show_options(struct seq_file *seq, struct dentry *dentry)
 		seq_puts(seq, ",inode_cache");
 	if (btrfs_test_opt(root, SKIP_BALANCE))
 		seq_puts(seq, ",skip_balance");
-	if (btrfs_test_opt(root, RECOVERY))
-		seq_puts(seq, ",recovery");
+	if (btrfs_test_opt(root, BACKUPROOT))
+		seq_puts(seq, ",backuproot");
 #ifdef CONFIG_BTRFS_FS_CHECK_INTEGRITY
 	if (btrfs_test_opt(root, CHECK_INTEGRITY_INCLUDING_EXTENT_DATA))
 		seq_puts(seq, ",check_int_data");
-- 
2.6.4

[PATCH v4 2/2] 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>
Reviewed-by: Chandan Rajendra <chandan@linux.vnet.ibm.com>
Tested-by: Austin S. Hemmelgarn <ahferroin7@gmail.com>
---
v2:
  Make RO check mandatory for btrfs_parse_options().
  Add btrfs_show_options() support for nologreplay.

  Document for btrfs-mount(5) will follow after the patch being merged.
v3:
  Apply the document suggestion from  Chandan Rajendra.
v4:
  Add 'norecovery' alias for 'nologreplay'.
---
 Documentation/filesystems/btrfs.txt |  9 +++++++++
 fs/btrfs/ctree.h                    |  4 +++-
 fs/btrfs/disk-io.c                  |  7 ++++---
 fs/btrfs/super.c                    | 30 +++++++++++++++++++++++++++---
 4 files changed, 43 insertions(+), 7 deletions(-)

diff --git a/Documentation/filesystems/btrfs.txt b/Documentation/filesystems/btrfs.txt
index cc84c63..78d584a 100644
--- a/Documentation/filesystems/btrfs.txt
+++ b/Documentation/filesystems/btrfs.txt
@@ -168,6 +168,15 @@ 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.
 
+  norecovery
+  nologreplay
+	Disable the log tree replay at mount time to prevent filesystem
+	from getting modified.
+	Must be used with 'ro' mount option.
+	A filesystem mounted with this option cannot transition to a
+	read-write mount via remount,rw - the filesystem must be unmounted
+	and mounted back again if read-write access is desired.
+
   backuproot
 	Enable attempts to use backup tree roots if a bad tree root is found at
 	mount time.
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 0b3bff1..4c23f34 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 c0e953d..b213439 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 920f0cd..29fce48 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -303,6 +303,7 @@ enum {
 	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_backuproot,
+	Opt_norecovery, Opt_nologreplay,
 #ifdef CONFIG_BTRFS_DEBUG
 	Opt_fragment_data, Opt_fragment_metadata, Opt_fragment_all,
 #endif
@@ -334,6 +335,8 @@ static match_table_t tokens = {
 	{Opt_noacl, "noacl"},
 	{Opt_notreelog, "notreelog"},
 	{Opt_treelog, "treelog"},
+	{Opt_norecovery, "norecovery"},
+	{Opt_nologreplay, "nologreplay"},
 	{Opt_flushoncommit, "flushoncommit"},
 	{Opt_noflushoncommit, "noflushoncommit"},
 	{Opt_ratio, "metadata_ratio=%d"},
@@ -372,7 +375,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];
@@ -387,8 +391,12 @@ int btrfs_parse_options(struct btrfs_root *root, char *options)
 	if (cache_gen)
 		btrfs_set_opt(info->mount_opt, SPACE_CACHE);
 
+	/*
+	 * Even the options are empty, we still need to do extra check
+	 * against new flags
+	 */
 	if (!options)
-		goto out;
+		goto check;
 
 	/*
 	 * strsep changes the string, duplicate it because parse_options
@@ -588,6 +596,11 @@ int btrfs_parse_options(struct btrfs_root *root, char *options)
 			btrfs_clear_and_info(root, NOTREELOG,
 					     "enabling tree log");
 			break;
+		case Opt_nologreplay:
+		case Opt_norecovery:
+			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");
@@ -758,6 +771,15 @@ int btrfs_parse_options(struct btrfs_root *root, char *options)
 			break;
 		}
 	}
+check:
+	/*
+	 * 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");
@@ -1159,6 +1181,8 @@ static int btrfs_show_options(struct seq_file *seq, struct dentry *dentry)
 		seq_puts(seq, ",ssd");
 	if (btrfs_test_opt(root, NOTREELOG))
 		seq_puts(seq, ",notreelog");
+	if (btrfs_test_opt(root, NOLOGREPLAY))
+		seq_puts(seq, ",nologreplay");
 	if (btrfs_test_opt(root, FLUSHONCOMMIT))
 		seq_puts(seq, ",flushoncommit");
 	if (btrfs_test_opt(root, DISCARD))
@@ -1642,7 +1666,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.4

Re: [PATCH v4 1/2] btrfs: Introduce new mount option backuproot to replace recovery

[Holger Hoffstätte]

On Tue, Dec 22, 2015 at 3:16 AM, Qu Wenruo <quwenruo@cn.fujitsu.com> wrote:
> Current "recovery" mount option will only try to use backup root.
> However the word "recovery" is too generic and may be confusing for some
> users.
>
> Here introduce a new and more specific mount option, "backuproot" to
> replace "recovery" mount option.
> "Recovery" will be kept for compatibility reason, but will be
> deprecated.

I agree that this makes much more sense from a user's perspective, so +1.

But why not go all the way: try backuproot automatically when the
initial mount fails,
log the fallback and simply deprecate/ignore the option?

Either this works - in which case there is no need to involve a human,
which may not
even exist - or it does not. If it does not, things are hosed anyway
and we can fail
properly.

Any reasons not to do this?

-h

Re: [PATCH v4 1/2] btrfs: Introduce new mount option backuproot to replace recovery

[Qu Wenruo]

On 12/22/2015 08:30 PM, Holger Hoffstätte wrote:
> On Tue, Dec 22, 2015 at 3:16 AM, Qu Wenruo <quwenruo@cn.fujitsu.com> wrote:
>> Current "recovery" mount option will only try to use backup root.
>> However the word "recovery" is too generic and may be confusing for some
>> users.
>>
>> Here introduce a new and more specific mount option, "backuproot" to
>> replace "recovery" mount option.
>> "Recovery" will be kept for compatibility reason, but will be
>> deprecated.
>
> I agree that this makes much more sense from a user's perspective, so +1.
>
> But why not go all the way: try backuproot automatically when the
> initial mount fails,
> log the fallback and simply deprecate/ignore the option?

Auto repair is commonly a good choice, but only for small or expected 
problem.

For case like need to replay log against power loss, that's completely 
OK to auto do it without any human input.

But for case like current tree/chunk/extent root corruption, it is 
already a *BIG* problem.
Either corrupted device or hidden Btrfs bug.

And in some case, using auto backup root may lead to more problem, 
especially when the generation difference is larger than 2, it's 
possible backup root points to completely wrong extents.

So here I still follow the original behavior.

Thanks,
Qu
>
> Either this works - in which case there is no need to involve a human,
> which may not
> even exist - or it does not. If it does not, things are hosed anyway
> and we can fail
> properly.
>
> Any reasons not to do this?
>
> -h
> --
> 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
>

Re: [PATCH v4 1/2] btrfs: Introduce new mount option backuproot to replace recovery

[Austin S. Hemmelgarn]

On 2015-12-22 07:30, Holger Hoffstätte wrote:
> On Tue, Dec 22, 2015 at 3:16 AM, Qu Wenruo <quwenruo@cn.fujitsu.com> wrote:
>> Current "recovery" mount option will only try to use backup root.
>> However the word "recovery" is too generic and may be confusing for some
>> users.
>>
>> Here introduce a new and more specific mount option, "backuproot" to
>> replace "recovery" mount option.
>> "Recovery" will be kept for compatibility reason, but will be
>> deprecated.
>
> I agree that this makes much more sense from a user's perspective, so +1.
>
> But why not go all the way: try backuproot automatically when the
> initial mount fails,
> log the fallback and simply deprecate/ignore the option?
>
> Either this works - in which case there is no need to involve a human,
> which may not
> even exist - or it does not. If it does not, things are hosed anyway
> and we can fail
> properly.
I have seen cases where mounting -o ro,recovery didn't work, but restore 
did, so there are cases where data is recoverable but this mount option 
won't work.
>
> Any reasons not to do this?
I'm not 100% certain, but I think that there is a chance if it doesn't 
work that it will make things worse.  That, and TBH, it really should be 
the administrator's choice; personally, if I come across a filesystem on 
one of my systems that needs this, then I'm nuking the filesystem and 
restoring from backup, because I don't trust that the rest of the 
filesystem isn't broken.  It's also consistent with other filesystems 
(at least ext4 requires an option to force using a backup superblock). 
Perhaps we could add a module parameter that specifies whether or not to 
automatically try this?

Re: [PATCH v4 1/2] btrfs: Introduce new mount option backuproot to replace recovery

[Austin S. Hemmelgarn]

On 2015-12-21 21:16, Qu Wenruo wrote:
> Current "recovery" mount option will only try to use backup root.
> However the word "recovery" is too generic and may be confusing for some
> users.
>
> Here introduce a new and more specific mount option, "backuproot" to
> replace "recovery" mount option.
> "Recovery" will be kept for compatibility reason, but will be
> deprecated.
>
> Also, since "backuproot" will only affect mount behavior and after
> open_ctree() it has nothing to do with the filesystem, so clear the flag
> after mount succeeded.
>
> This provides the basis for later unified "norecovery" mount option.
>
> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
> ---
> v4:
>    Newly introduced to avoid confusion with later 'norecovery' patch.
> ---
I ran my usual tests overnight on this part, and nothing broke, so you 
can add
Tested-by: Austin S. Hemmelgarn
here as well.

Re: [PATCH v4 1/2] btrfs: Introduce new mount option backuproot to replace recovery

[Christoph Anton Mitterer]

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

On Tue, 2015-12-22 at 10:16 +0800, Qu Wenruo wrote:
> Current "recovery" mount option will only try to use backup root.
> However the word "recovery" is too generic and may be confusing for
> some
> users.
I would rather call it something like "restorebackuproot" or
"usebackuproot",... caues "backuproot" sounds more like *doing* the
action "backup root".

Cheers,
Chris.

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

Re: [PATCH v4 1/2] btrfs: Introduce new mount option backuproot to replace recovery

[Qu Wenruo]

Christoph Anton Mitterer wrote on 2015/12/22 22:35 +0100:
> On Tue, 2015-12-22 at 10:16 +0800, Qu Wenruo wrote:
>> Current "recovery" mount option will only try to use backup root.
>> However the word "recovery" is too generic and may be confusing for
>> some
>> users.
> I would rather call it something like "restorebackuproot" or
> "usebackuproot",... caues "backuproot" sounds more like *doing* the
> action "backup root".

Make sense.

I'll pick "usebackuproot" for next version.

Thanks,
Qu

>
> Cheers,
> Chris.
>