Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make the crate independent of xenstore C library #23

Merged
merged 8 commits into from
Dec 10, 2024

Conversation

TSnake41
Copy link
Contributor

@TSnake41 TSnake41 commented Nov 7, 2024

Add a pure rust xenstore implementation to replace the C binding.

xenstore is exposed to useland by two interfaces :

  • xenstored unix socket (usually /run/xenstored/socket)
  • /dev/xen/xenbus device (or /proc/xen/xenbus (deprecated path)) (or another platform specific path)

(depending on whether the domain hosts xenstored or not)

Those two interfaces follow a same protocol as defined in xs_wire.h 1 and xenstore.txt 2 using regular read/write syscalls.

Implement this protocol using pure-Rust and standard library and replace the C binding by this implementation.

@TSnake41 TSnake41 changed the title Make the crate independent of xenstore C libraray Make the crate independent of xenstore C library Nov 7, 2024
@TSnake41 TSnake41 force-pushed the pure-rust branch 2 times, most recently from 0083138 to 86cc523 Compare November 12, 2024 16:13
@TSnake41
Copy link
Contributor Author

TSnake41 commented Nov 12, 2024

Changes in b510c8f would superseed #4

@TSnake41 TSnake41 force-pushed the pure-rust branch 8 times, most recently from 2e77900 to 49a5774 Compare November 21, 2024 11:26
@TSnake41 TSnake41 marked this pull request as ready for review November 21, 2024 11:29
@TSnake41
Copy link
Contributor Author

TSnake41 commented Nov 21, 2024

I think it is good for now, there is some stuff that could be improved for the future :

It would be useful to have a interface to xenstore management commands (GET_PERMS, SET_PERMS, INTRODUCE/RELEASE, ...). That's not a really difficult to implement though.

We can make the tokio implementation runtime-agnostic by relying only on futures crate and let the user provide its own way of reading XsMessage (e.g by Sink/Stream implementations but there are caveats for now rust-lang/futures-rs#2857); and move the tokio implementation to use it. It doesn't really increase the line count, but makes supporting this crate easier with another runtime (e.g smol, async-std, ...).
It also allows using arbitrary ways of reading XsMessage (testing, xenstore ring interface, ...).
I made a small experiment at https://github.com/TSnake41/xenstore-rs/tree/generic-async

We can do a similar thing with the synchronous implementation with a wrapper for some XsMessage reader/writer to make it implement Xs.

And if we need it, have a implementation for transactions.

Copy link
Owner

@Wenzel Wenzel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall LGTM
Thanks for the effort !

We can continue iterating on the design in future PRs as this one is already introducting a lot of changes.

Can you fix the CI (formatting) ?

Cargo.toml Outdated Show resolved Hide resolved
Make Xs a trait that define the XenStore API.
Add auxiliary traits for additional features.
Expose a async variant of Xs ((Local)AsyncXs), including watch ((Local)XsAsyncWatch).

Provide a standard library based implementation of Xs for Dom0 and DomU. (Unix platforms only)

Signed-off-by: Teddy Astie <teddy.astie@vates.tech>
Provide a tokio-based AsyncXs implementation.
This feature is guarded behind "async-tokio" feature.
It uses a underlying task that multiplexes the xenstore accesses and interacted by a channel under the hood.
XsTokio can be cloned and is Sync/Send for concurrent accesses to XenStore and provides watch support.

Signed-off-by: Teddy Astie <teddy.astie@vates.tech>
Add a example code that uses tokio instead of std implementation.

Signed-off-by: Teddy Astie <teddy.astie@vates.tech>
Needs --all-features to build/test with tokio.
Needed for cargo test to not fail on xenstore-cli-async.

Signed-off-by Teddy Astie <teddy.astie@vates.tech>
Fix possible deadlock when dropping multiples watchers that can clobber
the channel thus actually blocking "send_blocking" in watcher Drop.

Signed-off-by: Teddy Astie <teddy.astie@vates.tech>
I think that will make the life easier for xenstore crate users
(especially those that were using the old version)

Signed-off-by: Teddy Astie <teddy.astie@vates.tech>
xenstore doesn't require WRITE values to contain the NUL character,
if one is added to the payload, it will appear in xenstore.

Signed-off-by Teddy Astie <teddy.astie@vates.tech>
Signed-off-by Teddy Astie <teddy.astie@vates.tech>
@Wenzel Wenzel merged commit 20c0baf into Wenzel:master Dec 10, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants