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

virtio-bindings: use bindgen library from build.rs #327

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

stefanha
Copy link

Since commit 3e51e2d ("virtio-bindings: regenerate with bindgen 0.70.1") there are compile-time alignment checks in virtio-bindings code. These checks are specific to the architecture where bindgen was run and they can break builds on other architectures.

Here is an example i686 build failure after upgrading from virtio-bindings 0.2.2 to 0.2.4:
https://koji.fedoraproject.org/koji/taskinfo?taskID=127997322

The bindgen User Guide recommends generating bindings from build.rs because it avoids portability issues and the need to maintain a copy of the bindings for each target:
https://rust-lang.github.io/rust-bindgen/library-usage.html

Introduce an import-linux-headers.sh script that copies required virtio headers from a Linux headers tree into include/. Do not distribute the full Linux headers tree because it is large and has falls under various software licenses that are not compatible with this crate's license.

Generate the actual bindings at compile-time in build.rs and then include!() the output from src/lib.rs.

You can verify that the generated bindings have not changed significantly for x86_64 by diffing them against the previous commit. For example:

--- orig/virtio_gpu.rs
+++ new/virtio_gpu.rs
@@ -1,4 +1,4 @@
-/* automatically generated by rust-bindgen 0.70.1 */ +/* automatically generated by rust-bindgen 0.71.1 */

 #[repr(C)]
 #[derive(Default)]
@@ -58,7 +58,7 @@
 pub const VIRTIO_GPU_MAP_CACHE_WC: u32 = 3;
 pub type __u8 = ::std::os::raw::c_uchar;
 pub type __u32 = ::std::os::raw::c_uint;
-pub type __u64 = ::std::os::raw::c_ulonglong;
+pub type __u64 = ::std::os::raw::c_ulong;
 pub type __le32 = __u32;
 pub type __le64 = __u64;
 pub const virtio_gpu_ctrl_type_VIRTIO_GPU_UNDEFINED: virtio_gpu_ctrl_type = 0

The __u64 change is due to how <linux/types.h> defined __u64. We should avoid using that non-BSD licensed header. The result is equivalent now that the bindings are generated at compile-time.

Fixes: #326

Since commit 3e51e2d ("virtio-bindings: regenerate with bindgen 0.70.1")
there are compile-time alignment checks in virtio-bindings code. These
checks are specific to the architecture where bindgen was run and they
can break builds on other architectures.

Here is an example i686 build failure after upgrading from
virtio-bindings 0.2.2 to 0.2.4:
https://koji.fedoraproject.org/koji/taskinfo?taskID=127997322

The bindgen User Guide recommends generating bindings from build.rs
because it avoids portability issues and the need to maintain a copy of
the bindings for each target:
https://rust-lang.github.io/rust-bindgen/library-usage.html

Introduce an import-linux-headers.sh script that copies required virtio
headers from a Linux headers tree into include/. Do not distribute the
full Linux headers tree because it is large and has falls under various
software licenses that are not compatible with this crate's license.

Generate the actual bindings at compile-time in build.rs and then
include!() the output from src/lib.rs.

You can verify that the generated bindings from Linux 6.12 have not
changed significantly for x86_64 by diffing them against the previous
commit. For example:
--- orig/virtio_gpu.rs
+++ new/virtio_gpu.rs
@@ -1,4 +1,4 @@
-/* automatically generated by rust-bindgen 0.70.1 */
+/* automatically generated by rust-bindgen 0.71.1 */

 #[repr(C)]
 #[derive(Default)]
@@ -58,7 +58,7 @@
 pub const VIRTIO_GPU_MAP_CACHE_WC: u32 = 3;
 pub type __u8 = ::std::os::raw::c_uchar;
 pub type __u32 = ::std::os::raw::c_uint;
-pub type __u64 = ::std::os::raw::c_ulonglong;
+pub type __u64 = ::std::os::raw::c_ulong;
 pub type __le32 = __u32;
 pub type __le64 = __u64;
 pub const virtio_gpu_ctrl_type_VIRTIO_GPU_UNDEFINED: virtio_gpu_ctrl_type = 0

The __u64 change is due to how <linux/types.h> defined __u64. We should
avoid using that non-BSD licensed header. The result is equivalent now
that the bindings are generated at compile-time.

Fixes: rust-vmm#326
Signed-off-by: Stefan Hajnoczi <[email protected]>
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.

virtio-bindings compile-time alignment checks are architecture-specific
3 participants