[PATCH v5 13/13] util/vhost-user-server: vu_message_read(): improve error handling

Vladimir Sementsov-Ogievskiy posted 13 patches 1 week, 5 days ago
Maintainers: Eric Blake <eblake@redhat.com>, Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>, Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>, Gerd Hoffmann <kraxel@redhat.com>, Gustavo Romero <gustavo.romero@linaro.org>, Elena Ufimtseva <elena.ufimtseva@oracle.com>, Jagannathan Raman <jag.raman@oracle.com>, John Levon <john.levon@nutanix.com>, Thanos Makatos <thanos.makatos@nutanix.com>, "Cédric Le Goater" <clg@redhat.com>, Stefano Garzarella <sgarzare@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Hailiang Zhang <zhanghailiang@xfusion.com>, Peter Xu <peterx@redhat.com>, Fabiano Rosas <farosas@suse.de>, Jason Wang <jasowang@redhat.com>, Michael Roth <michael.roth@amd.com>, Kostiantyn Kostiuk <kkostiuk@redhat.com>, Fam Zheng <fam@euphon.net>, Alexander Bulekov <alxndr@bu.edu>, Bandan Das <bsd@redhat.com>, Stefan Hajnoczi <stefanha@redhat.com>, Darren Kenny <darren.kenny@oracle.com>, Qiuhao Li <Qiuhao.Li@outlook.com>, Laurent Vivier <lvivier@redhat.com>, Zhao Liu <zhao1.liu@intel.com>, Stefan Weil <sw@weilnetz.de>, Coiby Xu <Coiby.Xu@gmail.com>
[PATCH v5 13/13] util/vhost-user-server: vu_message_read(): improve error handling
Posted by Vladimir Sementsov-Ogievskiy 1 week, 5 days ago
1. Drop extra error_report_err(NULL), it will just crash, if we get
here.

2. Get and report error of qemu_set_blocking(), instead of aborting.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 util/vhost-user-server.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/util/vhost-user-server.c b/util/vhost-user-server.c
index 04c72a92aa..1dbe409f82 100644
--- a/util/vhost-user-server.c
+++ b/util/vhost-user-server.c
@@ -62,7 +62,7 @@ static void vmsg_close_fds(VhostUserMsg *vmsg)
     }
 }
 
-static void vmsg_unblock_fds(VhostUserMsg *vmsg)
+static bool vmsg_unblock_fds(VhostUserMsg *vmsg, Error **errp)
 {
     int i;
 
@@ -74,13 +74,16 @@ static void vmsg_unblock_fds(VhostUserMsg *vmsg)
      */
     if (vmsg->request == VHOST_USER_ADD_MEM_REG ||
         vmsg->request == VHOST_USER_SET_MEM_TABLE) {
-        return;
+        return true;
     }
 
     for (i = 0; i < vmsg->fd_num; i++) {
-        /* TODO: handle error more gracefully than aborting */
-        qemu_set_blocking(vmsg->fds[i], false, &error_abort);
+        if (!qemu_set_blocking(vmsg->fds[i], false, errp)) {
+            return false;
+        }
     }
+
+    return true;
 }
 
 static void panic_cb(VuDev *vu_dev, const char *buf)
@@ -123,7 +126,6 @@ vu_message_read(VuDev *vu_dev, int conn_fd, VhostUserMsg *vmsg)
 
     vmsg->fd_num = 0;
     if (!ioc) {
-        error_report_err(local_err);
         goto fail;
     }
 
@@ -177,7 +179,10 @@ vu_message_read(VuDev *vu_dev, int conn_fd, VhostUserMsg *vmsg)
     } while (read_bytes != VHOST_USER_HDR_SIZE);
 
     /* qio_channel_readv_full will make socket fds blocking, unblock them */
