On 10/14/26 7:20 AM, Ben Grande wrote:
On 26-10-13 20:34:07, Jim Fehlig wrote:
On 10/13/26 8:15 PM, Jim Fehlig wrote:
On 10/13/26 7:06 AM, Ben Grande wrote:
Fired on "on_reboot" values "restart" and "destroy", so event listeners
can react to this request. The value "destroy" is useful for
applications that have routines run between restarts,

It might be useful for your application, but IMO could be very confusing
to others. More below...

therefore the
reboot event is still fired in this case. If the value is set to
"preserve" then it doesn't seem appropriate to fire reboot event.

Resolves: https://github.com/QubesOS/qubes-issues/issues/8196
Signed-off-by: Ben Grande <[email protected]>
---
   docs/formatdomain.rst    | 14 +++++++++++---
   src/libvirt-domain.c     |  4 +++-
   src/libxl/libxl_domain.c |  4 ++++
   3 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index e4eb2d9ba3..a5388c5d11 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -2047,9 +2047,17 @@ Each of these states allow for the same four
possible actions.
      The domain will be terminated and then restarted with a new name. (Only
      supported by the libxl hypervisor driver.)
-QEMU/KVM/HVF supports the ``on_poweroff`` and ``on_reboot`` events handling the
-``destroy`` and ``restart`` actions, but the combination of ``on_poweroff`` set
-to ``restart`` and ``on_reboot`` set to ``destroy`` is forbidden.
+QEMU/KVM/HVF/XEN supports the ``on_poweroff`` and ``on_reboot`` events handling
+the ``destroy`` and ``restart`` actions, in which case, a notification is
+available when the reboot fires, using the event ID
+``VIR_DOMAIN_EVENT_ID_REBOOT``.

This sentence is a bit unclear IMO. It groups on_poweroff + on_reboot
together, then says "notification is available when the reboot fires".
I'd suggest two sentences, with the second one explicitly scoped. E.g.

QEMU/KVM/HVF/XEN supports the on_poweroff and on_reboot events handling
the destroy and restart actions. For on_reboot, a notification is
available via VIR_DOMAIN_EVENT_ID_REBOOT when the guest-initiated reboot
is detected.

+
+Setting ``on_reboot`` to ``destroy``, prevents libvirt from restarting the
+domain on its own, thus allowing a VMM that monitors
+``VIR_DOMAIN_EVENT_ID_REBOOT``, to run their own stop and start procedures.

This is an unrelated doc improvement and should be a separate patch IMO.

+
+The combination of ``on_poweroff`` set to ``restart`` and ``on_reboot`` set to
+``destroy`` is forbidden.
   The ``on_crash`` event supports these additional actions :since:`since 
0.8.4`.
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index acd9ffc44b..95764ebd2b 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -1779,7 +1779,9 @@ virDomainShutdownFlags(virDomainPtr domain,
unsigned int flags)
    * Note that the guest OS may ignore the request.
    * Additionally, the hypervisor may check and support the domain
    * 'on_reboot' XML setting resulting in a domain that shuts down instead
- * of rebooting.
+ * of rebooting. Depending on the hypervisor and 'on_reboot' setting,
+ * VIR_DOMAIN_EVENT_ID_REBOOT might be emitted. For more information, see
+ * https://libvirt.org/formatdomain.html#events-configuration
    *
    * If @flags is set to zero, then the hypervisor will choose the
    * method of shutdown it considers best. To have greater control
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index 51dc3edcc2..ab9df080ac 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -550,10 +550,14 @@ libxlDomainShutdownThread(void *opaque)
                                              
VIR_DOMAIN_EVENT_STOPPED_SHUTDOWN);
           switch ((virDomainLifecycleAction) vm->def->onReboot) {
           case VIR_DOMAIN_LIFECYCLE_ACTION_DESTROY:
+            virObjectEventStateQueue(driver->domainEventState,
+                                     virDomainEventRebootNewFromObj(vm));

This seems misleading to me. An app monitoring these events will receive
a REBOOT event, when in fact the domain will be destroyed, right?
AFAICT, in this case the qemu driver emits a SHUTDOWN event with detail
GUEST

Didn't post before but previous discussion about either firing the event
or sending the reboot reason on shutdown:

https://lists.libvirt.org/archives/list/[email protected]/thread/ZB5ALEFMTONTYZMJ6R2H6JWTOFL4ETER/

Ah, it would have been helpful to reference this in the cover letter :-).


So yes, I can deal with using a shutdown reason.

https://gitlab.com/libvirt/libvirt/-/blob/master/src/qemu/qemu_process.c?
ref_type=heads#L775

After sending, I noticed the following proposed patch adds a new SHUTDOWN
event detail 'VIR_DOMAIN_EVENT_SHUTDOWN_GUEST_RESET'

https://lists.libvirt.org/archives/list/[email protected]/thread/NFMQ3N5CITIVWK3425N72GUSVW33ARFF/

Would it be acceptable to create the event detail
`VIR_DOMAIN_EVENT_STOPPED_RESET`? Only VMMs that use
`VIR_DOMAIN_EVENT_SHUTDOWN` are 'qemu' and 'ch'.

Yes, that would be fine too IMO. VIR_DOMAIN_EVENT_STOPPED is used by bhyve, ch, libxl, lxc, qemu and vz. A new VIR_DOMAIN_EVENT_STOPPED_RESET detail would ride on a universally-emitted event, whereas a new SHUTDOWN reason means libxl must adopt a lifecycle event type it's never handled before. Peter, do you have an opinion on this?

BTW, to bikeshed a bit, perhaps VIR_DOMAIN_EVENT_STOPPED_REBOOT is a better name for the new detail? The related config knob is called 'on_reboot'. And REBOOT might avoid the (remote) possibility of falsely associating it with QEMU's monitor RESET event.

Regards,
Jim

Reply via email to