On 21/09/2023 19:15, Markus Armbruster wrote:
> "Zhijian Li (Fujitsu)" <lizhij...@fujitsu.com> writes:
>
>> On 18/09/2023 22:41, Markus Armbruster wrote:
>>> rdma_add_block() can't fail. Return void, and drop the unreachable
>>> error handling.
>>>
>>> Signed-off-by: Markus Armbruster<arm...@redhat.com>
>>> ---
>>> migration/rdma.c | 30 +++++++++---------------------
>>> 1 file changed, 9 insertions(+), 21 deletions(-)
>>>
>>
>> [...]
>>
>>> * during dynamic page registration.
>>> */
>>> -static int qemu_rdma_init_ram_blocks(RDMAContext *rdma)
>>> +static void qemu_rdma_init_ram_blocks(RDMAContext *rdma)
>>> {
>>> RDMALocalBlocks *local = &rdma->local_ram_blocks;
>>> int ret;
>>> @@ -646,14 +645,11 @@ static int qemu_rdma_init_ram_blocks(RDMAContext
>>> *rdma)
>>> assert(rdma->blockmap == NULL);
>>> memset(local, 0, sizeof *local);
>>> ret = foreach_not_ignored_block(qemu_rdma_init_one_block, rdma);
>>> - if (ret) {
>>> - return ret;
>>> - }
>>> + assert(!ret);
>>
>> Why we still need a new assert(), can we remove the ret together.
>>
>> foreach_not_ignored_block(qemu_rdma_init_one_block, rdma);
>> trace_qemu_rdma_init_ram_blocks(local->nb_blocks);
>
> The "the callback doesn't fail" is a non-local argument. The assertion
> checks it. I'd be fine with dropping it, since the argument is
> straightforward enough. Thoughts?
>
Both are fine, I prefer to drop it personally. :)
Anyway,
Reviewed-by: Li Zhijian <lizhij...@fujitsu.com>