-    vmsg_unblock_fds(vmsg);
+    if (!vmsg_unblock_fds(vmsg, &local_err)) {
+        error_report_err(local_err);
+        goto fail;
+    }
     if (vmsg->size > sizeof(vmsg->payload)) {
         error_report("Error: too big message request: %d, "
                      "size: vmsg->size: %u, "
-- 
2.48.1
Re: [PATCH v5 13/13] util/vhost-user-server: vu_message_read(): improve error handling
Posted by Daniel P. Berrangé 1 week, 5 days ago
On Tue, Sep 16, 2025 at 04:14:02PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> 1. Drop extra error_report_err(NULL), it will just crash, if we get
> here.
> 
> 2. Get and report error of qemu_set_blocking(), instead of aborting.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> ---
>  util/vhost-user-server.c | 17 +++++++++++------
>  1 file changed, 11 insertions(+), 6 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|


Re: [PATCH v5 13/13] util/vhost-user-server: vu_message_read(): improve error handling
Posted by Vladimir Sementsov-Ogievskiy 1 week, 4 days ago
On 16.09.25 18:50, Daniel P. Berrangé wrote:
> On Tue, Sep 16, 2025 at 04:14:02PM +0300, Vladimir Sementsov-Ogievskiy wrote:
>> 1. Drop extra error_report_err(NULL), it will just crash, if we get
>> here.
>>
>> 2. Get and report error of qemu_set_blocking(), instead of aborting.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
>> ---
>>   util/vhost-user-server.c | 17 +++++++++++------
>>   1 file changed, 11 insertions(+), 6 deletions(-)
> 
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> 

Thanks a lot!

Now the whole series is reviewed. Will you queue it
(together with base "[PATCH v4 0/2] save qemu-file incoming non-blocking fds")?
Or we should wait for ACCs from other maintainers?

-- 
Best regards,
Vladimir

Re: [PATCH v5 13/13] util/vhost-user-server: vu_message_read(): improve error handling
Posted by Daniel P. Berrangé 1 week, 2 days ago
On Wed, Sep 17, 2025 at 01:13:15PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> On 16.09.25 18:50, Daniel P. Berrangé wrote:
> > On Tue, Sep 16, 2025 at 04:14:02PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> > > 1. Drop extra error_report_err(NULL), it will just crash, if we get
> > > here.
> > > 
> > > 2. Get and report error of qemu_set_blocking(), instead of aborting.
> > > 
> > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> > > ---
> > >   util/vhost-user-server.c | 17 +++++++++++------
> > >   1 file changed, 11 insertions(+), 6 deletions(-)
> > 
> > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> > 
> 
> Thanks a lot!
> 
> Now the whole series is reviewed. Will you queue it
> (together with base "[PATCH v4 0/2] save qemu-file incoming non-blocking fds")?
> Or we should wait for ACCs from other maintainers?

I'm queuing it, with fixes for the minor problems I pointed out.

If possible, try to feed big patch series through gitlab.com CI in
your own qemu.git fork before sending to catch all edge cases.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|


Re: [PATCH v5 13/13] util/vhost-user-server: vu_message_read(): improve error handling
Posted by Vladimir Sementsov-Ogievskiy 1 week, 2 days ago
On 19.09.25 13:27, Daniel P. Berrangé wrote:
> On Wed, Sep 17, 2025 at 01:13:15PM +0300, Vladimir Sementsov-Ogievskiy wrote:
>> On 16.09.25 18:50, Daniel P. Berrangé wrote:
>>> On Tue, Sep 16, 2025 at 04:14:02PM +0300, Vladimir Sementsov-Ogievskiy wrote:
>>>> 1. Drop extra error_report_err(NULL), it will just crash, if we get
>>>> here.
>>>>
>>>> 2. Get and report error of qemu_set_blocking(), instead of aborting.
>>>>
>>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
>>>> ---
>>>>    util/vhost-user-server.c | 17 +++++++++++------
>>>>    1 file changed, 11 insertions(+), 6 deletions(-)
>>>
>>> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
>>>
>>
>> Thanks a lot!
>>
>> Now the whole series is reviewed. Will you queue it
>> (together with base "[PATCH v4 0/2] save qemu-file incoming non-blocking fds")?
>> Or we should wait for ACCs from other maintainers?
> 
> I'm queuing it, with fixes for the minor problems I pointed out.
> 
> If possible, try to feed big patch series through gitlab.com CI in
> your own qemu.git fork before sending to catch all edge cases.
> 

Thanks! I'll try do it with "[PATCH v5 00/19] virtio-net: live-TAP local migration".

-- 
Best regards,
Vladimir