diff mbox

[1/5] introduce powerdown_notifiers

Message ID 1346259767-991-2-git-send-email-imammedo@redhat.com
State New
Headers show

Commit Message

Igor Mammedov Aug. 29, 2012, 5:02 p.m. UTC 
notifier will be used for signaling powerdown request to guest in more
general way and intended to replace very specific
qemu_irq_rise(qemu_system_powerdown) and will allow to remove global
variable qemu_system_powerdown.

v2:
  do not make qemu_system_powerdown static,
      spotted-by: Paolo Bonzini <pbonzini@redhat.com>

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 sysemu.h |  1 +
 vl.c     | 15 ++++++++++++++-
 2 files changed, 15 insertions(+), 1 deletion(-)

Comments

Andreas Färber Aug. 29, 2012, 5:06 p.m. UTC | #1 
Am 29.08.2012 19:02, schrieb Igor Mammedov:
> notifier will be used for signaling powerdown request to guest in more
> general way and intended to replace very specific
> qemu_irq_rise(qemu_system_powerdown) and will allow to remove global
> variable qemu_system_powerdown.
> 
> v2:
>   do not make qemu_system_powerdown static,
>       spotted-by: Paolo Bonzini <pbonzini@redhat.com>

Is the "not" a typo or did you send the wrong patch? It seems to be
static below.

Andreas

> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
>  sysemu.h |  1 +
>  vl.c     | 15 ++++++++++++++-
>  2 files changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/sysemu.h b/sysemu.h
> index f765821..eb9a750 100644
> --- a/sysemu.h
> +++ b/sysemu.h
> @@ -53,6 +53,7 @@ void qemu_system_wakeup_enable(WakeupReason reason, bool enabled);
>  void qemu_register_wakeup_notifier(Notifier *notifier);
>  void qemu_system_shutdown_request(void);
>  void qemu_system_powerdown_request(void);
> +void qemu_register_powerdown_notifier(Notifier *notifier);
>  void qemu_system_debug_request(void);
>  void qemu_system_vmstop_request(RunState reason);
>  int qemu_shutdown_requested_get(void);
> diff --git a/vl.c b/vl.c
> index 7c577fa..8dc4b4f 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1355,6 +1355,8 @@ static int powerdown_requested;
>  static int debug_requested;
>  static int suspend_requested;
>  static int wakeup_requested;
> +static NotifierList powerdown_notifiers =
> +    NOTIFIER_LIST_INITIALIZER(powerdown_notifiers);
>  static NotifierList suspend_notifiers =
>      NOTIFIER_LIST_INITIALIZER(suspend_notifiers);
>  static NotifierList wakeup_notifiers =
> @@ -1563,12 +1565,23 @@ void qemu_system_shutdown_request(void)
>      qemu_notify_event();
>  }
>  
> +static void qemu_system_powerdown(void)
> +{
> +    monitor_protocol_event(QEVENT_POWERDOWN, NULL);
> +    notifier_list_notify(&powerdown_notifiers, NULL);
> +}
> +
>  void qemu_system_powerdown_request(void)
>  {
>      powerdown_requested = 1;
>      qemu_notify_event();
>  }
>  
> +void qemu_register_powerdown_notifier(Notifier *notifier)
> +{
> +    notifier_list_add(&powerdown_notifiers, notifier);
> +}
> +
>  void qemu_system_debug_request(void)
>  {
>      debug_requested = 1;
> @@ -1619,7 +1632,7 @@ static bool main_loop_should_exit(void)
>          monitor_protocol_event(QEVENT_WAKEUP, NULL);
>      }
>      if (qemu_powerdown_requested()) {
> -        monitor_protocol_event(QEVENT_POWERDOWN, NULL);
> +        qemu_system_powerdown();
>          qemu_irq_raise(qemu_system_powerdown);
>      }
>      if (qemu_vmstop_requested(&r)) {
>
Igor Mammedov Aug. 29, 2012, 9:55 p.m. UTC | #2 
On Wed, 29 Aug 2012 19:06:45 +0200
Andreas Färber <afaerber@suse.de> wrote:

> Am 29.08.2012 19:02, schrieb Igor Mammedov:
> > notifier will be used for signaling powerdown request to guest in more
> > general way and intended to replace very specific
> > qemu_irq_rise(qemu_system_powerdown) and will allow to remove global
> > variable qemu_system_powerdown.
> > 
> > v2:
> >   do not make qemu_system_powerdown static,
> >       spotted-by: Paolo Bonzini <pbonzini@redhat.com>
> 
> Is the "not" a typo or did you send the wrong patch? It seems to be
> static below.

Nope, it's not typo. There is global var qemu_system_powerdown, and in the
first version I've changed it to static for no good reason and that killed
bisectability of series. So I've reverted it to original state.

> 
> Andreas
> 
> > 
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> >  sysemu.h |  1 +
> >  vl.c     | 15 ++++++++++++++-
> >  2 files changed, 15 insertions(+), 1 deletion(-)
> > 
> > diff --git a/sysemu.h b/sysemu.h
> > index f765821..eb9a750 100644
> > --- a/sysemu.h
> > +++ b/sysemu.h
> > @@ -53,6 +53,7 @@ void qemu_system_wakeup_enable(WakeupReason reason, bool enabled);
> >  void qemu_register_wakeup_notifier(Notifier *notifier);
> >  void qemu_system_shutdown_request(void);
> >  void qemu_system_powerdown_request(void);
> > +void qemu_register_powerdown_notifier(Notifier *notifier);
> >  void qemu_system_debug_request(void);
> >  void qemu_system_vmstop_request(RunState reason);
> >  int qemu_shutdown_requested_get(void);
> > diff --git a/vl.c b/vl.c
> > index 7c577fa..8dc4b4f 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -1355,6 +1355,8 @@ static int powerdown_requested;
> >  static int debug_requested;
> >  static int suspend_requested;
> >  static int wakeup_requested;
> > +static NotifierList powerdown_notifiers =
> > +    NOTIFIER_LIST_INITIALIZER(powerdown_notifiers);
> >  static NotifierList suspend_notifiers =
> >      NOTIFIER_LIST_INITIALIZER(suspend_notifiers);
> >  static NotifierList wakeup_notifiers =
> > @@ -1563,12 +1565,23 @@ void qemu_system_shutdown_request(void)
> >      qemu_notify_event();
> >  }
> >  
> > +static void qemu_system_powerdown(void)
> > +{
> > +    monitor_protocol_event(QEVENT_POWERDOWN, NULL);
> > +    notifier_list_notify(&powerdown_notifiers, NULL);
> > +}
> > +
> >  void qemu_system_powerdown_request(void)
> >  {
> >      powerdown_requested = 1;
> >      qemu_notify_event();
> >  }
> >  
> > +void qemu_register_powerdown_notifier(Notifier *notifier)
> > +{
> > +    notifier_list_add(&powerdown_notifiers, notifier);
> > +}
> > +
> >  void qemu_system_debug_request(void)
> >  {
> >      debug_requested = 1;
> > @@ -1619,7 +1632,7 @@ static bool main_loop_should_exit(void)
> >          monitor_protocol_event(QEVENT_WAKEUP, NULL);
> >      }
> >      if (qemu_powerdown_requested()) {
> > -        monitor_protocol_event(QEVENT_POWERDOWN, NULL);
> > +        qemu_system_powerdown();
> >          qemu_irq_raise(qemu_system_powerdown);
> >      }
> >      if (qemu_vmstop_requested(&r)) {
> > 
> 
> 
> -- 
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
>
Igor Mammedov Aug. 30, 2012, 6:49 a.m. UTC | #3 
On Wed, 29 Aug 2012 19:06:45 +0200
Andreas Färber <afaerber@suse.de> wrote:

> Am 29.08.2012 19:02, schrieb Igor Mammedov:
> > notifier will be used for signaling powerdown request to guest in more
> > general way and intended to replace very specific
> > qemu_irq_rise(qemu_system_powerdown) and will allow to remove global
> > variable qemu_system_powerdown.
> > 
> > v2:
> >   do not make qemu_system_powerdown static,
> >       spotted-by: Paolo Bonzini <pbonzini@redhat.com>
> 
> Is the "not" a typo or did you send the wrong patch? It seems to be
> static below.
> 
> Andreas
> 
> > 
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> >  sysemu.h |  1 +
> >  vl.c     | 15 ++++++++++++++-
> >  2 files changed, 15 insertions(+), 1 deletion(-)
> > 
> > diff --git a/sysemu.h b/sysemu.h
> > index f765821..eb9a750 100644
> > --- a/sysemu.h
> > +++ b/sysemu.h
> > @@ -53,6 +53,7 @@ void qemu_system_wakeup_enable(WakeupReason reason, bool enabled);
> >  void qemu_register_wakeup_notifier(Notifier *notifier);
> >  void qemu_system_shutdown_request(void);
> >  void qemu_system_powerdown_request(void);
> > +void qemu_register_powerdown_notifier(Notifier *notifier);
> >  void qemu_system_debug_request(void);
> >  void qemu_system_vmstop_request(RunState reason);
> >  int qemu_shutdown_requested_get(void);
> > diff --git a/vl.c b/vl.c
> > index 7c577fa..8dc4b4f 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -1355,6 +1355,8 @@ static int powerdown_requested;
> >  static int debug_requested;
> >  static int suspend_requested;
> >  static int wakeup_requested;
> > +static NotifierList powerdown_notifiers =
> > +    NOTIFIER_LIST_INITIALIZER(powerdown_notifiers);
> >  static NotifierList suspend_notifiers =
> >      NOTIFIER_LIST_INITIALIZER(suspend_notifiers);
> >  static NotifierList wakeup_notifiers =
> > @@ -1563,12 +1565,23 @@ void qemu_system_shutdown_request(void)
> >      qemu_notify_event();
> >  }
> >  
> > +static void qemu_system_powerdown(void)
this is a bad naming that conflicts with global var qemu_system_powerdown, so
bisectability of series is still broken.
perhaps qemu_do_system_powerdown() would be better for function name,
although it doesn't match common name pattern used for this kind of functions.

