1. Patchew
  2. QEMU
  3. View series

[PATCH v3] migration/qemu-file: don't make incoming fds blocking again

Vladimir Sementsov-Ogievskiy posted 1 patch 2 weeks, 4 days ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20250910143156.1053779-1-vsementsov@yandex-team.ru
Maintainers: "Daniel P. Berrangé" <berrange@redhat.com>, Peter Xu <peterx@redhat.com>, Fabiano Rosas <farosas@suse.de>
There is a newer version of this series
include/io/channel.h  |  1 +
io/channel-socket.c   | 12 ++++++++----
io/channel.c          |  2 +-
migration/qemu-file.c |  3 ++-
4 files changed, 12 insertions(+), 6 deletions(-)
[PATCH v3] migration/qemu-file: don't make incoming fds blocking again
Posted by Vladimir Sementsov-Ogievskiy 2 weeks, 4 days ago 
In migration we want to pass fd "as is", not changing its
blocking status.

The only current user of these fds is CPR state (through VMSTATE_FD),
which of-course doesn't want to modify fds on target when source is
still running and use these fds.

Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---

v3: RESEND, add qemu-devel to CC, sorry for the noise
v2: rework, following Daniel's suggestion to use flag.

 include/io/channel.h  |  1 +
 io/channel-socket.c   | 12 ++++++++----
 io/channel.c          |  2 +-
 migration/qemu-file.c |  3 ++-
 4 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/include/io/channel.h b/include/io/channel.h
