Re: [Qemu-devel] [PATCH v2] s390/ipl: fix ipl with -no-reboot

[David Hildenbrand <david@redhat.com>]

On 21.06.2018 18:51, Christian Borntraeger wrote:
> 
> 
> On 06/21/2018 06:35 PM, Christian Borntraeger wrote:
>> kexec/kdump as well as the bootloader use a subcode of diagnose 308
>> that is supposed to reset the subsystem but not comprise a full
>> "reboot". With the latest refactoring this is now broken when
>> -no-reboot is used. This for example break virt-install from
>> iso images.
> 
> Its even worse. With this patch I can IPL with --no-reboot, but
> virt-install still creates an xml like
> 
>   <on_poweroff>destroy</on_poweroff>
>   <on_reboot>destroy</on_reboot>
>   <on_crash>destroy</on_crash>
> 
> With you patch a "soft reset" still seems to trigger an qapi event
> so that libvirt kills the guest due to that...

Either


> 
>>
>> We need to mark these "soft" reboots as ok for rebooting.
>>
>> Fixes: a30fb811cbe9 (s390x: refactor reset/reipl handling)
>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>> ---
>>  hw/s390x/ipl.c          | 8 +++++++-
>>  include/sysemu/sysemu.h | 3 +++
>>  vl.c                    | 2 +-
>>  3 files changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
>> index 0d67349004..ade6896af6 100644
>> --- a/hw/s390x/ipl.c
>> +++ b/hw/s390x/ipl.c
>> @@ -535,7 +535,13 @@ void s390_ipl_reset_request(CPUState *cs, enum s390_reset reset_type)
>>              ipl->iplb_valid = s390_gen_initial_iplb(ipl);
>>          }
>>      }
>> -    qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
>> +    if (reset_type == S390_RESET_MODIFIED_CLEAR ||
>> +        reset_type == S390_RESET_LOAD_NORMAL) {
>> +        /* ignore -no-reboot */
>> +        qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET_FORCE);
>> +    } else {
>> +        qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
>> +    }
>>      /* as this is triggered by a CPU, make sure to exit the loop */
>>      if (tcg_enabled()) {
>>          cpu_loop_exit(cs);
>> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
>> index e893f72f3b..345369d924 100644
>> --- a/include/sysemu/sysemu.h
>> +++ b/include/sysemu/sysemu.h
>> @@ -44,6 +44,9 @@ typedef enum ShutdownCause {
>>                                       turns that into a shutdown */
>>      SHUTDOWN_CAUSE_GUEST_PANIC,   /* Guest panicked, and command line turns
>>                                       that into a shutdown */
>> +    SHUTDOWN_CAUSE_GUEST_RESET_FORCE,/* Guest reset that should ignore
>> +                                        --no-reboot. This is useful for reset
>> +                                        like actions as s390 kexec/kdump */
>>      SHUTDOWN_CAUSE__MAX,
>>  } ShutdownCause;
>>  
>> diff --git a/vl.c b/vl.c
>> index b3426e03d0..722c3b9963 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -1674,7 +1674,7 @@ void qemu_system_guest_panicked(GuestPanicInformation *info)
>>  
>>  void qemu_system_reset_request(ShutdownCause reason)
>>  {
>> -    if (no_reboot) {
>> +    if (no_reboot && reason != SHUTDOWN_CAUSE_GUEST_RESET_FORCE) {
>>          shutdown_requested = reason;
>>      } else {
>>          reset_requested = reason;
>>
> 

Maybe

SHUTDOWN_CAUSE_GUEST_RESET_FORCE -> SHUTDOWN_CAUSE_GUEST_RESET_INTERNAL

diff --git a/vl.c b/vl.c
index b3426e03d0..198285be4b 100644
--- a/vl.c
+++ b/vl.c
@@ -1628,7 +1628,7 @@ void qemu_system_reset(ShutdownCause reason)
     } else {
         qemu_devices_reset();
     }
-    if (reason) {
+    if (reason != SHUTDOWN_CAUSE_GUEST_RESET_INTERNAL) {
         qapi_event_send_reset(shutdown_caused_by_guest(reason),
                               &error_abort);
     }


Or revert ;)

-- 

Thanks,

David / dhildenb