On 2018/9/2 下午5:56, Nikolay Borisov wrote:
>
>
> On 29.08.2018 15:47, Qu Wenruo wrote:
>>
>>
>> On 2018/8/29 下午8:33, Nikolay Borisov wrote:
>>>
>>>
>>> On 29.08.2018 15:09, Qu Wenruo wrote:
>>>>
>>>>
>>>> On 2018/8/29 下午4:35, Nikolay Borisov wrote:
>>>>> Here is the userspace tooling support for utilising the new metadata_uuid
>>>>> field,
>>>>> enabling the change of fsid without having to rewrite every metadata
>>>>> block. This
>>>>> patchset consists of adding support for the new field to various tools
>>>>> and
>>>>> files (Patch 1). The actual implementation of the new -m|-M options
>>>>> (which are
>>>>> described in more detail in Patch 2). A new misc-tests testcasei (Patch
>>>>> 3) which
>>>>> exercises the new options and verifies certain invariants hold (these are
>>>>> also
>>>>> described in Patch2). Patch 4 is more or less copy of the kernel
>>>>> conuterpart
>>>>> just reducing some duplication between btrfs_fs_info and btrfs_fs_devices
>>>>> structures.
>>>>
>>>> So to my understand, now we have another layer of UUID.
>>>>
>>>> Before we have one fsid, both used in superblock and tree blocks.
>>>>
>>>> Now we have 2 fsid, the one used in tree blocks are kept the same, but
>>>> changed its name to metadata_uuid in superblock.
>>>> And superblock::fsid will become a new field, and although they are the
>>>> same at mkfs time, they could change several times during its operation.
>>>>
>>>> This indeed makes uuid change super fast, only needs to update all
>>>> superblocks of the fs, instead of all tree blocks.
>>>>
>>>> However I have one nitpick of the design. Unlike XFS, btrfs supports
>>>> multiple devices.
>>>> If we have a raid10 fs with 4 devices, and it has already gone through
>>>> several UUID change (so its metadata uuid is already different from fsid).
>>>>
>>>> And during another UUID change procedure, we lost power while only
>>>> updated 2 super blocks, what will happen for kernel device assembly?
>>>>
>>>> (Although considering how fast the UUID change would happen, such case
>>>> should be super niche)
>>>
>>> Then I guess you will be fucked. I'm all ears for suggestion how to
>>> rectify this without skyrocketing the complexity. The current UUID
>>> rewrite method sets a flag int he superblock that FSID change is in
>>> progress and clears it once every metadatablock has been rewritten. I
>>> can piggyback on this mechanism but I'm not sure it provides 100%
>>> guarantee. Because by the some token you can set this flag, start
>>> writing the super blocks then lose power and then only some of the
>>> superblocks could have this flag set so we back at square 1.
>>
>> Well, forget it, considering how fast the new method is, such case
>> should be really rare.
>>
>>>
>>>>
>>>>>
>>>>> The intended usecase of this feature is to give the sysadmin the ability
>>>>> to
>>>>> create copies of filesystesm, change their uuid quickly and mount them
>>>>> alongside
>>>>> the original filesystem for, say, forensic purposes.
>>>>>
>>>>> One thing which still hasn't been set in stone is whether the new options
>>>>> will remain as -m|-M or whether they should subsume the current -u|-U -
>>>>> from
>>>>> the point of view of users nothing should change.
>>>>
>>>> Well, user would be surprised by how fast the new -m is, thus there is
>>>> still something changed :)
>>>>
>>>> I prefer to subsume current -u/-U, and use the new one if the incompat
>>>> feature is already set. Or fall back to original behavior.
>>>>
>>>> But I'm not a fan of using INCOMPAT flags as an indicator of changed
>>>> fsid/metadata uuid.
>>>> INCOMPAT feature should not change so easily nor acts as an indicator.
>>>>
>>>> That's to say, the flag should only be set at mkfs time, and then never
>>>> change unlike the 2nd patch (I don't even like btrfstune to change
>>>> incompat flags).
>>>>
>>>> E.g.
>>>> mkfs.btrfs -O metadata_uuid <device>, then we could use the new way to
>>>> change fsid without touching metadata uuid.
>>>> Or we could only use the old method.
>>>
>>> I disagree, I don't see any benefit in this but only added complexity.
>>> Can you elaborate more ?
>>
>> Well, the incompat feature is really introducing some incompatible
>> on-disk format change.
>> So if you're introducing the new metadata_uuid field, no matter if it
>> differs from fsid or not, it's a new field, and the incompat flag should
>> be set.
>>
>> To me, older kernel could recognize the new format when fsid matches
>> metadata uuid (since there in your current patchset, such case will not
>> have incompat flag set) is a little dangerous.
>>
>> What will happen if such old kernel/btrfs-progs tries to change fsid?
>> Older btrfs-progs doesn't know there is a new field, it will not touch
>> the metadata uuid field, just changing the fsid field along with all
>> tree blocks.
>>
>> This will cause a fs whose fsid doesn't match metadata uuid and has no
>> incompat flag set. This is definitely leading to compatibility problem.
>
> Nope, when both fsid/metadata_uuid match and the INCOMPAT flag is not
> set then on-disk we never save the metadata_uuid value and this value is
> set only in-memory, the only thing on-disk is fsid.
In my opinion, this is already more complex than forced new INCOMPAT
flag all the time.
You're already using 4 different branches to workaround the
compatibility, which already looks more complex than all time INCOMPAT flag.
And to me, I didn't really think using all time INCOMPAT flag is a
problem at all.
If someone really want to use it, btrfstune could be changed to add
METADATA_UUID flag easily for old fs.
And if the feature is well received we could just make this feature as
default later.
Thanks,
Qu
> In this case if one
> decides to rewrite the fsuid then this is fine - btrfs-progs disallow
> using -m|-M when fsid rewrite is in progress. So when the fsid/metadata
> is not set to old progs the kernel continue working as-is. When it's
> changed then the incompat flags will prevent us from doing harmful
> modifications.
>
>>
>> So we need to follow the incompat flags rule strictly, if on-disk format
>> is changed, we need the new incompat flag.
>>
>> Thanks,
>> Qu
>>>
>>>
>>>>
>>>> Thanks,
>>>> Qu
>>>>
>>>>> So this is something which
>>>>> I'd like to hear from the community. Of course the alternative of
>>>>> rewriting
>>>>> the metadata blocks will be assigne new options - perhaps -m|M ?
>>>>>
>>>>> I've tested this with multiple xfstest runs with the new tools installed
>>>>> as
>>>>> well as running btrfs-progs test and have observed no regressions.
>>>>>
>>>>> Nikolay Borisov (4):
>>>>> btrfs-progs: Add support for metadata_uuid field.
>>>>> btrfstune: Add support for changing the user uuid
>>>>> btrfs-progs: tests: Add tests for changing fsid feature
>>>>> btrfs-progs: Remove fsid/metdata_uuid fields from fs_info
>>>>>
>>>>> btrfstune.c | 174
>>>>> ++++++++++++++++++++++++-----
>>>>> check/main.c | 2 +-
>>>>> chunk-recover.c | 17 ++-
>>>>> cmds-filesystem.c | 2 +
>>>>> cmds-inspect-dump-super.c | 22 +++-
>>>>> convert/common.c | 2 +
>>>>> ctree.c | 15 +--
>>>>> ctree.h | 8 +-
>>>>> disk-io.c | 62 ++++++++--
>>>>> image/main.c | 25 +++--
>>>>> tests/misc-tests/033-metadata-uuid/test.sh | 137 +++++++++++++++++++++++
>>>>> volumes.c | 37 ++++--
>>>>> volumes.h | 1 +
>>>>> 13 files changed, 431 insertions(+), 73 deletions(-)
>>>>> create mode 100755 tests/misc-tests/033-metadata-uuid/test.sh
>>>>>
>>>>
