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

[Peter Xu <peterx@redhat.com>]

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