[PATCH 2/5] xfs: recovery of XLOG_UNMOUNT_TRANS leaks memory

[Dave Chinner]

On Fri, Sep 26, 2014 at 05:01:04AM -0700, Christoph Hellwig wrote:
> On Fri, Sep 26, 2014 at 12:19:09PM +1000, Dave Chinner wrote:
> > From: Dave Chinner <dchinner at redhat.com>
> > 
> > The XLOG_UNMOUNT_TRANS case skips the transaction, despite the fact
> > an unmount record is always in a standalone transaction. Hence
> > whenever we come across one of these we need to free the transaction
> > structure associated with it as there is no commit record that
> > follows it.
> > 
> > Signed-off-by: Dave Chinner <dchinner at redhat.com>
> 
> Looks good,
> 
> Reviewed-by: Christoph Hellwig <hch at lst.de>
> 
> > @@ -3600,8 +3605,10 @@ xlog_recover_ophdr_to_trans(
> >  	 * on this opheader is allocate a new recovery container to hold
> >  	 * the recovery ops that will follow.
> >  	 */
> > -	if (ohead->oh_flags & XLOG_START_TRANS)
> > +	if (ohead->oh_flags & XLOG_START_TRANS) {
> > +		ASSERT(be32_to_cpu(ohead->oh_len) == 0);
> >  		xlog_recover_new_tid(rhp, tid, be64_to_cpu(rhead->h_lsn));
> > +	}
> >  	return NULL;
> 
> .. but I suspect this hunk fits better into the previous patch.

Folded it into the first patch.

> Also shouldn't we handle any sort of on disk corruption more gracefully?

Yes, we should. However, the recovery code is so full of this sort
of ASSERT() checking that graceful handling of errors is fairly
significant piece of work. It's already on my "cleanup work for a
rainy day" list. ;)

Cheers,

Dave.
-- 
Dave Chinner
david at fromorbit.com