1. Patchew
  2. QEMU
  3. [PATCH 0/8] Fix deadlock with bdrv_open of self-served NBD
  4. View patch

[PATCH 5/8] qio: Let listening sockets remember their owning QIONetListener

Eric Blake posted 8 patches 1 week, 5 days ago
Maintainers: "Daniel P. Berrangé" <berrange@redhat.com>, Eric Blake <eblake@redhat.com>, Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>, Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>
There is a newer version of this series
[PATCH 5/8] qio: Let listening sockets remember their owning QIONetListener
Posted by Eric Blake 1 week, 5 days ago 
Make it easier to get from the sioc listening to a single address on
behalf of a NetListener back to its owning object, which will be
beneficial in an upcoming patch that teaches NetListener how to
interact with AioContext.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 include/io/channel-socket.h | 1 +
 io/channel-socket.c         | 1 +
 io/net-listener.c           | 1 +
 3 files changed, 3 insertions(+)

diff --git a/include/io/channel-socket.h b/include/io/channel-socket.h
index a88cf8b3a9f..eee686f3b4d 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;
+    struct QIONetListener *listener;
 };


diff --git a/io/channel-socket.c b/io/channel-socket.c
index 712b793eaf2..59e929f97c3 100644
--- a/io/channel-socket.c
+++ b/io/channel-socket.c
@@ -65,6 +65,7 @@ qio_channel_socket_new(void)
     sioc->fd = -1;
     sioc->zero_copy_queued = 0;
     sioc->zero_copy_sent = 0;
+    sioc->listener = NULL;

     ioc = QIO_CHANNEL(sioc);
     qio_channel_set_feature(ioc, QIO_CHANNEL_FEATURE_SHUTDOWN);
