Re: [PATCH 26/33] io/channel-socket: introduce qio_channel_socket_keep_nonblock()

["Daniel P. Berrangé" <berrange@redhat.com>]

On Wed, Aug 20, 2025 at 10:37:41AM -0400, Peter Xu wrote:
> On Wed, Aug 20, 2025 at 02:43:54PM +0100, Daniel P. Berrangé wrote:
> > On Wed, Aug 20, 2025 at 09:27:09AM -0400, Peter Xu wrote:
> > > On Wed, Aug 13, 2025 at 07:48:47PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> > > > Add a possibility to keep socket non-block status when passing
> > > > through qio channel. We need this to support migration of open
> > > > fds through migration channel.
> > > > 
> > > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> > > > ---
> > > >  include/io/channel-socket.h |  3 +++
> > > >  io/channel-socket.c         | 16 ++++++++++++----
> > > >  2 files changed, 15 insertions(+), 4 deletions(-)
> > > > 
> > > > diff --git a/include/io/channel-socket.h b/include/io/channel-socket.h
> > > > index a88cf8b3a9..0a4327d745 100644
> > > > --- a/include/io/channel-socket.h
> > > > +++ b/include/io/channel-socket.h
> > > > @@ -49,6 +49,7 @@ struct QIOChannelSocket {
> > > >      socklen_t remoteAddrLen;
> > > >      ssize_t zero_copy_queued;
> > > >      ssize_t zero_copy_sent;
> > > > +    bool keep_nonblock;
> > > >  };
> > > >  
> > > >  
> > > > @@ -275,4 +276,6 @@ int qio_channel_socket_set_send_buffer(QIOChannelSocket *ioc,
> > > >                                         size_t size,
> > > >                                         Error **errp);
> > > >  
> > > > +void qio_channel_socket_keep_nonblock(QIOChannel *ioc);
> > > > +
> > > >  #endif /* QIO_CHANNEL_SOCKET_H */
> > > > diff --git a/io/channel-socket.c b/io/channel-socket.c
> > > > index 3b7ca924ff..cd93d7f180 100644
> > > > --- a/io/channel-socket.c
> > > > +++ b/io/channel-socket.c
> > > > @@ -462,9 +462,16 @@ static void qio_channel_socket_finalize(Object *obj)
> > > >  }
> > > >  
> > > >  
> > > > +void qio_channel_socket_keep_nonblock(QIOChannel *ioc)
> > > > +{
> > > > +    QIOChannelSocket *sioc = QIO_CHANNEL_SOCKET(ioc);
> > > > +    sioc->keep_nonblock = true;
> > > > +}
> > > > +
> > > > +
> > > >  #ifndef WIN32
> > > >  static void qio_channel_socket_copy_fds(struct msghdr *msg,
> > > > -                                        int **fds, size_t *nfds)
> > > > +                                        int **fds, size_t *nfds, bool set_block)
> > > >  {
> > > >      struct cmsghdr *cmsg;
> > > >  
> > > > @@ -497,8 +504,9 @@ 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 (set_block) {
> > > > +                qemu_socket_set_block(fd);
> > > > +            }
> > > 
> > > "keep_nonblock" as a feature in iochannel is slightly hard to digest.  It
> > > can also be read as "keep the fd to be always nonblocking".
> > > 
> > > Is this feature required, or can this also be done in a get() or
> > > post_load() on the other side to set nonblock to whatever it should be
> > > (that dest QEMU should be aware of)?
> > 
> > Either we preserve state of the flag when receiving the FD,
> > or every QEMU backend that we're receiving FDs on behalf of
> > needs to reset the flag when migration passes over the FD.
> > 
> > The latter might actually be a more robust scheme. If we're
> > migrating across QEMU versions, there is not a strict
> > guarantee that the new QEMU version's backend will want the
> > O_NONBLOCK flag in the same state as the old QEMU version.
> > The code might have been re-written to work in a different
> > way than previously.
> 
> Good point.
> 
> Do you remember why we reset that in the very initial git commit?

Historical needs to receive FDs in QEMU have been in relation to external
non-QEMU processes. We reset the blocking state to ensure that all FDs
we receive have a well defined initial state, and the QEMU backends
consuming the FDs can then alter that as required.

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