[Qemu-devel] nbd structured reply

[Qemu-devel] nbd structured reply

[Vladimir Sementsov-Ogievskiy]

Hi all!

I'm about this:

"A server SHOULD try to minimize the number of chunks sent in a reply, 
but MUST NOT mark a chunk as final if there is still a possibility of 
detecting an error before transmission of that chunk completes"

What do we mean by "possibility"? Formally, such possibility exists 
always, so, we'll never mark a chunk as a final.

-- 
Best regards,
Vladimir

Re: [Qemu-devel] nbd structured reply

[Vladimir Sementsov-Ogievskiy]

05.10.2017 16:37, Eric Blake wrote:
> On 10/05/2017 06:30 AM, Vladimir Sementsov-Ogievskiy wrote:
>> 21.09.2017 15:18, Vladimir Sementsov-Ogievskiy wrote:
>>> Hi all!
>>>
>>> I'm about this:
>>>
>>> "A server SHOULD try to minimize the number of chunks sent in a reply,
>>> but MUST NOT mark a chunk as final if there is still a possibility of
>>> detecting an error before transmission of that chunk completes"
>>>
>>> What do we mean by "possibility"? Formally, such possibility exists
>>> always, so, we'll never mark a chunk as final.
>>>
>> One more question:
>>
>> for |NBD_REPLY_TYPE_ERROR and ||NBD_REPLY_TYPE_ERROR_OFFSET, why do we
>> need message_length field? why not to calc it as chunk.lenght - 4 for
>> ||NBD_REPLY_TYPE_ERROR and chunk.lenght - 12 for
>> ||NBD_REPLY_TYPE_ERROR_OFFSET?
> For consistency.  If _all_ NBD_REPLY_TYPE_ERROR* message have a
> message_length field, then it is easier to write a generic handler that

Oh, I've missed this in the spec, thanks. Of course it make sense.

> knows how to deal with an unknown error, no matter what command the
> error is sent in response to.  Ideally, a server should never send an
> error message that the client is not expecting, but having a robust
> protocol that lets clients deal with bad servers is worth the redundancy
> caused by being consistent, and we are more likely to add additional
> error modes to existing commands than we are to add more success modes.
>
>> For example, with NBD_REPLY_TYPE_OFFSET_DATA variable data length is
>> calculated, not specified separately.
> That's because non-error types don't have the same consistency concerns;
> if we want to introduce a new success response, we'll probably introduce
> it via a new command, rather than as a reply to an existing command.
> Furthermore, while error replies are likely to have a free-form text
> error description, success replies tend to not need it.  The layout of
> the error types is designed to make it easy to grab the free-form error
> message from a known location for display to the user, even if the
> client has no idea what the rest of the error means, as that may be a
> useful debugging aid.
>
>> What is the reason for server to send NBD_REPLY_TYPE_ERROR with
>> message_lenght < chunk.lenght - 4?
> In all likelihood, all well-written servers will never send garbage
> bytes (possible only when setting chunk.length larger than
> message_length + sizeof(documented fields)).  But we wrote the spec to
> be conservative, in case we want to add a later defined field that
> earlier clients will still gracefully ignore, rather than strict
> (allowing inequality, instead of requiring exact lengths, lets a client
> skip over what it considers garbage bytes rather than dropping the
> connection because a too-new server tried to send useful information in
> those bytes).
>


-- 
Best regards,
Vladimir

Re: [Qemu-devel] nbd structured reply

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 2860 bytes --]

On 10/05/2017 06:30 AM, Vladimir Sementsov-Ogievskiy wrote:
> 21.09.2017 15:18, Vladimir Sementsov-Ogievskiy wrote:
>> Hi all!
>>
>> I'm about this:
>>
>> "A server SHOULD try to minimize the number of chunks sent in a reply,
>> but MUST NOT mark a chunk as final if there is still a possibility of
>> detecting an error before transmission of that chunk completes"
>>
>> What do we mean by "possibility"? Formally, such possibility exists
>> always, so, we'll never mark a chunk as final.
>>
> 
> One more question:
> 
> for |NBD_REPLY_TYPE_ERROR and ||NBD_REPLY_TYPE_ERROR_OFFSET, why do we
> need message_length field? why not to calc it as chunk.lenght - 4 for
> ||NBD_REPLY_TYPE_ERROR and chunk.lenght - 12 for
> ||NBD_REPLY_TYPE_ERROR_OFFSET?

