On Mon, Nov 20, 2023 at 02:09:31PM -0500, Steven Sistare wrote:
> On 11/20/2023 8:22 AM, Fabiano Rosas wrote:
> > Steve Sistare <steven.sist...@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.sist...@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.
Would you split the patch into two? Moving qemu_in_vcpu_thread() is one,
the rest can be put into another, IMHO. That may also help to make the
review easier. OTOH the code changes look all correct here. Thanks,
--
Peter Xu
