From nobody Tue Nov 26 14:23:22 2024 Delivered-To: importer@patchew.org Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=pass(p=none dis=none) header.from=redhat.com ARC-Seal: i=1; a=rsa-sha256; t=1707364364; cv=none; d=zohomail.com; s=zohoarc; b=VdtemOXk4NVLVxOi62av2X6pN5QGmK/ythlmVIMb9C6BToaJil9JMwJWY4LAuR8eMHxb9THKFtW1FUFcPWvnS8rf9HmRYyIESHbIA1/Fp1FoErKPcE8WGqAhc51ohqzyn/P8tIYIYz2/TaCTbbhTDm8MC0/rlogwq42Vk6O1eO8= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1707364364; h=Content-Transfer-Encoding:Cc:Cc:Date:Date:From:From:In-Reply-To:List-Subscribe:List-Post:List-Id:List-Archive:List-Help:List-Unsubscribe:MIME-Version:Message-ID:References:Sender:Subject:Subject:To:To:Message-Id:Reply-To; bh=DuB/gS1fw5l58cDPYHu69lovXZ7IfV/xWWrobYs13f0=; b=FDzLUdEf5xXpgmqAhuy6cPHzB7wMIebpZCy68A7ZTVmwjaIN3ufjLZeC7U1okEqh+Zd6575BAfTOA3jIC91DfALR49GqRHbzxVEmbhPcDw7xngl99qooBspDk9KoxaGkqGmok79wxdlr8ey/qP3xHpPibKz2ZSzfoayAdpfpuCc= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=pass header.from= (p=none dis=none) Return-Path: Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) by mx.zohomail.com with SMTPS id 170736436412443.41879792977386; Wed, 7 Feb 2024 19:52:44 -0800 (PST) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1rXvRr-0003SM-Rr; Wed, 07 Feb 2024 22:51:39 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1rXvRq-0003Rq-GV for qemu-devel@nongnu.org; Wed, 07 Feb 2024 22:51:38 -0500 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1rXvRo-0003Il-TZ for qemu-devel@nongnu.org; Wed, 07 Feb 2024 22:51:38 -0500 Received: from mimecast-mx02.redhat.com (mx-ext.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-424-4rZejjg4P2-4HbCLSLQjPQ-1; Wed, 07 Feb 2024 22:51:34 -0500 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 5F24C29AC018; Thu, 8 Feb 2024 03:51:34 +0000 (UTC) Received: from x1n.redhat.com (unknown [10.72.116.45]) by smtp.corp.redhat.com (Postfix) with ESMTP id AB966200A2F4; Thu, 8 Feb 2024 03:51:31 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1707364296; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=DuB/gS1fw5l58cDPYHu69lovXZ7IfV/xWWrobYs13f0=; b=cnsxrRxA6qmgpVD/LACE1T6tYYPjx+/ya93G8FHR0tw+ylt49gc7DITO9i9ZmYUHZxSkW/ 17nA3MZMNGSZ/wKJQJmkV6UwyXS51uKzpR1/52VkGZc+qxdZuDSlr5+KX0neSeeIymMzl7 WbeqfW4KRDUD2O9Afy4reevq/VTpe9c= X-MC-Unique: 4rZejjg4P2-4HbCLSLQjPQ-1 From: peterx@redhat.com To: qemu-devel@nongnu.org Cc: Fabiano Rosas , Avihai Horon , peterx@redhat.com, =?UTF-8?q?Daniel=20P=20=2E=20Berrang=C3=A9?= Subject: [PATCH 1/2] migration/multifd: Cleanup TLS iochannel referencing Date: Thu, 8 Feb 2024 11:51:25 +0800 Message-ID: <20240208035126.370620-2-peterx@redhat.com> In-Reply-To: <20240208035126.370620-1-peterx@redhat.com> References: <20240208035126.370620-1-peterx@redhat.com> MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.4 Received-SPF: pass (zohomail.com: domain of gnu.org designates 209.51.188.17 as permitted sender) client-ip=209.51.188.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Received-SPF: pass client-ip=170.10.133.124; envelope-from=peterx@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -21 X-Spam_score: -2.2 X-Spam_bar: -- X-Spam_report: (-2.2 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.106, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H5=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: qemu-devel-bounces+importer=patchew.org@nongnu.org X-ZohoMail-DKIM: pass (identity @redhat.com) X-ZM-MESSAGEID: 1707364365609100002 Content-Type: text/plain; charset="utf-8" From: Peter Xu Commit a1af605bd5 ("migration/multifd: fix hangup with TLS-Multifd due to blocking handshake") introduced a thread for TLS channels, which will resolve the issue on blocking the main thread. However in the same commit p->c is slightly abused just to be able to pass over the pointer "p" into the thread. That's the major reason we'll need to conditionally free the io channel in the fault paths. To clean it up, using a separate structure to pass over both "p" and "tioc" in the tls handshake thread. Then we can make it a rule that p->c will never be set until the channel is completely setup. With that, we can drop the tricky conditional unref of the io channel in the error path. Signed-off-by: Peter Xu Reviewed-by: Fabiano Rosas --- migration/multifd.c | 37 +++++++++++++++++++++++-------------- 1 file changed, 23 insertions(+), 14 deletions(-) diff --git a/migration/multifd.c b/migration/multifd.c index adfe8c9a0a..4a85a6b7b3 100644 --- a/migration/multifd.c +++ b/migration/multifd.c @@ -873,16 +873,22 @@ out: =20 static void multifd_new_send_channel_async(QIOTask *task, gpointer opaque); =20 +typedef struct { + MultiFDSendParams *p; + QIOChannelTLS *tioc; +} MultiFDTLSThreadArgs; + static void *multifd_tls_handshake_thread(void *opaque) { - MultiFDSendParams *p =3D opaque; - QIOChannelTLS *tioc =3D QIO_CHANNEL_TLS(p->c); + MultiFDTLSThreadArgs *args =3D opaque; =20 - qio_channel_tls_handshake(tioc, + qio_channel_tls_handshake(args->tioc, multifd_new_send_channel_async, - p, + args->p, NULL, NULL); + g_free(args); + return NULL; } =20 @@ -892,6 +898,7 @@ static bool multifd_tls_channel_connect(MultiFDSendPara= ms *p, { MigrationState *s =3D migrate_get_current(); const char *hostname =3D s->hostname; + MultiFDTLSThreadArgs *args; QIOChannelTLS *tioc; =20 tioc =3D migration_tls_client_create(ioc, hostname, errp); @@ -906,11 +913,14 @@ static bool multifd_tls_channel_connect(MultiFDSendPa= rams *p, object_unref(OBJECT(ioc)); trace_multifd_tls_outgoing_handshake_start(ioc, tioc, hostname); qio_channel_set_name(QIO_CHANNEL(tioc), "multifd-tls-outgoing"); - p->c =3D QIO_CHANNEL(tioc); + + args =3D g_new0(MultiFDTLSThreadArgs, 1); + args->tioc =3D tioc; + args->p =3D p; =20 p->tls_thread_created =3D true; qemu_thread_create(&p->tls_thread, "multifd-tls-handshake-worker", - multifd_tls_handshake_thread, p, + multifd_tls_handshake_thread, args, QEMU_THREAD_JOINABLE); return true; } @@ -923,6 +933,7 @@ static bool multifd_channel_connect(MultiFDSendParams *= p, =20 migration_ioc_register_yank(ioc); p->registered_yank =3D true; + /* Setup p->c only if the channel is completely setup */ p->c =3D ioc; =20 p->thread_created =3D true; @@ -976,14 +987,12 @@ out: =20 trace_multifd_new_send_channel_async_error(p->id, local_err); multifd_send_set_error(local_err); - if (!p->c) { - /* - * If no channel has been created, drop the initial - * reference. Otherwise cleanup happens at - * multifd_send_channel_destroy() - */ - object_unref(OBJECT(ioc)); - } + /* + * For error cases (TLS or non-TLS), IO channel is always freed here + * rather than when cleanup multifd: since p->c is not set, multifd + * cleanup code doesn't even know its existance. + */ + object_unref(OBJECT(ioc)); error_free(local_err); } =20 --=20 2.43.0