On Wed, Apr 17, 2024 at 10:08:24AM +0200, Peter Krempa wrote: > On Wed, Apr 17, 2024 at 13:58:06 +0800, Sun Feng wrote: > > For abnormal shutoff reasons, we can start guest again with qemu hooks. > > Beware that: > > A hook script must not call back into libvirt, as the libvirt daemon is already waiting for the script to exit. > > A deadlock is likely to occur. > > https://libvirt.org/hooks.html#calling-libvirt-functions-from-within-a-hook-script > > Preferrably summarize what the patch is doing or suggest something that > is not strongly discouraged by the docs. > > > Signed-off-by: Sun Feng <loyou85@xxxxxxxxx> > > --- > > docs/hooks.rst | 3 ++- > > src/qemu/qemu_process.c | 3 ++- > > 2 files changed, 4 insertions(+), 2 deletions(-) > > > > diff --git a/docs/hooks.rst b/docs/hooks.rst > > index 1dbc492bd4..45856e4ca4 100644 > > --- a/docs/hooks.rst > > +++ b/docs/hooks.rst > > @@ -204,7 +204,8 @@ operation. There is no specific operation to indicate a "restart" is occurring. > > > > - When a QEMU guest is stopped, the qemu hook script is called in two > > locations, to match the startup. First, :since:`since 0.8.0`, the hook is > > - called before libvirt restores any labels: > > + called before libvirt restores any labels, :since:`since 9.10.0`, shutoff > > + reason is delivered with **extra argument**: > > Next release is going to be 10.3.0 > > > > > :: > > > > diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c > > index e4bcb628cf..c42f5c9139 100644 > > --- a/src/qemu/qemu_process.c > > +++ b/src/qemu/qemu_process.c > > @@ -8564,7 +8564,8 @@ void qemuProcessStop(virQEMUDriver *driver, > > /* we can't stop the operation even if the script raised an error */ > > ignore_value(virHookCall(VIR_HOOK_DRIVER_QEMU, vm->def->name, > > VIR_HOOK_QEMU_OP_STOPPED, VIR_HOOK_SUBOP_END, > > - NULL, xml, NULL)); > > + virDomainShutoffReasonTypeToString(reason), > > + xml, NULL)); > > To be honest I'm not exactly sold that this is needed as the reason for > this hook's existence is to simply clear up any resources that would be > set up by the start hook, thus it's usually irrelevant why that > happened. The thing that would convince me is that in outgoing migration there are some resources you might wish to skip cleanup of if they need to be preserved in some manner for the dst host. IIUC we don't currently have a way to indicate that the "stopped" hook is in response to an outgoing migration, and the "reason" would provide this. With that justification though, I would now query the asymmetry of only providing 'reason' to the 'stopped' hook. It should be provided to *all* the hooks - prepare, start, started, stop, stopped, release. With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| _______________________________________________ Devel mailing list -- devel@xxxxxxxxxxxxxxxxx To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx
