Re: [PATCH v2 08/12] qio: Provide accessor around QIONetListener->sioc

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

On Sat, Nov 08, 2025 at 04:59:29PM -0600, Eric Blake wrote:
> An upcoming patch needs to pass more than just sioc as the opaque
> pointer to an AioContext; but since our AioContext code in general
> (and its QIO Channel wrapper code) lacks a notify callback present
> with GSource, we do not have the trivial option of just g_malloc'ing a
> small struct to hold all that data coupled with a notify of g_free.
> Instead, the data pointer must outlive the registered handler; in
> fact, having the data pointer have the same lifetime as QIONetListener
> is adequate.
> 
> But the cleanest way to stick such a helper struct in QIONetListener
> will be to rearrange internal struct members.  And that in turn means
> that all existing code that currently directly accesses
> listener->nsioc and listener->sioc[] should instead go through
> accessor functions, to be immune to the upcoming struct layout
> changes.  So this patch adds accessor methods qio_net_listener_nsioc()
> and qio_net_listener_sioc(), and puts them to use.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>
> 
> ---
> v2: new patch
> ---
>  include/io/net-listener.h | 24 ++++++++++++++++++++++++
>  chardev/char-socket.c     |  3 ++-
>  io/net-listener.c         | 12 ++++++++++++
>  migration/socket.c        |  5 +++--
>  ui/vnc.c                  | 36 +++++++++++++++++++++++-------------
>  5 files changed, 64 insertions(+), 16 deletions(-)
> 
> diff --git a/include/io/net-listener.h b/include/io/net-listener.h
> index 42fbfab5467..2605d6aae1e 100644
> --- a/include/io/net-listener.h
> +++ b/include/io/net-listener.h
> @@ -184,4 +184,28 @@ void qio_net_listener_disconnect(QIONetListener *listener);
>   */
>  bool qio_net_listener_is_connected(QIONetListener *listener);
> 
> +
> +/**
> + * qio_net_listener_nsioc:
> + * @listener: the network listener object
> + *
> + * Determine the number of listener channels currently owned by the
> + * given listener.
> + *
> + * Returns: number of channels, or 0 if not listening
> + */
> +size_t qio_net_listener_nsioc(QIONetListener *listener);
> +
> +
> +/**
> + * qio_net_listener_sioc:
> + * @listener: the network listener object
> + * @n: index of the sioc to grab
> + *
> + * Accessor for the nth sioc owned by the listener.
> + *
> + * Returns: the requested listener, or #NULL if not in bounds
> + */
> +QIOChannelSocket *qio_net_listener_sioc(QIONetListener *listener, size_t n);
> +
>  #endif /* QIO_NET_LISTENER_H */
> diff --git a/chardev/char-socket.c b/chardev/char-socket.c
> index 62852e3caf5..022ae47d726 100644
> --- a/chardev/char-socket.c
> +++ b/chardev/char-socket.c
> @@ -1255,7 +1255,8 @@ static int qmp_chardev_open_socket_server(Chardev *chr,
>      }
> 
>      qapi_free_SocketAddress(s->addr);
> -    s->addr = socket_local_address(s->listener->sioc[0]->fd, errp);
> +    s->addr = socket_local_address(qio_net_listener_sioc(s->listener, 0)->fd,
> +                                   errp);

Oh pre-existing bug / undesirable code pattern. This should be calling
qio_channel_socket_get_local_address, which avoids the needs to re-call
getsockname() as QIOChanelSocket has cached the local address.

Once we do that, then we would have 4 cases in this patch doing

            qio_channel_socket_get_local_address(
                qio_net_listener_sioc(listener, i), errp);

which would suggest adding a helper we could call as

      qio_net_listener_get_local_address(listener, i, errp)



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