diff --git a/io/net-listener.c b/io/net-listener.c
index e1378b9a612..afdacdd5ff4 100644
--- a/io/net-listener.c
+++ b/io/net-listener.c
@@ -152,6 +152,7 @@ void qio_net_listener_add(QIONetListener *listener,
     if (listener->name) {
         qio_channel_set_name(QIO_CHANNEL(sioc), listener->name);
     }
+    sioc->listener = listener;

     listener->sioc = g_renew(QIOChannelSocket *, listener->sioc,
                              listener->nsioc + 1);
-- 
2.51.1
Re: [PATCH 5/8] qio: Let listening sockets remember their owning QIONetListener
Posted by Eric Blake 1 week, 3 days ago 
On Mon, Nov 03, 2025 at 02:10:56PM -0600, Eric Blake wrote:
> Make it easier to get from the sioc listening to a single address on
> behalf of a NetListener back to its owning object, which will be
> beneficial in an upcoming patch that teaches NetListener how to
> interact with AioContext.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>  include/io/channel-socket.h | 1 +
>  io/channel-socket.c         | 1 +
>  io/net-listener.c           | 1 +
>  3 files changed, 3 insertions(+)
> 
> diff --git a/include/io/channel-socket.h b/include/io/channel-socket.h
> index a88cf8b3a9f..eee686f3b4d 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;
> +    struct QIONetListener *listener;

Commenting on my own patch:

After re-reading docs/devel/style.rst, I can see that this particular
forward declaration of QIONetListener is not consistent with the
guidelines.  I have to have a forward reference, since the style guide
also forbids circular inclusion (net-listener.h already includes
channel-socket.h, so channel-socket.h cannot include net-listener.h);
but it may be better for me to move the forward reference into
include/qemu/typedefs.h rather than inlining it how I did here.

(It is a red herring that struct QIOChannelSocket{} already contains
two other uses of 'struct' in its declaration body - both of those are
for 'struct sockaddr_storage' which is the POSIX type always spelled
with struct, with no typical QEMU CamelCase wrapper)

> +++ b/io/channel-socket.c
> @@ -65,6 +65,7 @@ qio_channel_socket_new(void)
>      sioc->fd = -1;
>      sioc->zero_copy_queued = 0;
>      sioc->zero_copy_sent = 0;
> +    sioc->listener = NULL;

Also, I added an explicit zero initialization to the new member to
match existing explicit initializers.  But checking qom/object.c, I
see that object_new() first uses g_malloc() instead of g_new0(), but
then calls object_initialize_with_type() does a forced memset(,0,) -
so all object constructors that do explicit field initialization to
zero are doing redundant work.

Dropping the sioc->listener = NULL assignment from this patch thus
makes sense from the less work perspective, but now that I've pointed
it out, dropping the sioc->zero_copy_* = 0 lines makes sense too.  But
cleanups like that should probably be a separate patch, and maybe
touch as many clients of object_new() as possible (perhaps via
Coccinelle?), rather than shoehorning in a deletion of just those two
lines into a v2 of this particular patch.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org
Re: [PATCH 5/8] qio: Let listening sockets remember their owning QIONetListener
Posted by Eric Blake 1 week, 2 days ago 
On Wed, Nov 05, 2025 at 02:06:06PM -0600, Eric Blake wrote:
> On Mon, Nov 03, 2025 at 02:10:56PM -0600, Eric Blake wrote:
> > Make it easier to get from the sioc listening to a single address on
> > behalf of a NetListener back to its owning object, which will be
> > beneficial in an upcoming patch that teaches NetListener how to
> > interact with AioContext.
> > 
> > Signed-off-by: Eric Blake <eblake@redhat.com>
> > ---
> >  include/io/channel-socket.h | 1 +
> >  io/channel-socket.c         | 1 +
> >  io/net-listener.c           | 1 +
> >  3 files changed, 3 insertions(+)
> > 
> > diff --git a/include/io/channel-socket.h b/include/io/channel-socket.h
> > index a88cf8b3a9f..eee686f3b4d 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;
> > +    struct QIONetListener *listener;
> 
> Commenting on my own patch:
> 
> After re-reading docs/devel/style.rst, I can see that this particular
> forward declaration of QIONetListener is not consistent with the
> guidelines.  I have to have a forward reference, since the style guide
> also forbids circular inclusion (net-listener.h already includes
> channel-socket.h, so channel-socket.h cannot include net-listener.h);
> but it may be better for me to move the forward reference into
> include/qemu/typedefs.h rather than inlining it how I did here.

Then again, include/qemu/typedefs.h states "For struct types used in
only a few headers, judicious use of the struct tag instead of the
typedef name is commonly preferable."

So, to keep it simpler, I'll just justify my choice in the commit message.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org
Re: [PATCH 5/8] qio: Let listening sockets remember their owning QIONetListener
Posted by Daniel P. Berrangé 1 week, 1 day ago 
On Thu, Nov 06, 2025 at 12:35:05PM -0600, Eric Blake wrote:
> On Wed, Nov 05, 2025 at 02:06:06PM -0600, Eric Blake wrote:
> > On Mon, Nov 03, 2025 at 02:10:56PM -0600, Eric Blake wrote:
> > > Make it easier to get from the sioc listening to a single address on
> > > behalf of a NetListener back to its owning object, which will be
> > > beneficial in an upcoming patch that teaches NetListener how to
> > > interact with AioContext.
> > > 
> > > Signed-off-by: Eric Blake <eblake@redhat.com>
> > > ---
> > >  include/io/channel-socket.h | 1 +
> > >  io/channel-socket.c         | 1 +
> > >  io/net-listener.c           | 1 +
> > >  3 files changed, 3 insertions(+)
> > > 
> > > diff --git a/include/io/channel-socket.h b/include/io/channel-socket.h
> > > index a88cf8b3a9f..eee686f3b4d 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;
> > > +    struct QIONetListener *listener;
> > 
> > Commenting on my own patch:
> > 
> > After re-reading docs/devel/style.rst, I can see that this particular
> > forward declaration of QIONetListener is not consistent with the
> > guidelines.  I have to have a forward reference, since the style guide
> > also forbids circular inclusion (net-listener.h already includes
> > channel-socket.h, so channel-socket.h cannot include net-listener.h);
> > but it may be better for me to move the forward reference into
> > include/qemu/typedefs.h rather than inlining it how I did here.
> 
> Then again, include/qemu/typedefs.h states "For struct types used in
> only a few headers, judicious use of the struct tag instead of the
> typedef name is commonly preferable."
> 
> So, to keep it simpler, I'll just justify my choice in the commit message.

I would really rather we avoided the bi-directional pointer linkage
entirely. AFAICT, this is only required because the later patch is
passing a QIOChannelSocket as the opaque data parametr for a callback.

It would be preferrable if we would instead pass a standalonme data
struct containing the QIOChannelSocket + QIONetListener, and then
avoid this back-linkage.


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 :|
Re: [PATCH 5/8] qio: Let listening sockets remember their owning QIONetListener
Posted by Eric Blake 1 week, 1 day ago 
On Fri, Nov 07, 2025 at 08:50:16AM +0000, Daniel P. Berrangé wrote:

> > > > +++ 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;
> > > > +    struct QIONetListener *listener;
> > > 
> > > Commenting on my own patch:
> > > 
> > > After re-reading docs/devel/style.rst, I can see that this particular
> > > forward declaration of QIONetListener is not consistent with the
> > > guidelines.  I have to have a forward reference, since the style guide
> > > also forbids circular inclusion (net-listener.h already includes
> > > channel-socket.h, so channel-socket.h cannot include net-listener.h);
> > > but it may be better for me to move the forward reference into
> > > include/qemu/typedefs.h rather than inlining it how I did here.
> > 
> > Then again, include/qemu/typedefs.h states "For struct types used in
> > only a few headers, judicious use of the struct tag instead of the
> > typedef name is commonly preferable."
> > 
> > So, to keep it simpler, I'll just justify my choice in the commit message.
> 
> I would really rather we avoided the bi-directional pointer linkage
> entirely. AFAICT, this is only required because the later patch is
> passing a QIOChannelSocket as the opaque data parametr for a callback.
> 
> It would be preferrable if we would instead pass a standalonme data
> struct containing the QIOChannelSocket + QIONetListener, and then
> avoid this back-linkage.

Reasonable; a bit more verbose in net-listener.c, but less impact to
channel-socket.h, so I'll do that in v2.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.