From patchwork Mon Feb 11 18:24:26 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Daniel_P=2E_Berrang=C3=A9?= X-Patchwork-Id: 10806611 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id D0CD7922 for ; Mon, 11 Feb 2019 18:26:17 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id C10F62B09D for ; Mon, 11 Feb 2019 18:26:17 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id B46F92B0B2; Mon, 11 Feb 2019 18:26:17 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.9 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI autolearn=ham version=3.3.1 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 488F92B09D for ; Mon, 11 Feb 2019 18:26:17 +0000 (UTC) Received: from localhost ([127.0.0.1]:54343 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gtGHY-0007pK-Gc for patchwork-qemu-devel@patchwork.kernel.org; Mon, 11 Feb 2019 13:26:16 -0500 Received: from eggs.gnu.org ([209.51.188.92]:50047) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gtGGG-0007o0-0m for qemu-devel@nongnu.org; Mon, 11 Feb 2019 13:24:57 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gtGGE-0007cl-LO for qemu-devel@nongnu.org; Mon, 11 Feb 2019 13:24:55 -0500 Received: from mx1.redhat.com ([209.132.183.28]:37794) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1gtGGE-0007aq-Bm for qemu-devel@nongnu.org; Mon, 11 Feb 2019 13:24:54 -0500 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 136CDC0C2352; Mon, 11 Feb 2019 18:24:50 +0000 (UTC) Received: from localhost.localdomain.com (ovpn-112-51.ams2.redhat.com [10.36.112.51]) by smtp.corp.redhat.com (Postfix) with ESMTP id AEE585F74; Mon, 11 Feb 2019 18:24:44 +0000 (UTC) From: =?utf-8?q?Daniel_P=2E_Berrang=C3=A9?= To: qemu-devel@nongnu.org Date: Mon, 11 Feb 2019 18:24:26 +0000 Message-Id: <20190211182442.8542-1-berrange@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.32]); Mon, 11 Feb 2019 18:24:50 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH v3 00/16] chardev: refactoring & many bugfixes related tcp_chr_wait_connected X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Laurent Vivier , Thomas Huth , Yongji Xie , =?utf-8?q?Marc-Andr=C3=A9_Lureau?= , Paolo Bonzini Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: "Qemu-devel" X-Virus-Scanned: ClamAV using ClamSMTP This is a followup to v1: https://lists.gnu.org/archive/html/qemu-devel/2019-01/msg03344.html v2: http://lists.nongnu.org/archive/html/qemu-devel/2019-01/msg05947.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 v3: - Fix thread between qio_task_thread_worker unlocking mutex and qio_task_thread_result free'ing mutex (Peter) - Ensure tcp_chr_wait_connected destroys any pending reconnect_timer GSource - Use g_setenv/g_unsetenv instead of setenv/unsetenv for platform portability 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 | 490 ++++++++++++++++++------- chardev/char.c | 2 + include/io/task.h | 29 +- io/task.c | 101 ++++-- 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, 939 insertions(+), 340 deletions(-)