Re: [Libguestfs] [Qemu-devel] [PATCH 2/5] nbd: Prepare for NBD_CMD_FLAG_FAST_ZERO

On 8/30/19 1:07 PM, Vladimir Sementsov-Ogievskiy wrote: > 23.08.2019 17:37, Eric Blake wrote: >> Commit fe0480d6 and friends added BDRV_REQ_NO_FALLBACK as a way to >> avoid wasting time on a preliminary write-zero request that will later >> be rewritten by actual data, if it is known that the write-zero >> request will use a slow fallback; but in doing so, could not optimize >> for NBD. The NBD specification is now considering an extension that >> will allow passing on those semantics; this patch updates the new >> protocol bits and 'qemu-nbd --list' output to recognize the bit, as >> well as the new errno value possible when using the new flag; while >> upcoming patches will improve the client to use the feature when >> present, and the server to advertise support for it. >> >> Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; >> +++ b/nbd/server.c >> @@ -55,6 +55,8 @@ static int system_errno_to_nbd_errno(int err) >> return NBD_ENOSPC; >> case EOVERFLOW: >> return NBD_EOVERFLOW; >> + case ENOTSUP: >> + return NBD_ENOTSUP; > > This may provoke returning NBD_ENOTSUP in other cases, not only new one we are going to add. Correct. But the spec only said SHOULD avoid ENOTSUP in those other cases, not MUST avoid ENOTSUP; and in practice, either the client that is not suspecting it will treat it the same as NBD_EINVAL, or we've managed to get a slightly better error message across than normal. I don't see that as a real show-stopper. But if it bothers you,

note that in nbdkit, I actually coded things up to refuse to send NBD_EOVERFLOW unless NBD_CMD_FLAG_DF was set, and to refuse to send NBD_ENOTSUP unless NBD_CMD_FLAG_FAST_ZERO was set. I could copy that behavior here, if we want qemu to be that much stricter as a server. (Note to self: also check if, when using structured replies to send error messages, in addition to the code, if the string contains strerror(errno) from BEFORE the mapping, rather than after we've lost information to the more-limited NBD_E* values)