[PATCH] qemu: fix missing audit record and lifecycle event on delayed shutdown

Jim Fehlig via Devel posted 1 patch 3 days, 5 hours ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20260910225250.349387-1-jfehlig@suse.com
src/qemu/qemu_driver.c | 61 +++++++++++++++++++++++++-----------------
1 file changed, 36 insertions(+), 25 deletions(-)
[PATCH] qemu: fix missing audit record and lifecycle event on delayed shutdown
Posted by Jim Fehlig via Devel 3 days, 5 hours ago
From: Jim Fehlig <jfehlig@suse.com>

When a guest shuts down gracefully but its QEMU process takes a long
time to cleanup and exit, virProcessKillPainfullyDelay() (used by
qemuProcessKill()) times out and returns failure. This causes
qemuProcessBeginStopJob() to fail in processMonitorEOFEvent(), which
returns early without ever calling qemuProcessStop(), virDomainAuditStop()
or queuing the VIR_DOMAIN_EVENT_STOPPED lifecycle event.

In this case, qemuProcessKill() falls back to asynchronously monitoring
the QEMU process via a pidfd. Once the process is finally confirmed to
have exited, QEMU_PROCESS_EVENT_SHUTDOWN_COMPLETED is submitted and
handled by processShutdownCompletedEvent(). That function does complete
the stop by calling qemuProcessStop() and qemuDomainRemoveInactive(),
but it never generates the audit record or the lifecycle event, since that
logic lived only in processMonitorEOFEvent(). Management apps relying on
the VIR_DOMAIN_EVENT_STOPPED event, or on the audit log, never learn that
the domain actually stopped.

Fix this by factoring the reason-detection, qemuProcessStop(), audit and
lifecycle event logic out of processMonitorEOFEvent() into a new helper,
qemuProcessFinishStop(), and call it from both processMonitorEOFEvent()
and processShutdownCompletedEvent(). This ensures the audit record and
lifecycle event are always generated once the QEMU process has exited,
regardless of which path detected it.

Signed-off-by: Jim Fehlig <jfehlig@suse.com>
---
 src/qemu/qemu_driver.c | 61 +++++++++++++++++++++++++-----------------
 1 file changed, 36 insertions(+), 25 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 8498568623..8bb037966b 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4166,9 +4166,9 @@ processJobStatusChangeEvent(virDomainObj *vm,
 
 
 static void
-processMonitorEOFEvent(virQEMUDriver *driver,
-                       virDomainObj *vm,
-                       int domid)
+qemuProcessFinishStop(virQEMUDriver *driver,
+                      virDomainObj *vm,
+                      bool migration)
 {
     qemuDomainObjPrivate *priv = vm->privateData;
     int eventReason = VIR_DOMAIN_EVENT_STOPPED_SHUTDOWN;
@@ -4176,27 +4176,9 @@ processMonitorEOFEvent(virQEMUDriver *driver,
     const char *auditReason = "shutdown";
     unsigned int stopFlags = 0;
     virObjectEvent *event = NULL;
-    bool migration;
-
-    if (vm->def->id != domid) {
-        VIR_DEBUG("Domain %s was restarted, ignoring EOF",
-                  vm->def->name);
-        return;
-    }
-
-    if (qemuProcessBeginStopJob(vm, VIR_JOB_DESTROY, true) < 0)
-        return;
-
-    migration = vm->job->asyncJob == VIR_ASYNC_JOB_MIGRATION_IN;
-
-    if (!virDomainObjIsActive(vm)) {
-        VIR_DEBUG("Domain %p '%s' is not active, ignoring EOF",
-                  vm, vm->def->name);
-        goto endjob;
-    }
 
     if (virDomainObjGetState(vm, NULL) != VIR_DOMAIN_SHUTDOWN) {
-        VIR_DEBUG("Monitor connection to '%s' closed without SHUTDOWN event; "
+        VIR_DEBUG("qemu process for '%s' disappeared without SHUTDOWN event; "
                   "assuming the domain crashed", vm->def->name);
         eventReason = VIR_DOMAIN_EVENT_STOPPED_FAILED;
         stopReason = VIR_DOMAIN_SHUTOFF_CRASHED;
@@ -4214,6 +4196,34 @@ processMonitorEOFEvent(virQEMUDriver *driver,
     qemuProcessStop(vm, stopReason, VIR_ASYNC_JOB_NONE, stopFlags);
     virDomainAuditStop(vm, auditReason);
     virObjectEventStateQueue(driver->domainEventState, event);
+}
+
+
+static void
+processMonitorEOFEvent(virQEMUDriver *driver,
+                       virDomainObj *vm,
+                       int domid)
+{
+    bool migration;
+
+    if (vm->def->id != domid) {
+        VIR_DEBUG("Domain %s was restarted, ignoring EOF",
+                  vm->def->name);
+        return;
+    }
+
+    if (qemuProcessBeginStopJob(vm, VIR_JOB_DESTROY, true) < 0)
+        return;
+
+    migration = vm->job->asyncJob == VIR_ASYNC_JOB_MIGRATION_IN;
+
+    if (!virDomainObjIsActive(vm)) {
+        VIR_DEBUG("Domain %p '%s' is not active, ignoring EOF",
+                  vm, vm->def->name);
+        goto endjob;
+    }
+
+    qemuProcessFinishStop(driver, vm, migration);
 
  endjob:
     qemuDomainRemoveInactive(vm, 0, migration);
@@ -4370,13 +4380,14 @@ processNbdkitExitedEvent(virDomainObj *vm,
 
 
 static void
-processShutdownCompletedEvent(virDomainObj *vm)
+processShutdownCompletedEvent(virQEMUDriver *driver,
+                              virDomainObj *vm)
 {
     if (qemuProcessBeginStopJob(vm, VIR_JOB_DESTROY, true) < 0)
         return;
 
     if (virDomainObjIsActive(vm)) {
-        qemuProcessStop(vm, VIR_DOMAIN_SHUTOFF_UNKNOWN, VIR_ASYNC_JOB_NONE, 0);
+        qemuProcessFinishStop(driver, vm, false);
         qemuDomainRemoveInactive(vm, 0, false);
     }
 
@@ -4449,7 +4460,7 @@ qemuProcessEventHandler(void *data,
         processNbdkitExitedEvent(vm, processEvent->data);
         break;
     case QEMU_PROCESS_EVENT_SHUTDOWN_COMPLETED:
-        processShutdownCompletedEvent(vm);
+        processShutdownCompletedEvent(driver, vm);
         break;
     case QEMU_PROCESS_EVENT_LAST:
         break;
-- 
2.51.0