Re: [PATCH] logprint: Fix printing of AGF buffers

From: Dave Chinner <david@fromorbit.com>
To: Christoph Hellwig <hch@infradead.org>
Cc: Jan Kara <jack@suse.cz>, xfs@oss.sgi.com
Subject: Re: [PATCH] logprint: Fix printing of AGF buffers
Date: Wed, 16 Jul 2014 10:38:51 +1000	[thread overview]
Message-ID: <20140716003851.GO22339@dastard> (raw)
In-Reply-To: <20140715153922.GB5369@infradead.org>

On Tue, Jul 15, 2014 at 08:39:22AM -0700, Christoph Hellwig wrote:
> On Tue, Jul 15, 2014 at 04:09:38PM +0200, Jan Kara wrote:
> >   I had a look before I submitted this patch and I didn't find anything.
> > Now that I'm looking again, AGI buffers probably need a similar treatment.
> > Superblock buffers are already checked against fixed number so those don't
> > have a problem. Dquot buffers should be fine as well because those don't
> > have a checksum and other unlogged stuff. And I didn't find any other
> > structures in the log that would have the problem (please point me if I
> > missed something).
> > 
> > Regarding how to fix this cleanly - offsetof() seems like a reasonably
> > clean way to me. If you prefer to define number of bytes each buffer type
> > has to have in the log, I can do that as well. Or I could define
> > alternative structures only containing fields we need in the log so that we
> > can print info but this all seems like an additional complexity with
> > disputable gain...
> 
> I've taken a bit of a closer log (OMG, what a mess logprint is..), and
> it seems at least in this are the AGF and AGI are affected of the struct
> growth in v4.

Yes, logprint is a steaming pile. At some point I'll properly
abstract the kernel side log recovery code and share that with
userspace and then convert logprint to use that....

> It seems like in this specific case using your offsetoff trick is
> fine, it just needs a good comment explaining it.

I added this:

	/*
	 * The addition of spare space and the non-logged CRC format
	 * fields to the AGF mean that the size that is logged is almost
	 * always going to be smaller than the structure itself. Hence
	 * we need to make sure that the buffer contains all the data we
	 * want to print rather than just check against the structure
	 * size.
	 */

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs