[PATCH 6/7] rust: migration: implement ToMigrationState for Timer

Paolo Bonzini posted 7 patches 4 months, 3 weeks ago
Maintainers: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
There is a newer version of this series
[PATCH 6/7] rust: migration: implement ToMigrationState for Timer
Posted by Paolo Bonzini 4 months, 3 weeks ago
Timer is a complex struct, allow adding it to a struct that
uses #[derive(ToMigrationState)]; similar to vmstate_timer, only
the expiration time has to be preserved.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 rust/migration/src/migratable.rs | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/rust/migration/src/migratable.rs b/rust/migration/src/migratable.rs
index fa25317eea8..c4ad4f73d5c 100644
--- a/rust/migration/src/migratable.rs
+++ b/rust/migration/src/migratable.rs
@@ -202,6 +202,37 @@ fn restore_migrated_state(
     ) -> Result<(), InvalidError>;
 }
 
+impl ToMigrationState for util::timer::Timer {
+    type Migrated = i64;
+
+    fn snapshot_migration_state(&self, target: &mut i64) -> Result<(), InvalidError> {
+        // SAFETY: as_ptr() is unsafe to ensure that the caller reasons about
+        // the pinning of the data inside the Opaque<>.  Here all we do is
+        // access a field.
+        *target = unsafe { &*self.as_ptr() }.expire_time;
+        Ok(())
+    }
+
+    fn restore_migrated_state_mut(
+        &mut self,
+        source: Self::Migrated,
+        version_id: u8,
+    ) -> Result<(), InvalidError> {
+        self.restore_migrated_state(source, version_id)
+    }
+}
+
+impl ToMigrationStateShared for util::timer::Timer {
+    fn restore_migrated_state(&self, source: i64, _version_id: u8) -> Result<(), InvalidError> {
+        if source >= 0 {
+            self.modify(source as u64);
+        } else {
+            self.delete();
+        }
+        Ok(())
+    }
+}
+
 impl<T: ToMigrationStateShared, const N: usize> ToMigrationStateShared for [T; N]
 where
     [T::Migrated; N]: Default,
-- 
2.51.0
Re: [PATCH 6/7] rust: migration: implement ToMigrationState for Timer
Posted by Zhao Liu 4 months, 1 week ago
On Sat, Sep 20, 2025 at 04:29:57PM +0200, Paolo Bonzini wrote:
> Date: Sat, 20 Sep 2025 16:29:57 +0200
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [PATCH 6/7] rust: migration: implement ToMigrationState for Timer
> X-Mailer: git-send-email 2.51.0
> 
> Timer is a complex struct, allow adding it to a struct that
> uses #[derive(ToMigrationState)]; similar to vmstate_timer, only
> the expiration time has to be preserved.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  rust/migration/src/migratable.rs | 31 +++++++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
> 
> diff --git a/rust/migration/src/migratable.rs b/rust/migration/src/migratable.rs
> index fa25317eea8..c4ad4f73d5c 100644
> --- a/rust/migration/src/migratable.rs
> +++ b/rust/migration/src/migratable.rs
> @@ -202,6 +202,37 @@ fn restore_migrated_state(
>      ) -> Result<(), InvalidError>;
>  }
>  
> +impl ToMigrationState for util::timer::Timer {
> +    type Migrated = i64;

This converts a Timer to i64, then we don't need vmstate_info_timer
anymore. Good idea.

> +    fn snapshot_migration_state(&self, target: &mut i64) -> Result<(), InvalidError> {
> +        // SAFETY: as_ptr() is unsafe to ensure that the caller reasons about
> +        // the pinning of the data inside the Opaque<>.  Here all we do is
> +        // access a field.
> +        *target = unsafe { &*self.as_ptr() }.expire_time;

C side checks timer_pending(), which ensures when the timer is
inactive, it always saves u64::MAX.

But now we save the expire_time directly in Rust. I think this would be
possible to break the migration from Rust timer to C timer, because at
C side, timer_get() checks whether expire_time is -1 and we can't ensure
expire_time won't store -2 (or other unusual negative values).

So maybe it's better to follow C side behavior to return u64::MAX for
non-pending case?

> +        Ok(())
> +    }
> +
> +    fn restore_migrated_state_mut(
> +        &mut self,
> +        source: Self::Migrated,
> +        version_id: u8,
> +    ) -> Result<(), InvalidError> {
> +        self.restore_migrated_state(source, version_id)
> +    }
> +}
> +
> +impl ToMigrationStateShared for util::timer::Timer {
> +    fn restore_migrated_state(&self, source: i64, _version_id: u8) -> Result<(), InvalidError> {
> +        if source >= 0 {

This can work for the migration cases of C->Rust and Rust->Rust.

Thanks,
Zhao

> +            self.modify(source as u64);
> +        } else {
> +            self.delete();
> +        }
> +        Ok(())
> +    }
> +}
> +
>  impl<T: ToMigrationStateShared, const N: usize> ToMigrationStateShared for [T; N]
>  where
>      [T::Migrated; N]: Default,
> -- 
> 2.51.0
>
Re: [PATCH 6/7] rust: migration: implement ToMigrationState for Timer
Posted by Paolo Bonzini 4 months, 1 week ago
On 9/29/25 18:12, Zhao Liu wrote:
>> +    fn snapshot_migration_state(&self, target: &mut i64) -> Result<(), InvalidError> {
>> +        // SAFETY: as_ptr() is unsafe to ensure that the caller reasons about
>> +        // the pinning of the data inside the Opaque<>.  Here all we do is
>> +        // access a field.
>> +        *target = unsafe { &*self.as_ptr() }.expire_time;
> 
> C side checks timer_pending(), which ensures when the timer is
> inactive, it always saves u64::MAX.
> 
> But now we save the expire_time directly in Rust. I think this would be
> possible to break the migration from Rust timer to C timer, because at
> C side, timer_get() checks whether expire_time is -1 and we can't ensure
> expire_time won't store -2 (or other unusual negative values).

I think this should work in both cases because timer_pending() checks >= 
0 and negative values are extremely far in the future.  But I'll change 
it to timer_expire_time_ns() for safety and clarity.

Paolo