Re: [PATCH 11/52] migration/rdma: Drop rdma_add_block() error handling

[Zhijian Li (Fujitsu)]

On 21/09/2023 19:15, Markus Armbruster wrote:
> "Zhijian Li (Fujitsu)" <lizhijian@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<armbru@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 <lizhijian@fujitsu.com>