On Tue, Jan 15, 2019 at 07:53:51PM +0300, Yury Kotov wrote: > 15.01.2019, 18:39, "Daniel P. Berrangé" <berra...@redhat.com>: > > On Fri, Jan 11, 2019 at 04:36:11PM +0800, Yongji Xie wrote: > >> On Fri, 11 Jan 2019 at 16:32, Daniel P. Berrangé <berra...@redhat.com> > >> wrote: > >> > > >> > On Fri, Jan 11, 2019 at 03:50:40PM +0800, Yongji Xie wrote: > >> > > On Fri, 11 Jan 2019 at 00:41, Daniel P. Berrangé > >> <berra...@redhat.com> wrote: > >> > > > > >> > > > We need to fix qemu_chr_fe_wait_connected so that it does explicit > >> > > > synchronization wrt to any ongoing background connection process. > >> > > > It must only return once all TLS/telnet/websock handshakes have > >> > > > completed. If we fix that correctly, then I believe it will also > >> > > > solve the problem you're trying to address. > >> > > > > >> > > > >> > > Yes, I think this should be the right way to go. To fix it, my thought > >> > > is to track the async QIOChannelSocket in SocketChardev. Then we can > >> > > easily get the connection progress in qemu_chr_fe_wait_connected(). Do > >> > > you have any suggestion? > >> > > >> > I've got a few patches that refactor the code to fix this. I'll send > >> them > >> > today and CC you on them. > >> > > >> > >> That would be great! Thank you. > > > > It took me rather longer than expected to fully debug all scenarios, but > > I've finally sent patches: > > > > https://lists.gnu.org/archive/html/qemu-devel/2019-01/msg03344.html > > I didn't do a deep review and may be wrong, but I think the race is still > possible in the hotplug case. > > Example: > 1. User adds a chardev (qmp: chardev-add) with 'reconnect' option; > 2. Some main-loop iterations... > 3. Reconnect timer triggers and starts connection thread; > 4. User adds a vhost-user-blk (qmp: device-add): device realize -> > wait_connected. > > Here, there is a chance that we are in the wait_connected and the connection > thread is still running.
Hmm, dealing with device-add is tricky. tcp_chr_wait_connect rather assumes no main loop is running. I'll have to think about how we could possibly handle hotplug safely. 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 :|
