Re: [PATCH v2 3/3] failover: really display a warning when the primary device is not found

[Jason Wang <jasowang@redhat.com>]

On 2021/2/11 上午1:45, Laurent Vivier wrote:
> In failover_add_primary(), we search the id of the failover device by
> scanning the list of the devices in the opts list to find a device with
> a failover_pair_id equals to the id of the virtio-net device.
>
> If the failover_pair_id is not found, QEMU ignores the primary
> device silently (which also means it will not be hidden and
> it will be enabled directly at boot).
>
> After that, we search the id in the opts list to do a qdev_device_add()
> with it. The device will be always found as otherwise we had exited
> before, and thus the warning is never displayed.
>
> Fix that by moving the error report to the first exit condition.
> Also add a g_assert() to be sure the compiler will not complain
> about a possibly NULL pointer.
>
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>


Acked-by: Jason Wang <jasowang@redhat.com>


> ---
>   hw/net/virtio-net.c | 20 +++++++++-----------
>   1 file changed, 9 insertions(+), 11 deletions(-)
>
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index 1c5af08dc556..439f823b190c 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -855,21 +855,19 @@ static void failover_add_primary(VirtIONet *n, Error **errp)
>   
>       id = failover_find_primary_device_id(n);
>       if (!id) {
> +        error_setg(errp, "Primary device not found");
> +        error_append_hint(errp, "Virtio-net failover will not work. Make "
> +                          "sure primary device has parameter"
> +                          " failover_pair_id=%s\n", n->netclient_name);
>           return;
>       }
>       opts = qemu_opts_find(qemu_find_opts("device"), id);
> -    if (opts) {
> -        dev = qdev_device_add(opts, &err);
> -        if (err) {
> -            qemu_opts_del(opts);
> -        } else {
> -            object_unref(OBJECT(dev));
> -        }
> +    g_assert(opts); /* cannot be NULL because id was found using opts list */
> +    dev = qdev_device_add(opts, &err);
> +    if (err) {
> +        qemu_opts_del(opts);
>       } else {
> -        error_setg(errp, "Primary device not found");
> -        error_append_hint(errp, "Virtio-net failover will not work. Make "
> -                          "sure primary device has parameter"
> -                          " failover_pair_id=<virtio-net-id>\n");
> +        object_unref(OBJECT(dev));
>       }
>       error_propagate(errp, err);
>   }