On 10/18/19 12:32 AM, David Sterba wrote:
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.
David,
I understand the -f warning part we can have that too here, but that
wasn't review comments so far.
Per this thread's comments so far, it was only to keep undo fsid part
blocked and reasoning was arbitrary, neither the code or patch which
blocked it explains. It only sounded like punish the expert users
so that we can keep the naive users safe.
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
From the bug as I received.
create btrfs root-image for the vm use.
copy root-image to root-image1
copy root-image to root-image2
start vm1 using root-image1
(when root-image1 has issues; shutdown vm1)
start vm2 using root-image2 with root-image1 accessible.
login to vm2
(change fsid so that root-image1 can be mounted)
btrsftune -m remote/root-image1
mount -o loop remote/root-image1 /mnt
analyze, collect logs, fix remote/root-image1
umount /mnt
(Revert the changed fsid so that vm2 can boot) <<<< Usecase wants
this to work
btrfstune -M $(btrfs in dump-super remote/root-image2 | \
grep metadata_uuid | awk '{print $2}') \
remote/root-image2
logout from vm2
start vm1 using root-image1
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,
The change log explains the usecase. Looks like it wasn't sufficient
to bring the whole context correctly. My bad.
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.
This usecase was using ext4 before. They aren't happy that btrfs needs
btrfstune -m to mount a duplicate fsid and is still hoping that
we would provide a better seamless solution like as below so that it
to circumvent the duplicating fsid.
mount -o loop,random_temp_fsid root-image2 /mnt
where option random_tmp_fsid creates a kernel in-memory random-fsid
to mount the duplicate fsid.
If we are ok to support -o random_tmp_fsid (which I received a nack on
the other channel), then I can drop this btrfs-progs patch altogether.
I think we should allow the flexibility seamlessly. Also this kernel
approach doesn't confuse the user space as perceived, as both
metadata_uuid and fsid remains unchanged and it helps this use case
much better.
Thanks, Anand
