Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Merge tag 'pull-nbd-2023-07-19' of https://repo.or.cz/qemu/ericb into…
… staging

NBD patches through 2023-07-19

- Denis V. Lunev: fix hang with 'ssh ... "qemu-nbd -c"'
- Eric Blake: preliminary work towards NBD 64-bit extensions

# -----BEGIN PGP SIGNATURE-----
#
# iQEzBAABCAAdFiEEccLMIrHEYCkn0vOqp6FrSiUnQ2oFAmS4RwcACgkQp6FrSiUn
# Q2pXfQf/clnttPdw9BW2cJltFRKeMeZrgn8mut0S7jhC0DWIy6zanzp07MylryHP
# EyJ++dCbLEg8mueThL/n5mKsTS/OECtfZO9Ot11WmZqDZVtLKorfmy7YVI3VwMjI
# yQqrUIwiYxzZOkPban/MXofY6vJmuia5aGkEmYUyKiHvsLF3Hk2gHPB/qa2S+U6I
# QDmC032/L+/LgVkK5r/1vamwJNP29QI4DNp3RiTtcMK5sEZJfMsAZSxFDDdH2pqi
# 5gyVqw0zNl3vz6znoVy0XZ/8OUVloPKHswyf7xLlBukY1GL5D+aiXz2ilwBvk9aM
# SoZzYvaOOBDyJhSjapOvseTqXTNeqQ==
# =TB9t
# -----END PGP SIGNATURE-----
# gpg: Signature made Wed 19 Jul 2023 21:26:47 BST
# gpg:                using RSA key 71C2CC22B1C4602927D2F3AAA7A16B4A2527436A
# gpg: Good signature from "Eric Blake <eblake@redhat.com>" [full]
# gpg:                 aka "Eric Blake (Free Software Programmer) <ebb9@byu.net>" [full]
# gpg:                 aka "[jpeg image of size 6874]" [full]
# Primary key fingerprint: 71C2 CC22 B1C4 6029 27D2  F3AA A7A1 6B4A 2527 436A

* tag 'pull-nbd-2023-07-19' of https://repo.or.cz/qemu/ericb:
  nbd: Use enum for various negotiation modes
  nbd/client: Add safety check on chunk payload length
  nbd/client: Simplify cookie vs. index computation
  nbd: s/handle/cookie/ to match NBD spec
  nbd/server: Refactor to pass full request around
  nbd/server: Prepare for alternate-size headers
  nbd: Consistent typedef usage in header
  nbd/client: Use smarter assert
  qemu-nbd: make verbose bool and local variable in main()
  qemu-nbd: handle dup2() error when qemu-nbd finished setup process
  qemu-nbd: properly report error on error in dup2() after qemu_daemon()
  qemu-nbd: properly report error if qemu_daemon() is failed
  qemu-nbd: fix regression with qemu-nbd --fork run over ssh
  qemu-nbd: pass structure into nbd_client_thread instead of plain char*

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
  • Loading branch information
pm215 committed Jul 20, 2023
2 parents 67d1f0a + bfe04d0 commit d1181d2
Show file tree
Hide file tree
Showing 7 changed files with 332 additions and 235 deletions.
96 changes: 49 additions & 47 deletions block/nbd.c
@@ -1,8 +1,8 @@
/*
* QEMU Block driver for NBD
* QEMU Block driver for NBD
*
* Copyright (c) 2019 Virtuozzo International GmbH.
* Copyright (C) 2016 Red Hat, Inc.
* Copyright Red Hat
* Copyright (C) 2008 Bull S.A.S.
* Author: Laurent Vivier <Laurent.Vivier@bull.net>
*
Expand Down Expand Up @@ -50,8 +50,8 @@
#define EN_OPTSTR ":exportname="
#define MAX_NBD_REQUESTS 16

#define HANDLE_TO_INDEX(bs, handle) ((handle) ^ (uint64_t)(intptr_t)(bs))
#define INDEX_TO_HANDLE(bs, index) ((index) ^ (uint64_t)(intptr_t)(bs))
#define COOKIE_TO_INDEX(cookie) ((cookie) - 1)
#define INDEX_TO_COOKIE(index) ((index) + 1)

typedef struct {
Coroutine *coroutine;
Expand Down Expand Up @@ -417,25 +417,25 @@ static void coroutine_fn GRAPH_RDLOCK nbd_reconnect_attempt(BDRVNBDState *s)
reconnect_delay_timer_del(s);
}

static coroutine_fn int nbd_receive_replies(BDRVNBDState *s, uint64_t handle)
static coroutine_fn int nbd_receive_replies(BDRVNBDState *s, uint64_t cookie)
{
int ret;
uint64_t ind = HANDLE_TO_INDEX(s, handle), ind2;
uint64_t ind = COOKIE_TO_INDEX(cookie), ind2;
QEMU_LOCK_GUARD(&s->receive_mutex);

while (true) {
if (s->reply.handle == handle) {
if (s->reply.cookie == cookie) {
/* We are done */
return 0;
}

