qemu-devel
[Top][All Lists]
Advanced

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

Re: [PATCH V6 03/14] cpus: stop vm in suspended runstate


From: Steven Sistare
Subject: Re: [PATCH V6 03/14] cpus: stop vm in suspended runstate
Date: Fri, 22 Dec 2023 10:53:24 -0500
User-agent: Mozilla Thunderbird

On 12/22/2023 7:20 AM, Markus Armbruster wrote:
> Steve Sistare <steven.sistare@oracle.com> writes:
> 
>> Currently, a vm in the suspended state is not completely stopped.  The VCPUs
>> have been paused, but the cpu clock still runs, and runstate notifiers for
>> the transition to stopped have not been called.  This causes problems for
>> live migration.  Stale cpu timers_state is saved to the migration stream,
>> causing time errors in the guest when it wakes from suspend, and state that
>> would have been modified by runstate notifiers is wrong.
>>
>> Modify vm_stop to completely stop the vm if the current state is suspended,
>> transition to RUN_STATE_PAUSED, and remember that the machine was suspended.
>> Modify vm_start to restore the suspended state.
> 
> Can you explain this to me in terms of the @current_run_state state
> machine?  Like
> 
>     Before the patch, trigger X in state Y goes to state Z.
>     Afterwards, it goes to ...

Old behavior:
  RUN_STATE_SUSPENDED --> stop --> RUN_STATE_SUSPENDED

New behavior:
  RUN_STATE_SUSPENDED --> stop --> RUN_STATE_PAUSED
  RUN_STATE_PAUSED    --> cont --> RUN_STATE_SUSPENDED

>> This affects all callers of vm_stop and vm_start, notably, the qapi stop and
>> cont commands.  For example:
>>
>>     (qemu) info status
>>     VM status: paused (suspended)
>>
>>     (qemu) stop
>>     (qemu) info status
>>     VM status: paused
>>
>>     (qemu) cont
>>     (qemu) info status
>>     VM status: paused (suspended)
>>
>>     (qemu) system_wakeup
>>     (qemu) info status
>>     VM status: running
>>
>> Suggested-by: Peter Xu <peterx@redhat.com>
>> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
>> ---
>>  include/sysemu/runstate.h |  5 +++++
>>  qapi/misc.json            | 10 ++++++++--
>>  system/cpus.c             | 19 ++++++++++++++-----
>>  system/runstate.c         |  3 +++
>>  4 files changed, 30 insertions(+), 7 deletions(-)
>>
>> diff --git a/include/sysemu/runstate.h b/include/sysemu/runstate.h
>> index f6a337b..1d6828f 100644
>> --- a/include/sysemu/runstate.h
>> +++ b/include/sysemu/runstate.h
>> @@ -40,6 +40,11 @@ static inline bool shutdown_caused_by_guest(ShutdownCause 
>> cause)
>>      return cause >= SHUTDOWN_CAUSE_GUEST_SHUTDOWN;
>>  }
>>  
>> +static inline bool runstate_is_started(RunState state)
>> +{
>> +    return state == RUN_STATE_RUNNING || state == RUN_STATE_SUSPENDED;
>> +}
>> +
>>  void vm_start(void);
>>  
>>  /**
>> diff --git a/qapi/misc.json b/qapi/misc.json
>> index cda2eff..efb8d44 100644
>> --- a/qapi/misc.json
>> +++ b/qapi/misc.json
>> @@ -134,7 +134,7 @@
>>  ##
>>  # @stop:
>>  #
>> -# Stop all guest VCPU execution.
>> +# Stop all guest VCPU and VM execution.
> 
> Doesn't "stop all VM execution" imply "stop all guest vCPU execution"?

Agreed, so we simply have:

# @stop:
# Stop guest VM execution.

# @cont:
# Resume guest VM execution.

>>  #
>>  # Since: 0.14
>>  #
>> @@ -143,6 +143,9 @@
>    # Notes: This function will succeed even if the guest is already in
>    #     the stopped state.  In "inmigrate" state, it will ensure that
>>  #     the guest remains paused once migration finishes, as if the -S
>>  #     option was passed on the command line.
>>  #
>> +#     In the "suspended" state, it will completely stop the VM and
>> +#     cause a transition to the "paused" state. (Since 9.0)
>> +#
> 
> What user-observable (with query-status) state transitions are possible
> here?

{"status": "suspended", "singlestep": false, "running": false}
--> stop -->
{"status": "paused", "singlestep": false, "running": false}

>>  # Example:
>>  #
>>  # -> { "execute": "stop" }
>> @@ -153,7 +156,7 @@
>>  ##
>>  # @cont:
>>  #
>> -# Resume guest VCPU execution.
>> +# Resume guest VCPU and VM execution.
>>  #
>>  # Since: 0.14
>>  #
>> @@ -165,6 +168,9 @@
>    # Returns: If successful, nothing
>    #
>    # Notes: This command will succeed if the guest is currently running.
>    #     It will also succeed if the guest is in the "inmigrate" state;
>    #     in this case, the effect of the command is to make sure the
>>  #     guest starts once migration finishes, removing the effect of the
>>  #     -S command line option if it was passed.
>>  #
>> +#     If the VM was previously suspended, and not been reset or woken,
>> +#     this command will transition back to the "suspended" state. (Since 
>> 9.0)
> 
> Long line.

It fits in 80 columns, but perhaps this looks nicer:

#     If the VM was previously suspended, and not been reset or woken,
#     this command will transition back to the "suspended" state.
#     (Since 9.0)

> What user-observable state transitions are possible here?

{"status": "paused", "singlestep": false, "running": false}
--> cont -->
{"status": "suspended", "singlestep": false, "running": false}

>> +#
>>  # Example:
>>  #
>>  # -> { "execute": "cont" }
> 
> Should we update documentation of query-status, too?

IMO no. The new behavior changes the status/RunState field only, and the
domain of values does not change, only the transitions caused by the commands
described here.

- Steve

>    ##
>    # @StatusInfo:
>    #
>    # Information about VCPU run state
>    #
>    # @running: true if all VCPUs are runnable, false if not runnable
>    #
>    # @singlestep: true if using TCG with one guest instruction per
>    #     translation block
>    #
>    # @status: the virtual machine @RunState
>    #
>    # Features:
>    #
>    # @deprecated: Member 'singlestep' is deprecated (with no
>    #     replacement).
>    #
>    # Since: 0.14
>    #
>    # Notes: @singlestep is enabled on the command line with '-accel
>    #     tcg,one-insn-per-tb=on', or with the HMP 'one-insn-per-tb'
>    #     command.
>    ##
>    { 'struct': 'StatusInfo',
>      'data': {'running': 'bool',
>               'singlestep': { 'type': 'bool', 'features': [ 'deprecated' ]},
>               'status': 'RunState'} }
> 
>    ##
>    # @query-status:
>    #
>    # Query the run status of all VCPUs
>    #
>    # Returns: @StatusInfo reflecting all VCPUs
>    #
>    # Since: 0.14
>    #
>    # Example:
>    #
>    # -> { "execute": "query-status" }
>    # <- { "return": { "running": true,
>    #                  "singlestep": false,
>    #                  "status": "running" } }
>    ##
>    { 'command': 'query-status', 'returns': 'StatusInfo',
>      'allow-preconfig': true }
> 
> [...]



reply via email to

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