Messages in this thread | | | Date | Wed, 3 Aug 2022 14:19:13 +0800 | Subject | Re: [PATCH]btrfs: Fix fstest case btrfs/219 | From | Anand Jain <> |
| |
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
| |