[PATCH 08/10] oslib-posix: add qemu_fds_set_blocking() helper

Vladimir Sementsov-Ogievskiy posted 10 patches 5 months, 1 week ago
Maintainers: "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>, 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>, Stefan Berger <stefanb@linux.vnet.ibm.com>, Stefan Weil <sw@weilnetz.de>, Coiby Xu <Coiby.Xu@gmail.com>
There is a newer version of this series
[PATCH 08/10] oslib-posix: add qemu_fds_set_blocking() helper
Posted by Vladimir Sementsov-Ogievskiy 5 months, 1 week ago
And use it in io/channel-socket.c. This simplifies the following
commit, which will move this functionality from io/channel-socket.c
to the callers.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 include/qemu/osdep.h |  7 +++++++
 io/channel-socket.c  | 24 +++++++++++++-----------
 util/oslib-posix.c   | 12 ++++++++++++
 3 files changed, 32 insertions(+), 11 deletions(-)

diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index 1b38cb7e45..dde98d588c 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -689,6 +689,13 @@ ssize_t qemu_write_full(int fd, const void *buf, size_t count)
 void qemu_set_cloexec(int fd);
 bool qemu_set_blocking(int fd, bool block, Error **errp);
 
+/*
+ * qemu_fds_set_blockinging:
+ * Call qemu_socket_set_block() on several fds.
+ * When @nfds = 0, does nothing, @fds is not touched.
+ */
+bool qemu_fds_set_blockinging(int *fds, int nfds, bool block, Error **errp);
+
 /* Return a dynamically allocated directory path that is appropriate for storing
  * local state.
  *
diff --git a/io/channel-socket.c b/io/channel-socket.c
index 4f7e86f72f..96098b5bcc 100644
--- a/io/channel-socket.c
+++ b/io/channel-socket.c
@@ -472,8 +472,11 @@ static void qio_channel_socket_copy_fds(struct msghdr *msg,
     *fds = NULL;
 
     for (cmsg = CMSG_FIRSTHDR(msg); cmsg; cmsg = CMSG_NXTHDR(msg, cmsg)) {
-        int fd_size, i;
+        int fd_size;
         int gotfds;
+#ifndef MSG_CMSG_CLOEXEC
+        int i;
+#endif
 
         if (cmsg->cmsg_len < CMSG_LEN(sizeof(int)) ||
             cmsg->cmsg_level != SOL_SOCKET ||
@@ -491,20 +494,19 @@ static void qio_channel_socket_copy_fds(struct msghdr *msg,
         *fds = g_renew(int, *fds, *nfds + gotfds);
         memcpy(*fds + *nfds, CMSG_DATA(cmsg), fd_size);
 
+        /* O_NONBLOCK is preserved across SCM_RIGHTS so reset it */
+        /* TODO: don't crash on error, just handle it! */
+        qemu_fds_set_blockinging(*fds + *nfds, gotfds, true, &error_abort);
+
+#ifndef MSG_CMSG_CLOEXEC
         for (i = 0; i < gotfds; i++) {
             int fd = (*fds)[*nfds + i];
-            if (fd < 0) {
-                continue;
+            if (fd >= 0) {
+                qemu_set_cloexec(fd);
             }
-
-            /* O_NONBLOCK is preserved across SCM_RIGHTS so reset it */
-            /* TODO: don't crash on error, just handle it! */
-            qemu_set_blocking(fd, true, &error_abort);
-
-#ifndef MSG_CMSG_CLOEXEC
-            qemu_set_cloexec(fd);
-#endif
         }
+#endif
+
         *nfds += gotfds;
     }
 }
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index 8891d82db0..8589ff21ec 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -262,6 +262,18 @@ bool qemu_set_blocking(int fd, bool block, Error **errp)
     return true;
 }
 
