Re: [PATCH V1 11/32] cpu: disable ticks when suspended

[Steven Sistare]

On 9/11/2020 1:53 PM, Dr. David Alan Gilbert wrote:
> * Steve Sistare (steven.sistare@oracle.com) wrote:
>> After cprload, the guest console misbehaves.  You must type 8 characters
>> before any are echoed to the terminal.  Qemu was not sending interrupts
>> to the guest because the QEMU_CLOCK_VIRTUAL timers_state.cpu_clock_offset
>> was bad.  The offset is usually updated at cprsave time by the path
>>
>>   save_cpr_snapshot()
>>     vm_stop()
>>       do_vm_stop()
>>         if (runstate_is_running())
>>           cpu_disable_ticks();
>>             timers_state.cpu_clock_offset = cpu_get_clock_locked();
>>
>> However, if the guest is in RUN_STATE_SUSPENDED, then cpu_disable_ticks is
>> not called.  Further, the earlier transition to suspended in
>> qemu_system_suspend did not disable ticks.  To fix, call cpu_disable_ticks
>> from save_cpr_snapshot.
>>
>> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
> 
> Are you saying this is really a more generic bug with migrating when
> suspended and we should fix this anyway?

Yes.  Or when suspended and calling save_vmstate(), or 
qmp_xen_save_devices_state().
Each of those functions needs the same fix unless someone identifies a more
centralized way in the state transition logic to disable ticks.

- Steve

>> ---
>>  migration/savevm.c | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/migration/savevm.c b/migration/savevm.c
>> index f101039..00f493b 100644
>> --- a/migration/savevm.c
>> +++ b/migration/savevm.c
>> @@ -2729,6 +2729,11 @@ void save_cpr_snapshot(const char *file, const char 
>> *mode, Error **errp)
>>          return;
>>      }
>>  
>> +    /* Update timers_state before saving.  Suspend did not so do. */
>> +    if (runstate_check(RUN_STATE_SUSPENDED)) {
>> +        cpu_disable_ticks();
>> +    }
>> +
>>      vm_stop(RUN_STATE_SAVE_VM);
>>  
>>      ret = qemu_savevm_state(f, op, errp);
>> -- 
>> 1.8.3.1
>>