Re: [Qemu-devel] [PATCH 1/1] vl: change QEMU state machine for system reset

["Denis V. Lunev" <den@openvz.org>]

On 01/21/2016 01:35 PM, Paolo Bonzini wrote:
>
> On 19/01/2016 08:59, Denis V. Lunev wrote:
>> @@ -612,8 +617,10 @@ static const RunStateTransition runstate_transitions_def[] = {
>>   
>>       { RUN_STATE_FINISH_MIGRATE, RUN_STATE_RUNNING },
>>       { RUN_STATE_FINISH_MIGRATE, RUN_STATE_POSTMIGRATE },
>> +    { RUN_STATE_FINISH_MIGRATE, RUN_STATE_PRELAUNCH },
>>   
>>       { RUN_STATE_RESTORE_VM, RUN_STATE_RUNNING },
>> +    { RUN_STATE_RESTORE_VM, RUN_STATE_PRELAUNCH },
>>   
>>       { RUN_STATE_RUNNING, RUN_STATE_DEBUG },
>>       { RUN_STATE_RUNNING, RUN_STATE_INTERNAL_ERROR },
>> @@ -627,20 +634,25 @@ static const RunStateTransition runstate_transitions_def[] = {
>>       { RUN_STATE_RUNNING, RUN_STATE_GUEST_PANICKED },
>>   
>>       { RUN_STATE_SAVE_VM, RUN_STATE_RUNNING },
>> +    { RUN_STATE_SAVE_VM, RUN_STATE_PRELAUNCH },
> This should not happen; thus I would remove this line.
>
>>       { RUN_STATE_SHUTDOWN, RUN_STATE_PAUSED },
>>       { RUN_STATE_SHUTDOWN, RUN_STATE_FINISH_MIGRATE },
>> +    { RUN_STATE_SHUTDOWN, RUN_STATE_PRELAUNCH },
>>   
>>       { RUN_STATE_DEBUG, RUN_STATE_SUSPENDED },
>>       { RUN_STATE_RUNNING, RUN_STATE_SUSPENDED },
>>       { RUN_STATE_SUSPENDED, RUN_STATE_RUNNING },
>>       { RUN_STATE_SUSPENDED, RUN_STATE_FINISH_MIGRATE },
>> +    { RUN_STATE_SUSPENDED, RUN_STATE_PRELAUNCH },
>>   
>>       { RUN_STATE_WATCHDOG, RUN_STATE_RUNNING },
>>       { RUN_STATE_WATCHDOG, RUN_STATE_FINISH_MIGRATE },
>> +    { RUN_STATE_WATCHDOG, RUN_STATE_PRELAUNCH },
>>   
>>       { RUN_STATE_GUEST_PANICKED, RUN_STATE_RUNNING },
>>       { RUN_STATE_GUEST_PANICKED, RUN_STATE_FINISH_MIGRATE },
>> +    { RUN_STATE_GUEST_PANICKED, RUN_STATE_PRELAUNCH },
>>   
>>       { RUN_STATE__MAX, RUN_STATE__MAX },
>>   };
>> @@ -1886,8 +1899,10 @@ static bool main_loop_should_exit(void)
>>           cpu_synchronize_all_states();
>>           qemu_system_reset(VMRESET_REPORT);
>>           resume_all_vcpus();
>> -        if (runstate_needs_reset()) {
>> -            runstate_set(RUN_STATE_PAUSED);
>> +        if (!runstate_check(RUN_STATE_RUNNING) &&
>> +                !runstate_check(RUN_STATE_INMIGRATE) &&
>> +                !runstate_check(RUN_STATE_SAVE_VM)) {
> Since SAVE_VM should not happen here, I would leave this check out too.
>   If it happens, the lack of a SAVE_VM->PRELAUNCH transition will cause
> an assertion failure.
>
> Thanks for the patch!
>
> Paolo
>
:) cool. This one is tied to one of our internal bugs thus I have to 
finish it some way.
The rest will be handled in libvirt by my colleague.

I much more worry about bdrv_invalidate_cache(). This problem crashes QEMU
with 100% probability in our stress testing. Manually it crashes 1 times 
out of 5
('virsh managed-save && virsh start' in parallel with
'while /bin/true; do virsh qemu-monitor-command --hmp info block ; done'

Den