xemu-project/xemu
1
0
Fork 0
mirror of https://github.com/xemu-project/xemu.git synced 2024-12-05 02:06:40 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

nbd: generalize usage of nbd_read

Browse Source 
We generally do very similar things around nbd_read: error_prepend
specifying what we have tried to read, and be_to_cpu conversion of
integers.

So, it seems reasonable to move common things to helper functions,
which:
1. simplify code a bit
2. generalize nbd_read error descriptions, all starting with
   "Failed to read"
3. make it more difficult to forget to convert things from BE

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20190128165830.165170-1-vsementsov@virtuozzo.com>
[eblake: rename macro to DEF_NBD_READ_N and formatting tweaks;
checkpatch has false positive complaint]
Signed-off-by: Eric Blake <eblake@redhat.com>
This commit is contained in:
Vladimir Sementsov-Ogievskiy 2019-01-28 19:58:30 +03:00 committed by Eric Blake
parent 0ae2d54645
commit e6798f06a6
5 changed files with 71 additions and 83 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
block/nbd-client.c
View File

@ -338,10 +338,9 @@ static int nbd_co_receive_offset_data_payload(NBDClientSession *s,
return -EINVAL; return -EINVAL;
} }
if (nbd_read(s->ioc, &offset, sizeof(offset), errp) < 0) { if (nbd_read64(s->ioc, &offset, "OFFSET_DATA offset", errp) < 0) {
return -EIO; return -EIO;
} }
be64_to_cpus(&offset);
data_size = chunk->length - sizeof(offset); data_size = chunk->length - sizeof(offset);
assert(data_size); assert(data_size);
@ -388,7 +387,7 @@ static coroutine_fn int nbd_co_receive_structured_payload(
} }
*payload = g_new(char, len); *payload = g_new(char, len);
ret = nbd_read(s->ioc, *payload, len, errp); ret = nbd_read(s->ioc, *payload, len, "structured payload", errp);
if (ret < 0) { if (ret < 0) {
g_free(*payload); g_free(*payload);
*payload = NULL; *payload = NULL;

32
include/block/nbd.h
View File

@ -23,6 +23,7 @@
#include "qapi/qapi-types-block.h" #include "qapi/qapi-types-block.h"
#include "io/channel-socket.h" #include "io/channel-socket.h"
#include "crypto/tlscreds.h" #include "crypto/tlscreds.h"
#include "qapi/error.h"
/* Handshake phase structs - this struct is passed on the wire */ /* Handshake phase structs - this struct is passed on the wire */
@ -336,11 +337,38 @@ void nbd_server_start(SocketAddress *addr, const char *tls_creds,
* Reads @size bytes from @ioc. Returns 0 on success. * Reads @size bytes from @ioc. Returns 0 on success.
*/ */
static inline int nbd_read(QIOChannel *ioc, void *buffer, size_t size, static inline int nbd_read(QIOChannel *ioc, void *buffer, size_t size,
Error **errp) const char *desc, Error **errp)
{ {
return qio_channel_read_all(ioc, buffer, size, errp) < 0 ? -EIO : 0; int ret = qio_channel_read_all(ioc, buffer, size, errp) < 0 ? -EIO : 0;
if (ret < 0) {
if (desc) {
error_prepend(errp, "Failed to read %s: ", desc);
}
return -1;
}
return 0;
} }
#define DEF_NBD_READ_N(bits) \
static inline int nbd_read##bits(QIOChannel *ioc, \
uint##bits##_t *val, \
const char *desc, Error **errp) \
{ \
if (nbd_read(ioc, val, sizeof(*val), desc, errp) < 0) { \
return -1; \
} \
*val = be##bits##_to_cpu(*val); \
return 0; \
}
DEF_NBD_READ_N(16) /* Defines nbd_read16(). */
DEF_NBD_READ_N(32) /* Defines nbd_read32(). */
DEF_NBD_READ_N(64) /* Defines nbd_read64(). */
#undef DEF_NBD_READ_N
static inline bool nbd_reply_is_simple(NBDReply *reply) static inline bool nbd_reply_is_simple(NBDReply *reply)
{ {
return reply->magic == NBD_SIMPLE_REPLY_MAGIC; return reply->magic == NBD_SIMPLE_REPLY_MAGIC;

88
nbd/client.c
View File

@ -113,8 +113,7 @@ static int nbd_receive_option_reply(QIOChannel *ioc, uint32_t opt,
NBDOptionReply *reply, Error **errp) NBDOptionReply *reply, Error **errp)
{ {
QEMU_BUILD_BUG_ON(sizeof(*reply) != 20); QEMU_BUILD_BUG_ON(sizeof(*reply) != 20);
if (nbd_read(ioc, reply, sizeof(*reply), errp) < 0) { if (nbd_read(ioc, reply, sizeof(*reply), "option reply", errp) < 0) {
error_prepend(errp, "failed to read option reply: ");
nbd_send_opt_abort(ioc); nbd_send_opt_abort(ioc);
return -1; return -1;
} }
@ -166,8 +165,8 @@ static int nbd_handle_reply_err(QIOChannel *ioc, NBDOptionReply *reply,
goto cleanup; goto cleanup;
} }
msg = g_malloc(reply->length + 1); msg = g_malloc(reply->length + 1);
if (nbd_read(ioc, msg, reply->length, errp) < 0) { if (nbd_read(ioc, msg, reply->length, NULL, errp) < 0) {
error_prepend(errp, "failed to read option error %" PRIu32 error_prepend(errp, "Failed to read option error %" PRIu32
" (%s) message: ", " (%s) message: ",
reply->type, nbd_rep_lookup(reply->type)); reply->type, nbd_rep_lookup(reply->type));
goto cleanup; goto cleanup;
@ -284,12 +283,10 @@ static int nbd_receive_list(QIOChannel *ioc, char **name, char **description,
nbd_send_opt_abort(ioc); nbd_send_opt_abort(ioc);
return -1; return -1;
} }
if (nbd_read(ioc, &namelen, sizeof(namelen), errp) < 0) { if (nbd_read32(ioc, &namelen, "option name length", errp) < 0) {
error_prepend(errp, "failed to read option name length: ");
nbd_send_opt_abort(ioc); nbd_send_opt_abort(ioc);
return -1; return -1;
} }
namelen = be32_to_cpu(namelen);
len -= sizeof(namelen); len -= sizeof(namelen);
if (len < namelen) { if (len < namelen) {
error_setg(errp, "incorrect option name length"); error_setg(errp, "incorrect option name length");
@ -298,8 +295,7 @@ static int nbd_receive_list(QIOChannel *ioc, char **name, char **description,
} }
local_name = g_malloc(namelen + 1); local_name = g_malloc(namelen + 1);
if (nbd_read(ioc, local_name, namelen, errp) < 0) { if (nbd_read(ioc, local_name, namelen, "export name", errp) < 0) {
error_prepend(errp, "failed to read export name: ");
nbd_send_opt_abort(ioc); nbd_send_opt_abort(ioc);
goto out; goto out;
} }
@ -307,8 +303,7 @@ static int nbd_receive_list(QIOChannel *ioc, char **name, char **description,
len -= namelen; len -= namelen;
if (len) { if (len) {
local_desc = g_malloc(len + 1); local_desc = g_malloc(len + 1);
if (nbd_read(ioc, local_desc, len, errp) < 0) { if (nbd_read(ioc, local_desc, len, "export description", errp) < 0) {
error_prepend(errp, "failed to read export description: ");
nbd_send_opt_abort(ioc); nbd_send_opt_abort(ioc);
goto out; goto out;
} }
@ -410,13 +405,11 @@ static int nbd_opt_info_or_go(QIOChannel *ioc, uint32_t opt,
nbd_send_opt_abort(ioc); nbd_send_opt_abort(ioc);
return -1; return -1;
} }
if (nbd_read(ioc, &type, sizeof(type), errp) < 0) { if (nbd_read16(ioc, &type, "info type", errp) < 0) {
error_prepend(errp, "failed to read info type: ");
nbd_send_opt_abort(ioc); nbd_send_opt_abort(ioc);
return -1; return -1;
} }
len -= sizeof(type); len -= sizeof(type);
type = be16_to_cpu(type);
switch (type) { switch (type) {
case NBD_INFO_EXPORT: case NBD_INFO_EXPORT:
if (len != sizeof(info->size) + sizeof(info->flags)) { if (len != sizeof(info->size) + sizeof(info->flags)) {
@ -425,18 +418,14 @@ static int nbd_opt_info_or_go(QIOChannel *ioc, uint32_t opt,
nbd_send_opt_abort(ioc); nbd_send_opt_abort(ioc);
return -1; return -1;
} }
if (nbd_read(ioc, &info->size, sizeof(info->size), errp) < 0) { if (nbd_read64(ioc, &info->size, "info size", errp) < 0) {
error_prepend(errp, "failed to read info size: ");
nbd_send_opt_abort(ioc); nbd_send_opt_abort(ioc);
return -1; return -1;
} }
info->size = be64_to_cpu(info->size); if (nbd_read16(ioc, &info->flags, "info flags", errp) < 0) {
if (nbd_read(ioc, &info->flags, sizeof(info->flags), errp) < 0) {
error_prepend(errp, "failed to read info flags: ");
nbd_send_opt_abort(ioc); nbd_send_opt_abort(ioc);
return -1; return -1;
} }
info->flags = be16_to_cpu(info->flags);
trace_nbd_receive_negotiate_size_flags(info->size, info->flags); trace_nbd_receive_negotiate_size_flags(info->size, info->flags);
break; break;
@ -447,27 +436,23 @@ static int nbd_opt_info_or_go(QIOChannel *ioc, uint32_t opt,
nbd_send_opt_abort(ioc); nbd_send_opt_abort(ioc);
return -1; return -1;
} }
if (nbd_read(ioc, &info->min_block, sizeof(info->min_block), if (nbd_read32(ioc, &info->min_block, "info minimum block size",
errp) < 0) { errp) < 0) {
error_prepend(errp, "failed to read info minimum block size: ");
nbd_send_opt_abort(ioc); nbd_send_opt_abort(ioc);
return -1; return -1;
} }
info->min_block = be32_to_cpu(info->min_block);
if (!is_power_of_2(info->min_block)) { if (!is_power_of_2(info->min_block)) {
error_setg(errp, "server minimum block size %" PRIu32 error_setg(errp, "server minimum block size %" PRIu32
" is not a power of two", info->min_block); " is not a power of two", info->min_block);
nbd_send_opt_abort(ioc); nbd_send_opt_abort(ioc);
return -1; return -1;
} }
if (nbd_read(ioc, &info->opt_block, sizeof(info->opt_block), if (nbd_read32(ioc, &info->opt_block, "info preferred block size",
errp) < 0) { errp) < 0)
error_prepend(errp, {
"failed to read info preferred block size: ");
nbd_send_opt_abort(ioc); nbd_send_opt_abort(ioc);
return -1; return -1;
} }
info->opt_block = be32_to_cpu(info->opt_block);
if (!is_power_of_2(info->opt_block) || if (!is_power_of_2(info->opt_block) ||
info->opt_block < info->min_block) { info->opt_block < info->min_block) {
error_setg(errp, "server preferred block size %" PRIu32 error_setg(errp, "server preferred block size %" PRIu32
@ -475,13 +460,12 @@ static int nbd_opt_info_or_go(QIOChannel *ioc, uint32_t opt,
nbd_send_opt_abort(ioc); nbd_send_opt_abort(ioc);
return -1; return -1;
} }
if (nbd_read(ioc, &info->max_block, sizeof(info->max_block), if (nbd_read32(ioc, &info->max_block, "info maximum block size",
errp) < 0) { errp) < 0)
error_prepend(errp, "failed to read info maximum block size: "); {
nbd_send_opt_abort(ioc); nbd_send_opt_abort(ioc);
return -1; return -1;
} }
info->max_block = be32_to_cpu(info->max_block);
if (info->max_block < info->min_block) { if (info->max_block < info->min_block) {
error_setg(errp, "server maximum block size %" PRIu32 error_setg(errp, "server maximum block size %" PRIu32
" is not valid", info->max_block); " is not valid", info->max_block);
@ -731,14 +715,13 @@ static int nbd_receive_one_meta_context(QIOChannel *ioc,
return -1; return -1;
} }
if (nbd_read(ioc, &local_id, sizeof(local_id), errp) < 0) { if (nbd_read32(ioc, &local_id, "context id", errp) < 0) {
return -1; return -1;
} }
local_id = be32_to_cpu(local_id);
reply.length -= sizeof(local_id); reply.length -= sizeof(local_id);
local_name = g_malloc(reply.length + 1); local_name = g_malloc(reply.length + 1);
if (nbd_read(ioc, local_name, reply.length, errp) < 0) { if (nbd_read(ioc, local_name, reply.length, "context name", errp) < 0) {
g_free(local_name); g_free(local_name);
return -1; return -1;
} }
@ -896,11 +879,9 @@ static int nbd_start_negotiate(QIOChannel *ioc, QCryptoTLSCreds *tlscreds,
return -EINVAL; return -EINVAL;
} }
if (nbd_read(ioc, &magic, sizeof(magic), errp) < 0) { if (nbd_read64(ioc, &magic, "initial magic", errp) < 0) {
error_prepend(errp, "Failed to read initial magic: ");
return -EINVAL; return -EINVAL;
} }
magic = be64_to_cpu(magic);
trace_nbd_receive_negotiate_magic(magic); trace_nbd_receive_negotiate_magic(magic);
if (magic != NBD_INIT_MAGIC) { if (magic != NBD_INIT_MAGIC) {
@ -908,11 +889,9 @@ static int nbd_start_negotiate(QIOChannel *ioc, QCryptoTLSCreds *tlscreds,
return -EINVAL; return -EINVAL;
} }
if (nbd_read(ioc, &magic, sizeof(magic), errp) < 0) { if (nbd_read64(ioc, &magic, "server magic", errp) < 0) {
error_prepend(errp, "Failed to read server magic: ");
return -EINVAL; return -EINVAL;
} }
magic = be64_to_cpu(magic);
trace_nbd_receive_negotiate_magic(magic); trace_nbd_receive_negotiate_magic(magic);
if (magic == NBD_OPTS_MAGIC) { if (magic == NBD_OPTS_MAGIC) {
@ -920,11 +899,9 @@ static int nbd_start_negotiate(QIOChannel *ioc, QCryptoTLSCreds *tlscreds,
uint16_t globalflags; uint16_t globalflags;
bool fixedNewStyle = false; bool fixedNewStyle = false;
if (nbd_read(ioc, &globalflags, sizeof(globalflags), errp) < 0) { if (nbd_read16(ioc, &globalflags, "server flags", errp) < 0) {
error_prepend(errp, "Failed to read server flags: ");
return -EINVAL; return -EINVAL;
} }
globalflags = be16_to_cpu(globalflags);
trace_nbd_receive_negotiate_server_flags(globalflags); trace_nbd_receive_negotiate_server_flags(globalflags);
if (globalflags & NBD_FLAG_FIXED_NEWSTYLE) { if (globalflags & NBD_FLAG_FIXED_NEWSTYLE) {
fixedNewStyle = true; fixedNewStyle = true;
@ -992,17 +969,13 @@ static int nbd_negotiate_finish_oldstyle(QIOChannel *ioc, NBDExportInfo *info,
{ {
uint32_t oldflags; uint32_t oldflags;
if (nbd_read(ioc, &info->size, sizeof(info->size), errp) < 0) { if (nbd_read64(ioc, &info->size, "export length", errp) < 0) {
error_prepend(errp, "Failed to read export length: ");
return -EINVAL; return -EINVAL;
} }
info->size = be64_to_cpu(info->size);
if (nbd_read(ioc, &oldflags, sizeof(oldflags), errp) < 0) { if (nbd_read32(ioc, &oldflags, "export flags", errp) < 0) {
error_prepend(errp, "Failed to read export flags: ");
return -EINVAL; return -EINVAL;
} }
oldflags = be32_to_cpu(oldflags);
if (oldflags & ~0xffff) { if (oldflags & ~0xffff) {
error_setg(errp, "Unexpected export flags %0x" PRIx32, oldflags); error_setg(errp, "Unexpected export flags %0x" PRIx32, oldflags);
return -EINVAL; return -EINVAL;
@ -1079,17 +1052,13 @@ int nbd_receive_negotiate(QIOChannel *ioc, QCryptoTLSCreds *tlscreds,
} }
/* Read the response */ /* Read the response */
if (nbd_read(ioc, &info->size, sizeof(info->size), errp) < 0) { if (nbd_read64(ioc, &info->size, "export length", errp) < 0) {
error_prepend(errp, "Failed to read export length: ");
return -EINVAL; return -EINVAL;
} }
info->size = be64_to_cpu(info->size);
if (nbd_read(ioc, &info->flags, sizeof(info->flags), errp) < 0) { if (nbd_read16(ioc, &info->flags, "export flags", errp) < 0) {
error_prepend(errp, "Failed to read export flags: ");
return -EINVAL; return -EINVAL;
} }
info->flags = be16_to_cpu(info->flags);
break; break;
case 0: /* oldstyle, parse length and flags */ case 0: /* oldstyle, parse length and flags */
if (*info->name) { if (*info->name) {
@ -1379,7 +1348,7 @@ static int nbd_receive_simple_reply(QIOChannel *ioc, NBDSimpleReply *reply,
assert(reply->magic == NBD_SIMPLE_REPLY_MAGIC); assert(reply->magic == NBD_SIMPLE_REPLY_MAGIC);
ret = nbd_read(ioc, (uint8_t *)reply + sizeof(reply->magic), ret = nbd_read(ioc, (uint8_t *)reply + sizeof(reply->magic),
sizeof(*reply) - sizeof(reply->magic), errp); sizeof(*reply) - sizeof(reply->magic), "reply", errp);
if (ret < 0) { if (ret < 0) {
return ret; return ret;
} }
@ -1404,7 +1373,8 @@ static int nbd_receive_structured_reply_chunk(QIOChannel *ioc,
assert(chunk->magic == NBD_STRUCTURED_REPLY_MAGIC); assert(chunk->magic == NBD_STRUCTURED_REPLY_MAGIC);
ret = nbd_read(ioc, (uint8_t *)chunk + sizeof(chunk->magic), ret = nbd_read(ioc, (uint8_t *)chunk + sizeof(chunk->magic),
sizeof(*chunk) - sizeof(chunk->magic), errp); sizeof(*chunk) - sizeof(chunk->magic), "structured chunk",
errp);
if (ret < 0) { if (ret < 0) {
return ret; return ret;
} }

2
nbd/common.c
View File

@ -31,7 +31,7 @@ int nbd_drop(QIOChannel *ioc, size_t size, Error **errp)
buffer = sizeof(small) >= size ? small : g_malloc(MIN(65536, size)); buffer = sizeof(small) >= size ? small : g_malloc(MIN(65536, size));
while (size > 0) { while (size > 0) {
ssize_t count = MIN(65536, size); ssize_t count = MIN(65536, size);
ret = nbd_read(ioc, buffer, MIN(65536, size), errp); ret = nbd_read(ioc, buffer, MIN(65536, size), NULL, errp);
if (ret < 0) { if (ret < 0) {
goto cleanup; goto cleanup;

27
nbd/server.c
View File

@ -438,8 +438,7 @@ static int nbd_negotiate_handle_export_name(NBDClient *client,
error_setg(errp, "Bad length received"); error_setg(errp, "Bad length received");
return -EINVAL; return -EINVAL;
} }
if (nbd_read(client->ioc, name, client->optlen, errp) < 0) { if (nbd_read(client->ioc, name, client->optlen, "export name", errp) < 0) {
error_prepend(errp, "read failed: ");
return -EIO; return -EIO;
} }
name[client->optlen] = '\0'; name[client->optlen] = '\0';
@ -1046,11 +1045,9 @@ static int nbd_negotiate_options(NBDClient *client, uint16_t myflags,
... Rest of request ... Rest of request
*/ */
if (nbd_read(client->ioc, &flags, sizeof(flags), errp) < 0) { if (nbd_read32(client->ioc, &flags, "flags", errp) < 0) {
error_prepend(errp, "read failed: ");
return -EIO; return -EIO;
} }
flags = be32_to_cpu(flags);
trace_nbd_negotiate_options_flags(flags); trace_nbd_negotiate_options_flags(flags);
if (flags & NBD_FLAG_C_FIXED_NEWSTYLE) { if (flags & NBD_FLAG_C_FIXED_NEWSTYLE) {
fixedNewstyle = true; fixedNewstyle = true;
@ -1070,30 +1067,23 @@ static int nbd_negotiate_options(NBDClient *client, uint16_t myflags,
uint32_t option, length; uint32_t option, length;
uint64_t magic; uint64_t magic;
if (nbd_read(client->ioc, &magic, sizeof(magic), errp) < 0) { if (nbd_read64(client->ioc, &magic, "opts magic", errp) < 0) {
error_prepend(errp, "read failed: ");
return -EINVAL; return -EINVAL;
} }
magic = be64_to_cpu(magic);
trace_nbd_negotiate_options_check_magic(magic); trace_nbd_negotiate_options_check_magic(magic);
if (magic != NBD_OPTS_MAGIC) { if (magic != NBD_OPTS_MAGIC) {
error_setg(errp, "Bad magic received"); error_setg(errp, "Bad magic received");
return -EINVAL; return -EINVAL;
} }
if (nbd_read(client->ioc, &option, if (nbd_read32(client->ioc, &option, "option", errp) < 0) {
sizeof(option), errp) < 0) {
error_prepend(errp, "read failed: ");
return -EINVAL; return -EINVAL;
} }
option = be32_to_cpu(option);
client->opt = option; client->opt = option;
if (nbd_read(client->ioc, &length, sizeof(length), errp) < 0) { if (nbd_read32(client->ioc, &length, "option length", errp) < 0) {
error_prepend(errp, "read failed: ");
return -EINVAL; return -EINVAL;
} }
length = be32_to_cpu(length);
assert(!client->optlen); assert(!client->optlen);
client->optlen = length; client->optlen = length;
@ -1306,7 +1296,7 @@ static int nbd_receive_request(QIOChannel *ioc, NBDRequest *request,
uint32_t magic; uint32_t magic;
int ret; int ret;
ret = nbd_read(ioc, buf, sizeof(buf), errp); ret = nbd_read(ioc, buf, sizeof(buf), "request", errp);
if (ret < 0) { if (ret < 0) {
return ret; return ret;
} }
@ -2111,8 +2101,9 @@ static int nbd_co_receive_request(NBDRequestData *req, NBDRequest *request,
} }
} }
if (request->type == NBD_CMD_WRITE) { if (request->type == NBD_CMD_WRITE) {
if (nbd_read(client->ioc, req->data, request->len, errp) < 0) { if (nbd_read(client->ioc, req->data, request->len, "CMD_WRITE data",
error_prepend(errp, "reading from socket failed: "); errp) < 0)
{
return -EIO; return -EIO;
} }
req->complete = true; req->complete = true;