[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH V5 01/12] cpus: refactor vm_stop
From: |
Steven Sistare |
Subject: |
Re: [PATCH V5 01/12] cpus: refactor vm_stop |
Date: |
Mon, 20 Nov 2023 14:09:31 -0500 |
User-agent: |
Mozilla Thunderbird |
On 11/20/2023 8:22 AM, Fabiano Rosas wrote:
> Steve Sistare <steven.sistare@oracle.com> writes:
>> Refactor the vm_stop functions into one common subroutine do_vm_stop called
>> with options. No functional change.
>>
>> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
>> ---
>> system/cpus.c | 44 +++++++++++++++++---------------------------
>> 1 file changed, 17 insertions(+), 27 deletions(-)
>>
>> diff --git a/system/cpus.c b/system/cpus.c
>> index 0848e0d..f72c4be 100644
>> --- a/system/cpus.c
>> +++ b/system/cpus.c
>> @@ -252,10 +252,21 @@ void cpu_interrupt(CPUState *cpu, int mask)
>> }
>> }
>>
>> -static int do_vm_stop(RunState state, bool send_stop)
>> +static int do_vm_stop(RunState state, bool send_stop, bool force)
>> {
>> int ret = 0;
>>
>> + if (qemu_in_vcpu_thread()) {
>> + qemu_system_vmstop_request_prepare();
>> + qemu_system_vmstop_request(state);
>> + /*
>> + * FIXME: should not return to device code in case
>> + * vm_stop() has been requested.
>> + */
>> + cpu_stop_current();
>> + return 0;
>> + }
>
> At vm_stop_force_state(), this block used to be under
> runstate_is_running(), but now it runs unconditionally.
vm_stop_force_state callers should never be called in a vcpu thread, so this
block
is never executed for them. I could assert that.
> At vm_shutdown(), this block was not executed at all, but now it is.
vm_shutdown should never be called from a vcpu thread.
I could assert that.
- Steve
> We might need some words to explain why this patch doesn't affect
> functionality.
>> +
>> if (runstate_is_running()) {
>> runstate_set(state);
>> cpu_disable_ticks();
>> @@ -264,6 +275,8 @@ static int do_vm_stop(RunState state, bool send_stop)
>> if (send_stop) {
>> qapi_event_send_stop();
>> }
>> + } else if (force) {
>> + runstate_set(state);
>> }
>>
>> bdrv_drain_all();
>> @@ -278,7 +291,7 @@ static int do_vm_stop(RunState state, bool send_stop)
>> */
>> int vm_shutdown(void)
>> {
>> - return do_vm_stop(RUN_STATE_SHUTDOWN, false);
>> + return do_vm_stop(RUN_STATE_SHUTDOWN, false, false);
>> }
>>
>> bool cpu_can_run(CPUState *cpu)
>> @@ -656,18 +669,7 @@ void cpu_stop_current(void)
>>
>> int vm_stop(RunState state)
>> {
>> - if (qemu_in_vcpu_thread()) {
>> - qemu_system_vmstop_request_prepare();
>> - qemu_system_vmstop_request(state);
>> - /*
>> - * FIXME: should not return to device code in case
>> - * vm_stop() has been requested.
>> - */
>> - cpu_stop_current();
>> - return 0;
>> - }
>> -
>> - return do_vm_stop(state, true);
>> + return do_vm_stop(state, true, false);
>> }
>>
>> /**
>> @@ -723,19 +725,7 @@ void vm_start(void)
>> current state is forgotten forever */
>> int vm_stop_force_state(RunState state)
>> {
>> - if (runstate_is_running()) {
>> - return vm_stop(state);
>> - } else {
>> - int ret;
>> - runstate_set(state);
>> -
>> - bdrv_drain_all();
>> - /* Make sure to return an error if the flush in a previous vm_stop()
>> - * failed. */
>> - ret = bdrv_flush_all();
>> - trace_vm_stop_flush_all(ret);
>> - return ret;
>> - }
>> + return do_vm_stop(state, true, true);
>> }
>>
>> void qmp_memsave(int64_t addr, int64_t size, const char *filename,
[PATCH V5 03/12] cpus: pass runstate to vm_prepare_start, Steve Sistare, 2023/11/13
[PATCH V5 05/12] migration: preserve suspended runstate, Steve Sistare, 2023/11/13
[PATCH V5 09/12] tests/qtest: option to suspend during migration, Steve Sistare, 2023/11/13
[PATCH V5 06/12] migration: preserve suspended for snapshot, Steve Sistare, 2023/11/13