Re: [PATCH] hw/char/pl011: Fix clock migration failure

[Gavin Shan <gshan@redhat.com>]

Hi Peter,

On 3/17/21 8:09 PM, Peter Maydell wrote:
> On Wed, 17 Mar 2021 at 04:44, Gavin Shan <gshan@redhat.com> wrote:
>>
>> There is a added clock to trace buad rate change since v5.2.0 by
>> commit aac63e0e6ea3 ("hw/char/pl011: add a clock input"). The added
>> clock causes migration failure. For example, migration from v5.2.0
>> to v5.1.0 can fail with the following error messages:
>>
>>     qemu-system-aarch64: error while loading state for instance 0x0 \
>>                          of device 'pl011'
>>     qemu-system-aarch64: load of migration failed: No such file or \
>>                          directory
>>
>> This fixes the issue by reporting the baud rate change at post load
>> time so that the clock won't be migrated by sub-section to avoid the
>> migration failure.
>>
>> Cc: qemu-stable@nongnu.org
>> Fixes: aac63e0e6ea3 ("hw/char/pl011: add a clock input")
>> Signed-off-by: Gavin Shan <gshan@redhat.com>
>> ---
>>   hw/char/pl011.c | 22 +++++++++-------------
>>   1 file changed, 9 insertions(+), 13 deletions(-)
>>
>> diff --git a/hw/char/pl011.c b/hw/char/pl011.c
>> index c5621a195f..401bd28536 100644
>> --- a/hw/char/pl011.c
>> +++ b/hw/char/pl011.c
>> @@ -322,20 +322,20 @@ static const MemoryRegionOps pl011_ops = {
>>       .endianness = DEVICE_NATIVE_ENDIAN,
>>   };
>>
>> -static const VMStateDescription vmstate_pl011_clock = {
>> -    .name = "pl011/clock",
>> -    .version_id = 1,
>> -    .minimum_version_id = 1,
>> -    .fields = (VMStateField[]) {
>> -        VMSTATE_CLOCK(clk, PL011State),
>> -        VMSTATE_END_OF_LIST()
>> -    }
>> -};
>> +static int pl011_post_load(void *opaque, int version_id)
>> +{
>> +    PL011State *s = PL011(opaque);
>> +
>> +    pl011_trace_baudrate_change(s);
>> +
>> +    return 0;
>> +}
>>
>>   static const VMStateDescription vmstate_pl011 = {
>>       .name = "pl011",
>>       .version_id = 2,
>>       .minimum_version_id = 2,
>> +    .post_load = pl011_post_load,
>>       .fields = (VMStateField[]) {
>>           VMSTATE_UINT32(readbuff, PL011State),
>>           VMSTATE_UINT32(flags, PL011State),
>> @@ -355,10 +355,6 @@ static const VMStateDescription vmstate_pl011 = {
>>           VMSTATE_INT32(read_trigger, PL011State),
>>           VMSTATE_END_OF_LIST()
>>       },
>> -    .subsections = (const VMStateDescription * []) {
>> -        &vmstate_pl011_clock,
>> -        NULL
>> -    }
>>   };
> 
> Doesn't dropping the subsection break migration compat ?
> 

It's why this patch needs to be backported to stable branches.
In that way, we won't have migration compatible issue.

Thanks,
Gavin