xfs
[Top] [All Lists]

Re: [PATCH 03/48] libxfs: add crc format changes to generic btrees

To: Ben Myers <bpm@xxxxxxx>
Subject: Re: [PATCH 03/48] libxfs: add crc format changes to generic btrees
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Fri, 26 Jul 2013 10:39:53 +1000
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20130725171509.GZ3111@xxxxxxx>
References: <1370564771-4929-1-git-send-email-david@xxxxxxxxxxxxx> <1370564771-4929-4-git-send-email-david@xxxxxxxxxxxxx> <20130723182648.GI3111@xxxxxxx> <20130725004821.GC11222@dastard> <20130725171509.GZ3111@xxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Thu, Jul 25, 2013 at 12:15:09PM -0500, Ben Myers wrote:
> Dave,
> 
> On Thu, Jul 25, 2013 at 10:48:21AM +1000, Dave Chinner wrote:
> > On Tue, Jul 23, 2013 at 01:26:48PM -0500, Ben Myers wrote:
> > > On Fri, Jun 07, 2013 at 10:25:26AM +1000, Dave Chinner wrote:
> > > > From: Dave Chinner <dchinner@xxxxxxxxxx>
> > > > 
> > > > Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> > > 
> > > This patch mostly corresponds to commit ee1a47ab0e, and in some areas it 
> > > is
> > > equivalent but slightly different.  There are some other things in here 
> > > too:
> > > 
> > > * Addition of XFS_BUF_DADDR_NULL
> > > * rename of b_blkno to b_bn in struct xfs_buf
> > > * rename of b_fsprivate to b_fspriv in struct xfs_buf
> > > * addition of uuid_copy and uuid_equal, and libuuid to build
> > > 
> > > It all looks fine to me, except as below:
> > 
> > I think you'll find they are fixed up in later patches in the
> > series.
> > 
> > Indeed, I think it's a little late to asking for these patches to be
> > changed, considering that making significant changes to these first
> > few patches will mean that I have to rebase a 100 or so subsequent
> > patches.
> 
> You are mistaken...

Mistaken about what, exactly? That a rebase will take days to do and
retest, while an additional patch will take minutes? 

I don't even have this series set up with guilt anymore - it's
been so long since it was committed to the crc-dev branch that I
moved over to working on that branch and adding patches on top of it
to fix issues. I've simply assumed that this patchset is fixed in
concrete and any problems will be layered on top.

People are already using this code, so we've already got a
significant amount of test exposure to it. Going back and modifying
and completely rebasing it invalidates all that test coverage.
Rebasing is not a risk-free operation, and when that is combined
with the amount of time needed for a rebase of such a series, I'd
much prefer that code that is in the crc-dev tree remains untouched
and we layer fixes on top of it...

> > For issues that aren't fixed in later patches, I'll add new patches
> > to the end of the current series to fix them.
> 
> ...but I have no objection to taking this approach, so long as my
> concerns are addressed.

Which, for this patch, have already been addressed in subsequent
patches....

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx

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