* [PATCH] xfs: return -EINVAL instead of -EUCLEAN when mounting non-xfs [not found] <20121230015615.6cc9e03c@sf> @ 2012-12-29 23:16 ` Sergei Trofimovich 2012-12-29 23:32 ` Eric Sandeen 2013-01-03 18:19 ` Christoph Hellwig 0 siblings, 2 replies; 5+ messages in thread From: Sergei Trofimovich @ 2012-12-29 23:16 UTC (permalink / raw) To: xfs Cc: Sergei Trofimovich, Ben Myers, Alex Elder, linux-kernel, Dave Chinner, Phil White It fixes boot panic when trying to boot from btrfs filesystem. kernel tries to mount as xfs and gets fatal -EUCLEAN: [ 0.170000] VFS: Cannot open root device "ubda" or unknown-block(98,0): error -117 [ 0.170000] Please append a correct "root=" boot option; here are the available partitions: [ 0.170000] 6200 1048576 ubda driver: uml-blkdev [ 0.170000] Kernel panic - not syncing: VFS: Unable to mount root fs on unknown-block(98,0) init/do_mounts.c expects only -EINVAL as 'retry another' option. Fixes regression introduced by commit 98021821a502db347bd9c7671beeee6e8ce07ea6 Signed-off-by: Sergei Trofimovich <slyfox@gentoo.org> CC: Ben Myers <bpm@sgi.com> CC: Alex Elder <elder@kernel.org> CC: xfs@oss.sgi.com CC: linux-kernel@vger.kernel.org CC: Dave Chinner <dchinner@redhat.com> CC: Phil White <pwhite@sgi.com> --- fs/xfs/xfs_mount.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c index da50846..379cac1 100644 --- a/fs/xfs/xfs_mount.c +++ b/fs/xfs/xfs_mount.c @@ -641,41 +641,41 @@ xfs_sb_read_verify( /* * We may be probed for a filesystem match, so we may not want to emit * messages when the superblock buffer is not actually an XFS superblock. * If we find an XFS superblock, the run a normal, noisy mount because we are * really going to mount it and want to know about errors. */ static void xfs_sb_quiet_read_verify( struct xfs_buf *bp) { struct xfs_sb sb; xfs_sb_from_disk(&sb, XFS_BUF_TO_SBP(bp)); if (sb.sb_magicnum == XFS_SB_MAGIC) { /* XFS filesystem, verify noisily! */ xfs_sb_read_verify(bp); return; } /* quietly fail */ - xfs_buf_ioerror(bp, EFSCORRUPTED); + xfs_buf_ioerror(bp, EINVAL); } static void xfs_sb_write_verify( struct xfs_buf *bp) { xfs_sb_verify(bp); } const struct xfs_buf_ops xfs_sb_buf_ops = { .verify_read = xfs_sb_read_verify, .verify_write = xfs_sb_write_verify, }; static const struct xfs_buf_ops xfs_sb_quiet_buf_ops = { .verify_read = xfs_sb_quiet_read_verify, .verify_write = xfs_sb_write_verify, }; /* -- 1.8.0.2 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] xfs: return -EINVAL instead of -EUCLEAN when mounting non-xfs 2012-12-29 23:16 ` [PATCH] xfs: return -EINVAL instead of -EUCLEAN when mounting non-xfs Sergei Trofimovich @ 2012-12-29 23:32 ` Eric Sandeen 2012-12-30 2:29 ` Theodore Ts'o 2013-01-03 18:19 ` Christoph Hellwig 1 sibling, 1 reply; 5+ messages in thread From: Eric Sandeen @ 2012-12-29 23:32 UTC (permalink / raw) To: Sergei Trofimovich Cc: xfs, Alex Elder, Dave Chinner, linux-kernel, Ben Myers, Phil White On 12/29/12 5:16 PM, Sergei Trofimovich wrote: > It fixes boot panic when trying to boot from btrfs filesystem. > kernel tries to mount as xfs and gets fatal -EUCLEAN: > > [ 0.170000] VFS: Cannot open root device "ubda" or unknown-block(98,0): error -117 > [ 0.170000] Please append a correct "root=" boot option; here are the available partitions: > [ 0.170000] 6200 1048576 ubda driver: uml-blkdev > [ 0.170000] Kernel panic - not syncing: VFS: Unable to mount root fs on unknown-block(98,0) > > init/do_mounts.c expects only -EINVAL as 'retry another' option. > Fixes regression introduced by commit 98021821a502db347bd9c7671beeee6e8ce07ea6 yeah, that should work; great minds think alike ;) Our patches crossed in the ether I guess. XFS uses EWRONGFS as an alias for EINVAL internally in these cases, so maybe we should stick with that for consistency, *shrug* -Eric > Signed-off-by: Sergei Trofimovich <slyfox@gentoo.org> > CC: Ben Myers <bpm@sgi.com> > CC: Alex Elder <elder@kernel.org> > CC: xfs@oss.sgi.com > CC: linux-kernel@vger.kernel.org > CC: Dave Chinner <dchinner@redhat.com> > CC: Phil White <pwhite@sgi.com> > --- > fs/xfs/xfs_mount.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c > index da50846..379cac1 100644 > --- a/fs/xfs/xfs_mount.c > +++ b/fs/xfs/xfs_mount.c > @@ -641,41 +641,41 @@ xfs_sb_read_verify( > /* > * We may be probed for a filesystem match, so we may not want to emit > * messages when the superblock buffer is not actually an XFS superblock. > * If we find an XFS superblock, the run a normal, noisy mount because we are > * really going to mount it and want to know about errors. > */ > static void > xfs_sb_quiet_read_verify( > struct xfs_buf *bp) > { > struct xfs_sb sb; > > xfs_sb_from_disk(&sb, XFS_BUF_TO_SBP(bp)); > > if (sb.sb_magicnum == XFS_SB_MAGIC) { > /* XFS filesystem, verify noisily! */ > xfs_sb_read_verify(bp); > return; > } > /* quietly fail */ > - xfs_buf_ioerror(bp, EFSCORRUPTED); > + xfs_buf_ioerror(bp, EINVAL); > } > > static void > xfs_sb_write_verify( > struct xfs_buf *bp) > { > xfs_sb_verify(bp); > } > > const struct xfs_buf_ops xfs_sb_buf_ops = { > .verify_read = xfs_sb_read_verify, > .verify_write = xfs_sb_write_verify, > }; > > static const struct xfs_buf_ops xfs_sb_quiet_buf_ops = { > .verify_read = xfs_sb_quiet_read_verify, > .verify_write = xfs_sb_write_verify, > }; > > /* > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] xfs: return -EINVAL instead of -EUCLEAN when mounting non-xfs 2012-12-29 23:32 ` Eric Sandeen @ 2012-12-30 2:29 ` Theodore Ts'o 0 siblings, 0 replies; 5+ messages in thread From: Theodore Ts'o @ 2012-12-30 2:29 UTC (permalink / raw) To: Eric Sandeen Cc: Sergei Trofimovich, xfs, Alex Elder, Dave Chinner, linux-kernel, Ben Myers, Phil White On Sat, Dec 29, 2012 at 05:32:12PM -0600, Eric Sandeen wrote: > yeah, that should work; great minds think alike ;) Our patches > crossed in the ether I guess. > > XFS uses EWRONGFS as an alias for EINVAL internally in these cases, > so maybe we should stick with that for consistency, *shrug* I keep thinking we should try expanding the number of errno's so that the file system can give more fs-specific error codes, such that eventually, user programs could print out error messages that would make a lot more sense to users. What we'd have to do is to define the new errno's, and then wait for the new error_message() strings to propagate out to a new glibc release, and only then have the kernel start using the new errno values. It would be a pain in the tuckus to do, but in the long run the end result would be a lot better for end users and system administrators. - Ted ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] xfs: return -EINVAL instead of -EUCLEAN when mounting non-xfs 2012-12-29 23:16 ` [PATCH] xfs: return -EINVAL instead of -EUCLEAN when mounting non-xfs Sergei Trofimovich 2012-12-29 23:32 ` Eric Sandeen @ 2013-01-03 18:19 ` Christoph Hellwig 2013-01-08 19:48 ` Ben Myers 1 sibling, 1 reply; 5+ messages in thread From: Christoph Hellwig @ 2013-01-03 18:19 UTC (permalink / raw) To: Sergei Trofimovich Cc: xfs, Alex Elder, Dave Chinner, linux-kernel, Ben Myers, Phil White On Sun, Dec 30, 2012 at 02:16:50AM +0300, Sergei Trofimovich wrote: > It fixes boot panic when trying to boot from btrfs filesystem. > kernel tries to mount as xfs and gets fatal -EUCLEAN: > > [ 0.170000] VFS: Cannot open root device "ubda" or unknown-block(98,0): error -117 > [ 0.170000] Please append a correct "root=" boot option; here are the available partitions: > [ 0.170000] 6200 1048576 ubda driver: uml-blkdev > [ 0.170000] Kernel panic - not syncing: VFS: Unable to mount root fs on unknown-block(98,0) > > init/do_mounts.c expects only -EINVAL as 'retry another' option. > Fixes regression introduced by commit 98021821a502db347bd9c7671beeee6e8ce07ea6 Looks reasonable, but think xfs_readsb should simply be changed to turn all EFSCORRUPTED returns into EINVAL if loud is not set. The place that changes the errno value would also be a perfect place to comment why we are doing this in the code so that this knowledge doesn't get lost. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] xfs: return -EINVAL instead of -EUCLEAN when mounting non-xfs 2013-01-03 18:19 ` Christoph Hellwig @ 2013-01-08 19:48 ` Ben Myers 0 siblings, 0 replies; 5+ messages in thread From: Ben Myers @ 2013-01-08 19:48 UTC (permalink / raw) To: Christoph Hellwig Cc: Sergei Trofimovich, Alex Elder, Dave Chinner, linux-kernel, xfs, Phil White Hey, On Thu, Jan 03, 2013 at 01:19:29PM -0500, Christoph Hellwig wrote: > On Sun, Dec 30, 2012 at 02:16:50AM +0300, Sergei Trofimovich wrote: > > It fixes boot panic when trying to boot from btrfs filesystem. > > kernel tries to mount as xfs and gets fatal -EUCLEAN: > > > > [ 0.170000] VFS: Cannot open root device "ubda" or unknown-block(98,0): error -117 > > [ 0.170000] Please append a correct "root=" boot option; here are the available partitions: > > [ 0.170000] 6200 1048576 ubda driver: uml-blkdev > > [ 0.170000] Kernel panic - not syncing: VFS: Unable to mount root fs on unknown-block(98,0) > > > > init/do_mounts.c expects only -EINVAL as 'retry another' option. > > Fixes regression introduced by commit 98021821a502db347bd9c7671beeee6e8ce07ea6 > > Looks reasonable, but think xfs_readsb should simply be changed to > turn all EFSCORRUPTED returns into EINVAL if loud is not set. I think there are some EFSCORRUPTED returns that should not result in 'retry another' behavior. E.g. mounting an xfs filesystem where super block magic is good but you have a corrupted log probably shouldn't result in a retry with ext3. It is useful to be able to distinguish between that special case of sb->sb_magic != XFS_SB_MAGIC and a truly corrupted filesystem. Here EWRONGFS (EINVAL) in the quiet case is better than EFSCORRUPTED, but unfortunately we still can't tell EWRONGFS from other EINVAL returns in b_error. Adding a new errno as Ted suggested might make some sense. Regards, Ben ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-01-08 19:48 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20121230015615.6cc9e03c@sf>
2012-12-29 23:16 ` [PATCH] xfs: return -EINVAL instead of -EUCLEAN when mounting non-xfs Sergei Trofimovich
2012-12-29 23:32 ` Eric Sandeen
2012-12-30 2:29 ` Theodore Ts'o
2013-01-03 18:19 ` Christoph Hellwig
2013-01-08 19:48 ` Ben Myers
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox