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

[Zhao Liu <zhao1.liu@intel.com>]

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
>