* [PATCH 0/2] xfs_logprint: Get xfstests/xfs/295 passing w/ CRCs @ 2014-02-27 1:00 Eric Sandeen 2014-02-27 1:13 ` [PATCH 1/2] xfs_logprint: Don't error out after split items lose context Eric Sandeen 2014-02-27 1:15 ` [PATCH 2/2] xfs_logprint: don't advance op counter in xlog_print_trans_icreate Eric Sandeen 0 siblings, 2 replies; 5+ messages in thread From: Eric Sandeen @ 2014-02-27 1:00 UTC (permalink / raw) To: xfs-oss None of this really has to do w/ CRCs specificall, but it perturbed the log in a way that exposed a couple o' bugs. Patches follow. _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] xfs_logprint: Don't error out after split items lose context 2014-02-27 1:00 [PATCH 0/2] xfs_logprint: Get xfstests/xfs/295 passing w/ CRCs Eric Sandeen @ 2014-02-27 1:13 ` Eric Sandeen 2014-02-27 1:27 ` Dave Chinner 2014-02-27 1:15 ` [PATCH 2/2] xfs_logprint: don't advance op counter in xlog_print_trans_icreate Eric Sandeen 1 sibling, 1 reply; 5+ messages in thread From: Eric Sandeen @ 2014-02-27 1:13 UTC (permalink / raw) To: Eric Sandeen, xfs-oss xfs_logprint recognizes a "left over region from split log item" but then expects the *next* op to be a valid start to a new item. The problem is, we can split i.e. an xfs_inode_log_format item, skip over it, and then land on the xfs_icdinode_t data which follows it - this doesn't have a valid log item magic (XFS_LI_*) and we error out. This results in something like: xfs_logprint: unknown log operation type (494e) Fix this by recognizing that we've skipped over an item and lost the context we're in, so just continue skipping over op headers until we find the next valid start to a log item. Signed-off-by: Eric Sandeen <sandeen@redhat.com> --- diff --git a/logprint/log_misc.c b/logprint/log_misc.c index cf9d59d..7070fa3 100644 --- a/logprint/log_misc.c +++ b/logprint/log_misc.c @@ -874,7 +874,7 @@ xlog_print_record( int bad_hdr_warn) { xfs_caddr_t buf, ptr; - int read_len, skip; + int read_len, skip, lost_context = 0; int ret, n, i, j, k; if (print_no_print) @@ -995,7 +995,10 @@ xlog_print_record( if (xlog_print_find_tid(be32_to_cpu(op_head->oh_tid), op_head->oh_flags & XLOG_WAS_CONT_TRANS)) { printf(_("Left over region from split log item\n")); + /* Skip this leftover bit */ ptr += be32_to_cpu(op_head->oh_len); + /* We've lost context; don't complain if next one looks bad too */ + lost_context = 1; continue; } @@ -1050,7 +1053,7 @@ xlog_print_record( break; } default: { - if (bad_hdr_warn) { + if (bad_hdr_warn && !lost_context) { fprintf(stderr, _("%s: unknown log operation type (%x)\n"), progname, *(unsigned short *)ptr); @@ -1064,6 +1067,7 @@ xlog_print_record( } skip = 0; ptr += be32_to_cpu(op_head->oh_len); + lost_context = 0; } } /* switch */ } /* else */ _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] xfs_logprint: Don't error out after split items lose context 2014-02-27 1:13 ` [PATCH 1/2] xfs_logprint: Don't error out after split items lose context Eric Sandeen @ 2014-02-27 1:27 ` Dave Chinner 0 siblings, 0 replies; 5+ messages in thread From: Dave Chinner @ 2014-02-27 1:27 UTC (permalink / raw) To: Eric Sandeen; +Cc: Eric Sandeen, xfs-oss On Wed, Feb 26, 2014 at 07:13:25PM -0600, Eric Sandeen wrote: > xfs_logprint recognizes a "left over region from split log item" > but then expects the *next* op to be a valid start to a new > item. The problem is, we can split i.e. an xfs_inode_log_format > item, skip over it, and then land on the xfs_icdinode_t > data which follows it - this doesn't have a valid log item > magic (XFS_LI_*) and we error out. This results in something > like: > > xfs_logprint: unknown log operation type (494e) > > Fix this by recognizing that we've skipped over an item and > lost the context we're in, so just continue skipping over > op headers until we find the next valid start to a log item. > > Signed-off-by: Eric Sandeen <sandeen@redhat.com> Looks good - fixes the problem and does a good job of explaining it. :) 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] 5+ messages in thread
* [PATCH 2/2] xfs_logprint: don't advance op counter in xlog_print_trans_icreate 2014-02-27 1:00 [PATCH 0/2] xfs_logprint: Get xfstests/xfs/295 passing w/ CRCs Eric Sandeen 2014-02-27 1:13 ` [PATCH 1/2] xfs_logprint: Don't error out after split items lose context Eric Sandeen @ 2014-02-27 1:15 ` Eric Sandeen 2014-02-27 1:28 ` Dave Chinner 1 sibling, 1 reply; 5+ messages in thread From: Eric Sandeen @ 2014-02-27 1:15 UTC (permalink / raw) To: Eric Sandeen, xfs-oss xlog_print_trans_icreate is advancing the op counter "(*i)++" incorrectly; it only contains one region, and the loop which called it will properly advance the op once we're done. Found-by: Dave Chinner <dchinner@redhat.com> Signed-off-by: Eric Sandeen <sandeen@redhat.com> --- diff --git a/logprint/log_misc.c b/logprint/log_misc.c index 7070fa3..52f1e85 100644 --- a/logprint/log_misc.c +++ b/logprint/log_misc.c @@ -810,7 +810,6 @@ xlog_print_trans_icreate( memmove(&icl_buf, *ptr, MIN(sizeof(struct xfs_icreate_log), len)); icl = &icl_buf; - (*i)++; *ptr += len; /* handle complete header only */ _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] xfs_logprint: don't advance op counter in xlog_print_trans_icreate 2014-02-27 1:15 ` [PATCH 2/2] xfs_logprint: don't advance op counter in xlog_print_trans_icreate Eric Sandeen @ 2014-02-27 1:28 ` Dave Chinner 0 siblings, 0 replies; 5+ messages in thread From: Dave Chinner @ 2014-02-27 1:28 UTC (permalink / raw) To: Eric Sandeen; +Cc: Eric Sandeen, xfs-oss On Wed, Feb 26, 2014 at 07:15:52PM -0600, Eric Sandeen wrote: > xlog_print_trans_icreate is advancing the op counter > "(*i)++" incorrectly; it only contains one region, and > the loop which called it will properly advance the op > once we're done. > > Found-by: Dave Chinner <dchinner@redhat.com> > Signed-off-by: Eric Sandeen <sandeen@redhat.com> > --- > > diff --git a/logprint/log_misc.c b/logprint/log_misc.c > index 7070fa3..52f1e85 100644 > --- a/logprint/log_misc.c > +++ b/logprint/log_misc.c > @@ -810,7 +810,6 @@ xlog_print_trans_icreate( > > memmove(&icl_buf, *ptr, MIN(sizeof(struct xfs_icreate_log), len)); > icl = &icl_buf; > - (*i)++; > *ptr += len; > > /* handle complete header only */ *nod*. I'm pretty sure I introduced that bug - thanks for exposing it and fixing it :) 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] 5+ messages in thread
end of thread, other threads:[~2014-02-27 1:29 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-02-27 1:00 [PATCH 0/2] xfs_logprint: Get xfstests/xfs/295 passing w/ CRCs Eric Sandeen 2014-02-27 1:13 ` [PATCH 1/2] xfs_logprint: Don't error out after split items lose context Eric Sandeen 2014-02-27 1:27 ` Dave Chinner 2014-02-27 1:15 ` [PATCH 2/2] xfs_logprint: don't advance op counter in xlog_print_trans_icreate Eric Sandeen 2014-02-27 1:28 ` Dave Chinner
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox