[PATCH v6 2/5] shutdown: Prepare for use of an enum in reset/shutdown_request

[PATCH v6 2/5] shutdown: Prepare for use of an enum in reset/shutdown_request

[Eric Blake]

We want to track why a guest was shutdown; in particular, being able
to tell the difference between a guest request (such as ACPI request)
and host request (such as SIGINT) will prove useful to libvirt.
Since all requests eventually end up changing shutdown_requested in
vl.c, the logical change is to make that value track the reason,
rather than its current 0/1 contents.

Since command-line options control whether a reset request is turned
into a shutdown request instead, the same treatment is given to
reset_requested.

This patch adds an internal enum ShutdownCause that describes reasons
that a shutdown can be requested, and changes qemu_system_reset() to
pass the reason through, although for now it is not reported.  The
enum could be exported via QAPI at a later date, if deemed necessary,
but for now, there has not been a request to expose that much detail
to end clients.

For now, we only populate the reason with HOST_ERROR, along with FIXME
comments that describe our plans for how to pass an actual correct
reason.  The next patches will then actually wire things up to modify
events to report data based on the reason, and to pass the correct enum
value in from various call-sites that can trigger a reset/shutdown (big
enough that it was worth splitting from this patch).

Signed-off-by: Eric Blake <eblake@redhat.com>

---
v6: make ShutdownCause internal-only, add SHUTDOWN_CAUSE_NONE so that
comparison to 0 still works, tweak initial FIXME values
v5: no change
v4: s/ShutdownType/ShutdownCause/, no thanks to mingw header pollution
v3: new patch
---
 include/sysemu/sysemu.h | 22 ++++++++++++++++++---
 vl.c                    | 51 +++++++++++++++++++++++++++++++------------------
 hw/i386/xen/xen-hvm.c   |  7 +++++--
 migration/colo.c        |  2 +-
 migration/savevm.c      |  2 +-
 5 files changed, 58 insertions(+), 26 deletions(-)

diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 16175f7..e4da9d4 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -36,6 +36,22 @@ void vm_state_notify(int running, RunState state);
 #define VMRESET_SILENT   false
 #define VMRESET_REPORT   true

+/* Enumeration of various causes for shutdown. */
+typedef enum ShutdownCause ShutdownCause;
+enum ShutdownCause {
+    SHUTDOWN_CAUSE_NONE,          /* No shutdown requested yet */
+    SHUTDOWN_CAUSE_HOST_QMP,      /* Reaction to a QMP command, like 'quit' */
+    SHUTDOWN_CAUSE_HOST_SIGNAL,   /* Reaction to a signal, such as SIGINT */
+    SHUTDOWN_CAUSE_HOST_UI,       /* Reaction to UI event, like window close */
+    SHUTDOWN_CAUSE_HOST_ERROR,    /* An error prevents further use of guest */
+    SHUTDOWN_CAUSE_GUEST_SHUTDOWN,/* Guest requested shutdown, such as via
+                                     ACPI or other hardware-specific means */
+    SHUTDOWN_CAUSE_GUEST_RESET,   /* Guest requested reset, and command line
+                                     turns that into a shutdown */
+    SHUTDOWN_CAUSE_GUEST_PANIC,   /* Guest panicked, and command line turns
+                                     that into a shutdown */
+};
+
 void vm_start(void);
 int vm_prepare_start(void);
 int vm_stop(RunState state);
@@ -62,10 +78,10 @@ void qemu_system_debug_request(void);
 void qemu_system_vmstop_request(RunState reason);
 void qemu_system_vmstop_request_prepare(void);
 bool qemu_vmstop_requested(RunState *r);
-int qemu_shutdown_requested_get(void);
-int qemu_reset_requested_get(void);
+ShutdownCause qemu_shutdown_requested_get(void);
+ShutdownCause qemu_reset_requested_get(void);
 void qemu_system_killed(int signal, pid_t pid);
-void qemu_system_reset(bool report);
+void qemu_system_reset(bool report, ShutdownCause reason);
 void qemu_system_guest_panicked(GuestPanicInformation *info);
 size_t qemu_target_page_size(void);

diff --git a/vl.c b/vl.c
index f22a3ac..6069fb2 100644
--- a/vl.c
+++ b/vl.c
@@ -1597,8 +1597,9 @@ void vm_state_notify(int running, RunState state)
     }
 }

-static int reset_requested;
-static int shutdown_requested, shutdown_signal;
+static ShutdownCause reset_requested;
+static ShutdownCause shutdown_requested;
+static int shutdown_signal;
 static pid_t shutdown_pid;
 static int powerdown_requested;
 static int debug_requested;
@@ -1612,19 +1613,19 @@ static NotifierList wakeup_notifiers =
     NOTIFIER_LIST_INITIALIZER(wakeup_notifiers);
 static uint32_t wakeup_reason_mask = ~(1 << QEMU_WAKEUP_REASON_NONE);

-int qemu_shutdown_requested_get(void)
+ShutdownCause qemu_shutdown_requested_get(void)
 {
     return shutdown_requested;
 }

-int qemu_reset_requested_get(void)
+ShutdownCause qemu_reset_requested_get(void)
 {
     return reset_requested;
 }

 static int qemu_shutdown_requested(void)
 {
-    return atomic_xchg(&shutdown_requested, 0);
+    return atomic_xchg(&shutdown_requested, SHUTDOWN_CAUSE_NONE);
 }

 static void qemu_kill_report(void)
@@ -1647,14 +1648,14 @@ static void qemu_kill_report(void)
     }
 }

-static int qemu_reset_requested(void)
+static ShutdownCause qemu_reset_requested(void)
 {
-    int r = reset_requested;
+    ShutdownCause r = reset_requested;
     if (r && replay_checkpoint(CHECKPOINT_RESET_REQUESTED)) {
-        reset_requested = 0;
+        reset_requested = SHUTDOWN_CAUSE_NONE;
         return r;
     }
-    return false;
+    return SHUTDOWN_CAUSE_NONE;
 }

 static int qemu_suspend_requested(void)
@@ -1686,7 +1687,12 @@ static int qemu_debug_requested(void)
     return r;
 }

-void qemu_system_reset(bool report)
+/*
+ * Reset the VM. If @report is VMRESET_REPORT, issue an event, using
+ * the @reason interpreted as ShutdownCause for details.  Otherwise,
+ * @report is VMRESET_SILENT and @reason is ignored.
+ */
+void qemu_system_reset(bool report, ShutdownCause reason)
 {
     MachineClass *mc;

@@ -1700,6 +1706,7 @@ void qemu_system_reset(bool report)
         qemu_devices_reset();
     }
     if (report) {
+        assert(reason);
         qapi_event_send_reset(&error_abort);
     }
     cpu_synchronize_all_post_reset();
@@ -1738,9 +1745,10 @@ void qemu_system_guest_panicked(GuestPanicInformation *info)
 void qemu_system_reset_request(void)
 {
     if (no_reboot) {
-        shutdown_requested = 1;
+        /* FIXME - add a parameter to allow callers to specify reason */
+        shutdown_requested = SHUTDOWN_CAUSE_HOST_ERROR;
     } else {
-        reset_requested = 1;
+        reset_requested = SHUTDOWN_CAUSE_HOST_ERROR;
     }
     cpu_stop_current();
     qemu_notify_event();
@@ -1807,7 +1815,7 @@ void qemu_system_killed(int signal, pid_t pid)
     /* Cannot call qemu_system_shutdown_request directly because
      * we are in a signal handler.
      */
-    shutdown_requested = 1;
+    shutdown_requested = SHUTDOWN_CAUSE_HOST_SIGNAL;
     qemu_notify_event();
 }

@@ -1815,7 +1823,8 @@ void qemu_system_shutdown_request(void)
 {
     trace_qemu_system_shutdown_request();
     replay_shutdown_request();
-    shutdown_requested = 1;
+    /* FIXME - add a parameter to allow callers to specify reason */
+    shutdown_requested = SHUTDOWN_CAUSE_HOST_ERROR;
     qemu_notify_event();
 }

