diff mbox

[v4,17/17] ipmi: Add a force off function

Message ID 1447354953-18893-18-git-send-email-minyard@acm.org
State New
Headers show

Commit Message

Corey Minyard Nov. 12, 2015, 7:02 p.m. UTC 
From: Corey Minyard <cminyard@mvista.com>

Allow the IPMI interface to request a forced power off.

Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
 hw/ipmi/ipmi_bmc_extern.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Daniel P. Berrangé Nov. 13, 2015, 9:15 a.m. UTC | #1 
On Thu, Nov 12, 2015 at 01:02:33PM -0600, minyard@acm.org wrote:
> From: Corey Minyard <cminyard@mvista.com>
> 
> Allow the IPMI interface to request a forced power off.
> 
> Signed-off-by: Corey Minyard <cminyard@mvista.com>
> ---
>  hw/ipmi/ipmi_bmc_extern.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/hw/ipmi/ipmi_bmc_extern.c b/hw/ipmi/ipmi_bmc_extern.c
> index 05b9121..7ae6294 100644
> --- a/hw/ipmi/ipmi_bmc_extern.c
> +++ b/hw/ipmi/ipmi_bmc_extern.c
> @@ -52,6 +52,7 @@
>  #define   VM_CAPABILITIES_IRQ      0x04
>  #define   VM_CAPABILITIES_NMI      0x08
>  #define   VM_CAPABILITIES_ATTN     0x10
> +#define VM_CMD_FORCEOFF            0x09
>  
>  #define TYPE_IPMI_BMC_EXTERN "ipmi-bmc-extern"
>  #define IPMI_BMC_EXTERN(obj) OBJECT_CHECK(IPMIBmcExtern, (obj), \
> @@ -268,6 +269,10 @@ static void handle_hw_op(IPMIBmcExtern *ibe, unsigned char hw_op)
>      case VM_CMD_SEND_NMI:
>          k->do_hw_op(s, IPMI_SEND_NMI, 0);
>          break;
> +
> +    case VM_CMD_FORCEOFF:
> +        exit(0);
> +        break;

You should really be calling qemu_system_shutdown_request() rather than
exit() so that you run normal QEMU shutdown logic.

Regards,
Daniel
Corey Minyard Nov. 13, 2015, 1:22 p.m. UTC | #2 
On 11/13/2015 03:15 AM, Daniel P. Berrange wrote:
> On Thu, Nov 12, 2015 at 01:02:33PM -0600, minyard@acm.org wrote:
>> From: Corey Minyard <cminyard@mvista.com>
>>
>> Allow the IPMI interface to request a forced power off.
>>
>> Signed-off-by: Corey Minyard <cminyard@mvista.com>
>> ---
>>  hw/ipmi/ipmi_bmc_extern.c | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/hw/ipmi/ipmi_bmc_extern.c b/hw/ipmi/ipmi_bmc_extern.c
>> index 05b9121..7ae6294 100644
>> --- a/hw/ipmi/ipmi_bmc_extern.c
>> +++ b/hw/ipmi/ipmi_bmc_extern.c
>> @@ -52,6 +52,7 @@
>>  #define   VM_CAPABILITIES_IRQ      0x04
>>  #define   VM_CAPABILITIES_NMI      0x08
>>  #define   VM_CAPABILITIES_ATTN     0x10
>> +#define VM_CMD_FORCEOFF            0x09
>>  
>>  #define TYPE_IPMI_BMC_EXTERN "ipmi-bmc-extern"
>>  #define IPMI_BMC_EXTERN(obj) OBJECT_CHECK(IPMIBmcExtern, (obj), \
>> @@ -268,6 +269,10 @@ static void handle_hw_op(IPMIBmcExtern *ibe, unsigned char hw_op)
>>      case VM_CMD_SEND_NMI:
>>          k->do_hw_op(s, IPMI_SEND_NMI, 0);
>>          break;
>> +
>> +    case VM_CMD_FORCEOFF:
>> +        exit(0);
>> +        break;
> You should really be calling qemu_system_shutdown_request() rather than
> exit() so that you run normal QEMU shutdown logic.

qemu_system_shutdown_request() would generally have already been called using
VM_CMD_POWEROFF.  If I understand correctly, qemu_system_shutdown_request() does
a graceful shutdown and may not complete if the OS has failed.

The CMD_FORCEOFF is a "power off and don't ask me any questions" operation.  It's
used as a last resort if VM_CMD_POWEROFF fails.

-corey
Paolo Bonzini Nov. 13, 2015, 1:23 p.m. UTC | #3 
On 13/11/2015 14:22, Corey Minyard wrote:
> On 11/13/2015 03:15 AM, Daniel P. Berrange wrote:
>> On Thu, Nov 12, 2015 at 01:02:33PM -0600, minyard@acm.org wrote:
>>> From: Corey Minyard <cminyard@mvista.com>
>>>
>>> Allow the IPMI interface to request a forced power off.
>>>
>>> Signed-off-by: Corey Minyard <cminyard@mvista.com>
>>> ---
>>>  hw/ipmi/ipmi_bmc_extern.c | 5 +++++
>>>  1 file changed, 5 insertions(+)
>>>
>>> diff --git a/hw/ipmi/ipmi_bmc_extern.c b/hw/ipmi/ipmi_bmc_extern.c
>>> index 05b9121..7ae6294 100644
>>> --- a/hw/ipmi/ipmi_bmc_extern.c
>>> +++ b/hw/ipmi/ipmi_bmc_extern.c
>>> @@ -52,6 +52,7 @@
>>>  #define   VM_CAPABILITIES_IRQ      0x04
>>>  #define   VM_CAPABILITIES_NMI      0x08
>>>  #define   VM_CAPABILITIES_ATTN     0x10
>>> +#define VM_CMD_FORCEOFF            0x09
>>>  
>>>  #define TYPE_IPMI_BMC_EXTERN "ipmi-bmc-extern"
>>>  #define IPMI_BMC_EXTERN(obj) OBJECT_CHECK(IPMIBmcExtern, (obj), \
>>> @@ -268,6 +269,10 @@ static void handle_hw_op(IPMIBmcExtern *ibe, unsigned char hw_op)
>>>      case VM_CMD_SEND_NMI:
>>>          k->do_hw_op(s, IPMI_SEND_NMI, 0);
>>>          break;
>>> +
>>> +    case VM_CMD_FORCEOFF:
>>> +        exit(0);
>>> +        break;
>> You should really be calling qemu_system_shutdown_request() rather than
>> exit() so that you run normal QEMU shutdown logic.
> 
> qemu_system_shutdown_request() would generally have already been called using
> VM_CMD_POWEROFF.  If I understand correctly, qemu_system_shutdown_request() does
> a graceful shutdown and may not complete if the OS has failed.

No, that's qemu_system_powerdown_request.

Paolo

> The CMD_FORCEOFF is a "power off and don't ask me any questions" operation.  It's
> used as a last resort if VM_CMD_POWEROFF fails.
> 
> -corey
> 
>
Corey Minyard Nov. 13, 2015, 1:34 p.m. UTC | #4 
On 11/13/2015 07:23 AM, Paolo Bonzini wrote:
>
> On 13/11/2015 14:22, Corey Minyard wrote:
>> On 11/13/2015 03:15 AM, Daniel P. Berrange wrote:
>>> On Thu, Nov 12, 2015 at 01:02:33PM -0600, minyard@acm.org wrote:
>>>> From: Corey Minyard <cminyard@mvista.com>
>>>>
>>>> Allow the IPMI interface to request a forced power off.
>>>>
>>>> Signed-off-by: Corey Minyard <cminyard@mvista.com>
>>>> ---
>>>>  hw/ipmi/ipmi_bmc_extern.c | 5 +++++
>>>>  1 file changed, 5 insertions(+)
>>>>
>>>> diff --git a/hw/ipmi/ipmi_bmc_extern.c b/hw/ipmi/ipmi_bmc_extern.c
>>>> index 05b9121..7ae6294 100644
>>>> --- a/hw/ipmi/ipmi_bmc_extern.c
>>>> +++ b/hw/ipmi/ipmi_bmc_extern.c
>>>> @@ -52,6 +52,7 @@
>>>>  #define   VM_CAPABILITIES_IRQ      0x04
>>>>  #define   VM_CAPABILITIES_NMI      0x08
>>>>  #define   VM_CAPABILITIES_ATTN     0x10
>>>> +#define VM_CMD_FORCEOFF            0x09
>>>>  
>>>>  #define TYPE_IPMI_BMC_EXTERN "ipmi-bmc-extern"
>>>>  #define IPMI_BMC_EXTERN(obj) OBJECT_CHECK(IPMIBmcExtern, (obj), \
>>>> @@ -268,6 +269,10 @@ static void handle_hw_op(IPMIBmcExtern *ibe, unsigned char hw_op)
>>>>      case VM_CMD_SEND_NMI:
>>>>          k->do_hw_op(s, IPMI_SEND_NMI, 0);
>>>>          break;
>>>> +
>>>> +    case VM_CMD_FORCEOFF:
>>>> +        exit(0);
>>>> +        break;
>>> You should really be calling qemu_system_shutdown_request() rather than
>>> exit() so that you run normal QEMU shutdown logic.
>> qemu_system_shutdown_request() would generally have already been called using
>> VM_CMD_POWEROFF.  If I understand correctly, qemu_system_shutdown_request() does
>> a graceful shutdown and may not complete if the OS has failed.
> No, that's qemu_system_powerdown_request.
>
> Paolo

Ah, ok, got it.

Thanks Daniel and Paulo,

-corey

>
>> The CMD_FORCEOFF is a "power off and don't ask me any questions" operation.  It's
>> used as a last resort if VM_CMD_POWEROFF fails.
>>
>> -corey
>>
>>
Daniel P. Berrangé Nov. 13, 2015, 2:04 p.m. UTC | #5 
On Fri, Nov 13, 2015 at 02:23:35PM +0100, Paolo Bonzini wrote:
> 
> 
> On 13/11/2015 14:22, Corey Minyard wrote:
> > On 11/13/2015 03:15 AM, Daniel P. Berrange wrote:
> >> On Thu, Nov 12, 2015 at 01:02:33PM -0600, minyard@acm.org wrote:
> >>> From: Corey Minyard <cminyard@mvista.com>
> >>>
> >>> Allow the IPMI interface to request a forced power off.
> >>>
> >>> Signed-off-by: Corey Minyard <cminyard@mvista.com>
> >>> ---
> >>>  hw/ipmi/ipmi_bmc_extern.c | 5 +++++
> >>>  1 file changed, 5 insertions(+)
> >>>
> >>> diff --git a/hw/ipmi/ipmi_bmc_extern.c b/hw/ipmi/ipmi_bmc_extern.c
> >>> index 05b9121..7ae6294 100644
> >>> --- a/hw/ipmi/ipmi_bmc_extern.c
> >>> +++ b/hw/ipmi/ipmi_bmc_extern.c
> >>> @@ -52,6 +52,7 @@
> >>>  #define   VM_CAPABILITIES_IRQ      0x04
> >>>  #define   VM_CAPABILITIES_NMI      0x08
> >>>  #define   VM_CAPABILITIES_ATTN     0x10
> >>> +#define VM_CMD_FORCEOFF            0x09
> >>>  
> >>>  #define TYPE_IPMI_BMC_EXTERN "ipmi-bmc-extern"
> >>>  #define IPMI_BMC_EXTERN(obj) OBJECT_CHECK(IPMIBmcExtern, (obj), \
> >>> @@ -268,6 +269,10 @@ static void handle_hw_op(IPMIBmcExtern *ibe, unsigned char hw_op)
> >>>      case VM_CMD_SEND_NMI:
> >>>          k->do_hw_op(s, IPMI_SEND_NMI, 0);
> >>>          break;
> >>> +
> >>> +    case VM_CMD_FORCEOFF:
> >>> +        exit(0);
> >>> +        break;
> >> You should really be calling qemu_system_shutdown_request() rather than
> >> exit() so that you run normal QEMU shutdown logic.
> > 
> > qemu_system_shutdown_request() would generally have already been called using
> > VM_CMD_POWEROFF.  If I understand correctly, qemu_system_shutdown_request() does
> > a graceful shutdown and may not complete if the OS has failed.
> 
> No, that's qemu_system_powerdown_request.

Yep, for reference the reason I suggest qemu_system_shutdown_request() is
because that's the code path triggered when you send QEMU SIGTERM/SIGQUIT
etc, and this IMPI request feels like it should be doing equivalent handling
to that scenario.


Regards,
Daniel
diff mbox

Patch

diff --git a/hw/ipmi/ipmi_bmc_extern.c b/hw/ipmi/ipmi_bmc_extern.c
index 05b9121..7ae6294 100644
--- a/hw/ipmi/ipmi_bmc_extern.c
+++ b/hw/ipmi/ipmi_bmc_extern.c
@@ -52,6 +52,7 @@ 
 #define   VM_CAPABILITIES_IRQ      0x04
 #define   VM_CAPABILITIES_NMI      0x08
 #define   VM_CAPABILITIES_ATTN     0x10
+#define VM_CMD_FORCEOFF            0x09
 
 #define TYPE_IPMI_BMC_EXTERN "ipmi-bmc-extern"
 #define IPMI_BMC_EXTERN(obj) OBJECT_CHECK(IPMIBmcExtern, (obj), \
@@ -268,6 +269,10 @@  static void handle_hw_op(IPMIBmcExtern *ibe, unsigned char hw_op)
     case VM_CMD_SEND_NMI:
         k->do_hw_op(s, IPMI_SEND_NMI, 0);
         break;
+
+    case VM_CMD_FORCEOFF:
+        exit(0);
+        break;
     }
 }