Skip to content

Commit

Permalink
Merge remote-tracking branch 'remotes/ericb/tags/pull-nbd-2017-10-14'…
Browse files Browse the repository at this point in the history 
… into staging

nbd patches for 2017-10-14

- Marc-André Lureau - NBD: use g_new() family of functions
- Vladimir Sementsov-Ogievskiy - first half of 00/13 nbd minimal structured read

# gpg: Signature made Sun 15 Oct 2017 01:38:47 BST
# gpg:                using RSA key 0xA7A16B4A2527436A
# gpg: Good signature from "Eric Blake <[email protected]>"
# gpg:                 aka "Eric Blake (Free Software Programmer) <[email protected]>"
# gpg:                 aka "[jpeg image of size 6874]"
# Primary key fingerprint: 71C2 CC22 B1C4 6029 27D2  F3AA A7A1 6B4A 2527 436A

* remotes/ericb/tags/pull-nbd-2017-10-14:
  nbd: header constants indenting
  nbd/server: simplify reply transmission
  nbd/server: refactor nbd_co_send_simple_reply parameters
  nbd/server: do not use NBDReply structure
  nbd/server: structurize simple reply header sending
  nbd: rename some simple-request related objects to be _simple_
  block/nbd-client: refactor nbd_co_receive_reply
  block/nbd-client: assert qiov len once in nbd_co_request
  NBD: use g_new() family of functions

Signed-off-by: Peter Maydell <[email protected]>
  • Loading branch information
