diff mbox series

[14/17] rust/vmstate: Add unit test for vmstate_of macro

Message ID 20250317151236.536673-15-zhao1.liu@intel.com (mailing list archive)
State New
Headers show
Series rust/vmstate: Clean up, fix, enhance & test | expand

Commit Message

Zhao Liu March 17, 2025, 3:12 p.m. UTC
The vmstate has too many combinations of VMStateFlags and VMStateField.
Currently, the best way to test is to ensure that the Rust vmstate
definition is consistent with the (possibly corresponding) C version.

Add a unit test to cover some patterns accepted by vmstate_of macro,
which correspond to the following C version macros:
 * VMSTATE_U16
 * VMSTATE_UNUSED
 * VMSTATE_VARRAY_UINT16_UNSAFE
 * VMSTATE_VARRAY_MULTIPLY

Note: Because vmstate_info_* are defined in vmstate-types.c, it's
necessary to link libmigration to rust unit tests. In the future,
maybe it's possible to spilt libmigration from rust_qemu_api_objs.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 rust/qemu-api/meson.build            |   5 +-
 rust/qemu-api/src/vmstate.rs         |   6 +-
 rust/qemu-api/tests/tests.rs         |   2 +
 rust/qemu-api/tests/vmstate_tests.rs | 127 +++++++++++++++++++++++++++
 4 files changed, 134 insertions(+), 6 deletions(-)
 create mode 100644 rust/qemu-api/tests/vmstate_tests.rs

Comments

Paolo Bonzini March 17, 2025, 5:11 p.m. UTC | #1
Thanks very much for the tests!

On Mon, Mar 17, 2025 at 3:52 PM Zhao Liu <zhao1.liu@intel.com> wrote:
> -pub use crate::bindings::{VMStateDescription, VMStateField};
> -use crate::{
> -    bindings::VMStateFlags, callbacks::FnCall, prelude::*, qom::Owned, zeroable::Zeroable,
> -};
> +pub use crate::bindings::{VMStateDescription, VMStateField, VMStateFlags};

Does VMStateFlags have to be part of the public API?

> +    assert_eq!(foo_fields[0].info, unsafe {
> +        ::core::ptr::addr_of!(vmstate_info_int8)
> +    });

You can use & instead of addr_of here.

