Trying to create xdr bindings for Glusterfs

[cholcombe973]

I really love this project! This is the first rust xdr based project that has been able to parse any gluster .x file and successfully produce an output file. I tried it on a second file and it EOF failed. Maybe you could take a quick peek and see what you think. The file is: https://github.com/gluster/glusterfs/blob/master/rpc/xdr/src/rpc-common-xdr.x I think the parser is expecting more to the file.

[jsgf]

Cool! How is it failing for you? Does it print an error or does it just hang or something? How are you invoking it?

[jsgf]

Looks like its upset by unsigned groups[16]; - it's expecting unsigned int. Should be able to fix that up.

Hm, it also doesn't like struct gf_prog_detail *next; and struct gf_prog_detail *prog;. Will need to look into that.

[jsgf]

A workaround is to remove "struct" from those lines. XDR doesn't require the keyword there; I'm not sure it's actually valid syntax.

Also it would generate slightly better code with a typedef unsigned hyper u_quad_t in that .x file.

[cholcombe973]

Cool! Thanks for the quick response. I'll make those changes and let you
know in a little bit. This could very well be why the other parsers I tried
failed. Glusters xdr code is strange
On May 16, 2016 4:39 PM, "Jeremy Fitzhardinge" [email protected]
wrote:

A workaround is to remove "struct" from those lines. XDR doesn't require
the keyword there; I'm not sure it's actually valid syntax.

Also it would generate slightly better code with a typedef unsigned hyper
u_quad_t in that .x file.

—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#6 (comment)

[cholcombe973]

Ok I messed around with the file and used your suggestions to create this: https://gist.github.com/cholcombe973/96ced58add90320b740123b5c066740d That seems to have done the trick! It generated this code: https://gist.github.com/cholcombe973/4318ffa2aae17b8c5dde18a7f2b6a1d5. I think I might cat all these files together and then throw them up on crates.io as a Gluster xdr raw bindings crate.

[cholcombe973]

@jsgf Here's the everything.x https://gist.github.com/cholcombe973/d0785e1bd243c226646d42202b1804c4 The parser seems to not like the unsigned long struct members.

[cholcombe973]

Interesting. From the xdr rfc it doesn't look like long is a valid keyword.

[jsgf]

RFC4506 doesn't define "long" as a basic type in .x files; unsigned hyper is its 64 bit unsigned type. rpcgen does accept unsigned long and maps it to u_long.

[jsgf]

Also unsigned char uuid[16] not defined but rpcgen generates this as an array of 32-byte entries, which are bytes padded to 32 bits (ie, 4x16 = 64 bytes on the wire). This should probably be opaque uuid[16] to get the desired effect.

[cholcombe973]

Yeah I'm going to open a bug with redhat and see what they say about this. This is probably the reason why no other language bindings for gluster exist. :-/

[cholcombe973]

https://bugzilla.redhat.com/show_bug.cgi?id=1336889 bug opened

[jsgf]

Also RFC4506 doesn't include the RPC definition syntax, so xdrgen doesn't either.

I'm not sure if "struct foo" syntax is valid or not, or rather, if I should make it valid. The RFC does say that struct foo { int a; int b; }; is equivalent to typedef struct { int a; int b; } foo;, which implies that foo is a type name rather than a structure tag (as it would be in C), so struct foo is not naming a type.

[jsgf]

Another hiccup: priv and type are Rust keywords which the .x file uses, so the generated rust code is invalid. xdrgen doesn't attempt to remap symbols with rust keywords, so you need to modify the definition (I changed priv->private and type->ty for test purposes).

So the complete set of changes to get something working with xdrgen:

1. change unsigned long -> unsigned hyper
2. change unsigned char -> u8 (for compatibility, but it really should be opaque, but that would break the wire format)
3. change references to struct foo to plain foo in field definitions
4. exclude RPC program definitions
5. fix symbols which are Rust keywords

I'm considering adding support for struct foo in xdrgen (point 3), depending on how complex it turns out to be, because it seems harmless enough.

[cholcombe973]

Interesting. I also had to typedef a bunch of things that gluster forgot to do for some reason. I deleted all the struct foo's in there and that seems to have gotten it to generate. On the cargo build side it looks like the major issue is no method namedpackfound for typestd::vec::Vec` I updated my everything.x to show the typedef's and things I had to do :)

[jsgf]

A Vec of which type?

[cholcombe973]

oh sorry. It was a Vec<u8>

[jsgf]

It shouldn't be relying on pack/unpack for Vec<T>; it should be generating calls to xdr_codec::pack_array() or pack_flex() to make sure all the size bounds are handled correctly handled. Can you narrow down a test case?

[cholcombe973]

Yeah. It looks like this is causing the issue: https://gist.github.com/cholcombe973/d0785e1bd243c226646d42202b1804c4#file-everything-x-L1654

[jsgf]

I see. Hm.

[cholcombe973]

@jsgf Do you think it would be worthwhile to generate the Rust code with camelcase? I'm getting a ton of warnings. I can just turn them off but I was wondering

[jsgf]

Well, that opens the whole question of what transformations to apply to names from .x -> Rust. At the very least it should probably do something to avoid rust keywords in generated code. But what past that? How to make sure it doesn't create conflicting symbols?

For now, I just add an #[allow(dead_code, non_camel_case_types)] to keep the compiler quiet.

[cholcombe973]

I guess it depends on how much global knowledge the generator has. If it
has a lot it would be easier to rename things. I'll do the same thing as
you to keep the compiler quiet.

