From patchwork Thu Apr 19 15:09:20 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paolo Bonzini X-Patchwork-Id: 153806 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 BB496B7000 for ; Fri, 20 Apr 2012 01:10:46 +1000 (EST) Received: from localhost ([::1]:38421 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SKt0N-0000vS-Ut for incoming@patchwork.ozlabs.org; Thu, 19 Apr 2012 11:10:43 -0400 Received: from eggs.gnu.org ([208.118.235.92]:46993) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SKsze-0007yD-Cb for qemu-devel@nongnu.org; Thu, 19 Apr 2012 11:10:03 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SKszX-00019r-Kb for qemu-devel@nongnu.org; Thu, 19 Apr 2012 11:09:56 -0400 Received: from mail-pz0-f46.google.com ([209.85.210.46]:55249) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SKszX-0000tO-B4 for qemu-devel@nongnu.org; Thu, 19 Apr 2012 11:09:51 -0400 Received: by mail-pz0-f46.google.com with SMTP id z9so13381026dad.33 for ; Thu, 19 Apr 2012 08:09:50 -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=amzDbDFHOaOrBQretJZ0+MfGL8Xh3v+OliQIEjf592E=; b=pmmnjWCQFqAV/W4zdepckTmCVsZA+d4p5VnqywEFa8YEr5VhlgJ5Uudyl5yTXkJ2D+ Xg/miusBv6eZqtn0md0pdoiWh6NGzRVoxzs2JHmEaeJmS8/4v69jfuE2BCHMxdCmU2kk QPvMyQFDZx7RjXTc6pwETxiqbaaweVIW+229x4F0a/l3RjopS11Poc0VhZ0dxCxGUXYh 5B2n5Vp5uCXcYD8Oj2VNsQgy1slAzGTAOBUusiQEg2Tm45mM5EPk4FPOr9rFOAnQcwMt Zks9j0RGqPDmzqcRWdAI4v8p+yvMItpLxP5dvpgCEq+CayS4nEkLJvuYmwxCRB2NoZIW MXtg== Received: by 10.68.195.135 with SMTP id ie7mr5034861pbc.162.1334848190510; Thu, 19 Apr 2012 08:09:50 -0700 (PDT) Received: from yakj.usersys.redhat.com (93-34-182-16.ip50.fastwebnet.it. [93.34.182.16]) by mx.google.com with ESMTPS id pg6sm2479203pbb.41.2012.04.19.08.09.46 (version=TLSv1/SSLv3 cipher=OTHER); Thu, 19 Apr 2012 08:09:48 -0700 (PDT) From: Paolo Bonzini To: qemu-devel@nongnu.org Date: Thu, 19 Apr 2012 17:09:20 +0200 Message-Id: <1334848162-21401-6-git-send-email-pbonzini@redhat.com> X-Mailer: git-send-email 1.7.9.3 In-Reply-To: <1334848162-21401-1-git-send-email-pbonzini@redhat.com> References: <1334848162-21401-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.210.46 Subject: [Qemu-devel] [PATCH 5/7] nbd: do not block in nbd_wr_sync if no data at all is available 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 Right now, nbd_wr_sync will hang if no data at all is available on the socket and the other side is not going to provide any. Relax this by making it loop only for writes or partial reads. This fixes a race where one thread is executing qemu_aio_wait() and another is executing main_loop_wait(). Then, the select() call in main_loop_wait() can return stale data and call the "readable" callback with no data in the socket. Reported-by: Laurent Vivier Signed-off-by: Paolo Bonzini --- block/nbd.c | 12 ++++++++++-- nbd.c | 40 ++++++++++++++++++++++++++++++++++------ 2 files changed, 44 insertions(+), 8 deletions(-) diff --git a/block/nbd.c b/block/nbd.c index 1b0e384..e0af5b4 100644 --- a/block/nbd.c +++ b/block/nbd.c @@ -151,10 +151,18 @@ static void nbd_reply_ready(void *opaque) { BDRVNBDState *s = opaque; uint64_t i; + int ret; if (s->reply.handle == 0) { - /* No reply already in flight. Fetch a header. */ - if (nbd_receive_reply(s->sock, &s->reply) < 0) { + /* No reply already in flight. Fetch a header. It is possible + * that another thread has done the same thing in parallel, so + * the socket is not readable anymore. + */ + ret = nbd_receive_reply(s->sock, &s->reply); + if (ret == -EAGAIN) { + return; + } + if (ret < 0) { s->reply.handle = 0; goto fail; } diff --git a/nbd.c b/nbd.c index b31b3b2..4c6d7f1 100644 --- a/nbd.c +++ b/nbd.c @@ -78,9 +78,6 @@ /* That's all folks */ -#define read_sync(fd, buffer, size) nbd_wr_sync(fd, buffer, size, true) -#define write_sync(fd, buffer, size) nbd_wr_sync(fd, buffer, size, false) - ssize_t nbd_wr_sync(int fd, void *buffer, size_t size, bool do_read) { size_t offset = 0; @@ -107,7 +104,7 @@ ssize_t nbd_wr_sync(int fd, void *buffer, size_t size, bool do_read) err = socket_error(); /* recoverable error */ - if (err == EINTR || err == EAGAIN) { + if (err == EINTR || (offset > 0 && err == EAGAIN)) { continue; } @@ -126,6 +123,26 @@ ssize_t nbd_wr_sync(int fd, void *buffer, size_t size, bool do_read) return offset; } +static ssize_t read_sync(int fd, void *buffer, size_t size) +{ + /* Sockets are kept in blocking mode in the negotiation phase. After + * that, a non-readable socket simply means that another thread stole + * our request/reply. Synchronization is done with recv_coroutine, so + * that this is coroutine-safe. + */ + return nbd_wr_sync(fd, buffer, size, true); +} + +static ssize_t write_sync(int fd, void *buffer, size_t size) +{ + int ret; + do { + /* For writes, we do expect the socket to be writable. */ + ret = nbd_wr_sync(fd, buffer, size, false); + } while (ret == -EAGAIN); + return ret; +} + static void combine_addr(char *buf, size_t len, const char* address, uint16_t port) { @@ -203,6 +220,7 @@ static int nbd_send_negotiate(int csock, off_t size, uint32_t flags) [28 .. 151] reserved (0) */ + socket_set_block(csock); rc = -EINVAL; TRACE("Beginning negotiation."); @@ -222,6 +240,7 @@ static int nbd_send_negotiate(int csock, off_t size, uint32_t flags) TRACE("Negotiation succeeded."); rc = 0; fail: + socket_set_nonblock(csock); return rc; } @@ -235,6 +254,7 @@ int nbd_receive_negotiate(int csock, const char *name, uint32_t *flags, TRACE("Receiving negotiation."); + socket_set_block(csock); rc = -EINVAL; if (read_sync(csock, buf, 8) != 8) { @@ -349,6 +369,7 @@ int nbd_receive_negotiate(int csock, const char *name, uint32_t *flags, rc = 0; fail: + socket_set_nonblock(csock); return rc; } @@ -742,8 +763,11 @@ static ssize_t nbd_co_receive_request(NBDRequest *req, struct nbd_request *reque ssize_t rc; client->recv_coroutine = qemu_coroutine_self(); - if (nbd_receive_request(csock, request) < 0) { - rc = -EIO; + rc = nbd_receive_request(csock, request); + if (rc < 0) { + if (rc != -EAGAIN) { + rc = -EIO; + } goto out; } @@ -791,6 +815,9 @@ static void nbd_trip(void *opaque) TRACE("Reading request."); ret = nbd_co_receive_request(req, &request); + if (ret == -EAGAIN) { + goto done; + } if (ret == -EIO) { goto out; } @@ -901,6 +928,7 @@ static void nbd_trip(void *opaque) TRACE("Request/Reply complete"); +done: nbd_request_put(req); return;