Re: [PATCH V8 1/1] Guest stop notification

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxx>

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@xxxxxxxxx>
Signed-off-by: Raghavendra K T<raghavendra.kt@xxxxxxxxxxxxxxxxxx>

Cc: Eric B Munson<emunson@xxxxxxxxx>
Cc: Avi Kivity<avi@xxxxxxxxxx>
Cc: Marcelo Tosatti<mtosatti@xxxxxxxxxx>
Cc: Anthony Liguori<aliguori@xxxxxxxxxx>
Cc: Jan Kiszka<jan.kiszka@xxxxxxxxxxx>
Cc: "Andreas FÀrber"<afaerber@xxxxxxx>
---
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;
+            }
+        }
     }
 }


--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux