Re: [Qemu-devel] [PATCH v3 resend 4/8] rdma: core logic

[Paolo Bonzini <pbonzini@redhat.com>]

Il 18/07/2013 14:07, Michael R. Hines ha scritto:
> On 07/18/2013 03:30 AM, Marcel Apfelbaum wrote:
>> On Tue, 2013-07-16 at 12:48 -0400, mrhines@linux.vnet.ibm.com wrote:
>>> From: "Michael R. Hines" <mrhines@us.ibm.com>
>>>
>>> Code that does need to be visible is kept
>>> well contained inside this file and this is the only
>>> new additional file to the entire patch.
>>>
>>> This file includes the entire protocol and interfaces
>>> required to perform RDMA migration.
>>>
>>> Also, the configure and Makefile modifications to link
>>> this file are included.
>>>
>>> Full documentation is in docs/rdma.txt
>>>
>> This patch is too big (in my opinion).
>> I would split it into at least 3 patches:
>> 1. Generic RDMA code (this part can be reused by everyone who will
>> need RDMA in the future)
>> 2. RDMA transfer protocol (separating this will give us possibility
>> for optimization without touching the rest of the code)
>> 3. Migration related code
>>
> 
> Don't let the "v3" mislead you =). The patch actually *used* to look
> just like what
> you described (3 different ones), but after more than a dozen reviews
> since January
> I was told to join all the code into a single file by the reviewers.

Yeah, I guess I owe some explanation to Marcel (who is not RDMA-impaired
at all).  Because the reviewers (me especially) did not know much about
RDMA, we initially concentrated on having the right interfaces between
migration-rdma.c and the migration core.

Once we had something that core developers considered to be the "right"
interfaces, Michael's original split in 1/2/3 didn't make much sense
anymore, so in the end it was easier to just merge everything in a
single patch and treat it almost as a black box.

Having generic RDMA code in a separate file would be a nice thing, but I
don't think it's absolutely necessary in order to merge this code (which
I would like to have in 1.6).  We have already done "chainsaw" passes on
files in the past, at this point I prefer "release early, release often".

Paolo