diff mbox

ipmi: chassis poweroff should use qemu_system_shutdown_request()

Message ID 1472227531-22887-1-git-send-email-clg@kaod.org
State New
Headers show

Commit Message

Cédric Le Goater Aug. 26, 2016, 4:05 p.m. UTC
When issuing a chassis 'powerdown' control command, the routine
qemu_system_shutdown_request() should be used to exit the guest.
qemu_system_powerdown_request() will initiate a soft shutdown which is
not what is required by the IPMI (28.3 Chassis Control Command):

    0h = power down. Force system into soft off (S4/S45) state. This
    is for 'emergency' management power down actions. The command does
    not initiate a clean shut-down of the operating system prior to
    powering down the system
    
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---

 We could use qemu_system_powerdown_request() under 
 IPMI_SHUTDOWN_VIA_ACPI_OVERTEMP which is what is expected I think.

 hw/ipmi/ipmi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Greg Kurz Aug. 28, 2016, 8:44 p.m. UTC | #1
On Fri, 26 Aug 2016 18:05:31 +0200
Cédric Le Goater <clg@kaod.org> wrote:

> When issuing a chassis 'powerdown' control command, the routine
> qemu_system_shutdown_request() should be used to exit the guest.
> qemu_system_powerdown_request() will initiate a soft shutdown which is
> not what is required by the IPMI (28.3 Chassis Control Command):
> 
>     0h = power down. Force system into soft off (S4/S45) state. This
>     is for 'emergency' management power down actions. The command does
>     not initiate a clean shut-down of the operating system prior to
>     powering down the system
>     
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
> 

FWIW this had been suggested during the review:

https://lists.gnu.org/archive/html/qemu-devel/2015-11/msg03304.html

Acked-by: Greg Kurz <groug@kaod.org>

>  We could use qemu_system_powerdown_request() under 
>  IPMI_SHUTDOWN_VIA_ACPI_OVERTEMP which is what is expected I think.
> 

5h = Initiate a soft-shutdown of OS via ACPI by emulating a fatal
overtemperature. (optional)

This looks appropriate indeed.

>  hw/ipmi/ipmi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/ipmi/ipmi.c b/hw/ipmi/ipmi.c
> index f09f217e7835..f91c7b74ca38 100644
> --- a/hw/ipmi/ipmi.c
> +++ b/hw/ipmi/ipmi.c
> @@ -51,7 +51,7 @@ static int ipmi_do_hw_op(IPMIInterface *s, enum ipmi_op op, int checkonly)
>          if (checkonly) {
>              return 0;
>          }
> -        qemu_system_powerdown_request();
> +        qemu_system_shutdown_request();
>          return 0;
>  
>      case IPMI_SEND_NMI:
Corey Minyard Aug. 30, 2016, 1:16 a.m. UTC | #2
On 08/28/2016 03:44 PM, Greg Kurz wrote:
> On Fri, 26 Aug 2016 18:05:31 +0200
> Cédric Le Goater <clg@kaod.org> wrote:
>
>> When issuing a chassis 'powerdown' control command, the routine
>> qemu_system_shutdown_request() should be used to exit the guest.
>> qemu_system_powerdown_request() will initiate a soft shutdown which is
>> not what is required by the IPMI (28.3 Chassis Control Command):
>>
>>      0h = power down. Force system into soft off (S4/S45) state. This
>>      is for 'emergency' management power down actions. The command does
>>      not initiate a clean shut-down of the operating system prior to
>>      powering down the system
>>      
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>
> FWIW this had been suggested during the review:
>
> https://lists.gnu.org/archive/html/qemu-devel/2015-11/msg03304.html

I think I misread that when I did this originally.  Yes, you are right.

Acked-by: Corey Minyard <cminyard@mvista.com>

Can this go in now, or do I need to pull it into my tree?

-corey

