Patchwork [19/24] qdev: move reset handler list from vl.c to qdev.c

login
register
mail settings
Submitter Eduardo Habkost
Date Nov. 9, 2012, 2:56 p.m.
Message ID <1352473012-20500-20-git-send-email-ehabkost@redhat.com>
Download mbox | patch
Permalink /patch/198112/
State New
Headers show

Comments

Eduardo Habkost - Nov. 9, 2012, 2:56 p.m.
The core qdev code uses the reset handler list from vl.c, so move
qemu_register_reset(), qemu_unregister_reset() and qemu_devices_reset()
to qdev.c.

The function declarations were moved to a new qdev-reset.h file, that is
included by hw.h to keep compatibility, so we don't need to change all
files that use qemu_register_reset().

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/hw.h         |  6 +-----
 hw/qdev-reset.h | 11 +++++++++++
 hw/qdev.c       | 41 +++++++++++++++++++++++++++++++++++++++++
 hw/qdev.h       |  1 +
 sysemu.h        |  1 -
 vl.c            | 40 ----------------------------------------
 6 files changed, 54 insertions(+), 46 deletions(-)
 create mode 100644 hw/qdev-reset.h
Andreas Färber - Nov. 15, 2012, 1:54 a.m.
Am 09.11.2012 15:56, schrieb Eduardo Habkost:
> The core qdev code uses the reset handler list from vl.c, so move
> qemu_register_reset(), qemu_unregister_reset() and qemu_devices_reset()
> to qdev.c.
> 
> The function declarations were moved to a new qdev-reset.h file, that is
> included by hw.h to keep compatibility, so we don't need to change all
> files that use qemu_register_reset().
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  hw/hw.h         |  6 +-----
>  hw/qdev-reset.h | 11 +++++++++++
>  hw/qdev.c       | 41 +++++++++++++++++++++++++++++++++++++++++
>  hw/qdev.h       |  1 +
>  sysemu.h        |  1 -
>  vl.c            | 40 ----------------------------------------
>  6 files changed, 54 insertions(+), 46 deletions(-)
>  create mode 100644 hw/qdev-reset.h
> 
> diff --git a/hw/hw.h b/hw/hw.h
> index f530f6f..622a157 100644
> --- a/hw/hw.h
> +++ b/hw/hw.h
> @@ -14,6 +14,7 @@
>  #include "qemu-file.h"
>  #include "vmstate.h"
>  #include "qemu-log.h"
> +#include "qdev-reset.h"
>  
>  #ifdef NEED_CPU_H
>  #if TARGET_LONG_BITS == 64
> @@ -37,11 +38,6 @@
>  #endif
>  #endif
>  
> -typedef void QEMUResetHandler(void *opaque);
> -
> -void qemu_register_reset(QEMUResetHandler *func, void *opaque);
> -void qemu_unregister_reset(QEMUResetHandler *func, void *opaque);
> -
>  /* handler to set the boot_device order for a specific type of QEMUMachine */
>  /* return 0 if success */
>  typedef int QEMUBootSetHandler(void *opaque, const char *boot_devices);
> diff --git a/hw/qdev-reset.h b/hw/qdev-reset.h
> new file mode 100644
> index 0000000..40ae9a5
> --- /dev/null
> +++ b/hw/qdev-reset.h
> @@ -0,0 +1,11 @@
> +/* Device reset handler function registration, used by qdev */
> +#ifndef QDEV_RESET_H
> +#define QDEV_RESET_H
> +
> +typedef void QEMUResetHandler(void *opaque);
> +
> +void qemu_register_reset(QEMUResetHandler *func, void *opaque);
> +void qemu_unregister_reset(QEMUResetHandler *func, void *opaque);
> +void qemu_devices_reset(void);
> +
> +#endif /* QDEV_RESET_H */
> diff --git a/hw/qdev.c b/hw/qdev.c
> index 2cc6434..c242097 100644
> --- a/hw/qdev.c
> +++ b/hw/qdev.c
> @@ -35,6 +35,47 @@ int qdev_hotplug = 0;
>  static bool qdev_hot_added = false;
>  static bool qdev_hot_removed = false;
>  
> +typedef struct QEMUResetEntry {
> +    QTAILQ_ENTRY(QEMUResetEntry) entry;
> +    QEMUResetHandler *func;
> +    void *opaque;
> +} QEMUResetEntry;
> +
> +static QTAILQ_HEAD(reset_handlers, QEMUResetEntry) reset_handlers =
> +    QTAILQ_HEAD_INITIALIZER(reset_handlers);
> +
> +void qemu_register_reset(QEMUResetHandler *func, void *opaque)
> +{
> +    QEMUResetEntry *re = g_malloc0(sizeof(QEMUResetEntry));
> +
> +    re->func = func;
> +    re->opaque = opaque;
> +    QTAILQ_INSERT_TAIL(&reset_handlers, re, entry);
> +}
> +
> +void qemu_unregister_reset(QEMUResetHandler *func, void *opaque)
> +{
> +    QEMUResetEntry *re;
> +
> +    QTAILQ_FOREACH(re, &reset_handlers, entry) {
> +        if (re->func == func && re->opaque == opaque) {
> +            QTAILQ_REMOVE(&reset_handlers, re, entry);
> +            g_free(re);
> +            return;
> +        }
> +    }
> +}

My tired mind does not like this move and the naming qdev-reset.h.
The reset handling infrastructure is not limited to DeviceState (qdev),
it takes an opaque and is limited to softmmu whereas qdev prefers its
own DeviceClass::reset hook.

Andreas