@pm215
pm215 committed Oct 16, 2017
2 parents 48ae1f6 + 92652b1 commit 79b2a13
Show file tree
Hide file tree
Showing 7 changed files with 89 additions and 96 deletions.
18 changes: 10 additions & 8 deletions block/nbd-client.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,6 @@ static int nbd_co_send_request(BlockDriverState *bs,
qio_channel_set_cork(s->ioc, true);
rc = nbd_send_request(s->ioc, request);
if (rc >= 0 && !s->quit) {
assert(request->len == iov_size(qiov->iov, qiov->niov));
if (qio_channel_writev_all(s->ioc, qiov->iov, qiov->niov,
NULL) < 0) {
rc = -EIO;
Expand All @@ -181,11 +180,11 @@ static int nbd_co_send_request(BlockDriverState *bs,
}

static int nbd_co_receive_reply(NBDClientSession *s,
NBDRequest *request,
uint64_t handle,
QEMUIOVector *qiov)
{
int ret;
int i = HANDLE_TO_INDEX(s, request->handle);
int i = HANDLE_TO_INDEX(s, handle);

/* Wait until we're woken up by nbd_read_reply_entry. */
s->requests[i].receiving = true;
Expand All @@ -194,10 +193,9 @@ static int nbd_co_receive_reply(NBDClientSession *s,
if (!s->ioc || s->quit) {
ret = -EIO;
} else {
assert(s->reply.handle == request->handle);
assert(s->reply.handle == handle);
ret = -s->reply.error;
if (qiov && s->reply.error == 0) {
assert(request->len == iov_size(qiov->iov, qiov->niov));
if (qio_channel_readv_all(s->ioc, qiov->iov, qiov->niov,
NULL) < 0) {
ret = -EIO;
Expand Down Expand Up @@ -231,15 +229,19 @@ static int nbd_co_request(BlockDriverState *bs,
NBDClientSession *client = nbd_get_client_session(bs);
int ret;

assert(!qiov || request->type == NBD_CMD_WRITE ||
request->type == NBD_CMD_READ);
if (qiov) {
assert(request->type == NBD_CMD_WRITE || request->type == NBD_CMD_READ);
assert(request->len == iov_size(qiov->iov, qiov->niov));
} else {
assert(request->type != NBD_CMD_WRITE && request->type != NBD_CMD_READ);
}
ret = nbd_co_send_request(bs, request,
request->type == NBD_CMD_WRITE ? qiov : NULL);
if (ret < 0) {
return ret;
}

return nbd_co_receive_reply(client, request,
return nbd_co_receive_reply(client, request->handle,
request->type == NBD_CMD_READ ? qiov : NULL);
}

Expand Down
21 changes: 14 additions & 7 deletions include/block/nbd.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,15 +63,22 @@ struct NBDReply {
};
typedef struct NBDReply NBDReply;

typedef struct NBDSimpleReply {
uint32_t magic; /* NBD_SIMPLE_REPLY_MAGIC */
uint32_t error;
uint64_t handle;
} QEMU_PACKED NBDSimpleReply;

/* Transmission (export) flags: sent from server to client during handshake,
but describe what will happen during transmission */
#define NBD_FLAG_HAS_FLAGS (1 << 0) /* Flags are there */
#define NBD_FLAG_READ_ONLY (1 << 1) /* Device is read-only */
#define NBD_FLAG_SEND_FLUSH (1 << 2) /* Send FLUSH */
#define NBD_FLAG_SEND_FUA (1 << 3) /* Send FUA (Force Unit Access) */
#define NBD_FLAG_ROTATIONAL (1 << 4) /* Use elevator algorithm - rotational media */
#define NBD_FLAG_SEND_TRIM (1 << 5) /* Send TRIM (discard) */
#define NBD_FLAG_SEND_WRITE_ZEROES (1 << 6) /* Send WRITE_ZEROES */
#define NBD_FLAG_HAS_FLAGS (1 << 0) /* Flags are there */
#define NBD_FLAG_READ_ONLY (1 << 1) /* Device is read-only */
#define NBD_FLAG_SEND_FLUSH (1 << 2) /* Send FLUSH */
#define NBD_FLAG_SEND_FUA (1 << 3) /* Send FUA (Force Unit Access) */
#define NBD_FLAG_ROTATIONAL (1 << 4) /* Use elevator algorithm -
rotational media */
#define NBD_FLAG_SEND_TRIM (1 << 5) /* Send TRIM (discard) */
#define NBD_FLAG_SEND_WRITE_ZEROES (1 << 6) /* Send WRITE_ZEROES */

/* New-style handshake (global) flags, sent from server to client, and
control what will happen during handshake phase. */
Expand Down
4 changes: 2 additions & 2 deletions nbd/client.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -931,7 +931,7 @@ int nbd_receive_reply(QIOChannel *ioc, NBDReply *reply, Error **errp)
}

/* Reply
[ 0 .. 3] magic (NBD_REPLY_MAGIC)
[ 0 .. 3] magic (NBD_SIMPLE_REPLY_MAGIC)
[ 4 .. 7] error (0 == no error)
[ 7 .. 15] handle
*/
Expand All @@ -949,7 +949,7 @@ int nbd_receive_reply(QIOChannel *ioc, NBDReply *reply, Error **errp)
}
trace_nbd_receive_reply(magic, reply->error, reply->handle);

if (magic != NBD_REPLY_MAGIC) {
if (magic != NBD_SIMPLE_REPLY_MAGIC) {
error_setg(errp, "invalid magic (got 0x%" PRIx32 ")", magic);
return -EINVAL;
}
Expand Down
34 changes: 17 additions & 17 deletions nbd/nbd-internal.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,23 +46,23 @@
/* Size of oldstyle negotiation */
#define NBD_OLDSTYLE_NEGOTIATE_SIZE (8 + 8 + 8 + 4 + 124)

#define NBD_REQUEST_MAGIC 0x25609513
#define NBD_REPLY_MAGIC 0x67446698
#define NBD_OPTS_MAGIC 0x49484156454F5054LL
#define NBD_CLIENT_MAGIC 0x0000420281861253LL
#define NBD_REP_MAGIC 0x0003e889045565a9LL

#define NBD_SET_SOCK _IO(0xab, 0)
#define NBD_SET_BLKSIZE _IO(0xab, 1)
#define NBD_SET_SIZE _IO(0xab, 2)
#define NBD_DO_IT _IO(0xab, 3)
#define NBD_CLEAR_SOCK _IO(0xab, 4)
#define NBD_CLEAR_QUE _IO(0xab, 5)
#define NBD_PRINT_DEBUG _IO(0xab, 6)
#define NBD_SET_SIZE_BLOCKS _IO(0xab, 7)
#define NBD_DISCONNECT _IO(0xab, 8)
#define NBD_SET_TIMEOUT _IO(0xab, 9)
#define NBD_SET_FLAGS _IO(0xab, 10)
#define NBD_REQUEST_MAGIC 0x25609513
#define NBD_SIMPLE_REPLY_MAGIC 0x67446698
#define NBD_OPTS_MAGIC 0x49484156454F5054LL
#define NBD_CLIENT_MAGIC 0x0000420281861253LL
#define NBD_REP_MAGIC 0x0003e889045565a9LL

#define NBD_SET_SOCK _IO(0xab, 0)
#define NBD_SET_BLKSIZE _IO(0xab, 1)
#define NBD_SET_SIZE _IO(0xab, 2)
#define NBD_DO_IT _IO(0xab, 3)
#define NBD_CLEAR_SOCK _IO(0xab, 4)
#define NBD_CLEAR_QUE _IO(0xab, 5)
#define NBD_PRINT_DEBUG _IO(0xab, 6)
#define NBD_SET_SIZE_BLOCKS _IO(0xab, 7)
#define NBD_DISCONNECT _IO(0xab, 8)
#define NBD_SET_TIMEOUT _IO(0xab, 9)
#define NBD_SET_FLAGS _IO(0xab, 10)

/* NBD errors are based on errno numbers, so there is a 1:1 mapping,
* but only a limited set of errno values is specified in the protocol.
Expand Down
101 changes: 43 additions & 58 deletions nbd/server.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -902,26 +902,6 @@ static int nbd_receive_request(QIOChannel *ioc, NBDRequest *request,
return 0;
}

static int nbd_send_reply(QIOChannel *ioc, NBDReply *reply, Error **errp)
{
uint8_t buf[NBD_REPLY_SIZE];

reply->error = system_errno_to_nbd_errno(reply->error);

trace_nbd_send_reply(reply->error, reply->handle);

/* Reply
[ 0 .. 3] magic (NBD_REPLY_MAGIC)
[ 4 .. 7] error (0 == no error)
[ 7 .. 15] handle
*/
stl_be_p(buf, NBD_REPLY_MAGIC);
stl_be_p(buf + 4, reply->error);
stq_be_p(buf + 8, reply->handle);

return nbd_write(ioc, buf, sizeof(buf), errp);
}

#define MAX_NBD_REQUESTS 16

void nbd_client_get(NBDClient *client)
Expand Down Expand Up @@ -1047,7 +1027,7 @@ NBDExport *nbd_export_new(BlockDriverState *bs, off_t dev_offset, off_t size,
{
AioContext *ctx;
BlockBackend *blk;
NBDExport *exp = g_malloc0(sizeof(NBDExport));
NBDExport *exp = g_new0(NBDExport, 1);
uint64_t perm;
int ret;

Expand Down Expand Up @@ -1208,38 +1188,51 @@ void nbd_export_close_all(void)
}
}

static int nbd_co_send_reply(NBDRequestData *req, NBDReply *reply, int len,
Error **errp)
static int coroutine_fn nbd_co_send_iov(NBDClient *client, struct iovec *iov,
unsigned niov, Error **errp)
{
NBDClient *client = req->client;
int ret;

g_assert(qemu_in_coroutine());

trace_nbd_co_send_reply(reply->handle, reply->error, len);

qemu_co_mutex_lock(&client->send_lock);
client->send_coroutine = qemu_coroutine_self();

if (!len) {
ret = nbd_send_reply(client->ioc, reply, errp);
} else {
qio_channel_set_cork(client->ioc, true);
ret = nbd_send_reply(client->ioc, reply, errp);
if (ret == 0) {
ret = nbd_write(client->ioc, req->data, len, errp);
if (ret < 0) {
ret = -EIO;
}
}
qio_channel_set_cork(client->ioc, false);
}
ret = qio_channel_writev_all(client->ioc, iov, niov, errp) < 0 ? -EIO : 0;

client->send_coroutine = NULL;
qemu_co_mutex_unlock(&client->send_lock);

return ret;
}

static inline void set_be_simple_reply(NBDSimpleReply *reply, uint64_t error,
uint64_t handle)
{
stl_be_p(&reply->magic, NBD_SIMPLE_REPLY_MAGIC);
stl_be_p(&reply->error, error);
stq_be_p(&reply->handle, handle);
}

static int nbd_co_send_simple_reply(NBDClient *client,
uint64_t handle,
uint32_t error,
void *data,
size_t len,
Error **errp)
{
NBDSimpleReply reply;
int nbd_err = system_errno_to_nbd_errno(error);
struct iovec iov[] = {
{.iov_base = &reply, .iov_len = sizeof(reply)},
{.iov_base = data, .iov_len = len}
};

trace_nbd_co_send_simple_reply(handle, nbd_err, len);
set_be_simple_reply(&reply, nbd_err, handle);

return nbd_co_send_iov(client, iov, len ? 2 : 1, errp);
}

/* nbd_co_receive_request
* Collect a client request. Return 0 if request looks valid, -EIO to drop
* connection right away, and any other negative value to report an error to
Expand Down Expand Up @@ -1331,7 +1324,6 @@ static coroutine_fn void nbd_trip(void *opaque)
NBDExport *exp = client->exp;
NBDRequestData *req;
NBDRequest request = { 0 }; /* GCC thinks it can be used uninitialized */
NBDReply reply;
int ret;
int flags;
int reply_data_len = 0;
Expand All @@ -1351,11 +1343,7 @@ static coroutine_fn void nbd_trip(void *opaque)
goto disconnect;
}

reply.handle = request.handle;
reply.error = 0;

if (ret < 0) {
reply.error = -ret;
goto reply;
}

Expand All @@ -1374,7 +1362,6 @@ static coroutine_fn void nbd_trip(void *opaque)
ret = blk_co_flush(exp->blk);
if (ret < 0) {
error_setg_errno(&local_err, -ret, "flush failed");
reply.error = -ret;
break;
}
}
Expand All @@ -1383,7 +1370,6 @@ static coroutine_fn void nbd_trip(void *opaque)
req->data, request.len);
if (ret < 0) {
error_setg_errno(&local_err, -ret, "reading from file failed");
reply.error = -ret;
break;
}

Expand All @@ -1392,7 +1378,7 @@ static coroutine_fn void nbd_trip(void *opaque)
break;
case NBD_CMD_WRITE:
if (exp->nbdflags & NBD_FLAG_READ_ONLY) {
reply.error = EROFS;
ret = -EROFS;
break;
}

Expand All @@ -1404,14 +1390,13 @@ static coroutine_fn void nbd_trip(void *opaque)
req->data, request.len, flags);
if (ret < 0) {
error_setg_errno(&local_err, -ret, "writing to file failed");
reply.error = -ret;
}

break;
case NBD_CMD_WRITE_ZEROES:
if (exp->nbdflags & NBD_FLAG_READ_ONLY) {
error_setg(&local_err, "Server is read-only, return error");
reply.error = EROFS;
ret = -EROFS;
break;
}

Expand All @@ -1426,7 +1411,6 @@ static coroutine_fn void nbd_trip(void *opaque)
request.len, flags);
if (ret < 0) {
error_setg_errno(&local_err, -ret, "writing to file failed");
reply.error = -ret;
}

break;
Expand All @@ -1438,7 +1422,6 @@ static coroutine_fn void nbd_trip(void *opaque)
ret = blk_co_flush(exp->blk);
if (ret < 0) {
error_setg_errno(&local_err, -ret, "flush failed");
reply.error = -ret;
}

break;
Expand All @@ -1447,25 +1430,27 @@ static coroutine_fn void nbd_trip(void *opaque)
request.len);
if (ret < 0) {
error_setg_errno(&local_err, -ret, "discard failed");
reply.error = -ret;
}

break;
default:
error_setg(&local_err, "invalid request type (%" PRIu32 ") received",
request.type);
reply.error = EINVAL;
ret = -EINVAL;
}

reply:
if (local_err) {
/* If we are here local_err is not fatal error, already stored in
* reply.error */
/* If we get here, local_err was not a fatal error, and should be sent
* to the client. */
error_report_err(local_err);
local_err = NULL;
}

if (nbd_co_send_reply(req, &reply, reply_data_len, &local_err) < 0) {
if (nbd_co_send_simple_reply(req->client, request.handle,
ret < 0 ? -ret : 0,
req->data, reply_data_len, &local_err) < 0)
{
error_prepend(&local_err, "Failed to send reply: ");
goto disconnect;
}
Expand Down Expand Up @@ -1539,7 +1524,7 @@ void nbd_client_new(NBDExport *exp,
NBDClient *client;
Coroutine *co;

client = g_malloc0(sizeof(NBDClient));
client = g_new0(NBDClient, 1);
client->refcount = 1;
client->exp = exp;
client->tlscreds = tlscreds;
Expand Down
3 changes: 1 addition & 2 deletions nbd/trace-events
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,9 @@ nbd_negotiate_old_style(uint64_t size, unsigned flags) "advertising size %" PRIu
nbd_negotiate_new_style_size_flags(uint64_t size, unsigned flags) "advertising size %" PRIu64 " and flags 0x%x"
nbd_negotiate_success(void) "Negotiation succeeded"
nbd_receive_request(uint32_t magic, uint16_t flags, uint16_t type, uint64_t from, uint32_t len) "Got request: { magic = 0x%" PRIx32 ", .flags = 0x%" PRIx16 ", .type = 0x%" PRIx16 ", from = %" PRIu64 ", len = %" PRIu32 " }"
nbd_send_reply(int32_t error, uint64_t handle) "Sending response to client: { .error = %" PRId32 ", handle = %" PRIu64 " }"
nbd_blk_aio_attached(const char *name, void *ctx) "Export %s: Attaching clients to AIO context %p\n"
nbd_blk_aio_detach(const char *name, void *ctx) "Export %s: Detaching clients from AIO context %p\n"
nbd_co_send_reply(uint64_t handle, uint32_t error, int len) "Send reply: handle = %" PRIu64 ", error = %" PRIu32 ", len = %d"
nbd_co_send_simple_reply(uint64_t handle, uint32_t error, int len) "Send simple reply: handle = %" PRIu64 ", error = %" PRIu32 ", len = %d"
nbd_co_receive_request_decode_type(uint64_t handle, uint16_t type, const char *name) "Decoding type: handle = %" PRIu64 ", type = %" PRIu16 " (%s)"
nbd_co_receive_request_payload_received(uint64_t handle, uint32_t len) "Payload received: handle = %" PRIu64 ", len = %" PRIu32
nbd_co_receive_request_cmd_write(uint32_t len) "Reading %" PRIu32 " byte(s)"
Expand Down
Loading

0 comments on commit 79b2a13

Please sign in to comment.