Re: [qemu-s390x] [PATCH 1/3] qmp: don't emit the RESET event on wakeup

[Christian Borntraeger]

On 18.07.19 13:06, Paolo Bonzini wrote:
> On 18/07/19 12:39, Nicholas Piggin wrote:
>> Commit 1405819637f53 ("qmp: don't emit the RESET event on wakeup from
>> S3") changed system wakeup to avoid calling qapi_event_send_reset.
>> Commit 76ed4b18debfe ("s390/ipl: fix ipl with -no-reboot") appears to
>> have inadvertently broken that logic.
>>
>> Signed-off-by: Nicholas Piggin <address@hidden>
>> ---
>> I'm not quite sure if this patch is correct and haven't tested it, I
>> found it by inspection. If this patch is incorrect, I will have to
>> adjust patch 2.
>>
>>  vl.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/vl.c b/vl.c
>> index 5089fce6c5..ef3c7ab8b8 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -1550,7 +1550,7 @@ void qemu_system_reset(ShutdownCause reason)
>>      } else {
>>          qemu_devices_reset();
>>      }
>> -    if (reason != SHUTDOWN_CAUSE_SUBSYSTEM_RESET) {
>> +    if (reason && reason != SHUTDOWN_CAUSE_SUBSYSTEM_RESET) {
>>          qapi_event_send_reset(shutdown_caused_by_guest(reason), reason);
>>      }
>>      cpu_synchronize_all_post_reset();
>>
> 
> Yes, it seems correct and I've queued it for 4.1.

Yes makes sense. 
Would it be better to write out if(reason) e.g. replace "reason" with "reason 
!= SHUTDOWN_CAUSE_NONE" ?

Going even further, I am asking myself if something like

diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
index 60bd081d3ef3..406743566869 100644
--- a/hw/s390x/ipl.c
+++ b/hw/s390x/ipl.c
@@ -577,7 +577,7 @@ void s390_ipl_reset_request(CPUState *cs, enum s390_reset 
reset_type)
     if (reset_type == S390_RESET_MODIFIED_CLEAR ||
         reset_type == S390_RESET_LOAD_NORMAL) {
         /* ignore -no-reboot, send no event  */
-        qemu_system_reset_request(SHUTDOWN_CAUSE_SUBSYSTEM_RESET);
+        qemu_system_reset_request(SHUTDOWN_CAUSE_NONE);
     } else {
         qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
     }

would also work.