| Message ID | 20200604071807.61345-2-wqu@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: Introduce new rescue= mount options | expand |
On 6/4/20 3:18 AM, Qu Wenruo wrote: > This patch introduces a new "rescue=" mount option group for all those > mount options for data recovery. > > Different rescue sub options are seperated by ':'. E.g > "ro,rescue=nologreplay:usebackuproot". > (The original plan is to use ';', but ';' needs to be escaped/quoted, > or it will be interpreted by bash) > > And obviously, user can specify rescue options one by one like: > "ro,rescue=nologreplay,rescue=usebackuproot" > > The following mount options are converted to "rescue=", old mount > options are deprecated but still available for compatibility purpose: > > - usebackuproot > Now it's "rescue=usebackuproot" > > - nologreplay > Now it's "rescue=nologreplay" > > Signed-off-by: Qu Wenruo <wqu@suse.com> Reviewed-by: Josef Bacik <josef@toxicpanda.com> Thanks, Josef
On 4/6/20 3:18 pm, Qu Wenruo wrote: > This patch introduces a new "rescue=" mount option group for all those > mount options for data recovery. > > Different rescue sub options are seperated by ':'. E.g > "ro,rescue=nologreplay:usebackuproot". > (The original plan is to use ';', but ';' needs to be escaped/quoted, > or it will be interpreted by bash) I fell ':' isn't suitable here. > And obviously, user can specify rescue options one by one like: > "ro,rescue=nologreplay,rescue=usebackuproot" This should suffice right? Thanks, Anand
On Fri, Jun 05, 2020 at 06:04:01PM +0800, Anand Jain wrote: > On 4/6/20 3:18 pm, Qu Wenruo wrote: > > This patch introduces a new "rescue=" mount option group for all those > > mount options for data recovery. > > > > Different rescue sub options are seperated by ':'. E.g > > "ro,rescue=nologreplay:usebackuproot". > > (The original plan is to use ';', but ';' needs to be escaped/quoted, > > or it will be interpreted by bash) > > I fell ':' isn't suitable here. What do you suggest then? > > And obviously, user can specify rescue options one by one like: > > "ro,rescue=nologreplay,rescue=usebackuproot" > > This should suffice right? Setting the rescue= value separately should be supported, but requiring to write the option name for each value defeats the purpose to make it compact and user friendly.
On 5/6/20 7:36 pm, David Sterba wrote: > On Fri, Jun 05, 2020 at 06:04:01PM +0800, Anand Jain wrote: >> On 4/6/20 3:18 pm, Qu Wenruo wrote: >>> This patch introduces a new "rescue=" mount option group for all those >>> mount options for data recovery. >>> >>> Different rescue sub options are seperated by ':'. E.g >>> "ro,rescue=nologreplay:usebackuproot". >>> (The original plan is to use ';', but ';' needs to be escaped/quoted, >>> or it will be interpreted by bash) >> >> I fell ':' isn't suitable here. > > What do you suggest then? > There isn't any other choice, right? Probably that's the reason for -o device it is -o device=dev1,device=dev2 still remains separated? IMO if there isn't a choice it is ok to leave them separate. But as I commented in the other thread instead of -o rescue=skipbg:another1:another2 why not just -o rescue and mount thread shall skip the checks that fail and mount the fs in RO if possible. The dmesg -k must show the checks that were failed and had to skip to make the RO mount successful. So, that becomes clear about the errors which lead to the current RO mount, instead of going through the logs to figure out. This is a more user-friendly approach as there is one rescue option. But I am not sure if it is possible? Thanks, Anand >>> And obviously, user can specify rescue options one by one like: >>> "ro,rescue=nologreplay,rescue=usebackuproot" >> >> This should suffice right? > > Setting the rescue= value separately should be supported, but requiring > to write the option name for each value defeats the purpose to make it > compact and user friendly. >
On 2020/6/8 下午4:11, Anand Jain wrote: > On 5/6/20 7:36 pm, David Sterba wrote: >> On Fri, Jun 05, 2020 at 06:04:01PM +0800, Anand Jain wrote: >>> On 4/6/20 3:18 pm, Qu Wenruo wrote: >>>> This patch introduces a new "rescue=" mount option group for all those >>>> mount options for data recovery. >>>> >>>> Different rescue sub options are seperated by ':'. E.g >>>> "ro,rescue=nologreplay:usebackuproot". >>>> (The original plan is to use ';', but ';' needs to be escaped/quoted, >>>> or it will be interpreted by bash) >>> >>> I fell ':' isn't suitable here. >> >> What do you suggest then? >> > > There isn't any other choice, right? Probably that's the reason for > -o device it is -o device=dev1,device=dev2 still remains separated? > IMO if there isn't a choice it is ok to leave them separate. > > But as I commented in the other thread instead of > -o rescue=skipbg:another1:another2 why not just -o rescue > and mount thread shall skip the checks that fail and mount the > fs in RO if possible. That would make dependency complex. The skipbg already needs nologreplay and RO, and usebackuproot sometimes doesn't work as expected (in fact, that mount option has fewer success than we thought). I don't want to spend too much code on a salvage mount option group. Thanks, Qu > The dmesg -k must show the checks that > were failed and had to skip to make the RO mount successful. > So, that becomes clear about the errors which lead to the current RO > mount, instead of going through the logs to figure out. This is a more > user-friendly approach as there is one rescue option. But I am not > sure if it is possible? > > Thanks, Anand > > >>>> And obviously, user can specify rescue options one by one like: >>>> "ro,rescue=nologreplay,rescue=usebackuproot" >>> >>> This should suffice right? >> >> Setting the rescue= value separately should be supported, but requiring >> to write the option name for each value defeats the purpose to make it >> compact and user friendly. >> >
On Mon, Jun 08, 2020 at 04:11:57PM +0800, Anand Jain wrote: > On 5/6/20 7:36 pm, David Sterba wrote: > > On Fri, Jun 05, 2020 at 06:04:01PM +0800, Anand Jain wrote: > >> On 4/6/20 3:18 pm, Qu Wenruo wrote: > >>> This patch introduces a new "rescue=" mount option group for all those > >>> mount options for data recovery. > >>> > >>> Different rescue sub options are seperated by ':'. E.g > >>> "ro,rescue=nologreplay:usebackuproot". > >>> (The original plan is to use ';', but ';' needs to be escaped/quoted, > >>> or it will be interpreted by bash) > >> > >> I fell ':' isn't suitable here. > > > > What do you suggest then? > > There isn't any other choice, right? Probably that's the reason for > -o device it is -o device=dev1,device=dev2 still remains separated? > IMO if there isn't a choice it is ok to leave them separate. I don't think -o device is a good example to follow, we'd hardly find any good separator of the filenames, because device path can contain everything. /dev/disk/by-id eg. contains ":", so we'd need escaping. > But as I commented in the other thread instead of > -o rescue=skipbg:another1:another2 why not just -o rescue > and mount thread shall skip the checks that fail and mount the > fs in RO if possible. The dmesg -k must show the checks that > were failed and had to skip to make the RO mount successful. > So, that becomes clear about the errors which lead to the current RO > mount, instead of going through the logs to figure out. This is a more > user-friendly approach as there is one rescue option. But I am not > sure if it is possible? That could be a mode of rescue= that would try hard to get the filesystem mounted but by default it's better to separate the actions, so eg. usebackuproot is not done while skipbg would be the one to make the mount possible.
On Thu, Jun 04, 2020 at 03:18:06PM +0800, Qu Wenruo wrote: > This patch introduces a new "rescue=" mount option group for all those > mount options for data recovery. > > Different rescue sub options are seperated by ':'. E.g > "ro,rescue=nologreplay:usebackuproot". > (The original plan is to use ';', but ';' needs to be escaped/quoted, > or it will be interpreted by bash) The separators available: - "," already used for mount options - ";" needs shell escaping - "|" same - "+" that also looks ok - * & # $ % @ all would be confusing I guess so ":" seems like a good choice. > And obviously, user can specify rescue options one by one like: > "ro,rescue=nologreplay,rescue=usebackuproot" > > The following mount options are converted to "rescue=", old mount > options are deprecated but still available for compatibility purpose: > > - usebackuproot > Now it's "rescue=usebackuproot" > > - nologreplay > Now it's "rescue=nologreplay" > > Signed-off-by: Qu Wenruo <wqu@suse.com> I'll add the patches as topic branch to for-next and to misc-next eventually. Thanks.
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c index bc73fd670702..ed6d5d55ee93 100644 --- a/fs/btrfs/super.c +++ b/fs/btrfs/super.c @@ -326,7 +326,6 @@ enum { Opt_defrag, Opt_nodefrag, Opt_discard, Opt_nodiscard, Opt_discard_mode, - Opt_nologreplay, Opt_norecovery, Opt_ratio, Opt_rescan_uuid_tree, @@ -340,9 +339,13 @@ enum { Opt_subvolid, Opt_thread_pool, Opt_treelog, Opt_notreelog, - Opt_usebackuproot, Opt_user_subvol_rm_allowed, + /* Rescue options */ + Opt_rescue, + Opt_usebackuproot, + Opt_nologreplay, + /* Deprecated options */ Opt_alloc_start, Opt_recovery, @@ -390,7 +393,6 @@ static const match_table_t tokens = { {Opt_discard, "discard"}, {Opt_discard_mode, "discard=%s"}, {Opt_nodiscard, "nodiscard"}, - {Opt_nologreplay, "nologreplay"}, {Opt_norecovery, "norecovery"}, {Opt_ratio, "metadata_ratio=%u"}, {Opt_rescan_uuid_tree, "rescan_uuid_tree"}, @@ -408,9 +410,13 @@ static const match_table_t tokens = { {Opt_thread_pool, "thread_pool=%u"}, {Opt_treelog, "treelog"}, {Opt_notreelog, "notreelog"}, - {Opt_usebackuproot, "usebackuproot"}, {Opt_user_subvol_rm_allowed, "user_subvol_rm_allowed"}, + /* Recovery options */ + {Opt_rescue, "rescue=%s"}, + {Opt_nologreplay, "nologreplay"}, + {Opt_usebackuproot, "usebackuproot"}, + /* Deprecated options */ {Opt_alloc_start, "alloc_start=%s"}, {Opt_recovery, "recovery"}, @@ -433,6 +439,55 @@ static const match_table_t tokens = { {Opt_err, NULL}, }; +static const match_table_t rescue_tokens = { + {Opt_usebackuproot, "usebackuproot"}, + {Opt_nologreplay, "nologreplay"}, + {Opt_err, NULL}, +}; + +static int parse_rescue_options(struct btrfs_fs_info *info, const char *options) +{ + char *opts; + char *orig; + char *p; + substring_t args[MAX_OPT_ARGS]; + int ret = 0; + + opts = kstrdup(options, GFP_KERNEL); + if (!opts) + return -ENOMEM; + orig = opts; + + while ((p = strsep(&opts, ":")) != NULL) { + int token; + + if (!*p) + continue; + token = match_token(p, rescue_tokens, args); + switch (token){ + case Opt_usebackuproot: + btrfs_info(info, + "trying to use backup root at mount time"); + btrfs_set_opt(info->mount_opt, USEBACKUPROOT); + break; + case Opt_nologreplay: + btrfs_set_and_info(info, NOLOGREPLAY, + "disabling log replay at mount time"); + break; + case Opt_err: + btrfs_info(info, "unrecognized rescue option '%s'", p); + ret = -EINVAL; + goto out; + default: + break; + } + + } +out: + kfree(orig); + return ret; +} + /* * Regular mount options parser. Everything that is needed only when * reading in a new superblock is parsed here. @@ -689,6 +744,8 @@ int btrfs_parse_options(struct btrfs_fs_info *info, char *options, break; case Opt_norecovery: case Opt_nologreplay: + btrfs_warn(info, + "'nologreplay' is deprecated, use 'rescue=nologreplay' instead"); btrfs_set_and_info(info, NOLOGREPLAY, "disabling log replay at mount time"); break; @@ -791,10 +848,11 @@ int btrfs_parse_options(struct btrfs_fs_info *info, char *options, "disabling auto defrag"); break; case Opt_recovery: - btrfs_warn(info, - "'recovery' is deprecated, use 'usebackuproot' instead"); - /* fall through */ case Opt_usebackuproot: + btrfs_warn(info, + "'%s' is deprecated, use 'rescue=usebackuproot' instead", + token == Opt_recovery ? "recovery" : + "usebackuproot"); btrfs_info(info, "trying to use backup root at mount time"); btrfs_set_opt(info->mount_opt, USEBACKUPROOT); @@ -881,6 +939,11 @@ int btrfs_parse_options(struct btrfs_fs_info *info, char *options, btrfs_set_opt(info->mount_opt, REF_VERIFY); break; #endif + case Opt_rescue: + ret = parse_rescue_options(info, args[0].from); + if (ret < 0) + goto out; + break; case Opt_err: btrfs_err(info, "unrecognized mount option '%s'", p); ret = -EINVAL; @@ -1344,7 +1407,7 @@ static int btrfs_show_options(struct seq_file *seq, struct dentry *dentry) if (btrfs_test_opt(info, NOTREELOG)) seq_puts(seq, ",notreelog"); if (btrfs_test_opt(info, NOLOGREPLAY)) - seq_puts(seq, ",nologreplay"); + seq_puts(seq, ",rescue=nologreplay"); if (btrfs_test_opt(info, FLUSHONCOMMIT)) seq_puts(seq, ",flushoncommit"); if (btrfs_test_opt(info, DISCARD_SYNC))
This patch introduces a new "rescue=" mount option group for all those mount options for data recovery. Different rescue sub options are seperated by ':'. E.g "ro,rescue=nologreplay:usebackuproot". (The original plan is to use ';', but ';' needs to be escaped/quoted, or it will be interpreted by bash) And obviously, user can specify rescue options one by one like: "ro,rescue=nologreplay,rescue=usebackuproot" The following mount options are converted to "rescue=", old mount options are deprecated but still available for compatibility purpose: - usebackuproot Now it's "rescue=usebackuproot" - nologreplay Now it's "rescue=nologreplay" Signed-off-by: Qu Wenruo <wqu@suse.com> --- fs/btrfs/super.c | 79 +++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 71 insertions(+), 8 deletions(-)