> Acked-by: Greg Kurz <groug@kaod.org>
>
>>   We could use qemu_system_powerdown_request() under
>>   IPMI_SHUTDOWN_VIA_ACPI_OVERTEMP which is what is expected I think.
>>
> 5h = Initiate a soft-shutdown of OS via ACPI by emulating a fatal
> overtemperature. (optional)
>
> This looks appropriate indeed.
>
>>   hw/ipmi/ipmi.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/hw/ipmi/ipmi.c b/hw/ipmi/ipmi.c
>> index f09f217e7835..f91c7b74ca38 100644
>> --- a/hw/ipmi/ipmi.c
>> +++ b/hw/ipmi/ipmi.c
>> @@ -51,7 +51,7 @@ static int ipmi_do_hw_op(IPMIInterface *s, enum ipmi_op op, int checkonly)
>>           if (checkonly) {
>>               return 0;
>>           }
>> -        qemu_system_powerdown_request();
>> +        qemu_system_shutdown_request();
>>           return 0;
>>   
>>       case IPMI_SEND_NMI:
Cédric Le Goater Aug. 30, 2016, 1:43 p.m. UTC | #3
On 08/30/2016 03:16 AM, Corey Minyard wrote:
> On 08/28/2016 03:44 PM, Greg Kurz wrote:
>> On Fri, 26 Aug 2016 18:05:31 +0200
>> Cédric Le Goater <clg@kaod.org> wrote:
>>
>>> When issuing a chassis 'powerdown' control command, the routine
>>> qemu_system_shutdown_request() should be used to exit the guest.
>>> qemu_system_powerdown_request() will initiate a soft shutdown which is
>>> not what is required by the IPMI (28.3 Chassis Control Command):
>>>
>>>      0h = power down. Force system into soft off (S4/S45) state. This
>>>      is for 'emergency' management power down actions. The command does
>>>      not initiate a clean shut-down of the operating system prior to
>>>      powering down the system
>>>      Signed-off-by: Cédric Le Goater <clg@kaod.org>
>>> ---
>>>
>> FWIW this had been suggested during the review:
>>
>> https://lists.gnu.org/archive/html/qemu-devel/2015-11/msg03304.html
> 
> I think I misread that when I did this originally.  Yes, you are right.
> 
> Acked-by: Corey Minyard <cminyard@mvista.com>
> 
> Can this go in now, or do I need to pull it into my tree?

So we should ask Peter or Michael for that. no ? 

I think is 2.8 material unless we have some more time for 2.7. 

Thanks,

C.


> 
> -corey
> 
>> Acked-by: Greg Kurz <groug@kaod.org>
>>
>>>   We could use qemu_system_powerdown_request() under
>>>   IPMI_SHUTDOWN_VIA_ACPI_OVERTEMP which is what is expected I think.
>>>
>> 5h = Initiate a soft-shutdown of OS via ACPI by emulating a fatal
>> overtemperature. (optional)
>>
>> This looks appropriate indeed.
>>
>>>   hw/ipmi/ipmi.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/hw/ipmi/ipmi.c b/hw/ipmi/ipmi.c
>>> index f09f217e7835..f91c7b74ca38 100644
>>> --- a/hw/ipmi/ipmi.c
>>> +++ b/hw/ipmi/ipmi.c
>>> @@ -51,7 +51,7 @@ static int ipmi_do_hw_op(IPMIInterface *s, enum ipmi_op op, int checkonly)
>>>           if (checkonly) {
>>>               return 0;
>>>           }
>>> -        qemu_system_powerdown_request();
>>> +        qemu_system_shutdown_request();
>>>           return 0;
>>>         case IPMI_SEND_NMI:
> 
>
Peter Maydell Aug. 30, 2016, 2:14 p.m. UTC | #4
On 30 August 2016 at 14:43, Cédric Le Goater <clg@kaod.org> wrote:
> On 08/30/2016 03:16 AM, Corey Minyard wrote:
>> On 08/28/2016 03:44 PM, Greg Kurz wrote:
>>> On Fri, 26 Aug 2016 18:05:31 +0200
>>> Cédric Le Goater <clg@kaod.org> wrote:
>>>
>>>> When issuing a chassis 'powerdown' control command, the routine
>>>> qemu_system_shutdown_request() should be used to exit the guest.
>>>> qemu_system_powerdown_request() will initiate a soft shutdown which is
>>>> not what is required by the IPMI (28.3 Chassis Control Command):
>>>>
>>>>      0h = power down. Force system into soft off (S4/S45) state. This
>>>>      is for 'emergency' management power down actions. The command does
>>>>      not initiate a clean shut-down of the operating system prior to
>>>>      powering down the system
>>>>      Signed-off-by: Cédric Le Goater <clg@kaod.org>
>>>> ---
>>>>
>>> FWIW this had been suggested during the review:
>>>
>>> https://lists.gnu.org/archive/html/qemu-devel/2015-11/msg03304.html
>>
>> I think I misread that when I did this originally.  Yes, you are right.
>>
>> Acked-by: Corey Minyard <cminyard@mvista.com>
>>
>> Can this go in now, or do I need to pull it into my tree?
>
> So we should ask Peter or Michael for that. no ?
>
> I think is 2.8 material unless we have some more time for 2.7.

Not for 2.7 unless it is an absolutely release-critical
showstopper bugfix, which it doesn't look like to me
from a quick glance.

thanks
-- PMM
diff mbox

Patch

diff --git a/hw/ipmi/ipmi.c b/hw/ipmi/ipmi.c
index f09f217e7835..f91c7b74ca38 100644
--- a/hw/ipmi/ipmi.c
+++ b/hw/ipmi/ipmi.c
@@ -51,7 +51,7 @@  static int ipmi_do_hw_op(IPMIInterface *s, enum ipmi_op op, int checkonly)
         if (checkonly) {
             return 0;
         }
-        qemu_system_powerdown_request();
+        qemu_system_shutdown_request();
         return 0;
 
     case IPMI_SEND_NMI: