On Wed, 20 Feb 2013, Anthony Liguori wrote: > Stefano Stabellini <stefano.stabell...@eu.citrix.com> writes: > > On Wed, 20 Feb 2013, Anthony Liguori wrote: > >> Paolo Bonzini <pbonz...@redhat.com> 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.