On Mon, May 21, 2018 at 07:20:05AM -0400, Jeff Layton wrote: > > As a side note, it looks like __loop_update_dio will discard an error > from vfs_fsync, so certain ioctls against a loop device can cause errors > to be lost. It seems like those ought to get propagated to userland or > to the blockdev's mapping somehow. I'm not sure it's worth it. All of the ioctls that call loop_update_dio are used just to configure the loop device. In practice they are never called once the loop device is actually running. It might be a better choice is to just define that errors get reset if there is any attempt to reconfigure the loop device. One of these ioctls change the offset that a loop device maps onto the backing file. So for example, while offset 0 of the loop device might have corresponds beginning of the backing file, after executing the ioctl, offset 0 of the loop device no corresponds to offset 2MB of the backing store. This might happen if we are resetting the loop device to point at the first partition of a disk image, for example. I really don't think that preserving the error status when we are doing that kind of radical configuration makes sense. Or for example, when we change the block size of the loop device; if the underlying backing store is an IBM Mainframe DASD with a 2k block size, the reason why the error was signalled was because there was an attempt to write a 1k block onto a 2k block device. So again, resetting the error status of the loop device is the right thing to do. The final thing that's worth perhaps exploring is whether or not we need all of these knobs and ioctls. In particular, LOOP_CHANGE_FD is used by losetup. It exists because back in prehistory, some distribution installer needed it for some obscure corner case. If I remember correctly, it had to do with switching out the initramfs floppy disk with the root file system floppy disk. We've wasted developer bandwidth trying to deal with syzbot complaints about that ioctl, and it's not clear it was worth the effort, because it's not clear any real code actually _needs_ that ioctl. It might have been easier to comment out the ioctl, and see if anyone screamed. Given that loop device is maintianed mostly on the margins, and it's not clear that lots of complicated error handling during setup is really necessary, this is basically a please to keep things simple, or at least no more complicated than it has to be. Cheers, - Ted
