[adding qemu] On 3/23/19 6:57 AM, Eric Blake wrote: > On 3/23/19 6:42 AM, Richard W.M. Jones wrote: >> nbdkit (upstream 5a7a394c699) currently fails with qemu 2.12.0: >> >> $ ./nbdkit memory size=64M --run 'qemu-img convert $nbd /var/tmp/out' >> nbdkit: memory.2: error: invalid request: unknown command (7) ignored >> qemu-img: Protocol error: simple reply when structured reply chunk was >> expected >> >> This was a bug in qemu which was fixed upstream quite a long time ago >> by the commit I've attached at the end of this email. >>
>> ---------------------------------------------------------------------- >> 89aa0d87634e2cb98517509dc8bdb876f26ecf8b is the first bad commit >> commit 89aa0d87634e2cb98517509dc8bdb876f26ecf8b >> Author: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> >> Date: Fri Apr 27 17:20:01 2018 +0300 >> >> nbd/client: fix nbd_negotiate_simple_meta_context >> >> Initialize received variable. Otherwise, is is possible for server to >> answer without any contexts, but we will set context_id to something >> random (received_id is not initialized too) and return 1, which is >> wrong. >> Ouch - upstream qemu still has a latent bug in this area. When qemu as server sends an error to NBD_CMD_BLOCK_STATUS, it uses a structured error reply, so we haven't tickled it before. But the NBD spec permits a simple error reply to any command except NBD_CMD_READ when structured replies are negotiated, and that's what nbdkit currently implements. If I add this hack patch to qemu's server (to intentionally fail NBD_CMD_BLOCK_STATUS with a simple error), then the qemu client _should_ be able to just report that block status failed and keep the connection alive, rather than its current behavior of hanging up. So we still need a patch for qemu 4.0 for the client to accept simple errors. :( With the hack applied, I got: $ ./qemu-io -f raw nbd://localhost:10809 --trace=nbd_\* -c map ... 17218@1553343857.048440:nbd_send_request Sending request to server: { .from = 0, .len = 1049088, .handle = 94796979915216, .flags = 0x8, .type = 7 (block status) } 17218@1553343857.048643:nbd_receive_simple_reply Got simple reply: { .error = 22 (EINVAL), handle = 94796979915216 } 17218@1553343857.048655:nbd_co_request_fail Request failed { .from = 0, .len = 1049088, .handle = 94796979915216, .flags = 0x8, .type = 7 (block status) } ret = -22, err: Protocol error: simple reply when structured reply chunk was expected Failed to get allocation status: Invalid argument diff --git i/nbd/server.c w/nbd/server.c index fd013a2817a..35f549abbf9 100644 --- i/nbd/server.c +++ w/nbd/server.c @@ -2148,16 +2148,16 @@ static int nbd_co_receive_request(NBDRequestData *req, NBDRequest *request, * Returns 0 if connection is still live, -errno on failure to talk to client */ static coroutine_fn int nbd_send_generic_reply(NBDClient *client, - uint64_t handle, + NBDRequest *request, int ret, const char *error_msg, Error **errp) { - if (client->structured_reply && ret < 0) { - return nbd_co_send_structured_error(client, handle, -ret, error_msg, + if (client->structured_reply && ret < 0 && request->type == NBD_CMD_READ) { + return nbd_co_send_structured_error(client, request->handle, -ret, error_msg, errp); } else { - return nbd_co_send_simple_reply(client, handle, ret < 0 ? -ret : 0, + return nbd_co_send_simple_reply(client, request->handle, ret < 0 ? -ret : 0, NULL, 0, errp); } } @@ -2177,7 +2177,7 @@ static coroutine_fn int nbd_do_cmd_read(NBDClient *client, NBDRequest *request, if (request->flags & NBD_CMD_FLAG_FUA) { ret = blk_co_flush(exp->blk); if (ret < 0) { - return nbd_send_generic_reply(client, request->handle, ret, + return nbd_send_generic_reply(client, request, ret, "flush failed", errp); } } @@ -2192,7 +2192,7 @@ static coroutine_fn int nbd_do_cmd_read(NBDClient *client, NBDRequest *request, ret = blk_pread(exp->blk, request->from + exp->dev_offset, data, request->len); if (ret < 0 || request->type == NBD_CMD_CACHE) { - return nbd_send_generic_reply(client, request->handle, ret, + return nbd_send_generic_reply(client, request, ret, "reading from file failed", errp); } @@ -2234,7 +2234,7 @@ static coroutine_fn int nbd_handle_request(NBDClient *client, } ret = blk_pwrite(exp->blk, request->from + exp->dev_offset, data, request->len, flags); - return nbd_send_generic_reply(client, request->handle, ret, + return nbd_send_generic_reply(client, request, ret, "writing to file failed", errp); case NBD_CMD_WRITE_ZEROES: @@ -2247,7 +2247,7 @@ static coroutine_fn int nbd_handle_request(NBDClient *client, } ret = blk_pwrite_zeroes(exp->blk, request->from + exp->dev_offset, request->len, flags); - return nbd_send_generic_reply(client, request->handle, ret, + return nbd_send_generic_reply(client, request, ret, "writing to file failed", errp); case NBD_CMD_DISC: @@ -2256,7 +2256,7 @@ static coroutine_fn int nbd_handle_request(NBDClient *client, case NBD_CMD_FLUSH: ret = blk_co_flush(exp->blk); - return nbd_send_generic_reply(client, request->handle, ret, + return nbd_send_generic_reply(client, request, ret, "flush failed", errp); case NBD_CMD_TRIM: @@ -2265,12 +2265,14 @@ static coroutine_fn int nbd_handle_request(NBDClient *client, if (ret == 0 && request->flags & NBD_CMD_FLAG_FUA) { ret = blk_co_flush(exp->blk); } - return nbd_send_generic_reply(client, request->handle, ret, + return nbd_send_generic_reply(client, request, ret, "discard failed", errp); case NBD_CMD_BLOCK_STATUS: + return nbd_send_generic_reply(client, request, -EINVAL, + "no status for you today", errp); if (!request->len) { - return nbd_send_generic_reply(client, request->handle, -EINVAL, + return nbd_send_generic_reply(client, request, -EINVAL, "need non-zero length", errp); } if (client->export_meta.valid && @@ -2304,7 +2306,7 @@ static coroutine_fn int nbd_handle_request(NBDClient *client, return ret; } else { - return nbd_send_generic_reply(client, request->handle, -EINVAL, + return nbd_send_generic_reply(client, request, -EINVAL, "CMD_BLOCK_STATUS not negotiated", errp); } @@ -2312,7 +2314,7 @@ static coroutine_fn int nbd_handle_request(NBDClient *client, default: msg = g_strdup_printf("invalid request type (%" PRIu32 ") received", request->type); - ret = nbd_send_generic_reply(client, request->handle, -EINVAL, msg, + ret = nbd_send_generic_reply(client, request, -EINVAL, msg, errp); g_free(msg); return ret; @@ -2357,7 +2359,7 @@ static coroutine_fn void nbd_trip(void *opaque) Error *export_err = local_err; local_err = NULL; - ret = nbd_send_generic_reply(client, request.handle, -EINVAL, + ret = nbd_send_generic_reply(client, &request, -EINVAL, error_get_pretty(export_err), &local_err); error_free(export_err); } else { -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
_______________________________________________ Libguestfs mailing list Libguestfs@redhat.com https://www.redhat.com/mailman/listinfo/libguestfs