qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v22 00/25] replace QEMUOptionParameter with Qemu


From: Leandro Dorileo
Subject: Re: [Qemu-devel] [PATCH v22 00/25] replace QEMUOptionParameter with QemuOpts
Date: Fri, 21 Mar 2014 12:21:28 +0000
User-agent: Mutt/1.5.23 (2014-03-12)

Hi Kevin,

On Fri, Mar 21, 2014 at 11:34:53AM +0100, Kevin Wolf wrote:
> Am 21.03.2014 um 01:07 hat Leandro Dorileo geschrieben:
> > Hi Chunyan,
> > 
> > On Mon, Mar 10, 2014 at 03:31:36PM +0800, Chunyan Liu wrote:
> > > This patch series is to replace QEMUOptionParameter with QemuOpts, so 
> > > that only
> > > one Qemu Option structure is kept in QEMU code.
> > > 
> > 
> > 
> > Last night I took some time do take a deeper look at you series and the 
> > required
> > effort to do the QemuOptionParameter -> QemuOpts migration.
> > 
> > I think you've over complicated the things, I understand you tried to keep 
> > your
> > serie's bisectability (?), but the final result was something really hard to
> > review and to integrate as well. The overall approach wasn't even resolving 
> > the
> > bisectability problem since it breaks the tree until the last commit. 
> > Moreover,
> > in the path of getting things ready you created new problems and their 
> > respective
> > fixes, what we really don't need to.
> > 
> > In this regards you could have kept things as simple as possible and 
> > submitted
> > the patches in a "natural way", even if they were breaking the build 
> > between each
> > patch, you could get all the required maintainer's Reviewed-by + Tested-by +
> > Signed-off-by and so on for each individual patch and when it was time to
> > integrate get squashed the needed patches.
> 
> No, please not. If you have bisectability on the surface, but the bisect
> ends in a monster patch with several thousand lines of code, nothing is
> won.

I don't think that is the case (several thousand lines), with the experiment I 
did the
"squashable" patches are 769 insertions(+), 1076 deletions(-), of course, the 
patch
series is bigger than that - not that bigger, but bigger - but I'm mentioning 
only
the patches I think need to be squashed.

These patches represent the changes across the block layer. I see no gain on 
increasing
the patches complexity just to avoid a patch with that statistic. More over, 
the changes
are very simple and easily reviewable.

Regards...

-- 
Leandro Dorileo



reply via email to

[Prev in Thread] Current Thread [Next in Thread]