public inbox for kvm@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH V8 1/1] Guest stop notification
@ 2012-04-06  7:21 Raghavendra K T
  2012-04-06  8:59 ` Andreas Färber
  0 siblings, 1 reply; 6+ messages in thread
From: Raghavendra K T @ 2012-04-06  7:21 UTC (permalink / raw)
  To: Anthony Liguori, Jan Kiszka, Andreas Färber, Qemu-devel
  Cc: Marcelo Tosatti, KVM, Raghavendra K T, Srivatsa Vaddagiri,
	Michael J. Wolf, Avi Kivity, Eric B Munson

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 2551 bytes --]

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
 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;
     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));
+                }
+                return;
+            }
+        }
     }
 }
 

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH V8 1/1] Guest stop notification
  2012-04-06  7:21 [PATCH V8 1/1] Guest stop notification Raghavendra K T
@ 2012-04-06  8:59 ` Andreas Färber
  2012-04-06  9:49   ` Raghavendra K T
  0 siblings, 1 reply; 6+ messages in thread
From: Andreas Färber @ 2012-04-06  8:59 UTC (permalink / raw)
  To: Raghavendra K T
  Cc: Anthony Liguori, Jan Kiszka, Qemu-devel, Srivatsa Vaddagiri, KVM,
	Marcelo Tosatti, Avi Kivity, Michael J. Wolf, Eric B Munson

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.

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;
> +            }
> +        }
>      }
>  }
>  

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH V8 1/1] Guest stop notification
  2012-04-06  8:59 ` Andreas Färber
@ 2012-04-06  9:49   ` Raghavendra K T
  2012-04-06 13:01     ` [Qemu-devel] [PATCH V8 1/1] Guest stop notificationorry for rduplicate mail Raghavendra K T
  0 siblings, 1 reply; 6+ messages in thread
From: Raghavendra K T @ 2012-04-06  9:49 UTC (permalink / raw)
  To: Andreas Färber
  Cc: Anthony Liguori, Jan Kiszka, Qemu-devel, Srivatsa Vaddagiri, KVM,
	Marcelo Tosatti, Avi Kivity, Michael J. Wolf, Eric B Munson

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.
>
> 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;
+            }
+        }
      }
  }



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH V8 1/1] Guest stop notificationorry for rduplicate mail
  2012-04-06  9:49   ` Raghavendra K T
@ 2012-04-06 13:01     ` Raghavendra K T
  2012-04-06 21:09       ` Andreas Färber
  0 siblings, 1 reply; 6+ messages in thread
From: Raghavendra K T @ 2012-04-06 13:01 UTC (permalink / raw)
  To: Andreas Färber
  Cc: Anthony Liguori, KVM, Jan Kiszka, Marcelo Tosatti, Qemu-devel,
	Srivatsa Vaddagiri, Michael J. Wolf, Avi Kivity, Eric B Munson

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.
---
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>

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__,
+                            strerror(-ret));
+                }
+                return;
+            }
+        }
      }
  }


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH V8 1/1] Guest stop notificationorry for rduplicate mail
  2012-04-06 13:01     ` [Qemu-devel] [PATCH V8 1/1] Guest stop notificationorry for rduplicate mail Raghavendra K T
@ 2012-04-06 21:09       ` Andreas Färber
  2012-04-07  0:27         ` [Qemu-devel] [PATCH V8 1/1] Guest stop notificationorry for rduplicate mail ndreas Raghavendra K T
  0 siblings, 1 reply; 6+ messages in thread
From: Andreas Färber @ 2012-04-06 21:09 UTC (permalink / raw)
  To: Raghavendra K T
  Cc: Anthony Liguori, KVM, Jan Kiszka, Marcelo Tosatti, Qemu-devel,
	Srivatsa Vaddagiri, Michael J. Wolf, Avi Kivity, Eric B Munson

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;
> +            }
> +        }
>      }
>  }
> 

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH V8 1/1] Guest stop notificationorry for rduplicate mail ndreas
  2012-04-06 21:09       ` Andreas Färber
@ 2012-04-07  0:27         ` Raghavendra K T
  0 siblings, 0 replies; 6+ messages in thread
From: Raghavendra K T @ 2012-04-07  0:27 UTC (permalink / raw)
  To: Andreas Färber
  Cc: Anthony Liguori, KVM, Jan Kiszka, Marcelo Tosatti, Qemu-devel,
	Srivatsa Vaddagiri, Michael J. Wolf, Avi Kivity, Eric B Munson

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;
>> +            }
>> +        }
>>       }
>>   }
>>
>


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2012-04-07  0:28 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-04-06  7:21 [PATCH V8 1/1] Guest stop notification Raghavendra K T
2012-04-06  8:59 ` Andreas Färber
2012-04-06  9:49   ` Raghavendra K T
2012-04-06 13:01     ` [Qemu-devel] [PATCH V8 1/1] Guest stop notificationorry for rduplicate mail Raghavendra K T
2012-04-06 21:09       ` Andreas Färber
2012-04-07  0:27         ` [Qemu-devel] [PATCH V8 1/1] Guest stop notificationorry for rduplicate mail ndreas Raghavendra K T

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox