Fix: Graceful QEMU shutdown escalation to prevent disk corruption (#925)

* Fix: Graceful QEMU shutdown escalation to prevent disk corruption

QemuVM.stop() previously sent an ACPI powerdown and returned immediately,
leaving the 30s systemd SIGKILL as the only fallback. A SIGKILL terminates
QEMU without flushing disk caches, which can corrupt qcow2 metadata and
guest filesystems (e.g. missing kernel files after an in-guest apt upgrade).

The new shutdown sequence:
  t=0s   ACPI system_powerdown (guest handles clean shutdown)
  t=50s  QMP "quit" (QEMU flushes block device caches and exits)
  t=60s  systemd SIGKILL (last resort)

* qemu: wait for process exit after QMP quit before returning from stop()

---------

Co-authored-by: Ali EL BROUDI <ali.elb@gmail.com>

Author: odesenfans

packaging/aleph-vm/etc/systemd/system/aleph-vm-controller@.service

@@ -12,11 +12,11 @@ Environment=PYTHONPATH=/opt/aleph-vm/:$PYTHONPATH
 ExecStart=/usr/bin/python3 -m aleph.vm.controllers --config=/var/lib/aleph/vm/%i-controller.json
 Restart=on-failure
 # KillMode=Mixed is used so initially only the Python controller process receives the SIGTERM signal.
-# The controller catches it and sends a QEMU command to shut down the Guest VM, allowing it to clean up
-# properly and avoid disk corruption.
-# After 30s (TimeoutStopSec), if the process is still running, both the controller and subprocesses receive SIGKILL.
+# The controller catches it and sends an ACPI powerdown to the Guest VM, giving it time to shut down
+# cleanly. If the guest does not respond within 50s, the controller sends QMP "quit" which makes
+# QEMU flush its disk caches before exiting. The 60s SIGKILL is a last resort to avoid hung processes.
 KillMode=mixed
-TimeoutStopSec=30
+TimeoutStopSec=60
 
 [Install]
 WantedBy=multi-user.target

src/aleph/vm/hypervisors/qemu/qemuvm.py

@@ -12,6 +12,11 @@
 
 logger = logging.getLogger(__name__)
 
+# Seconds to wait for guest ACPI shutdown before escalating to QMP quit.
+# Must be shorter than systemd TimeoutStopSec (60s) to leave time for
+# QEMU to flush disk caches via "quit" before the SIGKILL deadline.
+GRACEFUL_SHUTDOWN_TIMEOUT = 50
+
 
 @dataclass
 class HostVolume:
@@ -229,11 +234,64 @@ def send_shutdown_message(self):
             logger.info("Shutdown message sent to %s", self.vm_hash)
             client.close()
 
-    async def stop(self):
-        """Stop the VM."""
-        self.send_shutdown_message()
+    def _send_qmp_quit(self):
+        """Tell QEMU to exit cleanly, flushing disk caches before terminating."""
+        client = self._get_qmpclient()
+        if client:
+            try:
+                client.command("quit")
+                logger.info("Sent QMP quit to VM %s", self.vm_hash)
+            except Exception:
+                logger.warning("QMP quit failed for VM %s", self.vm_hash, exc_info=True)
+            finally:
+                client.close()
 
+    def _close_journals(self):
         if self.journal_stdout and self.journal_stdout != asyncio.subprocess.DEVNULL:
             self.journal_stdout.close()
         if self.journal_stderr and self.journal_stderr != asyncio.subprocess.DEVNULL:
             self.journal_stderr.close()
+
+    async def stop(self):
+        """Stop the VM with graceful shutdown escalation.
+
+        Sends an ACPI powerdown and waits for the guest to shut down cleanly.
+        If the guest does not stop in time, sends QMP "quit" which makes QEMU
+        flush its disk caches and exit — avoiding the qcow2 corruption that
+        a SIGKILL would cause.
+        """
+        self.send_shutdown_message()
+
+        if self.qemu_process:
+            try:
+                await asyncio.wait_for(self.qemu_process.wait(), timeout=GRACEFUL_SHUTDOWN_TIMEOUT)
+                logger.info("VM %s shut down gracefully after ACPI powerdown", self.vm_hash)
+                self._close_journals()
+                return
+            except asyncio.TimeoutError:
+                logger.warning(
+                    "VM %s did not shut down within %ds, sending QMP quit",
+                    self.vm_hash,
+                    GRACEFUL_SHUTDOWN_TIMEOUT,
+                )
+
+        # Guest didn't respond to ACPI — tell QEMU to exit cleanly.
+        # Unlike SIGKILL, "quit" flushes block device caches first.
+        self._send_qmp_quit()
+
+        # Wait for QEMU to finish flushing and exit before returning,
+        # so callers that tear down network/tap/nftables after stop()
+        # don't race with a still-running qemu process.
+        if self.qemu_process:
+            remaining = 60 - GRACEFUL_SHUTDOWN_TIMEOUT
+            try:
+                await asyncio.wait_for(self.qemu_process.wait(), timeout=remaining)
+                logger.info("VM %s exited after QMP quit", self.vm_hash)
+            except asyncio.TimeoutError:
+                logger.warning(
+                    "VM %s still running %ds after QMP quit, " "systemd SIGKILL will handle it",
+                    self.vm_hash,
+                    remaining,
+                )
+
+        self._close_journals()