On Thu, Sep 12, 2019 at 08:45:50AM +0800, Anand Jain wrote: > In case of vm guest images copied from the golden image there is no > physical device or loop device or nbd device until its configured on > the host when required, so check for duplicate fsid at the time of > btrfstune -M is not convincing or a very limited approach. > > >> - As I said before, its a genuine use-case here where the user wants to > >> revert the fsid change, so that btrfs fs root image can be booted. > >> Its up-to the user if fsid is duplicate in the user space, as btrfs > >> kernel rightly fails the mount if its duplicate fsid anyways. > > > > Reverting the uuid is fine > > ok thanks. > > > and requiring the uuids to be unique is > > preventing the users doing stupid things unknowingly. > > Right it should be done. But.. > btrfstune -M is a wrong place. No it's not, it's exactly the right place because that's the moment when user is doing an action that has consequences and if there's room for mistakes, it should not be too easy. If the usecase is valid, it should be possible though. > Because it can't avoid all the > cases of fsid getting duplicated. But this does not matter. We're not protecting an image against external changes, but by accidental change by a concrete user action at a specific time. > Even after btrfstune -M, the fsid can be duplicated by the user. Yes of course, eg. by doing 'echo UUID | dd of=image seek=... bs=...', there's no way we can prevent users from doing that. But that command is up to user to check for the target device and we have no responsibility for the damage. > So what's the point in restricting the btrfstune -M and fail to > undo the changed fsid. The point is to prevent accidental damage. For the same reasons we have 'mkfs.btrfs -f' or 'btrfd device add -f' or even 'btrfstune -f -S 0'. I'm surprised and slightly disappointed that we have to go through these points, this is 101 of user interfaces. > > You seem to have a > > usecase where even duplicate uuids are required but I'm afraid it's not > > all clear how is it supposed to work. A few more examples or commands > > would be helpful. > > In the use case here, even the host is also running a copy of the golden > image (same fsid as vm guest) and because of duplicate fsid you can > only mount a vm guest image on the host after the btrfstune -m command > on the vm guest image. But after you have done that, as the vm guest > fsid is changed, it fails to boot, unfortunately changed fsid can not > be undone without this patch. I can't say I have a clear picture yet, can you please describe it in some more desriptive way, like host1: create image1-uuid1 host2: copy image1-uuid1 to image1-uuid2 host2: use image1-uuid2 host2: change image1-uuid2 back to uuid1 <-- I want this to work > The fsid can be duplicate by many different other ways anyways. So in > this case how does it matter if btrfstune -M tries to ensure that fsid > is unique among the blkid known set of devices, which may change any > time after btrfstune -M as well (just copy a vm guest and map it to > a loop device). So btrfstune -M should be free from this check and > help the use case as above. > > And if we are concerned about the duplicate fsid as I asked Nikolay > before, we need to know what are problems in specifies, so that it can > be fixed in separate patches, but definitely not in btrfstune -M as > it can't fix the duplicate fsid problem completely as vm images can > be copied and mapped to a loop/nbd device anytime even after > btrfstune -M. This only reiterates and was aswered above. The usecase was not explained at the beginning, so it got a NAK because it brought a potentially dangerous change. The next step is usecase clarification so we understand if there's a way to make it work for the common and your usecase. And we're almost there, but instead of handwaving that we can't prevent users doing lots of things, a simple 'so let's allow duplicate uuids with -f with a big warning and a paragraph in documentation, and btw here's a testcase' would do. The patch could have been merged a month ago.
