On Sun, 2018-05-20 at 15:41 -0400, Theodore Y. Ts'o wrote:
> On Sun, May 20, 2018 at 12:20:09PM -0700, Matthew Wilcox wrote:
> > Oh! I misunderstood. I thought that bd_inode->i_mapping pointed to
> > lo_backing_file->f_mapping.
> >
> > So how about this to preserve the error _in the file with the error_?
> >
> > if (bdev) {
> > bdput(bdev);
> > invalidate_bdev(bdev);
> > + filp->f_mapping->wb_err = bdev->bd_inode->i_mapping->wb_err;
> > + bdev->bd_inode->i_mapping->wb_err = 0;
> > }
> > set_capacity(lo->lo_disk, 0);
> > loop_sysfs_exit(lo);
>
> I don't think it's necessary. wb_err on the underlying file would
> have already been set when the error was set. So if someone tried
> calling fsync(2) on the underlying file, it would have collected the
> error already. Re-setting the error when we detach the loop device
> doesn't seem to make any sense.
>
(cc'ing linux-block)
Yes, the error would have been set on the original file already. As long
as lo_req_flush or __loop_update_dio weren't called before we detach the
device, it should still be possible to call fsync on the original fd and
get back the error.
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.
--
Jeff Layton <jlayton@xxxxxxxxxx>