+bool qemu_fds_set_blockinging(int *fds, int nfds, bool block, Error **errp)
+{
+    int i;
+    for (i = 0; i < nfds; i++) {
+        if (fds[i] >= 0 && !qemu_set_blocking(fds[i], block, errp)) {
+            return false;
+        }
+    }
+
+    return true;
+}
+
 int socket_set_fast_reuse(int fd)
 {
     int val = 1, ret;
-- 
2.48.1
Re: [PATCH 08/10] oslib-posix: add qemu_fds_set_blocking() helper
Posted by Daniel P. Berrangé 5 months ago
On Wed, Sep 03, 2025 at 12:44:08PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> And use it in io/channel-socket.c. This simplifies the following
> commit, which will move this functionality from io/channel-socket.c
> to the callers.

Looking at how many callers will still want the current
functionality, I'm inclined to add a flag for QIOChannel

  QIO_CHANNEL_READ_FLAG_PRESERVE_BLOCKING

which migration can pass to qio_channel_readv_full and
thus avoid adding this method and duplicated calls to
it in (almost) every caller.

> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> ---
>  include/qemu/osdep.h |  7 +++++++
>  io/channel-socket.c  | 24 +++++++++++++-----------
>  util/oslib-posix.c   | 12 ++++++++++++
>  3 files changed, 32 insertions(+), 11 deletions(-)
> 
> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> index 1b38cb7e45..dde98d588c 100644
> --- a/include/qemu/osdep.h
> +++ b/include/qemu/osdep.h
> @@ -689,6 +689,13 @@ ssize_t qemu_write_full(int fd, const void *buf, size_t count)
>  void qemu_set_cloexec(int fd);
>  bool qemu_set_blocking(int fd, bool block, Error **errp);
>  
> +/*
> + * qemu_fds_set_blockinging:
> + * Call qemu_socket_set_block() on several fds.
> + * When @nfds = 0, does nothing, @fds is not touched.
> + */
> +bool qemu_fds_set_blockinging(int *fds, int nfds, bool block, Error **errp);
> +
>  /* Return a dynamically allocated directory path that is appropriate for storing
>   * local state.
>   *
> diff --git a/io/channel-socket.c b/io/channel-socket.c
> index 4f7e86f72f..96098b5bcc 100644
> --- a/io/channel-socket.c
> +++ b/io/channel-socket.c
> @@ -472,8 +472,11 @@ static void qio_channel_socket_copy_fds(struct msghdr *msg,
>      *fds = NULL;
>  
>      for (cmsg = CMSG_FIRSTHDR(msg); cmsg; cmsg = CMSG_NXTHDR(msg, cmsg)) {
> -        int fd_size, i;
> +        int fd_size;
>          int gotfds;
> +#ifndef MSG_CMSG_CLOEXEC
> +        int i;
> +#endif
>  
>          if (cmsg->cmsg_len < CMSG_LEN(sizeof(int)) ||
>              cmsg->cmsg_level != SOL_SOCKET ||
> @@ -491,20 +494,19 @@ static void qio_channel_socket_copy_fds(struct msghdr *msg,
>          *fds = g_renew(int, *fds, *nfds + gotfds);
>          memcpy(*fds + *nfds, CMSG_DATA(cmsg), fd_size);
>  
> +        /* O_NONBLOCK is preserved across SCM_RIGHTS so reset it */
> +        /* TODO: don't crash on error, just handle it! */
> +        qemu_fds_set_blockinging(*fds + *nfds, gotfds, true, &error_abort);
> +
> +#ifndef MSG_CMSG_CLOEXEC
>          for (i = 0; i < gotfds; i++) {
>              int fd = (*fds)[*nfds + i];
> -            if (fd < 0) {
> -                continue;
> +            if (fd >= 0) {
> +                qemu_set_cloexec(fd);
>              }
> -
> -            /* O_NONBLOCK is preserved across SCM_RIGHTS so reset it */
> -            /* TODO: don't crash on error, just handle it! */
> -            qemu_set_blocking(fd, true, &error_abort);
> -
> -#ifndef MSG_CMSG_CLOEXEC
> -            qemu_set_cloexec(fd);
> -#endif
>          }
> +#endif
> +
>          *nfds += gotfds;
>      }
>  }
> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
> index 8891d82db0..8589ff21ec 100644
> --- a/util/oslib-posix.c
> +++ b/util/oslib-posix.c
> @@ -262,6 +262,18 @@ bool qemu_set_blocking(int fd, bool block, Error **errp)
>      return true;
>  }
>  
> +bool qemu_fds_set_blockinging(int *fds, int nfds, bool block, Error **errp)
> +{
> +    int i;
> +    for (i = 0; i < nfds; i++) {
> +        if (fds[i] >= 0 && !qemu_set_blocking(fds[i], block, errp)) {
> +            return false;
> +        }
> +    }
> +
> +    return true;
> +}
> +
>  int socket_set_fast_reuse(int fd)
>  {
>      int val = 1, ret;
> -- 
> 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 :|
Re: [PATCH 08/10] oslib-posix: add qemu_fds_set_blocking() helper
Posted by Vladimir Sementsov-Ogievskiy 5 months ago
On 10.09.25 12:52, Daniel P. Berrangé wrote:
> On Wed, Sep 03, 2025 at 12:44:08PM +0300, Vladimir Sementsov-Ogievskiy wrote:
>> And use it in io/channel-socket.c. This simplifies the following
>> commit, which will move this functionality from io/channel-socket.c
>> to the callers.
> 
> Looking at how many callers will still want the current
> functionality, I'm inclined to add a flag for QIOChannel
> 
>    QIO_CHANNEL_READ_FLAG_PRESERVE_BLOCKING
> 
> which migration can pass to qio_channel_readv_full and
> thus avoid adding this method and duplicated calls to
> it in (almost) every caller.
> 

Yes, I thought about this too, when preparing the series.

I'll make a patch, and make the fds-set-blocking refactoring series to be the separate one.

>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
>> ---
>>   include/qemu/osdep.h |  7 +++++++
>>   io/channel-socket.c  | 24 +++++++++++++-----------
>>   util/oslib-posix.c   | 12 ++++++++++++
>>   3 files changed, 32 insertions(+), 11 deletions(-)
>>
>> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
>> index 1b38cb7e45..dde98d588c 100644
>> --- a/include/qemu/osdep.h
>> +++ b/include/qemu/osdep.h
>> @@ -689,6 +689,13 @@ ssize_t qemu_write_full(int fd, const void *buf, size_t count)
>>   void qemu_set_cloexec(int fd);
>>   bool qemu_set_blocking(int fd, bool block, Error **errp);
>>   
>> +/*
>> + * qemu_fds_set_blockinging:
>> + * Call qemu_socket_set_block() on several fds.
>> + * When @nfds = 0, does nothing, @fds is not touched.
>> + */
>> +bool qemu_fds_set_blockinging(int *fds, int nfds, bool block, Error **errp);
>> +
>>   /* Return a dynamically allocated directory path that is appropriate for storing
>>    * local state.
>>    *
>> diff --git a/io/channel-socket.c b/io/channel-socket.c
>> index 4f7e86f72f..96098b5bcc 100644
>> --- a/io/channel-socket.c
>> +++ b/io/channel-socket.c
>> @@ -472,8 +472,11 @@ static void qio_channel_socket_copy_fds(struct msghdr *msg,
>>       *fds = NULL;
>>   
>>       for (cmsg = CMSG_FIRSTHDR(msg); cmsg; cmsg = CMSG_NXTHDR(msg, cmsg)) {
>> -        int fd_size, i;
>> +        int fd_size;
>>           int gotfds;
>> +#ifndef MSG_CMSG_CLOEXEC
>> +        int i;
>> +#endif
>>   
>>           if (cmsg->cmsg_len < CMSG_LEN(sizeof(int)) ||
>>               cmsg->cmsg_level != SOL_SOCKET ||
>> @@ -491,20 +494,19 @@ static void qio_channel_socket_copy_fds(struct msghdr *msg,
>>           *fds = g_renew(int, *fds, *nfds + gotfds);
>>           memcpy(*fds + *nfds, CMSG_DATA(cmsg), fd_size);
>>   
>> +        /* O_NONBLOCK is preserved across SCM_RIGHTS so reset it */
>> +        /* TODO: don't crash on error, just handle it! */
>> +        qemu_fds_set_blockinging(*fds + *nfds, gotfds, true, &error_abort);
>> +
>> +#ifndef MSG_CMSG_CLOEXEC
>>           for (i = 0; i < gotfds; i++) {
>>               int fd = (*fds)[*nfds + i];
>> -            if (fd < 0) {
>> -                continue;
>> +            if (fd >= 0) {
>> +                qemu_set_cloexec(fd);
>>               }
>> -
>> -            /* O_NONBLOCK is preserved across SCM_RIGHTS so reset it */
>> -            /* TODO: don't crash on error, just handle it! */
>> -            qemu_set_blocking(fd, true, &error_abort);
>> -
>> -#ifndef MSG_CMSG_CLOEXEC
>> -            qemu_set_cloexec(fd);
>> -#endif
>>           }
>> +#endif
>> +
>>           *nfds += gotfds;
>>       }
>>   }
>> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
>> index 8891d82db0..8589ff21ec 100644
>> --- a/util/oslib-posix.c
>> +++ b/util/oslib-posix.c
>> @@ -262,6 +262,18 @@ bool qemu_set_blocking(int fd, bool block, Error **errp)
>>       return true;
>>   }
>>   
>> +bool qemu_fds_set_blockinging(int *fds, int nfds, bool block, Error **errp)
>> +{
>> +    int i;
>> +    for (i = 0; i < nfds; i++) {
>> +        if (fds[i] >= 0 && !qemu_set_blocking(fds[i], block, errp)) {
>> +            return false;
>> +        }
>> +    }
>> +
>> +    return true;
>> +}
>> +
>>   int socket_set_fast_reuse(int fd)
>>   {
>>       int val = 1, ret;
>> -- 
>> 2.48.1
>>
> 
> With regards,
> Daniel


-- 
Best regards,
Vladimir

Re: [PATCH 08/10] oslib-posix: add qemu_fds_set_blocking() helper
Posted by Vladimir Sementsov-Ogievskiy 5 months, 1 week ago
On 03.09.25 12:44, Vladimir Sementsov-Ogievskiy wrote:
> And use it in io/channel-socket.c. This simplifies the following
> commit, which will move this functionality from io/channel-socket.c
> to the callers.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> ---
>   include/qemu/osdep.h |  7 +++++++
>   io/channel-socket.c  | 24 +++++++++++++-----------
>   util/oslib-posix.c   | 12 ++++++++++++
>   3 files changed, 32 insertions(+), 11 deletions(-)
> 
> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> index 1b38cb7e45..dde98d588c 100644
> --- a/include/qemu/osdep.h
> +++ b/include/qemu/osdep.h
> @@ -689,6 +689,13 @@ ssize_t qemu_write_full(int fd, const void *buf, size_t count)
>   void qemu_set_cloexec(int fd);
>   bool qemu_set_blocking(int fd, bool block, Error **errp);
>   
> +/*
> + * qemu_fds_set_blockinging:
> + * Call qemu_socket_set_block() on several fds.
> + * When @nfds = 0, does nothing, @fds is not touched.
> + */
> +bool qemu_fds_set_blockinging(int *fds, int nfds, bool block, Error **errp);

Oops. s/inging/ing/ is needed.



-- 
Best regards,
Vladimir