On Tue, Sep 05, 2023 at 05:41:33PM +0300, Vladimir Sementsov-Ogievskiy wrote: > > > > @@ -1899,7 +1899,7 @@ static int coroutine_fn > > > > nbd_co_send_simple_reply(NBDClient *client, > > > > NBDRequest *request, > > > > uint32_t error, > > > > void *data, > > > > - size_t len, > > > > + uint64_t len, > > > > Error **errp) > > > > { > > > > NBDSimpleReply reply; > > > > @@ -1910,6 +1910,7 @@ static int coroutine_fn > > > > nbd_co_send_simple_reply(NBDClient *client, > > > > }; > > > > > > > > assert(!len || !nbd_err); > > > > + assert(len <= NBD_MAX_STRING_SIZE); > > > > > > NBD_MAX_BUFFER_SIZE ? > > > > No. MAX_STRING_SIZE is 4k, MAX_BUFFER_SIZE is 32M. The smaller size > > is the correct bound here (an error message has to be transmitted as a > > single string, and the recipient does not expect more than a 4k error > > message). > > > > I miss something.. Why it's an error message? It's may be a simple reply for > CMD_READ, where len is request->len
Oh; I was confusing this with the length of an error message; but when an error message is sent over the wire, it's done with nbd_co_send_chunk_error(). And a quick audit only shows two callers of nbd_co_send_simple_reply(): nbd_send_generic_reply() which only uses it with data/len NULL/0; and nbd_do_cmd_read() does indeed use up to MAX_BUFFER_SIZE when structured mode was not negotiated. Our unit tests aren't covering interoperability with clients that don't request structured replies, so I wasn't seeing the problem with just 'make check' or any of the iotests; but libnbd's testsuite does do interop testing, and there I was able to trip this assertion. I'll fix the assertion. -- Eric Blake, Principal Software Engineer Red Hat, Inc. Virtualization: qemu.org | libguestfs.org