[21/26] rust: tests: allow writing more than one test

Commit Message

Paolo Bonzini Dec. 9, 2024, 12:37 p.m. UTC

Signed-off-by: Paolo Bonzini <[e-mail]>
---
 rust/qemu-api/tests/tests.rs | 109 ++++++++++++++++++++---------------
 1 file changed, 61 insertions(+), 48 deletions(-)

Comments

Zhao Liu Dec. 12, 2024, 10:04 a.m. UTC | #1

> +fn init_qom() {
> +    static ONCE: Mutex<Cell<bool>> = Mutex::new(Cell::new(false));
> +
> +    let g = ONCE.lock().unwrap();
> +    if !g.get() {
> +        unsafe {
> +            module_call_init(module_init_type::MODULE_INIT_QOM);
> +        }
> +        g.set(true);
> +    }
> +}

Only one question: what is the purpose of using a Mutex here?

Others LGTM,

Reviewed-by: Zhao Liu <[e-mail]>

Paolo Bonzini Dec. 16, 2024, 3:07 p.m. UTC | #2

On Thu, Dec 12, 2024 at 10:46 AM Zhao Liu <[e-mail]> wrote:
>
> > +fn init_qom() {
> > +    static ONCE: Mutex<Cell<bool>> = Mutex::new(Cell::new(false));
> > +
> > +    let g = ONCE.lock().unwrap();
> > +    if !g.get() {
> > +        unsafe {
> > +            module_call_init(module_init_type::MODULE_INIT_QOM);
> > +        }
> > +        g.set(true);
> > +    }
> > +}
>
> Only one question: what is the purpose of using a Mutex here?

Different #[test] functions can be run in parallel. For now there is
nothing that needs the BQL in the tests, so I used an independent
Mutex (which could become a LazyLock later when the minimum supported
Rust version is bumped).

It could also use unsafe { bql_lock(); } and a BqlCell. I left that
for when there will be a way to lock the BQL from safe Rust code, but
I can also do that now already.

Paolo

Patch

diff --git a/rust/qemu-api/tests/tests.rs b/rust/qemu-api/tests/tests.rs
index 68557fb85c7..18738a80008 100644
--- a/rust/qemu-api/tests/tests.rs
+++ b/rust/qemu-api/tests/tests.rs
@@ -2,7 +2,7 @@ 
 // Author(s): Manos Pitsidianakis <[e-mail]>
 // SPDX-License-Identifier: GPL-2.0-or-later
 
-use std::ffi::CStr;
+use std::{cell::Cell, ffi::CStr, sync::Mutex};
 
 use qemu_api::{
     bindings::*,
@@ -14,55 +14,68 @@ 
     zeroable::Zeroable,
 };
 
+// Test that macros can compile.
+pub static VMSTATE: VMStateDescription = VMStateDescription {
+    name: c_str!("name").as_ptr(),
+    unmigratable: true,
+    ..Zeroable::ZERO
+};
+
+#[derive(qemu_api_macros::offsets)]
+#[repr(C)]
+#[derive(qemu_api_macros::Object)]
+pub struct DummyState {
+    parent: DeviceState,
+    migrate_clock: bool,
+}
+
+declare_properties! {
+    DUMMY_PROPERTIES,
+        define_property!(
+            c_str!("migrate-clk"),
+            DummyState,
+            migrate_clock,
+            unsafe { &qdev_prop_bool },
+            bool
+        ),
+}
+
+unsafe impl ObjectType for DummyState {
+    type Class = <DeviceState as ObjectType>::Class;
+    const TYPE_NAME: &'static CStr = c_str!("dummy");
+}
+
+impl ObjectImpl for DummyState {
+    type ParentType = DeviceState;
+    const ABSTRACT: bool = false;
+}
+
+impl DeviceImpl for DummyState {
+    fn properties() -> &'static [Property] {
+        &DUMMY_PROPERTIES
+    }
+    fn vmsd() -> Option<&'static VMStateDescription> {
+        Some(&VMSTATE)
+    }
+}
+
+fn init_qom() {
+    static ONCE: Mutex<Cell<bool>> = Mutex::new(Cell::new(false));
+
+    let g = ONCE.lock().unwrap();
+    if !g.get() {
+        unsafe {
+            module_call_init(module_init_type::MODULE_INIT_QOM);
+        }
+        g.set(true);
+    }
+}
+
 #[test]
-fn test_device_decl_macros() {
-    // Test that macros can compile.
-    pub static VMSTATE: VMStateDescription = VMStateDescription {
-        name: c_str!("name").as_ptr(),
-        unmigratable: true,
-        ..Zeroable::ZERO
-    };
-
-    #[derive(qemu_api_macros::offsets)]
-    #[repr(C)]
-    #[derive(qemu_api_macros::Object)]
-    pub struct DummyState {
-        pub _parent: DeviceState,
-        pub migrate_clock: bool,
-    }
-
-    declare_properties! {
-        DUMMY_PROPERTIES,
-            define_property!(
-                c_str!("migrate-clk"),
-                DummyState,
-                migrate_clock,
-                unsafe { &qdev_prop_bool },
-                bool
-            ),
-    }
-
-    unsafe impl ObjectType for DummyState {
-        type Class = <DeviceState as ObjectType>::Class;
-        const TYPE_NAME: &'static CStr = c_str!("dummy");
-    }
-
-    impl ObjectImpl for DummyState {
-        type ParentType = DeviceState;
-        const ABSTRACT: bool = false;
-    }
-
-    impl DeviceImpl for DummyState {
-        fn properties() -> &'static [Property] {
-            &DUMMY_PROPERTIES
-        }
-        fn vmsd() -> Option<&'static VMStateDescription> {
-            Some(&VMSTATE)
-        }
-    }
-
+/// Create and immediately drop an instance.
+fn test_object_new() {
+    init_qom();
     unsafe {
-        module_call_init(module_init_type::MODULE_INIT_QOM);
         object_unref(object_new(DummyState::TYPE_NAME.as_ptr()).cast());
     }
 }