Re: [Qemu-devel] [PATCH v4 17/17] ipmi: Add a force off function

[Daniel P. Berrange]

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, address@hidden wrote:
> >>> From: Corey Minyard <address@hidden>
> >>>
> >>> Allow the IPMI interface to request a forced power off.
> >>>
> >>> Signed-off-by: Corey Minyard <address@hidden>
> >>> ---
> >>>  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
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|