From 08ddeee9fbb55768fe930d22947991fce60472fa Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?=
 <marmarek@invisiblethingslab.com>
Date: Sun, 21 Oct 2018 05:19:07 +0200
Subject: [PATCH] tests: improve VMs cleanup wrt custom templates

Cleanup VMs in template reverse topological order, not network one.
Network can be set to None to break dependency, but template can't. For
netvm to be changed, kill VMs first (kill doesn't check network
dependency), so netvm change will not trigger side effects (runtime
change, which could fail).

This fixes cleanup for tests creating custom templates - previously
order was undefined and if template was tried removed before its child
VMs, it fails. All the relevant files were removed later anyway, but it
lead to python objects leaks.
---
 qubes/tests/__init__.py | 33 ++++++++++++++++++++++-----------
 1 file changed, 22 insertions(+), 11 deletions(-)

diff --git a/qubes/tests/__init__.py b/qubes/tests/__init__.py
index 2940c7c7..b66a4ee7 100644
--- a/qubes/tests/__init__.py
+++ b/qubes/tests/__init__.py
@@ -789,13 +789,6 @@ class SystemTestCase(QubesTestCase):
         vmname = vm.name
         app = vm.app
 
-        try:
-            # XXX .is_running() may throw libvirtError if undefined
-            if vm.is_running():
-                self.loop.run_until_complete(vm.kill())
-        except:  # pylint: disable=bare-except
-            pass
-
         try:
             self.loop.run_until_complete(vm.remove_from_disk())
         except:  # pylint: disable=bare-except
@@ -876,18 +869,36 @@ class SystemTestCase(QubesTestCase):
         vms = list(vms)
         if not vms:
             return
+        # first kill all the domains, to avoid side effects of changing netvm
+        for vm in vms:
+            try:
+                # XXX .is_running() may throw libvirtError if undefined
+                if vm.is_running():
+                    self.loop.run_until_complete(vm.kill())
+            except:  # pylint: disable=bare-except
+                pass
         # break dependencies
         for vm in vms:
             vm.default_dispvm = None
-        # then remove in reverse topological order (wrt netvm), using naive
+            vm.netvm = None
+        # take app instance from any VM to be removed
+        app = vms[0].app
+        if app.default_dispvm in vms:
+            app.default_dispvm = None
+        if app.default_netvm in vms:
+            app.default_netvm = None
+        del app
+        # then remove in reverse topological order (wrt template), using naive
         # algorithm
-        # this heavily depends on lack of netvm loops
+        # this heavily depends on lack of template loops, but those are
+        # impossible
         while vms:
             vm = vms.pop(0)
             # make sure that all connected VMs are going to be removed,
             # otherwise this will loop forever
-            assert all(x in vms for x in vm.connected_vms)
-            if list(vm.connected_vms):
+            child_vms = list(getattr(vm, 'appvms', []))
+            assert all(x in vms for x in child_vms)
+            if child_vms:
                 # if still something use this VM, put it at the end of queue
                 # and try next one
                 vms.append(vm)