-
Notifications
You must be signed in to change notification settings - Fork 65
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
Allow building on 32bit platforms without AtomicU64 #187
Comments
Created a PR for this, !188 |
Hi! |
Yes I know the README says it's for two architectures (amd64 & aarch64). Unfortunately, other projects started to use parts of this code, mainly eventd/epoll stuff, and this is causing.. issues. For the time being, the thing I'm trying to build is virtiofsd — https://gitlab.com/virtio-fs/virtiofsd — with its dependencies. This is needed for qemu (which works on many platforms), where, starting with version 8.0, they switched from internal C-language implementation of virtiofsd to external rust-language one. And it turned out quite some crates use stuff provided by vmm-sys-utils. Maybe it's a good idea to package some of those bits as separate crates, even including into the std rust runtime (as those are mainly wrappers for the syscalls). |
Ah, I see. I'm a bit afraid here that vmm-sys-util is just the tip of the iceberg, and that you'll discover more issues further down the line (virtiofds pulls in 4 more rust-vmm dependencies, and all of the have the same supported platforms as vmm-sys-util). But for now, conditionally compiling out the parts that don't work on x32 (or other platforms) sounds reasonable to me. Another thing to keep in mind though is that we only continuously test on x86_64 and aarch64 platforms currently, so even if everything compiles, there's no guarantee that the result is "correct" on other platforms. If 32bit support is desired, there'd need to be people willing to maintain it, and integrate it into our testing infrastructure to make sure that it doesn't get accidentally broken again further down the line. |
FWIW, also opened https://gitlab.com/virtio-fs/virtiofsd/-/issues/94 to see what virtiofsd upstream thinks about supported archs/targets |
I looked at this more closely. While I don't have preferences for other 32bit platforms which lacks 64bit atomics for now, I do have a word about x32. This one does not need extra efforts, - if it doesn't work and needs some special care to maintain, those efforts are better spent elsewhere. The c_long vs i64 (the other issue) is worth to fix anyway (which will bring x32 in line with the rest of 32bit arches) Speaking of conditionally compiling 64bi atomics, unfortunately this wont help, as other crates (which are in deps of virtiofsd) use vmm_sys_utils::metrics anyway. It will let vmm-sys-utils itself to compile but not other more useful crates. |
I recently learned of https://github.com/taiki-e/portable-atomic It can be used in architectures that don't have AtomicU64. |
FWIW in Debian we gave up on packaging this stack for 32-bit.. |
Hi!
This code has src/metric.rs, which unconditionally uses std::sync::atomic::AtomicU64. Quite some platforms don't have 64-bit atomics with current rust, namely it is armel, mipsel, powerpc and x32 (heh). Maybe the metric module can be guarded with the right #[cfg target_has_atomic] ?
Trying to package some stuff for Debian, this crate come as a dependency...
Thanks!
The text was updated successfully, but these errors were encountered: