Messages in this thread | | | Date | Fri, 22 Jul 2022 01:34:11 -0400 | Subject | Re: [PATCH]btrfs: Fix fstest case btrfs/219 | From | hmsjwzb <> |
| |
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.
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.
Thanks
| |