> +
> +void qemu_devices_reset(void)
> +{
> +    QEMUResetEntry *re, *nre;
> +
> +    /* reset all devices */
> +    QTAILQ_FOREACH_SAFE(re, &reset_handlers, entry, nre) {
> +        re->func(re->opaque);
> +    }
> +}
> +
>  const VMStateDescription *qdev_get_vmsd(DeviceState *dev)
>  {
>      DeviceClass *dc = DEVICE_GET_CLASS(dev);
> diff --git a/hw/qdev.h b/hw/qdev.h
> index 365b8d6..2487b3b 100644
> --- a/hw/qdev.h
> +++ b/hw/qdev.h
> @@ -5,5 +5,6 @@
>  #include "qdev-core.h"
>  #include "qdev-properties.h"
>  #include "qdev-monitor.h"
> +#include "qdev-reset.h"
>  
>  #endif
> diff --git a/sysemu.h b/sysemu.h
> index ab1ef8b..51f19cc 100644
> --- a/sysemu.h
> +++ b/sysemu.h
> @@ -57,7 +57,6 @@ void qemu_system_vmstop_request(RunState reason);
>  int qemu_shutdown_requested_get(void);
>  int qemu_reset_requested_get(void);
>  void qemu_system_killed(int signal, pid_t pid);
> -void qemu_devices_reset(void);
>  void qemu_system_reset(bool report);
>  
>  void qemu_add_exit_notifier(Notifier *notify);
> diff --git a/vl.c b/vl.c
> index 4f03a72..c7448a2 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1456,14 +1456,6 @@ void vm_start(void)
>  
>  /* reset/shutdown handler */
>  
> -typedef struct QEMUResetEntry {
> -    QTAILQ_ENTRY(QEMUResetEntry) entry;
> -    QEMUResetHandler *func;
> -    void *opaque;
> -} QEMUResetEntry;
> -
> -static QTAILQ_HEAD(reset_handlers, QEMUResetEntry) reset_handlers =
> -    QTAILQ_HEAD_INITIALIZER(reset_handlers);
>  static int reset_requested;
>  static int shutdown_requested, shutdown_signal = -1;
>  static pid_t shutdown_pid;
> @@ -1560,38 +1552,6 @@ static bool qemu_vmstop_requested(RunState *r)
>      return false;
>  }
>  
> -void qemu_register_reset(QEMUResetHandler *func, void *opaque)
> -{
> -    QEMUResetEntry *re = g_malloc0(sizeof(QEMUResetEntry));
> -
> -    re->func = func;
> -    re->opaque = opaque;
> -    QTAILQ_INSERT_TAIL(&reset_handlers, re, entry);
> -}
> -
> -void qemu_unregister_reset(QEMUResetHandler *func, void *opaque)
> -{
> -    QEMUResetEntry *re;
> -
> -    QTAILQ_FOREACH(re, &reset_handlers, entry) {
> -        if (re->func == func && re->opaque == opaque) {
> -            QTAILQ_REMOVE(&reset_handlers, re, entry);
> -            g_free(re);
> -            return;
> -        }
> -    }
> -}
> -
> -void qemu_devices_reset(void)
> -{
> -    QEMUResetEntry *re, *nre;
> -
> -    /* reset all devices */
> -    QTAILQ_FOREACH_SAFE(re, &reset_handlers, entry, nre) {
> -        re->func(re->opaque);
> -    }
> -}
> -
>  void qemu_system_reset(bool report)
>  {
>      if (current_machine && current_machine->reset) {
>
Eduardo Habkost - Nov. 15, 2012, 6:42 p.m.
On Thu, Nov 15, 2012 at 02:54:57AM +0100, Andreas Färber wrote:
> Am 09.11.2012 15:56, schrieb Eduardo Habkost:
> > The core qdev code uses the reset handler list from vl.c, so move
> > qemu_register_reset(), qemu_unregister_reset() and qemu_devices_reset()
> > to qdev.c.
> > 
> > The function declarations were moved to a new qdev-reset.h file, that is
> > included by hw.h to keep compatibility, so we don't need to change all
> > files that use qemu_register_reset().
> > 
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > ---
> >  hw/hw.h         |  6 +-----
> >  hw/qdev-reset.h | 11 +++++++++++
> >  hw/qdev.c       | 41 +++++++++++++++++++++++++++++++++++++++++
> >  hw/qdev.h       |  1 +
> >  sysemu.h        |  1 -
> >  vl.c            | 40 ----------------------------------------
> >  6 files changed, 54 insertions(+), 46 deletions(-)
> >  create mode 100644 hw/qdev-reset.h
> > 
> > diff --git a/hw/hw.h b/hw/hw.h
> > index f530f6f..622a157 100644
> > --- a/hw/hw.h
> > +++ b/hw/hw.h
> > @@ -14,6 +14,7 @@
> >  #include "qemu-file.h"
> >  #include "vmstate.h"
> >  #include "qemu-log.h"
> > +#include "qdev-reset.h"
> >  
> >  #ifdef NEED_CPU_H
> >  #if TARGET_LONG_BITS == 64
> > @@ -37,11 +38,6 @@
> >  #endif
> >  #endif
> >  
> > -typedef void QEMUResetHandler(void *opaque);
> > -
> > -void qemu_register_reset(QEMUResetHandler *func, void *opaque);
> > -void qemu_unregister_reset(QEMUResetHandler *func, void *opaque);
> > -
> >  /* handler to set the boot_device order for a specific type of QEMUMachine */
> >  /* return 0 if success */
> >  typedef int QEMUBootSetHandler(void *opaque, const char *boot_devices);
> > diff --git a/hw/qdev-reset.h b/hw/qdev-reset.h
> > new file mode 100644
> > index 0000000..40ae9a5
> > --- /dev/null
> > +++ b/hw/qdev-reset.h
> > @@ -0,0 +1,11 @@
> > +/* Device reset handler function registration, used by qdev */
> > +#ifndef QDEV_RESET_H
> > +#define QDEV_RESET_H
> > +
> > +typedef void QEMUResetHandler(void *opaque);
> > +
> > +void qemu_register_reset(QEMUResetHandler *func, void *opaque);
> > +void qemu_unregister_reset(QEMUResetHandler *func, void *opaque);
> > +void qemu_devices_reset(void);
> > +
> > +#endif /* QDEV_RESET_H */
> > diff --git a/hw/qdev.c b/hw/qdev.c
> > index 2cc6434..c242097 100644
> > --- a/hw/qdev.c
> > +++ b/hw/qdev.c
> > @@ -35,6 +35,47 @@ int qdev_hotplug = 0;
> >  static bool qdev_hot_added = false;
> >  static bool qdev_hot_removed = false;
> >  
> > +typedef struct QEMUResetEntry {
> > +    QTAILQ_ENTRY(QEMUResetEntry) entry;
> > +    QEMUResetHandler *func;
> > +    void *opaque;
> > +} QEMUResetEntry;
> > +
> > +static QTAILQ_HEAD(reset_handlers, QEMUResetEntry) reset_handlers =
> > +    QTAILQ_HEAD_INITIALIZER(reset_handlers);
> > +
> > +void qemu_register_reset(QEMUResetHandler *func, void *opaque)
> > +{
> > +    QEMUResetEntry *re = g_malloc0(sizeof(QEMUResetEntry));
> > +
> > +    re->func = func;
> > +    re->opaque = opaque;
> > +    QTAILQ_INSERT_TAIL(&reset_handlers, re, entry);
> > +}
> > +
> > +void qemu_unregister_reset(QEMUResetHandler *func, void *opaque)
> > +{
> > +    QEMUResetEntry *re;
> > +
> > +    QTAILQ_FOREACH(re, &reset_handlers, entry) {
> > +        if (re->func == func && re->opaque == opaque) {
> > +            QTAILQ_REMOVE(&reset_handlers, re, entry);
> > +            g_free(re);
> > +            return;
> > +        }
> > +    }
> > +}
> 
> My tired mind does not like this move and the naming qdev-reset.h.
> The reset handling infrastructure is not limited to DeviceState (qdev),
> it takes an opaque and is limited to softmmu whereas qdev prefers its
> own DeviceClass::reset hook.

True, it isn't qdev-specific. But it doesn't belong to vl.c either.
Should we create a reset.o file just for those few lines of code, or is
there any obvious place where this code can go?

DeviceState CPUs have to be reset too, and DeviceState uses the
reset-handler system to make sure DeviceState objects are reset, so it
won't be softmmu-specific.

An alternative is to make empty stubs for qemu_[un]register_reset(), and
not move the reset-handler system to *-user. But somehow I feel better
having a working qemu_register_reset() function (and then adding a
qemu_devices_reset() call to *-user) than having a fake
qemu_register_reset() function and having to use a different API to
reset the CPUs on on *-user.

> 
> Andreas
> 
> > +
> > +void qemu_devices_reset(void)
> > +{
> > +    QEMUResetEntry *re, *nre;
> > +
> > +    /* reset all devices */
> > +    QTAILQ_FOREACH_SAFE(re, &reset_handlers, entry, nre) {
> > +        re->func(re->opaque);
> > +    }
> > +}
> > +
> >  const VMStateDescription *qdev_get_vmsd(DeviceState *dev)
> >  {
> >      DeviceClass *dc = DEVICE_GET_CLASS(dev);
> > diff --git a/hw/qdev.h b/hw/qdev.h
> > index 365b8d6..2487b3b 100644
> > --- a/hw/qdev.h
> > +++ b/hw/qdev.h
> > @@ -5,5 +5,6 @@
> >  #include "qdev-core.h"
> >  #include "qdev-properties.h"
> >  #include "qdev-monitor.h"
> > +#include "qdev-reset.h"
> >  
> >  #endif
> > diff --git a/sysemu.h b/sysemu.h
> > index ab1ef8b..51f19cc 100644
> > --- a/sysemu.h
> > +++ b/sysemu.h
> > @@ -57,7 +57,6 @@ void qemu_system_vmstop_request(RunState reason);
> >  int qemu_shutdown_requested_get(void);
> >  int qemu_reset_requested_get(void);
> >  void qemu_system_killed(int signal, pid_t pid);
> > -void qemu_devices_reset(void);
> >  void qemu_system_reset(bool report);
> >  
> >  void qemu_add_exit_notifier(Notifier *notify);
> > diff --git a/vl.c b/vl.c
> > index 4f03a72..c7448a2 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -1456,14 +1456,6 @@ void vm_start(void)
> >  
> >  /* reset/shutdown handler */
> >  
> > -typedef struct QEMUResetEntry {
> > -    QTAILQ_ENTRY(QEMUResetEntry) entry;
> > -    QEMUResetHandler *func;
> > -    void *opaque;
> > -} QEMUResetEntry;
> > -
> > -static QTAILQ_HEAD(reset_handlers, QEMUResetEntry) reset_handlers =
> > -    QTAILQ_HEAD_INITIALIZER(reset_handlers);
> >  static int reset_requested;
> >  static int shutdown_requested, shutdown_signal = -1;
> >  static pid_t shutdown_pid;
> > @@ -1560,38 +1552,6 @@ static bool qemu_vmstop_requested(RunState *r)
> >      return false;
> >  }
> >  
> > -void qemu_register_reset(QEMUResetHandler *func, void *opaque)
> > -{
> > -    QEMUResetEntry *re = g_malloc0(sizeof(QEMUResetEntry));
> > -
> > -    re->func = func;
> > -    re->opaque = opaque;
> > -    QTAILQ_INSERT_TAIL(&reset_handlers, re, entry);
> > -}
> > -
> > -void qemu_unregister_reset(QEMUResetHandler *func, void *opaque)
> > -{
> > -    QEMUResetEntry *re;
> > -
> > -    QTAILQ_FOREACH(re, &reset_handlers, entry) {
> > -        if (re->func == func && re->opaque == opaque) {
> > -            QTAILQ_REMOVE(&reset_handlers, re, entry);
> > -            g_free(re);
> > -            return;
> > -        }
> > -    }
> > -}
> > -
> > -void qemu_devices_reset(void)
> > -{
> > -    QEMUResetEntry *re, *nre;
> > -
> > -    /* reset all devices */
> > -    QTAILQ_FOREACH_SAFE(re, &reset_handlers, entry, nre) {
> > -        re->func(re->opaque);
> > -    }
> > -}
> > -
> >  void qemu_system_reset(bool report)
> >  {
> >      if (current_machine && current_machine->reset) {
> > 
> 
> 
> -- 
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Peter Maydell - Nov. 15, 2012, 6:45 p.m.
On 15 November 2012 18:42, Eduardo Habkost <ehabkost@redhat.com> wrote:
> DeviceState CPUs have to be reset too, and DeviceState uses the
> reset-handler system to make sure DeviceState objects are reset, so it
> won't be softmmu-specific.
>
> An alternative is to make empty stubs for qemu_[un]register_reset(), and
> not move the reset-handler system to *-user. But somehow I feel better
> having a working qemu_register_reset() function (and then adding a
> qemu_devices_reset() call to *-user) than having a fake
> qemu_register_reset() function and having to use a different API to
> reset the CPUs on on *-user.

cf the pretty nasty stuff in linux-user/main.c which currently
calls cpu_reset() but only for some targets.

-- PMM
Igor Mammedov - Nov. 30, 2012, 4:56 p.m.
On Thu, 15 Nov 2012 16:42:41 -0200
Eduardo Habkost <ehabkost@redhat.com> wrote:

> On Thu, Nov 15, 2012 at 02:54:57AM +0100, Andreas Färber wrote:
> > Am 09.11.2012 15:56, schrieb Eduardo Habkost:
> > > The core qdev code uses the reset handler list from vl.c, so move
> > > qemu_register_reset(), qemu_unregister_reset() and qemu_devices_reset()
> > > to qdev.c.
> > > 
> > > The function declarations were moved to a new qdev-reset.h file, that is
> > > included by hw.h to keep compatibility, so we don't need to change all
> > > files that use qemu_register_reset().
> > > 
> > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > > ---
> > >  hw/hw.h         |  6 +-----
> > >  hw/qdev-reset.h | 11 +++++++++++
> > >  hw/qdev.c       | 41 +++++++++++++++++++++++++++++++++++++++++
> > >  hw/qdev.h       |  1 +
> > >  sysemu.h        |  1 -
> > >  vl.c            | 40 ----------------------------------------
> > >  6 files changed, 54 insertions(+), 46 deletions(-)
> > >  create mode 100644 hw/qdev-reset.h
> > > 
> > > diff --git a/hw/hw.h b/hw/hw.h
> > > index f530f6f..622a157 100644
> > > --- a/hw/hw.h
> > > +++ b/hw/hw.h
> > > @@ -14,6 +14,7 @@
> > >  #include "qemu-file.h"
> > >  #include "vmstate.h"
> > >  #include "qemu-log.h"
> > > +#include "qdev-reset.h"
> > >  
> > >  #ifdef NEED_CPU_H
> > >  #if TARGET_LONG_BITS == 64
> > > @@ -37,11 +38,6 @@
> > >  #endif
> > >  #endif
> > >  
> > > -typedef void QEMUResetHandler(void *opaque);
> > > -
> > > -void qemu_register_reset(QEMUResetHandler *func, void *opaque);
> > > -void qemu_unregister_reset(QEMUResetHandler *func, void *opaque);
> > > -
> > >  /* handler to set the boot_device order for a specific type of
> > > QEMUMachine */ /* return 0 if success */
> > >  typedef int QEMUBootSetHandler(void *opaque, const char *boot_devices);
> > > diff --git a/hw/qdev-reset.h b/hw/qdev-reset.h
> > > new file mode 100644
> > > index 0000000..40ae9a5
> > > --- /dev/null
> > > +++ b/hw/qdev-reset.h
> > > @@ -0,0 +1,11 @@
> > > +/* Device reset handler function registration, used by qdev */
> > > +#ifndef QDEV_RESET_H
> > > +#define QDEV_RESET_H
> > > +
> > > +typedef void QEMUResetHandler(void *opaque);
> > > +
> > > +void qemu_register_reset(QEMUResetHandler *func, void *opaque);
> > > +void qemu_unregister_reset(QEMUResetHandler *func, void *opaque);
> > > +void qemu_devices_reset(void);
> > > +
> > > +#endif /* QDEV_RESET_H */
> > > diff --git a/hw/qdev.c b/hw/qdev.c
> > > index 2cc6434..c242097 100644
> > > --- a/hw/qdev.c
> > > +++ b/hw/qdev.c
> > > @@ -35,6 +35,47 @@ int qdev_hotplug = 0;
> > >  static bool qdev_hot_added = false;
> > >  static bool qdev_hot_removed = false;
> > >  
> > > +typedef struct QEMUResetEntry {
> > > +    QTAILQ_ENTRY(QEMUResetEntry) entry;
> > > +    QEMUResetHandler *func;
> > > +    void *opaque;
> > > +} QEMUResetEntry;
> > > +
> > > +static QTAILQ_HEAD(reset_handlers, QEMUResetEntry) reset_handlers =
> > > +    QTAILQ_HEAD_INITIALIZER(reset_handlers);
> > > +
> > > +void qemu_register_reset(QEMUResetHandler *func, void *opaque)
> > > +{
> > > +    QEMUResetEntry *re = g_malloc0(sizeof(QEMUResetEntry));
> > > +
> > > +    re->func = func;
> > > +    re->opaque = opaque;
> > > +    QTAILQ_INSERT_TAIL(&reset_handlers, re, entry);
> > > +}
> > > +
> > > +void qemu_unregister_reset(QEMUResetHandler *func, void *opaque)
> > > +{
> > > +    QEMUResetEntry *re;
> > > +
> > > +    QTAILQ_FOREACH(re, &reset_handlers, entry) {
> > > +        if (re->func == func && re->opaque == opaque) {
> > > +            QTAILQ_REMOVE(&reset_handlers, re, entry);
> > > +            g_free(re);
> > > +            return;
> > > +        }
> > > +    }
> > > +}
> > 
> > My tired mind does not like this move and the naming qdev-reset.h.
> > The reset handling infrastructure is not limited to DeviceState (qdev),
> > it takes an opaque and is limited to softmmu whereas qdev prefers its
> > own DeviceClass::reset hook.
> 
> True, it isn't qdev-specific. But it doesn't belong to vl.c either.
> Should we create a reset.o file just for those few lines of code, or is
> there any obvious place where this code can go?
> 
> DeviceState CPUs have to be reset too, and DeviceState uses the
> reset-handler system to make sure DeviceState objects are reset, so it
> won't be softmmu-specific.
> 
> An alternative is to make empty stubs for qemu_[un]register_reset(), and
> not move the reset-handler system to *-user. But somehow I feel better
> having a working qemu_register_reset() function (and then adding a
> qemu_devices_reset() call to *-user) than having a fake
> qemu_register_reset() function and having to use a different API to
> reset the CPUs on on *-user.
There is no real need to add anything, *-user uses pretty much well defined
cpu_reset() and doesn't need anything else to improve it.

PS:
reset-handler callbacks is not much loved thing in qemu, and people would
like to get rid of it eventually. If one checks target-i386/cpu.c:

/* TODO: remove me, when reset over QOM tree is implemented */
static void x86_cpu_machine_reset_cb(void *opaque)

some day in far far (or maybe not far) future there won't be reset cb for
x86-cpu.
Could you just leave reset-handler be a *-softmmu only part.
> 
> > 
> > Andreas
> > 
> > > +
> > > +void qemu_devices_reset(void)
> > > +{
> > > +    QEMUResetEntry *re, *nre;
> > > +
> > > +    /* reset all devices */
> > > +    QTAILQ_FOREACH_SAFE(re, &reset_handlers, entry, nre) {
> > > +        re->func(re->opaque);
> > > +    }
> > > +}
> > > +
> > >  const VMStateDescription *qdev_get_vmsd(DeviceState *dev)
> > >  {
> > >      DeviceClass *dc = DEVICE_GET_CLASS(dev);
> > > diff --git a/hw/qdev.h b/hw/qdev.h
> > > index 365b8d6..2487b3b 100644
> > > --- a/hw/qdev.h
> > > +++ b/hw/qdev.h
> > > @@ -5,5 +5,6 @@
> > >  #include "qdev-core.h"
> > >  #include "qdev-properties.h"
> > >  #include "qdev-monitor.h"
> > > +#include "qdev-reset.h"
> > >  
> > >  #endif
> > > diff --git a/sysemu.h b/sysemu.h
> > > index ab1ef8b..51f19cc 100644
> > > --- a/sysemu.h
> > > +++ b/sysemu.h
> > > @@ -57,7 +57,6 @@ void qemu_system_vmstop_request(RunState reason);
> > >  int qemu_shutdown_requested_get(void);
> > >  int qemu_reset_requested_get(void);
> > >  void qemu_system_killed(int signal, pid_t pid);
> > > -void qemu_devices_reset(void);
> > >  void qemu_system_reset(bool report);
> > >  
> > >  void qemu_add_exit_notifier(Notifier *notify);
> > > diff --git a/vl.c b/vl.c
> > > index 4f03a72..c7448a2 100644
> > > --- a/vl.c
> > > +++ b/vl.c
> > > @@ -1456,14 +1456,6 @@ void vm_start(void)
> > >  
> > >  /* reset/shutdown handler */
> > >  
> > > -typedef struct QEMUResetEntry {
> > > -    QTAILQ_ENTRY(QEMUResetEntry) entry;
> > > -    QEMUResetHandler *func;
> > > -    void *opaque;
> > > -} QEMUResetEntry;
> > > -
> > > -static QTAILQ_HEAD(reset_handlers, QEMUResetEntry) reset_handlers =
> > > -    QTAILQ_HEAD_INITIALIZER(reset_handlers);
> > >  static int reset_requested;
> > >  static int shutdown_requested, shutdown_signal = -1;
> > >  static pid_t shutdown_pid;
> > > @@ -1560,38 +1552,6 @@ static bool qemu_vmstop_requested(RunState *r)
> > >      return false;
> > >  }
> > >  
> > > -void qemu_register_reset(QEMUResetHandler *func, void *opaque)
> > > -{
> > > -    QEMUResetEntry *re = g_malloc0(sizeof(QEMUResetEntry));
> > > -
> > > -    re->func = func;
> > > -    re->opaque = opaque;
> > > -    QTAILQ_INSERT_TAIL(&reset_handlers, re, entry);
> > > -}
> > > -
> > > -void qemu_unregister_reset(QEMUResetHandler *func, void *opaque)
> > > -{
> > > -    QEMUResetEntry *re;
> > > -
> > > -    QTAILQ_FOREACH(re, &reset_handlers, entry) {
> > > -        if (re->func == func && re->opaque == opaque) {
> > > -            QTAILQ_REMOVE(&reset_handlers, re, entry);
> > > -            g_free(re);
> > > -            return;
> > > -        }
> > > -    }
> > > -}
> > > -
> > > -void qemu_devices_reset(void)
> > > -{
> > > -    QEMUResetEntry *re, *nre;
> > > -
> > > -    /* reset all devices */
> > > -    QTAILQ_FOREACH_SAFE(re, &reset_handlers, entry, nre) {
> > > -        re->func(re->opaque);
> > > -    }
> > > -}
> > > -
> > >  void qemu_system_reset(bool report)
> > >  {
> > >      if (current_machine && current_machine->reset) {
> > > 
> > 
> > 
> > -- 
> > SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> > GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
>
Eduardo Habkost - Nov. 30, 2012, 9:38 p.m.
On Fri, Nov 30, 2012 at 05:56:26PM +0100, Igor Mammedov wrote:
> On Thu, 15 Nov 2012 16:42:41 -0200
> Eduardo Habkost <ehabkost@redhat.com> wrote:
> 
> > On Thu, Nov 15, 2012 at 02:54:57AM +0100, Andreas Färber wrote:
> > > Am 09.11.2012 15:56, schrieb Eduardo Habkost:
> > > > The core qdev code uses the reset handler list from vl.c, so move
> > > > qemu_register_reset(), qemu_unregister_reset() and qemu_devices_reset()
> > > > to qdev.c.
> > > > 
> > > > The function declarations were moved to a new qdev-reset.h file, that is
> > > > included by hw.h to keep compatibility, so we don't need to change all
> > > > files that use qemu_register_reset().
> > > > 
> > > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > > > ---
> > > >  hw/hw.h         |  6 +-----
> > > >  hw/qdev-reset.h | 11 +++++++++++
> > > >  hw/qdev.c       | 41 +++++++++++++++++++++++++++++++++++++++++
> > > >  hw/qdev.h       |  1 +
> > > >  sysemu.h        |  1 -
> > > >  vl.c            | 40 ----------------------------------------
> > > >  6 files changed, 54 insertions(+), 46 deletions(-)
> > > >  create mode 100644 hw/qdev-reset.h
> > > > 
> > > > diff --git a/hw/hw.h b/hw/hw.h
> > > > index f530f6f..622a157 100644
> > > > --- a/hw/hw.h
> > > > +++ b/hw/hw.h
> > > > @@ -14,6 +14,7 @@
> > > >  #include "qemu-file.h"
> > > >  #include "vmstate.h"
> > > >  #include "qemu-log.h"
> > > > +#include "qdev-reset.h"
> > > >  
> > > >  #ifdef NEED_CPU_H
> > > >  #if TARGET_LONG_BITS == 64
> > > > @@ -37,11 +38,6 @@
> > > >  #endif
> > > >  #endif
> > > >  
> > > > -typedef void QEMUResetHandler(void *opaque);
> > > > -
> > > > -void qemu_register_reset(QEMUResetHandler *func, void *opaque);
> > > > -void qemu_unregister_reset(QEMUResetHandler *func, void *opaque);
> > > > -
> > > >  /* handler to set the boot_device order for a specific type of
> > > > QEMUMachine */ /* return 0 if success */
> > > >  typedef int QEMUBootSetHandler(void *opaque, const char *boot_devices);
> > > > diff --git a/hw/qdev-reset.h b/hw/qdev-reset.h
> > > > new file mode 100644
> > > > index 0000000..40ae9a5
> > > > --- /dev/null
> > > > +++ b/hw/qdev-reset.h
> > > > @@ -0,0 +1,11 @@
> > > > +/* Device reset handler function registration, used by qdev */
> > > > +#ifndef QDEV_RESET_H
> > > > +#define QDEV_RESET_H
> > > > +
> > > > +typedef void QEMUResetHandler(void *opaque);
> > > > +
> > > > +void qemu_register_reset(QEMUResetHandler *func, void *opaque);
> > > > +void qemu_unregister_reset(QEMUResetHandler *func, void *opaque);
> > > > +void qemu_devices_reset(void);
> > > > +
> > > > +#endif /* QDEV_RESET_H */
> > > > diff --git a/hw/qdev.c b/hw/qdev.c
> > > > index 2cc6434..c242097 100644
> > > > --- a/hw/qdev.c
> > > > +++ b/hw/qdev.c
> > > > @@ -35,6 +35,47 @@ int qdev_hotplug = 0;
> > > >  static bool qdev_hot_added = false;
> > > >  static bool qdev_hot_removed = false;
> > > >  
> > > > +typedef struct QEMUResetEntry {
> > > > +    QTAILQ_ENTRY(QEMUResetEntry) entry;
> > > > +    QEMUResetHandler *func;
> > > > +    void *opaque;
> > > > +} QEMUResetEntry;
> > > > +
> > > > +static QTAILQ_HEAD(reset_handlers, QEMUResetEntry) reset_handlers =
> > > > +    QTAILQ_HEAD_INITIALIZER(reset_handlers);
> > > > +
> > > > +void qemu_register_reset(QEMUResetHandler *func, void *opaque)
> > > > +{
> > > > +    QEMUResetEntry *re = g_malloc0(sizeof(QEMUResetEntry));
> > > > +
> > > > +    re->func = func;
> > > > +    re->opaque = opaque;
> > > > +    QTAILQ_INSERT_TAIL(&reset_handlers, re, entry);
> > > > +}
> > > > +
> > > > +void qemu_unregister_reset(QEMUResetHandler *func, void *opaque)
> > > > +{
> > > > +    QEMUResetEntry *re;
> > > > +
> > > > +    QTAILQ_FOREACH(re, &reset_handlers, entry) {
> > > > +        if (re->func == func && re->opaque == opaque) {
> > > > +            QTAILQ_REMOVE(&reset_handlers, re, entry);
> > > > +            g_free(re);
> > > > +            return;
> > > > +        }
> > > > +    }
> > > > +}
> > > 
> > > My tired mind does not like this move and the naming qdev-reset.h.
> > > The reset handling infrastructure is not limited to DeviceState (qdev),
> > > it takes an opaque and is limited to softmmu whereas qdev prefers its
> > > own DeviceClass::reset hook.
> > 
> > True, it isn't qdev-specific. But it doesn't belong to vl.c either.
> > Should we create a reset.o file just for those few lines of code, or is
> > there any obvious place where this code can go?
> > 
> > DeviceState CPUs have to be reset too, and DeviceState uses the
> > reset-handler system to make sure DeviceState objects are reset, so it
> > won't be softmmu-specific.
> > 
> > An alternative is to make empty stubs for qemu_[un]register_reset(), and
> > not move the reset-handler system to *-user. But somehow I feel better
> > having a working qemu_register_reset() function (and then adding a
> > qemu_devices_reset() call to *-user) than having a fake
> > qemu_register_reset() function and having to use a different API to
> > reset the CPUs on on *-user.
> There is no real need to add anything, *-user uses pretty much well defined
> cpu_reset() and doesn't need anything else to improve it.

cpu_reset() is not that well-defined, otherwise we wouldn't have this on
linux-user:

#if defined(TARGET_I386) || defined(TARGET_SPARC) || defined(TARGET_PPC)
    cpu_reset(ENV_GET_CPU(env));
#endif

(I have no idea why we have that #ifdef).

But that can be fixed, eventually, and:

> 
> PS:
> reset-handler callbacks is not much loved thing in qemu, and people would
> like to get rid of it eventually. If one checks target-i386/cpu.c:
> 
> /* TODO: remove me, when reset over QOM tree is implemented */
> static void x86_cpu_machine_reset_cb(void *opaque)
> 
> some day in far far (or maybe not far) future there won't be reset cb for
> x86-cpu.
> Could you just leave reset-handler be a *-softmmu only part.

I was considering this as well: even if cpu_reset() is not in great
shape, qemu_register_reset() is something we would like to kill as well.

I sent a v8 DeviceState series including reset.o on *-user, already, but
I will try removing it on a v9 series, next week.


> > 
> > > 
> > > Andreas
> > > 
> > > > +
> > > > +void qemu_devices_reset(void)
> > > > +{
> > > > +    QEMUResetEntry *re, *nre;
> > > > +
> > > > +    /* reset all devices */
> > > > +    QTAILQ_FOREACH_SAFE(re, &reset_handlers, entry, nre) {
> > > > +        re->func(re->opaque);
> > > > +    }
> > > > +}
> > > > +
> > > >  const VMStateDescription *qdev_get_vmsd(DeviceState *dev)
> > > >  {
> > > >      DeviceClass *dc = DEVICE_GET_CLASS(dev);
> > > > diff --git a/hw/qdev.h b/hw/qdev.h
> > > > index 365b8d6..2487b3b 100644
> > > > --- a/hw/qdev.h
> > > > +++ b/hw/qdev.h
> > > > @@ -5,5 +5,6 @@
> > > >  #include "qdev-core.h"
> > > >  #include "qdev-properties.h"
> > > >  #include "qdev-monitor.h"
> > > > +#include "qdev-reset.h"
> > > >  
> > > >  #endif
> > > > diff --git a/sysemu.h b/sysemu.h
> > > > index ab1ef8b..51f19cc 100644
> > > > --- a/sysemu.h
> > > > +++ b/sysemu.h
> > > > @@ -57,7 +57,6 @@ void qemu_system_vmstop_request(RunState reason);
> > > >  int qemu_shutdown_requested_get(void);
> > > >  int qemu_reset_requested_get(void);
> > > >  void qemu_system_killed(int signal, pid_t pid);
> > > > -void qemu_devices_reset(void);
> > > >  void qemu_system_reset(bool report);
> > > >  
> > > >  void qemu_add_exit_notifier(Notifier *notify);
> > > > diff --git a/vl.c b/vl.c
> > > > index 4f03a72..c7448a2 100644
> > > > --- a/vl.c
> > > > +++ b/vl.c
> > > > @@ -1456,14 +1456,6 @@ void vm_start(void)
> > > >  
> > > >  /* reset/shutdown handler */
> > > >  
> > > > -typedef struct QEMUResetEntry {
> > > > -    QTAILQ_ENTRY(QEMUResetEntry) entry;
> > > > -    QEMUResetHandler *func;
> > > > -    void *opaque;
> > > > -} QEMUResetEntry;
> > > > -
> > > > -static QTAILQ_HEAD(reset_handlers, QEMUResetEntry) reset_handlers =
> > > > -    QTAILQ_HEAD_INITIALIZER(reset_handlers);
> > > >  static int reset_requested;
> > > >  static int shutdown_requested, shutdown_signal = -1;
> > > >  static pid_t shutdown_pid;
> > > > @@ -1560,38 +1552,6 @@ static bool qemu_vmstop_requested(RunState *r)
> > > >      return false;
> > > >  }
> > > >  
> > > > -void qemu_register_reset(QEMUResetHandler *func, void *opaque)
> > > > -{
> > > > -    QEMUResetEntry *re = g_malloc0(sizeof(QEMUResetEntry));
> > > > -
> > > > -    re->func = func;
> > > > -    re->opaque = opaque;
> > > > -    QTAILQ_INSERT_TAIL(&reset_handlers, re, entry);
> > > > -}
> > > > -
> > > > -void qemu_unregister_reset(QEMUResetHandler *func, void *opaque)
> > > > -{
> > > > -    QEMUResetEntry *re;
> > > > -
> > > > -    QTAILQ_FOREACH(re, &reset_handlers, entry) {
> > > > -        if (re->func == func && re->opaque == opaque) {
> > > > -            QTAILQ_REMOVE(&reset_handlers, re, entry);
> > > > -            g_free(re);
> > > > -            return;
> > > > -        }
> > > > -    }
> > > > -}
> > > > -
> > > > -void qemu_devices_reset(void)
> > > > -{
> > > > -    QEMUResetEntry *re, *nre;
> > > > -
> > > > -    /* reset all devices */
> > > > -    QTAILQ_FOREACH_SAFE(re, &reset_handlers, entry, nre) {
> > > > -        re->func(re->opaque);
> > > > -    }
> > > > -}
> > > > -
> > > >  void qemu_system_reset(bool report)
> > > >  {
> > > >      if (current_machine && current_machine->reset) {
> > > > 
> > > 
> > > 
> > > -- 
> > > SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> > > GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
> > 
>
Peter Maydell - Dec. 1, 2012, 11:26 a.m.
On 30 November 2012 21:38, Eduardo Habkost <ehabkost@redhat.com> wrote:
> cpu_reset() is not that well-defined, otherwise we wouldn't have this on
> linux-user:
>
> #if defined(TARGET_I386) || defined(TARGET_SPARC) || defined(TARGET_PPC)
>     cpu_reset(ENV_GET_CPU(env));
> #endif
>
> (I have no idea why we have that #ifdef).

I think this is because the different targets disagree about whether
the CPU should be reset on initial construction or whether it needs
a specific reset call. (The current setup with #ifdefs is among other
things a historical effect as a result of various refactorings in
the past; you can trace the git history if you're interested.)

-- PMM
Andreas Färber - Dec. 2, 2012, 5:44 a.m.
Am 01.12.2012 12:26, schrieb Peter Maydell:
> On 30 November 2012 21:38, Eduardo Habkost <ehabkost@redhat.com> wrote:
>> cpu_reset() is not that well-defined, otherwise we wouldn't have this on
>> linux-user:
>>
>> #if defined(TARGET_I386) || defined(TARGET_SPARC) || defined(TARGET_PPC)
>>     cpu_reset(ENV_GET_CPU(env));
>> #endif
>>
>> (I have no idea why we have that #ifdef).
> 
> I think this is because the different targets disagree about whether
> the CPU should be reset on initial construction or whether it needs
> a specific reset call. (The current setup with #ifdefs is among other
> things a historical effect as a result of various refactorings in
> the past; you can trace the git history if you're interested.)

Peter and me had long IRC discussions about how to fix this in the past:

* On my qom-cpu-copy branch I have a patch queued that drops the
  #ifdef above, accepting that CPUs may get reset twice then.
  => Dispels doubt for target authors; doubts about correctness though.

* PMM suggested to move cpu_clone_regs() from target-*/cpu.h to *-user/.
  => Would lead to duplication between linux-user and bsd-user; ABI?

* PMM suggested to replace cpu_copy() with ABI-specific code in *-user/.
  Unfortunately I don't quite remember the details of how... ;)

The x86 APIC refactorings that Iguardo have done do bring us very close
to sane cpu_reset() semantics (ignoring the two hands full of
hard/soft/... reset variants that ppc and other architectures feature).

Declaring cpu_reset() inferior to reset handlers due to the linux-user
mess is going into the wrong direction - some targets seem to ignore
reset or fork/clone completely at the moment, so the state we see cannot
be considered fully correct.
In particular the above reset is being performed *after* cpu_copy()
memcpy()'ed random memory contents (which for some targets may contain
pointers), undoing the copying in large parts. Therefore, when all
targets reset as part of cpu_init() (or by moving the cpu_reset() call
into early cpu_copy()?) we could get rid of it in do_fork() and of its
weird conditions.

Andreas
Peter Maydell - Dec. 2, 2012, 1:37 p.m.
On 2 December 2012 05:44, Andreas Färber <afaerber@suse.de> wrote:
> Am 01.12.2012 12:26, schrieb Peter Maydell:
>> On 30 November 2012 21:38, Eduardo Habkost <ehabkost@redhat.com> wrote:
>>> cpu_reset() is not that well-defined, otherwise we wouldn't have this on
>>> linux-user:
>>>
>>> #if defined(TARGET_I386) || defined(TARGET_SPARC) || defined(TARGET_PPC)
>>>     cpu_reset(ENV_GET_CPU(env));
>>> #endif
>>>
>>> (I have no idea why we have that #ifdef).
>>
>> I think this is because the different targets disagree about whether
>> the CPU should be reset on initial construction or whether it needs
>> a specific reset call. (The current setup with #ifdefs is among other
>> things a historical effect as a result of various refactorings in
>> the past; you can trace the git history if you're interested.)
>
> Peter and me had long IRC discussions about how to fix this in the past:
>
> * On my qom-cpu-copy branch I have a patch queued that drops the
>   #ifdef above, accepting that CPUs may get reset twice then.
>   => Dispels doubt for target authors; doubts about correctness though.

So on its own, this will break things, because the cpu_reset() happens
after the cpu_copy, so we'll end up not in fact copying most of the
registers across from the original thread. In fact this looks broken
for i386/sparc/ppc at the moment, and is probably only working to the
extent that user programs don't actually care about the register state
across the clone() syscall. [and i386 threads are busted anyhow.]
If we want to retain cpu_copy for the moment, the correct place for
the cpu_reset() is in cpu_copy() itself, after the cpu_init but before
the memcpy. [I see actually you mention this later in your email.]

> * PMM suggested to move cpu_clone_regs() from target-*/cpu.h to *-user/.
>   => Would lead to duplication between linux-user and bsd-user; ABI?

clone_regs should definitely be in *-user/, because it is in fact
ABI dependent -- which register the 0 return value from the clone()
syscall ends up in is up to the ABI. Also, there won't in fact be
any duplication problem here because bsd-user doesn't implement
threads and doesn't call cpu_clone_regs().

> * PMM suggested to replace cpu_copy() with ABI-specific code in *-user/.
>   Unfortunately I don't quite remember the details of how... ;)

I don't know that I got as far as the details, but the general idea
is that the CPU state that needs to be propagated to the CPU in the
new thread is only the user-space-facing bits, which *-user/ already
needs to know about (and fish directly in CPUState for) to get things
like signal handlers right. So rather than doing a huge memcpy of
all the CPU's internal state, we just create and reset a CPU as normal,
and then have the *-user/ code copy across those bits of register
state which matter to userspace. I think this is much cleaner because
it means the CPU emulation itself has a simple interface ("create
cpu" and "reset cpu") and the details of copying thread state across
are restricted to the user-mode code.

-- PMM

Patch

diff --git a/hw/hw.h b/hw/hw.h
index f530f6f..622a157 100644
--- a/hw/hw.h
+++ b/hw/hw.h
@@ -14,6 +14,7 @@ 
 #include "qemu-file.h"
 #include "vmstate.h"
 #include "qemu-log.h"
+#include "qdev-reset.h"
 
 #ifdef NEED_CPU_H
 #if TARGET_LONG_BITS == 64
@@ -37,11 +38,6 @@ 
 #endif
 #endif
 
-typedef void QEMUResetHandler(void *opaque);
-
-void qemu_register_reset(QEMUResetHandler *func, void *opaque);
-void qemu_unregister_reset(QEMUResetHandler *func, void *opaque);
-
 /* handler to set the boot_device order for a specific type of QEMUMachine */
 /* return 0 if success */
 typedef int QEMUBootSetHandler(void *opaque, const char *boot_devices);
diff --git a/hw/qdev-reset.h b/hw/qdev-reset.h
new file mode 100644
index 0000000..40ae9a5
--- /dev/null
+++ b/hw/qdev-reset.h
@@ -0,0 +1,11 @@ 
+/* Device reset handler function registration, used by qdev */
+#ifndef QDEV_RESET_H
+#define QDEV_RESET_H
+
+typedef void QEMUResetHandler(void *opaque);
+
+void qemu_register_reset(QEMUResetHandler *func, void *opaque);
+void qemu_unregister_reset(QEMUResetHandler *func, void *opaque);
+void qemu_devices_reset(void);
+
+#endif /* QDEV_RESET_H */
diff --git a/hw/qdev.c b/hw/qdev.c
index 2cc6434..c242097 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -35,6 +35,47 @@  int qdev_hotplug = 0;
 static bool qdev_hot_added = false;
 static bool qdev_hot_removed = false;
 
+typedef struct QEMUResetEntry {
+    QTAILQ_ENTRY(QEMUResetEntry) entry;
+    QEMUResetHandler *func;
+    void *opaque;
+} QEMUResetEntry;
+
+static QTAILQ_HEAD(reset_handlers, QEMUResetEntry) reset_handlers =
+    QTAILQ_HEAD_INITIALIZER(reset_handlers);
+
+void qemu_register_reset(QEMUResetHandler *func, void *opaque)
+{
+    QEMUResetEntry *re = g_malloc0(sizeof(QEMUResetEntry));
+
+    re->func = func;
+    re->opaque = opaque;
+    QTAILQ_INSERT_TAIL(&reset_handlers, re, entry);
+}
+
+void qemu_unregister_reset(QEMUResetHandler *func, void *opaque)
+{
+    QEMUResetEntry *re;
+
+    QTAILQ_FOREACH(re, &reset_handlers, entry) {
+        if (re->func == func && re->opaque == opaque) {
+            QTAILQ_REMOVE(&reset_handlers, re, entry);
+            g_free(re);
+            return;
+        }
+    }
+}
+
+void qemu_devices_reset(void)
+{
+    QEMUResetEntry *re, *nre;
+
+    /* reset all devices */
+    QTAILQ_FOREACH_SAFE(re, &reset_handlers, entry, nre) {
+        re->func(re->opaque);
+    }
+}
+
 const VMStateDescription *qdev_get_vmsd(DeviceState *dev)
 {
     DeviceClass *dc = DEVICE_GET_CLASS(dev);
diff --git a/hw/qdev.h b/hw/qdev.h
index 365b8d6..2487b3b 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -5,5 +5,6 @@ 
 #include "qdev-core.h"
 #include "qdev-properties.h"
 #include "qdev-monitor.h"
+#include "qdev-reset.h"
 
 #endif
diff --git a/sysemu.h b/sysemu.h
index ab1ef8b..51f19cc 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -57,7 +57,6 @@  void qemu_system_vmstop_request(RunState reason);
 int qemu_shutdown_requested_get(void);
 int qemu_reset_requested_get(void);
 void qemu_system_killed(int signal, pid_t pid);
-void qemu_devices_reset(void);
 void qemu_system_reset(bool report);
 
 void qemu_add_exit_notifier(Notifier *notify);
diff --git a/vl.c b/vl.c
index 4f03a72..c7448a2 100644
--- a/vl.c
+++ b/vl.c
@@ -1456,14 +1456,6 @@  void vm_start(void)
 
 /* reset/shutdown handler */
 
-typedef struct QEMUResetEntry {
-    QTAILQ_ENTRY(QEMUResetEntry) entry;
-    QEMUResetHandler *func;
-    void *opaque;
-} QEMUResetEntry;
-
-static QTAILQ_HEAD(reset_handlers, QEMUResetEntry) reset_handlers =
-    QTAILQ_HEAD_INITIALIZER(reset_handlers);
 static int reset_requested;
 static int shutdown_requested, shutdown_signal = -1;
 static pid_t shutdown_pid;
@@ -1560,38 +1552,6 @@  static bool qemu_vmstop_requested(RunState *r)
     return false;
 }
 
-void qemu_register_reset(QEMUResetHandler *func, void *opaque)
-{
-    QEMUResetEntry *re = g_malloc0(sizeof(QEMUResetEntry));
-
-    re->func = func;
-    re->opaque = opaque;
-    QTAILQ_INSERT_TAIL(&reset_handlers, re, entry);
-}
-
-void qemu_unregister_reset(QEMUResetHandler *func, void *opaque)
-{
-    QEMUResetEntry *re;
-
-    QTAILQ_FOREACH(re, &reset_handlers, entry) {
-        if (re->func == func && re->opaque == opaque) {
-            QTAILQ_REMOVE(&reset_handlers, re, entry);
-            g_free(re);
-            return;
-        }
-    }
-}
-
-void qemu_devices_reset(void)
-{
-    QEMUResetEntry *re, *nre;
-
-    /* reset all devices */
-    QTAILQ_FOREACH_SAFE(re, &reset_handlers, entry, nre) {
-        re->func(re->opaque);
-    }
-}
-
 void qemu_system_reset(bool report)
 {
     if (current_machine && current_machine->reset) {