On Sun, May 20, 2018 at 12:29:54PM -0400, Theodore Y. Ts'o wrote: > On Sun, May 20, 2018 at 05:58:43AM -0700, Matthew Wilcox wrote: > > On Sun, May 20, 2018 at 07:45:31AM -0400, Jeff Layton wrote: > > > [PATCH] loop: clear wb_err in bd_inode when detaching backing file > > > > Is this the right thing to do? Putting the test-suite aside for the > > moment, if I have a loop device on a file and I hit a real error on the > > storage backing that file, I don't see why detaching the loop device > > from the file should clear the error. > > > > I'm really tempted to say that we should fix the test-suite to consume > > the error; once it's been read by at least one reader, it won't go back > > to zero, but neither will it show up for new readers. > > You can't detach the backing store if there are any open file > descriptors (or if there is another loop device keeping the loop > device open, or if there is a file system mounted on it, etc.). > > If you can detach the backing store, once you detach the backing > store, the loop device is *gone*. The user of /dev/loop0 will very > likely be a completely different backing store, so returning an error > simply doesn't make any sense. There is a very good chance it will be > a completely different backing store, with potentially a different > user, so returning a carried over error is just going confuse, annoy, > and anger the user..... 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);