if (s->reply.handle != 0) {
if (s->reply.cookie != 0) {
/*
* Some other request is being handled now. It should already be
* woken by whoever set s->reply.handle (or never wait in this
* woken by whoever set s->reply.cookie (or never wait in this
* yield). So, we should not wake it here.
*/
ind2 = HANDLE_TO_INDEX(s, s->reply.handle);
ind2 = COOKIE_TO_INDEX(s->reply.cookie);
assert(!s->requests[ind2].receiving);

s->requests[ind].receiving = true;
Expand All @@ -445,9 +445,9 @@ static coroutine_fn int nbd_receive_replies(BDRVNBDState *s, uint64_t handle)
/*
* We may be woken for 2 reasons:
* 1. From this function, executing in parallel coroutine, when our
* handle is received.
* cookie is received.
* 2. From nbd_co_receive_one_chunk(), when previous request is
* finished and s->reply.handle set to 0.
* finished and s->reply.cookie set to 0.
* Anyway, it's OK to lock the mutex and go to the next iteration.
*/

Expand All @@ -456,8 +456,8 @@ static coroutine_fn int nbd_receive_replies(BDRVNBDState *s, uint64_t handle)
continue;
}

/* We are under mutex and handle is 0. We have to do the dirty work. */
assert(s->reply.handle == 0);
/* We are under mutex and cookie is 0. We have to do the dirty work. */
assert(s->reply.cookie == 0);
ret = nbd_receive_reply(s->bs, s->ioc, &s->reply, NULL);
if (ret <= 0) {
ret = ret ? ret : -EIO;
Expand All @@ -468,12 +468,12 @@ static coroutine_fn int nbd_receive_replies(BDRVNBDState *s, uint64_t handle)
nbd_channel_error(s, -EINVAL);
return -EINVAL;
}
ind2 = HANDLE_TO_INDEX(s, s->reply.handle);
ind2 = COOKIE_TO_INDEX(s->reply.cookie);
if (ind2 >= MAX_NBD_REQUESTS || !s->requests[ind2].coroutine) {
nbd_channel_error(s, -EINVAL);
return -EINVAL;
}
if (s->reply.handle == handle) {
if (s->reply.cookie == cookie) {
/* We are done */
return 0;
}
Expand Down Expand Up @@ -519,7 +519,7 @@ nbd_co_send_request(BlockDriverState *bs, NBDRequest *request,
qemu_mutex_unlock(&s->requests_lock);

qemu_co_mutex_lock(&s->send_mutex);
request->handle = INDEX_TO_HANDLE(s, i);
request->cookie = INDEX_TO_COOKIE(i);

assert(s->ioc);

Expand Down Expand Up @@ -828,11 +828,11 @@ static coroutine_fn int nbd_co_receive_structured_payload(
* corresponding to the server's error reply), and errp is unchanged.
*/
static coroutine_fn int nbd_co_do_receive_one_chunk(
BDRVNBDState *s, uint64_t handle, bool only_structured,
BDRVNBDState *s, uint64_t cookie, bool only_structured,
int *request_ret, QEMUIOVector *qiov, void **payload, Error **errp)
{
int ret;
int i = HANDLE_TO_INDEX(s, handle);
int i = COOKIE_TO_INDEX(cookie);
void *local_payload = NULL;
NBDStructuredReplyChunk *chunk;

Expand All @@ -841,14 +841,14 @@ static coroutine_fn int nbd_co_do_receive_one_chunk(
}
*request_ret = 0;

ret = nbd_receive_replies(s, handle);
ret = nbd_receive_replies(s, cookie);
if (ret < 0) {
error_setg(errp, "Connection closed");
return -EIO;
}
assert(s->ioc);

assert(s->reply.handle == handle);
assert(s->reply.cookie == cookie);

if (nbd_reply_is_simple(&s->reply)) {
if (only_structured) {
Expand Down Expand Up @@ -918,11 +918,11 @@ static coroutine_fn int nbd_co_do_receive_one_chunk(
* Return value is a fatal error code or normal nbd reply error code
*/
static coroutine_fn int nbd_co_receive_one_chunk(
BDRVNBDState *s, uint64_t handle, bool only_structured,
BDRVNBDState *s, uint64_t cookie, bool only_structured,
int *request_ret, QEMUIOVector *qiov, NBDReply *reply, void **payload,
Error **errp)
{
int ret = nbd_co_do_receive_one_chunk(s, handle, only_structured,
int ret = nbd_co_do_receive_one_chunk(s, cookie, only_structured,
request_ret, qiov, payload, errp);

if (ret < 0) {
Expand All @@ -932,7 +932,7 @@ static coroutine_fn int nbd_co_receive_one_chunk(
/* For assert at loop start in nbd_connection_entry */
*reply = s->reply;
}
s->reply.handle = 0;
s->reply.cookie = 0;

nbd_recv_coroutines_wake(s);

Expand Down Expand Up @@ -975,18 +975,18 @@ static void nbd_iter_request_error(NBDReplyChunkIter *iter, int ret)
* NBD_FOREACH_REPLY_CHUNK
* The pointer stored in @payload requires g_free() to free it.
*/
#define NBD_FOREACH_REPLY_CHUNK(s, iter, handle, structured, \
#define NBD_FOREACH_REPLY_CHUNK(s, iter, cookie, structured, \
qiov, reply, payload) \
for (iter = (NBDReplyChunkIter) { .only_structured = structured }; \
nbd_reply_chunk_iter_receive(s, &iter, handle, qiov, reply, payload);)
nbd_reply_chunk_iter_receive(s, &iter, cookie, qiov, reply, payload);)

/*
* nbd_reply_chunk_iter_receive
* The pointer stored in @payload requires g_free() to free it.
*/
static bool coroutine_fn nbd_reply_chunk_iter_receive(BDRVNBDState *s,
NBDReplyChunkIter *iter,
uint64_t handle,
uint64_t cookie,
QEMUIOVector *qiov,
NBDReply *reply,
void **payload)
Expand All @@ -1005,7 +1005,7 @@ static bool coroutine_fn nbd_reply_chunk_iter_receive(BDRVNBDState *s,
reply = &local_reply;
}

ret = nbd_co_receive_one_chunk(s, handle, iter->only_structured,
ret = nbd_co_receive_one_chunk(s, cookie, iter->only_structured,
&request_ret, qiov, reply, payload,
&local_err);
if (ret < 0) {
Expand Down Expand Up @@ -1038,20 +1038,21 @@ static bool coroutine_fn nbd_reply_chunk_iter_receive(BDRVNBDState *s,

break_loop:
qemu_mutex_lock(&s->requests_lock);
s->requests[HANDLE_TO_INDEX(s, handle)].coroutine = NULL;
s->requests[COOKIE_TO_INDEX(cookie)].coroutine = NULL;
s->in_flight--;
qemu_co_queue_next(&s->free_sema);
qemu_mutex_unlock(&s->requests_lock);

return false;
}

static int coroutine_fn nbd_co_receive_return_code(BDRVNBDState *s, uint64_t handle,
int *request_ret, Error **errp)
static int coroutine_fn
nbd_co_receive_return_code(BDRVNBDState *s, uint64_t cookie,
int *request_ret, Error **errp)
{
NBDReplyChunkIter iter;

NBD_FOREACH_REPLY_CHUNK(s, iter, handle, false, NULL, NULL, NULL) {
NBD_FOREACH_REPLY_CHUNK(s, iter, cookie, false, NULL, NULL, NULL) {
/* nbd_reply_chunk_iter_receive does all the work */
}

Expand All @@ -1060,16 +1061,17 @@ static int coroutine_fn nbd_co_receive_return_code(BDRVNBDState *s, uint64_t han
return iter.ret;
}

static int coroutine_fn nbd_co_receive_cmdread_reply(BDRVNBDState *s, uint64_t handle,
uint64_t offset, QEMUIOVector *qiov,
int *request_ret, Error **errp)
static int coroutine_fn
nbd_co_receive_cmdread_reply(BDRVNBDState *s, uint64_t cookie,
uint64_t offset, QEMUIOVector *qiov,
int *request_ret, Error **errp)
{
NBDReplyChunkIter iter;
NBDReply reply;
void *payload = NULL;
Error *local_err = NULL;

NBD_FOREACH_REPLY_CHUNK(s, iter, handle, s->info.structured_reply,
NBD_FOREACH_REPLY_CHUNK(s, iter, cookie, s->info.structured_reply,
qiov, &reply, &payload)
{
int ret;
Expand Down Expand Up @@ -1112,10 +1114,10 @@ static int coroutine_fn nbd_co_receive_cmdread_reply(BDRVNBDState *s, uint64_t h
return iter.ret;
}

static int coroutine_fn nbd_co_receive_blockstatus_reply(BDRVNBDState *s,
uint64_t handle, uint64_t length,
NBDExtent *extent,
int *request_ret, Error **errp)
static int coroutine_fn
nbd_co_receive_blockstatus_reply(BDRVNBDState *s, uint64_t cookie,
uint64_t length, NBDExtent *extent,
int *request_ret, Error **errp)
{
NBDReplyChunkIter iter;
NBDReply reply;
Expand All @@ -1124,7 +1126,7 @@ static int coroutine_fn nbd_co_receive_blockstatus_reply(BDRVNBDState *s,
bool received = false;

assert(!extent->length);
NBD_FOREACH_REPLY_CHUNK(s, iter, handle, false, NULL, &reply, &payload) {
NBD_FOREACH_REPLY_CHUNK(s, iter, cookie, false, NULL, &reply, &payload) {
int ret;
NBDStructuredReplyChunk *chunk = &reply.structured;

Expand Down Expand Up @@ -1194,11 +1196,11 @@ nbd_co_request(BlockDriverState *bs, NBDRequest *request,
continue;
}

ret = nbd_co_receive_return_code(s, request->handle,
ret = nbd_co_receive_return_code(s, request->cookie,
&request_ret, &local_err);
if (local_err) {
trace_nbd_co_request_fail(request->from, request->len,
request->handle, request->flags,
request->cookie, request->flags,
request->type,
nbd_cmd_lookup(request->type),
ret, error_get_pretty(local_err));
Expand Down Expand Up @@ -1253,10 +1255,10 @@ nbd_client_co_preadv(BlockDriverState *bs, int64_t offset, int64_t bytes,
continue;
}

ret = nbd_co_receive_cmdread_reply(s, request.handle, offset, qiov,
ret = nbd_co_receive_cmdread_reply(s, request.cookie, offset, qiov,
&request_ret, &local_err);
if (local_err) {
trace_nbd_co_request_fail(request.from, request.len, request.handle,
trace_nbd_co_request_fail(request.from, request.len, request.cookie,
request.flags, request.type,
nbd_cmd_lookup(request.type),
ret, error_get_pretty(local_err));
Expand Down Expand Up @@ -1411,11 +1413,11 @@ static int coroutine_fn GRAPH_RDLOCK nbd_client_co_block_status(
continue;
}

ret = nbd_co_receive_blockstatus_reply(s, request.handle, bytes,
ret = nbd_co_receive_blockstatus_reply(s, request.cookie, bytes,
&extent, &request_ret,
&local_err);
if (local_err) {
trace_nbd_co_request_fail(request.from, request.len, request.handle,
trace_nbd_co_request_fail(request.from, request.len, request.cookie,
request.flags, request.type,
nbd_cmd_lookup(request.type),
ret, error_get_pretty(local_err));
Expand Down

0 comments on commit d1181d2

Please sign in to comment.