| Message ID | 4F7EE91A.9020204@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
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ärber wrote: >>> Am 06.04.2012 09:21, schrieb Raghavendra K T: >>>> From: Eric B Munson<emunson@mgebm.net> >>>> >>>> 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 uses >>>> the qemu >>>> Notifier system to tell the guest it is about to be stopped. >>>> >>>> Signed-off-by: Eric B Munson<emunson@mgebm.net> >>>> Signed-off-by: Raghavendra K T<raghavendra.kt@linux.vnet.ibm.com> >>>> >>>> Cc: Eric B Munson<emunson@mgebm.net> >>>> Cc: Avi Kivity<avi@redhat.com> >>>> Cc: Marcelo Tosatti<mtosatti@redhat.com> >>>> Cc: Anthony Liguori<aliguori@us.ibm.com> >>>> Cc: Jan Kiszka<jan.kiszka@siemens.com> >>>> Cc: "Andreas FÀrber"<afaerber@suse.de> >>>> --- >>>> 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. > > I think I should have added Acked-by and resent full patch. So here is > it. sorry for duplicate mail. No, it was not intended as such since I can't ack the ioctl. Resends are 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 <emunson@mgebm.net> > > 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 uses > the qemu > Notifier system to tell the guest it is about to be stopped. > > Acked-by: "Andreas Färber" <afaerber@suse.de> > Signed-off-by: Eric B Munson <emunson@mgebm.net> > Signed-off-by: Raghavendra K T <raghavendra.kt@linux.vnet.ibm.com> *-bys should be added in chronological order, i.e. at the bottom. > > Cc: Eric B Munson <emunson@mgebm.net> > Cc: Avi Kivity <avi@redhat.com> > Cc: Marcelo Tosatti <mtosatti@redhat.com> > Cc: Anthony Liguori <aliguori@us.ibm.com> > Cc: Jan Kiszka <jan.kiszka@siemens.com> > Cc: "Andreas Färber" <afaerber@suse.de> > --- > 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 > > Changes from V6: > Remove unnecessary include > > Changes from V5: > KVM_GUEST_PAUSED is now a per vm ioctl instead of per vcpu > > Changes from V4: > Test if the guest paused capability is available before use > > Changes from V3: > Collapse new state change notification function into existsing function. > Correct whitespace issues > Change ioctl name to KVMCLOCK_GUEST_PAUSED > Use for loop to iterate vpcu's > > Changes from V2: > Move ioctl into hw/kvmclock.c so as other arches can use it as it is > implemented > > Changes from V1: > Remove unnecessary encapsulating function > --- > > 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 = opaque; > + CPUArchState *penv = first_cpu; > + int cap_clock_ctrl = kvm_check_extension(kvm_state, > KVM_CAP_KVMCLOCK_CTRL); > + int ret; > > if (running) { > s->clock_valid = false; > + > + 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", __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#Function-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ärber <afaerber@suse.de> Andreas > + strerror(-ret)); > + } > + return; > + } > + } > } > } >
On 04/07/2012 02:39 AM, Andreas Färber wrote: > 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ärber wrote: >>>> Am 06.04.2012 09:21, schrieb Raghavendra K T: >>>>> From: Eric B Munson<emunson@mgebm.net> >>>>> >>>>> 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 uses >>>>> the qemu >>>>> Notifier system to tell the guest it is about to be stopped. >>>>> >>>>> Signed-off-by: Eric B Munson<emunson@mgebm.net> >>>>> Signed-off-by: Raghavendra K T<raghavendra.kt@linux.vnet.ibm.com> >>>>> >>>>> Cc: Eric B Munson<emunson@mgebm.net> >>>>> Cc: Avi Kivity<avi@redhat.com> >>>>> Cc: Marcelo Tosatti<mtosatti@redhat.com> >>>>> Cc: Anthony Liguori<aliguori@us.ibm.com> >>>>> Cc: Jan Kiszka<jan.kiszka@siemens.com> >>>>> Cc: "Andreas FÀrber"<afaerber@suse.de> >>>>> --- >>>>> 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. >> >> I think I should have added Acked-by and resent full patch. So here is >> it. sorry for duplicate mail. > > No, it was not intended as such since I can't ack the ioctl. Resends are > 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. Ok. Thanks Andreas. sending V9 shortly > [...] >> + if (ret) { >> + if (ret != -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#Function-Names > __FUNCTION__ usage is currently 432 vs. __func__ 579, so not wrong. > will correct them. > If you want to leave it that way you can add my > > Reviewed-by: Andreas Färber<afaerber@suse.de> > > Andreas > >> + strerror(-ret)); >> + } >> + return; >> + } >> + } >> } >> } >> >
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 = opaque; + CPUArchState *penv = first_cpu; + int cap_clock_ctrl = kvm_check_extension(kvm_state, KVM_CAP_KVMCLOCK_CTRL); + int ret; if (running) { s->clock_valid = false; + + 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", __FUNCTION__, + strerror(-ret)); + } + return; + } + } }