Re: [Qemu-devel] [PATCH v7 3/5] shutdown: Add source information to SHUTDOWN and RESET

[Markus Armbruster <armbru@redhat.com>]

Eric Blake <eblake@redhat.com> writes:

> On 05/09/2017 06:56 AM, Markus Armbruster wrote:
>> Eric Blake <eblake@redhat.com> writes:
>> 
>>> 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.
>>>
>
>>> @@ -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);
>> 
>> I'm fine with using SHUTDOWN_CAUSE_GUEST_SHUTDOWN for suspend, but have
>> you considered SHUTDOWN_CAUSE_GUEST_SUSPEND?
>
> It was easy to do
> s/qemu_system_shutdown_request()/qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN)/
> for all hw/ files.  Harder would be picking a difference between
> _SHUTDOWN and a new _SUSPEND. I can do it if hardware owners want the
> distinction; but remember that this series will intentionally NOT expose
> that distinction to QMP, so I don't know how much it will buy us.

Understand.  What about clarifying SHUTDOWN_CAUSE_GUEST_SHUTDOWN's
comment

    /* Guest requested shutdown, such as via
       ACPI or other hardware-specific means */

by adding something like "(including suspend)"?  Can do on commit, we
just have to agree on a specific working.

[...]