Il lun 17 feb 2025, 07:35 Zhao Liu <[email protected]> ha scritto:

> > @@ -347,14 +330,13 @@ static const VMStateDescription vmstate_hpet = {
> >      .version_id = 2,
> >      .minimum_version_id = 1,
> >      .pre_save = hpet_pre_save,
> > -    .pre_load = hpet_pre_load,
> >      .post_load = hpet_post_load,
> >      .fields = (const VMStateField[]) {
> >          VMSTATE_UINT64(config, HPETState),
> >          VMSTATE_UINT64(isr, HPETState),
> >          VMSTATE_UINT64(hpet_counter, HPETState),
> > -        VMSTATE_UINT8_V(num_timers, HPETState, 2),
> > -        VMSTATE_VALIDATE("num_timers in range",
> hpet_validate_num_timers),
> > +        VMSTATE_UINT8_V(num_timers_save, HPETState, 2),
>
> This change is safe since it doesn't change the vmstate layout so that
> there's no need for bumping up the version.
>
> But I still have the question as the comment in v1 [*]. User doesn't
> have any way to modify the number of timers, why not just replace this
> vmstate field with "VMSTATE_UNUSED_V(2, 1)"?
>

Because I didn't want to bump the version; your suggestion is indeed
simpler but it would break migration to past versions of QEMU, which check
that num_timers is in range. VMSTATE_UNUSED would write a zero.

For Rust, I think it's easier to place the check in the post_load callback
BTW.

Paolo


Or do you think we should keep the status quo for the future use, even
> if these properties have not been modified yet?
>
> [*]: https://lore.kernel.org/qemu-devel/[email protected]/
>
> > +        VMSTATE_VALIDATE("num_timers must match",
> hpet_validate_num_timers),
> >          VMSTATE_STRUCT_VARRAY_UINT8(timer, HPETState, num_timers, 0,
> >                                      vmstate_hpet_timer, HPETTimer),
> >          VMSTATE_END_OF_LIST()
> > --
> > 2.48.1
> >
> >
>
>

Reply via email to