Il 10/04/2013 14:34, Michael R. Hines ha scritto:
> On 04/10/2013 03:50 AM, Paolo Bonzini wrote:
>> Il 10/04/2013 06:29, [email protected] ha scritto:
>>> From: "Michael R. Hines" <[email protected]>
>>>
>>> RDMA performs very slowly with zero-page checking.
>>> Without the ability to disable it, RDMA throughput and
>>> latency promises and high performance links cannot be
>>> fully realized.
>>>
>>> On the other hand, dynamic page registration support is also
>>> included in the RDMA protocol. This second capability also
>>> cannot be fully realized without the ability to enable zero
>>> page scanning.
>>>
>>> So, we have two new capabilities which work together:
>>>
>>> 1. migrate_set_capability check_for_zero on|off (default on)
>>> 2. migrate_set_capability chunk_register_destination on|off (default
>>> off)
>>>
>>> Signed-off-by: Michael R. Hines <[email protected]>
>> Michael, please listen to _all_ review comments.
>>
>> 1) I asked you to place check_for_zero in a separate patch.
>>
>> 2) Again, patch 3 cannot compile without this one. The code should
>> compile after each patch, with and without --enable-rdma.
>
> My apologies - I misunderstood the request. I am indeed addressing every
> comment.
>
> When you said separate, I thought you meant a different patch series
> altpgether.
>
> This is my first time, so the meaning of "separate" was not clear =)
>
> And yes, patch 3 does in fact compile both with and without --enable-rdma.
How does this:
+#ifdef CONFIG_RDMA
+const QEMURamControlOps qemu_rdma_write_control = {
+ .before_ram_iterate = qemu_ram_registration_start,
+ .after_ram_iterate = qemu_rdma_registration_stop,
+ .register_ram_iterate = qemu_rdma_registration_handle,
+ .save_page = qemu_rdma_save_page,
+};
compile (and link) successfully without a definition of
qemu_ram_registration_start, which is in patch 5? (Honest question).
Similarly, patch 5 cannot link without qemu_ram_foreach_block.
Anyway, patch 1 does not compile with --enable-rdma. :)
> You flip-flopped on me =)
> You said conditionalize it, then make a separate patch, then delete it
> altogether =)
Make qemu_set_nonblock conditional, dropping the assert.
Do not touch the implementation of qemu_set_nonblock. (Yes, this was a
mess).
Paolo