For consistency.  If _all_ NBD_REPLY_TYPE_ERROR* message have a
message_length field, then it is easier to write a generic handler that
knows how to deal with an unknown error, no matter what command the
error is sent in response to.  Ideally, a server should never send an
error message that the client is not expecting, but having a robust
protocol that lets clients deal with bad servers is worth the redundancy
caused by being consistent, and we are more likely to add additional
error modes to existing commands than we are to add more success modes.

> 
> For example, with NBD_REPLY_TYPE_OFFSET_DATA variable data length is
> calculated, not specified separately.

That's because non-error types don't have the same consistency concerns;
if we want to introduce a new success response, we'll probably introduce
it via a new command, rather than as a reply to an existing command.
Furthermore, while error replies are likely to have a free-form text
error description, success replies tend to not need it.  The layout of
the error types is designed to make it easy to grab the free-form error
message from a known location for display to the user, even if the
client has no idea what the rest of the error means, as that may be a
useful debugging aid.

> 
> What is the reason for server to send NBD_REPLY_TYPE_ERROR with
> message_lenght < chunk.lenght - 4?

In all likelihood, all well-written servers will never send garbage
bytes (possible only when setting chunk.length larger than
message_length + sizeof(documented fields)).  But we wrote the spec to
be conservative, in case we want to add a later defined field that
earlier clients will still gracefully ignore, rather than strict
(allowing inequality, instead of requiring exact lengths, lets a client
skip over what it considers garbage bytes rather than dropping the
connection because a too-new server tried to send useful information in
those bytes).

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

Re: [Qemu-devel] nbd structured reply

[Vladimir Sementsov-Ogievskiy]

fix cc to nbd@other.debian.org

05.10.2017 14:30, Vladimir Sementsov-Ogievskiy wrote:
> 21.09.2017 15:18, Vladimir Sementsov-Ogievskiy wrote:
>> Hi all!
>>
>> I'm about this:
>>
>> "A server SHOULD try to minimize the number of chunks sent in a 
>> reply, but MUST NOT mark a chunk as final if there is still a 
>> possibility of detecting an error before transmission of that chunk 
>> completes"
>>
>> What do we mean by "possibility"? Formally, such possibility exists 
>> always, so, we'll never mark a chunk as final.
>>
>
> One more question:
>
> for |NBD_REPLY_TYPE_ERROR and ||NBD_REPLY_TYPE_ERROR_OFFSET, why do we 
> need message_length field? why not to calc it as chunk.lenght - 4 for 
> ||NBD_REPLY_TYPE_ERROR and chunk.lenght - 12 for 
> ||NBD_REPLY_TYPE_ERROR_OFFSET?
>
> For example, with NBD_REPLY_TYPE_OFFSET_DATA variable data length is 
> calculated, not specified separately.
>
> What is the reason for server to send NBD_REPLY_TYPE_ERROR with 
> message_lenght < chunk.lenght - 4?
> |
>
> -- 
> Best regards,
> Vladimir


-- 
Best regards,
Vladimir

Re: [Qemu-devel] nbd structured reply

[Vladimir Sementsov-Ogievskiy]

21.09.2017 15:18, Vladimir Sementsov-Ogievskiy wrote:
> Hi all!
>
> I'm about this:
>
> "A server SHOULD try to minimize the number of chunks sent in a reply, 
> but MUST NOT mark a chunk as final if there is still a possibility of 
> detecting an error before transmission of that chunk completes"
>
> What do we mean by "possibility"? Formally, such possibility exists 
> always, so, we'll never mark a chunk as final.
>

One more question:

for |NBD_REPLY_TYPE_ERROR and ||NBD_REPLY_TYPE_ERROR_OFFSET, why do we 
need message_length field? why not to calc it as chunk.lenght - 4 for 
||NBD_REPLY_TYPE_ERROR and chunk.lenght - 12 for 
||NBD_REPLY_TYPE_ERROR_OFFSET?

For example, with NBD_REPLY_TYPE_OFFSET_DATA variable data length is 
calculated, not specified separately.

What is the reason for server to send NBD_REPLY_TYPE_ERROR with 
message_lenght < chunk.lenght - 4?
|

-- 
Best regards,
Vladimir

Re: [Qemu-devel] nbd structured reply

[Wouter Verhelst]

On Fri, Sep 22, 2017 at 05:57:07PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> The obvious behavior of client is to fail the whole read if it received one
> error chunk.

Not necessarily.

If a user-space program requests to read X bytes of data, but there is
an error at X-N, then the obvious way to handle that is for the read()
call to return with X-N bytes first, and for the next read() call to
return with -1, and errno set to EIO.

