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

[Eric Blake <eblake@redhat.com>]

[-- Attachment #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 #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]