rust-vsock / vsock-rs

Virtio socket support for Rust
https://crates.io/crates/vsock
Apache License 2.0
27 stars 19 forks source link

Get rid of the dependency on the nix version #12

Closed Tim-Zhang closed 3 years ago

Tim-Zhang commented 3 years ago

VsockStream::connect(nix::sys::socket::VsockAddr) should be VsockStream::connect(cid, port) VsockListener::bind(nix::sys::socket::VsockAddr) should be VsockListener::bind(cid, port)

After this change, our users can use different nix version with vsock-rs.

@jeromegn @elmarco @qwandor What do you guys think?

qwandor commented 3 years ago

That's fine by me, though the other option is to re-export the VsockAddr type from nix as part of the vsock-rs API.

jeromegn commented 3 years ago

I think accepting cid, port directly makes sense. Especially since there's nothing more you can do with a VsockAddr before using it with connect or bind.

elmarco commented 3 years ago

Works for me.

Tim-Zhang commented 3 years ago

But I am still not sure what to do with the return value like pub fn peer_addr(&self) -> Result<SockAddr> and pub fn local_addr(&self) -> Result<SockAddr> . does re-export work for them? @qwandor

I think there are only two ways:

  1. return a struct defined in vsock-rs
  2. just ignore return values, keep it as it is.

Do you have any good ideas?

elmarco commented 3 years ago

What about simply Result<(u32,u32)> ?

qwandor commented 3 years ago

I think re-exporting is a better solution than defining a new struct for the same thing. Then it is compatible with the nix type if a user of the library does choose to use the same version of nix, and otherwise they can still access the fields they need.