Re: [Qemu-devel] [PATCH v4 for-4.0 1/7] char-socket: Enable "nowait" option on client sockets

[Yury Kotov]

15.01.2019, 18:39, "Daniel P. Berrangé" <address@hidden>:
> On Fri, Jan 11, 2019 at 04:36:11PM +0800, Yongji Xie wrote:
>>  On Fri, 11 Jan 2019 at 16:32, Daniel P. Berrangé <address@hidden> 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é <address@hidden> 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
>
> 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 :|

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.

Regards,
Yury