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é <[email protected]>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <[email protected]>
> ---
>
> 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
