Re: [Qemu-devel] [PATCH 1/5] introduce powerdown_notifiers

[Igor Mammedov <imammedo@redhat.com>]

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


-- 
Regards,
  Igor