Structured reads allow for that kind of behaviour.

-- 
Could you people please use IRC like normal people?!?

  -- Amaya Rodrigo Sastre, trying to quiet down the buzz in the DebConf 2008
     Hacklab

Re: [Qemu-devel] nbd structured reply

[Vladimir Sementsov-Ogievskiy]

22.09.2017 23:36, Eric Blake wrote:
> On 09/22/2017 09:57 AM, Vladimir Sementsov-Ogievskiy wrote:
>
>>> If you have suggestions for improving the NBD spec wording, feel free to
>>> propose a doc patch.
>>>
>> Thanks, now I understand.. However I don't have good idea of wording..
>>
>>
>> Another thing: server can send several error and success chunks on
>> CMD_READ..
> Yes, and that is intentional. A server that spawns sub-tasks to read
> portions of the request to various parallel workers, and then sends
> those responses back to the client as sub-tasks finish, can indeed send
> multiple errors before sending the final chunk complete message.
>
>> The obvious behavior of client is to fail the whole read if it received
>> one error chunk.
> Yes, the read has failed if the client sees at least one error chunk.
> The read is not successful unless there are no error chunks, and the
> server has sent chunks describing every portion of the request (the
> server is not allowed to send chunks that overlap, nor to send a
> successful chunk after sending an error for the same offset, nor to send
> a success message if it has not covered the entire request - but IS
> allowed to send multiple error chunks).
>
>> And, actually, client is not interesting in all following chunks for
>> this request.
> Perhaps not, but the client is not in control of how much the server
> sends, so it must gracefully deal with the remaining traffic from the
> server while waiting for the server to finally send the last chunk.
>
>> I think
>> we need some additional negotiation flag for this, which says that error
>> chunk
>> must be final.
> Why? It adds complexity, for no real reason.  (Read errors are not
> common, so it is okay if dealing with read errors requires more network
> traffic than normal).

Hmm, when writing error handling, it may seem that the program is 
processing errors
almost all the time). Ok, really they are not common, so it doesn't 
really matter.

>
>> May be we need also a method (command) to cancel one of inflight
>> requests, but it
>> is not so important.
> Again, that would add complexity that may be really hard to justify (we
> want to be able to implement stateless servers; a server that has to
> parse a second message from a client requesting to abort an in-flight
> command has to track state).  So I don't think it is worth it.
>

-- 
Best regards,
Vladimir

Re: [Qemu-devel] nbd structured reply

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 2297 bytes --]

On 09/22/2017 09:57 AM, Vladimir Sementsov-Ogievskiy wrote:

>> If you have suggestions for improving the NBD spec wording, feel free to
>> propose a doc patch.
>>
> 
> Thanks, now I understand.. However I don't have good idea of wording..
> 
> 
> Another thing: server can send several error and success chunks on
> CMD_READ..

Yes, and that is intentional. A server that spawns sub-tasks to read
portions of the request to various parallel workers, and then sends
those responses back to the client as sub-tasks finish, can indeed send
multiple errors before sending the final chunk complete message.

> 
> The obvious behavior of client is to fail the whole read if it received
> one error chunk.

Yes, the read has failed if the client sees at least one error chunk.
The read is not successful unless there are no error chunks, and the
server has sent chunks describing every portion of the request (the
server is not allowed to send chunks that overlap, nor to send a
successful chunk after sending an error for the same offset, nor to send
a success message if it has not covered the entire request - but IS
allowed to send multiple error chunks).

> And, actually, client is not interesting in all following chunks for
> this request.

Perhaps not, but the client is not in control of how much the server
sends, so it must gracefully deal with the remaining traffic from the
server while waiting for the server to finally send the last chunk.

> I think
> we need some additional negotiation flag for this, which says that error
> chunk
> must be final.

Why? It adds complexity, for no real reason.  (Read errors are not
common, so it is okay if dealing with read errors requires more network
traffic than normal).

> 
> May be we need also a method (command) to cancel one of inflight
> requests, but it
> is not so important.

Again, that would add complexity that may be really hard to justify (we
want to be able to implement stateless servers; a server that has to
parse a second message from a client requesting to abort an in-flight
command has to track state).  So I don't think it is worth it.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

Re: [Qemu-devel] nbd structured reply

[Vladimir Sementsov-Ogievskiy]

