xfs
[Top] [All Lists]

Re: [PATCH] xfsdump:fill in bs_forkoff

To: Eric Sandeen <sandeen@xxxxxxxxxx>
Subject: Re: [PATCH] xfsdump:fill in bs_forkoff
From: Ben Myers <bpm@xxxxxxx>
Date: Wed, 31 Oct 2012 14:46:54 -0500
Cc: xfs-oss <xfs@xxxxxxxxxxx>
In-reply-to: <50903075.6060600@xxxxxxxxxx>
References: <5080D0BD.3000304@xxxxxxxxxx> <20121030194718.GD405@xxxxxxx> <50903075.6060600@xxxxxxxxxx>
User-agent: Mutt/1.5.20 (2009-06-14)
Hi Eric,

On Tue, Oct 30, 2012 at 02:54:29PM -0500, Eric Sandeen wrote:
> On 10/30/12 2:47 PM, Ben Myers wrote:
> > Hey Eric,
> > 
> > On Thu, Oct 18, 2012 at 11:02:05PM -0500, Eric Sandeen wrote:
> >> Upstream, the structure containing bs_forkoff is actually zeroed
> >> prior to these functions, but when pulling the patch back to an
> >> older xfsdump, we got checksum errors due to an uninitialized
> >> bs_forkoff not matching in dump vs. restore.
> >>
> >> So even though forkoff won't be explicitly restored from
> >> a dump, do explicitly set it in these routines to keep checksums
> >> happy.
> >>
> >> Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxx>
> > 
> > Would you say that this is appropriate for the upcoming release?
> 
> Hm.
> 
> The zeroing isn't in a really obvious spot, IIRC, so explicitly
> filling in all members leaves nothing to chance.
> 
> OTOH it's a member that (will/should) never get restored,
> so filling it in is a little confusing.  What do you think?
>
> I think it should be harmless to functionality either way.

It seemed like it could be an important bugfix but I wasn't really sure so I
asked.  Since it sounds like it's not a big deal, lets just hold off till after
the release...

Thanks,
        Ben

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