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);
+ }
#ifndef MSG_CMSG_CLOEXEC
qemu_set_cloexec(fd);
@@ -556,7 +564,7 @@ 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, !sioc->keep_nonblock);
}
return ret;
--
2.48.1On 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);
> + }
>
> #ifndef MSG_CMSG_CLOEXEC
> qemu_set_cloexec(fd);
> @@ -556,7 +564,7 @@ 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, !sioc->keep_nonblock);
> }
>
> return ret;
If this is needed, then it should be done by defining another flag
constant to be passed to qio_channel_read*, not via a new API
QIO_CHANNEL_READ_FLAG_FD_PRESERVE_NONBLOCKING
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 :|
On 20.08.25 16:37, Daniel P. Berrangé 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);
>> + }
>>
>> #ifndef MSG_CMSG_CLOEXEC
>> qemu_set_cloexec(fd);
>> @@ -556,7 +564,7 @@ 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, !sioc->keep_nonblock);
>> }
>>
>> return ret;
>
> If this is needed, then it should be done by defining another flag
> constant to be passed to qio_channel_read*, not via a new API
>
> QIO_CHANNEL_READ_FLAG_FD_PRESERVE_NONBLOCKING
>
Ok, thanks, will do.
--
Best regards,
Vladimir
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)?
>
> #ifndef MSG_CMSG_CLOEXEC
> qemu_set_cloexec(fd);
> @@ -556,7 +564,7 @@ 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, !sioc->keep_nonblock);
> }
>
> return ret;
> --
> 2.48.1
>
>
--
Peter Xu
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.
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 :|
On 20.08.25 16:43, 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.
>
What I dislike in the way, when we reset to blocking always, and set non-blocking again where needed:
1. Extra fcntl calls for nothing (I think actually, in most cases, for fds passed through migration stream(s) we'll want to keep fd as is)
2. When we reset to blocking on target, it's visible on source and may break things.
In these series it's probably doesn't really matter, as at the time when we get the descriptor on target, it should not be used anymore on source.
But for example, in CPR-transfer, where descriptors are passed in the preliminary stage, and source is running and use the descriptors, we shouldn't change the non-blocking status of fd on target. Probably, CPR-transfer for now only works with fds which are blpcking, so we don't have a problem.
So, I think, that better default is preserve state of the flag for fds passed through migration stream. And backends may modify it if needed (I think, in most cases - they will not need).
--
Best regards,
Vladimir
On Thu, Aug 21, 2025 at 03:07:57PM +0300, Vladimir Sementsov-Ogievskiy wrote: > What I dislike in the way, when we reset to blocking always, and set > non-blocking again where needed: > > 1. Extra fcntl calls for nothing (I think actually, in most cases, for > fds passed through migration stream(s) we'll want to keep fd as is) > > 2. When we reset to blocking on target, it's visible on source and may > break things. > > In these series it's probably doesn't really matter, as at the time when > we get the descriptor on target, it should not be used anymore on source. > > But for example, in CPR-transfer, where descriptors are passed in the > preliminary stage, and source is running and use the descriptors, we > shouldn't change the non-blocking status of fd on target. Probably, > CPR-transfer for now only works with fds which are blpcking, so we don't > have a problem. > > So, I think, that better default is preserve state of the flag for fds > passed through migration stream. And backends may modify it if needed (I > think, in most cases - they will not need). I agree having that as a default iochannel behavior is questionable. If it was defined for any fd-passing protocols making sure nonblocking status is predictable (in this case, fds will be always blocking), IMHO it should be done in the protocol layer either constantly setting or clearing NONBLOCK flag before sending the fds, rather than having it silently processed in iochannel internals. Do we know how many existing users are relying such behavior? I wonder if we could still push this operation to the protocols that will need it, then we can avoid this slightly awkward iochannel feature flag, because the iochannel user will simply always have full control. -- Peter Xu
On Thu, Aug 21, 2025 at 09:45:36AM -0400, Peter Xu wrote: > On Thu, Aug 21, 2025 at 03:07:57PM +0300, Vladimir Sementsov-Ogievskiy wrote: > > What I dislike in the way, when we reset to blocking always, and set > > non-blocking again where needed: > > > > 1. Extra fcntl calls for nothing (I think actually, in most cases, for > > fds passed through migration stream(s) we'll want to keep fd as is) > > > > 2. When we reset to blocking on target, it's visible on source and may > > break things. > > > > In these series it's probably doesn't really matter, as at the time when > > we get the descriptor on target, it should not be used anymore on source. > > > > But for example, in CPR-transfer, where descriptors are passed in the > > preliminary stage, and source is running and use the descriptors, we > > shouldn't change the non-blocking status of fd on target. Probably, > > CPR-transfer for now only works with fds which are blpcking, so we don't > > have a problem. > > > > So, I think, that better default is preserve state of the flag for fds > > passed through migration stream. And backends may modify it if needed (I > > think, in most cases - they will not need). > > I agree having that as a default iochannel behavior is questionable. > > If it was defined for any fd-passing protocols making sure nonblocking > status is predictable (in this case, fds will be always blocking), IMHO it > should be done in the protocol layer either constantly setting or clearing > NONBLOCK flag before sending the fds, rather than having it silently > processed in iochannel internals. We explicitly did not want to rely on the senders to do this, as the senders generally does not know what receiving QEMU wants to do with the FDs. Clearing the non-blocking flag was chosen to make the default state of passed-in FDs, be identical to the default state of FDs that QEMU opens itself. This removes the possibility of bugs we've seen in the past, where code assumed an FD's blocking flag was in its default state you get from 'socket()' and then broke when receiving a pre-opened FD over QEMU in a different state. Basically anything using SocketAddress should be getting an FD in the same state whether opened by QEMU or passed in. > Do we know how many existing users are relying such behavior? I wonder if > we could still push this operation to the protocols that will need it, then > we can avoid this slightly awkward iochannel feature flag, because the > iochannel user will simply always have full control. Primarily this is relevant to the monitor QMP/HMP, which in turns makes it is relevant to UNIX chardevs. There are a number of other areas in QEMU calling qio_channel_readv_full() with a non-NULL fds argument though. 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 :|
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?
Thanks,
--
Peter Xu
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 :|
© 2016 - 2025 Red Hat, Inc.