qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH] timer/aspeed: fix vmstate version id


From: Peter Maydell
Subject: Re: [Qemu-devel] [PATCH] timer/aspeed: fix vmstate version id
Date: Mon, 23 Apr 2018 10:34:21 +0100

On 23 April 2018 at 10:28, Cédric Le Goater <address@hidden> wrote:
> On 04/23/2018 11:12 AM, Peter Maydell wrote:
>>> diff --git a/hw/timer/aspeed_timer.c b/hw/timer/aspeed_timer.c
>>> index 50acbf530a3a..7df19bd9df91 100644
>>> --- a/hw/timer/aspeed_timer.c
>>> +++ b/hw/timer/aspeed_timer.c
>>> @@ -498,8 +498,8 @@ static const VMStateDescription vmstate_aspeed_timer = {
>>>
>>>  static const VMStateDescription vmstate_aspeed_timer_state = {
>>>      .name = "aspeed.timerctrl",
>>> -    .version_id = 1,
>>> -    .minimum_version_id = 1,
>>> +    .version_id = 2,
>>> +    .minimum_version_id = 2,
>>>      .fields = (VMStateField[]) {
>>>          VMSTATE_UINT32(ctrl, AspeedTimerCtrlState),
>>>          VMSTATE_UINT32(ctrl2, AspeedTimerCtrlState),
>> Wouldn't it be simpler to just fix the incorrect value in
>> the VMSTATE_STRUCT_ARRAY(timers, AspeedTimerCtrlState,
>> line ?
>
> Yes. Also.
>
> Or bring back all the version ids to 1, as we never supported
> migration before.

I think it's nice to at least do the "bump version" thing, so you
get a (hopefully comprehensible) error rather than just wrong
data if you do try a cross version migration, so I would
vote for just fixing the one thing that was wrong: the
number in VMSTATE_STRUCT_ARRAY is the version to be used of
the substruct, so it didn't need to be bumped in commit
1d3e65aa7a; the main version numbers for vmstate_aspeed_timer
did need to be bumped because part of the main struct changed.

thanks
-- PMM



reply via email to

[Prev in Thread] Current Thread [Next in Thread]