On Thu 17-07-14 09:17:25, Dave Chinner wrote: > On Wed, Jul 16, 2014 at 01:11:05AM -0700, Christoph Hellwig wrote: > > On Wed, Jul 16, 2014 at 10:38:51AM +1000, Dave Chinner wrote: > > > 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, > > > > I'd prefer to mention v4 filesystems as well: > > > > /* > > * v4 filesystems only contain the fields before the uuid, and > > * even v5 filesystems don't usually log any field beneath it. > > */ > > It actually has nothing to do with v4 or v5 filesystems - it's to do > with the fact that we do partial buffer logging but logprint is > assuming the structure is always logging as a whole. There's > mistakes like this all through logprint - we whack them like this > with a big stick (i.e. refuse to parse the structure) when they are > found. > > Did you notice the way that log_print_all.c handled these issues? > For the AGI, it simply looks at the length of the region and sizes > the output accordingly. And for the AGF, it just ignores the size of > the region and assumes that it captured everything that is to be > printed. > > IOWs, we've played whack-a-mole on this again while ignoring the > fundamental issues ithat still remain: > > - that logprint has a lot of assumptions that simply aren't > true; and > - that logprint simply does not handle split region > continuations like the kernel recovery code does. > > Both of these things lead to having to handle these strange "out of > space" cases in multiple places, and simply not handling them in > places that actually need to. > > These are just more reasons why logprint - as it says itself in a > couple of comments - needs a complete rewrite. Yeah, I've noticed that logprint definitely doesn't handle all possible cases and the code is ... well, organic :). OTOH it worked for me (except for this bug) when I needed so for a debug tool like that it seemed good enough. I don't feel like rewriting it from scratch (at least in near future) but if someone can find time for that, it would be surely welcome. Honza -- Jan Kara <jack@xxxxxxx> SUSE Labs, CR _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs
