On 8/3/15 12:40 AM, Dave Chinner wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
>
> Now that sb_uuid can be changed by the user, we cannot use this to
> validate the metadata blocks being recovered belong to this
> filesystem. We must check against the sb_meta_uuid as that will
> remain unchanged.
>
> There is a complication in this code - the superblock itself. We can
> not check the sb_meta_uuid unconditionally, as that may not be set
> on disk. Hence we must verify the superblock sb_uuid matches between
> the log record and the in-core superblock.
>
> Found by inspection after the previous two problems were found.
So, I also had this in my older patchset, I think it's needed for
proper log recovery as well. I'm not sure why I didn't hit the
xlog_recover_get_buf_lsn problem, though:
diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
index 4f5784f..4da291c 100644
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -325,7 +325,7 @@ xlog_header_check_recover(
XFS_ERROR_REPORT("xlog_header_check_recover(1)",
XFS_ERRLEVEL_HIGH, mp);
return -EFSCORRUPTED;
- } else if (unlikely(!uuid_equal(&mp->m_sb.sb_uuid, &head->h_fs_uuid))) {
+ } else if (!uuid_equal(&mp->m_sb.sb_uuid, &head->h_fs_uuid)) {
xfs_warn(mp,
"dirty log entry has mismatched uuid - can't recover");
xlog_header_check_dump(mp, head);
@@ -353,7 +353,7 @@ xlog_header_check_mount(
* by IRIX and continue.
*/
xfs_warn(mp, "nil uuid in log - IRIX style log");
- } else if (unlikely(!uuid_equal(&mp->m_sb.sb_uuid, &head->h_fs_uuid))) {
+ } else if (!uuid_equal(&mp->m_sb.sb_uuid, &head->h_fs_uuid)) {
xfs_warn(mp, "log has mismatched uuid - can't recover");
xlog_header_check_dump(mp, head);
XFS_ERROR_REPORT("xlog_header_check_mount",
as for your patch ... looks fine. Not sure why I didn't hit that.
Reviewed-by: Eric Sandeen <sandeen@xxxxxxxxxx> unless you want
to roll the above changes in as well...
-Eric
> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> ---
> fs/xfs/xfs_log_recover.c | 14 ++++++++++++--
> 1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
> index c674b40..6f83d12 100644
> --- a/fs/xfs/xfs_log_recover.c
> +++ b/fs/xfs/xfs_log_recover.c
> @@ -1895,15 +1895,25 @@ xlog_recover_get_buf_lsn(
> */
> goto recover_immediately;
> case XFS_SB_MAGIC:
> + /*
> + * superblock uuids are magic. We may or may not have a
> + * sb_meta_uuid on disk, but it will be set in the in-core
> + * superblock. We set the uuid pointer for verification
> + * according to the superblock feature mask to ensure we check
> + * the relevant UUID in the superblock.
> + */
> lsn = be64_to_cpu(((struct xfs_dsb *)blk)->sb_lsn);
> - uuid = &((struct xfs_dsb *)blk)->sb_uuid;
> + if (xfs_sb_version_hasmetauuid(&mp->m_sb))
> + uuid = &((struct xfs_dsb *)blk)->sb_meta_uuid;
> + else
> + uuid = &((struct xfs_dsb *)blk)->sb_uuid;
> break;
> default:
> break;
> }
>
> if (lsn != (xfs_lsn_t)-1) {
> - if (!uuid_equal(&mp->m_sb.sb_uuid, uuid))
> + if (!uuid_equal(&mp->m_sb.sb_meta_uuid, uuid))
> goto recover_immediately;
> return lsn;
> }
>
|