> > +{
> > +    monitor_protocol_event(QEVENT_POWERDOWN, NULL);
> > +    notifier_list_notify(&powerdown_notifiers, NULL);
> > +}
> > +
> >  void qemu_system_powerdown_request(void)
> >  {
> >      powerdown_requested = 1;
> >      qemu_notify_event();
> >  }
> >  
> > +void qemu_register_powerdown_notifier(Notifier *notifier)
> > +{
> > +    notifier_list_add(&powerdown_notifiers, notifier);
> > +}
> > +
> >  void qemu_system_debug_request(void)
> >  {
> >      debug_requested = 1;
> > @@ -1619,7 +1632,7 @@ static bool main_loop_should_exit(void)
> >          monitor_protocol_event(QEVENT_WAKEUP, NULL);
> >      }
> >      if (qemu_powerdown_requested()) {
> > -        monitor_protocol_event(QEVENT_POWERDOWN, NULL);
> > +        qemu_system_powerdown();
> >          qemu_irq_raise(qemu_system_powerdown);
> >      }
> >      if (qemu_vmstop_requested(&r)) {
> > 
> 
> 
> -- 
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Paolo Bonzini Aug. 30, 2012, 7:41 a.m. UTC | #4 
Il 30/08/2012 08:49, Igor Mammedov ha scritto:
>>> > > +static void qemu_system_powerdown(void)
> this is a bad naming that conflicts with global var qemu_system_powerdown, so
> bisectability of series is still broken.
> perhaps qemu_do_system_powerdown() would be better for function name,
> although it doesn't match common name pattern used for this kind of functions.
> 

Just inline the function in this patch, and uninline it later.

Paolo
Igor Mammedov Aug. 30, 2012, 8:17 a.m. UTC | #5 
On Thu, 30 Aug 2012 09:41:55 +0200
Paolo Bonzini <pbonzini@redhat.com> wrote:

> Il 30/08/2012 08:49, Igor Mammedov ha scritto:
> >>> > > +static void qemu_system_powerdown(void)
> > this is a bad naming that conflicts with global var qemu_system_powerdown, so
> > bisectability of series is still broken.
> > perhaps qemu_do_system_powerdown() would be better for function name,
> > although it doesn't match common name pattern used for this kind of functions.
> > 
> 
> Just inline the function in this patch, and uninline it later.
Thanks, fixed in https://github.com/imammedo/qemu/tree/cpu_as_device.WIP
> 
> Paolo
>
diff mbox

Patch

diff --git a/sysemu.h b/sysemu.h
index f765821..eb9a750 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -53,6 +53,7 @@  void qemu_system_wakeup_enable(WakeupReason reason, bool enabled);
 void qemu_register_wakeup_notifier(Notifier *notifier);
 void qemu_system_shutdown_request(void);
 void qemu_system_powerdown_request(void);
+void qemu_register_powerdown_notifier(Notifier *notifier);
 void qemu_system_debug_request(void);
 void qemu_system_vmstop_request(RunState reason);
 int qemu_shutdown_requested_get(void);
diff --git a/vl.c b/vl.c
index 7c577fa..8dc4b4f 100644
--- a/vl.c
+++ b/vl.c
@@ -1355,6 +1355,8 @@  static int powerdown_requested;
 static int debug_requested;
 static int suspend_requested;
 static int wakeup_requested;
+static NotifierList powerdown_notifiers =
+    NOTIFIER_LIST_INITIALIZER(powerdown_notifiers);
 static NotifierList suspend_notifiers =
     NOTIFIER_LIST_INITIALIZER(suspend_notifiers);
 static NotifierList wakeup_notifiers =
@@ -1563,12 +1565,23 @@  void qemu_system_shutdown_request(void)
     qemu_notify_event();
 }
 
+static void qemu_system_powerdown(void)
+{
+    monitor_protocol_event(QEVENT_POWERDOWN, NULL);
+    notifier_list_notify(&powerdown_notifiers, NULL);
+}
+
 void qemu_system_powerdown_request(void)
 {
     powerdown_requested = 1;
     qemu_notify_event();
 }
 
+void qemu_register_powerdown_notifier(Notifier *notifier)
+{
+    notifier_list_add(&powerdown_notifiers, notifier);
+}
+
 void qemu_system_debug_request(void)
 {
     debug_requested = 1;
@@ -1619,7 +1632,7 @@  static bool main_loop_should_exit(void)
         monitor_protocol_event(QEVENT_WAKEUP, NULL);
     }
     if (qemu_powerdown_requested()) {
-        monitor_protocol_event(QEVENT_POWERDOWN, NULL);
+        qemu_system_powerdown();
         qemu_irq_raise(qemu_system_powerdown);
     }
     if (qemu_vmstop_requested(&r)) {