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

From: Markus Armbruster <armbru@redhat.com>
To: Eric Blake <eblake@redhat.com>
Cc: Peter Maydell <peter.maydell@linaro.org>,
	"open list:Overall" <kvm@vger.kernel.org>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>,
	qemu-devel@nongnu.org, Max Filippov <jcmvbkbc@gmail.com>,
	Gerd Hoffmann <kraxel@redhat.com>,
	"Edgar E. Iglesias" <edgar.iglesias@gmail.com>,
	Rob Herring <robh@kernel.org>,
	Stefano Stabellini <sstabellini@kernel.org>,
	Cornelia Huck <cornelia.huck@de.ibm.com>,
	Magnus Damm <magnus.damm@gmail.com>,
	Alexander Graf <agraf@suse.de>,
	Christian Borntraeger <borntraeger@de.ibm.com>,
	Anthony Perard <anthony.perard@citrix.com>,
	"open list:X86" <xen-devel@lists.xenproject.org>,
	David Gibson <david@gibson.dropbear.id.au>,
	Artyom Tarasenko <atar4qemu@gmail.com>,
	Eduardo Habkost <ehabkost@redhat
Subject: Re: [Qemu-devel] [PATCH v7 3/5] shutdown: Add source information to SHUTDOWN and RESET
Date: Wed, 10 May 2017 16:44:12 +0200	[thread overview]
Message-ID: <87fugcwz43.fsf@dusky.pond.sub.org> (raw)
In-Reply-To: <81782139-d66d-72d4-c228-9dd9073a33b9@redhat.com> (Eric Blake's message of "Tue, 9 May 2017 09:07:26 -0500")

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.

[...]