[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 32/52] migration/rdma: Fix error handling around rdma_getaddr
From: |
Zhijian Li (Fujitsu) |
Subject: |
Re: [PATCH 32/52] migration/rdma: Fix error handling around rdma_getaddrinfo() |
Date: |
Mon, 25 Sep 2023 07:32:12 +0000 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.11.0 |
On 18/09/2023 22:41, Markus Armbruster wrote:
> qemu_rdma_resolve_host() and qemu_rdma_dest_init() iterate over
> addresses to find one that works, holding onto the first Error from
> qemu_rdma_broken_ipv6_kernel() for use when no address works. Issues:
>
> 1. If @errp was &error_abort or &error_fatal, we'd terminate instead
> of trying the next address. Can't actually happen, since no caller
> passes these arguments.
>
> 2. When @errp is a pointer to a variable containing NULL, and
> qemu_rdma_broken_ipv6_kernel() fails, the variable no longer
> contains NULL. Subsequent iterations pass it again, violating
> Error usage rules. Dangerous, as setting an error would then trip
> error_setv()'s assertion. Works only because
> qemu_rdma_broken_ipv6_kernel() and the code following the loops
> carefully avoids setting a second error.
>
> 3. If qemu_rdma_broken_ipv6_kernel() fails, and then a later iteration
> finds a working address, @errp still holds the first error from
> qemu_rdma_broken_ipv6_kernel(). If we then run into another error,
> we report the qemu_rdma_broken_ipv6_kernel() failure instead.
>
> 4. If we don't run into another error, we leak the Error object.
>
> Use a local error variable, and propagate to @errp. This fixes 3. and
> also cleans up 1 and partly 2.
>
> Free this error when we have a working address. This fixes 4.
>
> Pass the local error variable to qemu_rdma_broken_ipv6_kernel() only
> until it fails. Pass null on any later iterations. This cleans up
> the remainder of 2.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Li Zhijian <lizhijian@fujitsu.com>
- [PATCH 27/52] migration/rdma: Drop superfluous assignments to @ret, (continued)
- [PATCH 27/52] migration/rdma: Drop superfluous assignments to @ret, Markus Armbruster, 2023/09/18
- [PATCH 20/52] migration/rdma: Drop dead qemu_rdma_data_init() code for !@host_port, Markus Armbruster, 2023/09/18
- [PATCH 46/52] migration/rdma: Silence qemu_rdma_reg_control(), Markus Armbruster, 2023/09/18
- [PATCH 06/52] migration/rdma: Clean up two more harmless signed vs. unsigned issues, Markus Armbruster, 2023/09/18
- [PATCH 32/52] migration/rdma: Fix error handling around rdma_getaddrinfo(), Markus Armbruster, 2023/09/18
- Re: [PATCH 32/52] migration/rdma: Fix error handling around rdma_getaddrinfo(),
Zhijian Li (Fujitsu) <=
- [PATCH 11/52] migration/rdma: Drop rdma_add_block() error handling, Markus Armbruster, 2023/09/18
- [PATCH 34/52] migration/rdma: Convert qemu_rdma_exchange_recv() to Error, Markus Armbruster, 2023/09/18
- [PATCH 30/52] migration/rdma: Delete inappropriate error_report() in macro ERROR(), Markus Armbruster, 2023/09/18
- [PATCH 14/52] migration/rdma: Use bool for two RDMAContext flags, Markus Armbruster, 2023/09/18