Re: [PATCH v2 33/53] migration/rdma: Fix error handling around rdma_getaddrinfo()

[Juan Quintela]

Markus Armbruster <armbru@redhat.com> 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>

Reviewed-by: Juan Quintela <quintela@redhat.com>