Merge remote-tracking branch 'origin/pr/742'

* origin/pr/742:
  Clean preloaded disposable remnants on next boot

Author: marmarek

qubes/vm/dispvm.py

@@ -433,6 +433,17 @@ async def on_domain_shutdown(self, _event, **_kwargs):
         """Do auto cleanup if enabled"""
         await self._auto_cleanup()
 
+    @qubes.events.handler("domain-remove-from-disk")
+    async def on_domain_delete(self, _event, **_kwargs) -> None:
+        """
+        On volume removal, remove preloaded disposable from ``preload-dispvm``
+        feature in disposable template. If the feature is still here, it means
+        the ``domain-shutdown`` cleanup was bypassed, possibly by improper
+        shutdown, which can happen when a disposable is running, qubesd stops
+        and system reboots.
+        """
+        self._preload_cleanup()
+
     @qubes.events.handler("property-pre-reset:template")
     def on_property_pre_reset_template(self, event, name, oldvalue=None):
         """Forbid deleting template of VM"""  # pylint: disable=unused-argument
@@ -612,18 +623,26 @@ def use_preload(self):
             appvm.fire_event_async("domain-preload-dispvm-used", dispvm=self)
         )
 
+    def _preload_cleanup(self):
+        """Cleanup preload from list"""
+        if self.name in self.template.get_feat_preload():
+            self.log.info("Automatic cleanup removes qube from preload list")
+            self.template.remove_preload_from_list([self.name])
+
     async def _bare_cleanup(self):
         """Cleanup bare DispVM objects."""
         if self in self.app.domains:
             del self.app.domains[self]
             await self.remove_from_disk()
             self.app.save()
 
-    def _preload_cleanup(self):
-        """Cleanup preload from list"""
-        if self.name in self.template.get_feat_preload():
-            self.log.info("Automatic cleanup removes qube from preload list")
-            self.template.remove_preload_from_list([self.name])
+    async def _auto_cleanup(self):
+        """Do auto cleanup if enabled"""
+        if not self.auto_cleanup:
+            return
+        self._preload_cleanup()
+        if self in self.app.domains:
+            await self._bare_cleanup()
 
     async def cleanup(self):
         """Clean up after the DispVM
@@ -633,20 +652,14 @@ async def cleanup(self):
         """
         if self not in self.app.domains:
             return
+        running = True
         try:
             await self.kill()
         except qubes.exc.QubesVMNotStartedError:
-            pass
-        # This will be done automatically if event 'domain-shutdown' is
-        # triggered and 'auto_cleanup' evaluates to 'True'.
-        if not self.auto_cleanup:
-            self._preload_cleanup()
-            if self in self.app.domains:
-                await self._bare_cleanup()
-
-    async def _auto_cleanup(self):
-        """Do auto cleanup if enabled"""
-        if self.auto_cleanup:
+            running = False
+        # Full cleanup will be done automatically if event 'domain-shutdown' is
+        # triggered and "auto_cleanup=True".
+        if not running or not self.auto_cleanup:
             self._preload_cleanup()
             if self in self.app.domains:
                 await self._bare_cleanup()

qubes/vm/mix/dvmtemplate.py

@@ -68,10 +68,21 @@ def on_domain_loaded(self, event):  # pylint: disable=unused-argument
 
         # Preloading was in progress (either preloading but not completed or
         # requested but not delivered) and qubesd stopped.
+        #
+        # Or qubesd stopped and the qube was destroyed/killed in the meantime,
+        # shutdown was not called by qubesd so the qube is still present. The
+        # "preload-dispvm-completed" is used to check if this was a preloaded
+        # qube instead of "is_preload()" because it might not be in the
+        # "preload-dispvm" list anymore if the following happened: "removed
+        # from list -> scheduled cleanup -> stopped qubesd".
         preload_in_progress = [
             qube
             for qube in self.dispvms
-            if qube.features.get("preload-dispvm-in-progress", False)
+            if (
+                not qube.is_running()
+                and qube.features.get("preload-dispvm-completed", False)
+            )
+            or qube.features.get("preload-dispvm-in-progress", False)
         ]
         if preload_in_progress:
             changes = True
@@ -84,6 +95,7 @@ def on_domain_loaded(self, event):  # pylint: disable=unused-argument
             )
             for dispvm in preload_in_progress:
                 asyncio.ensure_future(dispvm.cleanup())
+
         if changes:
             self.app.save()
 

qubes/vm/qubesvm.py

@@ -348,7 +348,7 @@ class QubesVM(qubes.vm.mix.net.NetVMMixin, qubes.vm.LocalVM):
             Fired after the qube was loaded from :file:`qubes.xml`
 
             :param subject: Event emitter (the qube object)
-            :param event: Event name (``'domain-loaded'``)
+            :param event: Event name (``'domain-load'``)
 
         .. event:: domain-pre-start \
                 (subject, event, start_guid, mem_required)