[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 15/15] chardev: tcp: postpone TLS work until
From: |
Daniel P . Berrangé |
Subject: |
Re: [Qemu-devel] [PATCH v2 15/15] chardev: tcp: postpone TLS work until machine done |
Date: |
Thu, 1 Mar 2018 16:03:04 +0000 |
User-agent: |
Mutt/1.9.2 (2017-12-15) |
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 :|
- Re: [Qemu-devel] [PATCH v2 11/15] qio: non-default context for TLS handshake, (continued)
- [Qemu-devel] [PATCH v2 12/15] chardev: introduce chr_machine_done hook, Peter Xu, 2018/03/01
- [Qemu-devel] [PATCH v2 13/15] char: use chardev's gcontext for async connect, Peter Xu, 2018/03/01
- [Qemu-devel] [PATCH v2 14/15] chardev: tcp: postpone async connection setup, Peter Xu, 2018/03/01
- [Qemu-devel] [PATCH v2 15/15] chardev: tcp: postpone TLS work until machine done, Peter Xu, 2018/03/01
- Re: [Qemu-devel] [PATCH v2 00/15] qio: general non-default GMainContext support, Daniel P . Berrangé, 2018/03/01