21.09.2017 16:55, Eric Blake wrote:
> [updating CC to point to the correct new NBD list]
>
> On 09/21/2017 07:18 AM, Vladimir Sementsov-Ogievskiy wrote:
>> Hi all!
>>
>> I'm about this:
>>
>> "A server SHOULD try to minimize the number of chunks sent in a reply,
>> but MUST NOT mark a chunk as final if there is still a possibility of
>> detecting an error before transmission of that chunk completes"
>>
>> What do we mean by "possibility"? Formally, such possibility exists
>> always, so, we'll never mark a chunk as final.
> Here's what is envisioned.  It is fairly easy to write a server that
> receives an NBD_CMD_READ, issues the header reply, and only then
> dispatches the I/O to collect the data from the file system to send
> after the header.  But if you encounter EIO (or other problem) while
> reading from the file system, you're stuck if you've already sent the
> header.  In this scenario, if you mark the header being as final, you
> are unable to report the EIO (it is one of the flaws with non-structured
> reads that makes the spec say that if you detect an error after sending
> the header, you have no choice but to terminate the connection).
>
> An alternative server implementation would buffer the header, perform
> the I/O to collect data from the file system, and only when it is all
> present in memory, it finally sends the reply to the client.  In this
> scenario, since you wait until all data is buffered, you know up front
> whether an error occurred while collecting the I/O, and can therefore
> send the data as the final packet right away (whether error or success).
>   However, it potentially requires more memory to buffer requests like
> this, compared to the server that sends information piecemeal as soon as
> it obtains data.
>
> The intent is not to detect errors in sending the data over the wire,
> but in detecting errors in reading the data from the underlying storage
> (or however else the data to be read is being collected).  A chunk must
> not be marked final unless you can guarantee that there will be no
> further errors in collecting data for the reply.
>
> If you have suggestions for improving the NBD spec wording, feel free to
> propose a doc patch.
>

Thanks, now I understand.. However I don't have good idea of wording..


Another thing: server can send several error and success chunks on 
CMD_READ..

The obvious behavior of client is to fail the whole read if it received 
one error chunk.
And, actually, client is not interesting in all following chunks for 
this request. I think
we need some additional negotiation flag for this, which says that error 
chunk
must be final.

May be we need also a method (command) to cancel one of inflight 
requests, but it
is not so important.


-- 
Best regards,
Vladimir

Re: [Qemu-devel] nbd structured reply

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 2286 bytes --]

[updating CC to point to the correct new NBD list]

On 09/21/2017 07:18 AM, Vladimir Sementsov-Ogievskiy wrote:
> Hi all!
> 
> I'm about this:
> 
> "A server SHOULD try to minimize the number of chunks sent in a reply,
> but MUST NOT mark a chunk as final if there is still a possibility of
> detecting an error before transmission of that chunk completes"
> 
> What do we mean by "possibility"? Formally, such possibility exists
> always, so, we'll never mark a chunk as final.

Here's what is envisioned.  It is fairly easy to write a server that
receives an NBD_CMD_READ, issues the header reply, and only then
dispatches the I/O to collect the data from the file system to send
after the header.  But if you encounter EIO (or other problem) while
reading from the file system, you're stuck if you've already sent the
header.  In this scenario, if you mark the header being as final, you
are unable to report the EIO (it is one of the flaws with non-structured
reads that makes the spec say that if you detect an error after sending
the header, you have no choice but to terminate the connection).

An alternative server implementation would buffer the header, perform
the I/O to collect data from the file system, and only when it is all
present in memory, it finally sends the reply to the client.  In this
scenario, since you wait until all data is buffered, you know up front
whether an error occurred while collecting the I/O, and can therefore
send the data as the final packet right away (whether error or success).
 However, it potentially requires more memory to buffer requests like
this, compared to the server that sends information piecemeal as soon as
it obtains data.

The intent is not to detect errors in sending the data over the wire,
but in detecting errors in reading the data from the underlying storage
(or however else the data to be read is being collected).  A chunk must
not be marked final unless you can guarantee that there will be no
further errors in collecting data for the reply.

If you have suggestions for improving the NBD spec wording, feel free to
propose a doc patch.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

[Qemu-devel] nbd structured reply

[Vladimir Sementsov-Ogievskiy]

Hi all!

I'm about this:

"A server SHOULD try to minimize the number of chunks sent in a reply, 
but MUST NOT mark a chunk as final if there is still a possibility of 
detecting an error before transmission of that chunk completes"

What do we mean by "possibility"? Formally, such possibility exists 
always, so, we'll never mark a chunk as final.

-- 
Best regards,
Vladimir