From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?ISO-8859-15?Q?Andreas_F=E4rber?= Subject: Re: [Qemu-devel] [PATCH V8 1/1] Guest stop notificationorry for rduplicate mail Date: Fri, 06 Apr 2012 23:09:09 +0200 Message-ID: <4F7F5B75.5020501@suse.de> References: <20120406072057.28181.31911.sendpatchset@codeblue> <4F7EB056.4080104@suse.de> <4F7EBC1F.5030300@linux.vnet.ibm.com> <4F7EE91A.9020204@linux.vnet.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: Anthony Liguori , KVM , Jan Kiszka , Marcelo Tosatti , Qemu-devel , Srivatsa Vaddagiri , "Michael J. Wolf" , Avi Kivity , Eric B Munson To: Raghavendra K T Return-path: Received: from cantor2.suse.de ([195.135.220.15]:44087 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755115Ab2DFVJQ (ORCPT ); Fri, 6 Apr 2012 17:09:16 -0400 In-Reply-To: <4F7EE91A.9020204@linux.vnet.ibm.com> Sender: kvm-owner@vger.kernel.org List-ID: Am 06.04.2012 15:01, schrieb Raghavendra K T: > On 04/06/2012 03:19 PM, Raghavendra K T wrote: >> On 04/06/2012 02:29 PM, Andreas F=E4rber wrote: >>> Am 06.04.2012 09:21, schrieb Raghavendra K T: >>>> From: Eric B Munson >>>> >>>> Often when a guest is stopped from the qemu console, it will repor= t >>>> spurious >>>> soft lockup warnings on resume. There are kernel patches being >>>> discussed that >>>> will give the host the ability to tell the guest that it is being >>>> stopped and >>>> should ignore the soft lockup warning that generates. This patch u= ses >>>> the qemu >>>> Notifier system to tell the guest it is about to be stopped. >>>> >>>> Signed-off-by: Eric B Munson >>>> Signed-off-by: Raghavendra K T >>>> >>>> Cc: Eric B Munson >>>> Cc: Avi Kivity >>>> 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. >=20 > I think I should have added Acked-by and resent full patch. So here i= s > it. sorry for duplicate mail. No, it was not intended as such since I can't ack the ioctl. Resends ar= e best done with git-send-email, i.e. a v9 with change log (whether as reply or not, opinions are divided) to make sure the right version gets applied in the end. > --- > From: Eric B Munson >=20 > Often when a guest is stopped from the qemu console, it will report > spurious > soft lockup warnings on resume. There are kernel patches being > discussed that > will give the host the ability to tell the guest that it is being > stopped 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 > Acked-by: "Andreas F=E4rber" > Signed-off-by: Eric B Munson > Signed-off-by: Raghavendra K T *-bys should be added in chronological order, i.e. at the bottom. >=20 > Cc: Eric B Munson > Cc: Avi Kivity > Cc: Marcelo Tosatti > Cc: Anthony Liguori > Cc: Jan Kiszka > Cc: "Andreas F=E4rber" > --- > Changes from V7: > capabilty changed to KVM_CAP_KVMCLOCK_CTRL > KVM_GUEST_PAUSED is pervcpu again > CPUState renamed to CPUArchState > KVMCLOCK_GUEST_PAUSED changed to KVM_KVMCLOCK_CTRL > incorporated Andrea's comments (__FUNCTION__) etc >=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..a6aa6e4 100644 > --- a/hw/kvm/clock.c > +++ b/hw/kvm/clock.c > @@ -65,9 +65,27 @@ static void kvmclock_vm_state_change(void *opaque, > int running, > RunState state) > { > KVMClockState *s =3D opaque; > + CPUArchState *penv =3D first_cpu; > + int cap_clock_ctrl =3D kvm_check_extension(kvm_state, > KVM_CAP_KVMCLOCK_CTRL); > + int ret; >=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, > + " %s: %s\n", __FUNCTION__, Is the whitespace before %s intentional? Wasn't there in v8. The GCC manual recommends __func__, like I suggested, saying it's C99. http://gcc.gnu.org/onlinedocs/gcc-4.7.0/gcc/Function-Names.html#Functio= n-Names __FUNCTION__ usage is currently 432 vs. __func__ 579, so not wrong. If you want to leave it that way you can add my Reviewed-by: Andreas F=E4rber Andreas > + strerror(-ret)); > + } > + 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