xfs: don't skip rtmount when there's a realtime device
PATCH 1/1 xfs: don't skip rtmount when there's a realtime device
From: Darrick J. Wong <[email protected]>
Don't ever skip the realtime bitmap / summary inode initialization if there's a realtime device attached, because we'd rather fail the mount if iget declines to retrieve a NULL inode pointer. Right now, if someone sets rbmino to NULLFSINO on a rt-capable filesystem, mounts it, and writes a file to the rt device, we'll blow up.
--- fs/xfs/xfs_rtalloc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
fs/xfs/xfs_rtalloc.c +2 -1
@@ -1206,7 +1206,8 @@ xfs_rtmount_inodes(xfs_sb_t *sbp; sbp = &mp->m_sb; -if (sbp->sb_rbmino ==NULLFSINO ) +if (!xfs_sb_version_hasrealtime (&mp->m_sb) && + sbp->sb_rbmino ==NULLFSINO )return 0 ; error =xfs_iget (mp,NULL , sbp->sb_rbmino,0 ,0 , &mp->m_rbmip);if (error)
Quoted 10 lines · Darrick J. Wong
From: Darrick J. Wong <[email protected]>
Don't ever skip the realtime bitmap / summary inode initialization if there's a realtime device attached, because we'd rather fail the mount if iget declines to retrieve a NULL inode pointer. Right now, if someone sets rbmino to NULLFSINO on a rt-capable filesystem, mounts it, and writes a file to the rt device, we'll blow up.
Signed-off-by Darrick J. Wong <[email protected]>---
Makes sense.
Quoted 17 lines
fs/xfs/xfs_rtalloc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)fs/xfs/xfs_rtalloc.c +2
-1@@ -1206,7 +1206,8 @@ xfs_rtmount_inodes(xfs_sb_t *sbp; sbp = &mp->m_sb; -if (sbp->sb_rbmino ==NULLFSINO ) +if (!xfs_sb_version_hasrealtime (&mp->m_sb) && + sbp->sb_rbmino ==NULLFSINO )return 0 ; error =xfs_iget (mp,NULL , sbp->sb_rbmino,0 ,0 , &mp->m_rbmip);if (error)
Quoted 19 lines · Darrick J. Wong
From: Darrick J. Wong <[email protected]>
Don't ever skip the realtime bitmap / summary inode initialization if there's a realtime device attached, because we'd rather fail the mount if iget declines to retrieve a NULL inode pointer. Right now, if someone sets rbmino to NULLFSINO on a rt-capable filesystem, mounts it, and writes a file to the rt device, we'll blow up.
Signed-off-by Darrick J. Wong <[email protected]>--- fs/xfs/xfs_rtalloc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)fs/xfs/xfs_rtalloc.c +0
-0
fs/xfs/xfs_rtalloc.c xfs_rtmount_inodes( · line 1213
Quoted 9 lines
fs/xfs/xfs_rtalloc.c +2
-1@@ -1206,7 +1206,8 @@ xfs_rtmount_inodes(xfs_sb_t *sbp; sbp = &mp->m_sb; -if (sbp->sb_rbmino ==NULLFSINO ) +if (!xfs_sb_version_hasrealtime (&mp->m_sb) && + sbp->sb_rbmino ==NULLFSINO )return 0 ; error =xfs_iget (mp,NULL , sbp->sb_rbmino,0 ,0 , &mp->m_rbmip);if (error)
Seems fine as far as it goes, but now we have this weird situation where for a filesystem without the realtime feature, we'll just skip this part and mount if sb_rbmino is NULL, but if sb_rsumino is NULL, we'll fail the iget and fail the mount.
So while there's noting really wrong with the fix, it seems like it could all be made a bit more consistent while you're here. (classic move by me!) ;)
-Eric
Quoted 37 lines · Eric Sandeen
Quoted 19 lines · Darrick J. Wong
From: Darrick J. Wong <[email protected]>
Don't ever skip the realtime bitmap / summary inode initialization if there's a realtime device attached, because we'd rather fail the mount if iget declines to retrieve a NULL inode pointer. Right now, if someone sets rbmino to NULLFSINO on a rt-capable filesystem, mounts it, and writes a file to the rt device, we'll blow up.
Signed-off-by Darrick J. Wong <[email protected]>--- fs/xfs/xfs_rtalloc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)fs/xfs/xfs_rtalloc.c +0
-0fs/xfs/xfs_rtalloc.c xfs_rtmount_inodes( · line 1213
Quoted 9 lines
fs/xfs/xfs_rtalloc.c +2
-1@@ -1206,7 +1206,8 @@ xfs_rtmount_inodes(xfs_sb_t *sbp; sbp = &mp->m_sb; -if (sbp->sb_rbmino ==NULLFSINO ) +if (!xfs_sb_version_hasrealtime (&mp->m_sb) && + sbp->sb_rbmino ==NULLFSINO )return 0 ; error =xfs_iget (mp,NULL , sbp->sb_rbmino,0 ,0 , &mp->m_rbmip);if (error)Seems fine as far as it goes, but now we have this weird situation where for a filesystem without the realtime feature, we'll just skip this part and mount if
sb_rbminois NULL, but ifsb_rsuminois NULL, we'll fail the iget and fail the mount.
Quoted 2 lines
So while there's noting really wrong with the fix, it seems like it could all be made a bit more consistent while you're here. (classic move by me!) ;)
Hmm. Well if there /aren't supposed/ to be any XFSes out there with garbage/null realtime bitmap / summary inodes, then I'm happy to get rid of the "if (sbp->sb_rbmino == NULLFSINO) return 0;" logic. The current logic is sorta weird, but I don't know if that was a deliberate design decision or just a logic bug that needs to go away.
Though now that I see that we can indeed add a realtime section to a filesystem that didn't previously have one, I guess I should go look at the rtrmapbt growfs code a little more closely.
--D
Quoted 1 line
-Eric