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

[Vladimir Sementsov-Ogievskiy]

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 <address@hidden>
> 
>>> +++ 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