From patchwork Fri Jan 15 16:04:27 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paolo Bonzini X-Patchwork-Id: 8042941 Return-Path: X-Original-To: patchwork-qemu-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 73F58BEEE5 for ; Fri, 15 Jan 2016 16:09:04 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 545D82041A for ; Fri, 15 Jan 2016 16:09:03 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 1839E2040F for ; Fri, 15 Jan 2016 16:09:02 +0000 (UTC) Received: from localhost ([::1]:47811 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aK6vt-000724-BJ for patchwork-qemu-devel@patchwork.kernel.org; Fri, 15 Jan 2016 11:09:01 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59198) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aK6ry-00080c-6g for qemu-devel@nongnu.org; Fri, 15 Jan 2016 11:05:01 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aK6rr-0003KZ-HX for qemu-devel@nongnu.org; Fri, 15 Jan 2016 11:04:58 -0500 Received: from mail-wm0-x241.google.com ([2a00:1450:400c:c09::241]:34726) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aK6rr-0003K8-6e for qemu-devel@nongnu.org; Fri, 15 Jan 2016 11:04:51 -0500 Received: by mail-wm0-x241.google.com with SMTP id b14so3558907wmb.1 for ; Fri, 15 Jan 2016 08:04:51 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:from:to:cc:subject:date:message-id:in-reply-to:references; bh=gHU19IxqCuAz3UE5Jym1H+C2J3chcLEgFWM1k29tvI0=; b=HDurfd+sjEyZD7TccJx/kvsba6kdbuZXM37JTchuK1cu16vBXlQjKMArB6Zz1nHfKd lZkRdAYzaCPItnb5RxHVgI/ZbfCOP1qaGD6IZq+lLsJFYR20su37Il1K2v6XQPZZPGWw WHJdvsmV7vT8zMMhouHQYYqlCHeXPuv3Ux68vcokaWASFLjmPMBS0pViybekNyk/9Aq1 +Wf3wAf6QxXLg8P/PVWyJB2OzHYKLfWU+WgqgKutzPJKwCdo1kCcVunoW435zqpaANFo GcF2Hi1C9NgRT2SEswcJlMOe12CxF6IvYKUVQ3Pl44hVgIy4ebQeAP+9z/m1l48z+nbZ M6xA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:sender:from:to:cc:subject:date:message-id :in-reply-to:references; bh=gHU19IxqCuAz3UE5Jym1H+C2J3chcLEgFWM1k29tvI0=; b=ielCqt0pwF7OAnQabAra1l4QUXu9wI+/6F5YtUNFTHFY3ZH8hZ8Hg4LJR/RJ4J3Wuf rm3zJawg8BIOuBFZUbqMZyLQYnkjXSZmTv3St+VJwEEqRBTu4mEn5+ML0PWeEVSB/KQ0 r25DtiIl9PI2bExOZMqSWYpMIKF6M2O98Rx+dC7fHlGw9wSf2XHFW8eJPr0vIrEtp2+8 A8NgahS/JkVaNHSMeD95/6ktdXUoVU3POuC/lJS8J1dKF9bG/HyORIsTahMJJRVVhxb0 ZV98XBv+dDl6EWBaVlgET2Rdf6qL4xjzFjK5GmZdnin6lqazo7lTEmIrWp/i8DOfNhaO JTbA== X-Gm-Message-State: ALoCoQkxc620750/8OAYQwv5CmOhICNxX1/WdxFjPItksQRfgNOfGgyz09sNIOWykM9Mty97lIiNk2aF8C5qWZmUBBlLoHuSmg== X-Received: by 10.194.220.233 with SMTP id pz9mr12189339wjc.95.1452873890553; Fri, 15 Jan 2016 08:04:50 -0800 (PST) Received: from 640k.lan (94-39-195-126.adsl-ull.clienti.tiscali.it. [94.39.195.126]) by smtp.gmail.com with ESMTPSA id c15sm3103036wmd.19.2016.01.15.08.04.49 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 15 Jan 2016 08:04:49 -0800 (PST) From: Paolo Bonzini To: qemu-devel@nongnu.org Date: Fri, 15 Jan 2016 17:04:27 +0100 Message-Id: <1452873871-138914-13-git-send-email-pbonzini@redhat.com> X-Mailer: git-send-email 1.8.3.1 In-Reply-To: <1452873871-138914-1-git-send-email-pbonzini@redhat.com> References: <1452873871-138914-1-git-send-email-pbonzini@redhat.com> X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 2a00:1450:400c:c09::241 Cc: Fam Zheng Subject: [Qemu-devel] [PULL 11/15] nbd-server: Coroutine based negotiation 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+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org X-Spam-Status: No, score=-6.8 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI, T_DKIM_INVALID, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP From: Fam Zheng Create a coroutine in nbd_client_new, so that nbd_send_negotiate doesn't need qemu_set_block(). Handlers need to be set temporarily for csock fd in case the coroutine yields during I/O. With this, if the other end disappears in the middle of the negotiation, we don't block the whole event loop. To make the code clearer, unify all function names that belong to negotiate, so they are less likely to be misused. This is important because we rely on negotiation staying in main loop, as commented in nbd_negotiate_read/write(). Signed-off-by: Fam Zheng Message-Id: <1452760863-25350-4-git-send-email-famz@redhat.com> Signed-off-by: Paolo Bonzini --- nbd/server.c | 150 ++++++++++++++++++++++++++++++++++++++++------------------- 1 file changed, 103 insertions(+), 47 deletions(-) diff --git a/nbd/server.c b/nbd/server.c index ba25ce3..8752885 100644 --- a/nbd/server.c +++ b/nbd/server.c @@ -93,13 +93,45 @@ static void nbd_set_handlers(NBDClient *client); static void nbd_unset_handlers(NBDClient *client); static void nbd_update_can_read(NBDClient *client); -static ssize_t drop_sync(int fd, size_t size) +static void nbd_negotiate_continue(void *opaque) +{ + qemu_coroutine_enter(opaque, NULL); +} + +static ssize_t nbd_negotiate_read(int fd, void *buffer, size_t size) +{ + ssize_t ret; + + assert(qemu_in_coroutine()); + /* Negotiation are always in main loop. */ + qemu_set_fd_handler(fd, nbd_negotiate_continue, NULL, + qemu_coroutine_self()); + ret = read_sync(fd, buffer, size); + qemu_set_fd_handler(fd, NULL, NULL, NULL); + return ret; + +} + +static ssize_t nbd_negotiate_write(int fd, void *buffer, size_t size) +{ + ssize_t ret; + + assert(qemu_in_coroutine()); + /* Negotiation are always in main loop. */ + qemu_set_fd_handler(fd, NULL, nbd_negotiate_continue, + qemu_coroutine_self()); + ret = write_sync(fd, buffer, size); + qemu_set_fd_handler(fd, NULL, NULL, NULL); + return ret; +} + +static ssize_t nbd_negotiate_drop_sync(int fd, size_t size) { ssize_t ret, dropped = size; uint8_t *buffer = g_malloc(MIN(65536, size)); while (size > 0) { - ret = read_sync(fd, buffer, MIN(65536, size)); + ret = nbd_negotiate_read(fd, buffer, MIN(65536, size)); if (ret < 0) { g_free(buffer); return ret; @@ -140,96 +172,96 @@ static ssize_t drop_sync(int fd, size_t size) */ -static int nbd_send_rep(int csock, uint32_t type, uint32_t opt) +static int nbd_negotiate_send_rep(int csock, uint32_t type, uint32_t opt) { uint64_t magic; uint32_t len; magic = cpu_to_be64(NBD_REP_MAGIC); - if (write_sync(csock, &magic, sizeof(magic)) != sizeof(magic)) { + if (nbd_negotiate_write(csock, &magic, sizeof(magic)) != sizeof(magic)) { LOG("write failed (rep magic)"); return -EINVAL; } opt = cpu_to_be32(opt); - if (write_sync(csock, &opt, sizeof(opt)) != sizeof(opt)) { + if (nbd_negotiate_write(csock, &opt, sizeof(opt)) != sizeof(opt)) { LOG("write failed (rep opt)"); return -EINVAL; } type = cpu_to_be32(type); - if (write_sync(csock, &type, sizeof(type)) != sizeof(type)) { + if (nbd_negotiate_write(csock, &type, sizeof(type)) != sizeof(type)) { LOG("write failed (rep type)"); return -EINVAL; } len = cpu_to_be32(0); - if (write_sync(csock, &len, sizeof(len)) != sizeof(len)) { + if (nbd_negotiate_write(csock, &len, sizeof(len)) != sizeof(len)) { LOG("write failed (rep data length)"); return -EINVAL; } return 0; } -static int nbd_send_rep_list(int csock, NBDExport *exp) +static int nbd_negotiate_send_rep_list(int csock, NBDExport *exp) { uint64_t magic, name_len; uint32_t opt, type, len; name_len = strlen(exp->name); magic = cpu_to_be64(NBD_REP_MAGIC); - if (write_sync(csock, &magic, sizeof(magic)) != sizeof(magic)) { + if (nbd_negotiate_write(csock, &magic, sizeof(magic)) != sizeof(magic)) { LOG("write failed (magic)"); return -EINVAL; } opt = cpu_to_be32(NBD_OPT_LIST); - if (write_sync(csock, &opt, sizeof(opt)) != sizeof(opt)) { + if (nbd_negotiate_write(csock, &opt, sizeof(opt)) != sizeof(opt)) { LOG("write failed (opt)"); return -EINVAL; } type = cpu_to_be32(NBD_REP_SERVER); - if (write_sync(csock, &type, sizeof(type)) != sizeof(type)) { + if (nbd_negotiate_write(csock, &type, sizeof(type)) != sizeof(type)) { LOG("write failed (reply type)"); return -EINVAL; } len = cpu_to_be32(name_len + sizeof(len)); - if (write_sync(csock, &len, sizeof(len)) != sizeof(len)) { + if (nbd_negotiate_write(csock, &len, sizeof(len)) != sizeof(len)) { LOG("write failed (length)"); return -EINVAL; } len = cpu_to_be32(name_len); - if (write_sync(csock, &len, sizeof(len)) != sizeof(len)) { + if (nbd_negotiate_write(csock, &len, sizeof(len)) != sizeof(len)) { LOG("write failed (length)"); return -EINVAL; } - if (write_sync(csock, exp->name, name_len) != name_len) { + if (nbd_negotiate_write(csock, exp->name, name_len) != name_len) { LOG("write failed (buffer)"); return -EINVAL; } return 0; } -static int nbd_handle_list(NBDClient *client, uint32_t length) +static int nbd_negotiate_handle_list(NBDClient *client, uint32_t length) { int csock; NBDExport *exp; csock = client->sock; if (length) { - if (drop_sync(csock, length) != length) { + if (nbd_negotiate_drop_sync(csock, length) != length) { return -EIO; } - return nbd_send_rep(csock, NBD_REP_ERR_INVALID, NBD_OPT_LIST); + return nbd_negotiate_send_rep(csock, NBD_REP_ERR_INVALID, NBD_OPT_LIST); } /* For each export, send a NBD_REP_SERVER reply. */ QTAILQ_FOREACH(exp, &exports, next) { - if (nbd_send_rep_list(csock, exp)) { + if (nbd_negotiate_send_rep_list(csock, exp)) { return -EINVAL; } } /* Finish with a NBD_REP_ACK. */ - return nbd_send_rep(csock, NBD_REP_ACK, NBD_OPT_LIST); + return nbd_negotiate_send_rep(csock, NBD_REP_ACK, NBD_OPT_LIST); } -static int nbd_handle_export_name(NBDClient *client, uint32_t length) +static int nbd_negotiate_handle_export_name(NBDClient *client, uint32_t length) { int rc = -EINVAL, csock = client->sock; char name[256]; @@ -242,7 +274,7 @@ static int nbd_handle_export_name(NBDClient *client, uint32_t length) LOG("Bad length received"); goto fail; } - if (read_sync(csock, name, length) != length) { + if (nbd_negotiate_read(csock, name, length) != length) { LOG("read failed"); goto fail; } @@ -261,7 +293,7 @@ fail: return rc; } -static int nbd_receive_options(NBDClient *client) +static int nbd_negotiate_options(NBDClient *client) { int csock = client->sock; uint32_t flags; @@ -280,7 +312,7 @@ static int nbd_receive_options(NBDClient *client) ... Rest of request */ - if (read_sync(csock, &flags, sizeof(flags)) != sizeof(flags)) { + if (nbd_negotiate_read(csock, &flags, sizeof(flags)) != sizeof(flags)) { LOG("read failed"); return -EIO; } @@ -296,7 +328,7 @@ static int nbd_receive_options(NBDClient *client) uint32_t tmp, length; uint64_t magic; - if (read_sync(csock, &magic, sizeof(magic)) != sizeof(magic)) { + if (nbd_negotiate_read(csock, &magic, sizeof(magic)) != sizeof(magic)) { LOG("read failed"); return -EINVAL; } @@ -306,12 +338,13 @@ static int nbd_receive_options(NBDClient *client) return -EINVAL; } - if (read_sync(csock, &tmp, sizeof(tmp)) != sizeof(tmp)) { + if (nbd_negotiate_read(csock, &tmp, sizeof(tmp)) != sizeof(tmp)) { LOG("read failed"); return -EINVAL; } - if (read_sync(csock, &length, sizeof(length)) != sizeof(length)) { + if (nbd_negotiate_read(csock, &length, + sizeof(length)) != sizeof(length)) { LOG("read failed"); return -EINVAL; } @@ -320,7 +353,7 @@ static int nbd_receive_options(NBDClient *client) TRACE("Checking option"); switch (be32_to_cpu(tmp)) { case NBD_OPT_LIST: - ret = nbd_handle_list(client, length); + ret = nbd_negotiate_handle_list(client, length); if (ret < 0) { return ret; } @@ -330,19 +363,25 @@ static int nbd_receive_options(NBDClient *client) return -EINVAL; case NBD_OPT_EXPORT_NAME: - return nbd_handle_export_name(client, length); + return nbd_negotiate_handle_export_name(client, length); default: tmp = be32_to_cpu(tmp); LOG("Unsupported option 0x%x", tmp); - nbd_send_rep(client->sock, NBD_REP_ERR_UNSUP, tmp); + nbd_negotiate_send_rep(client->sock, NBD_REP_ERR_UNSUP, tmp); return -EINVAL; } } } -static int nbd_send_negotiate(NBDClient *client) +typedef struct { + NBDClient *client; + Coroutine *co; +} NBDClientNewData; + +static coroutine_fn int nbd_negotiate(NBDClientNewData *data) { + NBDClient *client = data->client; int csock = client->sock; char buf[8 + 8 + 8 + 128]; int rc; @@ -368,7 +407,6 @@ static int nbd_send_negotiate(NBDClient *client) [28 .. 151] reserved (0) */ - qemu_set_block(csock); rc = -EINVAL; TRACE("Beginning negotiation."); @@ -385,16 +423,16 @@ static int nbd_send_negotiate(NBDClient *client) } if (client->exp) { - if (write_sync(csock, buf, sizeof(buf)) != sizeof(buf)) { + if (nbd_negotiate_write(csock, buf, sizeof(buf)) != sizeof(buf)) { LOG("write failed"); goto fail; } } else { - if (write_sync(csock, buf, 18) != 18) { + if (nbd_negotiate_write(csock, buf, 18) != 18) { LOG("write failed"); goto fail; } - rc = nbd_receive_options(client); + rc = nbd_negotiate_options(client); if (rc != 0) { LOG("option negotiation failed"); goto fail; @@ -403,7 +441,8 @@ static int nbd_send_negotiate(NBDClient *client) assert ((client->exp->nbdflags & ~65535) == 0); cpu_to_be64w((uint64_t*)(buf + 18), client->exp->size); cpu_to_be16w((uint16_t*)(buf + 26), client->exp->nbdflags | myflags); - if (write_sync(csock, buf + 18, sizeof(buf) - 18) != sizeof(buf) - 18) { + if (nbd_negotiate_write(csock, buf + 18, + sizeof(buf) - 18) != sizeof(buf) - 18) { LOG("write failed"); goto fail; } @@ -412,7 +451,6 @@ static int nbd_send_negotiate(NBDClient *client) TRACE("Negotiation succeeded."); rc = 0; fail: - qemu_set_nonblock(csock); return rc; } @@ -1028,25 +1066,43 @@ static void nbd_update_can_read(NBDClient *client) } } +static coroutine_fn void nbd_co_client_start(void *opaque) +{ + NBDClientNewData *data = opaque; + NBDClient *client = data->client; + NBDExport *exp = client->exp; + + if (exp) { + nbd_export_get(exp); + } + if (nbd_negotiate(data)) { + shutdown(client->sock, 2); + client->close(client); + goto out; + } + qemu_co_mutex_init(&client->send_lock); + nbd_set_handlers(client); + + if (exp) { + QTAILQ_INSERT_TAIL(&exp->clients, client, next); + } +out: + g_free(data); +} + void nbd_client_new(NBDExport *exp, int csock, void (*close_fn)(NBDClient *)) { NBDClient *client; + NBDClientNewData *data = g_new(NBDClientNewData, 1); + client = g_malloc0(sizeof(NBDClient)); client->refcount = 1; client->exp = exp; client->sock = csock; client->can_read = true; - if (nbd_send_negotiate(client)) { - shutdown(client->sock, 2); - close_fn(client); - return; - } client->close = close_fn; - qemu_co_mutex_init(&client->send_lock); - nbd_set_handlers(client); - if (exp) { - QTAILQ_INSERT_TAIL(&exp->clients, client, next); - nbd_export_get(exp); - } + data->client = client; + data->co = qemu_coroutine_create(nbd_co_client_start); + qemu_coroutine_enter(data->co, data); }