On Tue, May 2, 2017 at 5:47 PM, Christoph Hellwig <hch@xxxxxx> wrote: > On Tue, May 02, 2017 at 05:27:36PM +0300, Amir Goldstein wrote: >> On Tue, May 2, 2017 at 5:17 PM, Christoph Hellwig <hch@xxxxxx> wrote: >> > On Tue, May 02, 2017 at 05:13:56PM +0300, Amir Goldstein wrote: >> >> How can it create problems if uniqueness is not guaranteed with >> >> Current s_uuid? Even if we did make the xfs uuid table code generic >> >> It couldn't be the vfs default. Filesystems will have to opt in. >> > >> > It creates problems if you e.g. mount an ext4 fs and a dm snaphot of >> > it. The non-XFS file systems are simply buggy in this regard. >> > >> > Non-uniqueue uuids are an absolute no-go. >> >> I'm not sure I follow your specific concern here. >> Surely you are not proposing to get rid of the nouuid >> mount option, are you? So what's the point of hiding >> the fact that there are 2 mounted filesystems with the >> same uuid from VFS? > > Because it breaks people using s_uuid. Christoph, I do agree with you that that situation where s_uuid is unique on a system is preferred over what we have today. I disagree that not respecting uniqueness that was not respected until now has the potential to break any user. New users that require uniqueness from s_uuid, those are the ones that should be checking for the not yet existing flag SB_I_AM_UNIQUE. Old users cannot claim that uniqueness has been broken. Whether or not there are users that need to check SB_I_AM_UNIQUE and whether or not we need to hoist the xfs unique uuid implementation to VFS remains to be seen. > Take a look at cleancache, > which identifies a pool with it. Once you have to snapshot with > the same uuid the pool concept is broken. Same for any sort of > use in file handles. > I am not sure that cleancache is a good example for your argument. s_uuid is needed for the cleancache_init_shared_fs() case, only used by ocfs2 in-tree fs. From what I can gather from cleancache documentation, this is meant for a use case where 2 different VMs use the same shared pool. If I understood correctly, then making s_uuid unique on a single system is not good enough for this use case anyway. > The U in UUID stands for unique, and we must ensure that's actually > true. Maybe we should re-interpret the U of Universal as Utopic ;-) > >> >> Because that is the the only implication of exporting >> s_uuid regardless of nouuid mount option. >> >> Whether or not ext4 and other fs should restrict >> multi mount of same uuid is a completely different >> issue. > > It's not. The whole point of exporting s_uuid is to have a uniqueue > identifier for a superblock. If it's not actually uniqueue there is > no point in having or using it. Well it's useful for my use case. I need s_uuid as a sanity check and IIUC, so does cleancache. I don't know about EVM/IMA, but apparently, it did well with pseudo uniqueness so far. Cheers, Amir.