| Message ID | 1459874336-16936-1-git-send-email-alex@alex.org.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 04/05/2016 10:38 AM, Alex Bligh wrote: > Amend the NBD_OPT_SELECT and NBD_OPT_GO documentation as > follows: > > * Allow a name to be specified on NBD_OPT_GO > > * Make clear the rules for default device selection > > * Remove the provision concerning TLS resetting device selection > > * Remove NBD_REP_ERR_INVALID as a reply to NBD_OPT_GO as there > is now no necessity for a prior NBD_OPT_SELECT I guess if the client requests "" without earlier SELECT, and the server has no default, the reply would be NBD_REP_ERR_UNKNOWN rather than ERR_INVALID. > > * Make it clear NBD_OPT_GO is in effect a better alternative > for NBD_OPT_EXPORT_NAME > > * Make it clear the NBD_OPT_SELECT and NBD_OPT_GO are in > essence the same command, save that NBD_OPT_GO puts you > into transmission mode if successful. > > * Clarify permitted option returns outside TLS to prevent > export enumeration. > > * Controversial: remove 'length' 32 bit quantity from > NBD_OPT_SELECT (and don't copy it into NBD_OPT_GO) so it > looks exactly like NBD_OPT_EXPORT_NAME bar the reply. > This length is unnecessary as it's in the option header > anyway. Makes sense to me; we could drop the 'Controversial' marker, unless anyone can come up with a reason why we'd ever want a structure with more than just the export name as the data passed along with SELECT/GO, where having the header length distinct from the name length would let us pass the extra data without needing yet another NBD_OPT_ command. > @@ -676,21 +682,36 @@ option reply type. > server. > - `NBD_REP_ERR_TLS_REQD`: The server does not wish to export this > block device unless the client negotiates TLS first. > - - `NBD_REP_SERVER`: The server accepts the chosen export. In this > - case, the `NBD_REP_SERVER` message's data takes on a different > + - `NBD_REP_SERVER`: The server accepts the chosen export. > + > + Additionally, the server MAY reply `NBD_REP_ERR_TLS_REQD` to s/reply/reply with/ > + requests for exports that are unknown if TLS has not been are unknown, instead of `NBD_REP_ERR_UNKNOWN`, if > + negotiated. This is so that clients cannot without TLS s/cannot without TLS/without TLS cannot/ > + enumerate exports. > + > + In the case of `NBD_REP_SERVER`, the message's data takes on a different > interpretation than the default (so as to provide additional > binary information normally sent in reply to > `NBD_OPT_EXPORT_NAME`, in place of the default UTF-8 free-form > string); this layout is shared with the successful response to > - `NBD_OPT_GO`. The option reply length MUST be *length of > - name* + 14, and the option data has the following layout: > - > - - 32 bits, length of name (unsigned) > - - Name of the export. This name MAY be different from the one > - given in the `NBD_OPT_SELECT` option in case the server has > - multiple alternate names for a single export. > - - 64 bits, size of the export in bytes (unsigned) > - - 16 bits, transmission flags > + `NBD_OPT_GO`. The option reply length MUST be > + *length of name* + 14, and the option data has the following layout: > + > + - 32 bits, length of name (unsigned) > + - Name of the export. This name MAY be different from the one > + given in the `NBD_OPT_SELECT` option in case the server has Indentation. Maybe "given in the `NBD_OPT_SELECT` or `NBD_OPT_GO` option" > + multiple alternate names for a single export, or a default > + export was specified. > + - 64 bits, size of the export in bytes (unsigned) > + - 16 bits, transmission flags. The values of the transmission flags > + MAY differ from what was sent earlier in response to > + an earlier `NBD_OPT_SELECT` (if any), based on other options > + that were negotiated in the meantime. > + > + The values of the transmission flags > + MAY differ from what was sent earlier in response to > + `NBD_OPT_SELECT`, based on other options that were negotiated in > + the meantime. This statement is given twice. It's only needed once, but maybe with this wording and layout: - 16 bits, transmission flags. The values of the transmission flags MAY differ from what was sent in response to an earlier `NBD_OPT_SELECT` (if any), based on... May also be worth a statement under NBD_OPT_SELECT that any reply other than NBD_REP_SERVER removes any prior selection made by an earlier NBD_OPT_SELECT (if any) (we already state that under NBD_OPT_GO, but repeating or moving it here makes more sense, since it is the failure of NBD_OPT_SELECT and not the action of NBD_OPT_GO that clears a selection). Also, we may want to make sure that a failed NBD_OPT_GO also wipes out the current selection. > > - For backwards compatibility, clients should be prepared to also > handle `NBD_REP_ERR_UNSUP`. In this case, they should fall back to > @@ -699,34 +720,56 @@ option reply type. > * `NBD_OPT_GO` > > The client wishes to terminate the negotiation phase and progress to > - the transmission phase. Possible replies from the server include: > + the transmission phase. This client MAY issue this command after s/This client/The client/ > + an `NBD_OPT_SELECT`, or MAY issue it without a previous > + `NBD_OPT_SELECT`. > > - - Servers MUST NOT send `NBD_REP_ERR_UNSUP` as a reply to this > - message if they do not also send it as a reply to the > - `NBD_OPT_SELECT` message. > + Data: > + > + - Name of the export (as with `NBD_OPT_EXPORT_NAME`, the length > + comes from the option header). > + > + If no name is specified (i.e. a zero length string is provided) > + then the export selected with the immediately previous valid > + `NBD_OPT_SELECT` will be selected (if any), or if no previous > + `NBD_OPT_SELECT` valid has been issued, the default export will be > + selected. Reads awkwardly. How about: If no name is specified (i.e. a zero length string is provided), the operation reuses the selection from the previous `NBD_OPT_SELECT` (if there was one and it was successful), otherwise it requests the server's default export. > + > + `NBD_OPT_GO` can thus be used as a substitute for `NBD_OPT_EXPORT_NAME` > + that returns errors. > + > + The server replies with one of the following: > + > + - `NBD_REP_ERR_UNKNOWN`: The chosen export does not exist on this > + server. > + - `NBD_REP_ERR_TLS_REQD`: The server does not wish to export this > + block device unless the client negotiates TLS first. > + - `NBD_REP_SERVER`: The server accepts the chosen export. > + - For backwards compatibility, clients should be prepared to also > + handle `NBD_REP_ERR_UNSUP`. In this case, they should fall back to > + using `NBD_OPT_EXPORT_NAME`. We lost the bit about servers MUST NOT fail with NBD_REP_ERR_UNSUP on NBD_OPT_GO unless they also fail NBD_OPT_SERVER in the same manner. Basically, we want to guarantee that servers implement both options at the same time, even if a client can get away with using only NBD_OPT_GO. > + > + Additionally, the server MAY reply `NBD_REP_ERR_TLS_REQD` to > + requests for exports that are unknown if TLS has not been > + negotiated. This is so that clients cannot without TLS > + enumerate exports. Copy whatever wording fixes you make above to this spot as well. > + > + The format of `NBD_REP_SERVER` is identical to the reply > + for `NBD_OPT_SELECT`, except for the fact that both client > + and server subseequently enter the transmission phase. By using this s/subseequently/subsequently/ > + reply the server acknowledges the connection, using the same > + format for the reply as in `NBD_OPT_SELECT` (thus allowing the client > + to receive the same transmission flags as would have been sent > + by `NBD_OPT_EXPORT_NAME`); as per the note therein these > + transmission flags MAY differ from those returned by any > + previous `NBD_OPT_SELECT`. The server MUST NOT send any zero > + padding bytes after the `NBD_REP_SERVER` data, whether or not the > + client negotiated the `NBD_FLAG_C_NO_ZEROES` flag. After sending > + this reply the server MUST immediately move to the transmission > + phase, and after receiving this reply, the MUST immediately move s/the MUST/the client MUST/ > + to the transmission phase; therefore, the server MUST NOT send this > + particular reply until all other pending option requests have > + had their final reply. > > ### `WRITE_ZEROES` extension > > Overall makes sense. I wonder if we can compress things further by stating something along the lines of: * `NBD_OPT_GO` Identical in behavior to `NBD_OPT_SELECT`, except for: and then list just the differences (move into transmission phase on success, transmission flags may differ, and empty string can be special cased to reuse an earlier selection), rather than copying everything verbatim.
Eric, In brief I agree with all of that. I'm tempted to redo it so it's much simpler now we all seem to agree that carrying state is a bad thing. Alex On 5 Apr 2016, at 18:48, Eric Blake <eblake@redhat.com> wrote: > On 04/05/2016 10:38 AM, Alex Bligh wrote: >> Amend the NBD_OPT_SELECT and NBD_OPT_GO documentation as >> follows: >> >> * Allow a name to be specified on NBD_OPT_GO >> >> * Make clear the rules for default device selection >> >> * Remove the provision concerning TLS resetting device selection >> >> * Remove NBD_REP_ERR_INVALID as a reply to NBD_OPT_GO as there >> is now no necessity for a prior NBD_OPT_SELECT > > I guess if the client requests "" without earlier SELECT, and the server > has no default, the reply would be NBD_REP_ERR_UNKNOWN rather than > ERR_INVALID. > >> >> * Make it clear NBD_OPT_GO is in effect a better alternative >> for NBD_OPT_EXPORT_NAME >> >> * Make it clear the NBD_OPT_SELECT and NBD_OPT_GO are in >> essence the same command, save that NBD_OPT_GO puts you >> into transmission mode if successful. >> >> * Clarify permitted option returns outside TLS to prevent >> export enumeration. >> >> * Controversial: remove 'length' 32 bit quantity from >> NBD_OPT_SELECT (and don't copy it into NBD_OPT_GO) so it >> looks exactly like NBD_OPT_EXPORT_NAME bar the reply. >> This length is unnecessary as it's in the option header >> anyway. > > Makes sense to me; we could drop the 'Controversial' marker, unless > anyone can come up with a reason why we'd ever want a structure with > more than just the export name as the data passed along with SELECT/GO, > where having the header length distinct from the name length would let > us pass the extra data without needing yet another NBD_OPT_ command. > > >> @@ -676,21 +682,36 @@ option reply type. >> server. >> - `NBD_REP_ERR_TLS_REQD`: The server does not wish to export this >> block device unless the client negotiates TLS first. >> - - `NBD_REP_SERVER`: The server accepts the chosen export. In this >> - case, the `NBD_REP_SERVER` message's data takes on a different >> + - `NBD_REP_SERVER`: The server accepts the chosen export. >> + >> + Additionally, the server MAY reply `NBD_REP_ERR_TLS_REQD` to > > s/reply/reply with/ > >> + requests for exports that are unknown if TLS has not been > > are unknown, instead of `NBD_REP_ERR_UNKNOWN`, if > >> + negotiated. This is so that clients cannot without TLS > > s/cannot without TLS/without TLS cannot/ > >> + enumerate exports. >> + >> + In the case of `NBD_REP_SERVER`, the message's data takes on a different >> interpretation than the default (so as to provide additional >> binary information normally sent in reply to >> `NBD_OPT_EXPORT_NAME`, in place of the default UTF-8 free-form >> string); this layout is shared with the successful response to >> - `NBD_OPT_GO`. The option reply length MUST be *length of >> - name* + 14, and the option data has the following layout: >> - >> - - 32 bits, length of name (unsigned) >> - - Name of the export. This name MAY be different from the one >> - given in the `NBD_OPT_SELECT` option in case the server has >> - multiple alternate names for a single export. >> - - 64 bits, size of the export in bytes (unsigned) >> - - 16 bits, transmission flags >> + `NBD_OPT_GO`. The option reply length MUST be >> + *length of name* + 14, and the option data has the following layout: >> + >> + - 32 bits, length of name (unsigned) >> + - Name of the export. This name MAY be different from the one >> + given in the `NBD_OPT_SELECT` option in case the server has > > Indentation. Maybe "given in the `NBD_OPT_SELECT` or `NBD_OPT_GO` option" > >> + multiple alternate names for a single export, or a default >> + export was specified. >> + - 64 bits, size of the export in bytes (unsigned) >> + - 16 bits, transmission flags. The values of the transmission flags >> + MAY differ from what was sent earlier in response to >> + an earlier `NBD_OPT_SELECT` (if any), based on other options >> + that were negotiated in the meantime. >> + >> + The values of the transmission flags >> + MAY differ from what was sent earlier in response to >> + `NBD_OPT_SELECT`, based on other options that were negotiated in >> + the meantime. > > This statement is given twice. It's only needed once, but maybe with > this wording and layout: > > - 16 bits, transmission flags. > > The values of the transmission flags MAY differ from what was sent in > response to an earlier `NBD_OPT_SELECT` (if any), based on... > > May also be worth a statement under NBD_OPT_SELECT that any reply other > than NBD_REP_SERVER removes any prior selection made by an earlier > NBD_OPT_SELECT (if any) (we already state that under NBD_OPT_GO, but > repeating or moving it here makes more sense, since it is the failure of > NBD_OPT_SELECT and not the action of NBD_OPT_GO that clears a > selection). Also, we may want to make sure that a failed NBD_OPT_GO > also wipes out the current selection. > >> >> - For backwards compatibility, clients should be prepared to also >> handle `NBD_REP_ERR_UNSUP`. In this case, they should fall back to >> @@ -699,34 +720,56 @@ option reply type. >> * `NBD_OPT_GO` >> >> The client wishes to terminate the negotiation phase and progress to >> - the transmission phase. Possible replies from the server include: >> + the transmission phase. This client MAY issue this command after > > s/This client/The client/ > >> + an `NBD_OPT_SELECT`, or MAY issue it without a previous >> + `NBD_OPT_SELECT`. >> > >> - - Servers MUST NOT send `NBD_REP_ERR_UNSUP` as a reply to this >> - message if they do not also send it as a reply to the >> - `NBD_OPT_SELECT` message. >> + Data: >> + >> + - Name of the export (as with `NBD_OPT_EXPORT_NAME`, the length >> + comes from the option header). >> + >> + If no name is specified (i.e. a zero length string is provided) >> + then the export selected with the immediately previous valid >> + `NBD_OPT_SELECT` will be selected (if any), or if no previous >> + `NBD_OPT_SELECT` valid has been issued, the default export will be >> + selected. > > Reads awkwardly. How about: > > If no name is specified (i.e. a zero length string is provided), the > operation reuses the selection from the previous `NBD_OPT_SELECT` (if > there was one and it was successful), otherwise it requests the server's > default export. > >> + >> + `NBD_OPT_GO` can thus be used as a substitute for `NBD_OPT_EXPORT_NAME` >> + that returns errors. >> + >> + The server replies with one of the following: >> + >> + - `NBD_REP_ERR_UNKNOWN`: The chosen export does not exist on this >> + server. >> + - `NBD_REP_ERR_TLS_REQD`: The server does not wish to export this >> + block device unless the client negotiates TLS first. >> + - `NBD_REP_SERVER`: The server accepts the chosen export. >> + - For backwards compatibility, clients should be prepared to also >> + handle `NBD_REP_ERR_UNSUP`. In this case, they should fall back to >> + using `NBD_OPT_EXPORT_NAME`. > > We lost the bit about servers MUST NOT fail with NBD_REP_ERR_UNSUP on > NBD_OPT_GO unless they also fail NBD_OPT_SERVER in the same manner. > Basically, we want to guarantee that servers implement both options at > the same time, even if a client can get away with using only NBD_OPT_GO. > >> + >> + Additionally, the server MAY reply `NBD_REP_ERR_TLS_REQD` to >> + requests for exports that are unknown if TLS has not been >> + negotiated. This is so that clients cannot without TLS >> + enumerate exports. > > Copy whatever wording fixes you make above to this spot as well. > >> + >> + The format of `NBD_REP_SERVER` is identical to the reply >> + for `NBD_OPT_SELECT`, except for the fact that both client >> + and server subseequently enter the transmission phase. By using this > > s/subseequently/subsequently/ > >> + reply the server acknowledges the connection, using the same >> + format for the reply as in `NBD_OPT_SELECT` (thus allowing the client >> + to receive the same transmission flags as would have been sent >> + by `NBD_OPT_EXPORT_NAME`); as per the note therein these >> + transmission flags MAY differ from those returned by any >> + previous `NBD_OPT_SELECT`. The server MUST NOT send any zero >> + padding bytes after the `NBD_REP_SERVER` data, whether or not the >> + client negotiated the `NBD_FLAG_C_NO_ZEROES` flag. After sending >> + this reply the server MUST immediately move to the transmission >> + phase, and after receiving this reply, the MUST immediately move > > s/the MUST/the client MUST/ > >> + to the transmission phase; therefore, the server MUST NOT send this >> + particular reply until all other pending option requests have >> + had their final reply. >> >> ### `WRITE_ZEROES` extension >> >> > > Overall makes sense. I wonder if we can compress things further by > stating something along the lines of: > > * `NBD_OPT_GO` > > Identical in behavior to `NBD_OPT_SELECT`, except for: > > and then list just the differences (move into transmission phase on > success, transmission flags may differ, and empty string can be special > cased to reuse an earlier selection), rather than copying everything > verbatim. > > -- > Eric Blake eblake redhat com +1-919-301-3266 > Libvirt virtualization library http://libvirt.org > -- Alex Bligh
diff --git a/doc/proto.md b/doc/proto.md index 35a3266..06601fb 100644 --- a/doc/proto.md +++ b/doc/proto.md @@ -659,6 +659,8 @@ To remedy this, a `SELECT` extension is envisioned. This extension adds two option requests and one error reply type, and extends one existing option reply type. +Both options have identical formats for requests and replies. + * `NBD_OPT_SELECT` The client wishes to select the export with the given name for use @@ -667,8 +669,12 @@ option reply type. Data: - - 32 bits, length of name (unsigned) - - Name of the export + - Name of the export (as with `NBD_OPT_EXPORT_NAME`, the length + comes from the option header). + + If no name is specified (i.e. a zero length string is provided), + the default export (if any) is specified, as with + `NBD_OPT_EXPORT_NAME`. The server replies with one of the following: @@ -676,21 +682,36 @@ option reply type. server. - `NBD_REP_ERR_TLS_REQD`: The server does not wish to export this block device unless the client negotiates TLS first. - - `NBD_REP_SERVER`: The server accepts the chosen export. In this - case, the `NBD_REP_SERVER` message's data takes on a different + - `NBD_REP_SERVER`: The server accepts the chosen export. + + Additionally, the server MAY reply `NBD_REP_ERR_TLS_REQD` to + requests for exports that are unknown if TLS has not been + negotiated. This is so that clients cannot without TLS + enumerate exports. + + In the case of `NBD_REP_SERVER`, the message's data takes on a different interpretation than the default (so as to provide additional binary information normally sent in reply to `NBD_OPT_EXPORT_NAME`, in place of the default UTF-8 free-form string); this layout is shared with the successful response to - `NBD_OPT_GO`. The option reply length MUST be *length of - name* + 14, and the option data has the following layout: - - - 32 bits, length of name (unsigned) - - Name of the export. This name MAY be different from the one - given in the `NBD_OPT_SELECT` option in case the server has - multiple alternate names for a single export. - - 64 bits, size of the export in bytes (unsigned) - - 16 bits, transmission flags + `NBD_OPT_GO`. The option reply length MUST be + *length of name* + 14, and the option data has the following layout: + + - 32 bits, length of name (unsigned) + - Name of the export. This name MAY be different from the one + given in the `NBD_OPT_SELECT` option in case the server has + multiple alternate names for a single export, or a default + export was specified. + - 64 bits, size of the export in bytes (unsigned) + - 16 bits, transmission flags. The values of the transmission flags + MAY differ from what was sent earlier in response to + an earlier `NBD_OPT_SELECT` (if any), based on other options + that were negotiated in the meantime. + + The values of the transmission flags + MAY differ from what was sent earlier in response to + `NBD_OPT_SELECT`, based on other options that were negotiated in + the meantime. - For backwards compatibility, clients should be prepared to also handle `NBD_REP_ERR_UNSUP`. In this case, they should fall back to @@ -699,34 +720,56 @@ option reply type. * `NBD_OPT_GO` The client wishes to terminate the negotiation phase and progress to - the transmission phase. Possible replies from the server include: + the transmission phase. This client MAY issue this command after + an `NBD_OPT_SELECT`, or MAY issue it without a previous + `NBD_OPT_SELECT`. - - `NBD_REP_SERVER`: The server acknowledges, using the same format - for the reply as in `NBD_OPT_SELECT` (thus allowing the client - to receive the same transmission flags as would have been sent - by `NBD_OPT_EXPORT_NAME`). The values of the transmission flags - MAY differ from what was sent earlier in response to - `NBD_OPT_SELECT`, based on other options that were negotiated in - the meantime. The server MUST NOT send any zero padding bytes - after the `NBD_REP_SERVER` data, whether or not the client - negotiated the `NBD_FLAG_C_NO_ZEROES` flag. After receiving - this reply, the client and the server have both moved to the - transmission phase; therefore, the server MUST NOT send this - particular reply until all other pending option requests have - had their final reply. - - `NBD_REP_ERR_INVALID`: No `NBD_OPT_SELECT` command was - previously issued during this negotiation (there is no default); - or, the most recent `NBD_OPT_SELECT` command resulted in an error - reply (selecting a different export clears the currently selected - export, even if the new export does not exist on the server); or, - the most recent `NBD_OPT_SELECT` command issued during this - negotiation occurred before TLS was successfully negotiated - (negotiating TLS clears the selected export). - - Servers MAY also choose to send `NBD_REP_ERR_TLS_REQD` rather than - `NBD_REP_ERR_INVALID` if no non-TLS exports are supported. - - Servers MUST NOT send `NBD_REP_ERR_UNSUP` as a reply to this - message if they do not also send it as a reply to the - `NBD_OPT_SELECT` message. + Data: + + - Name of the export (as with `NBD_OPT_EXPORT_NAME`, the length + comes from the option header). + + If no name is specified (i.e. a zero length string is provided) + then the export selected with the immediately previous valid + `NBD_OPT_SELECT` will be selected (if any), or if no previous + `NBD_OPT_SELECT` valid has been issued, the default export will be + selected. + + `NBD_OPT_GO` can thus be used as a substitute for `NBD_OPT_EXPORT_NAME` + that returns errors. + + The server replies with one of the following: + + - `NBD_REP_ERR_UNKNOWN`: The chosen export does not exist on this + server. + - `NBD_REP_ERR_TLS_REQD`: The server does not wish to export this + block device unless the client negotiates TLS first. + - `NBD_REP_SERVER`: The server accepts the chosen export. + - For backwards compatibility, clients should be prepared to also + handle `NBD_REP_ERR_UNSUP`. In this case, they should fall back to + using `NBD_OPT_EXPORT_NAME`. + + Additionally, the server MAY reply `NBD_REP_ERR_TLS_REQD` to + requests for exports that are unknown if TLS has not been + negotiated. This is so that clients cannot without TLS + enumerate exports. + + The format of `NBD_REP_SERVER` is identical to the reply + for `NBD_OPT_SELECT`, except for the fact that both client + and server subseequently enter the transmission phase. By using this + reply the server acknowledges the connection, using the same + format for the reply as in `NBD_OPT_SELECT` (thus allowing the client + to receive the same transmission flags as would have been sent + by `NBD_OPT_EXPORT_NAME`); as per the note therein these + transmission flags MAY differ from those returned by any + previous `NBD_OPT_SELECT`. The server MUST NOT send any zero + padding bytes after the `NBD_REP_SERVER` data, whether or not the + client negotiated the `NBD_FLAG_C_NO_ZEROES` flag. After sending + this reply the server MUST immediately move to the transmission + phase, and after receiving this reply, the MUST immediately move + to the transmission phase; therefore, the server MUST NOT send this + particular reply until all other pending option requests have + had their final reply. ### `WRITE_ZEROES` extension
Amend the NBD_OPT_SELECT and NBD_OPT_GO documentation as follows: * Allow a name to be specified on NBD_OPT_GO * Make clear the rules for default device selection * Remove the provision concerning TLS resetting device selection * Remove NBD_REP_ERR_INVALID as a reply to NBD_OPT_GO as there is now no necessity for a prior NBD_OPT_SELECT * Make it clear NBD_OPT_GO is in effect a better alternative for NBD_OPT_EXPORT_NAME * Make it clear the NBD_OPT_SELECT and NBD_OPT_GO are in essence the same command, save that NBD_OPT_GO puts you into transmission mode if successful. * Clarify permitted option returns outside TLS to prevent export enumeration. * Controversial: remove 'length' 32 bit quantity from NBD_OPT_SELECT (and don't copy it into NBD_OPT_GO) so it looks exactly like NBD_OPT_EXPORT_NAME bar the reply. This length is unnecessary as it's in the option header anyway. Signed-off-by: Alex Bligh <alex@alex.org.uk> --- doc/proto.md | 123 ++++++++++++++++++++++++++++++++++++++++------------------- 1 file changed, 83 insertions(+), 40 deletions(-)