On Tue, Oct 14, 2025 at 8:33 AM Zhao Liu <[email protected]> wrote: > > +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); > > Timer::modify() is the wrapper of timer_mod(), and timer_mod() will > re-compute the expire time with scale factor.
Ouch, of course you're right. > So here we should use a binding of timer_mod_ns() - Timer::modify_ns() > instead of Timer::modify(), just like C side did. > > Moreover, for Timer::modify_ns() & Timer::modify(), the simplest > way is to build them based on timer_mod_ns() & timer_mod() (but this > sounds like shifting the responsibility for type checking and overflow > checking to the C side :-( ). > > Alternative, we can make Timer::modify() based on Timer::modify_ns(), > but then it involves many numeric type conversions: scale is i32 but > expire_time is u64, and Timer::modify_ns() may accept a u64 argument. > > Fortunately,, we can make Timer's modify() accept i64 to reduce type > conversions. I think it's better to make it accept u64 and assert that it's not above i64::MAX (with .try_into().unwrap()). Anyway I left out this patch, and it can be picked up when HPET starts using ToMigrationState. Paolo
