31.08.2019 2:37, Eric Blake wrote: > 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 <ebl...@redhat.com> > >>> +++ 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,
No, I think it doesn't. OK, ENOTSUP is ENOTSUP anyway, it shouldn't be treated incorrectly. > 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) > -- Best regards, Vladimir
