nbd patches for 2018-10-03

Fix bugs in NBD_CMD_CACHE, drop support for oldstyle NBD server,
 minor build and doc fixes
 
 - Denis V. Lunev: nbd: fix NBD_CMD_CACHE negitiation... [retitled]
 - Vladimir Sementsov-Ogievskiy: 0/2 server: drop old-style negotiation
 - Eric Blake: qemu-nbd: Document --tls-creds
 - Vladimir Sementsov-Ogievskiy: nbd/server: fix NBD_CMD_CACHE
 - Peter Maydell: nbd: Don't take address of fields in packed structs
 -----BEGIN PGP SIGNATURE-----
 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg
 
 iQEcBAABCAAGBQJbtiF0AAoJEKeha0olJ0NqqgsH/RKL+SPl8S4xslZ8pTcXL2OW
 AgMZInxKAe3MDX6NiCYFDWU4/32NNzPluWMh7DGu3mqFQiVySz9oAz2csZ4Fd5nm
 NfY6Ewu64HF/REN7dNLfyrbmfzD3PdCUAa2PsasH1wzKArTklyfJH/QeOHbJ60H8
 5AjthM/ksFNSAZXp+R+gzr2zGHN1cXwNBn87tKndPYaiIc95azPSZOnQpLLN9gfH
 YwxCKk+opeEJNMuXf/PdkksbxqNxHsBCYSAL9NzjzS/+T8GIpHQAZvKGSk6FKDp+
 x+KcC2pL7Lmu6iJWGIo5hGSSsBP1aXp0MJ/dUD+jo13LWNwzGFmovNI4pUuEK2A=
 =0s7A
 -----END PGP SIGNATURE-----

Merge remote-tracking branch 'remotes/ericb/tags/pull-nbd-2018-10-03-v2' into staging

nbd patches for 2018-10-03

Fix bugs in NBD_CMD_CACHE, drop support for oldstyle NBD server,
minor build and doc fixes

- Denis V. Lunev: nbd: fix NBD_CMD_CACHE negitiation... [retitled]
- Vladimir Sementsov-Ogievskiy: 0/2 server: drop old-style negotiation
- Eric Blake: qemu-nbd: Document --tls-creds
- Vladimir Sementsov-Ogievskiy: nbd/server: fix NBD_CMD_CACHE
- Peter Maydell: nbd: Don't take address of fields in packed structs

# gpg: Signature made Thu 04 Oct 2018 15:19:32 BST
# gpg:                using RSA key A7A16B4A2527436A
# gpg: Good signature from "Eric Blake <eblake@redhat.com>"
# gpg:                 aka "Eric Blake (Free Software Programmer) <ebb9@byu.net>"
# 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-2018-10-03-v2:
  nbd: fix NBD_FLAG_SEND_CACHE value
  nbd/server: drop old-style negotiation
  qemu-nbd: drop old-style negotiation
  qemu-nbd: Document --tls-creds
  nbd/server: fix NBD_CMD_CACHE
  nbd: Don't take address of fields in packed structs

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
This commit is contained in:
Peter Maydell 2018-10-05 14:52:54 +01:00
commit d21ee59ae5
5 changed files with 63 additions and 97 deletions

View File

