Re: [PATCH V5 01/12] cpus: refactor vm_stop

[Fabiano Rosas]

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.

At vm_shutdown(), this block was not executed at all, but now it is.

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,