All of lore.kernel.org
 help / color / mirror / Atom feed
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

      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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.