| Message ID | 1459161798-32120-4-git-send-email-den@openvz.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 03/28/2016 04:43 AM, Denis V. Lunev wrote: > From: Pavel Borzenkov <pborzenkov@virtuozzo.com> > > Add separate "Command flags" section to make it clear which flags are > currently defined by the protocol. > > Signed-off-by: Pavel Borzenkov <pborzenkov@virtuozzo.com> > Reviewed-by: Roman Kagan <rkagan@virtuozzo.com> > Signed-off-by: Denis V. Lunev <den@openvz.org> > CC: Wouter Verhelst <w@uter.be> > CC: Eric Blake <eblake@redhat.com> > CC: Alex Bligh <alex@alex.org.uk> > --- > doc/proto.md | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/doc/proto.md b/doc/proto.md > index 036d6d9..662f741 100644 > --- a/doc/proto.md > +++ b/doc/proto.md > @@ -485,6 +485,16 @@ The following request types exist: > Currently one such message is known: `NBD_CMD_CACHE`, with type set to > 5, implemented by xnbd. > > +#### Command flags > + > +This field of 16 bits is sent by the client with every request and provides > +additional information to the server to execute the command. Refer to > +aforementioned "Request types" section for information about the flags Maybe: s/aforementioned "Request types" section/the "Request types" section above/ > +supported by particular commands. > + > +- bit 0, `NBD_CMD_FLAG_FUA`; should be set to 1 if the client requires > + "Force Unit Access" mode of operation Trailing dot? Should you also mention which command(s) it is valid with? (NBD_CMD_WRITE for now, until other extension commands are added) It might also be worth mentioning that the flag should not be sent unless export flags included NBD_FLAG_SEND_FUA. > + > #### Error values > > The error values are used for the error field in the reply message. >
On Mon, Mar 28, 2016 at 07:45:27AM -0600, Eric Blake wrote: > On 03/28/2016 04:43 AM, Denis V. Lunev wrote: > > From: Pavel Borzenkov <pborzenkov@virtuozzo.com> > > > > Add separate "Command flags" section to make it clear which flags are > > currently defined by the protocol. > > > > Signed-off-by: Pavel Borzenkov <pborzenkov@virtuozzo.com> > > Reviewed-by: Roman Kagan <rkagan@virtuozzo.com> > > Signed-off-by: Denis V. Lunev <den@openvz.org> > > CC: Wouter Verhelst <w@uter.be> > > CC: Eric Blake <eblake@redhat.com> > > CC: Alex Bligh <alex@alex.org.uk> > > --- > > doc/proto.md | 10 ++++++++++ > > 1 file changed, 10 insertions(+) > > > > diff --git a/doc/proto.md b/doc/proto.md > > index 036d6d9..662f741 100644 > > --- a/doc/proto.md > > +++ b/doc/proto.md > > @@ -485,6 +485,16 @@ The following request types exist: > > Currently one such message is known: `NBD_CMD_CACHE`, with type set to > > 5, implemented by xnbd. > > > > +#### Command flags > > + > > +This field of 16 bits is sent by the client with every request and provides > > +additional information to the server to execute the command. Refer to > > +aforementioned "Request types" section for information about the flags > > Maybe: > > s/aforementioned "Request types" section/the "Request types" section above/ > > > +supported by particular commands. > > + > > +- bit 0, `NBD_CMD_FLAG_FUA`; should be set to 1 if the client requires > > + "Force Unit Access" mode of operation > > Trailing dot? Should you also mention which command(s) it is valid > with? (NBD_CMD_WRITE for now, until other extension commands are added) > It might also be worth mentioning that the flag should not be sent > unless export flags included NBD_FLAG_SEND_FUA. > > > + > > #### Error values > > > > The error values are used for the error field in the reply message. Yes, I agree that these are all (typographical, but still) improvements. If you can update with that, I'll happily apply that. Regards,
On 03/28/2016 04:43 AM, Denis V. Lunev wrote: > From: Pavel Borzenkov <pborzenkov@virtuozzo.com> > > Add separate "Command flags" section to make it clear which flags are > currently defined by the protocol. > > Signed-off-by: Pavel Borzenkov <pborzenkov@virtuozzo.com> > Reviewed-by: Roman Kagan <rkagan@virtuozzo.com> > Signed-off-by: Denis V. Lunev <den@openvz.org> > CC: Wouter Verhelst <w@uter.be> > CC: Eric Blake <eblake@redhat.com> > CC: Alex Bligh <alex@alex.org.uk> > --- > doc/proto.md | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/doc/proto.md b/doc/proto.md > index 036d6d9..662f741 100644 > --- a/doc/proto.md > +++ b/doc/proto.md > @@ -485,6 +485,16 @@ The following request types exist: > Currently one such message is known: `NBD_CMD_CACHE`, with type set to > 5, implemented by xnbd. > > +#### Command flags > + I think that this new content would belong better as a subsection under '#### Flag Fields', alongside the mention of all other flags. I'm going to propose a v2 of this patch with that alternate position, for comparison.
On 03/29/2016 10:01 AM, Eric Blake wrote: > On 03/28/2016 04:43 AM, Denis V. Lunev wrote: >> From: Pavel Borzenkov <pborzenkov@virtuozzo.com> >> >> Add separate "Command flags" section to make it clear which flags are >> currently defined by the protocol. >> >> Signed-off-by: Pavel Borzenkov <pborzenkov@virtuozzo.com> >> Reviewed-by: Roman Kagan <rkagan@virtuozzo.com> >> Signed-off-by: Denis V. Lunev <den@openvz.org> >> CC: Wouter Verhelst <w@uter.be> >> CC: Eric Blake <eblake@redhat.com> >> CC: Alex Bligh <alex@alex.org.uk> >> --- >> doc/proto.md | 10 ++++++++++ >> 1 file changed, 10 insertions(+) >> >> diff --git a/doc/proto.md b/doc/proto.md >> index 036d6d9..662f741 100644 >> --- a/doc/proto.md >> +++ b/doc/proto.md >> @@ -485,6 +485,16 @@ The following request types exist: >> Currently one such message is known: `NBD_CMD_CACHE`, with type set to >> 5, implemented by xnbd. >> >> +#### Command flags >> + > > I think that this new content would belong better as a subsection under > '#### Flag Fields', alongside the mention of all other flags. I'm going > to propose a v2 of this patch with that alternate position, for comparison. Hmm, maybe not. I just looked again, and '#### Flag fields' is a subsection of '### Handshake phase', while you are correct that command flags belong to '### Transmission phase'.
diff --git a/doc/proto.md b/doc/proto.md index 036d6d9..662f741 100644 --- a/doc/proto.md +++ b/doc/proto.md @@ -485,6 +485,16 @@ The following request types exist: Currently one such message is known: `NBD_CMD_CACHE`, with type set to 5, implemented by xnbd. +#### Command flags + +This field of 16 bits is sent by the client with every request and provides +additional information to the server to execute the command. Refer to +aforementioned "Request types" section for information about the flags +supported by particular commands. + +- bit 0, `NBD_CMD_FLAG_FUA`; should be set to 1 if the client requires + "Force Unit Access" mode of operation + #### Error values The error values are used for the error field in the reply message.