* [PATCH] xfsprogs: remove incorrect l_sectBBsize assignment in xfs_repair
@ 2013-10-07 17:35 Eric Sandeen
2013-10-10 13:56 ` Carlos Maiolino
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Eric Sandeen @ 2013-10-07 17:35 UTC (permalink / raw)
To: xfs-oss; +Cc: Markus Trippelsdorf
Commit e0607266 xfsprogs: add crc format support to repair
added a 2nd assignment to l_sectBBsize:
log.l_sectBBsize = 1 << mp->m_sb.sb_logsectlog;
which is incorrect; sb_logsectlog is log2 of the sector size,
in bytes; l_sectBBsize is the size of the log sector in
512-byte units.
So for a 4k sector size log, we were assigning 4096 rather
than 8. This broke xlog_find_tail, and caused xfs_repair
to think that a log was dirty even when it was clean:
"ERROR: The filesystem has valuable metadata changes in a log"
(xfs_logprint didn't have this error, so xfs_logprint -t
agreed that the filesystem really was clean).
Just remove the incorrect assignment; it was already properly
assigned about 12 lines prior:
log.l_sectBBsize = BTOBB(x.lbsize);
and things work again.
(This worked accidentally for 512-sector devices, because
we special-case those and set sb_logsectlog to "0" rather
than 9, so l_sectBBsize came out to "1" (as in 1 sector),
as it should have).
Reporteed-by: Markus Trippelsdorf <markus@trippelsdorf.de>
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
diff --git a/repair/phase2.c b/repair/phase2.c
index a62854e..2817fed 100644
--- a/repair/phase2.c
+++ b/repair/phase2.c
@@ -64,7 +64,6 @@ zero_log(xfs_mount_t *mp)
ASSERT(mp->m_sb.sb_logsectlog >= BBSHIFT);
}
log.l_sectbb_mask = (1 << log.l_sectbb_log) - 1;
- log.l_sectBBsize = 1 << mp->m_sb.sb_logsectlog;
if ((error = xlog_find_tail(&log, &head_blk, &tail_blk))) {
do_warn(_("zero_log: cannot find log head/tail "
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] xfsprogs: remove incorrect l_sectBBsize assignment in xfs_repair
2013-10-07 17:35 [PATCH] xfsprogs: remove incorrect l_sectBBsize assignment in xfs_repair Eric Sandeen
@ 2013-10-10 13:56 ` Carlos Maiolino
2013-10-14 21:07 ` Dave Chinner
2013-10-18 17:26 ` Rich Johnston
2 siblings, 0 replies; 4+ messages in thread
From: Carlos Maiolino @ 2013-10-10 13:56 UTC (permalink / raw)
To: xfs
This looks good to me,
Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
On Mon, Oct 07, 2013 at 12:35:16PM -0500, Eric Sandeen wrote:
> Commit e0607266 xfsprogs: add crc format support to repair
>
> added a 2nd assignment to l_sectBBsize:
>
> log.l_sectBBsize = 1 << mp->m_sb.sb_logsectlog;
>
> which is incorrect; sb_logsectlog is log2 of the sector size,
> in bytes; l_sectBBsize is the size of the log sector in
> 512-byte units.
>
> So for a 4k sector size log, we were assigning 4096 rather
> than 8. This broke xlog_find_tail, and caused xfs_repair
> to think that a log was dirty even when it was clean:
>
> "ERROR: The filesystem has valuable metadata changes in a log"
>
> (xfs_logprint didn't have this error, so xfs_logprint -t
> agreed that the filesystem really was clean).
>
> Just remove the incorrect assignment; it was already properly
> assigned about 12 lines prior:
>
> log.l_sectBBsize = BTOBB(x.lbsize);
>
> and things work again.
>
> (This worked accidentally for 512-sector devices, because
> we special-case those and set sb_logsectlog to "0" rather
> than 9, so l_sectBBsize came out to "1" (as in 1 sector),
> as it should have).
>
> Reporteed-by: Markus Trippelsdorf <markus@trippelsdorf.de>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
>
> diff --git a/repair/phase2.c b/repair/phase2.c
> index a62854e..2817fed 100644
> --- a/repair/phase2.c
> +++ b/repair/phase2.c
> @@ -64,7 +64,6 @@ zero_log(xfs_mount_t *mp)
> ASSERT(mp->m_sb.sb_logsectlog >= BBSHIFT);
> }
> log.l_sectbb_mask = (1 << log.l_sectbb_log) - 1;
> - log.l_sectBBsize = 1 << mp->m_sb.sb_logsectlog;
>
> if ((error = xlog_find_tail(&log, &head_blk, &tail_blk))) {
> do_warn(_("zero_log: cannot find log head/tail "
>
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs
--
Carlos
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] xfsprogs: remove incorrect l_sectBBsize assignment in xfs_repair
2013-10-07 17:35 [PATCH] xfsprogs: remove incorrect l_sectBBsize assignment in xfs_repair Eric Sandeen
2013-10-10 13:56 ` Carlos Maiolino
@ 2013-10-14 21:07 ` Dave Chinner
2013-10-18 17:26 ` Rich Johnston
2 siblings, 0 replies; 4+ messages in thread
From: Dave Chinner @ 2013-10-14 21:07 UTC (permalink / raw)
To: Eric Sandeen; +Cc: Markus Trippelsdorf, xfs-oss
On Mon, Oct 07, 2013 at 12:35:16PM -0500, Eric Sandeen wrote:
> Commit e0607266 xfsprogs: add crc format support to repair
>
> added a 2nd assignment to l_sectBBsize:
>
> log.l_sectBBsize = 1 << mp->m_sb.sb_logsectlog;
>
> which is incorrect; sb_logsectlog is log2 of the sector size,
> in bytes; l_sectBBsize is the size of the log sector in
> 512-byte units.
Yes, that is wrong. It looks like a hunk of code I didn't cleanup
properly when I did some patch reordering and rebasing. IIRC, the
above patch was written before I realised that all of xfsprogs
failed to handle log sector sizes != 512 bytes properly, and that
the log CRC code required that to work correctly. Hence a generic
fix to the problem was added in commit:
999f0b9 xfsprogs: updata libxlog to current kernel code
Which fixed repair, db and logprint. That is where this code:
> Just remove the incorrect assignment; it was already properly
> assigned about 12 lines prior:
>
> log.l_sectBBsize = BTOBB(x.lbsize);
>
> and things work again.
came from.
Basically, I didn't clean all the random fix fragments out of the
repair patch, and so I broke log secotr size != 512 byte cases for
repair again....
> Reporteed-by: Markus Trippelsdorf <markus@trippelsdorf.de>
Reported-by the reportee. ;)
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
>
> diff --git a/repair/phase2.c b/repair/phase2.c
> index a62854e..2817fed 100644
> --- a/repair/phase2.c
> +++ b/repair/phase2.c
> @@ -64,7 +64,6 @@ zero_log(xfs_mount_t *mp)
> ASSERT(mp->m_sb.sb_logsectlog >= BBSHIFT);
> }
> log.l_sectbb_mask = (1 << log.l_sectbb_log) - 1;
> - log.l_sectBBsize = 1 << mp->m_sb.sb_logsectlog;
>
> if ((error = xlog_find_tail(&log, &head_blk, &tail_blk))) {
> do_warn(_("zero_log: cannot find log head/tail "
Looks good,
Reviewed-by: Dave Chinner <dchinner@redhat.com>
--
Dave Chinner
david@fromorbit.com
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] xfsprogs: remove incorrect l_sectBBsize assignment in xfs_repair
2013-10-07 17:35 [PATCH] xfsprogs: remove incorrect l_sectBBsize assignment in xfs_repair Eric Sandeen
2013-10-10 13:56 ` Carlos Maiolino
2013-10-14 21:07 ` Dave Chinner
@ 2013-10-18 17:26 ` Rich Johnston
2 siblings, 0 replies; 4+ messages in thread
From: Rich Johnston @ 2013-10-18 17:26 UTC (permalink / raw)
To: Eric Sandeen, xfs-oss; +Cc: Markus Trippelsdorf
This has been committed.
Thanks
--Rich
commit 5425725c56bccb9a91847cb74cfdadc8b44fecf7
Author: Eric Sandeen <sandeen@redhat.com>
Date: Mon Oct 7 17:35:16 2013 +0000
xfsprogs: remove incorrect l_sectBBsize assignment in xfs_repair
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-10-18 17:26 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-10-07 17:35 [PATCH] xfsprogs: remove incorrect l_sectBBsize assignment in xfs_repair Eric Sandeen
2013-10-10 13:56 ` Carlos Maiolino
2013-10-14 21:07 ` Dave Chinner
2013-10-18 17:26 ` Rich Johnston
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.