Hi On Wed, Jan 23, 2019 at 6:27 PM Daniel P. Berrangé <berra...@redhat.com> wrote: > > This is a followup to > > v1: https://lists.gnu.org/archive/html/qemu-devel/2019-01/msg03344.html > > This series comes out of a discussion between myself & Yongji Xie in: > > https://lists.gnu.org/archive/html/qemu-devel/2019-01/msg01881.html > > I eventually understood that the problem faced was that > tcp_chr_wait_connected was racing with the background connection attempt > previously started, causing two connections to be established. This > broke because some vhost user servers only allow a single connection. > > After messing around with the code alot the final solution was in fact > very easy. We simply have to delay the first background connection > attempt until the main loop is running. It will then automatically > turn into a no-op if tcp_chr_wait_connected has been run. This is > dealt with in the last patch in this series > > I believe this should solve the problem Yongji Xie faced, and thus not > require us to add support for "nowait" option with client sockets at > all. The reconnect=1 option effectively already implements nowait > semantics, and now plays nicely with tcp_chr_wait_connected. > > In investigating this I found various other bugs that needed fixing and > identified some useful refactoring to simplify / clarify the code, hence > this very long series. > > This series passes make check-unit & check-qtest (for x86_64 target). > > I did a test with tests/vhost-user-bridge too, which was in fact already > broken for the same reason Yongji Xie found - it only accepts a single > connection. This series fixes use of reconnect=1 with vhost-user-bridge. > > Changed in v2: > > - Rewrite the way we synchronize in tcp_chr_wait_connected again > to cope with chardev+device hotplug scenario > - Add extensive unit test coverage > - Resolve conflicts with git master > > Daniel P. Berrangé (16): > io: store reference to thread information in the QIOTask struct > io: add qio_task_wait_thread to join with a background thread > chardev: fix validation of options for QMP created chardevs > chardev: forbid 'reconnect' option with server sockets > chardev: forbid 'wait' option with client sockets > chardev: remove many local variables in qemu_chr_parse_socket > chardev: ensure qemu_chr_parse_compat reports missing driver error > chardev: remove unused 'sioc' variable & cleanup paths > chardev: split tcp_chr_wait_connected into two methods > chardev: split up qmp_chardev_open_socket connection code > chardev: use a state machine for socket connection state > chardev: honour the reconnect setting in tcp_chr_wait_connected > chardev: disallow TLS/telnet/websocket with tcp_chr_wait_connected > chardev: fix race with client connections in tcp_chr_wait_connected > tests: expand coverage of socket chardev test > chardev: ensure termios is fully initialized > > chardev/char-serial.c | 2 +- > chardev/char-socket.c | 487 ++++++++++++++++++------- > chardev/char.c | 2 + > include/io/task.h | 29 +- > io/task.c | 88 +++-- > io/trace-events | 2 + > tests/ivshmem-test.c | 2 +- > tests/libqtest.c | 4 +- > tests/test-char.c | 643 ++++++++++++++++++++++++--------- > tests/test-filter-redirector.c | 4 +- > 10 files changed, 928 insertions(+), 335 deletions(-) >
With the fix squashed, I am queuing this series. Reviewed-by: Marc-André Lureau <marcandre.lur...@redhat.com> > -- > 2.20.1 >
