Re: [PATCH v4] migration/rdma: add x-rdma-chunk-size parameter

Hi Peter,

Sorry for late reply. I just got the time and machine to look into this.

For current upstream code, qemu_rdma_write() len is always
TARGET_PAGE_SIZE (4KB), which is far less than the 1MB chunk size.
The mergeable check bounds current_length within a single chunk, so
current_length can never reach rdma_merge_max(). You're right that
it's dead code in this case.

I did experiment with len being 2MB and chunk size being 1MB, where
the condition does trigger: mergeable fails, current_length resets
to 0, then += 2MB, hitting rdma_merge_max() (2 * 1MB = 2MB). But
even in that case, removing this flush caused no issue, since the
buffer gets flushed on the next qemu_rdma_write() call when
mergeable fails anyway or on rdma_registration_stop() at the end.

Thanks,
Samuel


On 2026/7/30 03:18, Peter Xu wrote:
> I'm looking at some RDMA security tickets recently, and I stumbled on top
> of this chunk size problem.  Since this is very recently touched, I figured
> maybe I should ask here,
>
> On Mon, Mar 30, 2026 at 02:47:58PM +0800, Samuel Zhang wrote:
>> @@ -2176,7 +2178,7 @@ static int qemu_rdma_write(RDMAContext *rdma,
>>       rdma->current_length += len;
>>   
>>       /* flush it if buffer is too large */
>> -    if (rdma->current_length >= RDMA_MERGE_MAX) {
>> +    if (rdma->current_length >= rdma_merge_max()) {
> [1]
>
> What is this rdma_merge_max()?  Why it's twice the chunk size, and even if
> I believe when reaching here the current_length should have been throttled
> by chunk size too?
>
>>           return qemu_rdma_write_flush(rdma, errp);
>>       }
> I mean, in qemu_rdma_buffer_mergeable():
>
>      ...
>      chunk_end = ram_chunk_end(block, rdma->current_chunk);
>      ...
>      if ((host_addr + len) > chunk_end) {
>          return false;
>      }
>
> I believe it means when len will go beyond chunk size, it'll force a flush
> already.
>
> With that, when reaching [1] above, we either should have just flushed, or
> we're under chunk size (which further guarantees smaller than
> 2*chunk_size).
>
> What did I miss?  Or, is this line [1] dead code?
>
> Thanks,
>