On 2018年01月08日 13:58, Anand Jain wrote:
>
>
> On 01/08/2018 01:25 PM, Qu Wenruo wrote:
>>
>>
>> On 2018年01月08日 13:13, Anand Jain wrote:
>>>
>>>
>>> On 01/08/2018 01:08 PM, Qu Wenruo wrote:
>>>>
>>>>
>>>> On 2018年01月08日 13:04, Anand Jain wrote:
>>>>> Userland sets SUPER_FLAG_CHANGING_FSID and resets it only when
>>>>> changing
>>>>> fsid is complete. Its not a good idea to mount the device anything in
>>>>> between, so this patch fails the mount if SB SUPER_FLAG_CHANGING_FSID
>>>>> is set.
>>>>>
>>>>> Signed-off-by: Anand Jain <anand.jain@xxxxxxxxxx>
>>>>> cc: wqu@xxxxxxxx
>>>>> ---
>>>>> v1->v2: Oops. Fix cut and paste error, remove ~. My bad.
>>>>>
>>>>> fs/btrfs/disk-io.c | 7 ++++++-
>>>>> include/uapi/linux/btrfs_tree.h | 1 +
>>>>> 2 files changed, 7 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
>>>>> index a69e5944dc08..0dd215258ff9 100644
>>>>> --- a/fs/btrfs/disk-io.c
>>>>> +++ b/fs/btrfs/disk-io.c
>>>>> @@ -61,7 +61,8 @@
>>>>> BTRFS_HEADER_FLAG_RELOC |\
>>>>> BTRFS_SUPER_FLAG_ERROR |\
>>>>> BTRFS_SUPER_FLAG_SEEDING |\
>>>>> - BTRFS_SUPER_FLAG_METADUMP)
>>>>> + BTRFS_SUPER_FLAG_METADUMP |\
>>>>> + BTRFS_SUPER_FLAG_CHANGING_FSID)
>>>>
>>>> Still the same problem here.
>>>>
>>>> Since that super flag is excluded in btrfs_check_super_valid(), I
>>>> didn't
>>>> see any meaning including it into BTRFS_SUPER_FLAG_SUPP, and it will
>>>> confuse later BTRFS_SUPER_FLAG_SUPP users.
>>>
>>>
>>> Anyway BTRFS_SUPER_FLAG_SUPP as such does not stop to mount, it
>>> just warns. So when there is a new flag which is not supported
>>> we get this warning. As in the case with BTRFS_SUPER_FLAG_METADUMP_V2.
>>> BTRFS_SUPER_FLAG_METADUMP_V2 in kernel is defined but not supported.
>>> However since we are supporting BTRFS_SUPER_FLAG_CHANGING_FSID, so
>>> adding to BTRFS_SUPER_FLAG_SUPP make sense to me. ?
>>
>> We still refuse to mount the fs if CHANGING_FSID is set, which I believe
>> should be called "unsupported"
>>
>> On the other hand it would be better to include METADUMP_V2 into SUPP,
>> as we allow it to be mounted and now kernel knows that flag, output a
>> warning doesn't seems necessary now.
>>
>> It would be even better to change the later SUPP flags checks, into
>> something not only output some meaningless numeric flags, but also
>> output the human readable flags if it's recognized but unsupported. (For
>> CHANGIND_FSID).
>
> Originally [1] looks like there isn't any design specific reason not to
> fail the mount (which I didn't want to alter to fail) but just to warn.
> Will change it to fail.
>
> [1]
> commit 319e4d0661e5323c9f9945f0f8fb5905e5fe74c3
> btrfs: Enhance super validation check
My fault, I should add the missing super flags at that time, and fail
the mount.
Thanks,
Qu
>
> Thanks, Anand
>
>> Thanks,
>> Qu
>>
>>>
>>> Thanks, Anand
>>>
>>>
>>>> Thanks,
>>>> Qu
>>>>
>>>>> static const struct extent_io_ops btree_extent_io_ops;
>>>>> static void end_workqueue_fn(struct btrfs_work *work);
>>>>> @@ -3906,6 +3907,10 @@ static int btrfs_check_super_valid(struct
>>>>> btrfs_fs_info *fs_info)
>>>>> btrfs_err(fs_info, "no valid FS found");
>>>>> ret = -EINVAL;
>>>>> }
>>>>> + if (btrfs_super_flags(sb) & BTRFS_SUPER_FLAG_CHANGING_FSID) {
>>>>> + btrfs_err(fs_info, "SUPER_FLAG_CHANGING_FSID is set");
>>>>> + ret = -EINVAL;
>>>>> + }
>>>>> if (btrfs_super_flags(sb) & ~BTRFS_SUPER_FLAG_SUPP)
>>>>> btrfs_warn(fs_info, "unrecognized super flag: %llu",
>>>>> btrfs_super_flags(sb) & ~BTRFS_SUPER_FLAG_SUPP);
>>>>> diff --git a/include/uapi/linux/btrfs_tree.h
>>>>> b/include/uapi/linux/btrfs_tree.h
>>>>> index 38ab0e06259a..aff1356c2bb8 100644
>>>>> --- a/include/uapi/linux/btrfs_tree.h
>>>>> +++ b/include/uapi/linux/btrfs_tree.h
>>>>> @@ -457,6 +457,7 @@ struct btrfs_free_space_header {
>>>>> #define BTRFS_SUPER_FLAG_SEEDING (1ULL << 32)
>>>>> #define BTRFS_SUPER_FLAG_METADUMP (1ULL << 33)
>>>>> #define BTRFS_SUPER_FLAG_METADUMP_V2 (1ULL << 34)
>>>>> +#define BTRFS_SUPER_FLAG_CHANGING_FSID (1ULL << 35)
>>>>> /*
>>>>>
>>>>
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Attachment:
signature.asc
Description: OpenPGP digital signature
