xfs
[Top] [All Lists]

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

To: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Subject: Re: [PATCH] logprint: Fix printing of AGF buffers
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Wed, 16 Jul 2014 10:38:51 +1000
Cc: Jan Kara <jack@xxxxxxx>, xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20140715153922.GB5369@xxxxxxxxxxxxx>
References: <1405349100-19734-1-git-send-email-jack@xxxxxxx> <20140715101931.GC30363@xxxxxxxxxxxxx> <20140715140938.GA1733@xxxxxxxxxxxxx> <20140715153922.GB5369@xxxxxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
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@xxxxxxxxxxxxx

<Prev in Thread] Current Thread [Next in Thread>