Re: [PULL 12/40] rust: build integration test for the qemu_api crate

[Bernhard Beschow]

Am 4. November 2024 17:26:51 UTC schrieb Paolo Bonzini <pbonzini@redhat.com>:
>Adjust the integration test to compile with a subset of QEMU object
>files, and make it actually create an object of the class it defines.
>
>Follow the Rust filesystem conventions, where tests go in tests/ if
>they use the library in the same way any other code would.
>
>Reviewed-by: Junjie Mao <junjie.mao@hotmail.com>
>Reviewed-by: Kevin Wolf <kwolf@redhat.com>
>Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>---
> meson.build                  | 10 ++++-
> rust/qemu-api/meson.build    | 26 ++++++++++--
> rust/qemu-api/src/lib.rs     |  3 --
> rust/qemu-api/src/tests.rs   | 49 ----------------------
> rust/qemu-api/tests/tests.rs | 78 ++++++++++++++++++++++++++++++++++++
> 5 files changed, 110 insertions(+), 56 deletions(-)
> delete mode 100644 rust/qemu-api/src/tests.rs
> create mode 100644 rust/qemu-api/tests/tests.rs

When `--enable-modules` is passed to configure, this patch results in numerous 
undefined symbols.

Best regards,
Bernhard

>
>diff --git a/meson.build b/meson.build
>index 34328f7394c..d360120b233 100644
>--- a/meson.build
>+++ b/meson.build
>@@ -3340,7 +3340,15 @@ if have_rust and have_system
> 
>   # Prohibit code that is forbidden in Rust 2024
>   rustc_args += ['-D', 'unsafe_op_in_unsafe_fn']
>-  add_project_arguments(rustc_args, native: false, language: 'rust')
>+
>+  # Apart from procedural macros, our Rust executables will often link
>+  # with C code, so include all the libraries that C code needs.  This
>+  # is safe; https://github.com/rust-lang/rust/pull/54675 says that
>+  # passing -nodefaultlibs to the linker "was more ideological to
>+  # start with than anything".
>+  add_project_arguments(rustc_args + ['-C', 'default-linker-libraries'],
>+      native: false, language: 'rust')
>+
>   add_project_arguments(rustc_args, native: true, language: 'rust')
> endif
> 
>diff --git a/rust/qemu-api/meson.build b/rust/qemu-api/meson.build
>index 42ea815fa5a..1fc36078027 100644
>--- a/rust/qemu-api/meson.build
>+++ b/rust/qemu-api/meson.build
>@@ -14,11 +14,31 @@ _qemu_api_rs = static_library(
>     '--cfg', 'MESON',
>     # '--cfg', 'feature="allocator"',
>   ],
>-  dependencies: [
>-    qemu_api_macros,
>-  ],
> )
> 
> qemu_api = declare_dependency(
>   link_with: _qemu_api_rs,
>+  dependencies: qemu_api_macros,
> )
>+
>+# Rust executables do not support objects, so add an intermediate step.
>+rust_qemu_api_objs = static_library(
>+    'rust_qemu_api_objs',
>+    objects: [libqom.extract_all_objects(recursive: false),
>+              libhwcore.extract_all_objects(recursive: false)])
>+
>+test('rust-qemu-api-integration',
>+    executable(
>+        'rust-qemu-api-integration',
>+        'tests/tests.rs',
>+        override_options: ['rust_std=2021', 'build.rust_std=2021'],
>+        rust_args: ['--test'],
>+        install: false,
>+        dependencies: [qemu_api, qemu_api_macros],
>+        link_whole: [rust_qemu_api_objs, libqemuutil]),
>+    args: [
>+        '--test',
>+        '--format', 'pretty',
>+    ],
>+    protocol: 'rust',
>+    suite: ['unit', 'rust'])
>diff --git a/rust/qemu-api/src/lib.rs b/rust/qemu-api/src/lib.rs
>index e72fb4b4bb1..6bc68076aae 100644
>--- a/rust/qemu-api/src/lib.rs
>+++ b/rust/qemu-api/src/lib.rs
>@@ -30,9 +30,6 @@ unsafe impl Sync for bindings::VMStateDescription {}
> pub mod definitions;
> pub mod device_class;
> 
>-#[cfg(test)]
>-mod tests;
>-
> use std::alloc::{GlobalAlloc, Layout};
> 
> #[cfg(HAVE_GLIB_WITH_ALIGNED_ALLOC)]
>diff --git a/rust/qemu-api/src/tests.rs b/rust/qemu-api/src/tests.rs
>deleted file mode 100644
>index df54edbd4e2..00000000000
>--- a/rust/qemu-api/src/tests.rs
>+++ /dev/null
>@@ -1,49 +0,0 @@
>-// Copyright 2024, Linaro Limited
>-// Author(s): Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
>-// SPDX-License-Identifier: GPL-2.0-or-later
>-
>-use crate::{
>-    bindings::*, declare_properties, define_property, device_class_init, 
>vm_state_description,
>-};
>-
>-#[test]
>-fn test_device_decl_macros() {
>-    // Test that macros can compile.
>-    vm_state_description! {
>-        VMSTATE,
>-        name: c"name",
>-        unmigratable: true,
>-    }
>-
>-    #[repr(C)]
>-    pub struct DummyState {
>-        pub char_backend: CharBackend,
>-        pub migrate_clock: bool,
>-    }
>-
>-    declare_properties! {
>-        DUMMY_PROPERTIES,
>-            define_property!(
>-                c"chardev",
>-                DummyState,
>-                char_backend,
>-                unsafe { &qdev_prop_chr },
>-                CharBackend
>-            ),
>-            define_property!(
>-                c"migrate-clk",
>-                DummyState,
>-                migrate_clock,
>-                unsafe { &qdev_prop_bool },
>-                bool
>-            ),
>-    }
>-
>-    device_class_init! {
>-        dummy_class_init,
>-        props => DUMMY_PROPERTIES,
>-        realize_fn => None,
>-        reset_fn => None,
>-        vmsd => VMSTATE,
>-    }
>-}
>diff --git a/rust/qemu-api/tests/tests.rs b/rust/qemu-api/tests/tests.rs
>new file mode 100644
>index 00000000000..aa1e0568c69
>--- /dev/null
>+++ b/rust/qemu-api/tests/tests.rs
>@@ -0,0 +1,78 @@
>+// Copyright 2024, Linaro Limited
>+// Author(s): Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
>+// SPDX-License-Identifier: GPL-2.0-or-later
>+
>+use core::ffi::CStr;
>+
>+use qemu_api::{
>+    bindings::*,
>+    declare_properties, define_property,
>+    definitions::{Class, ObjectImpl},
>+    device_class_init, vm_state_description,
>+};
>+
>+#[test]
>+fn test_device_decl_macros() {
>+    // Test that macros can compile.
>+    vm_state_description! {
>+        VMSTATE,
>+        name: c"name",
>+        unmigratable: true,
>+    }
>+
>+    #[repr(C)]
>+    #[derive(qemu_api_macros::Object)]
>+    pub struct DummyState {
>+        pub _parent: DeviceState,
>+        pub migrate_clock: bool,
>+    }
>+
>+    #[repr(C)]
>+    pub struct DummyClass {
>+        pub _parent: DeviceClass,
>+    }
>+
>+    declare_properties! {
>+        DUMMY_PROPERTIES,
>+            define_property!(
>+                c"migrate-clk",
>+                DummyState,
>+                migrate_clock,
>+                unsafe { &qdev_prop_bool },
>+                bool
>+            ),
>+    }
>+
>+    device_class_init! {
>+        dummy_class_init,
>+        props => DUMMY_PROPERTIES,
>+        realize_fn => None,
>+        legacy_reset_fn => None,
>+        vmsd => VMSTATE,
>+    }
>+
>+    impl ObjectImpl for DummyState {
>+        type Class = DummyClass;
>+        const TYPE_INFO: qemu_api::bindings::TypeInfo = qemu_api::type_info! 
>{ Self };
>+        const TYPE_NAME: &'static CStr = c"dummy";
>+        const PARENT_TYPE_NAME: Option<&'static CStr> = Some(TYPE_DEVICE);
>+        const ABSTRACT: bool = false;
>+        const INSTANCE_INIT: Option<unsafe extern "C" fn(obj: *mut Object)> = 
>None;
>+        const INSTANCE_POST_INIT: Option<unsafe extern "C" fn(obj: *mut 
>Object)> = None;
>+        const INSTANCE_FINALIZE: Option<unsafe extern "C" fn(obj: *mut 
>Object)> = None;
>+    }
>+
>+    impl Class for DummyClass {
>+        const CLASS_INIT: Option<
>+            unsafe extern "C" fn(klass: *mut ObjectClass, data: *mut 
>core::ffi::c_void),
>+        > = Some(dummy_class_init);
>+        const CLASS_BASE_INIT: Option<
>+            unsafe extern "C" fn(klass: *mut ObjectClass, data: *mut 
>core::ffi::c_void),
>+        > = None;
>+    }
>+
>+    unsafe {
>+        module_call_init(module_init_type::MODULE_INIT_QOM);
>+        object_unref(object_new(DummyState::TYPE_NAME.as_ptr()) as *mut _);
>+    }
>+}