[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 resend 4/8] rdma: core logic
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH v3 resend 4/8] rdma: core logic |
Date: |
Thu, 18 Jul 2013 14:22:59 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130625 Thunderbird/17.0.7 |
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, address@hidden wrote:
>>> From: "Michael R. Hines" <address@hidden>
>>>
>>> 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
- [Qemu-devel] [PATCH v3 resend 1/8] rdma: update documentation to reflect new unpin support, (continued)
- [Qemu-devel] [PATCH v3 resend 1/8] rdma: update documentation to reflect new unpin support, mrhines, 2013/07/16
- [Qemu-devel] [PATCH v3 resend 2/8] rdma: bugfix: ram_control_save_page(), mrhines, 2013/07/16
- [Qemu-devel] [PATCH v3 resend 5/8] rdma: send pc.ram, mrhines, 2013/07/16
- [Qemu-devel] [PATCH v3 resend 7/8] rdma: introduce MIG_STATE_NONE and change MIG_STATE_SETUP state transition, mrhines, 2013/07/16
- [Qemu-devel] [PATCH v3 resend 3/8] rdma: introduce ram_handle_compressed(), mrhines, 2013/07/16
- [Qemu-devel] [PATCH v3 resend 8/8] rdma: account for the time spent in MIG_STATE_SETUP through QMP, mrhines, 2013/07/16
- [Qemu-devel] [PATCH v3 resend 6/8] rdma: allow state transitions between other states besides ACTIVE, mrhines, 2013/07/16
- [Qemu-devel] [PATCH v3 resend 4/8] rdma: core logic, mrhines, 2013/07/16
- [Qemu-devel] [PATCH v3 resend 0/8] rdma: core logic, mrhines, 2013/07/22
- [Qemu-devel] [PATCH v3 resend 7/8] rdma: introduce MIG_STATE_NONE and change MIG_STATE_SETUP state transition, mrhines, 2013/07/22
- [Qemu-devel] [PATCH v3 resend 2/8] rdma: bugfix: ram_control_save_page(), mrhines, 2013/07/22
- [Qemu-devel] [PATCH v3 resend 1/8] rdma: update documentation to reflect new unpin support, mrhines, 2013/07/22
- [Qemu-devel] [PATCH v3 resend 3/8] rdma: introduce ram_handle_compressed(), mrhines, 2013/07/22
- [Qemu-devel] [PATCH v3 resend 5/8] rdma: send pc.ram, mrhines, 2013/07/22
- [Qemu-devel] [PATCH v3 resend 4/8] rdma: core logic, mrhines, 2013/07/22
- [Qemu-devel] [PATCH v3 resend 8/8] rdma: account for the time spent in MIG_STATE_SETUP through QMP, mrhines, 2013/07/22
- [Qemu-devel] [PATCH v3 resend 6/8] rdma: allow state transitions between other states besides ACTIVE, mrhines, 2013/07/22