Re: [Qemu-devel] [PATCH 3/3] char-socket: delay setting fd-pass feature until connected

[Daniel P . Berrangé]

On Tue, Jul 17, 2018 at 02:52:39PM +0200, Marc-André Lureau wrote:
> Wait for QIO channel connection completion, and check the feature set
> by QIO. This fixes setting fd-pass chardev feature on
> SOCKET_ADDRESS_FD where fd has AF_UNIX.
> 
> Signed-off-by: Marc-André Lureau <address@hidden>
> ---
>  chardev/char-socket.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/chardev/char-socket.c b/chardev/char-socket.c
> index 6daa8d003a..7387e632d4 100644
> --- a/chardev/char-socket.c
> +++ b/chardev/char-socket.c
> @@ -789,6 +789,9 @@ static int tcp_chr_new_client(Chardev *chr, 
> QIOChannelSocket *sioc)
>  
>      qio_channel_set_blocking(s->ioc, false, NULL);
>  
> +    if (qio_channel_has_feature(s->ioc, QIO_CHANNEL_FEATURE_FD_PASS)) {
> +        qemu_chr_set_feature(chr, QEMU_CHAR_FEATURE_FD_PASS);
> +    }

I really don't like this approach as it has unpleasant async/race prone
semantics or the external users of the chardev.

With the current approach they know that once the chardev is created,
the features have been initialized.

With this approach, the features are only initialized once the client
connection has been completed, or once the server has started listening,
which may or may not be the case once the chardev constructor completes.

>      /* be isn't opened until we get a connection */
>      *be_opened = false;
> -- 
> 2.18.0.129.ge3331758f1
> 

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