On Tue, Sep 16, 2025 at 04:13:53PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> Currently, we just always pass NULL as errp argument. That doesn't
> look good.
>
> Some realizations of interface may actually report errors.
> Channel-socket realization actually either ignore or crash on
> errors, but we are going to straighten it out to always reporting
> an errp in further commits.
>
> So, convert all callers to either handle the error (where environment
> allows) or explicitly use &error_abort.
>
> Take also a chance to change the return value to more convenient
> bool (keeping also in mind, that underlying realizations may
> return -1 on failure, not -errno).
>
> Suggested-by: Daniel P. Berrangé <[email protected]>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <[email protected]>
> Reviewed-by: Daniel P. Berrangé <[email protected]>
> ---
> block/nbd.c | 4 +++-
> chardev/char-socket.c | 20 ++++++++++++++++----
> hw/remote/proxy.c | 6 +++++-
> hw/remote/remote-obj.c | 6 +++++-
> hw/vfio-user/proxy.c | 11 ++++++++---
> include/io/channel.h | 6 +++---
> io/channel.c | 4 ++--
This needed to change channel-tls.c and channel-websock.c because
their impls of qio_channel_set_blocking callback in turn called
qio_channel_set_blocking, so had an int vs bool conversion mistake
introduced in this patch.
> nbd/server.c | 4 +++-
> scsi/qemu-pr-helper.c | 9 ++++++---
> tests/unit/io-channel-helpers.c | 5 +++--
> tests/unit/test-io-channel-tls.c | 4 ++--
> tools/i386/qemu-vmsr-helper.c | 6 ++++--
> ui/vnc.c | 2 +-
> util/vhost-user-server.c | 7 ++++++-
> 14 files changed, 67 insertions(+), 27 deletions(-)
>
> diff --git a/block/nbd.c b/block/nbd.c
> index d5a2b21c6d..5d231d5c4e 100644
> --- a/block/nbd.c
> +++ b/block/nbd.c
> @@ -351,7 +351,9 @@ int coroutine_fn
> nbd_co_do_establish_connection(BlockDriverState *bs,
> return ret;
> }
>
> - qio_channel_set_blocking(s->ioc, false, NULL);
> + if (!qio_channel_set_blocking(s->ioc, false, errp)) {
> + return -EINVAL;
> + }
> qio_channel_set_follow_coroutine_ctx(s->ioc, true);
>
> /* successfully connected */
> diff --git a/chardev/char-socket.c b/chardev/char-socket.c
> index 1be078dfc0..cb4ec78ebe 100644
> --- a/chardev/char-socket.c
> +++ b/chardev/char-socket.c
> @@ -530,16 +530,24 @@ static int tcp_chr_sync_read(Chardev *chr, const
> uint8_t *buf, int len)
> SocketChardev *s = SOCKET_CHARDEV(chr);
> int size;
> int saved_errno;
> + Error *local_err = NULL;
>
> if (s->state != TCP_CHARDEV_STATE_CONNECTED) {
> return 0;
> }
>
> - qio_channel_set_blocking(s->ioc, true, NULL);
> + if (!qio_channel_set_blocking(s->ioc, true, &local_err)) {
> + error_report_err(local_err);
> + return -1;
> + }
> size = tcp_chr_recv(chr, (void *) buf, len);
> saved_errno = errno;
> if (s->state != TCP_CHARDEV_STATE_DISCONNECTED) {
> - qio_channel_set_blocking(s->ioc, false, NULL);
> + if (!qio_channel_set_blocking(s->ioc, false, &local_err)) {
> + error_report_err(local_err);
> + /* failed to recover non-blocking state */
> + tcp_chr_disconnect(chr);
> + }
> }
> if (size == 0) {
> /* connection closed */
> @@ -884,18 +892,22 @@ static void tcp_chr_set_client_ioc_name(Chardev *chr,
> static int tcp_chr_new_client(Chardev *chr, QIOChannelSocket *sioc)
> {
> SocketChardev *s = SOCKET_CHARDEV(chr);
> + Error *local_err = NULL;
>
> if (s->state != TCP_CHARDEV_STATE_CONNECTING) {
> return -1;
> }
>
> + if (!qio_channel_set_blocking(QIO_CHANNEL(sioc), false, &local_err)) {
> + error_report_err(local_err);
> + return -1;
> + }
> +
> s->ioc = QIO_CHANNEL(sioc);
> object_ref(OBJECT(sioc));
> s->sioc = sioc;
> object_ref(OBJECT(sioc));
>
> - qio_channel_set_blocking(s->ioc, false, NULL);
> -
> if (s->do_nodelay) {
> qio_channel_set_delay(s->ioc, false);
> }
> diff --git a/hw/remote/proxy.c b/hw/remote/proxy.c
> index b0165aa2a1..18e0f7a064 100644
> --- a/hw/remote/proxy.c
> +++ b/hw/remote/proxy.c
> @@ -112,8 +112,12 @@ static void pci_proxy_dev_realize(PCIDevice *device,
> Error **errp)
> return;
> }
>
> + if (!qio_channel_set_blocking(dev->ioc, true, errp)) {
> + object_unref(dev->ioc);
> + return;
> + }
> +
> qemu_mutex_init(&dev->io_mutex);
> - qio_channel_set_blocking(dev->ioc, true, NULL);
>
> pci_conf[PCI_LATENCY_TIMER] = 0xff;
> pci_conf[PCI_INTERRUPT_PIN] = 0x01;
> diff --git a/hw/remote/remote-obj.c b/hw/remote/remote-obj.c
> index 85882902d7..3402068ab9 100644
> --- a/hw/remote/remote-obj.c
> +++ b/hw/remote/remote-obj.c
> @@ -107,7 +107,11 @@ static void remote_object_machine_done(Notifier
> *notifier, void *data)
> error_report_err(err);
> return;
> }
> - qio_channel_set_blocking(ioc, false, NULL);
> + if (!qio_channel_set_blocking(ioc, false, &err)) {
> + error_report_err(err);
> + object_unref(OBJECT(ioc));
> + return;
> + }
>
> o->dev = dev;
>
> diff --git a/hw/vfio-user/proxy.c b/hw/vfio-user/proxy.c
> index 2c03d49f97..bbd7ec243d 100644
> --- a/hw/vfio-user/proxy.c
> +++ b/hw/vfio-user/proxy.c
> @@ -886,10 +886,11 @@ VFIOUserProxy *vfio_user_connect_dev(SocketAddress
> *addr, Error **errp)
> sioc = qio_channel_socket_new();
> ioc = QIO_CHANNEL(sioc);
> if (qio_channel_socket_connect_sync(sioc, addr, errp) < 0) {
> - object_unref(OBJECT(ioc));
> - return NULL;
> + goto fail;
> + }
> + if (!qio_channel_set_blocking(ioc, false, errp)) {
> + goto fail;
> }
> - qio_channel_set_blocking(ioc, false, NULL);
>
> proxy = g_malloc0(sizeof(VFIOUserProxy));
> proxy->sockname = g_strdup_printf("unix:%s", sockname);
> @@ -923,6 +924,10 @@ VFIOUserProxy *vfio_user_connect_dev(SocketAddress
> *addr, Error **errp)
> QLIST_INSERT_HEAD(&vfio_user_sockets, proxy, next);
>
> return proxy;
> +
> +fail:
> + object_unref(OBJECT(ioc));
> + return NULL;
> }
>
> void vfio_user_set_handler(VFIODevice *vbasedev,
> diff --git a/include/io/channel.h b/include/io/channel.h
> index c7f64506f7..999a8f5f23 100644
> --- a/include/io/channel.h
> +++ b/include/io/channel.h
> @@ -531,9 +531,9 @@ int coroutine_mixed_fn qio_channel_write_all(QIOChannel
> *ioc,
> * return QIO_CHANNEL_ERR_BLOCK if they would otherwise
> * block on I/O
> */
> -int qio_channel_set_blocking(QIOChannel *ioc,
> - bool enabled,
> - Error **errp);
> +bool qio_channel_set_blocking(QIOChannel *ioc,
> + bool enabled,
> + Error **errp);
>
> /**
> * qio_channel_set_follow_coroutine_ctx:
> diff --git a/io/channel.c b/io/channel.c
> index ebd9322765..852e684938 100644
> --- a/io/channel.c
> +++ b/io/channel.c
> @@ -359,12 +359,12 @@ int coroutine_mixed_fn qio_channel_write_all(QIOChannel
> *ioc,
> }
>
>
> -int qio_channel_set_blocking(QIOChannel *ioc,
> +bool qio_channel_set_blocking(QIOChannel *ioc,
> bool enabled,
> Error **errp)
> {
> QIOChannelClass *klass = QIO_CHANNEL_GET_CLASS(ioc);
> - return klass->io_set_blocking(ioc, enabled, errp);
> + return klass->io_set_blocking(ioc, enabled, errp) == 0;
> }
>
>
> diff --git a/nbd/server.c b/nbd/server.c
> index d242be9811..acec0487a8 100644
> --- a/nbd/server.c
> +++ b/nbd/server.c
> @@ -1411,7 +1411,9 @@ static coroutine_fn int nbd_negotiate(NBDClient
> *client, Error **errp)
> ....options sent, ending in NBD_OPT_EXPORT_NAME or NBD_OPT_GO....
> */
>
> - qio_channel_set_blocking(client->ioc, false, NULL);
> + if (!qio_channel_set_blocking(client->ioc, false, errp)) {
> + return -EINVAL;
> + }
> qio_channel_set_follow_coroutine_ctx(client->ioc, true);
>
> trace_nbd_negotiate_begin();
> diff --git a/scsi/qemu-pr-helper.c b/scsi/qemu-pr-helper.c
> index b69dd982d6..074b4db472 100644
> --- a/scsi/qemu-pr-helper.c
> +++ b/scsi/qemu-pr-helper.c
> @@ -733,8 +733,11 @@ static void coroutine_fn prh_co_entry(void *opaque)
> uint32_t flags;
> int r;
>
> - qio_channel_set_blocking(QIO_CHANNEL(client->ioc),
> - false, NULL);
> + if (!qio_channel_set_blocking(QIO_CHANNEL(client->ioc),
> + false, &local_err)) {
> + goto out;
> + }
> +
> qio_channel_set_follow_coroutine_ctx(QIO_CHANNEL(client->ioc), true);
>
> /* A very simple negotiation for future extensibility. No features
> @@ -786,6 +789,7 @@ static void coroutine_fn prh_co_entry(void *opaque)
> }
> }
>
> +out:
> if (local_err) {
> if (verbose == 0) {
> error_free(local_err);
> @@ -794,7 +798,6 @@ static void coroutine_fn prh_co_entry(void *opaque)
> }
> }
>
> -out:
> object_unref(OBJECT(client->ioc));
> g_free(client);
> }
> diff --git a/tests/unit/io-channel-helpers.c b/tests/unit/io-channel-helpers.c
> index c0799c21c2..22b42d14cd 100644
> --- a/tests/unit/io-channel-helpers.c
> +++ b/tests/unit/io-channel-helpers.c
> @@ -20,6 +20,7 @@
>
> #include "qemu/osdep.h"
> #include "io-channel-helpers.h"
> +#include "qapi/error.h"
> #include "qemu/iov.h"
>
> struct QIOChannelTest {
> @@ -109,8 +110,8 @@ void qio_channel_test_run_threads(QIOChannelTest *test,
> test->src = src;
> test->dst = dst;
>
> - qio_channel_set_blocking(test->dst, blocking, NULL);
> - qio_channel_set_blocking(test->src, blocking, NULL);
> + qio_channel_set_blocking(test->dst, blocking, &error_abort);
> + qio_channel_set_blocking(test->src, blocking, &error_abort);
>
> reader = g_thread_new("reader",
> test_io_thread_reader,
> diff --git a/tests/unit/test-io-channel-tls.c
> b/tests/unit/test-io-channel-tls.c
> index e036ac5df4..6f282ad45d 100644
> --- a/tests/unit/test-io-channel-tls.c
> +++ b/tests/unit/test-io-channel-tls.c
> @@ -184,8 +184,8 @@ static void test_io_channel_tls(const void *opaque)
> * thread, so we need these non-blocking to avoid deadlock
> * of ourselves
> */
> - qio_channel_set_blocking(QIO_CHANNEL(clientChanSock), false, NULL);
> - qio_channel_set_blocking(QIO_CHANNEL(serverChanSock), false, NULL);
> + qio_channel_set_blocking(QIO_CHANNEL(clientChanSock), false,
> &error_abort);
> + qio_channel_set_blocking(QIO_CHANNEL(serverChanSock), false,
> &error_abort);
>
> /* Now the real part of the test, setup the sessions */
> clientChanTLS = qio_channel_tls_new_client(
> diff --git a/tools/i386/qemu-vmsr-helper.c b/tools/i386/qemu-vmsr-helper.c
> index 5f19a48cbd..6c0f4fe870 100644
> --- a/tools/i386/qemu-vmsr-helper.c
> +++ b/tools/i386/qemu-vmsr-helper.c
> @@ -213,8 +213,10 @@ static void coroutine_fn vh_co_entry(void *opaque)
> uint64_t vmsr;
> int r;
>
> - qio_channel_set_blocking(QIO_CHANNEL(client->ioc),
> - false, NULL);
> + if (!qio_channel_set_blocking(QIO_CHANNEL(client->ioc),
> + false, &local_err)) {
> + goto out;
> + }
>
> qio_channel_set_follow_coroutine_ctx(QIO_CHANNEL(client->ioc), true);
>
> diff --git a/ui/vnc.c b/ui/vnc.c
> index 68ca4a68e7..8ca77b2971 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -3337,7 +3337,7 @@ static void vnc_connect(VncDisplay *vd,
> QIOChannelSocket *sioc,
>
> VNC_DEBUG("New client on socket %p\n", vs->sioc);
> update_displaychangelistener(&vd->dcl, VNC_REFRESH_INTERVAL_BASE);
> - qio_channel_set_blocking(vs->ioc, false, NULL);
> + qio_channel_set_blocking(vs->ioc, false, &error_abort);
> if (vs->ioc_tag) {
> g_source_remove(vs->ioc_tag);
> }
> diff --git a/util/vhost-user-server.c b/util/vhost-user-server.c
> index b19229074a..d805a92394 100644
> --- a/util/vhost-user-server.c
> +++ b/util/vhost-user-server.c
> @@ -336,6 +336,7 @@ static void vu_accept(QIONetListener *listener,
> QIOChannelSocket *sioc,
> gpointer opaque)
> {
> VuServer *server = opaque;
> + Error *local_err = NULL;
>
> if (server->sioc) {
> warn_report("Only one vhost-user client is allowed to "
> @@ -368,7 +369,11 @@ static void vu_accept(QIONetListener *listener,
> QIOChannelSocket *sioc,
> object_ref(OBJECT(server->ioc));
>
> /* TODO vu_message_write() spins if non-blocking! */
> - qio_channel_set_blocking(server->ioc, false, NULL);
> + if (!qio_channel_set_blocking(server->ioc, false, &local_err)) {
> + error_report_err(local_err);
> + vu_deinit(&server->vu_dev);
> + return;
> + }
>
> qio_channel_set_follow_coroutine_ctx(server->ioc, true);
>
> --
> 2.48.1
>
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 :|
