Message ID | 20190325190104.30213-3-eblake@redhat.com |
---|---|
State | New |
Headers | show |
Series | nbd: simple errors to BLOCK_STATUS | expand |
25.03.2019 22:01, Eric Blake wrote: > The NBD spec is clear that when structured replies are active, a > simple error reply is acceptable to any command except for > NBD_CMD_READ. However, we were mistakenly requiring structured errors > for NBD_CMD_BLOCK_STATUS, and hanging up on a server that gave a > simple error (since qemu does not behave as such a server, we didn't > notice the problem until now). Broken since its introduction in > commit 78a33ab5 (v2.12). > > Noticed while debugging a separate failure reported by nbdkit while > working out its initial implementation of BLOCK_STATUS, although it > turns out that nbdkit also chose to send structured error replies for > BLOCK_STATUS, so I had to manually provoke the situation by hacking > qemu's server to send a simple error reply: > > | diff --git i/nbd/server.c w/nbd/server.c > | index fd013a2817a..833288d7c45 100644 > | 00--- i/nbd/server.c > | +++ w/nbd/server.c > | @@ -2269,6 +2269,8 @@ static coroutine_fn int nbd_handle_request(NBDClient *client, > | "discard failed", errp); > | > | case NBD_CMD_BLOCK_STATUS: > | + return nbd_co_send_simple_reply(client, request->handle, ENOMEM, > | + NULL, 0, errp); > | if (!request->len) { > | return nbd_send_generic_reply(client, request->handle, -EINVAL, > | "need non-zero length", errp); > | > > Signed-off-by: Eric Blake <eblake@redhat.com> > Message-Id: <20190323142455.5301-1-eblake@redhat.com> > Acked-by: Richard W.M. Jones <rjones@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > --- > block/nbd-client.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/block/nbd-client.c b/block/nbd-client.c > index b37a5963013..a3b70d14004 100644 > --- a/block/nbd-client.c > +++ b/block/nbd-client.c > @@ -729,9 +729,7 @@ static int nbd_co_receive_blockstatus_reply(NBDClientSession *s, > bool received = false; > > assert(!extent->length); > - NBD_FOREACH_REPLY_CHUNK(s, iter, handle, s->info.structured_reply, > - NULL, &reply, &payload) > - { > + NBD_FOREACH_REPLY_CHUNK(s, iter, handle, false, NULL, &reply, &payload) { > int ret; > NBDStructuredReplyChunk *chunk = &reply.structured; >
diff --git a/block/nbd-client.c b/block/nbd-client.c index b37a5963013..a3b70d14004 100644 --- a/block/nbd-client.c +++ b/block/nbd-client.c @@ -729,9 +729,7 @@ static int nbd_co_receive_blockstatus_reply(NBDClientSession *s, bool received = false; assert(!extent->length); - NBD_FOREACH_REPLY_CHUNK(s, iter, handle, s->info.structured_reply, - NULL, &reply, &payload) - { + NBD_FOREACH_REPLY_CHUNK(s, iter, handle, false, NULL, &reply, &payload) { int ret; NBDStructuredReplyChunk *chunk = &reply.structured;