From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?ISO-8859-15?Q?Andreas_F=E4rber?= Subject: Re: [PATCH V8 1/1] Guest stop notification Date: Fri, 06 Apr 2012 10:59:02 +0200 Message-ID: <4F7EB056.4080104@suse.de> References: <20120406072057.28181.31911.sendpatchset@codeblue> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: Anthony Liguori , Jan Kiszka , Qemu-devel , Srivatsa Vaddagiri , KVM , Marcelo Tosatti , Avi Kivity , "Michael J. Wolf" , Eric B Munson To: Raghavendra K T Return-path: Received: from cantor2.suse.de ([195.135.220.15]:55150 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752822Ab2DFI7K (ORCPT ); Fri, 6 Apr 2012 04:59:10 -0400 In-Reply-To: <20120406072057.28181.31911.sendpatchset@codeblue> Sender: kvm-owner@vger.kernel.org List-ID: Am 06.04.2012 09:21, schrieb Raghavendra K T: > From: Eric B Munson >=20 > Often when a guest is stopped from the qemu console, it will report s= purious > soft lockup warnings on resume. There are kernel patches being discu= ssed that > will give the host the ability to tell the guest that it is being sto= pped and > should ignore the soft lockup warning that generates. This patch use= s the qemu > Notifier system to tell the guest it is about to be stopped. >=20 > Signed-off-by: Eric B Munson =20 > Signed-off-by: Raghavendra K T >=20 > Cc: Eric B Munson > Cc: Avi Kivity =20 > Cc: Marcelo Tosatti > Cc: Anthony Liguori > Cc: Jan Kiszka > Cc: "Andreas F=C3=A4rber" > --- > Changes from V7: > capabilty changed to KVM_CAP_KVMCLOCK_CTRL > KVM_GUEST_PAUSED is pervcpu again > CPUState renamed to CPUArchState Thanks, change looks right to me. Long-term I should probably consider supplying some cpu_foreach() macro to iterate over them, but that would still need manual declaration of a properly typed variable for the CPUArchState -> CPUState switch. > KVMCLOCK_GUEST_PAUSED changed to KVM_KVMCLOCK_CTRL >=20 > Changes from V6: > Remove unnecessary include >=20 > Changes from V5: > KVM_GUEST_PAUSED is now a per vm ioctl instead of per vcpu >=20 > Changes from V4: > Test if the guest paused capability is available before use >=20 > Changes from V3: > Collapse new state change notification function into existsing funct= ion. > Correct whitespace issues > Change ioctl name to KVMCLOCK_GUEST_PAUSED > Use for loop to iterate vpcu's >=20 > Changes from V2: > Move ioctl into hw/kvmclock.c so as other arches can use it as it is > implemented >=20 > Changes from V1: > Remove unnecessary encapsulating function > --- >=20 > diff --git a/hw/kvm/clock.c b/hw/kvm/clock.c > index 446bd62..c8a34a5 100644 > --- a/hw/kvm/clock.c > +++ b/hw/kvm/clock.c > @@ -64,10 +64,28 @@ static int kvmclock_post_load(void *opaque, int v= ersion_id) > static void kvmclock_vm_state_change(void *opaque, int running, > RunState state) > { > + int ret; Minor nitpick: We usually assign opaque values first thing in the function, so maybe order ret last if you resend? > KVMClockState *s =3D opaque; > + CPUArchState *penv =3D first_cpu; > + int cap_clock_ctrl =3D kvm_check_extension(kvm_state, KVM_CAP_KV= MCLOCK_CTRL); > =20 > if (running) { > s->clock_valid =3D false; > + > + 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, > + "kvmclock_vm_state_change: %s\n", > + strerror(-ret)); I always recommend to use __func__. Otherwise looks okay to me. Andreas > + } > + return; > + } > + } > } > } > =20 --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCrn= berg