@@ -1846,13 +1855,16 @@ void qemu_system_debug_request(void)
 static bool main_loop_should_exit(void)
 {
     RunState r;
+    ShutdownCause request;
+
     if (qemu_debug_requested()) {
         vm_stop(RUN_STATE_DEBUG);
     }
     if (qemu_suspend_requested()) {
         qemu_system_suspend();
     }
-    if (qemu_shutdown_requested()) {
+    request = qemu_shutdown_requested();
+    if (request) {
         qemu_kill_report();
         qapi_event_send_shutdown(&error_abort);
         if (no_shutdown) {
@@ -1861,9 +1873,10 @@ static bool main_loop_should_exit(void)
             return true;
         }
     }
-    if (qemu_reset_requested()) {
+    request = qemu_reset_requested();
+    if (request) {
         pause_all_vcpus();
-        qemu_system_reset(VMRESET_REPORT);
+        qemu_system_reset(VMRESET_REPORT, request);
         resume_all_vcpus();
         if (!runstate_check(RUN_STATE_RUNNING) &&
                 !runstate_check(RUN_STATE_INMIGRATE)) {
@@ -1872,7 +1885,7 @@ static bool main_loop_should_exit(void)
     }
     if (qemu_wakeup_requested()) {
         pause_all_vcpus();
-        qemu_system_reset(VMRESET_SILENT);
+        qemu_system_reset(VMRESET_SILENT, SHUTDOWN_CAUSE_NONE);
         notifier_list_notify(&wakeup_notifiers, &wakeup_reason);
         wakeup_reason = QEMU_WAKEUP_REASON_NONE;
         resume_all_vcpus();
@@ -4686,7 +4699,7 @@ int main(int argc, char **argv, char **envp)
        reading from the other reads, because timer polling functions query
        clock values from the log. */
     replay_checkpoint(CHECKPOINT_RESET);
-    qemu_system_reset(VMRESET_SILENT);
+    qemu_system_reset(VMRESET_SILENT, SHUTDOWN_CAUSE_NONE);
     register_global_state();
     if (replay_mode != REPLAY_MODE_NONE) {
         replay_vmstate_init();
diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
index b1c05ff..b1001c1 100644
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -1089,11 +1089,14 @@ static void cpu_handle_ioreq(void *opaque)
          * causes Xen to powerdown the domain.
          */
         if (runstate_is_running()) {
+            ShutdownCause request;
+
             if (qemu_shutdown_requested_get()) {
                 destroy_hvm_domain(false);
             }
-            if (qemu_reset_requested_get()) {
-                qemu_system_reset(VMRESET_REPORT);
+            request = qemu_reset_requested_get();
+            if (request) {
+                qemu_system_reset(VMRESET_REPORT, request);
                 destroy_hvm_domain(true);
             }
         }
diff --git a/migration/colo.c b/migration/colo.c
index 963c802..bf5b7e9 100644
--- a/migration/colo.c
+++ b/migration/colo.c
@@ -623,7 +623,7 @@ void *colo_process_incoming_thread(void *opaque)
         }

         qemu_mutex_lock_iothread();
-        qemu_system_reset(VMRESET_SILENT);
+        qemu_system_reset(VMRESET_SILENT, SHUTDOWN_CAUSE_NONE);
         vmstate_loading = true;
         if (qemu_loadvm_state(fb) < 0) {
             error_report("COLO: loadvm failed");
diff --git a/migration/savevm.c b/migration/savevm.c
index a00c1ab..9ac2d22 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2300,7 +2300,7 @@ int load_vmstate(const char *name)
         return -EINVAL;
     }

-    qemu_system_reset(VMRESET_SILENT);
+    qemu_system_reset(VMRESET_SILENT, SHUTDOWN_CAUSE_NONE);
     mis->from_src_file = f;

     aio_context_acquire(aio_context);
-- 
2.9.3


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Qemu-devel] [PATCH v6 2/5] shutdown: Prepare for use of an enum in reset/shutdown_request

[Markus Armbruster]

Eric Blake <eblake@redhat.com> writes:

> We want to track why a guest was shutdown; in particular, being able
> to tell the difference between a guest request (such as ACPI request)
> and host request (such as SIGINT) will prove useful to libvirt.
> Since all requests eventually end up changing shutdown_requested in
> vl.c, the logical change is to make that value track the reason,
> rather than its current 0/1 contents.
>
> Since command-line options control whether a reset request is turned
> into a shutdown request instead, the same treatment is given to
> reset_requested.
>
> This patch adds an internal enum ShutdownCause that describes reasons
> that a shutdown can be requested, and changes qemu_system_reset() to
> pass the reason through, although for now it is not reported.  The
> enum could be exported via QAPI at a later date, if deemed necessary,
> but for now, there has not been a request to expose that much detail
> to end clients.
>
> For now, we only populate the reason with HOST_ERROR, along with FIXME
> comments that describe our plans for how to pass an actual correct
> reason.

In other words, replacing 0 by SHUTDOWN_CAUSE_NONE, and 1 by
SHUTDOWN_CAUSE_HOST_ERROR.  Makes sense.

>          The next patches will then actually wire things up to modify
> events to report data based on the reason, and to pass the correct enum
> value in from various call-sites that can trigger a reset/shutdown (big
> enough that it was worth splitting from this patch).
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
>
> ---
> v6: make ShutdownCause internal-only, add SHUTDOWN_CAUSE_NONE so that
> comparison to 0 still works, tweak initial FIXME values
> v5: no change
> v4: s/ShutdownType/ShutdownCause/, no thanks to mingw header pollution
> v3: new patch
> ---
>  include/sysemu/sysemu.h | 22 ++++++++++++++++++---
>  vl.c                    | 51 +++++++++++++++++++++++++++++++------------------
>  hw/i386/xen/xen-hvm.c   |  7 +++++--
>  migration/colo.c        |  2 +-
>  migration/savevm.c      |  2 +-
>  5 files changed, 58 insertions(+), 26 deletions(-)
>
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index 16175f7..e4da9d4 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -36,6 +36,22 @@ void vm_state_notify(int running, RunState state);
>  #define VMRESET_SILENT   false
>  #define VMRESET_REPORT   true
>
> +/* Enumeration of various causes for shutdown. */
> +typedef enum ShutdownCause ShutdownCause;
> +enum ShutdownCause {

Why define the typedef separately here?  What's wrong with

    typedef enum ShutdownCause {
        ...
    } ShutdownCause;

?

> +    SHUTDOWN_CAUSE_NONE,          /* No shutdown requested yet */

Comment is fine.  Possible alternative: /* No shutdown request pending */

> +    SHUTDOWN_CAUSE_HOST_QMP,      /* Reaction to a QMP command, like 'quit' */
> +    SHUTDOWN_CAUSE_HOST_SIGNAL,   /* Reaction to a signal, such as SIGINT */
> +    SHUTDOWN_CAUSE_HOST_UI,       /* Reaction to UI event, like window close */
> +    SHUTDOWN_CAUSE_HOST_ERROR,    /* An error prevents further use of guest */
> +    SHUTDOWN_CAUSE_GUEST_SHUTDOWN,/* Guest requested shutdown, such as via
> +                                     ACPI or other hardware-specific means */
> +    SHUTDOWN_CAUSE_GUEST_RESET,   /* Guest requested reset, and command line
> +                                     turns that into a shutdown */
> +    SHUTDOWN_CAUSE_GUEST_PANIC,   /* Guest panicked, and command line turns
> +                                     that into a shutdown */
> +};
> +
>  void vm_start(void);
>  int vm_prepare_start(void);
>  int vm_stop(RunState state);
> @@ -62,10 +78,10 @@ void qemu_system_debug_request(void);
>  void qemu_system_vmstop_request(RunState reason);
>  void qemu_system_vmstop_request_prepare(void);
>  bool qemu_vmstop_requested(RunState *r);
> -int qemu_shutdown_requested_get(void);
> -int qemu_reset_requested_get(void);
> +ShutdownCause qemu_shutdown_requested_get(void);
> +ShutdownCause qemu_reset_requested_get(void);
>  void qemu_system_killed(int signal, pid_t pid);
> -void qemu_system_reset(bool report);
> +void qemu_system_reset(bool report, ShutdownCause reason);
>  void qemu_system_guest_panicked(GuestPanicInformation *info);
>  size_t qemu_target_page_size(void);
>
> diff --git a/vl.c b/vl.c
> index f22a3ac..6069fb2 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1597,8 +1597,9 @@ void vm_state_notify(int running, RunState state)
>      }
>  }
>
> -static int reset_requested;
> -static int shutdown_requested, shutdown_signal;
> +static ShutdownCause reset_requested;
> +static ShutdownCause shutdown_requested;
> +static int shutdown_signal;
>  static pid_t shutdown_pid;
>  static int powerdown_requested;
>  static int debug_requested;
> @@ -1612,19 +1613,19 @@ static NotifierList wakeup_notifiers =
>      NOTIFIER_LIST_INITIALIZER(wakeup_notifiers);
>  static uint32_t wakeup_reason_mask = ~(1 << QEMU_WAKEUP_REASON_NONE);
>
> -int qemu_shutdown_requested_get(void)
> +ShutdownCause qemu_shutdown_requested_get(void)
>  {
>      return shutdown_requested;
>  }
>
> -int qemu_reset_requested_get(void)
> +ShutdownCause qemu_reset_requested_get(void)
>  {
>      return reset_requested;
>  }
>
>  static int qemu_shutdown_requested(void)
>  {
> -    return atomic_xchg(&shutdown_requested, 0);
> +    return atomic_xchg(&shutdown_requested, SHUTDOWN_CAUSE_NONE);
>  }
>
>  static void qemu_kill_report(void)
> @@ -1647,14 +1648,14 @@ static void qemu_kill_report(void)
>      }
>  }
>
> -static int qemu_reset_requested(void)
> +static ShutdownCause qemu_reset_requested(void)
>  {
> -    int r = reset_requested;
> +    ShutdownCause r = reset_requested;

Good opportunity to insert a blank line here.

>      if (r && replay_checkpoint(CHECKPOINT_RESET_REQUESTED)) {
> -        reset_requested = 0;
> +        reset_requested = SHUTDOWN_CAUSE_NONE;
>          return r;
>      }
> -    return false;
> +    return SHUTDOWN_CAUSE_NONE;
>  }
>
>  static int qemu_suspend_requested(void)
> @@ -1686,7 +1687,12 @@ static int qemu_debug_requested(void)
>      return r;
>  }
>
> -void qemu_system_reset(bool report)
> +/*
> + * Reset the VM. If @report is VMRESET_REPORT, issue an event, using
> + * the @reason interpreted as ShutdownCause for details.  Otherwise,
> + * @report is VMRESET_SILENT and @reason is ignored.
> + */

"interpreted as ShutdownCause"?  It *is* a ShutdownCause.  Leftover?

> +void qemu_system_reset(bool report, ShutdownCause reason)
>  {
>      MachineClass *mc;
>
> @@ -1700,6 +1706,7 @@ void qemu_system_reset(bool report)
>          qemu_devices_reset();
>      }
>      if (report) {
> +        assert(reason);
>          qapi_event_send_reset(&error_abort);
>      }
>      cpu_synchronize_all_post_reset();

Looks like we're not using @reason "for details" just yet.

