Bug 1336889 - Gluster's XDR does not conform to RFC spec
Summary: Gluster's XDR does not conform to RFC spec
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: GlusterFS
Classification: Community
Component: rpc
Version: mainline
Hardware: All
OS: All
medium
unspecified
Target Milestone: ---
Assignee: Amar Tumballi
QA Contact:
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2016-05-17 17:23 UTC by Chris Holcombe
Modified: 2018-03-15 11:16 UTC (History)
6 users (show)

Fixed In Version: glusterfs-4.0.0
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2018-03-15 11:16:50 UTC
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Embargoed:


Attachments (Terms of Use)

Description Chris Holcombe 2016-05-17 17:23:05 UTC
Description of problem:  In an attempt to generate bindings to Gluster's XDR with Rust I've run into a few problems.  It appears the XDR .x spec files are using keywords that are not legal according to the RFC spec https://tools.ietf.org/html/rfc4506.html.  This hinders language adoption of Gluster.  If anyone would like to produce pure language bindings this prevents them from succeeding.  Indeed I think community adoption of Gluster would benefit from having a wider range of XDR bindings on which to build tooling.

Some of the problems I've encountered: 
1. RFC4506 doesn't define "long" as a basic type in .x files; unsigned hyper is its 64 bit unsigned type. 
2. Also unsigned char uuid[16] not defined
Possibly other problems that I haven't gotten to yet as the parser moves through the files.

Version-Release number of selected component (if applicable):


How reproducible: Always reproducible.


Steps to Reproduce:
1. git clone https://github.com/jsgf/rust-xdr
2. cargo build using rust nightly
3. rust-xdr/xdrgen/target/debug/xdrgen /home/user/glusterfs/rpc/xdr/src/everything.x

Actual results: The parser errors out.  Please refer to this troubleshooting session for more detailed information: https://github.com/jsgf/rust-xdr/issues/6


Expected results: Valid language bindings in Rust


Additional info:

Comment 2 Jeremy Fitzhardinge 2016-05-17 18:23:49 UTC
Rust xdrgen closely follows RFC4506's syntax, where as the normal "rpcgen" program seems to accept a lot more. I'm not sure where that's defined.

Specifically, the syntax problems are:

1. "unsigned long" is not a defined XDR type. This should be "unsigned hyper".

2. "unsigned char" is also not defined. Its uses in the form "unsigned char uuid[16]" are almost certainly not what was intended; this will serialize as an array of 16 bytes padded to 32 bits, generating 16x4=64 bytes on the wire. The correct form is "opaque uuid[16]", but changing this will break the wire format. A compatible fix is to define a "byte" or "uchar" type and use that: "byte uuid[16]" (in Rust, this would map to "u8")

3. "struct foo" is not a valid way to reference a struct type in a field. RFC4506 defines "struct foo { int a; int b; }" as being equivalent to "typedef struct { int a; int b; } foo", so "foo" is a type name. Therefore, references to it should simply be "foo".

(The Gluster .x files also specify RPC program definitions, which are not covered by RFC4506, so they're out of scope. The Rust binding will need to come up with something else for that.)

Comment 3 Niels de Vos 2016-05-18 08:20:29 UTC
I'm assuming that this bug was intended to be filed against the upstream Gluster Community project, and not the Red Hat Gluster Storage product. Moving the bug to the community project now.

Thanks for the clear description and additional notes. I'm not sure how easy this can get addressed without looking into the details. If you have suggestions or would like to assist in making the changes, please start a discussion about this on the gluster-devel list (I think you need to subscribe before posting).

Comment 4 Amar Tumballi 2017-10-24 14:00:53 UTC
Planning to not resolve below:

>> 2. "unsigned char" is also not defined. Its uses in the form "unsigned char uuid[16]" are almost certainly not what was intended; this will serialize as an array of 16 bytes padded to 32 bits, generating 16x4=64 bytes on the wire. The correct form is "opaque uuid[16]", but changing this will break the wire format. A compatible fix is to define a "byte" or "uchar" type and use that: "byte uuid[16]" (in Rust, this would map to "u8")

As it will cause the glusterd cluster break, which anyways is targeted to be taken out of support by 4.1.x releases.

Comment 5 Worker Ant 2017-11-14 13:50:18 UTC
REVIEW: https://review.gluster.org/18767 (xdr: comply with RFC4506) posted (#1) for review on master by Amar Tumballi

Comment 6 Worker Ant 2017-11-24 09:18:55 UTC
COMMIT: https://review.gluster.org/18767 committed in master by \"Amar Tumballi\" <amarts> with a commit message- xdr: comply with RFC4506

Change-Id: I03098a54b8d37f6201129007cf31b31d97c30a23
BUG: 1336889
Signed-off-by: Amar Tumballi <amarts>

Comment 7 Amar Tumballi 2018-01-30 05:38:16 UTC
The changes in mgmt rpc is not yet done as we are planning to retire glusterd1 by 4.3. Other than that the other changes are now part of master, and would be released as part of Gluster 4.0

Comment 8 Shyamsundar 2018-03-15 11:16:50 UTC
This bug is getting closed because a release has been made available that should address the reported issue. In case the problem is still not fixed with glusterfs-4.0.0, please open a new bug report.

glusterfs-4.0.0 has been announced on the Gluster mailinglists [1], packages for several distributions should become available in the near future. Keep an eye on the Gluster Users mailinglist [2] and the update infrastructure for your distribution.

[1] http://lists.gluster.org/pipermail/announce/2018-March/000092.html
[2] https://www.gluster.org/pipermail/gluster-users/


Note You need to log in before you can comment on or make changes to this bug.