Re: [Qemu-devel] [PATCH v2 15/15] chardev: tcp: postpone TLS work until machine done

[Daniel P . Berrangé]

On Thu, Mar 01, 2018 at 04:44:38PM +0800, Peter Xu wrote:
> TLS handshake may create background GSource tasks, while we won't know
> the correct GMainContext until the whole chardev (including frontend)
> inited.  Let's postpone the initial TLS handshake until machine done.
> 
> If we dynamically add tcp chardev, it won't be affected since we have a
> new tcp_chr_machine_done flag to know whether we should postpone it or
> not.
> 
> Signed-off-by: Peter Xu <address@hidden>
> ---
>  chardev/char-socket.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)

I don't like this patch either for the same reasons as previous
patch - its creating different behaviour depending on whether
the 'wait' flag happens to have been set in -chardev.

> 
> diff --git a/chardev/char-socket.c b/chardev/char-socket.c
> index 2b355fc7a8..13aeca0b27 100644
> --- a/chardev/char-socket.c
> +++ b/chardev/char-socket.c
> @@ -72,6 +72,8 @@ typedef struct {
>  static gboolean socket_reconnect_timeout(gpointer opaque);
>  static void tcp_chr_telnet_init(Chardev *chr);
>  
> +static bool tcp_chr_machine_done;
> +
>  static void tcp_chr_reconn_timer_cancel(SocketChardev *s)
>  {
>      if (s->reconnect_timer) {
> @@ -719,6 +721,11 @@ static void tcp_chr_tls_init(Chardev *chr)
>      Error *err = NULL;
>      gchar *name;
>  
> +    if (!tcp_chr_machine_done) {
> +        /* This will be postponed to machine_done notifier */
> +        return;
> +    }
> +
>      if (s->is_listen) {
>          tioc = qio_channel_tls_new_server(
>              s->ioc, s->tls_creds,
> @@ -1131,10 +1138,17 @@ static int tcp_chr_machine_done_hook(Chardev *chr)
>  {
>      SocketChardev *s = SOCKET_CHARDEV(chr);
>  
> +    /* Set it multiple times won't hurt */
> +    tcp_chr_machine_done = true;
> +
>      if (s->reconnect_time) {
>          tcp_chr_connect_async(chr);
>      }
>  
> +    if (s->tls_creds) {
> +        tcp_chr_tls_init(chr);
> +    }
> +
>      return 0;
>  }
>  
> -- 
> 2.14.3
> 

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