From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:44465) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TIV9C-0002jo-9y for qemu-devel@nongnu.org; Sun, 30 Sep 2012 21:50:15 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TIV9A-0007Oc-T4 for qemu-devel@nongnu.org; Sun, 30 Sep 2012 21:50:14 -0400 Received: from mx3-phx2.redhat.com ([209.132.183.24]:40820) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TIV9A-0007Mq-K4 for qemu-devel@nongnu.org; Sun, 30 Sep 2012 21:50:12 -0400 Date: Sun, 30 Sep 2012 21:50:07 -0400 (EDT) From: Amos Kong Message-ID: <1389829631.4053667.1349056207458.JavaMail.root@redhat.com> In-Reply-To: <20120930230516.GA3923@amt.cnet> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 1/1] kvmclock: fix guest stop notification List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Marcelo Tosatti Cc: kvm list , Raghavendra K T , qemu list , Eric B Munson , Amit Shah , Paolo Bonzini , Laszlo Ersek , Andreas =?utf-8?Q?F=C3=A4rber?= ----- 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:=20 ## 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. > > >=20 > > > 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 > > > CC: Raghavendra K T > > > CC: Andreas F=C3=A4rber > > > CC: Marcelo Tosatti > > > CC: Paolo Bonzini > > > CC: Laszlo Ersek > > > Signed-off-by: Amit Shah > > > --- > > > hw/kvm/clock.c | 21 +++++++++++---------- > > > 1 files changed, 11 insertions(+), 10 deletions(-) > > >=20 > > > 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. > > > =20 > > > if (running) { > > > s->clock_valid =3D false; > > > + return; > > > + } > > > =20 > > > - if (!cap_clock_ctrl) { > > > - return; > > > - } > > > - for (penv =3D first_cpu; penv !=3D NULL; penv =3D > > > penv->next_cpu) { > > > - ret =3D kvm_vcpu_ioctl(penv, KVM_KVMCLOCK_CTRL, 0); > > > - if (ret) { > > > - if (ret !=3D -EINVAL) { > > > - fprintf(stderr, "%s: %s\n", __func__, > > > strerror(-ret)); > > > - } > > > - return; > > > + if (!cap_clock_ctrl) { > > > + return; > > > + } > > > + for (penv =3D first_cpu; penv !=3D NULL; penv =3D penv->next_cpu= ) > > > { > > > + ret =3D kvm_vcpu_ioctl(penv, KVM_KVMCLOCK_CTRL, 0); > > > + if (ret) { > > > + if (ret !=3D -EINVAL) { > > > + fprintf(stderr, "%s: %s\n", __func__, > > > strerror(-ret)); > > > } > > > + return; > > > } > > > } > > > } > > > -- > > > 1.7.7.6 > >=20 > > ACK > >=20 > > Avi, please merge through uq/master. >=20 > 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. Thanks, Amos