qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: "Marc-André Lureau" <marcandre.lureau@redhat.com>
Cc: qemu-devel@nongnu.org, Paolo Bonzini <pbonzini@redhat.com>
Subject: Re: [Qemu-devel] [PATCH 3/3] char-socket: delay setting fd-pass feature until connected
Date: Tue, 17 Jul 2018 14:01:30 +0100	[thread overview]
Message-ID: <20180717130130.GQ18603@redhat.com> (raw)
In-Reply-To: <20180717125239.22517-4-marcandre.lureau@redhat.com>

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 <marcandre.lureau@redhat.com>
> ---
>  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 :|

  reply	other threads:[~2018-07-17 13:01 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-17 12:52 [Qemu-devel] [PATCH 0/3] chardev: fix setting FD_PASS for socket type=fd Marc-André Lureau
2018-07-17 12:52 ` [Qemu-devel] [PATCH 1/3] char-socket: correctly set has_reconnect when parsing QemuOpts Marc-André Lureau
2018-07-17 13:20   ` Daniel P. Berrangé
2018-07-17 12:52 ` [Qemu-devel] [PATCH 2/3] char-socket: make 'fd' incompatible with 'reconnect' Marc-André Lureau
2018-07-17 13:20   ` Daniel P. Berrangé
2018-07-17 12:52 ` [Qemu-devel] [PATCH 3/3] char-socket: delay setting fd-pass feature until connected Marc-André Lureau
2018-07-17 13:01   ` Daniel P. Berrangé [this message]
2018-07-17 13:07     ` Marc-André Lureau
2018-07-17 13:11       ` Daniel P. Berrangé
2018-07-17 13:16         ` Marc-André Lureau

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180717130130.GQ18603@redhat.com \
    --to=berrange@redhat.com \
    --cc=marcandre.lureau@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).