rust-vsock / vsock-rs

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

Timeouts: SendTimeout/write ReadTimeout/read #30

Closed robem closed 1 year ago

jalil-salame commented 1 year ago

Please ammend your commit so that it follows the guidelines: https://github.com/kata-containers/community/blob/main/CONTRIBUTING.md#patch-format

Specifically, add a body and sign-off your commit.

With git commit --sign-off --amend you can change the commit message, then you can force push the changes to your repository (git push --force)

robem commented 1 year ago

This seems to be a simple change where the timeouts where swapped:

set_read_timeout would use SendTimeout instead of RecieveTimeout.

Looks good to me, but I couldn't find any documentation stating that the reads follow the RecieveTimeout, and writes the SendTiemout so if you could point me to some resource I will gladly approve it.

I discovered it while testing the API. Both timeouts translate into socket options (https://linux.die.net/man/3/setsockopt). "SO_RCVTIMEO Sets the timeout value that specifies the maximum amount of time an input function waits until it completes." Similar "SO_SNDTIMEO" but for "output functions".

jalil-salame commented 1 year ago

The body of the message should wrap at 72 characters, sorry for the hassle, here it is wrapped to 72 characters:

This change fixes the implementation of `set_read_timeout` and
`set_write_timeout` API. In the previous version, `set_read_timeout`
would set the write timeout and vice versa