Patchwork [v5,12/12] suspend: add qmp events

login
register
mail settings
Submitter Gerd Hoffmann
Date Feb. 15, 2012, 10:28 a.m.
Message ID <1329301701-30395-13-git-send-email-kraxel@redhat.com>
Download mbox | patch
Permalink /patch/141307/
State New
Headers show

Comments

Gerd Hoffmann - Feb. 15, 2012, 10:28 a.m.
Send qmp events on suspend and wakeup so libvirt
has a chance to track the vm state.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 monitor.c |    6 ++++++
 monitor.h |    2 ++
 vl.c      |   15 +++++++++++++++
 3 files changed, 23 insertions(+), 0 deletions(-)
Anthony Liguori - Feb. 17, 2012, 2:23 p.m.
On 02/15/2012 04:28 AM, Gerd Hoffmann wrote:
> Send qmp events on suspend and wakeup so libvirt
> has a chance to track the vm state.
>
> Signed-off-by: Gerd Hoffmann<kraxel@redhat.com>

Luiz, please Ack.

Regards,

Anthony Liguori

> ---
>   monitor.c |    6 ++++++
>   monitor.h |    2 ++
>   vl.c      |   15 +++++++++++++++
>   3 files changed, 23 insertions(+), 0 deletions(-)
>
> diff --git a/monitor.c b/monitor.c
> index aadbdcb..e6f5fad 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -485,6 +485,12 @@ void monitor_protocol_event(MonitorEvent event, QObject *data)
>           case QEVENT_BLOCK_JOB_CANCELLED:
>               event_name = "BLOCK_JOB_CANCELLED";
>               break;
> +        case QEVENT_SUSPEND:
> +            event_name = "SUSPEND";
> +            break;
> +        case QEVENT_WAKEUP:
> +            event_name = "WAKEUP";
> +            break;
>           default:
>               abort();
>               break;
> diff --git a/monitor.h b/monitor.h
> index b72ea07..9df3bab 100644
> --- a/monitor.h
> +++ b/monitor.h
> @@ -38,6 +38,8 @@ typedef enum MonitorEvent {
>       QEVENT_SPICE_DISCONNECTED,
>       QEVENT_BLOCK_JOB_COMPLETED,
>       QEVENT_BLOCK_JOB_CANCELLED,
> +    QEVENT_SUSPEND,
> +    QEVENT_WAKEUP,
>       QEVENT_MAX,
>   } MonitorEvent;
>
> diff --git a/vl.c b/vl.c
> index bfdcb7c..570ea05 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1416,6 +1416,7 @@ static void qemu_system_suspend(void)
>   {
>       pause_all_vcpus();
>       notifier_list_notify(&suspend_notifiers, NULL);
> +    monitor_protocol_event(QEVENT_SUSPEND, NULL);
>       is_suspended = true;
>   }
>
> @@ -1436,12 +1437,26 @@ void qemu_register_suspend_notifier(Notifier *notifier)
>
>   void qemu_system_wakeup_request(WakeupReason reason)
>   {
> +    static const char *names[] = {
> +        [QEMU_WAKEUP_REASON_OTHER]   = "other",
> +        [QEMU_WAKEUP_REASON_RTC]     = "rtc",
> +        [QEMU_WAKEUP_REASON_PMTIMER] = "pmtimer",
> +    };
> +    const char *name;
> +    QObject *data;
> +
>       if (!is_suspended) {
>           return;
>       }
>       if (!(wakeup_reason_mask&  (1<<  reason))) {
>           return;
>       }
> +
> +    name = (reason<  ARRAY_SIZE(names)) ? names[reason] : "unknown";
> +    data = qobject_from_jsonf("{ 'reason': %s }", name);
> +    monitor_protocol_event(QEVENT_WAKEUP, data);
> +    qobject_decref(data);
> +
>       notifier_list_notify(&wakeup_notifiers,&reason);
>       reset_requested = 1;
>       qemu_notify_event();
Luiz Capitulino - Feb. 17, 2012, 5:33 p.m.
On Wed, 15 Feb 2012 11:28:21 +0100
Gerd Hoffmann <kraxel@redhat.com> wrote:

> Send qmp events on suspend and wakeup so libvirt
> has a chance to track the vm state.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  monitor.c |    6 ++++++
>  monitor.h |    2 ++
>  vl.c      |   15 +++++++++++++++
>  3 files changed, 23 insertions(+), 0 deletions(-)
> 
> diff --git a/monitor.c b/monitor.c
> index aadbdcb..e6f5fad 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -485,6 +485,12 @@ void monitor_protocol_event(MonitorEvent event, QObject *data)
>          case QEVENT_BLOCK_JOB_CANCELLED:
>              event_name = "BLOCK_JOB_CANCELLED";
>              break;
> +        case QEVENT_SUSPEND:
> +            event_name = "SUSPEND";
> +            break;
> +        case QEVENT_WAKEUP:
> +            event_name = "WAKEUP";
> +            break;
>          default:
>              abort();
>              break;
> diff --git a/monitor.h b/monitor.h
> index b72ea07..9df3bab 100644
> --- a/monitor.h
> +++ b/monitor.h
> @@ -38,6 +38,8 @@ typedef enum MonitorEvent {
>      QEVENT_SPICE_DISCONNECTED,
>      QEVENT_BLOCK_JOB_COMPLETED,
>      QEVENT_BLOCK_JOB_CANCELLED,
> +    QEVENT_SUSPEND,
> +    QEVENT_WAKEUP,
>      QEVENT_MAX,
>  } MonitorEvent;
>  
> diff --git a/vl.c b/vl.c
> index bfdcb7c..570ea05 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1416,6 +1416,7 @@ static void qemu_system_suspend(void)
>  {
>      pause_all_vcpus();
>      notifier_list_notify(&suspend_notifiers, NULL);
> +    monitor_protocol_event(QEVENT_SUSPEND, NULL);
>      is_suspended = true;
>  }
>  
> @@ -1436,12 +1437,26 @@ void qemu_register_suspend_notifier(Notifier *notifier)
>  
>  void qemu_system_wakeup_request(WakeupReason reason)
>  {
> +    static const char *names[] = {
> +        [QEMU_WAKEUP_REASON_OTHER]   = "other",
> +        [QEMU_WAKEUP_REASON_RTC]     = "rtc",
> +        [QEMU_WAKEUP_REASON_PMTIMER] = "pmtimer",

Is the reason really important for mngt? Can we just leave it out?

> +    };
> +    const char *name;
> +    QObject *data;
> +
>      if (!is_suspended) {
>          return;
>      }
>      if (!(wakeup_reason_mask & (1 << reason))) {
>          return;
>      }
> +
> +    name = (reason < ARRAY_SIZE(names)) ? names[reason] : "unknown";
> +    data = qobject_from_jsonf("{ 'reason': %s }", name);
> +    monitor_protocol_event(QEVENT_WAKEUP, data);
> +    qobject_decref(data);
> +
>      notifier_list_notify(&wakeup_notifiers, &reason);
>      reset_requested = 1;
>      qemu_notify_event();
Gerd Hoffmann - Feb. 21, 2012, 10 a.m.
On 02/17/12 18:33, Luiz Capitulino wrote:
> On Wed, 15 Feb 2012 11:28:21 +0100
> Gerd Hoffmann <kraxel@redhat.com> wrote:
> 
>> Send qmp events on suspend and wakeup so libvirt
>> has a chance to track the vm state.

[ added libvirt to Cc:, leaving full context.
  this is about qmp events when the guest enters/leaves s3 ].

>> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
>> ---
>>  monitor.c |    6 ++++++
>>  monitor.h |    2 ++
>>  vl.c      |   15 +++++++++++++++
>>  3 files changed, 23 insertions(+), 0 deletions(-)
>>
>> diff --git a/monitor.c b/monitor.c
>> index aadbdcb..e6f5fad 100644
>> --- a/monitor.c
>> +++ b/monitor.c
>> @@ -485,6 +485,12 @@ void monitor_protocol_event(MonitorEvent event, QObject *data)
>>          case QEVENT_BLOCK_JOB_CANCELLED:
>>              event_name = "BLOCK_JOB_CANCELLED";
>>              break;
>> +        case QEVENT_SUSPEND:
>> +            event_name = "SUSPEND";
>> +            break;
>> +        case QEVENT_WAKEUP:
>> +            event_name = "WAKEUP";
>> +            break;
>>          default:
>>              abort();
>>              break;
>> diff --git a/monitor.h b/monitor.h
>> index b72ea07..9df3bab 100644
>> --- a/monitor.h
>> +++ b/monitor.h
>> @@ -38,6 +38,8 @@ typedef enum MonitorEvent {
>>      QEVENT_SPICE_DISCONNECTED,
>>      QEVENT_BLOCK_JOB_COMPLETED,
>>      QEVENT_BLOCK_JOB_CANCELLED,
>> +    QEVENT_SUSPEND,
>> +    QEVENT_WAKEUP,
>>      QEVENT_MAX,
>>  } MonitorEvent;
>>  
>> diff --git a/vl.c b/vl.c
>> index bfdcb7c..570ea05 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -1416,6 +1416,7 @@ static void qemu_system_suspend(void)
>>  {
>>      pause_all_vcpus();
>>      notifier_list_notify(&suspend_notifiers, NULL);
>> +    monitor_protocol_event(QEVENT_SUSPEND, NULL);
>>      is_suspended = true;
>>  }
>>  
>> @@ -1436,12 +1437,26 @@ void qemu_register_suspend_notifier(Notifier *notifier)
>>  
>>  void qemu_system_wakeup_request(WakeupReason reason)
>>  {
>> +    static const char *names[] = {
>> +        [QEMU_WAKEUP_REASON_OTHER]   = "other",
>> +        [QEMU_WAKEUP_REASON_RTC]     = "rtc",
>> +        [QEMU_WAKEUP_REASON_PMTIMER] = "pmtimer",
> 
> Is the reason really important for mngt? Can we just leave it out?

I assumed the wakeup reason could be useful, but dunno.
Zapping the code is no problem of course.

Lets ask mgmt aka libvirt folks ;)

cheers,
  Gerd

>> +    };
>> +    const char *name;
>> +    QObject *data;
>> +
>>      if (!is_suspended) {
>>          return;
>>      }
>>      if (!(wakeup_reason_mask & (1 << reason))) {
>>          return;
>>      }
>> +
>> +    name = (reason < ARRAY_SIZE(names)) ? names[reason] : "unknown";
>> +    data = qobject_from_jsonf("{ 'reason': %s }", name);
>> +    monitor_protocol_event(QEVENT_WAKEUP, data);
>> +    qobject_decref(data);
>> +
>>      notifier_list_notify(&wakeup_notifiers, &reason);
>>      reset_requested = 1;
>>      qemu_notify_event();
>
Eric Blake - Feb. 21, 2012, 4:30 p.m.
On 02/21/2012 03:00 AM, Gerd Hoffmann wrote:
> [ added libvirt to Cc:, leaving full context.
>   this is about qmp events when the guest enters/leaves s3 ].
> 

>>> @@ -1436,12 +1437,26 @@ void qemu_register_suspend_notifier(Notifier *notifier)
>>>  
>>>  void qemu_system_wakeup_request(WakeupReason reason)
>>>  {
>>> +    static const char *names[] = {
>>> +        [QEMU_WAKEUP_REASON_OTHER]   = "other",
>>> +        [QEMU_WAKEUP_REASON_RTC]     = "rtc",
>>> +        [QEMU_WAKEUP_REASON_PMTIMER] = "pmtimer",
>>
>> Is the reason really important for mngt? Can we just leave it out?
> 
> I assumed the wakeup reason could be useful, but dunno.
> Zapping the code is no problem of course.
> 
> Lets ask mgmt aka libvirt folks ;)

If you present the reason, libvirt will pass it on to the management
app.  But I don't see any reason why libvirt itself would care about the
reason, so it won't hurt libvirt if you drop a reason field.  I don't
know whether higher-level management apps, like oVirt, would care or not.

Isn't this something where it is easier to omit first and add later once
we have a use case, than to add up front only to find that no one cares?
Gerd Hoffmann - Feb. 22, 2012, 12:08 p.m.
Hi,

> Isn't this something where it is easier to omit first and add later
> once we have a use case, than to add up front only to find that no
> one cares?

Yes. I'll drop it.

cheers,
  Gerd
Luiz Capitulino - Feb. 22, 2012, 1:22 p.m.
On Wed, 22 Feb 2012 13:08:16 +0100
Gerd Hoffmann <kraxel@redhat.com> wrote:

>   Hi,
> 
> > Isn't this something where it is easier to omit first and add later
> > once we have a use case, than to add up front only to find that no
> > one cares?
> 
> Yes. I'll drop it.

Yes, I think it's better to drop it for now and add it back if really needed,
when we'll know better how mngt apps are going to use this.

Patch

diff --git a/monitor.c b/monitor.c
index aadbdcb..e6f5fad 100644
--- a/monitor.c
+++ b/monitor.c
@@ -485,6 +485,12 @@  void monitor_protocol_event(MonitorEvent event, QObject *data)
         case QEVENT_BLOCK_JOB_CANCELLED:
             event_name = "BLOCK_JOB_CANCELLED";
             break;
+        case QEVENT_SUSPEND:
+            event_name = "SUSPEND";
+            break;
+        case QEVENT_WAKEUP:
+            event_name = "WAKEUP";
+            break;
         default:
             abort();
             break;
diff --git a/monitor.h b/monitor.h
index b72ea07..9df3bab 100644
--- a/monitor.h
+++ b/monitor.h
@@ -38,6 +38,8 @@  typedef enum MonitorEvent {
     QEVENT_SPICE_DISCONNECTED,
     QEVENT_BLOCK_JOB_COMPLETED,
     QEVENT_BLOCK_JOB_CANCELLED,
+    QEVENT_SUSPEND,
+    QEVENT_WAKEUP,
     QEVENT_MAX,
 } MonitorEvent;
 
diff --git a/vl.c b/vl.c
index bfdcb7c..570ea05 100644
--- a/vl.c
+++ b/vl.c
@@ -1416,6 +1416,7 @@  static void qemu_system_suspend(void)
 {
     pause_all_vcpus();
     notifier_list_notify(&suspend_notifiers, NULL);
+    monitor_protocol_event(QEVENT_SUSPEND, NULL);
     is_suspended = true;
 }
 
@@ -1436,12 +1437,26 @@  void qemu_register_suspend_notifier(Notifier *notifier)
 
 void qemu_system_wakeup_request(WakeupReason reason)
 {
+    static const char *names[] = {
+        [QEMU_WAKEUP_REASON_OTHER]   = "other",
+        [QEMU_WAKEUP_REASON_RTC]     = "rtc",
+        [QEMU_WAKEUP_REASON_PMTIMER] = "pmtimer",
+    };
+    const char *name;
+    QObject *data;
+
     if (!is_suspended) {
         return;
     }
     if (!(wakeup_reason_mask & (1 << reason))) {
         return;
     }
+
+    name = (reason < ARRAY_SIZE(names)) ? names[reason] : "unknown";
+    data = qobject_from_jsonf("{ 'reason': %s }", name);
+    monitor_protocol_event(QEVENT_WAKEUP, data);
+    qobject_decref(data);
+
     notifier_list_notify(&wakeup_notifiers, &reason);
     reset_requested = 1;
     qemu_notify_event();