On Wed, May 18, 2016, 2:13 PM Jeremy Fitzhardinge [email protected]
wrote:

Well, that opens the whole question of what transformations to apply to
names from .x -> Rust. At the very least it should probably do something to
avoid rust keywords in generated code. But what past that? How to make sure
it doesn't create conflicting symbols?

For now, I just add an #[allow] to keep the compiler quiet.

—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#6 (comment)

[jsgf]

BTW, I just pushed a change to fix the typedef opaque foo[123] problem; it now generates a Rust tuple struct type to wrap it - pub struct foo(pub Vec<u8>); - which implements Pack/Unpack.

[jsgf]

I also just pushed support for parsing rpcgen extensions: "unsigned" -> "unsigned int" and "struct foo" -> "foo" as a type name. I didn't support "unsigned long" because it's ambiguous ("long" isn't a keyword, so in principle it could be a field name), and also unclear what it means (I'd assumed it was a 64-bit value, but I think its actually 32 bits).

[cholcombe973]

Cool. I'll try and run xdrgen against some of the gluster files and see what it says :)

[cholcombe973]

Looks much better! I posted the code here: https://github.com/cholcombe973/gluster-xdr

[cholcombe973]

Now that I think about it. I think I have some tcp packet captures of gluster rpc that I can test this generated code against to see if it matches

[jsgf]

Yeah, that would be great. Interop testing is a big hole in the test suite.

[cholcombe973]

@jsgf have you considered writing an NFS server with this code? It should be possible and it would gain rust safety :)

[jsgf]

There's a lot more to NFS than XDR...

[cholcombe973]

Right but the foundation is there :-). It would be a huge undertaking. I
was looking at libnfs the other day (it's a client library) and started
wondering what it would take to build the server side

On Sep 15, 2016 8:26 PM, "Jeremy Fitzhardinge" [email protected]
wrote:

There's a lot more to NFS than XDR...

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#6 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AC6qExT8lZsrisMrdCiNvgKdUgSeGrxuks5qqgxQgaJpZM4Ifxta
.

[cholcombe973]

So I took a look at generating the xdr bindings again. It's really close!
This one failed because type is a reserved word: glusterfs3-xdr.x and this one worked just fine: rpc-common

[jsgf]

Check out the version in master - I added remapping for reserved words (adds a _). It also builds in stable.

[cholcombe973]

Ok I pointed it at master. It looks like it's really close:

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Generic("incompat selector Named(\"fop_enum\", Ident(\"glusterfs_fop_t\")) case Ident(\"GF_FOP_STAT\")")', /buildslave/rust-buildbot/slave/stable-dist-rustc-linux/build/src/libcore/result.rs:837

[jsgf]

Yeah, I got the same. I think you need to manually inline glusterfs-fops.x to get its definitions (xdrgen can't do anything useful with %#include "glusterfs-fops.h"). You probably need to do the same for rpc-common-xdr.x.

[jsgf]

Yeah, there's several problems in the Gluster .x files:

• using types like quad_t, u_quad_t, which need type aliases in Rust
• constants like LEASE_ID_SIZE which aren't defined in the .x file
• using char foo[SIZE] instead of opaque foo[SIZE]

The last one I can add support to xdr-codec for, though I'm a bit concerned it would result in people accidentally using it when they didn't mean to. Maybe I should put it behind a feature flag.

[cholcombe973]

Yeah it wouldn't hurt to have it behind a feature flag. I agree their .x files are a little whacky. I suspect their Makefile either patches them up or there's an import somewhere that I'm missing.

…

On Wed, Mar 1, 2017 at 1:06 PM, Jeremy Fitzhardinge < ***@***.***> wrote: Yeah, there's several problems in the Gluster .x files: - using types like quad_t, u_quad_t, which need type aliases in Rust - constants like LEASE_ID_SIZE which aren't defined in the .x file - using char foo[SIZE] instead of opaque foo[SIZE] The last one I can add support to xdr-codec for, though I'm a bit concerned it would result in people accidentally using it when they didn't mean to. Maybe I should put it behind a feature flag. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#6 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AC6qE-GbXQ7_kAuTWS8AMD20XqW_yeM_ks5rhd3WgaJpZM4Ifxta> .

[jsgf]

I think they work OK in C. It just defines things at the C level knowing that the generated code can access them and understand the types.

[jsgf]

OK, I just published xdrgen and xdr-codec 0.4.0. xdr-codec has a bytecodec feature flag to support pack/unpack on i8/u8.

[cholcombe973]

Sweet! Much appreciated.
Redhat just loves to produce broken .x files. I'm trying to generate against this .x file now: https://github.com/gluster/gluster-block/blob/master/rpc/rpcl/block.x I think the nested enum's at the end are what is breaking it. I commented them out for now.

[cholcombe973]

Looks like most of the errors are just rust complaining about Debug and Eq not being implemented for i8: https://gist.github.com/cholcombe973/c4b85e45884ea4bc795b02a7885bbd1b. Rust stops at 32 for Debug: impl Debug for [T; 32] where T: Debug

[cholcombe973]

So I enabled bytecodec and that got me further but I'm still running into a bunch of compile errors. I can get it to compile correctly by pulling in the generated code and editing it but I'd love to avoid doing that :). Here's a gist: https://gist.github.com/cholcombe973/d7a83647ae51cdcc1fcb2d390c27ee4c

[jsgf]

That's a rust limitation (you can only derive traits for arrays up to 32 elements) that I guess xdrgen could work around. Could you file a separate issue for it?

[jsgf]

Filed #18.

[jsgf]

I'm going to close this for now - file new issues if they come up.