On 07/03/2018 13:40, Daniel P. Berrangé wrote: > On Wed, Mar 07, 2018 at 12:36:50PM +0000, Daniel P. Berrangé wrote: >> On Tue, Mar 06, 2018 at 01:33:20PM +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. >>> >>> For dynamically created tcp chardev, we don't postpone that by checking >>> the init_machine_done variable. >> >> Not sure I see the need for this one - we've already postponed the >> acceptance of a client in the patch 7. > > Opps, meant to remove this comment, in favour of the later comment - ignore > this bit.
Since time is ticking for soft freeze, I'll queue the series without this patch. Paolo >> >>> >>> Signed-off-by: Peter Xu <pet...@redhat.com> >>> --- >>> chardev/char-socket.c | 10 ++++++++++ >>> 1 file changed, 10 insertions(+) >>> >>> diff --git a/chardev/char-socket.c b/chardev/char-socket.c >>> index bd40864f87..997c70dd7d 100644 >>> --- a/chardev/char-socket.c >>> +++ b/chardev/char-socket.c >>> @@ -31,6 +31,7 @@ >>> #include "qemu/option.h" >>> #include "qapi/error.h" >>> #include "qapi/clone-visitor.h" >>> +#include "sysemu/sysemu.h" >>> >>> #include "chardev/char-io.h" >>> >>> @@ -722,6 +723,11 @@ static void tcp_chr_tls_init(Chardev *chr) >>> Error *err = NULL; >>> gchar *name; >>> >>> + if (!machine_init_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, >>> @@ -1145,6 +1151,10 @@ static int tcp_chr_machine_done_hook(Chardev *chr) >>> tcp_chr_connect_async(chr); >>> } >>> >>> + if (s->tls_creds) { >>> + tcp_chr_tls_init(chr); >>> + } >> >> This looks questionable - AFAICT, there's no guarantee we have any >> client connection active when the machine dnoe hook runs. Only if >> the chardev is set in client mode, and reconnect_time is *not* set, >> but this seems to be run unconditionally. >> >> >> 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 >> :| >> > > Regards, > Daniel >