Re: [Qemu-devel] [PATCH 4/9] main-loop: refactor qemu_system_shutdown_request()

[Stefano Stabellini]

On Wed, 20 Feb 2013, Anthony Liguori wrote:
> Stefano Stabellini <address@hidden> writes:
> > On Wed, 20 Feb 2013, Anthony Liguori wrote:
> >> Paolo Bonzini <address@hidden> writes:
> >> 
> >> > Il 20/02/2013 16:32, Anthony Liguori ha scritto:
> >> >> +    if (no_shutdown) {
> >> >> +        vm_stop(RUN_STATE_SHUTDOWN);
> >> >> +    } else {
> >> >> +        main_loop_quit();
> >> >> +    }
> >> >
> >> > Would it make sense to call vm_stop() unconditionally?  Then Xen can
> >> > just use the vmstate_change handler as a hook.
> >> >
> >> > Similarly, for reset it can just use qemu_register_reset.
> >> 
> >> Yeah, that's probably doable.
> >> 
> >> But can't we just factor out the destroy_hvm_domain(false); to be called
> >> after the main loop exits?  We could even add a machine finalize hook
> >> for this purpose.  Likewise, destroy_hvm_domain(true) could be called
> >> through a machine-specific reset handler.
> >> 
> >> Stefano, does this make sense?  I can do a quick but I'm not really
> >> setup to test it.
> >
> > Reading the code and the comment in cpu_handle_ioreq:
> >
> > /*
> >  * We do this before we send the response so that the tools
> >  * have the opportunity to pick up on the reset before the
> >  * guest resumes and does a hlt with interrupts disabled which
> >  * causes Xen to powerdown the domain.
> >  */
> >
> > it seems to me that the difficult case is reset: if the guest requests a
> > machine reboot (for example via hw/piix_pci.c:rcr_write), according to
> > the comment QEMU needs to call destroy_hvm_domain(true) before notifying
> > the hypervisor that it is done with the IO request. The notification is
> > done by xc_evtchn_notify at the end of cpu_handle_ioreq.
> 
> When any of these functions are called, the current CPU is put into
> stopped state via cpu_stop_current().
> 
> Could you trigger off of that to avoid doing the xc_evtchn_notify() and
> then trigger on the eventual resume_all_vcpus() to then call
> xc_evtchn_notify()?

I don't like the fact that something strictly related to the VM
destruction becomes tied to a generic cpu_stop event that could or
could not be called at VM destruction.

In fact even today kvmapic.c calls pause_all_vcpus (that calls
cpu_stop_current()). Granted we don't use kvmapic.c on Xen, but I think
there is no reason why other devices are going to be start calling
cpu_stop in the future.
It makes the whole thing more fragile.