On Sun, Sep 30, 2012 at 09:50:07PM -0400, Amos Kong wrote:
> ----- Original Message -----
> > On Thu, Sep 20, 2012 at 09:46:41AM -0300, Marcelo Tosatti wrote:
> > > On Thu, Sep 20, 2012 at 01:55:20PM +0530, Amit Shah wrote:
> > > > Commit f349c12c0434e29c79ecde89029320c4002f7253 added the guest
> > > > stop
>
> In commitlog of f349c12c0434e29c79ecde89029320c4002f7253:
>
> ## This patch uses the qemu Notifier system to tell the guest it _is about to be_ stopped
>
>
> > > > notification, but it did it in a way that the stop notification
> > > > would
> > > > never reach the kernel. The kvm_vm_state_changed() function gets
> > > > a
> > > > value of 0 for the 'running' parameter when the VM is stopped,
> > > > making
> > > > all the code added previously dead code.
> > > >
> > > > This patch reworks the code so that it's called when 'running' is
> > > > 0,
> > > > which indicates the VM was stopped.
>
> Amit, did you touch any real issue? guest gets call trace with current code?
> which kind of context?
>
> Someone told me he got call trace when shutdown guest by 'init 0', I didn't
> verify this issue.
>
> > > > CC: Eric B Munson <emunson@xxxxxxxxx>
> > > > CC: Raghavendra K T <raghavendra.kt@xxxxxxxxxxxxxxxxxx>
> > > > CC: Andreas Färber <afaerber@xxxxxxx>
> > > > CC: Marcelo Tosatti <mtosatti@xxxxxxxxxx>
> > > > CC: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> > > > CC: Laszlo Ersek <lersek@xxxxxxxxxx>
> > > > Signed-off-by: Amit Shah <amit.shah@xxxxxxxxxx>
> > > > ---
> > > > hw/kvm/clock.c | 21 +++++++++++----------
> > > > 1 files changed, 11 insertions(+), 10 deletions(-)
> > > >
> > > > diff --git a/hw/kvm/clock.c b/hw/kvm/clock.c
> > > > index 824b978..f3427eb 100644
> > > > --- a/hw/kvm/clock.c
> > > > +++ b/hw/kvm/clock.c
> > > > @@ -71,18 +71,19 @@ static void kvmclock_vm_state_change(void
> > > > *opaque, int running,
>
>
> I found this function is only called when resume vm
> (here running is 1, it means vm is already resumed?
> we don't call that ioctl _before_ resume).
>
> kvmclock_vm_state_change() is not called when I stop vm
> through qemu monitor command.
void vm_start(void)
{
if (!runstate_is_running()) {
cpu_enable_ticks();
runstate_set(RUN_STATE_RUNNING);
vm_state_notify(1, RUN_STATE_RUNNING);
resume_all_vcpus();
monitor_protocol_event(QEVENT_RESUME, NULL);
}
}
'running' is a bad name that causes confusion because it refers to the
present moment (which is not precise). IMO, better name would be 'new_state'.
> > > > if (running) {
> > > > s->clock_valid = false;
> > > > + return;
> > > > + }
> > > >
> > > > - if (!cap_clock_ctrl) {
> > > > - return;
> > > > - }
> > > > - for (penv = first_cpu; penv != NULL; penv =
> > > > penv->next_cpu) {
> > > > - ret = kvm_vcpu_ioctl(penv, KVM_KVMCLOCK_CTRL, 0);
> > > > - if (ret) {
> > > > - if (ret != -EINVAL) {
> > > > - fprintf(stderr, "%s: %s\n", __func__,
> > > > strerror(-ret));
> > > > - }
> > > > - return;
> > > > + if (!cap_clock_ctrl) {
> > > > + return;
> > > > + }
> > > > + for (penv = first_cpu; penv != NULL; penv = penv->next_cpu)
> > > > {
> > > > + ret = kvm_vcpu_ioctl(penv, KVM_KVMCLOCK_CTRL, 0);
> > > > + if (ret) {
> > > > + if (ret != -EINVAL) {
> > > > + fprintf(stderr, "%s: %s\n", __func__,
> > > > strerror(-ret));
> > > > }
> > > > + return;
> > > > }
> > > > }
> > > > }
> > > > --
> > > > 1.7.7.6
> > >
> > > ACK
> > >
> > > Avi, please merge through uq/master.
> >
> > NACK, guest should be notified when the VM is starting, not
> > when stopping.
>
> # from api.txt
> ioctl (KVM_CAP_KVMCLOCK_CTRL) can be called any time _after_ pausing
> the vcpu, but _before_ it is resumed.
This is before its actually resumed. From the QEMU code pov, "actually
resumed" would be the point where it calls ioctl(vcpu_fd, KVM_RUN).
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
