Re: [Qemu-devel] [PATCH V8 1/1] Guest stop notification

[Raghavendra K T]

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<address@hidden>
> >
> > 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<address@hidden>
> > Signed-off-by: Raghavendra K T<address@hidden>
> >
> > Cc: Eric B Munson<address@hidden>
> > Cc: Avi Kivity<address@hidden>
> > Cc: Marcelo Tosatti<address@hidden>
> > Cc: Anthony Liguori<address@hidden>
> > Cc: Jan Kiszka<address@hidden>
> > Cc: "Andreas FÀrber"<address@hidden>
> > ---
> > 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
> >
> > 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..c8a34a5 100644
> > --- a/hw/kvm/clock.c
> > +++ b/hw/kvm/clock.c
> > @@ -64,10 +64,28 @@ static int kvmclock_post_load(void *opaque, int version_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 = opaque;
> > +    CPUArchState *penv = first_cpu;
> > +    int cap_clock_ctrl = kvm_check_extension(kvm_state, KVM_CAP_KVMCLOCK_CTRL);
> >
> >       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,
> > +                            "kvmclock_vm_state_change: %s\n",
> > +                            strerror(-ret));
>
> I always recommend to use __func__. Otherwise looks okay to me.
>
> Andreas
>
> > +                }
> > +                return;
> > +            }
> > +        }
> >       }
> >   }
Thanks for Review. Sending with comments incorporated.
---
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;
+            }
+        }
     }
 }