index 234e5db70d..5394f50768 100644
--- a/include/io/channel.h
+++ b/include/io/channel.h
@@ -36,6 +36,7 @@ OBJECT_DECLARE_TYPE(QIOChannel, QIOChannelClass,
 
 #define QIO_CHANNEL_READ_FLAG_MSG_PEEK 0x1
 #define QIO_CHANNEL_READ_FLAG_RELAXED_EOF 0x2
+#define QIO_CHANNEL_READ_FLAG_PRESERVE_BLOCKING 0x4
 
 typedef enum QIOChannelFeature QIOChannelFeature;
 
diff --git a/io/channel-socket.c b/io/channel-socket.c
index 3b7ca924ff..2f6e2d84a3 100644
--- a/io/channel-socket.c
+++ b/io/channel-socket.c
@@ -464,7 +464,8 @@ static void qio_channel_socket_finalize(Object *obj)
 
 #ifndef WIN32
 static void qio_channel_socket_copy_fds(struct msghdr *msg,
-                                        int **fds, size_t *nfds)
+                                        int **fds, size_t *nfds,
+                                        bool preserve_blocking)
 {
     struct cmsghdr *cmsg;
 
@@ -497,8 +498,10 @@ static void qio_channel_socket_copy_fds(struct msghdr *msg,
                 continue;
             }
 
-            /* O_NONBLOCK is preserved across SCM_RIGHTS so reset it */
-            qemu_socket_set_block(fd);
+            if (!preserve_blocking) {
+                /* O_NONBLOCK is preserved across SCM_RIGHTS so reset it */
+                qemu_socket_set_block(fd);
+            }
 
 #ifndef MSG_CMSG_CLOEXEC
             qemu_set_cloexec(fd);
@@ -556,7 +559,8 @@ static ssize_t qio_channel_socket_readv(QIOChannel *ioc,
     }
 
     if (fds && nfds) {
-        qio_channel_socket_copy_fds(&msg, fds, nfds);
+        qio_channel_socket_copy_fds(
+            &msg, fds, nfds, flags & QIO_CHANNEL_READ_FLAG_PRESERVE_BLOCKING);
     }
 
     return ret;
diff --git a/io/channel.c b/io/channel.c
index ebd9322765..50d5f7b10b 100644
--- a/io/channel.c
+++ b/io/channel.c
@@ -58,7 +58,7 @@ ssize_t qio_channel_readv_full(QIOChannel *ioc,
 {
     QIOChannelClass *klass = QIO_CHANNEL_GET_CLASS(ioc);
 
-    if ((fds || nfds) &&
+    if ((fds || nfds || (flags & QIO_CHANNEL_READ_FLAG_PRESERVE_BLOCKING)) &&
         !qio_channel_has_feature(ioc, QIO_CHANNEL_FEATURE_FD_PASS)) {
         error_setg_errno(errp, EINVAL,
                          "Channel does not support file descriptor passing");
diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index b6ac190034..92c7b5678b 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -324,6 +324,7 @@ static ssize_t coroutine_mixed_fn qemu_fill_buffer(QEMUFile *f)
     size_t nfd = 0;
     int **pfds = f->can_pass_fd ? &fds : NULL;
     size_t *pnfd = f->can_pass_fd ? &nfd : NULL;
+    int flags = f->can_pass_fd ? QIO_CHANNEL_READ_FLAG_PRESERVE_BLOCKING : 0;
 
     assert(!qemu_file_is_writable(f));
 
@@ -340,7 +341,7 @@ static ssize_t coroutine_mixed_fn qemu_fill_buffer(QEMUFile *f)
 
     do {
         struct iovec iov = { f->buf + pending, IO_BUF_SIZE - pending };
-        len = qio_channel_readv_full(f->ioc, &iov, 1, pfds, pnfd, 0,
+        len = qio_channel_readv_full(f->ioc, &iov, 1, pfds, pnfd, flags,
                                      &local_error);
         if (len == QIO_CHANNEL_ERR_BLOCK) {
             if (qemu_in_coroutine()) {
-- 
2.48.1
Re: [PATCH v3] migration/qemu-file: don't make incoming fds blocking again
Posted by Peter Xu 2 weeks, 3 days ago 
On Wed, Sep 10, 2025 at 05:31:56PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> In migration we want to pass fd "as is", not changing its
> blocking status.
> 
> The only current user of these fds is CPR state (through VMSTATE_FD),
> which of-course doesn't want to modify fds on target when source is
> still running and use these fds.
> 
> Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> ---
> 
> v3: RESEND, add qemu-devel to CC, sorry for the noise
> v2: rework, following Daniel's suggestion to use flag.
> 
>  include/io/channel.h  |  1 +
>  io/channel-socket.c   | 12 ++++++++----
>  io/channel.c          |  2 +-
>  migration/qemu-file.c |  3 ++-
>  4 files changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/include/io/channel.h b/include/io/channel.h
> index 234e5db70d..5394f50768 100644
> --- a/include/io/channel.h
> +++ b/include/io/channel.h
> @@ -36,6 +36,7 @@ OBJECT_DECLARE_TYPE(QIOChannel, QIOChannelClass,
>  
>  #define QIO_CHANNEL_READ_FLAG_MSG_PEEK 0x1
>  #define QIO_CHANNEL_READ_FLAG_RELAXED_EOF 0x2
> +#define QIO_CHANNEL_READ_FLAG_PRESERVE_BLOCKING 0x4

Shouldn't the name reflect FD somehow?  Or it can imply preservation
blocking for the channel itself.

  QIO_CHANNEL_READ_FLAG_FD_PRESERVE_BLOCKING 
  QIO_CHANNEL_READ_FLAG_PRESERVE_FD_BLOCKING 
  ...

>  
>  typedef enum QIOChannelFeature QIOChannelFeature;
>  
> diff --git a/io/channel-socket.c b/io/channel-socket.c
> index 3b7ca924ff..2f6e2d84a3 100644
> --- a/io/channel-socket.c
> +++ b/io/channel-socket.c
> @@ -464,7 +464,8 @@ static void qio_channel_socket_finalize(Object *obj)
>  
>  #ifndef WIN32
>  static void qio_channel_socket_copy_fds(struct msghdr *msg,
> -                                        int **fds, size_t *nfds)
> +                                        int **fds, size_t *nfds,
> +                                        bool preserve_blocking)
>  {
>      struct cmsghdr *cmsg;
>  
> @@ -497,8 +498,10 @@ static void qio_channel_socket_copy_fds(struct msghdr *msg,
>                  continue;
>              }
>  
> -            /* O_NONBLOCK is preserved across SCM_RIGHTS so reset it */
> -            qemu_socket_set_block(fd);
> +            if (!preserve_blocking) {
> +                /* O_NONBLOCK is preserved across SCM_RIGHTS so reset it */
> +                qemu_socket_set_block(fd);
> +            }
>  
>  #ifndef MSG_CMSG_CLOEXEC
>              qemu_set_cloexec(fd);
> @@ -556,7 +559,8 @@ static ssize_t qio_channel_socket_readv(QIOChannel *ioc,
>      }
>  
>      if (fds && nfds) {
> -        qio_channel_socket_copy_fds(&msg, fds, nfds);
> +        qio_channel_socket_copy_fds(
> +            &msg, fds, nfds, flags & QIO_CHANNEL_READ_FLAG_PRESERVE_BLOCKING);
>      }
>  
>      return ret;
> diff --git a/io/channel.c b/io/channel.c
> index ebd9322765..50d5f7b10b 100644
> --- a/io/channel.c
> +++ b/io/channel.c
> @@ -58,7 +58,7 @@ ssize_t qio_channel_readv_full(QIOChannel *ioc,
>  {
>      QIOChannelClass *klass = QIO_CHANNEL_GET_CLASS(ioc);
>  
> -    if ((fds || nfds) &&
> +    if ((fds || nfds || (flags & QIO_CHANNEL_READ_FLAG_PRESERVE_BLOCKING)) &&

Not a huge deal, but.. IMHO we can simply ignore this flag when fds==NULL.

It can also make callers' lives slightly easier too by always passing in
this flag when necessary, like in below.

>          !qio_channel_has_feature(ioc, QIO_CHANNEL_FEATURE_FD_PASS)) {
>          error_setg_errno(errp, EINVAL,
>                           "Channel does not support file descriptor passing");
> diff --git a/migration/qemu-file.c b/migration/qemu-file.c
> index b6ac190034..92c7b5678b 100644
> --- a/migration/qemu-file.c
> +++ b/migration/qemu-file.c
> @@ -324,6 +324,7 @@ static ssize_t coroutine_mixed_fn qemu_fill_buffer(QEMUFile *f)
>      size_t nfd = 0;
>      int **pfds = f->can_pass_fd ? &fds : NULL;
>      size_t *pnfd = f->can_pass_fd ? &nfd : NULL;
> +    int flags = f->can_pass_fd ? QIO_CHANNEL_READ_FLAG_PRESERVE_BLOCKING : 0;
>  
>      assert(!qemu_file_is_writable(f));
>  
> @@ -340,7 +341,7 @@ static ssize_t coroutine_mixed_fn qemu_fill_buffer(QEMUFile *f)
>  
>      do {
>          struct iovec iov = { f->buf + pending, IO_BUF_SIZE - pending };
> -        len = qio_channel_readv_full(f->ioc, &iov, 1, pfds, pnfd, 0,
> +        len = qio_channel_readv_full(f->ioc, &iov, 1, pfds, pnfd, flags,
>                                       &local_error);
>          if (len == QIO_CHANNEL_ERR_BLOCK) {
>              if (qemu_in_coroutine()) {
> -- 
> 2.48.1
> 

-- 
Peter Xu
Re: [PATCH v3] migration/qemu-file: don't make incoming fds blocking again
Posted by Vladimir Sementsov-Ogievskiy 2 weeks, 3 days ago 
On 10.09.25 19:47, Peter Xu wrote:
> On Wed, Sep 10, 2025 at 05:31:56PM +0300, Vladimir Sementsov-Ogievskiy wrote:
>> In migration we want to pass fd "as is", not changing its
>> blocking status.
>>
>> The only current user of these fds is CPR state (through VMSTATE_FD),
>> which of-course doesn't want to modify fds on target when source is
>> still running and use these fds.
>>
>> Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
>> ---
>>
>> v3: RESEND, add qemu-devel to CC, sorry for the noise
>> v2: rework, following Daniel's suggestion to use flag.
>>
>>   include/io/channel.h  |  1 +
>>   io/channel-socket.c   | 12 ++++++++----
>>   io/channel.c          |  2 +-
>>   migration/qemu-file.c |  3 ++-
>>   4 files changed, 12 insertions(+), 6 deletions(-)
>>
>> diff --git a/include/io/channel.h b/include/io/channel.h
>> index 234e5db70d..5394f50768 100644
>> --- a/include/io/channel.h
>> +++ b/include/io/channel.h
>> @@ -36,6 +36,7 @@ OBJECT_DECLARE_TYPE(QIOChannel, QIOChannelClass,
>>   
>>   #define QIO_CHANNEL_READ_FLAG_MSG_PEEK 0x1
>>   #define QIO_CHANNEL_READ_FLAG_RELAXED_EOF 0x2
>> +#define QIO_CHANNEL_READ_FLAG_PRESERVE_BLOCKING 0x4
> 
> Shouldn't the name reflect FD somehow?  Or it can imply preservation
> blocking for the channel itself.
> 
>    QIO_CHANNEL_READ_FLAG_FD_PRESERVE_BLOCKING
>    QIO_CHANNEL_READ_FLAG_PRESERVE_FD_BLOCKING
>    ...
> 
>>   
>>   typedef enum QIOChannelFeature QIOChannelFeature;
>>   
>> diff --git a/io/channel-socket.c b/io/channel-socket.c
>> index 3b7ca924ff..2f6e2d84a3 100644
>> --- a/io/channel-socket.c
>> +++ b/io/channel-socket.c
>> @@ -464,7 +464,8 @@ static void qio_channel_socket_finalize(Object *obj)
>>   
>>   #ifndef WIN32
>>   static void qio_channel_socket_copy_fds(struct msghdr *msg,
>> -                                        int **fds, size_t *nfds)
>> +                                        int **fds, size_t *nfds,
>> +                                        bool preserve_blocking)
>>   {
>>       struct cmsghdr *cmsg;
>>   
>> @@ -497,8 +498,10 @@ static void qio_channel_socket_copy_fds(struct msghdr *msg,
>>                   continue;
>>               }
>>   
>> -            /* O_NONBLOCK is preserved across SCM_RIGHTS so reset it */
>> -            qemu_socket_set_block(fd);
>> +            if (!preserve_blocking) {
>> +                /* O_NONBLOCK is preserved across SCM_RIGHTS so reset it */
>> +                qemu_socket_set_block(fd);
>> +            }
>>   
>>   #ifndef MSG_CMSG_CLOEXEC
>>               qemu_set_cloexec(fd);
>> @@ -556,7 +559,8 @@ static ssize_t qio_channel_socket_readv(QIOChannel *ioc,
>>       }
>>   
>>       if (fds && nfds) {
>> -        qio_channel_socket_copy_fds(&msg, fds, nfds);
>> +        qio_channel_socket_copy_fds(
>> +            &msg, fds, nfds, flags & QIO_CHANNEL_READ_FLAG_PRESERVE_BLOCKING);
>>       }
>>   
>>       return ret;
>> diff --git a/io/channel.c b/io/channel.c
>> index ebd9322765..50d5f7b10b 100644
>> --- a/io/channel.c
>> +++ b/io/channel.c
>> @@ -58,7 +58,7 @@ ssize_t qio_channel_readv_full(QIOChannel *ioc,
>>   {
>>       QIOChannelClass *klass = QIO_CHANNEL_GET_CLASS(ioc);
>>   
>> -    if ((fds || nfds) &&
>> +    if ((fds || nfds || (flags & QIO_CHANNEL_READ_FLAG_PRESERVE_BLOCKING)) &&
> 
> Not a huge deal, but.. IMHO we can simply ignore this flag when fds==NULL.
> 
> It can also make callers' lives slightly easier too by always passing in
> this flag when necessary, like in below.
> 
>>           !qio_channel_has_feature(ioc, QIO_CHANNEL_FEATURE_FD_PASS)) {
>>           error_setg_errno(errp, EINVAL,
>>                            "Channel does not support file descriptor passing");
>> diff --git a/migration/qemu-file.c b/migration/qemu-file.c
>> index b6ac190034..92c7b5678b 100644
>> --- a/migration/qemu-file.c
>> +++ b/migration/qemu-file.c
>> @@ -324,6 +324,7 @@ static ssize_t coroutine_mixed_fn qemu_fill_buffer(QEMUFile *f)
>>       size_t nfd = 0;
>>       int **pfds = f->can_pass_fd ? &fds : NULL;
>>       size_t *pnfd = f->can_pass_fd ? &nfd : NULL;
>> +    int flags = f->can_pass_fd ? QIO_CHANNEL_READ_FLAG_PRESERVE_BLOCKING : 0;
>>   
>>       assert(!qemu_file_is_writable(f));
>>   
>> @@ -340,7 +341,7 @@ static ssize_t coroutine_mixed_fn qemu_fill_buffer(QEMUFile *f)
>>   
>>       do {
>>           struct iovec iov = { f->buf + pending, IO_BUF_SIZE - pending };
>> -        len = qio_channel_readv_full(f->ioc, &iov, 1, pfds, pnfd, 0,
>> +        len = qio_channel_readv_full(f->ioc, &iov, 1, pfds, pnfd, flags,
>>                                        &local_error);
>>           if (len == QIO_CHANNEL_ERR_BLOCK) {
>>               if (qemu_in_coroutine()) {
>> -- 
>> 2.48.1
>>
> 

I'm OK with both changes, can resend. Daniel, are you?

-- 
Best regards,
Vladimir
Re: [PATCH v3] migration/qemu-file: don't make incoming fds blocking again
Posted by Daniel P. Berrangé 2 weeks, 4 days ago 
On Wed, Sep 10, 2025 at 05:31:56PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> In migration we want to pass fd "as is", not changing its
> blocking status.
> 
> The only current user of these fds is CPR state (through VMSTATE_FD),
> which of-course doesn't want to modify fds on target when source is
> still running and use these fds.
> 
> Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>

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 :|

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.