lkml.org 
[lkml]   [2022]   [Aug]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH]btrfs: Fix fstest case btrfs/219
From





On 27/07/2022 02:38, David Sterba wrote:
> On Fri, Jul 22, 2022 at 01:34:11AM -0400, hmsjwzb wrote:
>> On 7/21/22 09:37, Nikolay Borisov wrote:
>>> On 21.07.22 г. 11:36 ч., Flint.Wang wrote:
>>>> Hi,
>>>> fstest btrfs/291 failed.
>>>>
>>>> [How to reproduce]
>>>> mkdir -p /mnt/test/219.mnt
>>>> xfs_io -f -c "truncate 256m" /mnt/test/219.img1
>>>> mkfs.btrfs /mnt/test/219.img1
>>>> cp /mnt/test/219.img1 /mnt/test/219.img2
>>>> mount -o loop /mnt/test/219.img1 /mnt/test/219.mnt
>>>> umount /mnt/test/219.mnt
>>>> losetup -f --show /mnt/test/219.img1 dev
>>>> mount /dev/loop0 /mnt/test/219.mnt
>>>> umount /mnt/test/219.mnt
>>>> mount -o loop /mnt/test/219.img2 /mnt/test/219.mnt
>>>>
>>>> [Root cause]
>>>> if (fs_devices->opened && found_transid < device->generation) {
>>>>     /*
>>>>      * That is if the FS is _not_ mounted and if you
>>>>      * are here, that means there is more than one
>>>>      * disk with same uuid and devid.We keep the one
>>>>      * with larger generation number or the last-in if
>>>>      * generation are equal.
>>>>      */
>>>>     mutex_unlock(&fs_devices->device_list_mutex);
>>>>     return ERR_PTR(-EEXIST);
>>>> }
>>>>
>>>> [Personal opinion]
>>>> User might back up a block device to another. I think it is improper
>>>> to forbid user from mounting it.
>>>>
>>>> Signed-off-by: Flint.Wang <hmsjwzb@zoho.com>
>>>
>>>
>>> This lacks any explanation whatsoever so it's not possible to judge whether the fix is correct or not.
>>>
>>>> ---
>>>>   fs/btrfs/volumes.c | 2 +-
>>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
>>>> index 6aa6bc769569a..76af32032ac85 100644
>>>> --- a/fs/btrfs/volumes.c
>>>> +++ b/fs/btrfs/volumes.c
>>>> @@ -900,7 +900,7 @@ static noinline struct btrfs_device *device_list_add(const char *path,
>>>>            * tracking a problem where systems fail mount by subvolume id
>>>>            * when we reject replacement on a mounted FS.
>>>>            */
>>>> -        if (!fs_devices->opened && found_transid < device->generation) {
>>>> +        if (fs_devices->opened && found_transid < device->generation) {
>>>>               /*
>>>>                * That is if the FS is _not_ mounted and if you
>>>>                * are here, that means there is more than one
>>
>> Hi Nikolay,
>>
>> It seems the failure of btrfs/219 needs some explanation.
>>
>> Here is the thing.
>> 1. A storage device A with btrfs filesystem is running on a host.
>> 2. For example, we backup the device A to an exactly some device B.
>> 3. The device A continue to run for a while so the device->generation is getting bigger.
>> 4. Then you umount the device A and try to mount device B.
>> 5. Kernel find that device A has the same UUID as device B and has bigger device->generation.
>> So the mount request of device B will be rejected.
>
> That's on purpose, devices are matched by UUIDs and making block copies
> of the same filesystem is known "don't do that" and discouraged.
>
> If you must store the block copies then you can change the UUID by
> btrfstune, there are two ways (fast metadata_uuid, and slow rewriting
> all metadata uuids in all blocks).
>
>>
>> if (!fs_devices->opened && found_transid < device->generation) {
>> /*
>> * That is if the FS is _not_ mounted and if you
>> * are here, that means there is more than one
>> * disk with same uuid and devid.We keep the one
>> * with larger generation number or the last-in if
>> * generation are equal.
>> */
>> mutex_unlock(&fs_devices->device_list_mutex);
>> return ERR_PTR(-EEXIST);
>> }
>>
>> I think it is improper to reject that request. Because device A is not in open state.
>
> But this would prevent mounting A. There should really be some way to
> distiguish the filesystems, the block device is not a stable identifier,
> the UUID is. Imagine having 10 copies of the same filesystem identified
> by the same UUID and device UUID, but with different generations and
> data. That's asking for problems.
>
> There's not much the filesystem driver can do than to avoid using old
> devices and giving preference to the highest generation device. All
> devices with btrfs signature are registered in memory and this is the
> primary source when mounting the devices, not the block device itself.


David,

The unintegrated patch [1] also used the same use case.

[1]
[PATCH v2][RESEND] btrfs: allow single disk devices to mount with
older generations

IMO device-copy and mount (without changing the UUID) can be allowed
for a single device btrfs volume only. We even have a fstest case
btrfs/219, which tests single device duplicate UUIDs.

Please integrate [1].

-Anand

\
 
 \ /
  Last update: 2022-08-03 08:21    [W:0.082 / U:0.236 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site