> +    assert_eq!(foo_fields[0].version_id, 0);
> +    assert_eq!(foo_fields[0].size, 1);
> +    assert_eq!(foo_fields[0].num, 0);
> +    assert_eq!(foo_fields[0].flags, VMStateFlags::VMS_SINGLE);
> +    assert_eq!(foo_fields[0].vmsd.is_null(), true);
> +    assert_eq!(foo_fields[0].field_exists.is_none(), true);
> +
> +    // 2nd VMStateField ("unused") in VMSTATE_FOOA (corresponding to VMSTATE_UNUSED)
> +    assert_eq!(
> +        unsafe { CStr::from_ptr(foo_fields[1].name) }.to_bytes_with_nul(),
> +        b"unused\0"
> +    );
> +    assert_eq!(foo_fields[1].offset, 0);
> +    assert_eq!(foo_fields[1].num_offset, 0);
> +    assert_eq!(foo_fields[1].info, unsafe {
> +        ::core::ptr::addr_of!(vmstate_info_unused_buffer)
> +    });
> +    assert_eq!(foo_fields[1].version_id, 0);
> +    assert_eq!(foo_fields[1].size, 8);
> +    assert_eq!(foo_fields[1].num, 0);
> +    assert_eq!(foo_fields[1].flags, VMStateFlags::VMS_BUFFER);
> +    assert_eq!(foo_fields[1].vmsd.is_null(), true);
> +    assert_eq!(foo_fields[1].field_exists.is_none(), true);
> +
> +    // 3rd VMStateField ("arr") in VMSTATE_FOOA (corresponding to
> +    // VMSTATE_VARRAY_UINT16_UNSAFE)
> +    assert_eq!(
> +        unsafe { CStr::from_ptr(foo_fields[2].name) }.to_bytes_with_nul(),
> +        b"arr\0"
> +    );
> +    assert_eq!(foo_fields[2].offset, 0);
> +    assert_eq!(foo_fields[2].num_offset, 4);
> +    assert_eq!(foo_fields[2].info, unsafe {
> +        ::core::ptr::addr_of!(vmstate_info_uint8)
> +    });
> +    assert_eq!(foo_fields[2].version_id, 0);
> +    assert_eq!(foo_fields[2].size, 1);
> +    assert_eq!(foo_fields[2].num, 0);
> +    assert_eq!(foo_fields[2].flags, VMStateFlags::VMS_VARRAY_UINT16);
> +    assert_eq!(foo_fields[2].vmsd.is_null(), true);
> +    assert_eq!(foo_fields[2].field_exists.is_none(), true);
> +
> +    // 4th VMStateField ("arr_mul") in VMSTATE_FOOA (corresponding to
> +    // VMSTATE_VARRAY_MULTIPLY)
> +    assert_eq!(
> +        unsafe { CStr::from_ptr(foo_fields[3].name) }.to_bytes_with_nul(),
> +        b"arr_mul\0"
> +    );
> +    assert_eq!(foo_fields[3].offset, 6);
> +    assert_eq!(foo_fields[3].num_offset, 12);
> +    assert_eq!(foo_fields[3].info, unsafe {
> +        ::core::ptr::addr_of!(vmstate_info_int8)
> +    });
> +    assert_eq!(foo_fields[3].version_id, 0);
> +    assert_eq!(foo_fields[3].size, 1);
> +    assert_eq!(foo_fields[3].num, 16);
> +    assert_eq!(
> +        foo_fields[3].flags.0,
> +        VMStateFlags::VMS_VARRAY_UINT32.0 | VMStateFlags::VMS_MULTIPLY_ELEMENTS.0
> +    );
> +    assert_eq!(foo_fields[3].vmsd.is_null(), true);
> +    assert_eq!(foo_fields[3].field_exists.is_none(), true);
> +
> +    // The last VMStateField in VMSTATE_FOOA.
> +    assert_eq!(foo_fields[4].flags, VMStateFlags::VMS_END);
> +}
> --
> 2.34.1
>
Zhao Liu March 18, 2025, 6:45 a.m. UTC | #2
On Mon, Mar 17, 2025 at 06:11:35PM +0100, Paolo Bonzini wrote:
> Date: Mon, 17 Mar 2025 18:11:35 +0100
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: Re: [PATCH 14/17] rust/vmstate: Add unit test for vmstate_of macro
> 
> Thanks very much for the tests!
> 
> On Mon, Mar 17, 2025 at 3:52 PM Zhao Liu <zhao1.liu@intel.com> wrote:
> > -pub use crate::bindings::{VMStateDescription, VMStateField};
> > -use crate::{
> > -    bindings::VMStateFlags, callbacks::FnCall, prelude::*, qom::Owned, zeroable::Zeroable,
> > -};
> > +pub use crate::bindings::{VMStateDescription, VMStateField, VMStateFlags};
> 
> Does VMStateFlags have to be part of the public API?

I can do `use qemu_api::bindings::VMStateFlags` in vmstate_test.rs
directly, which is better since it's the raw value of VMStateField that
I need to check!

> > +    assert_eq!(foo_fields[0].info, unsafe {
> > +        ::core::ptr::addr_of!(vmstate_info_int8)
> > +    });
> 
> You can use & instead of addr_of here.

Thanks! Will fix.

Regards,
Zhao
diff mbox series

Patch

