On 2023/5/6 01:34, Goffredo Baroncelli wrote:
On 05/05/2023 09.21, Qu Wenruo wrote:
I would prefer a much simpler but more explicit method.
Just introduce a new compat_ro feature, maybe call it SINGLE_DEV.
It is not clear to me if we need that.
I don't understand in what checking for SINGLE_DEV is different from
btrfs_super_block.disks_num == 1.
Because disks_num == 1 doesn't exclude the ability to add new disks in
the future.
Without that new SINGLE_DEV compat_ro, we should still do the regular
device scan.
Let me to argument:
I see two scenarios:
1) mount two different fs with the same UUID NOT at the same time:
This could be done now with small change in the kernel:
- we need to NOT store the data of a filesystem when a disk is
scanned IF it is composed by only one disk
- after the unmount we need to discard the data too (checking again
that the filesystem is composed by only one disk)
No limit is needed to add/replace a disk. Of course after a disk is
added a filesystem with the same UUID cannot be mounted without a
full cycle of --forget.
The problem is, what if:
- Both btrfs have single disk
- Both btrfs have the same fsid
- Both btrfs have been mounted
- Then one of btrfs is going to add a new disk
We either:
- Don't scan nor trace the device/fsid anyway
Then after unmount, the new two disks btrfs can not be mounted
and need extra scan/forgot/whatever to reassemble list.
And that would also cause fsid conflicts if not properly handled
between the single and multi disk btrfs.
- Scan and record the fsid/device at device add time
This means we should reject the device add.
This can sometimes cause confusion to the end user, just because they
have mounted another fs, now they can not add a new device.
And this is going to change device add code path quite hugely.
We currently expects all device scan/trace thing done way before
mount.
Such huge change can lead to hidden bugs.
To me, neither is good to the end users.
A SINGLE_DEV feature would reject the corner case in a way more
user-friendly and clear way.
With SINGLE_DEV feature, just no dev add/replace/delete no matter
what.
I have to point out that this problem would be easily solved in
userspace if we switch from the current model where the disks are
scanned asynchronously (udev which call btrfs dev scan) to a model
where the disk are scanned at mount time by a mount.btrfs helper.
A mount.btrfs helper, also could be a place to put some more clear error
message like "we cannot mount this filesystem because one disk of a
raid5 is missing, try passing -o degraded"
or "we cannot mount this filesystem because we detect a brain split
problem" ....
2) mount two different fs with the same UUID at the SAME time:
This is a bit more complicated; we need to store a virtual UUID
somewhere.
However sometime we need to use the real fsid (during a write),
and sometime we need to use the virtual_uuid (e.g. for
/sys/fs/btrfs/<uuid>)
Another thing is, we already have too many uuids.
Some are unavoidable like fsid and device uuid.
But I still prefer not to add a new layer of unnecessary uuids.
Thanks,
Qu
Both in 1) and 2) we need to/it is enough to have
btrfs_super_block.disks_num == 1
In the case 2) using a virtual_uuid mount option will prevent
to add a disk.
