On Thu, Oct 19, 2023 at 06:40:27AM -0700, Christoph Hellwig wrote:
> On Thu, Oct 19, 2023 at 10:33:36AM +0200, Christian Brauner wrote:
> > some device removal. I also messed around with the loop code and
> > specifically used LOOP_CHANGE_FD to force a disk_force_media_change() on
> > a filesystem.
>
> Can you wire that up for either blktests or xfstests as well?
Yeah, I'll try to find some time to do this.
So while I was testing this I realized that the behavior of
LOOP_CHANGE_FD changed in commit 9f65c489b68d ("loop: raise media_change
event") and I'm not clear whether this is actually correct or not.
loop(4) states
"Switch the backing store of the loop device to the new file identified
file descriptor specified in the (third) ioctl(2) argument, which is an
integer. This operation is possible only if the loop device is
read-only and the new backing store is the same size and type as the
old backing store."
So the original use-case for this ioctl seems to have been to silently
swap out the backing store. Specifcially it seems to have been used once
upon a time for live images together with device mapper over read-only
loop devices. Where device mapper can direct the writes to some other
location or sm.
Assuming that's correct, I think as long as you have something like
device mapper that doesn't use blk_holder_ops it would still work. But
that commit changed behavior for filesystems because we now do:
loop_change_fd()
-> disk_force_media_change()
-> bdev_mark_dead()
-> bdev->bd_holder_ops->mark_dead::fs_mark_dead()
So in essence if you have a filesystem on a loop device via:
truncate -s 10G img1
mkfs.xfs -f img1
LOOP_DEV=$(sudo losetup -f --read-only --show img1)
truncate -s 10G img2
sudo ./loop_change_fd $LOOP_DEV ./img2
We'll shut down that filesystem. I personally think that's correct and
it's buggy not to do that when we have the ability to inform the fs
about media removal but technically it kinda defeats the original
use-case for LOOP_CHANGE_FD.
In practice however, I don't think it matters because I think no one is
using LOOP_CHANGE_FD in that way. Right now all this is a useful for is
a bdev_mark_dead() test.
And one final question:
dev_set_uevent_suppress(disk_to_dev(lo->lo_disk), 1);
disk_force_media_change(lo->lo_disk);
/* more stuff */
dev_set_uevent_suppress(disk_to_dev(lo->lo_disk), 0);
What exactly does that achieve? Does it just delay the delivery of the
uevent after the disk sequence number was changed in
disk_force_media_change()? Because it doesn't seem to actually prevent
uevent generation.
