From: Leandro Dorileo <l@dorileo.org>
To: Kevin Wolf <kwolf@redhat.com>
Cc: Chunyan Liu <cyliu@suse.com>, stefanha@redhat.com, qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH v22 00/25] replace QEMUOptionParameter with QemuOpts
Date: Fri, 21 Mar 2014 12:21:28 +0000 [thread overview]
Message-ID: <20140321122128.GA18432@dorilex> (raw)
In-Reply-To: <20140321103453.GB3091@noname.redhat.com>
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
prev parent reply other threads:[~2014-03-21 12:22 UTC|newest]
Thread overview: 105+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-03-10 7:31 [Qemu-devel] [PATCH v22 00/25] replace QEMUOptionParameter with QemuOpts Chunyan Liu
2014-03-10 7:31 ` [Qemu-devel] [PATCH v22 01/25] add def_value_str to QemuOptDesc Chunyan Liu
2014-03-10 19:52 ` Eric Blake
2014-03-11 13:29 ` Stefan Hajnoczi
2014-03-12 2:45 ` Chunyan Liu
2014-03-12 8:27 ` Stefan Hajnoczi
2014-03-13 2:46 ` Chunyan Liu
2014-03-13 12:13 ` Stefan Hajnoczi
2014-03-10 7:31 ` [Qemu-devel] [PATCH v22 02/25] qapi: output def_value_str when query command line options Chunyan Liu
2014-03-10 19:57 ` Eric Blake
2014-03-11 6:14 ` Hu Tao
2014-03-10 7:31 ` [Qemu-devel] [PATCH v22 03/25] improve some functions in qemu-option.c Chunyan Liu
2014-03-10 20:29 ` Eric Blake
2014-03-10 21:21 ` Eric Blake
2014-03-11 7:26 ` Chunyan Liu
2014-03-11 21:00 ` Leandro Dorileo
2014-03-16 21:19 ` Leandro Dorileo
2014-03-18 7:41 ` Chunyan Liu
2014-03-12 6:49 ` Chunyan Liu
2014-03-17 19:58 ` Leandro Dorileo
2014-03-18 7:49 ` Chunyan Liu
2014-03-10 7:31 ` [Qemu-devel] [PATCH v22 04/25] improve assertion in qemu_opt_get functions Chunyan Liu
2014-03-10 21:44 ` Eric Blake
2014-03-12 6:34 ` Chunyan Liu
2014-03-10 7:31 ` [Qemu-devel] [PATCH v22 05/25] add some QemuOpts functions for replace work Chunyan Liu
2014-03-10 23:28 ` Eric Blake
2014-03-11 5:29 ` Chunyan Liu
2014-03-11 11:59 ` Eric Blake
2014-03-12 3:10 ` Chunyan Liu
2014-03-12 12:40 ` Eric Blake
2014-03-13 5:16 ` Chunyan Liu
2014-03-18 5:34 ` Chunyan Liu
2014-03-17 19:35 ` Leandro Dorileo
2014-03-18 3:03 ` Chunyan Liu
2014-03-10 7:31 ` [Qemu-devel] [PATCH v22 06/25] add convert functions between QEMUOptionParameter to QemuOpts Chunyan Liu
2014-03-11 4:46 ` Eric Blake
2014-03-10 7:31 ` [Qemu-devel] [PATCH v22 07/25] change block layer to support both QemuOpts and QEMUOptionParamter Chunyan Liu
2014-03-11 4:34 ` Eric Blake
2014-03-11 16:54 ` Eric Blake
2014-03-12 6:26 ` Chunyan Liu
2014-03-10 7:31 ` [Qemu-devel] [PATCH v22 08/25] cow.c: replace QEMUOptionParameter with QemuOpts Chunyan Liu
2014-03-11 14:12 ` Stefan Hajnoczi
2014-03-11 15:28 ` Eric Blake
2014-03-20 6:56 ` Chunyan Liu
2014-03-10 7:31 ` [Qemu-devel] [PATCH v22 09/25] gluster.c: " Chunyan Liu
2014-03-11 14:15 ` Stefan Hajnoczi
2014-03-11 16:58 ` Eric Blake
2014-03-10 7:31 ` [Qemu-devel] [PATCH v22 10/25] iscsi.c: " Chunyan Liu
2014-03-11 14:17 ` Stefan Hajnoczi
2014-03-10 7:31 ` [Qemu-devel] [PATCH v22 11/25] qcow.c: " Chunyan Liu
2014-03-11 14:18 ` Stefan Hajnoczi
2014-03-11 17:05 ` Eric Blake
2014-03-10 7:31 ` [Qemu-devel] [PATCH v22 12/25] qcow2.c: " Chunyan Liu
2014-03-11 14:21 ` Stefan Hajnoczi
2014-03-11 14:22 ` Stefan Hajnoczi
2014-03-10 7:31 ` [Qemu-devel] [PATCH v22 13/25] qed.c: " Chunyan Liu
2014-03-11 14:24 ` Stefan Hajnoczi
2014-03-20 9:08 ` Chun Yan Liu
2014-03-20 14:14 ` Stefan Hajnoczi
2014-03-10 7:31 ` [Qemu-devel] [PATCH v22 14/25] raw-posix.c: " Chunyan Liu
2014-03-11 14:25 ` Stefan Hajnoczi
2014-03-10 7:31 ` [Qemu-devel] [PATCH v22 15/25] raw-win32.c: " Chunyan Liu
2014-03-11 14:41 ` Stefan Hajnoczi
2014-03-10 7:31 ` [Qemu-devel] [PATCH v22 16/25] raw_bsd.c: " Chunyan Liu
2014-03-11 14:44 ` Stefan Hajnoczi
2014-03-10 7:31 ` [Qemu-devel] [PATCH v22 17/25] rbd.c: " Chunyan Liu
2014-03-11 14:46 ` Stefan Hajnoczi
2014-03-10 7:31 ` [Qemu-devel] [PATCH v22 18/25] sheepdog.c: " Chunyan Liu
2014-03-11 16:01 ` Stefan Hajnoczi
2014-03-10 7:31 ` [Qemu-devel] [PATCH v22 19/25] ssh.c: " Chunyan Liu
2014-03-11 16:01 ` Stefan Hajnoczi
2014-03-10 7:31 ` [Qemu-devel] [PATCH v22 20/25] vdi.c: " Chunyan Liu
2014-03-11 17:50 ` Stefan Hajnoczi
2014-03-10 7:31 ` [Qemu-devel] [PATCH v22 21/25] vmdk.c: " Chunyan Liu
2014-03-11 17:51 ` Stefan Hajnoczi
2014-03-10 7:31 ` [Qemu-devel] [PATCH v22 22/25] vpc.c: " Chunyan Liu
2014-03-11 17:55 ` Stefan Hajnoczi
2014-03-10 7:31 ` [Qemu-devel] [PATCH v22 23/25] vhdx.c: " Chunyan Liu
2014-03-11 17:56 ` Stefan Hajnoczi
2014-03-10 7:32 ` [Qemu-devel] [PATCH v22 24/25] vvfat.c: " Chunyan Liu
2014-03-11 17:06 ` Eric Blake
2014-03-11 18:01 ` Stefan Hajnoczi
2014-03-10 7:32 ` [Qemu-devel] [PATCH v22 25/25] cleanup QEMUOptionParameter Chunyan Liu
2014-03-11 14:06 ` Stefan Hajnoczi
2014-03-17 19:29 ` Leandro Dorileo
2014-03-17 19:43 ` Leandro Dorileo
2014-03-10 7:36 ` [Qemu-devel] [PATCH v22 00/25] replace QEMUOptionParameter with QemuOpts Chun Yan Liu
2014-03-10 7:37 ` Chun Yan Liu
2014-03-10 7:37 ` Chun Yan Liu
2014-03-10 7:38 ` Chun Yan Liu
2014-03-10 7:39 ` Chun Yan Liu
2014-03-10 7:39 ` Chun Yan Liu
2014-03-10 20:22 ` Stefan Hajnoczi
2014-03-11 3:07 ` Chunyan Liu
2014-03-10 22:45 ` Eric Blake
2014-03-11 18:03 ` Stefan Hajnoczi
2014-03-21 0:07 ` Leandro Dorileo
2014-03-21 10:09 ` Chunyan Liu
2014-03-21 12:31 ` Leandro Dorileo
2014-03-24 3:02 ` Chunyan Liu
2014-03-24 15:00 ` Leandro Dorileo
2014-03-25 7:15 ` Chunyan Liu
2014-04-03 9:46 ` Chunyan Liu
2014-03-21 10:34 ` Kevin Wolf
2014-03-21 12:21 ` Leandro Dorileo [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20140321122128.GA18432@dorilex \
--to=l@dorileo.org \
--cc=cyliu@suse.com \
--cc=kwolf@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=stefanha@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).