On 2019/6/12 下午11:09, David Sterba wrote:
> On Wed, Jun 12, 2019 at 02:36:56PM +0800, Qu Wenruo wrote:
>> This patch introduces a new "rescue=" mount option for all those mount
>> options for data recovery.
>>
>> Different rescue sub options are seperated by ':'. E.g
>> "ro,rescue=no_log_replay:use_backup_root".
>> (The original plan is to use ';', but ';' needs to be escaped/quoted,
>> or it will be interpreted by bash)
>
> ':' as separator is fine
>
>> The following mount options are converted to "rescue=", old mount
>> options are deprecated but still available for compatibility purpose:
>>
>> - usebackuproot
>> Now it's "rescue=use_backup_root"
>>
>> - nologreplay
>> Now it's "rescue=no_log_replay"
>>
>> The new underscore is here to make the option more readable and make
>> spell check happier.
>
> Who uses spell checker on mount options, really? I'd expect that the new
> syntax would build on top of the old so the exact same names of the
> options are now shifted to the value of 'rescue='.
>
> The usability is better with switching
>
> -o usebackuproot
>
> to
>
> -o rescue=usebackuproot
The problem is, every time I see things like usebackuproot or
nologreplay, it's not that easy to understand them just by a quick glance.
Further more, they are already rescue options, not something we would
need to use in a daily basis.
A little longer but easier to read looks valid to me in such use case.
Thanks,
Qu
>
> ie. just prepending 'rescue='.
>
>> Signed-off-by: Qu Wenruo <wqu@xxxxxxxx>
>> ---
>> fs/btrfs/super.c | 65 +++++++++++++++++++++++++++++++++++++++++++++---
>> 1 file changed, 62 insertions(+), 3 deletions(-)
>>
>> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
>> index 64f20725615a..4512f25dcf69 100644
>> --- a/fs/btrfs/super.c
>> +++ b/fs/btrfs/super.c
>> @@ -310,7 +310,6 @@ enum {
>> Opt_datasum, Opt_nodatasum,
>> Opt_defrag, Opt_nodefrag,
>> Opt_discard, Opt_nodiscard,
>> - Opt_nologreplay,
>> Opt_ratio,
>> Opt_rescan_uuid_tree,
>> Opt_skip_balance,
>> @@ -323,7 +322,6 @@ enum {
>> Opt_subvolid,
>> Opt_thread_pool,
>> Opt_treelog, Opt_notreelog,
>> - Opt_usebackuproot,
>> Opt_user_subvol_rm_allowed,
>>
>> /* Deprecated options */
>> @@ -341,6 +339,8 @@ enum {
>> #ifdef CONFIG_BTRFS_FS_REF_VERIFY
>> Opt_ref_verify,
>> #endif
>> + /* Rescue options */
>> + Opt_rescue, Opt_usebackuproot, Opt_nologreplay,
>
> The options have been sorted and grouped, don't mess it up again please.
> Check the list and find a better place than after the deprecated and
> debugging options.
>
Attachment:
signature.asc
Description: OpenPGP digital signature