> @@ -1738,9 +1745,10 @@ void qemu_system_guest_panicked(GuestPanicInformation *info)
>  void qemu_system_reset_request(void)
>  {
>      if (no_reboot) {
> -        shutdown_requested = 1;
> +        /* FIXME - add a parameter to allow callers to specify reason */
> +        shutdown_requested = SHUTDOWN_CAUSE_HOST_ERROR;
>      } else {
> -        reset_requested = 1;
> +        reset_requested = SHUTDOWN_CAUSE_HOST_ERROR;
>      }
>      cpu_stop_current();
>      qemu_notify_event();
> @@ -1807,7 +1815,7 @@ void qemu_system_killed(int signal, pid_t pid)
>      /* Cannot call qemu_system_shutdown_request directly because
>       * we are in a signal handler.
>       */
> -    shutdown_requested = 1;
> +    shutdown_requested = SHUTDOWN_CAUSE_HOST_SIGNAL;

Should this be SHUTDOWN_CAUSE_HOST_ERROR, to be updated in the next
patch?  Alternatively, tweak this patch's commit message?

>      qemu_notify_event();
>  }
>
> @@ -1815,7 +1823,8 @@ void qemu_system_shutdown_request(void)
>  {
>      trace_qemu_system_shutdown_request();
>      replay_shutdown_request();
> -    shutdown_requested = 1;
> +    /* FIXME - add a parameter to allow callers to specify reason */
> +    shutdown_requested = SHUTDOWN_CAUSE_HOST_ERROR;
>      qemu_notify_event();
>  }
>
> @@ -1846,13 +1855,16 @@ void qemu_system_debug_request(void)
>  static bool main_loop_should_exit(void)
>  {
>      RunState r;
> +    ShutdownCause request;
> +
>      if (qemu_debug_requested()) {
>          vm_stop(RUN_STATE_DEBUG);
>      }
>      if (qemu_suspend_requested()) {
>          qemu_system_suspend();
>      }
> -    if (qemu_shutdown_requested()) {
> +    request = qemu_shutdown_requested();
> +    if (request) {
>          qemu_kill_report();
>          qapi_event_send_shutdown(&error_abort);
>          if (no_shutdown) {

The detour through @request appears isn't necessary here.  Perhaps you
do it for consistency with the next hunk.  Do you?  Just asking to make
sure I get what you're doing.

Hmm, there's another one in xen-hvm.c, but consistency hardly applies
there.  If later patches add more uses, you might want delay the change
until then.

> @@ -1861,9 +1873,10 @@ static bool main_loop_should_exit(void)
>              return true;
>          }
>      }
> -    if (qemu_reset_requested()) {
> +    request = qemu_reset_requested();
> +    if (request) {
>          pause_all_vcpus();
> -        qemu_system_reset(VMRESET_REPORT);
> +        qemu_system_reset(VMRESET_REPORT, request);
>          resume_all_vcpus();
>          if (!runstate_check(RUN_STATE_RUNNING) &&
>                  !runstate_check(RUN_STATE_INMIGRATE)) {
> @@ -1872,7 +1885,7 @@ static bool main_loop_should_exit(void)
>      }
>      if (qemu_wakeup_requested()) {
>          pause_all_vcpus();
> -        qemu_system_reset(VMRESET_SILENT);
> +        qemu_system_reset(VMRESET_SILENT, SHUTDOWN_CAUSE_NONE);
>          notifier_list_notify(&wakeup_notifiers, &wakeup_reason);
>          wakeup_reason = QEMU_WAKEUP_REASON_NONE;
>          resume_all_vcpus();
> @@ -4686,7 +4699,7 @@ int main(int argc, char **argv, char **envp)
>         reading from the other reads, because timer polling functions query
>         clock values from the log. */
>      replay_checkpoint(CHECKPOINT_RESET);
> -    qemu_system_reset(VMRESET_SILENT);
> +    qemu_system_reset(VMRESET_SILENT, SHUTDOWN_CAUSE_NONE);
>      register_global_state();
>      if (replay_mode != REPLAY_MODE_NONE) {
>          replay_vmstate_init();
> diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
> index b1c05ff..b1001c1 100644
> --- a/hw/i386/xen/xen-hvm.c
> +++ b/hw/i386/xen/xen-hvm.c
> @@ -1089,11 +1089,14 @@ static void cpu_handle_ioreq(void *opaque)
>           * causes Xen to powerdown the domain.
>           */
>          if (runstate_is_running()) {
> +            ShutdownCause request;
> +
>              if (qemu_shutdown_requested_get()) {
>                  destroy_hvm_domain(false);
>              }
> -            if (qemu_reset_requested_get()) {
> -                qemu_system_reset(VMRESET_REPORT);
> +            request = qemu_reset_requested_get();
> +            if (request) {
> +                qemu_system_reset(VMRESET_REPORT, request);
>                  destroy_hvm_domain(true);
>              }
>          }
> diff --git a/migration/colo.c b/migration/colo.c
> index 963c802..bf5b7e9 100644
> --- a/migration/colo.c
> +++ b/migration/colo.c
> @@ -623,7 +623,7 @@ void *colo_process_incoming_thread(void *opaque)
>          }
>
>          qemu_mutex_lock_iothread();
> -        qemu_system_reset(VMRESET_SILENT);
> +        qemu_system_reset(VMRESET_SILENT, SHUTDOWN_CAUSE_NONE);
>          vmstate_loading = true;
>          if (qemu_loadvm_state(fb) < 0) {
>              error_report("COLO: loadvm failed");
> diff --git a/migration/savevm.c b/migration/savevm.c
> index a00c1ab..9ac2d22 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -2300,7 +2300,7 @@ int load_vmstate(const char *name)
>          return -EINVAL;
>      }
>
> -    qemu_system_reset(VMRESET_SILENT);
> +    qemu_system_reset(VMRESET_SILENT, SHUTDOWN_CAUSE_NONE);
>      mis->from_src_file = f;
>
>      aio_context_acquire(aio_context);

You seem to be passing SHUTDOWN_CAUSE_NONE exactly with VMRESET_SILENT.
Would it be possible to have SHUTDOWN_CAUSE_NONE imply !report, any
other case imply report, and get rid of the first parameter?

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Qemu-devel] [PATCH v6 2/5] shutdown: Prepare for use of an enum in reset/shutdown_request

[Eric Blake]

[-- Attachment #1.1.1: Type: text/plain, Size: 6409 bytes --]

On 05/08/2017 01:26 PM, Markus Armbruster wrote:
> Eric Blake <eblake@redhat.com> writes:
> 
>> We want to track why a guest was shutdown; in particular, being able
>> to tell the difference between a guest request (such as ACPI request)
>> and host request (such as SIGINT) will prove useful to libvirt.
>> Since all requests eventually end up changing shutdown_requested in
>> vl.c, the logical change is to make that value track the reason,
>> rather than its current 0/1 contents.
>>
>> Since command-line options control whether a reset request is turned
>> into a shutdown request instead, the same treatment is given to
>> reset_requested.
>>
>> This patch adds an internal enum ShutdownCause that describes reasons
>> that a shutdown can be requested, and changes qemu_system_reset() to
>> pass the reason through, although for now it is not reported.  The
>> enum could be exported via QAPI at a later date, if deemed necessary,
>> but for now, there has not been a request to expose that much detail
>> to end clients.
>>
>> For now, we only populate the reason with HOST_ERROR, along with FIXME
>> comments that describe our plans for how to pass an actual correct
>> reason.
> 
> In other words, replacing 0 by SHUTDOWN_CAUSE_NONE, and 1 by
> SHUTDOWN_CAUSE_HOST_ERROR.  Makes sense.

Maybe I could have ordered HOST_ERROR to actually be 1...


>> +/* Enumeration of various causes for shutdown. */
>> +typedef enum ShutdownCause ShutdownCause;
>> +enum ShutdownCause {
> 
> Why define the typedef separately here?  What's wrong with
> 
>     typedef enum ShutdownCause {
>         ...
>     } ShutdownCause;
> 
> ?

That would work too.  I don't know if the code base has a strong
preference for one form over the other.

> 
>> +    SHUTDOWN_CAUSE_NONE,          /* No shutdown requested yet */
> 
> Comment is fine.  Possible alternative: /* No shutdown request pending */
> 
>> +    SHUTDOWN_CAUSE_HOST_QMP,      /* Reaction to a QMP command, like 'quit' */
>> +    SHUTDOWN_CAUSE_HOST_SIGNAL,   /* Reaction to a signal, such as SIGINT */
>> +    SHUTDOWN_CAUSE_HOST_UI,       /* Reaction to UI event, like window close */
>> +    SHUTDOWN_CAUSE_HOST_ERROR,    /* An error prevents further use of guest */

...rather than 4.  I don't know that it matters much.


>> -static int qemu_reset_requested(void)
>> +static ShutdownCause qemu_reset_requested(void)
>>  {
>> -    int r = reset_requested;
>> +    ShutdownCause r = reset_requested;
> 
> Good opportunity to insert a blank line here.
> 

Sure.

>>      if (r && replay_checkpoint(CHECKPOINT_RESET_REQUESTED)) {
>> -        reset_requested = 0;
>> +        reset_requested = SHUTDOWN_CAUSE_NONE;
>>          return r;
>>      }
>> -    return false;
>> +    return SHUTDOWN_CAUSE_NONE;
>>  }
>>
>>  static int qemu_suspend_requested(void)
>> @@ -1686,7 +1687,12 @@ static int qemu_debug_requested(void)
>>      return r;
>>  }
>>
>> -void qemu_system_reset(bool report)
>> +/*
>> + * Reset the VM. If @report is VMRESET_REPORT, issue an event, using
>> + * the @reason interpreted as ShutdownCause for details.  Otherwise,
>> + * @report is VMRESET_SILENT and @reason is ignored.
>> + */
> 
> "interpreted as ShutdownCause"?  It *is* a ShutdownCause.  Leftover?

Oh, yeah. In v5, the parameter was 'int'.

> 
>> +void qemu_system_reset(bool report, ShutdownCause reason)
>>  {
>>      MachineClass *mc;
>>
>> @@ -1700,6 +1706,7 @@ void qemu_system_reset(bool report)
>>          qemu_devices_reset();
>>      }
>>      if (report) {
>> +        assert(reason);
>>          qapi_event_send_reset(&error_abort);
>>      }
>>      cpu_synchronize_all_post_reset();
> 
> Looks like we're not using @reason "for details" just yet.

Correct. I can add a FIXME (to be removed in the later patch where it is
used) if that is desired.


>> @@ -1807,7 +1815,7 @@ void qemu_system_killed(int signal, pid_t pid)
>>      /* Cannot call qemu_system_shutdown_request directly because
>>       * we are in a signal handler.
>>       */
>> -    shutdown_requested = 1;
>> +    shutdown_requested = SHUTDOWN_CAUSE_HOST_SIGNAL;
> 
> Should this be SHUTDOWN_CAUSE_HOST_ERROR, to be updated in the next
> patch?  Alternatively, tweak this patch's commit message?

This is the one case that we actually do have a strong cause affiliated
with the reason without having to resort to changing function
signatures.  Commit message tweak is better.

>> @@ -1846,13 +1855,16 @@ void qemu_system_debug_request(void)
>>  static bool main_loop_should_exit(void)
>>  {
>>      RunState r;
>> +    ShutdownCause request;
>> +
>>      if (qemu_debug_requested()) {
>>          vm_stop(RUN_STATE_DEBUG);
>>      }
>>      if (qemu_suspend_requested()) {
>>          qemu_system_suspend();
>>      }
>> -    if (qemu_shutdown_requested()) {
>> +    request = qemu_shutdown_requested();
>> +    if (request) {
>>          qemu_kill_report();
>>          qapi_event_send_shutdown(&error_abort);
>>          if (no_shutdown) {
> 
> The detour through @request appears isn't necessary here.  Perhaps you
> do it for consistency with the next hunk.  Do you?  Just asking to make
> sure I get what you're doing.

Consistency with the next hunk, AND because a later patch then uses
'request' to pass an additional parameter to qapi_event_send_shutdown().

> 
> Hmm, there's another one in xen-hvm.c, but consistency hardly applies
> there.  If later patches add more uses, you might want delay the change
> until then.

Can do, if it makes incremental reviews easier.


>> +++ b/migration/savevm.c
>> @@ -2300,7 +2300,7 @@ int load_vmstate(const char *name)
>>          return -EINVAL;
>>      }
>>
>> -    qemu_system_reset(VMRESET_SILENT);
>> +    qemu_system_reset(VMRESET_SILENT, SHUTDOWN_CAUSE_NONE);
>>      mis->from_src_file = f;
>>
>>      aio_context_acquire(aio_context);
> 
> You seem to be passing SHUTDOWN_CAUSE_NONE exactly with VMRESET_SILENT.
> Would it be possible to have SHUTDOWN_CAUSE_NONE imply !report, any
> other case imply report, and get rid of the first parameter?

Indeed, and it would also get rid of the ugly
 #define VMRESET_SILENT false

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Qemu-devel] [PATCH v6 2/5] shutdown: Prepare for use of an enum in reset/shutdown_request

[Markus Armbruster]

Eric Blake <eblake@redhat.com> writes:

> On 05/08/2017 01:26 PM, Markus Armbruster wrote:
>> Eric Blake <eblake@redhat.com> writes:
>> 
>>> We want to track why a guest was shutdown; in particular, being able
>>> to tell the difference between a guest request (such as ACPI request)
>>> and host request (such as SIGINT) will prove useful to libvirt.
>>> Since all requests eventually end up changing shutdown_requested in
>>> vl.c, the logical change is to make that value track the reason,
>>> rather than its current 0/1 contents.
>>>
>>> Since command-line options control whether a reset request is turned
>>> into a shutdown request instead, the same treatment is given to
>>> reset_requested.
>>>
>>> This patch adds an internal enum ShutdownCause that describes reasons
>>> that a shutdown can be requested, and changes qemu_system_reset() to
>>> pass the reason through, although for now it is not reported.  The
>>> enum could be exported via QAPI at a later date, if deemed necessary,
>>> but for now, there has not been a request to expose that much detail
>>> to end clients.
>>>
>>> For now, we only populate the reason with HOST_ERROR, along with FIXME
>>> comments that describe our plans for how to pass an actual correct
>>> reason.
>> 
>> In other words, replacing 0 by SHUTDOWN_CAUSE_NONE, and 1 by
>> SHUTDOWN_CAUSE_HOST_ERROR.  Makes sense.
>
> Maybe I could have ordered HOST_ERROR to actually be 1...

Might be marginally worthwhile if you can split patches so that the one
replacing int by ShutdownCause doesn't change anything but names.

>>> +/* Enumeration of various causes for shutdown. */
>>> +typedef enum ShutdownCause ShutdownCause;
>>> +enum ShutdownCause {
>> 
>> Why define the typedef separately here?  What's wrong with
>> 
>>     typedef enum ShutdownCause {
>>         ...
>>     } ShutdownCause;
>> 
>> ?
>
> That would work too.  I don't know if the code base has a strong
> preference for one form over the other.

I don't have numbers, but I think we use the split form pretty much only
when there's a reason for the split, such as defining an incomplete type
in a header, and completing it elsewhere.

>>> +    SHUTDOWN_CAUSE_NONE,          /* No shutdown requested yet */
>> 
>> Comment is fine.  Possible alternative: /* No shutdown request pending */
>> 
>>> +    SHUTDOWN_CAUSE_HOST_QMP,      /* Reaction to a QMP command, like 'quit' */
>>> +    SHUTDOWN_CAUSE_HOST_SIGNAL,   /* Reaction to a signal, such as SIGINT */
>>> +    SHUTDOWN_CAUSE_HOST_UI,       /* Reaction to UI event, like window close */
>>> +    SHUTDOWN_CAUSE_HOST_ERROR,    /* An error prevents further use of guest */
>
> ...rather than 4.  I don't know that it matters much.
>
>
>>> -static int qemu_reset_requested(void)
>>> +static ShutdownCause qemu_reset_requested(void)
>>>  {
>>> -    int r = reset_requested;
>>> +    ShutdownCause r = reset_requested;
>> 
>> Good opportunity to insert a blank line here.
>> 
>
> Sure.
>
>>>      if (r && replay_checkpoint(CHECKPOINT_RESET_REQUESTED)) {
>>> -        reset_requested = 0;
>>> +        reset_requested = SHUTDOWN_CAUSE_NONE;
>>>          return r;
>>>      }
>>> -    return false;
>>> +    return SHUTDOWN_CAUSE_NONE;
>>>  }
>>>
>>>  static int qemu_suspend_requested(void)
>>> @@ -1686,7 +1687,12 @@ static int qemu_debug_requested(void)
>>>      return r;
>>>  }
>>>
>>> -void qemu_system_reset(bool report)
>>> +/*
>>> + * Reset the VM. If @report is VMRESET_REPORT, issue an event, using
>>> + * the @reason interpreted as ShutdownCause for details.  Otherwise,
>>> + * @report is VMRESET_SILENT and @reason is ignored.
>>> + */
>> 
>> "interpreted as ShutdownCause"?  It *is* a ShutdownCause.  Leftover?
>
> Oh, yeah. In v5, the parameter was 'int'.

Easy enough to clean up :)

>>> +void qemu_system_reset(bool report, ShutdownCause reason)
>>>  {
>>>      MachineClass *mc;
>>>
>>> @@ -1700,6 +1706,7 @@ void qemu_system_reset(bool report)
>>>          qemu_devices_reset();
>>>      }
>>>      if (report) {
>>> +        assert(reason);
>>>          qapi_event_send_reset(&error_abort);
>>>      }
>>>      cpu_synchronize_all_post_reset();
>> 
>> Looks like we're not using @reason "for details" just yet.
>
> Correct. I can add a FIXME (to be removed in the later patch where it is
> used) if that is desired.

Not necessary if the function comment refrains from claiming it *is*
used.

>>> @@ -1807,7 +1815,7 @@ void qemu_system_killed(int signal, pid_t pid)
>>>      /* Cannot call qemu_system_shutdown_request directly because
>>>       * we are in a signal handler.
>>>       */
>>> -    shutdown_requested = 1;
>>> +    shutdown_requested = SHUTDOWN_CAUSE_HOST_SIGNAL;
>> 
>> Should this be SHUTDOWN_CAUSE_HOST_ERROR, to be updated in the next
>> patch?  Alternatively, tweak this patch's commit message?
>
> This is the one case that we actually do have a strong cause affiliated
> with the reason without having to resort to changing function
> signatures.  Commit message tweak is better.

Works for me.

>>> @@ -1846,13 +1855,16 @@ void qemu_system_debug_request(void)
>>>  static bool main_loop_should_exit(void)
>>>  {
>>>      RunState r;
>>> +    ShutdownCause request;
>>> +
>>>      if (qemu_debug_requested()) {
>>>          vm_stop(RUN_STATE_DEBUG);
>>>      }
>>>      if (qemu_suspend_requested()) {
>>>          qemu_system_suspend();
>>>      }
>>> -    if (qemu_shutdown_requested()) {
>>> +    request = qemu_shutdown_requested();
>>> +    if (request) {
>>>          qemu_kill_report();
>>>          qapi_event_send_shutdown(&error_abort);
>>>          if (no_shutdown) {
>> 
>> The detour through @request appears isn't necessary here.  Perhaps you
>> do it for consistency with the next hunk.  Do you?  Just asking to make
>> sure I get what you're doing.
>
> Consistency with the next hunk, AND because a later patch then uses
> 'request' to pass an additional parameter to qapi_event_send_shutdown().
>
>> 
>> Hmm, there's another one in xen-hvm.c, but consistency hardly applies
>> there.  If later patches add more uses, you might want delay the change
>> until then.
>
> Can do, if it makes incremental reviews easier.

Use your judgement.

>>> +++ b/migration/savevm.c
>>> @@ -2300,7 +2300,7 @@ int load_vmstate(const char *name)
>>>          return -EINVAL;
>>>      }
>>>
>>> -    qemu_system_reset(VMRESET_SILENT);
>>> +    qemu_system_reset(VMRESET_SILENT, SHUTDOWN_CAUSE_NONE);
>>>      mis->from_src_file = f;
>>>
>>>      aio_context_acquire(aio_context);
>> 
>> You seem to be passing SHUTDOWN_CAUSE_NONE exactly with VMRESET_SILENT.
>> Would it be possible to have SHUTDOWN_CAUSE_NONE imply !report, any
>> other case imply report, and get rid of the first parameter?
>
> Indeed, and it would also get rid of the ugly
>  #define VMRESET_SILENT false

I'd love that.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH v6 3/5] shutdown: Add source information to SHUTDOWN and RESET

[Eric Blake]

Time to wire up all the call sites that request a shutdown or
reset to use the enum added in the previous patch.

It would have been less churn to keep the common case with no
arguments as meaning guest-triggered, and only modified the
host-triggered code paths, via a wrapper function, but then we'd
still have to audit that I didn't miss any host-triggered spots;
changing the signature forces us to double-check that I correctly
categorized all callers.

Since command line options can change whether a guest reset request
causes an actual reset vs. a shutdown, it's easy to also add the
information to reset requests.

Replay adds a FIXME to preserve the cause across the replay stream,
that will be tackled in the next patch.

Signed-off-by: Eric Blake <eblake@redhat.com>
Acked-by: David Gibson <david@gibson.dropbear.id.au> [ppc parts]
Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> [SPARC part]

---
v6: defer event additions to later, add reviews of unchanged portions
v5: drop accidental addition of unrelated files
v4: s/ShutdownType/ShutdownCause/, no thanks to mingw header pollution
v3: retitle again, fix qemu-iotests, use enum rather than raw bool
in all callers
v2: retitle (was "event: Add signal information to SHUTDOWN"),
completely rework to post bool based on whether it is guest-initiated
v1: initial submission, exposing just Unix signals from host
---
 include/sysemu/sysemu.h     |  4 ++--
 vl.c                        | 17 ++++++++---------
 hw/acpi/core.c              |  4 ++--
 hw/arm/highbank.c           |  4 ++--
 hw/arm/integratorcp.c       |  2 +-
 hw/arm/musicpal.c           |  2 +-
 hw/arm/omap1.c              | 10 ++++++----
 hw/arm/omap2.c              |  2 +-
 hw/arm/spitz.c              |  2 +-
 hw/arm/stellaris.c          |  2 +-
 hw/arm/tosa.c               |  2 +-
 hw/i386/pc.c                |  2 +-
 hw/i386/xen/xen-hvm.c       |  2 +-
 hw/input/pckbd.c            |  4 ++--
 hw/ipmi/ipmi.c              |  4 ++--
 hw/isa/lpc_ich9.c           |  2 +-
 hw/mips/boston.c            |  2 +-
 hw/mips/mips_malta.c        |  2 +-
 hw/mips/mips_r4k.c          |  4 ++--
 hw/misc/arm_sysctl.c        |  8 ++++----
 hw/misc/cbus.c              |  2 +-
 hw/misc/macio/cuda.c        |  4 ++--
 hw/misc/slavio_misc.c       |  4 ++--
 hw/misc/zynq_slcr.c         |  2 +-
 hw/pci-host/apb.c           |  4 ++--
 hw/pci-host/bonito.c        |  2 +-
 hw/pci-host/piix.c          |  2 +-
 hw/ppc/e500.c               |  2 +-
 hw/ppc/mpc8544_guts.c       |  2 +-
 hw/ppc/ppc.c                |  2 +-
 hw/ppc/ppc405_uc.c          |  2 +-
 hw/ppc/spapr_hcall.c        |  2 +-
 hw/ppc/spapr_rtas.c         |  4 ++--
 hw/s390x/ipl.c              |  2 +-
 hw/sh4/r2d.c                |  2 +-
 hw/timer/etraxfs_timer.c    |  2 +-
 hw/timer/m48t59.c           |  4 ++--
 hw/timer/milkymist-sysctl.c |  4 ++--
 hw/timer/pxa2xx_timer.c     |  2 +-
 hw/watchdog/watchdog.c      |  2 +-
 hw/xenpv/xen_domainbuild.c  |  2 +-
 hw/xtensa/xtfpga.c          |  2 +-
 kvm-all.c                   |  6 +++---
 os-win32.c                  |  2 +-
 qmp.c                       |  4 ++--
 replay/replay.c             |  3 ++-
 target/alpha/sys_helper.c   |  4 ++--
 target/arm/psci.c           |  4 ++--
 target/i386/excp_helper.c   |  2 +-
 target/i386/hax-all.c       |  6 +++---
 target/i386/helper.c        |  2 +-
 target/i386/kvm.c           |  2 +-
 target/s390x/helper.c       |  2 +-
 target/s390x/kvm.c          |  4 ++--
 target/s390x/misc_helper.c  |  4 ++--
 target/sparc/int32_helper.c |  2 +-
 ui/sdl.c                    |  2 +-
 ui/sdl2.c                   |  4 ++--
 trace-events                |  2 +-
 ui/cocoa.m                  |  2 +-
 60 files changed, 98 insertions(+), 96 deletions(-)

diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index e4da9d4..89d0e3e 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -65,13 +65,13 @@ typedef enum WakeupReason {
     QEMU_WAKEUP_REASON_OTHER,
 } WakeupReason;

-void qemu_system_reset_request(void);
+void qemu_system_reset_request(ShutdownCause reason);
 void qemu_system_suspend_request(void);
 void qemu_register_suspend_notifier(Notifier *notifier);
 void qemu_system_wakeup_request(WakeupReason reason);
 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_shutdown_request(ShutdownCause reason);
 void qemu_system_powerdown_request(void);
 void qemu_register_powerdown_notifier(Notifier *notifier);
 void qemu_system_debug_request(void);
diff --git a/vl.c b/vl.c
index 6069fb2..9579d5f 100644
--- a/vl.c
+++ b/vl.c
@@ -1725,7 +1725,7 @@ void qemu_system_guest_panicked(GuestPanicInformation *info)
     if (!no_shutdown) {
         qapi_event_send_guest_panicked(GUEST_PANIC_ACTION_POWEROFF,
                                        !!info, info, &error_abort);
-        qemu_system_shutdown_request();
+        qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_PANIC);
     }

     if (info) {
@@ -1742,13 +1742,12 @@ void qemu_system_guest_panicked(GuestPanicInformation *info)
     }
 }

-void qemu_system_reset_request(void)
+void qemu_system_reset_request(ShutdownCause reason)
 {
     if (no_reboot) {
-        /* FIXME - add a parameter to allow callers to specify reason */
-        shutdown_requested = SHUTDOWN_CAUSE_HOST_ERROR;
+        shutdown_requested = reason;
     } else {
-        reset_requested = SHUTDOWN_CAUSE_HOST_ERROR;
+        reset_requested = reason;
     }
     cpu_stop_current();
     qemu_notify_event();
@@ -1819,12 +1818,12 @@ void qemu_system_killed(int signal, pid_t pid)
     qemu_notify_event();
 }

-void qemu_system_shutdown_request(void)
+void qemu_system_shutdown_request(ShutdownCause reason)
 {
-    trace_qemu_system_shutdown_request();
+    trace_qemu_system_shutdown_request(reason);
+    /* FIXME - add a parameter to let replay preserve reason */
     replay_shutdown_request();
-    /* FIXME - add a parameter to allow callers to specify reason */
-    shutdown_requested = SHUTDOWN_CAUSE_HOST_ERROR;
+    shutdown_requested = reason;
     qemu_notify_event();
 }

diff --git a/hw/acpi/core.c b/hw/acpi/core.c
index e890a5d..95fcac9 100644
--- a/hw/acpi/core.c
+++ b/hw/acpi/core.c
@@ -561,7 +561,7 @@ static void acpi_pm1_cnt_write(ACPIREGS *ar, uint16_t val)
         uint16_t sus_typ = (val >> 10) & 7;
         switch(sus_typ) {
         case 0: /* soft power off */
-            qemu_system_shutdown_request();
+            qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
             break;
         case 1:
             qemu_system_suspend_request();
@@ -569,7 +569,7 @@ static void acpi_pm1_cnt_write(ACPIREGS *ar, uint16_t val)
         default:
             if (sus_typ == ar->pm1.cnt.s4_val) { /* S4 request */
                 qapi_event_send_suspend_disk(&error_abort);
-                qemu_system_shutdown_request();
+                qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
             }
             break;
         }
diff --git a/hw/arm/highbank.c b/hw/arm/highbank.c
index 0a4508c..d209b97 100644
--- a/hw/arm/highbank.c
+++ b/hw/arm/highbank.c
@@ -108,9 +108,9 @@ static void hb_regs_write(void *opaque, hwaddr offset,

     if (offset == 0xf00) {
         if (value == 1 || value == 2) {
-            qemu_system_reset_request();
+            qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
         } else if (value == 3) {
-            qemu_system_shutdown_request();
+            qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
         }
     }

diff --git a/hw/arm/integratorcp.c b/hw/arm/integratorcp.c
index 5610ffc..ca3eca1 100644
--- a/hw/arm/integratorcp.c
+++ b/hw/arm/integratorcp.c
@@ -158,7 +158,7 @@ static void integratorcm_do_remap(IntegratorCMState *s)
 static void integratorcm_set_ctrl(IntegratorCMState *s, uint32_t value)
 {
     if (value & 8) {
-        qemu_system_reset_request();
+        qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
     }
     if ((s->cm_ctrl ^ value) & 1) {
         /* (value & 1) != 0 means the green "MISC LED" is lit.
diff --git a/hw/arm/musicpal.c b/hw/arm/musicpal.c
index cbbca4e..9c710f7 100644
--- a/hw/arm/musicpal.c
+++ b/hw/arm/musicpal.c
@@ -898,7 +898,7 @@ static void mv88w8618_pit_write(void *opaque, hwaddr offset,

     case MP_BOARD_RESET:
         if (value == MP_BOARD_RESET_MAGIC) {
-            qemu_system_reset_request();
+            qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
         }
         break;
     }
diff --git a/hw/arm/omap1.c b/hw/arm/omap1.c
index b3cf0ec..54582bd 100644
--- a/hw/arm/omap1.c
+++ b/hw/arm/omap1.c
@@ -355,7 +355,7 @@ static void omap_wd_timer_write(void *opaque, hwaddr addr,
                 /* XXX: on T|E hardware somehow this has no effect,
                  * on Zire 71 it works as specified.  */
                 s->reset = 1;
-                qemu_system_reset_request();
+                qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
             }
         }
         s->last_wr = value & 0xff;
@@ -1545,8 +1545,10 @@ static inline void omap_clkm_idlect1_update(struct omap_mpu_state_s *s,
     if (value & (1 << 11)) {                            /* SETARM_IDLE */
         cpu_interrupt(CPU(s->cpu), CPU_INTERRUPT_HALT);
     }
-    if (!(value & (1 << 10)))				/* WKUP_MODE */
-        qemu_system_shutdown_request();	/* XXX: disable wakeup from IRQ */
+    if (!(value & (1 << 10))) {                         /* WKUP_MODE */
+        /* XXX: disable wakeup from IRQ */
+        qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
+    }

 #define SET_CANIDLE(clock, bit)				\
     if (diff & (1 << bit)) {				\
@@ -1693,7 +1695,7 @@ static void omap_clkm_write(void *opaque, hwaddr addr,
         diff = s->clkm.arm_rstct1 ^ value;
         s->clkm.arm_rstct1 = value & 0x0007;
         if (value & 9) {
-            qemu_system_reset_request();
+            qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
             s->clkm.cold_start = 0xa;
         }
         if (diff & ~value & 4) {				/* DSP_RST */
diff --git a/hw/arm/omap2.c b/hw/arm/omap2.c
index cf1b4ba..8afb854 100644
--- a/hw/arm/omap2.c
+++ b/hw/arm/omap2.c
@@ -1610,7 +1610,7 @@ static void omap_prcm_write(void *opaque, hwaddr addr,
     case 0x450:	/* RM_RSTCTRL_WKUP */
         /* TODO: reset */
         if (value & 2)
-            qemu_system_reset_request();
+            qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
         break;
     case 0x454:	/* RM_RSTTIME_WKUP */
         s->rsttime_wkup = value & 0x1fff;
diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c
index fe2d5a7..7e9f579 100644
--- a/hw/arm/spitz.c
+++ b/hw/arm/spitz.c
@@ -848,7 +848,7 @@ static void spitz_lcd_hsync_handler(void *opaque, int line, int level)
 static void spitz_reset(void *opaque, int line, int level)
 {
     if (level) {
-        qemu_system_reset_request();
+        qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
     }
 }

diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c
index ea7a809..cf6e7be 100644
--- a/hw/arm/stellaris.c
+++ b/hw/arm/stellaris.c
@@ -1197,7 +1197,7 @@ static
 void do_sys_reset(void *opaque, int n, int level)
 {
     if (level) {
-        qemu_system_reset_request();
+        qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
     }
 }

diff --git a/hw/arm/tosa.c b/hw/arm/tosa.c
index 9f58a23..2421b81 100644
--- a/hw/arm/tosa.c
+++ b/hw/arm/tosa.c
@@ -90,7 +90,7 @@ static void tosa_out_switch(void *opaque, int line, int level)
 static void tosa_reset(void *opaque, int line, int level)
 {
     if (level) {
-        qemu_system_reset_request();
+        qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
     }
 }

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index f3b372a..427f123 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -519,7 +519,7 @@ static void port92_write(void *opaque, hwaddr addr, uint64_t val,
     s->outport = val;
     qemu_set_irq(s->a20_out, (val >> 1) & 1);
     if ((val & 1) && !(oldval & 1)) {
-        qemu_system_reset_request();
+        qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
     }
 }

diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
index b1001c1..a8c3c1c 100644
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -1398,7 +1398,7 @@ void xen_shutdown_fatal_error(const char *fmt, ...)
     va_end(ap);
     fprintf(stderr, "Will destroy the domain.\n");
     /* destroy the domain */
-    qemu_system_shutdown_request();
+    qemu_system_shutdown_request(SHUTDOWN_CAUSE_HOST_ERROR);
 }

 void xen_hvm_modified_memory(ram_addr_t start, ram_addr_t length)
diff --git a/hw/input/pckbd.c b/hw/input/pckbd.c
index d414288..c479f82 100644
--- a/hw/input/pckbd.c
+++ b/hw/input/pckbd.c
@@ -226,7 +226,7 @@ static void outport_write(KBDState *s, uint32_t val)
     s->outport = val;
     qemu_set_irq(s->a20_out, (val >> 1) & 1);
     if (!(val & 1)) {
-        qemu_system_reset_request();
+        qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
     }
 }

@@ -301,7 +301,7 @@ static void kbd_write_command(void *opaque, hwaddr addr,
         s->outport &= ~KBD_OUT_A20;
         break;
     case KBD_CCMD_RESET:
-        qemu_system_reset_request();
+        qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
         break;
     case KBD_CCMD_NO_OP:
         /* ignore that */
diff --git a/hw/ipmi/ipmi.c b/hw/ipmi/ipmi.c
index 5cf1caa..afafe14 100644
--- a/hw/ipmi/ipmi.c
+++ b/hw/ipmi/ipmi.c
@@ -44,14 +44,14 @@ static int ipmi_do_hw_op(IPMIInterface *s, enum ipmi_op op, int checkonly)
         if (checkonly) {
             return 0;
         }
-        qemu_system_reset_request();
+        qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
         return 0;

     case IPMI_POWEROFF_CHASSIS:
         if (checkonly) {
             return 0;
         }
-        qemu_system_shutdown_request();
+        qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
         return 0;

     case IPMI_SEND_NMI:
diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
index a0866c3..2b09354 100644
--- a/hw/isa/lpc_ich9.c
+++ b/hw/isa/lpc_ich9.c
@@ -606,7 +606,7 @@ static void ich9_rst_cnt_write(void *opaque, hwaddr addr, uint64_t val,
     ICH9LPCState *lpc = opaque;

     if (val & 4) {
-        qemu_system_reset_request();
+        qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
         return;
     }
     lpc->rst_cnt = val & 0xA; /* keep FULL_RST (bit 3) and SYS_RST (bit 1) */
diff --git a/hw/mips/boston.c b/hw/mips/boston.c
index 83f7b82..53d1e0c 100644
--- a/hw/mips/boston.c
+++ b/hw/mips/boston.c
@@ -232,7 +232,7 @@ static void boston_platreg_write(void *opaque, hwaddr addr,
         break;
     case PLAT_SOFTRST_CTL:
         if (val & PLAT_SOFTRST_CTL_SYSRESET) {
-            qemu_system_reset_request();
+            qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
         }
         break;
     default:
diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
index 5dd177e..7814c39 100644
--- a/hw/mips/mips_malta.c
+++ b/hw/mips/mips_malta.c
@@ -470,7 +470,7 @@ static void malta_fpga_write(void *opaque, hwaddr addr,
     /* SOFTRES Register */
     case 0x00500:
         if (val == 0x42)
-            qemu_system_reset_request ();
+            qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
         break;

     /* BRKRES Register */
diff --git a/hw/mips/mips_r4k.c b/hw/mips/mips_r4k.c
index 748586e..f4de9fc 100644
--- a/hw/mips/mips_r4k.c
+++ b/hw/mips/mips_r4k.c
@@ -53,9 +53,9 @@ static void mips_qemu_write (void *opaque, hwaddr addr,
                              uint64_t val, unsigned size)
 {
     if ((addr & 0xffff) == 0 && val == 42)
-        qemu_system_reset_request ();
+        qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
     else if ((addr & 0xffff) == 4 && val == 42)
-        qemu_system_shutdown_request ();
+        qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
 }

 static uint64_t mips_qemu_read (void *opaque, hwaddr addr,
diff --git a/hw/misc/arm_sysctl.c b/hw/misc/arm_sysctl.c
index 8524008..b20b44e 100644
--- a/hw/misc/arm_sysctl.c
+++ b/hw/misc/arm_sysctl.c
@@ -351,13 +351,13 @@ static bool vexpress_cfgctrl_write(arm_sysctl_state *s, unsigned int dcc,
         break;
     case SYS_CFG_SHUTDOWN:
         if (site == SYS_CFG_SITE_MB && device == 0) {
-            qemu_system_shutdown_request();
+            qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
             return true;
         }
         break;
     case SYS_CFG_REBOOT:
         if (site == SYS_CFG_SITE_MB && device == 0) {
-            qemu_system_reset_request();
+            qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
             return true;
         }
         break;
@@ -429,7 +429,7 @@ static void arm_sysctl_write(void *opaque, hwaddr offset,
             if (s->lockval == LOCK_VALUE) {
                 s->resetlevel = val;
                 if (val & 0x100) {
-                    qemu_system_reset_request();
+                    qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
                 }
             }
             break;
@@ -438,7 +438,7 @@ static void arm_sysctl_write(void *opaque, hwaddr offset,
             if (s->lockval == LOCK_VALUE) {
                 s->resetlevel = val;
                 if (val & 0x04) {
-                    qemu_system_reset_request();
+                    qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
                 }
             }
             break;
diff --git a/hw/misc/cbus.c b/hw/misc/cbus.c
index 0c207e3..677274c 100644
--- a/hw/misc/cbus.c
+++ b/hw/misc/cbus.c
@@ -356,7 +356,7 @@ static inline void retu_write(CBusRetu *s, int reg, uint16_t val)

     case RETU_REG_WATCHDOG:
         if (val == 0 && (s->cc[0] & 2))
-            qemu_system_shutdown_request();
+            qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
         break;

     case RETU_REG_TXCR:
diff --git a/hw/misc/macio/cuda.c b/hw/misc/macio/cuda.c
index 05c02fb..008d8bd 100644
--- a/hw/misc/macio/cuda.c
+++ b/hw/misc/macio/cuda.c
@@ -612,7 +612,7 @@ static bool cuda_cmd_powerdown(CUDAState *s,
         return false;
     }

-    qemu_system_shutdown_request();
+    qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
     return true;
 }

@@ -624,7 +624,7 @@ static bool cuda_cmd_reset_system(CUDAState *s,
         return false;
     }

-    qemu_system_reset_request();
+    qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
     return true;
 }

diff --git a/hw/misc/slavio_misc.c b/hw/misc/slavio_misc.c
index edd5de0..18ff677 100644
--- a/hw/misc/slavio_misc.c
+++ b/hw/misc/slavio_misc.c
@@ -258,7 +258,7 @@ static void slavio_aux2_mem_writeb(void *opaque, hwaddr addr,
         val &= AUX2_PWROFF;
     s->aux2 = val;
     if (val & AUX2_PWROFF)
-        qemu_system_shutdown_request();
+        qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
     slavio_misc_update_irq(s);
 }

@@ -338,7 +338,7 @@ static void slavio_sysctrl_mem_writel(void *opaque, hwaddr addr,
     case 0:
         if (val & SYS_RESET) {
             s->sysctrl = SYS_RESETSTAT;
-            qemu_system_reset_request();
+            qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
         }
         break;
     default:
diff --git a/hw/misc/zynq_slcr.c b/hw/misc/zynq_slcr.c
index 7891219..44304d4 100644
--- a/hw/misc/zynq_slcr.c
+++ b/hw/misc/zynq_slcr.c
@@ -405,7 +405,7 @@ static void zynq_slcr_write(void *opaque, hwaddr offset,
     switch (offset) {
     case PSS_RST_CTRL:
         if (val & R_PSS_RST_CTRL_SOFT_RST) {
-            qemu_system_reset_request();
+            qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
         }
         break;
     }
diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c
index 653e711..83fc6a9 100644
--- a/hw/pci-host/apb.c
+++ b/hw/pci-host/apb.c
@@ -482,9 +482,9 @@ static void apb_config_writel (void *opaque, hwaddr addr,
             s->reset_control |= val & RESET_WMASK;
             if (val & SOFT_POR) {
                 s->nr_resets = 0;
-                qemu_system_reset_request();
+                qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
             } else if (val & SOFT_XIR) {
-                qemu_system_reset_request();
+                qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
             }
         }
         break;
diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c
index 1999ece..b1d41d0 100644
--- a/hw/pci-host/bonito.c
+++ b/hw/pci-host/bonito.c
@@ -269,7 +269,7 @@ static void bonito_writel(void *opaque, hwaddr addr,
         }
         s->regs[saddr] = val;
         if (reset) {
-            qemu_system_reset_request();
+            qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
         }
         break;
     case BONITO_INTENSET:
diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index f9218aa..c1d6a85 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -638,7 +638,7 @@ static void rcr_write(void *opaque, hwaddr addr, uint64_t val, unsigned len)
     PIIX3State *d = opaque;

     if (val & 4) {
-        qemu_system_reset_request();
+        qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
         return;
     }
     d->rcr = val & 2; /* keep System Reset type only */
diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index f7df238..62f1857 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -774,7 +774,7 @@ static qemu_irq *ppce500_init_mpic(MachineState *machine, PPCE500Params *params,
 static void ppce500_power_off(void *opaque, int line, int on)
 {
     if (on) {
-        qemu_system_shutdown_request();
+        qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
     }
 }

diff --git a/hw/ppc/mpc8544_guts.c b/hw/ppc/mpc8544_guts.c
index ba69178..ce1254b 100644
--- a/hw/ppc/mpc8544_guts.c
+++ b/hw/ppc/mpc8544_guts.c
@@ -98,7 +98,7 @@ static void mpc8544_guts_write(void *opaque, hwaddr addr,
     switch (addr) {
     case MPC8544_GUTS_ADDR_RSTCR:
         if (value & MPC8544_GUTS_RSTCR_RESET) {
-            qemu_system_reset_request();
+            qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
         }
         break;
     default:
diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
index 5f93083..224184d 100644
--- a/hw/ppc/ppc.c
+++ b/hw/ppc/ppc.c
@@ -412,7 +412,7 @@ static void ppce500_set_irq(void *opaque, int pin, int level)
             if (level) {
                 LOG_IRQ("%s: reset the PowerPC system\n",
                             __func__);
-                qemu_system_reset_request();
+                qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
             }
             break;
         case PPCE500_INPUT_RESET_CORE:
diff --git a/hw/ppc/ppc405_uc.c b/hw/ppc/ppc405_uc.c
index d5df94a..fc32e96 100644
--- a/hw/ppc/ppc405_uc.c
+++ b/hw/ppc/ppc405_uc.c
@@ -1807,7 +1807,7 @@ void ppc40x_chip_reset(PowerPCCPU *cpu)
 void ppc40x_system_reset(PowerPCCPU *cpu)
 {
     printf("Reset PowerPC system\n");
-    qemu_system_reset_request();
+    qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
 }

 void store_40x_dbcr0 (CPUPPCState *env, uint32_t val)
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 9f18f75..2735fe9 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -1166,7 +1166,7 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
     spapr_ovec_cleanup(ov5_updates);

     if (spapr->cas_reboot) {
-        qemu_system_reset_request();
+        qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
     } else {
         /* If ppc_spapr_reset() did not set up a HPT but one is necessary
          * (because the guest isn't going to use radix) then set it up here. */
diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index 619f32c..128d993 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -110,7 +110,7 @@ static void rtas_power_off(PowerPCCPU *cpu, sPAPRMachineState *spapr,
         rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
         return;
     }
-    qemu_system_shutdown_request();
+    qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
     cpu_stop_current();
     rtas_st(rets, 0, RTAS_OUT_SUCCESS);
 }
@@ -124,7 +124,7 @@ static void rtas_system_reboot(PowerPCCPU *cpu, sPAPRMachineState *spapr,
         rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
         return;
     }
-    qemu_system_reset_request();
+    qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
     rtas_st(rets, 0, RTAS_OUT_SUCCESS);
 }

diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
index 7978c7d..e5ab0ad 100644
--- a/hw/s390x/ipl.c
+++ b/hw/s390x/ipl.c
@@ -363,7 +363,7 @@ void s390_reipl_request(void)
     S390IPLState *ipl = get_ipl_device();

     ipl->reipl_requested = true;
-    qemu_system_reset_request();
+    qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
 }

 void s390_ipl_prepare_cpu(S390CPU *cpu)
diff --git a/hw/sh4/r2d.c b/hw/sh4/r2d.c
index 8f520ce..e6fc74e 100644
--- a/hw/sh4/r2d.c
+++ b/hw/sh4/r2d.c
@@ -164,7 +164,7 @@ r2d_fpga_write(void *opaque, hwaddr addr, uint64_t value, unsigned int size)
 	break;
     case PA_POWOFF:
         if (value & 1) {
-            qemu_system_shutdown_request();
+            qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
         }
         break;
     case PA_VERREG:
diff --git a/hw/timer/etraxfs_timer.c b/hw/timer/etraxfs_timer.c
index 8e18236..d13bc30 100644
--- a/hw/timer/etraxfs_timer.c
+++ b/hw/timer/etraxfs_timer.c
@@ -207,7 +207,7 @@ static void watchdog_hit(void *opaque)
         qemu_irq_raise(t->nmi);
     }
     else
-        qemu_system_reset_request();
+        qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);

     t->wd_hits++;
 }
diff --git a/hw/timer/m48t59.c b/hw/timer/m48t59.c
index 474981a..4a064fb 100644
--- a/hw/timer/m48t59.c
+++ b/hw/timer/m48t59.c
@@ -1,7 +1,7 @@
 /*
  * QEMU M48T59 and M48T08 NVRAM emulation for PPC PREP and Sparc platforms
  *
- * Copyright (c) 2003-2005, 2007 Jocelyn Mayer
+ * Copyright (c) 2003-2005, 2007, 2017 Jocelyn Mayer
  * Copyright (c) 2013 Hervé Poussineau
  *
  * Permission is hereby granted, free of charge, to any person obtaining a copy
@@ -159,7 +159,7 @@ static void watchdog_cb (void *opaque)
 	NVRAM->buffer[0x1FF7] = 0x00;
 	NVRAM->buffer[0x1FFC] &= ~0x40;
         /* May it be a hw CPU Reset instead ? */
-        qemu_system_reset_request();
+        qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
     } else {
 	qemu_set_irq(NVRAM->IRQ, 1);
 	qemu_set_irq(NVRAM->IRQ, 0);
diff --git a/hw/timer/milkymist-sysctl.c b/hw/timer/milkymist-sysctl.c
index 4488590..93bc6e17 100644
--- a/hw/timer/milkymist-sysctl.c
+++ b/hw/timer/milkymist-sysctl.c
@@ -90,7 +90,7 @@ static void sysctl_icap_write(MilkymistSysctlState *s, uint32_t value)
     trace_milkymist_sysctl_icap_write(value);
     switch (value & 0xffff) {
     case 0x000e:
-        qemu_system_shutdown_request();
+        qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
         break;
     }
 }
@@ -195,7 +195,7 @@ static void sysctl_write(void *opaque, hwaddr addr, uint64_t value,
         s->regs[addr] = 1;
         break;
     case R_SYSTEM_ID:
-        qemu_system_reset_request();
+        qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
         break;

     case R_GPIO_IN:
diff --git a/hw/timer/pxa2xx_timer.c b/hw/timer/pxa2xx_timer.c
index 59002b4..68ba5a7 100644
--- a/hw/timer/pxa2xx_timer.c
+++ b/hw/timer/pxa2xx_timer.c
@@ -401,7 +401,7 @@ static void pxa2xx_timer_tick(void *opaque)
     if (t->num == 3)
         if (i->reset3 & 1) {
             i->reset3 = 0;
-            qemu_system_reset_request();
+            qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
         }
 }

diff --git a/hw/watchdog/watchdog.c b/hw/watchdog/watchdog.c
index 2aeaf1f..0c5c9cd 100644
--- a/hw/watchdog/watchdog.c
+++ b/hw/watchdog/watchdog.c
@@ -110,7 +110,7 @@ void watchdog_perform_action(void)
     switch (watchdog_action) {
     case WDT_RESET:             /* same as 'system_reset' in monitor */
         qapi_event_send_watchdog(WATCHDOG_EXPIRATION_ACTION_RESET, &error_abort);
-        qemu_system_reset_request();
+        qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
         break;

     case WDT_SHUTDOWN:          /* same as 'system_powerdown' in monitor */
diff --git a/hw/xenpv/xen_domainbuild.c b/hw/xenpv/xen_domainbuild.c
index 457a897..c89ced2 100644
--- a/hw/xenpv/xen_domainbuild.c
+++ b/hw/xenpv/xen_domainbuild.c
@@ -148,7 +148,7 @@ static void xen_domain_poll(void *opaque)
     return;

 quit:
-    qemu_system_shutdown_request();
+    qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
 }

 static int xen_domain_watcher(void)
diff --git a/hw/xtensa/xtfpga.c b/hw/xtensa/xtfpga.c
index 11176e2..4636f8e 100644
--- a/hw/xtensa/xtfpga.c
+++ b/hw/xtensa/xtfpga.c
@@ -100,7 +100,7 @@ static void lx60_fpga_write(void *opaque, hwaddr addr,

     case 0x10: /*board reset*/
         if (val == 0xdead) {
-            qemu_system_reset_request();
+            qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
         }
         break;
     }
diff --git a/kvm-all.c b/kvm-all.c
index 90b8573..7df27c8 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -2052,7 +2052,7 @@ int kvm_cpu_exec(CPUState *cpu)
             break;
         case KVM_EXIT_SHUTDOWN:
             DPRINTF("shutdown\n");
-            qemu_system_reset_request();
+            qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
             ret = EXCP_INTERRUPT;
             break;
         case KVM_EXIT_UNKNOWN:
@@ -2066,11 +2066,11 @@ int kvm_cpu_exec(CPUState *cpu)
         case KVM_EXIT_SYSTEM_EVENT:
             switch (run->system_event.type) {
             case KVM_SYSTEM_EVENT_SHUTDOWN:
-                qemu_system_shutdown_request();
+                qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
                 ret = EXCP_INTERRUPT;
                 break;
             case KVM_SYSTEM_EVENT_RESET:
-                qemu_system_reset_request();
+                qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
                 ret = EXCP_INTERRUPT;
                 break;
             case KVM_SYSTEM_EVENT_CRASH:
diff --git a/os-win32.c b/os-win32.c
index ae98574..586a7c7 100644
--- a/os-win32.c
+++ b/os-win32.c
@@ -52,7 +52,7 @@ int setenv(const char *name, const char *value, int overwrite)

 static BOOL WINAPI qemu_ctrl_handler(DWORD type)
 {
-    qemu_system_shutdown_request();
+    qemu_system_shutdown_request(SHUTDOWN_CAUSE_HOST_SIGNAL);
     /* Windows 7 kills application when the function returns.
        Sleep here to give QEMU a try for closing.
        Sleep period is 10000ms because Windows kills the program
diff --git a/qmp.c b/qmp.c
index ab74cd7..95949d0 100644
--- a/qmp.c
+++ b/qmp.c
@@ -84,7 +84,7 @@ UuidInfo *qmp_query_uuid(Error **errp)
 void qmp_quit(Error **errp)
 {
     no_shutdown = 0;
-    qemu_system_shutdown_request();
+    qemu_system_shutdown_request(SHUTDOWN_CAUSE_HOST_QMP);
 }

 void qmp_stop(Error **errp)
@@ -105,7 +105,7 @@ void qmp_stop(Error **errp)

 void qmp_system_reset(Error **errp)
 {
-    qemu_system_reset_request();
+    qemu_system_reset_request(SHUTDOWN_CAUSE_HOST_QMP);
 }

 void qmp_system_powerdown(Error **erp)
diff --git a/replay/replay.c b/replay/replay.c
index f810628..604fa4f 100644
--- a/replay/replay.c
+++ b/replay/replay.c
@@ -51,7 +51,8 @@ bool replay_next_event_is(int event)
         switch (replay_state.data_kind) {
         case EVENT_SHUTDOWN:
             replay_finish_event();
-            qemu_system_shutdown_request();
+            /* FIXME - store actual reason */
+            qemu_system_shutdown_request(SHUTDOWN_CAUSE_HOST_ERROR);
             break;
         default:
             /* clock, time_t, checkpoint and other events */
diff --git a/target/alpha/sys_helper.c b/target/alpha/sys_helper.c
index 652195d..ac22323 100644
--- a/target/alpha/sys_helper.c
+++ b/target/alpha/sys_helper.c
@@ -60,9 +60,9 @@ void helper_tb_flush(CPUAlphaState *env)
 void helper_halt(uint64_t restart)
 {
     if (restart) {
-        qemu_system_reset_request();
+        qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
     } else {
-        qemu_system_shutdown_request();
+        qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
     }
 }

diff --git a/target/arm/psci.c b/target/arm/psci.c
index ade9fe2..fc34b26 100644
--- a/target/arm/psci.c
+++ b/target/arm/psci.c
@@ -137,7 +137,7 @@ void arm_handle_psci_call(ARMCPU *cpu)
         }
         break;
     case QEMU_PSCI_0_2_FN_SYSTEM_RESET:
-        qemu_system_reset_request();
+        qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
         /* QEMU reset and shutdown are async requests, but PSCI
          * mandates that we never return from the reset/shutdown
          * call, so power the CPU off now so it doesn't execute
@@ -145,7 +145,7 @@ void arm_handle_psci_call(ARMCPU *cpu)
          */
         goto cpu_off;
     case QEMU_PSCI_0_2_FN_SYSTEM_OFF:
-        qemu_system_shutdown_request();
+        qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
         goto cpu_off;
     case QEMU_PSCI_0_1_FN_CPU_ON:
     case QEMU_PSCI_0_2_FN_CPU_ON:
diff --git a/target/i386/excp_helper.c b/target/i386/excp_helper.c
index ee596c6..b769772 100644
--- a/target/i386/excp_helper.c
+++ b/target/i386/excp_helper.c
@@ -59,7 +59,7 @@ static int check_exception(CPUX86State *env, int intno, int *error_code,

         qemu_log_mask(CPU_LOG_RESET, "Triple fault\n");

-        qemu_system_reset_request();
+        qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
         return EXCP_HLT;
     }
 #endif
diff --git a/target/i386/hax-all.c b/target/i386/hax-all.c
index ef13015..7346931 100644
--- a/target/i386/hax-all.c
+++ b/target/i386/hax-all.c
@@ -540,14 +540,14 @@ static int hax_vcpu_hax_exec(CPUArchState *env)
         /* Guest state changed, currently only for shutdown */
         case HAX_EXIT_STATECHANGE:
             fprintf(stdout, "VCPU shutdown request\n");
-            qemu_system_shutdown_request();
+            qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
             hax_vcpu_sync_state(env, 0);
             ret = 1;
             break;
         case HAX_EXIT_UNKNOWN_VMEXIT:
             fprintf(stderr, "Unknown VMX exit %x from guest\n",
                     ht->_exit_reason);
-            qemu_system_reset_request();
+            qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
             hax_vcpu_sync_state(env, 0);
             cpu_dump_state(cpu, stderr, fprintf, 0);
             ret = -1;
@@ -578,7 +578,7 @@ static int hax_vcpu_hax_exec(CPUArchState *env)
             break;
         default:
             fprintf(stderr, "Unknown exit %x from HAX\n", ht->_exit_status);
-            qemu_system_reset_request();
+            qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
             hax_vcpu_sync_state(env, 0);
             cpu_dump_state(cpu, stderr, fprintf, 0);
             ret = 1;
diff --git a/target/i386/helper.c b/target/i386/helper.c
index f11cac6..ee7eff2 100644
--- a/target/i386/helper.c
+++ b/target/i386/helper.c
@@ -1212,7 +1212,7 @@ static void do_inject_x86_mce(CPUState *cs, run_on_cpu_data data)
                            " triple fault\n",
                            cs->cpu_index);
             qemu_log_mask(CPU_LOG_RESET, "Triple fault\n");
-            qemu_system_reset_request();
+            qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
             return;
         }
         if (banks[1] & MCI_STATUS_VAL) {
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index 55865db..cfd7695 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -2930,7 +2930,7 @@ int kvm_arch_process_async_events(CPUState *cs)

         if (env->exception_injected == EXCP08_DBLE) {
             /* this means triple fault */
-            qemu_system_reset_request();
+            qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
             cs->exit_request = 1;
             return 0;
         }
diff --git a/target/s390x/helper.c b/target/s390x/helper.c
index 68bd2f9..d2bb9aa 100644
--- a/target/s390x/helper.c
+++ b/target/s390x/helper.c
@@ -266,7 +266,7 @@ void load_psw(CPUS390XState *env, uint64_t mask, uint64_t addr)
         S390CPU *cpu = s390_env_get_cpu(env);
         if (s390_cpu_halt(cpu) == 0) {
 #ifndef CONFIG_USER_ONLY
-            qemu_system_shutdown_request();
+            qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
 #endif
         }
     }
diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index 1a249d8..284f5ef 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -1929,7 +1929,7 @@ static int handle_intercept(S390CPU *cpu)
             cpu_synchronize_state(cs);
             if (s390_cpu_halt(cpu) == 0) {
                 if (is_special_wait_psw(cs)) {
-                    qemu_system_shutdown_request();
+                    qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
                 } else {
                     qemu_system_guest_panicked(NULL);
                 }
@@ -1938,7 +1938,7 @@ static int handle_intercept(S390CPU *cpu)
             break;
         case ICPT_CPU_STOP:
             if (s390_cpu_set_state(CPU_STATE_STOPPED, cpu) == 0) {
-                qemu_system_shutdown_request();
+                qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
             }
             if (cpu->env.sigp_order == SIGP_STOP_STORE_STATUS) {
                 kvm_s390_store_status(cpu, KVM_S390_STORE_STATUS_DEF_ADDR,
diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c
index eca8244..768043e 100644
--- a/target/s390x/misc_helper.c
+++ b/target/s390x/misc_helper.c
@@ -533,11 +533,11 @@ uint32_t HELPER(sigp)(CPUS390XState *env, uint64_t order_code, uint32_t r1,
         break;
 #if !defined(CONFIG_USER_ONLY)
     case SIGP_RESTART:
-        qemu_system_reset_request();
+        qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
         cpu_loop_exit(CPU(s390_env_get_cpu(env)));
         break;
     case SIGP_STOP:
-        qemu_system_shutdown_request();
+        qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
         cpu_loop_exit(CPU(s390_env_get_cpu(env)));
         break;
 #endif
diff --git a/target/sparc/int32_helper.c b/target/sparc/int32_helper.c
index 09afe13..eec9a4d 100644
--- a/target/sparc/int32_helper.c
+++ b/target/sparc/int32_helper.c
@@ -109,7 +109,7 @@ void sparc_cpu_do_interrupt(CPUState *cs)
     if (env->psret == 0) {
         if (cs->exception_index == 0x80 &&
             env->def->features & CPU_FEATURE_TA0_SHUTDOWN) {
-            qemu_system_shutdown_request();
+            qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
         } else {
             cpu_abort(cs, "Trap 0x%02x while interrupts disabled, Error state",
                       cs->exception_index);
diff --git a/ui/sdl.c b/ui/sdl.c
index 37c21a0..bd51ffd 100644
--- a/ui/sdl.c
+++ b/ui/sdl.c
@@ -837,7 +837,7 @@ static void sdl_refresh(DisplayChangeListener *dcl)
         case SDL_QUIT:
             if (!no_quit) {
                 no_shutdown = 0;
-                qemu_system_shutdown_request();
+                qemu_system_shutdown_request(SHUTDOWN_CAUSE_HOST_UI);
             }
             break;
         case SDL_MOUSEMOTION:
diff --git a/ui/sdl2.c b/ui/sdl2.c
index faf9bdf..e092636 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -568,7 +568,7 @@ static void handle_windowevent(SDL_Event *ev)
     case SDL_WINDOWEVENT_CLOSE:
         if (!no_quit) {
             no_shutdown = 0;
-            qemu_system_shutdown_request();
+            qemu_system_shutdown_request(SHUTDOWN_CAUSE_HOST_UI);
         }
         break;
     case SDL_WINDOWEVENT_SHOWN:
@@ -611,7 +611,7 @@ void sdl2_poll_events(struct sdl2_console *scon)
         case SDL_QUIT:
             if (!no_quit) {
                 no_shutdown = 0;
-                qemu_system_shutdown_request();
+                qemu_system_shutdown_request(SHUTDOWN_CAUSE_HOST_UI);
             }
             break;
         case SDL_MOUSEMOTION:
diff --git a/trace-events b/trace-events
index e582d63..433865f 100644
--- a/trace-events
+++ b/trace-events
@@ -38,7 +38,7 @@ vm_state_notify(int running, int reason) "running %d reason %d"
 load_file(const char *name, const char *path) "name %s location %s"
 runstate_set(int new_state) "new state %d"
 system_wakeup_request(int reason) "reason=%d"
-qemu_system_shutdown_request(void) ""
+qemu_system_shutdown_request(int reason) "reason=%d"
 qemu_system_powerdown_request(void) ""

 # spice-qemu-char.c
diff --git a/ui/cocoa.m b/ui/cocoa.m
index 207555e..f89f686 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -934,7 +934,7 @@ QemuCocoaView *cocoaView;
 {
     COCOA_DEBUG("QemuCocoaAppController: applicationWillTerminate\n");

-    qemu_system_shutdown_request();
+    qemu_system_shutdown_request(SHUTDOWN_CAUSE_HOST_UI);
     exit(0);
 }

-- 
2.9.3


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v6 3/5] shutdown: Add source information to SHUTDOWN and RESET

[David Gibson]

[-- Attachment #1.1: Type: text/plain, Size: 2221 bytes --]

On Fri, May 05, 2017 at 02:38:08PM -0500, Eric Blake wrote:
> Time to wire up all the call sites that request a shutdown or
> reset to use the enum added in the previous patch.
> 
> It would have been less churn to keep the common case with no
> arguments as meaning guest-triggered, and only modified the
> host-triggered code paths, via a wrapper function, but then we'd
> still have to audit that I didn't miss any host-triggered spots;
> changing the signature forces us to double-check that I correctly
> categorized all callers.
> 
> Since command line options can change whether a guest reset request
> causes an actual reset vs. a shutdown, it's easy to also add the
> information to reset requests.
> 
> Replay adds a FIXME to preserve the cause across the replay stream,
> that will be tackled in the next patch.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>
> Acked-by: David Gibson <david@gibson.dropbear.id.au> [ppc parts]
> Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> [SPARC part]

[snip]

> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 9f18f75..2735fe9 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -1166,7 +1166,7 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
>      spapr_ovec_cleanup(ov5_updates);
> 
>      if (spapr->cas_reboot) {
> -        qemu_system_reset_request();
> +        qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);

I'm not 100% sure about this one, since I'm not sure 100% of how the
different enum values are defined.  This one is tripped when feature
negotiation between firmware and guest can't be satisfied without
rebooting (next time round the firmware will use some different
options).

So it's essentially a firmware/hypervisor triggered reset, but one
that should only ever be tripped during early guest boot.  Is
CAUSE_GUEST_RESET correct for that?

Apart from this, ppc changes

Acked-by: David Gibson <david@gibson.dropbear.id.au>

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v6 3/5] shutdown: Add source information to SHUTDOWN and RESET

[Eric Blake]

[-- Attachment #1.1.1: Type: text/plain, Size: 3253 bytes --]

On 05/08/2017 12:26 AM, David Gibson wrote:
> On Fri, May 05, 2017 at 02:38:08PM -0500, Eric Blake wrote:
>> Time to wire up all the call sites that request a shutdown or
>> reset to use the enum added in the previous patch.
>>
>> It would have been less churn to keep the common case with no
>> arguments as meaning guest-triggered, and only modified the
>> host-triggered code paths, via a wrapper function, but then we'd
>> still have to audit that I didn't miss any host-triggered spots;
>> changing the signature forces us to double-check that I correctly
>> categorized all callers.
>>
>> Since command line options can change whether a guest reset request
>> causes an actual reset vs. a shutdown, it's easy to also add the
>> information to reset requests.
>>
>> Replay adds a FIXME to preserve the cause across the replay stream,
>> that will be tackled in the next patch.
>>
>> Signed-off-by: Eric Blake <eblake@redhat.com>
>> Acked-by: David Gibson <david@gibson.dropbear.id.au> [ppc parts]
>> Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> [SPARC part]
> 
> [snip]
> 
>> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
>> index 9f18f75..2735fe9 100644
>> --- a/hw/ppc/spapr_hcall.c
>> +++ b/hw/ppc/spapr_hcall.c
>> @@ -1166,7 +1166,7 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
>>      spapr_ovec_cleanup(ov5_updates);
>>
>>      if (spapr->cas_reboot) {
>> -        qemu_system_reset_request();
>> +        qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
> 
> I'm not 100% sure about this one, since I'm not sure 100% of how the
> different enum values are defined.  This one is tripped when feature
> negotiation between firmware and guest can't be satisfied without
> rebooting (next time round the firmware will use some different
> options).

Patch 2/5 introduced the enum.  The biggest part of the patch (for now)
is that anything SHUTDOWN_CAUSE_HOST_ will be exposed to the QMP client
as host-triggered, anything SHUTDOWN_CAUSE_GUEST_ will be exposed as
guest-triggered.  I basically used SHUTDOWN_CAUSE_GUEST_RESET for any
call to qemu_system_reset_requst() underneath the hw/ tree, because the
hw/ tree is emulating guest behavior and therefore it is presumably a
reset caused by a guest request.

> 
> So it's essentially a firmware/hypervisor triggered reset, but one
> that should only ever be tripped during early guest boot.  Is
> CAUSE_GUEST_RESET correct for that?

Of course, I'm not an export on SPAPR, so I'll happily change it to
anything else if you think that is more appropriate. But the rule of
thumb I went by is whether this is qemu emulating a bare-metal
reset/shutdown, vs. qemu killing the guest without waiting for guest
instructions to reach some magic
memory/register/ACPI/who-knows-what-else request.  While it may happen
only early during guest boot, it is still the guest firmware that is
requesting it, and not qemu causing a unilateral death.

> 
> Apart from this, ppc changes
> 
> Acked-by: David Gibson <david@gibson.dropbear.id.au>

Thanks.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v6 3/5] shutdown: Add source information to SHUTDOWN and RESET

[David Gibson]

[-- Attachment #1.1: Type: text/plain, Size: 4347 bytes --]

On Mon, May 08, 2017 at 09:32:42AM -0500, Eric Blake wrote:
> On 05/08/2017 12:26 AM, David Gibson wrote:
> > On Fri, May 05, 2017 at 02:38:08PM -0500, Eric Blake wrote:
> >> Time to wire up all the call sites that request a shutdown or
> >> reset to use the enum added in the previous patch.
> >>
> >> It would have been less churn to keep the common case with no
> >> arguments as meaning guest-triggered, and only modified the
> >> host-triggered code paths, via a wrapper function, but then we'd
> >> still have to audit that I didn't miss any host-triggered spots;
> >> changing the signature forces us to double-check that I correctly
> >> categorized all callers.
> >>
> >> Since command line options can change whether a guest reset request
> >> causes an actual reset vs. a shutdown, it's easy to also add the
> >> information to reset requests.
> >>
> >> Replay adds a FIXME to preserve the cause across the replay stream,
> >> that will be tackled in the next patch.
> >>
> >> Signed-off-by: Eric Blake <eblake@redhat.com>
> >> Acked-by: David Gibson <david@gibson.dropbear.id.au> [ppc parts]
> >> Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> [SPARC part]
> > 
> > [snip]
> > 
> >> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> >> index 9f18f75..2735fe9 100644
> >> --- a/hw/ppc/spapr_hcall.c
> >> +++ b/hw/ppc/spapr_hcall.c
> >> @@ -1166,7 +1166,7 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
> >>      spapr_ovec_cleanup(ov5_updates);
> >>
> >>      if (spapr->cas_reboot) {
> >> -        qemu_system_reset_request();
> >> +        qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
> > 
> > I'm not 100% sure about this one, since I'm not sure 100% of how the
> > different enum values are defined.  This one is tripped when feature
> > negotiation between firmware and guest can't be satisfied without
> > rebooting (next time round the firmware will use some different
> > options).
> 
> Patch 2/5 introduced the enum.  The biggest part of the patch (for now)
> is that anything SHUTDOWN_CAUSE_HOST_ will be exposed to the QMP client
> as host-triggered, anything SHUTDOWN_CAUSE_GUEST_ will be exposed as
> guest-triggered.  I basically used SHUTDOWN_CAUSE_GUEST_RESET for any
> call to qemu_system_reset_requst() underneath the hw/ tree, because the
> hw/ tree is emulating guest behavior and therefore it is presumably a
> reset caused by a guest request.

Ok.

> > So it's essentially a firmware/hypervisor triggered reset, but one
> > that should only ever be tripped during early guest boot.  Is
> > CAUSE_GUEST_RESET correct for that?
> 
> Of course, I'm not an export on SPAPR, so I'll happily change it to
> anything else if you think that is more appropriate. But the rule of
> thumb I went by is whether this is qemu emulating a bare-metal
> reset/shutdown, vs. qemu killing the guest without waiting for guest
> instructions to reach some magic
> memory/register/ACPI/who-knows-what-else request.  While it may happen
> only early during guest boot, it is still the guest firmware that is
> requesting it, and not qemu causing a unilateral death.

So, I think GUEST_RESET is the right choice here.  The distinctions
are blurry, because PAPR is built as a paravirtualized platform -
there is no bare metal equivalent.  For example, on PowerVM this
really would be initiated by guest firmware, but it's working in
communication with the hypervisor.  But with qemu and KVM, we actually
implement all this negotiation logic in qemu directly (this is easier
than having complex communication channels between guest firware and
qemu).

But I guess the point is that this is a "business as usual" reboot and
the guest is expected to continue booting at some point after this,
rather than being killed by qemu.

Things get more complicated still if we think about what happens if
this feature negotiation fails - then we can't boot the guest OS.  At
the moment the few cases where this happens, I think we just exit
qemu, but notifying this as a host caused shutdown might be
appropriate.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel