From patchwork Tue Jan 19 13:09:11 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: =?utf-8?q?Daniel_P=2E_Berrang=C3=A9?= X-Patchwork-Id: 569983 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 509AA14032B for ; Wed, 20 Jan 2016 00:22:52 +1100 (AEDT) Received: from localhost ([::1]:36926 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aLW84-0007O6-P0 for incoming@patchwork.ozlabs.org; Tue, 19 Jan 2016 08:15:24 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39636) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aLW2T-000772-2u for qemu-devel@nongnu.org; Tue, 19 Jan 2016 08:09:38 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aLW2R-0005tj-ER for qemu-devel@nongnu.org; Tue, 19 Jan 2016 08:09:37 -0500 Received: from mx1.redhat.com ([209.132.183.28]:40579) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aLW2M-0005rw-4I; Tue, 19 Jan 2016 08:09:30 -0500 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (Postfix) with ESMTPS id D5F3B8DFE0; Tue, 19 Jan 2016 13:09:29 +0000 (UTC) Received: from t530wlan.home.berrange.com.com (vpn1-6-80.ams2.redhat.com [10.36.6.80]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u0JD9Qat012785; Tue, 19 Jan 2016 08:09:28 -0500 From: "Daniel P. Berrange" To: qemu-devel@nongnu.org Date: Tue, 19 Jan 2016 13:09:11 +0000 Message-Id: <1453208963-16834-2-git-send-email-berrange@redhat.com> In-Reply-To: <1453208963-16834-1-git-send-email-berrange@redhat.com> References: <1453208963-16834-1-git-send-email-berrange@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.26 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 209.132.183.28 Cc: Paolo Bonzini , qemu-block@nongnu.org Subject: [Qemu-devel] [PATCH v3 01/13] nbd: convert block client to use I/O channels for connection setup 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 This converts the NBD block driver client to use the QIOChannelSocket class for initial connection setup. The NbdClientSession struct has two pointers, one to the master QIOChannelSocket providing the raw data channel, and one to a QIOChannel which is the current channel used for I/O. Initially the two point to the same object, but when TLS support is added, they will point to different objects. In this initial conversion though, all I/O is still actually done using the raw POSIX sockets APIs. Signed-off-by: Daniel P. Berrange --- Makefile | 6 +++--- block/nbd-client.c | 59 ++++++++++++++++++++++++++++-------------------------- block/nbd-client.h | 8 ++++++-- block/nbd.c | 39 ++++++++++++++++++------------------ tests/Makefile | 2 +- 5 files changed, 61 insertions(+), 53 deletions(-) diff --git a/Makefile b/Makefile index d0de2d4..b7b0f24 100644 --- a/Makefile +++ b/Makefile @@ -234,9 +234,9 @@ util/module.o-cflags = -D'CONFIG_BLOCK_MODULES=$(block-modules)' qemu-img.o: qemu-img-cmds.h -qemu-img$(EXESUF): qemu-img.o $(block-obj-y) $(crypto-obj-y) $(qom-obj-y) libqemuutil.a libqemustub.a -qemu-nbd$(EXESUF): qemu-nbd.o $(block-obj-y) $(crypto-obj-y) $(qom-obj-y) libqemuutil.a libqemustub.a -qemu-io$(EXESUF): qemu-io.o $(block-obj-y) $(crypto-obj-y) $(qom-obj-y) libqemuutil.a libqemustub.a +qemu-img$(EXESUF): qemu-img.o $(block-obj-y) $(crypto-obj-y) $(io-obj-y) $(qom-obj-y) libqemuutil.a libqemustub.a +qemu-nbd$(EXESUF): qemu-nbd.o $(block-obj-y) $(crypto-obj-y) $(io-obj-y) $(qom-obj-y) libqemuutil.a libqemustub.a +qemu-io$(EXESUF): qemu-io.o $(block-obj-y) $(crypto-obj-y) $(io-obj-y) $(qom-obj-y) libqemuutil.a libqemustub.a qemu-bridge-helper$(EXESUF): qemu-bridge-helper.o diff --git a/block/nbd-client.c b/block/nbd-client.c index b7fd17a..bf5b0a3 100644 --- a/block/nbd-client.c +++ b/block/nbd-client.c @@ -27,7 +27,6 @@ */ #include "nbd-client.h" -#include "qemu/sockets.h" #define HANDLE_TO_INDEX(bs, handle) ((handle) ^ ((uint64_t)(intptr_t)bs)) #define INDEX_TO_HANDLE(bs, index) ((index) ^ ((uint64_t)(intptr_t)bs)) @@ -48,12 +47,16 @@ static void nbd_teardown_connection(BlockDriverState *bs) NbdClientSession *client = nbd_get_client_session(bs); /* finish any pending coroutines */ - shutdown(client->sock, 2); + qio_channel_shutdown(client->ioc, + QIO_CHANNEL_SHUTDOWN_BOTH, + NULL); nbd_recv_coroutines_enter_all(client); nbd_client_detach_aio_context(bs); - closesocket(client->sock); - client->sock = -1; + object_unref(OBJECT(client->sioc)); + client->sioc = NULL; + object_unref(OBJECT(client->ioc)); + client->ioc = NULL; } static void nbd_reply_ready(void *opaque) @@ -68,7 +71,7 @@ static void nbd_reply_ready(void *opaque) * 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); + ret = nbd_receive_reply(s->sioc->fd, &s->reply); if (ret == -EAGAIN) { return; } @@ -124,27 +127,23 @@ static int nbd_co_send_request(BlockDriverState *bs, s->send_coroutine = qemu_coroutine_self(); aio_context = bdrv_get_aio_context(bs); - aio_set_fd_handler(aio_context, s->sock, false, + aio_set_fd_handler(aio_context, s->sioc->fd, false, nbd_reply_ready, nbd_restart_write, bs); if (qiov) { - if (!s->is_unix) { - socket_set_cork(s->sock, 1); - } - rc = nbd_send_request(s->sock, request); + qio_channel_set_cork(s->ioc, true); + rc = nbd_send_request(s->sioc->fd, request); if (rc >= 0) { - ret = qemu_co_sendv(s->sock, qiov->iov, qiov->niov, + ret = qemu_co_sendv(s->sioc->fd, qiov->iov, qiov->niov, offset, request->len); if (ret != request->len) { rc = -EIO; } } - if (!s->is_unix) { - socket_set_cork(s->sock, 0); - } + qio_channel_set_cork(s->ioc, false); } else { - rc = nbd_send_request(s->sock, request); + rc = nbd_send_request(s->sioc->fd, request); } - aio_set_fd_handler(aio_context, s->sock, false, + aio_set_fd_handler(aio_context, s->sioc->fd, false, nbd_reply_ready, NULL, bs); s->send_coroutine = NULL; qemu_co_mutex_unlock(&s->send_mutex); @@ -165,7 +164,7 @@ static void nbd_co_receive_reply(NbdClientSession *s, reply->error = EIO; } else { if (qiov && reply->error == 0) { - ret = qemu_co_recvv(s->sock, qiov->iov, qiov->niov, + ret = qemu_co_recvv(s->sioc->fd, qiov->iov, qiov->niov, offset, request->len); if (ret != request->len) { reply->error = EIO; @@ -349,14 +348,14 @@ int nbd_client_co_discard(BlockDriverState *bs, int64_t sector_num, void nbd_client_detach_aio_context(BlockDriverState *bs) { aio_set_fd_handler(bdrv_get_aio_context(bs), - nbd_get_client_session(bs)->sock, + nbd_get_client_session(bs)->sioc->fd, false, NULL, NULL, NULL); } void nbd_client_attach_aio_context(BlockDriverState *bs, AioContext *new_context) { - aio_set_fd_handler(new_context, nbd_get_client_session(bs)->sock, + aio_set_fd_handler(new_context, nbd_get_client_session(bs)->sioc->fd, false, nbd_reply_ready, NULL, bs); } @@ -369,39 +368,43 @@ void nbd_client_close(BlockDriverState *bs) .len = 0 }; - if (client->sock == -1) { + if (client->ioc == NULL) { return; } - nbd_send_request(client->sock, &request); + nbd_send_request(client->sioc->fd, &request); nbd_teardown_connection(bs); } -int nbd_client_init(BlockDriverState *bs, int sock, const char *export, - Error **errp) +int nbd_client_init(BlockDriverState *bs, QIOChannelSocket *sioc, + const char *export, Error **errp) { NbdClientSession *client = nbd_get_client_session(bs); int ret; /* NBD handshake */ logout("session init %s\n", export); - qemu_set_block(sock); - ret = nbd_receive_negotiate(sock, export, + qio_channel_set_blocking(QIO_CHANNEL(sioc), true, NULL); + + ret = nbd_receive_negotiate(sioc->fd, export, &client->nbdflags, &client->size, errp); if (ret < 0) { logout("Failed to negotiate with the NBD server\n"); - closesocket(sock); return ret; } qemu_co_mutex_init(&client->send_mutex); qemu_co_mutex_init(&client->free_sema); - client->sock = sock; + client->sioc = sioc; + object_ref(OBJECT(client->sioc)); + client->ioc = QIO_CHANNEL(sioc); + object_ref(OBJECT(client->ioc)); /* Now that we're connected, set the socket to be non-blocking and * kick the reply mechanism. */ - qemu_set_nonblock(sock); + qio_channel_set_blocking(QIO_CHANNEL(sioc), false, NULL); + nbd_client_attach_aio_context(bs, bdrv_get_aio_context(bs)); logout("Established connection with NBD server\n"); diff --git a/block/nbd-client.h b/block/nbd-client.h index e841340..e8b3283 100644 --- a/block/nbd-client.h +++ b/block/nbd-client.h @@ -4,6 +4,7 @@ #include "qemu-common.h" #include "block/nbd.h" #include "block/block_int.h" +#include "io/channel-socket.h" /* #define DEBUG_NBD */ @@ -17,7 +18,8 @@ #define MAX_NBD_REQUESTS 16 typedef struct NbdClientSession { - int sock; + QIOChannelSocket *sioc; /* The master data channel */ + QIOChannel *ioc; /* The current I/O channel which may differ (eg TLS) */ uint32_t nbdflags; off_t size; @@ -34,7 +36,9 @@ typedef struct NbdClientSession { NbdClientSession *nbd_get_client_session(BlockDriverState *bs); -int nbd_client_init(BlockDriverState *bs, int sock, const char *export_name, +int nbd_client_init(BlockDriverState *bs, + QIOChannelSocket *sock, + const char *export_name, Error **errp); void nbd_client_close(BlockDriverState *bs); diff --git a/block/nbd.c b/block/nbd.c index 416f42b..072fc5f 100644 --- a/block/nbd.c +++ b/block/nbd.c @@ -30,7 +30,6 @@ #include "qemu/uri.h" #include "block/block_int.h" #include "qemu/module.h" -#include "qemu/sockets.h" #include "qapi/qmp/qdict.h" #include "qapi/qmp/qjson.h" #include "qapi/qmp/qint.h" @@ -239,25 +238,25 @@ NbdClientSession *nbd_get_client_session(BlockDriverState *bs) return &s->client; } -static int nbd_establish_connection(BlockDriverState *bs, - SocketAddress *saddr, - Error **errp) +static QIOChannelSocket *nbd_establish_connection(SocketAddress *saddr, + Error **errp) { - BDRVNBDState *s = bs->opaque; - int sock; + QIOChannelSocket *sioc; + Error *local_err = NULL; - sock = socket_connect(saddr, errp, NULL, NULL); + sioc = qio_channel_socket_new(); - if (sock < 0) { - logout("Failed to establish connection to NBD server\n"); - return -EIO; + qio_channel_socket_connect_sync(sioc, + saddr, + &local_err); + if (local_err) { + error_propagate(errp, local_err); + return NULL; } - if (!s->client.is_unix) { - socket_set_nodelay(sock); - } + qio_channel_set_delay(QIO_CHANNEL(sioc), false); - return sock; + return sioc; } static int nbd_open(BlockDriverState *bs, QDict *options, int flags, @@ -265,7 +264,8 @@ static int nbd_open(BlockDriverState *bs, QDict *options, int flags, { BDRVNBDState *s = bs->opaque; char *export = NULL; - int result, sock; + int result; + QIOChannelSocket *sioc; SocketAddress *saddr; /* Pop the config into our state object. Exit if invalid. */ @@ -277,15 +277,16 @@ static int nbd_open(BlockDriverState *bs, QDict *options, int flags, /* establish TCP connection, return error if it fails * TODO: Configurable retry-until-timeout behaviour. */ - sock = nbd_establish_connection(bs, saddr, errp); + sioc = nbd_establish_connection(saddr, errp); qapi_free_SocketAddress(saddr); - if (sock < 0) { + if (!sioc) { g_free(export); - return sock; + return -ECONNREFUSED; } /* NBD handshake */ - result = nbd_client_init(bs, sock, export, errp); + result = nbd_client_init(bs, sioc, export, errp); + object_unref(OBJECT(sioc)); g_free(export); return result; } diff --git a/tests/Makefile b/tests/Makefile index b7352f1..4c2958e 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -390,8 +390,8 @@ test-qapi-obj-y = tests/test-qapi-visit.o tests/test-qapi-types.o \ tests/test-qapi-event.o tests/test-qmp-introspect.o \ $(test-qom-obj-y) test-crypto-obj-y = $(crypto-obj-y) $(test-qom-obj-y) -test-block-obj-y = $(block-obj-y) $(test-crypto-obj-y) test-io-obj-y = $(io-obj-y) $(test-crypto-obj-y) +test-block-obj-y = $(block-obj-y) $(test-io-obj-y) tests/check-qint$(EXESUF): tests/check-qint.o $(test-util-obj-y) tests/check-qstring$(EXESUF): tests/check-qstring.o $(test-util-obj-y)