Re: [PATCH 1/2] btrfs: Introduce the virtual_fsid feature

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 2023/7/6 06:52, Guilherme G. Piccoli wrote:
On 05/05/2023 04: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.

By this, we can avoid multiple meanings of the same super member, nor
need any special mount option.
Remember, mount option is never a good way to enable/disable a new feature.

The better method to enable/disable a feature should be mkfs and btrfstune.

Then go mostly the same of your patch, but maybe with something extra:

- Disbale multi-dev code
    Include device add/replace/removal, this is already done in your
    patch.

- Completely skip device scanning
    I see no reason to keep btrfs with SINGLE_DEV feature to be added to
    the device list at all.
    It only needs to be scanned at mount time, and never be kept in the
    in-memory device list.


Hi Qu, I'm implementing this compat_ro idea of yours, but I'd like to
ask your input in some "design decisions" I'm facing here.

(a) I've skipped the device_list_add() step of adding the recent created
fs_devices struct to fs_uuids list, but I kept the btrfs_device creation
step. With that, the mount of two filesystems with same fsid fails..at
sysfs directory creation!

Of course - because it tries to add the same folder name to
/sys/fs/btrfs/ !!! I have some options here:

(I) Should I keep using a random generated fsid for single_dev devices,
in order we can mount many of them while not messing too much with the
code? I'd continue "piggybacking" on metadata_uuid idea if (I) is the
proper choice.

(II) Or maybe track down all fsid usage in the code (like this sysfs
case) and deal with that? In the sysfs case, we could change that folder
name to some other format, like fsid.NUM for single_dev devices, whereas
NUM is an incremental value for devices mounted with same fsid.

I'm not too fond of this alternative due to its complexity and "API"
breakage - userspace already expects /sys/fs/btrfs/ entries to be the fsid.

(III) Should we hide the filesystem from sysfs (and other potential
conflicts that come from same fsid mounting points)? Seems a hideous
approach, due to API breakage and bug potentials.

Personally speaking, I would go one of the following solution:

- Keep the sysfs, but adds a refcount to the sysfs related structures
  If we still go register the sysfs interface, then we have to keep a
  refcount, or any of the same fsid got unmounted, we would remove the
  whole sysfs entry.

- Skip the sysfs entry completely for any fs with the new compat_ro flag
  This would your idea (III), but the sysfs interface itself is not that
  critical (we add and remove entries from time to time), so I believe
  it's feasible to hide the sysfs for certain features.


Maybe there are other choices, better than mine - lemme know if you have
some ideas!

Also, one last question/confirmation: you mentioned that "The better
method to enable/disable a feature should be mkfs" - you mean the same
way mkfs could be used to set features like "raid56" or "no-holes"?

Yes.


By checking "mkfs.btrfs -O list-all", I don't see metadata_uuid for
example, which is confined to btrfstune it seems. I'm already modifying
btrfs-progs/mkfs, but since I'm emailing you, why not confirm, right? heh

I'm not familiar with metadata_uuid, but there are similar features like
seeding, which is only available in btrfstune, but not in mkfs.

It's not that uncommon, but yeah, you have found something we should
improve on.

Thanks,
Qu


Thanks again for the advice and suggestions - much appreciated!
Cheers,


Guilherme




[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [NTFS 3]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [NTFS 3]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux