[libvirt] [PATCH] bhyve: implement domainShutdown

[Roman Bogorodskiy]

  Daniel P. Berrange wrote:

> On Tue, Mar 29, 2016 at 10:52:09AM -0400, Laine Stump wrote:
> > On 03/29/2016 08:24 AM, Daniel P. Berrange wrote:
> > >On Mon, Mar 28, 2016 at 10:27:18AM +0300, Roman Bogorodskiy wrote:
> > >>Bhyve supports ACPI shutdown by issuing SIGTERM signal to the bhyve
> > >>process. Add the bhyveDomainShutdown() function and
> > >>virBhyveProcessShutdown() helper function that just sends SIGTERM to
> > >>VM's bhyve process. If a guest supports ACPI shutdown then process
> > >>will be terminated and this event will be noticed by the bhyve monitor
> > >>code that will handle setting proper status and clean up VM's resources.
> > >>
> > >>Also, remove a warning in domainDestroy in case if
> > >>virProcessKillPainfully() returns 1, meaning that it killed process
> > >>using SIGKILL. This behavior should be expected when using 'destroy'.
> > >Hmm, so destroy is supposed to be equivalent to physically removing
> > >the power plug.  The existing code is calling virProcessShutdownPainfully
> > >which starts by sending SIGTERM and then switches to SIGKILL.  So this
> > >means that your virDomainDestroy implementation is mistakenly trying todo
> > >a graceful shutdown initially and then switching to hard shutdown after a
> > >bit of a delay.
> > 
> > For context - the qemu driver does this for destroy as well. I think at
> > least one of the reasons is to allow the qemu process to flush the buffers
> > for the disk image files. I recall this was causing some amount of pain for
> > ovirt (or maybe someone else, I forget)
> 
> Yep it is different in QEMU - SIGTERM doesn't trigger any guest OS ACPI
> event with QEMU - it merely lets QEMU gracefully close the block
> layer functions and flush I/O.
> 
> > >Has bhyve always used SIGTERM to trigger ACPI shutdown, or is this a
> > >recent addition ?  I would tend to suggest we need to go straight to
> > >SIGKILL for virDomainDestroy to avoid doing ACPI shutdown when we don't
> > >want it.
> > >
> > >It is kind of a shame they used SIGTERM for triggering ACPI imho but
> > >oh well.
> > 
> > Yes, it means there is no way to implement a "clean power off" (which would
> > give bhyve the chance to clean up some critical things before pulling the
> > plug).
> 
> Yeah, exactly, it is desirable to be able to do a immediate forced shutdown
> of guest OS while still letting the host process exit cleanly.

That's an interesting point. I noticed that there's also 'bhyvectl
--force-poweroff' command, if it does what it looks like it should do
then I guess it would be good to use it instead of the kill -9 call.
I'll check its behaviour and update the patch accordingly.

> 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 :|

Roman Bogorodskiy
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20160329/d04c1f2c/attachment-0001.sig>