Re: [PATCH 8/9] rust/hpet: Support migration

[Paolo Bonzini]

Il mer 16 apr 2025, 12:00 Zhao Liu <zhao1.liu@intel.com> ha scritto:

> So the problem is that, in a "&'a Foo<T>", T must also be "T: 'a".
> One solution is for vmsd() to return an
> Option<VMStateDescription<Self>>, and do Box::into_raw(Box::new(vmsd))
> in the class_init method. Once we have const_refs_static, "fn vmsd()"
> can become a const and the Box is not needed anymore.

Thanks so much, that's a good idea!

About `Box::into_raw(Box::new(vmsd))`, do you think it's necessary to use
Box::leak(Box::new(*))? (though the Box<> isn't actively dropped during
the class's existence)

It's the same; leak and into_raw only differ in the return type. You can use leak if you prefer.

> I attach the conversion I did of the other devices and tests. I am not
> sure if it's possible to avoid having a huge patch to do everything at
> once (except HPET since that can be added separately).

Thank you again! From my initial thoughts: Splitting is also possible,
but it requires first renaming VMStateDescription<T> to
VMStateDescriptionWrapper<T>, then replacing it in pl011 and test (and
hpet) one by one, and finally renaming it back to VMStateDescription<T>.
If you prefer this approach, I can help you split your patch below.

Or maybe first you keep vmsd() return Option<&bindings::VMStateDescription> and stick a get_vmsd_ref() in "fn vmsd()", then in a final patch you make it return Option<VMStateDescription<T>> and introduce the Box::leak(Box::new(...)) trick.

> +            vmstate_struct!(HPETState, timers[0 .. num_timers], &VMSTATE_HPET_TIMER, BqlRefCell<HPETTimer>, HPETState::validate_num_timers).with_version_id(0),

And it seems like you don't oppose the hack in patch 1? ;-)

It's not bad, but I haven't looked at why it's needed yet (i.e. why a constant function would be evaluating a destructor).

Paolo