From patchwork Wed Sep 19 13:49:49 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paolo Bonzini X-Patchwork-Id: 185043 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 3EC682C00FE for ; Wed, 19 Sep 2012 23:51:01 +1000 (EST) Received: from localhost ([::1]:48673 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TEKg6-0001o8-NS for incoming@patchwork.ozlabs.org; Wed, 19 Sep 2012 09:50:58 -0400 Received: from eggs.gnu.org ([208.118.235.92]:39679) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TEKfc-0001EG-E8 for qemu-devel@nongnu.org; Wed, 19 Sep 2012 09:50:34 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TEKfW-0005ZV-6x for qemu-devel@nongnu.org; Wed, 19 Sep 2012 09:50:28 -0400 Received: from mail-pb0-f45.google.com ([209.85.160.45]:56240) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TEKfW-0005IH-0A for qemu-devel@nongnu.org; Wed, 19 Sep 2012 09:50:22 -0400 Received: by mail-pb0-f45.google.com with SMTP id rp12so2503409pbb.4 for ; Wed, 19 Sep 2012 06:50:21 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:from:to:subject:date:message-id:x-mailer:in-reply-to :references; bh=bdoIfMosS/baTpef51T1Kf7Ts6uiBUXSvM30cn5zJGg=; b=I6yvUycftqWcopb7EGcZR79s4IIT1/rBaAWG3ZDdqXoZKUbwNLr89WEkygNXQ32dee LJWhcPesvb1thNKgx/h4Qy5gbE/VCDig2kSG8sIuVE11Xcn+/YuYnemEnj9L/quXzhtG 7oPG5po4FMxmn9KV9tm07g6Xxv+upFa9EeqmV6D1dYO9eWHrOsnHs15RSIeHDRdVZHp1 rclmWclUFi//bMqgo99h7FdX3tHO2Nwk75mS5PdHLgGrt2i/Ala9YjHj4kV/NwcG7aQc gHcrwLuMVPFXFMXAdf9UOTCf6sZShd3p2/CsCllVhaBRWAIQO0jVT9paQ5SfkxGQ6fGX pcTw== Received: by 10.66.75.106 with SMTP id b10mr6905588paw.73.1348062621199; Wed, 19 Sep 2012 06:50:21 -0700 (PDT) Received: from yakj.usersys.redhat.com (93-34-169-1.ip50.fastwebnet.it. [93.34.169.1]) by mx.google.com with ESMTPS id jw14sm623562pbb.36.2012.09.19.06.50.18 (version=TLSv1/SSLv3 cipher=OTHER); Wed, 19 Sep 2012 06:50:20 -0700 (PDT) From: Paolo Bonzini To: qemu-devel@nongnu.org Date: Wed, 19 Sep 2012 15:49:49 +0200 Message-Id: <1348062596-30446-6-git-send-email-pbonzini@redhat.com> X-Mailer: git-send-email 1.7.12 In-Reply-To: <1348062596-30446-1-git-send-email-pbonzini@redhat.com> References: <1348062596-30446-1-git-send-email-pbonzini@redhat.com> X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 209.85.160.45 Subject: [Qemu-devel] [PATCH 05/12] nbd: do not leak nbd_trip coroutines when a connection is torn down X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Because nbd_client_close removes the I/O handlers for the client socket, there is no way that any suspended coroutines are restarted. This will be a problem with the QEMU embedded NBD server, because we will have a QMP command to forcibly close all connections with the clients. Instead, we can exploit the reference counting of NBDClients; shutdown the client socket, which will make it readable and writeable. Also call the close callback, which will release the user's reference. The coroutines then will fail and exit cleanly, and release all remaining references, until the last refcount finally triggers the closure of the client. Signed-off-by: Paolo Bonzini --- nbd.c | 33 +++++++++++++++++++++++++++------ nbd.h | 1 + 2 file modificati, 28 inserzioni(+), 6 rimozioni(-) diff --git a/nbd.c b/nbd.c index 4aeb80a..eb72f4a 100644 --- a/nbd.c +++ b/nbd.c @@ -109,6 +109,7 @@ struct NBDClient { Coroutine *send_coroutine; int nb_requests; + bool closing; }; /* That's all folks */ @@ -655,19 +656,35 @@ void nbd_client_get(NBDClient *client) void nbd_client_put(NBDClient *client) { if (--client->refcount == 0) { + /* The last reference should be dropped by client->close, + * which is called by nbd_client_close. + */ + assert(client->closing); + + qemu_set_fd_handler2(client->sock, NULL, NULL, NULL, NULL); + close(client->sock); + client->sock = -1; g_free(client); } } -static void nbd_client_close(NBDClient *client) +void nbd_client_close(NBDClient *client) { - qemu_set_fd_handler2(client->sock, NULL, NULL, NULL, NULL); - close(client->sock); - client->sock = -1; + if (client->closing) { + return; + } + + client->closing = true; + + /* Force requests to finish. They will drop their own references, + * then we'll close the socket and free the NBDClient. + */ + shutdown(client->sock, 2); + + /* Also tell the client, so that they release their reference. */ if (client->close) { client->close(client); } - nbd_client_put(client); } static NBDRequest *nbd_request_get(NBDClient *client) @@ -810,14 +827,18 @@ out: static void nbd_trip(void *opaque) { NBDClient *client = opaque; - NBDRequest *req = nbd_request_get(client); NBDExport *exp = client->exp; + NBDRequest *req; struct nbd_request request; struct nbd_reply reply; ssize_t ret; TRACE("Reading request."); + if (client->closing) { + return; + } + req = nbd_request_get(client); ret = nbd_co_receive_request(req, &request); if (ret == -EAGAIN) { goto done; diff --git a/nbd.h b/nbd.h index a9038dc..8b84a50 100644 --- a/nbd.h +++ b/nbd.h @@ -84,6 +84,7 @@ void nbd_export_close(NBDExport *exp); NBDClient *nbd_client_new(NBDExport *exp, int csock, void (*close)(NBDClient *)); +void nbd_client_close(NBDClient *client); void nbd_client_get(NBDClient *client); void nbd_client_put(NBDClient *client);