[v3] Error reporting cleanup, a fix, and &error_warn removal

[PATCH v3 12/13] ivshmem-flat: Mark an instance of missing error handling FIXME

Markus Armbruster posted 13 patches 5 days, 7 hours ago

Diff against v1 v2
Download mbox

Maintainers: Jonathan Cameron <jonathan.cameron@huawei.com>, Fan Ni <fan.ni@samsung.com>, "Alex Bennée" <alex.bennee@linaro.org>, Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>, Dmitry Osipenko <dmitry.osipenko@collabora.com>, "Michael S. Tsirkin" <mst@redhat.com>, Gustavo Romero <gustavo.romero@linaro.org>, Jason Wang <jasowang@redhat.com>, Elena Ufimtseva <elena.ufimtseva@oracle.com>, Jagannathan Raman <jag.raman@oracle.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Markus Armbruster <armbru@redhat.com>, Michael Roth <michael.roth@amd.com>, Stefan Weil <sw@weilnetz.de>, "Daniel P. Berrangé" <berrange@redhat.com>, Steve Sistare <steven.sistare@oracle.com>, Peter Xu <peterx@redhat.com>, Fabiano Rosas <farosas@suse.de>, "Dr. David Alan Gilbert" <dave@treblig.org>, Samuel Thibault <samuel.thibault@ens-lyon.org>, Richard Henderson <richard.henderson@linaro.org>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, Stefan Hajnoczi <stefanha@redhat.com>, Fam Zheng <fam@euphon.net>

Expand all Fold all

[PATCH v3 12/13] ivshmem-flat: Mark an instance of missing error handling FIXME

Posted by Markus Armbruster 5 days, 7 hours ago

ivshmem-flat's ivshmem_flat_add_vector() neglects to handle
qemu_set_blocking() failure.  It used to silently ignore errors there.
Recent commit 6f607941b1c (treewide: use qemu_set_blocking instead of
g_unix_set_fd_nonblocking) changed it to warn (without mentioning it
the commit message, tsk, tsk, tsk).

Note that ivshmem-pci's process_msg_connect() handles this error.

Add a FIXME comment to mark the missing error handling.

Cc: Gustavo Romero <gustavo.romero@linaro.org>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/misc/ivshmem-flat.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/hw/misc/ivshmem-flat.c b/hw/misc/ivshmem-flat.c
index e83e6c6ee9..27ee8c9218 100644
--- a/hw/misc/ivshmem-flat.c
+++ b/hw/misc/ivshmem-flat.c
@@ -138,6 +138,8 @@ static void ivshmem_flat_remove_peer(IvshmemFTState *s, uint16_t peer_id)
 static void ivshmem_flat_add_vector(IvshmemFTState *s, IvshmemPeer *peer,
                                     int vector_fd)
 {
+    Error *err = NULL;
+
     if (peer->vector_counter >= IVSHMEM_MAX_VECTOR_NUM) {
         trace_ivshmem_flat_add_vector_failure(peer->vector_counter,
                                               vector_fd, peer->id);
@@ -154,8 +156,10 @@ static void ivshmem_flat_add_vector(IvshmemFTState *s, IvshmemPeer *peer,
      * peer.
      */
     peer->vector[peer->vector_counter].id = peer->vector_counter;
-    /* WARNING: qemu_socket_set_nonblock() return code ignored */
-    qemu_set_blocking(vector_fd, false, &error_warn);
+    if (!qemu_set_blocking(vector_fd, false, &err)) {
+        /* FIXME handle the error */
+        warn_report_err(err);
+    }
     event_notifier_init_fd(&peer->vector[peer->vector_counter].event_notifier,
                            vector_fd);
 
-- 
2.49.0

Re: [PATCH v3 12/13] ivshmem-flat: Mark an instance of missing error handling FIXME

Posted by Vladimir Sementsov-Ogievskiy 5 days, 6 hours ago

On 23.09.25 12:09, Markus Armbruster wrote:
> ivshmem-flat's ivshmem_flat_add_vector() neglects to handle
> qemu_set_blocking() failure.  It used to silently ignore errors there.
> Recent commit 6f607941b1c (treewide: use qemu_set_blocking instead of
> g_unix_set_fd_nonblocking) changed it to warn (without mentioning it
> the commit message, tsk, tsk, tsk).

Yes, my fault.
> 
> Note that ivshmem-pci's process_msg_connect() handles this error.
> 
> Add a FIXME comment to mark the missing error handling.
> 
> Cc: Gustavo Romero <gustavo.romero@linaro.org>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>

> ---
>   hw/misc/ivshmem-flat.c | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/misc/ivshmem-flat.c b/hw/misc/ivshmem-flat.c
> index e83e6c6ee9..27ee8c9218 100644
> --- a/hw/misc/ivshmem-flat.c
> +++ b/hw/misc/ivshmem-flat.c
> @@ -138,6 +138,8 @@ static void ivshmem_flat_remove_peer(IvshmemFTState *s, uint16_t peer_id)
>   static void ivshmem_flat_add_vector(IvshmemFTState *s, IvshmemPeer *peer,
>                                       int vector_fd)
>   {
> +    Error *err = NULL;
> +
>       if (peer->vector_counter >= IVSHMEM_MAX_VECTOR_NUM) {
>           trace_ivshmem_flat_add_vector_failure(peer->vector_counter,
>                                                 vector_fd, peer->id);
> @@ -154,8 +156,10 @@ static void ivshmem_flat_add_vector(IvshmemFTState *s, IvshmemPeer *peer,
>        * peer.
>        */
>       peer->vector[peer->vector_counter].id = peer->vector_counter;
> -    /* WARNING: qemu_socket_set_nonblock() return code ignored */
> -    qemu_set_blocking(vector_fd, false, &error_warn);
> +    if (!qemu_set_blocking(vector_fd, false, &err)) {
> +        /* FIXME handle the error */
> +        warn_report_err(err);
> +    }
>       event_notifier_init_fd(&peer->vector[peer->vector_counter].event_notifier,
>                              vector_fd);
>   


-- 
Best regards,
Vladimir