diff --git a/rust/qemu-api/meson.build b/rust/qemu-api/meson.build
index a3f226ccc2a5..858685ddd4a4 100644
--- a/rust/qemu-api/meson.build
+++ b/rust/qemu-api/meson.build
@@ -58,7 +58,8 @@  rust_qemu_api_objs = static_library(
               libchardev.extract_all_objects(recursive: false),
               libcrypto.extract_all_objects(recursive: false),
               libauthz.extract_all_objects(recursive: false),
-              libio.extract_all_objects(recursive: false)])
+              libio.extract_all_objects(recursive: false),
+              libmigration.extract_all_objects(recursive: false)])
 rust_qemu_api_deps = declare_dependency(
     dependencies: [
       qom_ss.dependencies(),
@@ -71,7 +72,7 @@  rust_qemu_api_deps = declare_dependency(
 test('rust-qemu-api-integration',
     executable(
         'rust-qemu-api-integration',
-        'tests/tests.rs',
+        files('tests/tests.rs', 'tests/vmstate_tests.rs'),
         override_options: ['rust_std=2021', 'build.rust_std=2021'],
         rust_args: ['--test'],
         install: false,
diff --git a/rust/qemu-api/src/vmstate.rs b/rust/qemu-api/src/vmstate.rs
index 4eefd54ca120..3fdd5948f6d7 100644
--- a/rust/qemu-api/src/vmstate.rs
+++ b/rust/qemu-api/src/vmstate.rs
@@ -27,10 +27,8 @@ 
 use core::{marker::PhantomData, mem, ptr::NonNull};
 use std::os::raw::{c_int, c_void};
 
-pub use crate::bindings::{VMStateDescription, VMStateField};
-use crate::{
-    bindings::VMStateFlags, callbacks::FnCall, prelude::*, qom::Owned, zeroable::Zeroable,
-};
+pub use crate::bindings::{VMStateDescription, VMStateField, VMStateFlags};
+use crate::{callbacks::FnCall, prelude::*, qom::Owned, zeroable::Zeroable};
 
 /// This macro is used to call a function with a generic argument bound
 /// to the type of a field.  The function must take a
diff --git a/rust/qemu-api/tests/tests.rs b/rust/qemu-api/tests/tests.rs
index 269122e7ec19..99a7aab6fed9 100644
--- a/rust/qemu-api/tests/tests.rs
+++ b/rust/qemu-api/tests/tests.rs
@@ -17,6 +17,8 @@ 
     zeroable::Zeroable,
 };
 
+mod vmstate_tests;
+
 // Test that macros can compile.
 pub static VMSTATE: VMStateDescription = VMStateDescription {
     name: c_str!("name").as_ptr(),
diff --git a/rust/qemu-api/tests/vmstate_tests.rs b/rust/qemu-api/tests/vmstate_tests.rs
new file mode 100644
index 000000000000..25b28b298066
--- /dev/null
+++ b/rust/qemu-api/tests/vmstate_tests.rs
@@ -0,0 +1,127 @@ 
+// Copyright (C) 2025 Intel Corporation.
+// Author(s): Zhao Liu <zhai1.liu@intel.com>
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+use std::{ffi::CStr, mem::size_of, slice};
+
+use qemu_api::{
+    bindings::{vmstate_info_int8, vmstate_info_uint8, vmstate_info_unused_buffer},
+    c_str,
+    vmstate::{VMStateDescription, VMStateField, VMStateFlags},
+    vmstate_fields, vmstate_of, vmstate_unused,
+    zeroable::Zeroable,
+};
+
+const FOO_ARRAY_MAX: usize = 3;
+
+// =========================== Test VMSTATE_FOOA ===========================
+// Test the use cases of the vmstate macro, corresponding to the following C
+// macro variants:
+//   * VMSTATE_FOOA:
+//     - VMSTATE_U16
+//     - VMSTATE_UNUSED
+//     - VMSTATE_VARRAY_UINT16_UNSAFE
+//     - VMSTATE_VARRAY_MULTIPLY
+#[repr(C)]
+#[derive(qemu_api_macros::offsets)]
+struct FooA {
+    arr: [u8; FOO_ARRAY_MAX],
+    num: u16,
+    arr_mul: [i8; FOO_ARRAY_MAX],
+    num_mul: u32,
+    elem: i8,
+}
+
+static VMSTATE_FOOA: VMStateDescription = VMStateDescription {
+    name: c_str!("foo_a").as_ptr(),
+    version_id: 1,
+    minimum_version_id: 1,
+    fields: vmstate_fields! {
+        vmstate_of!(FooA, elem),
+        vmstate_unused!(size_of::<i64>()),
+        vmstate_of!(FooA, arr, [0 .. num], version = 0),
+        vmstate_of!(FooA, arr_mul, [0 .. num_mul * 16]),
+    },
+    ..Zeroable::ZERO
+};
+
+#[test]
+fn test_vmstate_macro_fooa() {
+    let foo_fields: &[VMStateField] = unsafe { slice::from_raw_parts(VMSTATE_FOOA.fields, 5) };
+
+    // 1st VMStateField ("elem") in VMSTATE_FOOA (corresponding to VMSTATE_U16)
+    assert_eq!(
+        unsafe { CStr::from_ptr(foo_fields[0].name) }.to_bytes_with_nul(),
+        b"elem\0"
+    );
+    assert_eq!(foo_fields[0].offset, 16);
+    assert_eq!(foo_fields[0].num_offset, 0);
+    assert_eq!(foo_fields[0].info, unsafe {
+        ::core::ptr::addr_of!(vmstate_info_int8)
+    });
+    assert_eq!(foo_fields[0].version_id, 0);
+    assert_eq!(foo_fields[0].size, 1);
+    assert_eq!(foo_fields[0].num, 0);
+    assert_eq!(foo_fields[0].flags, VMStateFlags::VMS_SINGLE);
+    assert_eq!(foo_fields[0].vmsd.is_null(), true);
+    assert_eq!(foo_fields[0].field_exists.is_none(), true);
+
+    // 2nd VMStateField ("unused") in VMSTATE_FOOA (corresponding to VMSTATE_UNUSED)
+    assert_eq!(
+        unsafe { CStr::from_ptr(foo_fields[1].name) }.to_bytes_with_nul(),
+        b"unused\0"
+    );
+    assert_eq!(foo_fields[1].offset, 0);
+    assert_eq!(foo_fields[1].num_offset, 0);
+    assert_eq!(foo_fields[1].info, unsafe {
+        ::core::ptr::addr_of!(vmstate_info_unused_buffer)
+    });
+    assert_eq!(foo_fields[1].version_id, 0);
+    assert_eq!(foo_fields[1].size, 8);
+    assert_eq!(foo_fields[1].num, 0);
+    assert_eq!(foo_fields[1].flags, VMStateFlags::VMS_BUFFER);
+    assert_eq!(foo_fields[1].vmsd.is_null(), true);
+    assert_eq!(foo_fields[1].field_exists.is_none(), true);
+
+    // 3rd VMStateField ("arr") in VMSTATE_FOOA (corresponding to
+    // VMSTATE_VARRAY_UINT16_UNSAFE)
+    assert_eq!(
+        unsafe { CStr::from_ptr(foo_fields[2].name) }.to_bytes_with_nul(),
+        b"arr\0"
+    );
+    assert_eq!(foo_fields[2].offset, 0);
+    assert_eq!(foo_fields[2].num_offset, 4);
+    assert_eq!(foo_fields[2].info, unsafe {
+        ::core::ptr::addr_of!(vmstate_info_uint8)
+    });
+    assert_eq!(foo_fields[2].version_id, 0);
+    assert_eq!(foo_fields[2].size, 1);
+    assert_eq!(foo_fields[2].num, 0);
+    assert_eq!(foo_fields[2].flags, VMStateFlags::VMS_VARRAY_UINT16);
+    assert_eq!(foo_fields[2].vmsd.is_null(), true);
+    assert_eq!(foo_fields[2].field_exists.is_none(), true);
+
+    // 4th VMStateField ("arr_mul") in VMSTATE_FOOA (corresponding to
+    // VMSTATE_VARRAY_MULTIPLY)
+    assert_eq!(
+        unsafe { CStr::from_ptr(foo_fields[3].name) }.to_bytes_with_nul(),
+        b"arr_mul\0"
+    );
+    assert_eq!(foo_fields[3].offset, 6);
+    assert_eq!(foo_fields[3].num_offset, 12);
+    assert_eq!(foo_fields[3].info, unsafe {
+        ::core::ptr::addr_of!(vmstate_info_int8)
+    });
+    assert_eq!(foo_fields[3].version_id, 0);
+    assert_eq!(foo_fields[3].size, 1);
+    assert_eq!(foo_fields[3].num, 16);
+    assert_eq!(
+        foo_fields[3].flags.0,
+        VMStateFlags::VMS_VARRAY_UINT32.0 | VMStateFlags::VMS_MULTIPLY_ELEMENTS.0
+    );
+    assert_eq!(foo_fields[3].vmsd.is_null(), true);
+    assert_eq!(foo_fields[3].field_exists.is_none(), true);
+
+    // The last VMStateField in VMSTATE_FOOA.
+    assert_eq!(foo_fields[4].flags, VMStateFlags::VMS_END);
+}