@ -36,8 +36,7 @@ static void nbd_accept(QIONetListener *listener, QIOChannelSocket *cioc,
gpointer opaque)
{
qio_channel_set_name(QIO_CHANNEL(cioc), "nbd-server");
nbd_client_new(NULL, cioc,
nbd_server->tlscreds, NULL,
nbd_client_new(cioc, nbd_server->tlscreds, NULL,
nbd_blockdev_client_closed);
}

View File

@ -135,7 +135,9 @@ typedef struct NBDExtent {
#define NBD_FLAG_SEND_TRIM (1 << 5) /* Send TRIM (discard) */
#define NBD_FLAG_SEND_WRITE_ZEROES (1 << 6) /* Send WRITE_ZEROES */
#define NBD_FLAG_SEND_DF (1 << 7) /* Send DF (Do not Fragment) */
#define NBD_FLAG_SEND_CACHE (1 << 8) /* Send CACHE (prefetch) */
#define NBD_FLAG_CAN_MULTI_CONN (1 << 8) /* Multi-client cache consistent */
#define NBD_FLAG_SEND_RESIZE (1 << 9) /* Send resize */
#define NBD_FLAG_SEND_CACHE (1 << 10) /* Send CACHE (prefetch) */
/* New-style handshake (global) flags, sent from server to client, and
control what will happen during handshake phase. */
@ -308,8 +310,7 @@ void nbd_export_set_name(NBDExport *exp, const char *name);
void nbd_export_set_description(NBDExport *exp, const char *description);
void nbd_export_close_all(void);
void nbd_client_new(NBDExport *exp,
QIOChannelSocket *sioc,
void nbd_client_new(QIOChannelSocket *sioc,
QCryptoTLSCreds *tlscreds,
const char *tlsaclname,
void (*close_fn)(NBDClient *, bool));

View File

@ -117,10 +117,10 @@ static int nbd_receive_option_reply(QIOChannel *ioc, uint32_t opt,
nbd_send_opt_abort(ioc);
return -1;
}
be64_to_cpus(&reply->magic);
be32_to_cpus(&reply->option);
be32_to_cpus(&reply->type);
be32_to_cpus(&reply->length);
reply->magic = be64_to_cpu(reply->magic);
reply->option = be32_to_cpu(reply->option);
reply->type = be32_to_cpu(reply->type);
reply->length = be32_to_cpu(reply->length);
trace_nbd_receive_option_reply(reply->option, nbd_opt_lookup(reply->option),
reply->type, nbd_rep_lookup(reply->type),
@ -396,7 +396,7 @@ static int nbd_opt_go(QIOChannel *ioc, const char *wantname,
return -1;
}
len -= sizeof(type);
be16_to_cpus(&type);
type = be16_to_cpu(type);
switch (type) {
case NBD_INFO_EXPORT:
if (len != sizeof(info->size) + sizeof(info->flags)) {
@ -410,13 +410,13 @@ static int nbd_opt_go(QIOChannel *ioc, const char *wantname,
nbd_send_opt_abort(ioc);
return -1;
}
be64_to_cpus(&info->size);
info->size = be64_to_cpu(info->size);
if (nbd_read(ioc, &info->flags, sizeof(info->flags), errp) < 0) {
error_prepend(errp, "failed to read info flags: ");
nbd_send_opt_abort(ioc);
return -1;
}
be16_to_cpus(&info->flags);
info->flags = be16_to_cpu(info->flags);
trace_nbd_receive_negotiate_size_flags(info->size, info->flags);
break;
@ -433,7 +433,7 @@ static int nbd_opt_go(QIOChannel *ioc, const char *wantname,
nbd_send_opt_abort(ioc);
return -1;
}
be32_to_cpus(&info->min_block);
info->min_block = be32_to_cpu(info->min_block);
if (!is_power_of_2(info->min_block)) {
error_setg(errp, "server minimum block size %" PRIu32
" is not a power of two", info->min_block);
@ -447,7 +447,7 @@ static int nbd_opt_go(QIOChannel *ioc, const char *wantname,
nbd_send_opt_abort(ioc);
return -1;
}
be32_to_cpus(&info->opt_block);
info->opt_block = be32_to_cpu(info->opt_block);
if (!is_power_of_2(info->opt_block) ||
info->opt_block < info->min_block) {
error_setg(errp, "server preferred block size %" PRIu32
@ -461,7 +461,7 @@ static int nbd_opt_go(QIOChannel *ioc, const char *wantname,
nbd_send_opt_abort(ioc);
return -1;
}
be32_to_cpus(&info->max_block);
info->max_block = be32_to_cpu(info->max_block);
if (info->max_block < info->min_block) {
error_setg(errp, "server maximum block size %" PRIu32
" is not valid", info->max_block);
@ -668,7 +668,7 @@ static int nbd_negotiate_simple_meta_context(QIOChannel *ioc,
if (nbd_read(ioc, &received_id, sizeof(received_id), errp) < 0) {
return -1;
}
be32_to_cpus(&received_id);
received_id = be32_to_cpu(received_id);
reply.length -= sizeof(received_id);
name = g_malloc(reply.length + 1);
@ -872,13 +872,13 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name,
error_prepend(errp, "Failed to read export length: ");
goto fail;
}
be64_to_cpus(&info->size);
info->size = be64_to_cpu(info->size);
if (nbd_read(ioc, &info->flags, sizeof(info->flags), errp) < 0) {
error_prepend(errp, "Failed to read export flags: ");
goto fail;
}
be16_to_cpus(&info->flags);
info->flags = be16_to_cpu(info->flags);
} else if (magic == NBD_CLIENT_MAGIC) {
uint32_t oldflags;
@ -895,13 +895,13 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name,
error_prepend(errp, "Failed to read export length: ");
goto fail;
}
be64_to_cpus(&info->size);
info->size = be64_to_cpu(info->size);
if (nbd_read(ioc, &oldflags, sizeof(oldflags), errp) < 0) {
error_prepend(errp, "Failed to read export flags: ");
goto fail;
}
be32_to_cpus(&oldflags);
oldflags = be32_to_cpu(oldflags);
if (oldflags & ~0xffff) {
error_setg(errp, "Unexpected export flags %0x" PRIx32, oldflags);
goto fail;
@ -1080,8 +1080,8 @@ static int nbd_receive_simple_reply(QIOChannel *ioc, NBDSimpleReply *reply,
return ret;
}
be32_to_cpus(&reply->error);
be64_to_cpus(&reply->handle);
reply->error = be32_to_cpu(reply->error);
reply->handle = be64_to_cpu(reply->handle);
return 0;
}
@ -1105,10 +1105,10 @@ static int nbd_receive_structured_reply_chunk(QIOChannel *ioc,
return ret;
}
be16_to_cpus(&chunk->flags);
be16_to_cpus(&chunk->type);
be64_to_cpus(&chunk->handle);
be32_to_cpus(&chunk->length);
chunk->flags = be16_to_cpu(chunk->flags);
chunk->type = be16_to_cpu(chunk->type);
chunk->handle = be64_to_cpu(chunk->handle);
chunk->length = be32_to_cpu(chunk->length);
return 0;
}
@ -1128,7 +1128,7 @@ int nbd_receive_reply(QIOChannel *ioc, NBDReply *reply, Error **errp)
return ret;
}
be32_to_cpus(&reply->magic);
reply->magic = be32_to_cpu(reply->magic);
switch (reply->magic) {
case NBD_SIMPLE_REPLY_MAGIC:

View File

@ -333,7 +333,7 @@ static int nbd_opt_read_name(NBDClient *client, char *name, uint32_t *length,
if (ret <= 0) {
return ret;
}
cpu_to_be32s(&len);
len = cpu_to_be32(len);
if (len > NBD_MAX_NAME_SIZE) {
return nbd_opt_invalid(client, errp,
@ -486,7 +486,7 @@ static int nbd_negotiate_send_info(NBDClient *client,
if (rc < 0) {
return rc;
}
cpu_to_be16s(&info);
info = cpu_to_be16(info);
if (nbd_write(client->ioc, &info, sizeof(info), errp) < 0) {
return -EIO;
}
@ -551,14 +551,14 @@ static int nbd_negotiate_handle_info(NBDClient *client, uint16_t myflags,
if (rc <= 0) {
return rc;
}
be16_to_cpus(&requests);
requests = be16_to_cpu(requests);
trace_nbd_negotiate_handle_info_requests(requests);
while (requests--) {
rc = nbd_opt_read(client, &request, sizeof(request), errp);
if (rc <= 0) {
return rc;
}
be16_to_cpus(&request);
request = be16_to_cpu(request);
trace_nbd_negotiate_handle_info_request(request,
nbd_info_lookup(request));
/* We care about NBD_INFO_NAME and NBD_INFO_BLOCK_SIZE;
@ -618,9 +618,9 @@ static int nbd_negotiate_handle_info(NBDClient *client, uint16_t myflags,
/* maximum - At most 32M, but smaller as appropriate. */
sizes[2] = MIN(blk_get_max_transfer(exp->blk), NBD_MAX_BUFFER_SIZE);
trace_nbd_negotiate_handle_info_block_size(sizes[0], sizes[1], sizes[2]);
cpu_to_be32s(&sizes[0]);
cpu_to_be32s(&sizes[1]);
cpu_to_be32s(&sizes[2]);
sizes[0] = cpu_to_be32(sizes[0]);
sizes[1] = cpu_to_be32(sizes[1]);
sizes[2] = cpu_to_be32(sizes[2]);
rc = nbd_negotiate_send_info(client, NBD_INFO_BLOCK_SIZE,
sizeof(sizes), sizes, errp);
if (rc < 0) {
@ -904,7 +904,7 @@ static int nbd_negotiate_meta_query(NBDClient *client,
if (ret <= 0) {
return ret;
}
cpu_to_be32s(&len);
len = cpu_to_be32(len);
if (len < ns_len) {
trace_nbd_negotiate_meta_query_skip("length too short");
@ -971,7 +971,7 @@ static int nbd_negotiate_meta_queries(NBDClient *client,
if (ret <= 0) {
return ret;
}
cpu_to_be32s(&nb_queries);
nb_queries = cpu_to_be32(nb_queries);
trace_nbd_negotiate_meta_context(nbd_opt_lookup(client->opt),
export_name, nb_queries);
@ -1049,7 +1049,7 @@ static int nbd_negotiate_options(NBDClient *client, uint16_t myflags,
error_prepend(errp, "read failed: ");
return -EIO;
}
be32_to_cpus(&flags);
flags = be32_to_cpu(flags);
trace_nbd_negotiate_options_flags(flags);
if (flags & NBD_FLAG_C_FIXED_NEWSTYLE) {
fixedNewstyle = true;
@ -1253,7 +1253,6 @@ static coroutine_fn int nbd_negotiate(NBDClient *client, Error **errp)
const uint16_t myflags = (NBD_FLAG_HAS_FLAGS | NBD_FLAG_SEND_TRIM |
NBD_FLAG_SEND_FLUSH | NBD_FLAG_SEND_FUA |
NBD_FLAG_SEND_WRITE_ZEROES | NBD_FLAG_SEND_CACHE);
bool oldStyle;
/* Old style negotiation header, no room for options
[ 0 .. 7] passwd ("NBDMAGIC")
@ -1274,33 +1273,19 @@ static coroutine_fn int nbd_negotiate(NBDClient *client, Error **errp)
trace_nbd_negotiate_begin();
memcpy(buf, "NBDMAGIC", 8);
oldStyle = client->exp != NULL && !client->tlscreds;
if (oldStyle) {
trace_nbd_negotiate_old_style(client->exp->size,
client->exp->nbdflags | myflags);
stq_be_p(buf + 8, NBD_CLIENT_MAGIC);
stq_be_p(buf + 16, client->exp->size);
stl_be_p(buf + 24, client->exp->nbdflags | myflags);
stq_be_p(buf + 8, NBD_OPTS_MAGIC);
stw_be_p(buf + 16, NBD_FLAG_FIXED_NEWSTYLE | NBD_FLAG_NO_ZEROES);
if (nbd_write(client->ioc, buf, sizeof(buf), errp) < 0) {
error_prepend(errp, "write failed: ");
return -EINVAL;
}
} else {
stq_be_p(buf + 8, NBD_OPTS_MAGIC);
stw_be_p(buf + 16, NBD_FLAG_FIXED_NEWSTYLE | NBD_FLAG_NO_ZEROES);
if (nbd_write(client->ioc, buf, 18, errp) < 0) {
error_prepend(errp, "write failed: ");
return -EINVAL;
}
ret = nbd_negotiate_options(client, myflags, errp);
if (ret != 0) {
if (ret < 0) {
error_prepend(errp, "option negotiation failed: ");
}
return ret;
if (nbd_write(client->ioc, buf, 18, errp) < 0) {
error_prepend(errp, "write failed: ");
return -EINVAL;
}
ret = nbd_negotiate_options(client, myflags, errp);
if (ret != 0) {
if (ret < 0) {
error_prepend(errp, "option negotiation failed: ");
}
return ret;
}
assert(!client->optlen);
@ -1900,8 +1885,8 @@ static int blockstatus_to_extents(BlockDriverState *bs, uint64_t offset,
extents_end = extent + 1;
for (extent = extents; extent < extents_end; extent++) {
cpu_to_be32s(&extent->flags);
cpu_to_be32s(&extent->length);
extent->flags = cpu_to_be32(extent->flags);
extent->length = cpu_to_be32(extent->length);
}
*bytes -= remaining_bytes;
@ -2177,7 +2162,8 @@ static coroutine_fn int nbd_do_cmd_read(NBDClient *client, NBDRequest *request,
}
if (client->structured_reply && !(request->flags & NBD_CMD_FLAG_DF) &&
request->len) {
request->len && request->type != NBD_CMD_CACHE)
{
return nbd_co_send_sparse_read(client, request->handle, request->from,
data, request->len, errp);
}
@ -2395,13 +2381,8 @@ static void nbd_client_receive_next_request(NBDClient *client)
static coroutine_fn void nbd_co_client_start(void *opaque)
{
NBDClient *client = opaque;
NBDExport *exp = client->exp;
Error *local_err = NULL;
if (exp) {
nbd_export_get(exp);
QTAILQ_INSERT_TAIL(&exp->clients, client, next);
}
qemu_co_mutex_init(&client->send_lock);
if (nbd_negotiate(client, &local_err)) {
@ -2416,13 +2397,11 @@ static coroutine_fn void nbd_co_client_start(void *opaque)
}
/*
* Create a new client listener on the given export @exp, using the
* given channel @sioc. Begin servicing it in a coroutine. When the
* connection closes, call @close_fn with an indication of whether the
* client completed negotiation.
* Create a new client listener using the given channel @sioc.
* Begin servicing it in a coroutine. When the connection closes, call
* @close_fn with an indication of whether the client completed negotiation.
*/
void nbd_client_new(NBDExport *exp,
QIOChannelSocket *sioc,
void nbd_client_new(QIOChannelSocket *sioc,
QCryptoTLSCreds *tlscreds,
const char *tlsaclname,
void (*close_fn)(NBDClient *, bool))
@ -2432,7 +2411,6 @@ void nbd_client_new(NBDExport *exp,
client = g_new0(NBDClient, 1);
client->refcount = 1;
client->exp = exp;
client->tlscreds = tlscreds;
if (tlscreds) {
object_ref(OBJECT(client->tlscreds));

View File

@ -56,7 +56,6 @@
#define MBR_SIZE 512
static NBDExport *exp;
static bool newproto;
static int verbose;
static char *srcpath;
static SocketAddress *saddr;
@ -84,8 +83,8 @@ static void usage(const char *name)
" -e, --shared=NUM device can be shared by NUM clients (default '1')\n"
" -t, --persistent don't exit on the last connection\n"
" -v, --verbose display extra debugging information\n"
" -x, --export-name=NAME expose export by name\n"
" -D, --description=TEXT with -x, also export a human-readable description\n"
" -x, --export-name=NAME expose export by name (default is empty string)\n"
" -D, --description=TEXT export a human-readable description\n"
"\n"
"Exposing part of the image:\n"
" -o, --offset=OFFSET offset into the image\n"
@ -94,6 +93,7 @@ static void usage(const char *name)
"General purpose options:\n"
" --object type,id=ID,... define an object such as 'secret' for providing\n"
" passwords and/or encryption keys\n"
" --tls-creds=ID use id of an earlier --object to provide TLS\n"
" -T, --trace [[enable=]<pattern>][,events=<file>][,file=<file>]\n"
" specify tracing options\n"
" --fork fork off the server process and exit the parent\n"
@ -354,8 +354,7 @@ static void nbd_accept(QIONetListener *listener, QIOChannelSocket *cioc,
nb_fds++;
nbd_update_server_watch();
nbd_client_new(newproto ? NULL : exp, cioc,
tlscreds, NULL, nbd_client_closed);
nbd_client_new(cioc, tlscreds, NULL, nbd_client_closed);
}
static void nbd_update_server_watch(void)
@ -549,7 +548,7 @@ int main(int argc, char **argv)
Error *local_err = NULL;
BlockdevDetectZeroesOptions detect_zeroes = BLOCKDEV_DETECT_ZEROES_OPTIONS_OFF;
QDict *options = NULL;
const char *export_name = NULL;
const char *export_name = ""; /* Default export name */
const char *export_description = NULL;
const char *tlscredsid = NULL;
bool imageOpts = false;
@ -808,11 +807,6 @@ int main(int argc, char **argv)
error_report("TLS is not supported with a host device");
exit(EXIT_FAILURE);
}
if (!export_name) {
/* Set the default NBD protocol export name, since
* we *must* use new style protocol for TLS */
export_name = "";
}
tlscreds = nbd_get_tls_creds(tlscredsid, &local_err);
if (local_err) {
error_report("Failed to get TLS creds %s",
@ -1013,14 +1007,8 @@ int main(int argc, char **argv)
error_report_err(local_err);
exit(EXIT_FAILURE);
}
if (export_name) {
nbd_export_set_name(exp, export_name);
nbd_export_set_description(exp, export_description);
newproto = true;
} else if (export_description) {
error_report("Export description requires an export name");
exit(EXIT_FAILURE);
}
nbd_export_set_name(exp, export_name);
nbd_export_set_description(exp, export_description);
if (device) {
int ret;