Try to get the tests to work

They fail because the root volume is not actually updated.

qubes/tests/vm/dispvm.py

@@ -113,19 +113,19 @@ def test_002_template_change(self):
                 'get_new_unused_dispid': mock.Mock(return_value=42),
                 '__getitem__.side_effect': orig_getitem
             })
-            dispvm = self.app.add_new_vm(qubes.vm.dispvm.DispVM,
+            self.dispvm = self.app.add_new_vm(qubes.vm.dispvm.DispVM,
                 name='test-dispvm', template=self.appvm)
 
-            dispvm.template = self.appvm
-            dispvm.start()
+            self.dispvm.template = self.appvm
+            self.dispvm.start()
             if not self.app.vmm.offline_mode:
                 assert not dispvm.is_halted()
                 with self.assertRaises(qubes.exc.QubesVMNotHaltedError):
-                    dispvm.template = self.appvm
+                    self.dispvm.template = self.appvm
             with self.assertRaises(qubes.exc.QubesValueError):
-                dispvm.template = qubes.property.DEFAULT
-            dispvm.kill()
-            dispvm.template = self.appvm
+                self.dispvm.template = qubes.property.DEFAULT
+            self.dispvm.kill()
+            self.dispvm.template = self.appvm
 
     def test_003_dvmtemplate_template_change(self):
         self.appvm.template_for_dispvms = True
@@ -141,8 +141,7 @@ def test_003_dvmtemplate_template_change(self):
             self.dispvm = self.app.add_new_vm(qubes.vm.dispvm.DispVM,
                 name='test-dispvm', template=self.appvm)
 
-            with self.assertRaises(qubes.exc.QubesVMNotHaltedError):
-                self.appvm.template = self.template
+            self.appvm.template = self.template
             with self.assertRaises(qubes.exc.QubesValueError):
                 self.appvm.template = qubes.property.DEFAULT
 
@@ -172,9 +171,10 @@ def test_010_create_direct(self):
                 'get_new_unused_dispid': mock.Mock(return_value=42),
                 '__getitem__.side_effect': orig_getitem
             })
-            dispvm = self.app.add_new_vm(qubes.vm.dispvm.DispVM,
+            self.dispvm = self.app.add_new_vm(qubes.vm.dispvm.DispVM,
                 name='test-dispvm', template=self.appvm)
             mock_domains.get_new_unused_dispid.assert_called_once_with()
+        dispvm=self.dispvm
         self.assertEqual(dispvm.name, 'test-dispvm')
         self.assertEqual(dispvm.template, self.appvm)
         self.assertEqual(dispvm.label, self.appvm.label)
@@ -235,3 +235,80 @@ def test_020_copy_storage_pool(self, mock_makedirs, mock_symlink):
                          self.appvm.volumes['root'].pool)
         self.assertEqual(dispvm.volumes['volatile'].pool,
                          self.appvm.volumes['volatile'].pool)
+
+    def test_021_storage_template_change(self):
+        self.appvm.template_for_dispvms = True
+        orig_domains = self.app.domains
+        with mock.patch.object(self.app, 'domains', wraps=self.app.domains) \
+                as mock_domains:
+            mock_domains.configure_mock(**{
+                'get_new_unused_dispid': mock.Mock(return_value=42),
+                '__getitem__.side_effect': orig_domains.__getitem__,
+                '__iter__.side_effect': orig_domains.__iter__,
+                '__setitem__.side_effect': orig_domains.__setitem__,
+            })
+            vm = self.dispvm = self.app.add_new_vm(qubes.vm.dispvm.DispVM,
+                name='test-dispvm', template=self.appvm)
+            self.loop.run_until_complete(vm.create_on_disk())
+            # create new mock, so new template will get different volumes
+            self.app.pools['default'] = mock.Mock(**{
+                'init_volume.return_value.pool': 'default'})
+            template2 = qubes.vm.templatevm.TemplateVM(self.app, None,
+                qid=3, name=qubes.tests.VMPREFIX + 'template2')
+            self.app.domains[template2.name] = template2
+            self.app.domains[template2] = template2
+            self.appvm.template = template2
+
+        self.assertFalse(vm.volume_config['root']['save_on_stop'])
+        self.assertTrue(vm.volume_config['root']['snap_on_start'])
+        self.assertNotEqual(vm.volume_config['root'].get('source', None),
+            self.template.volumes['root'].source)
+        self.assertIs(template2, self.app.domains[template2.name])
+        self.assertEqual(vm.volume_config['root'].get('source', None),
+            template2.volumes['root'].source)
+
+    def test_022_storage_app_change(self):
+        self.appvm.template_for_dispvms = True
+        self.assertTrue(self.appvm.events_enabled)
+        orig_domains = self.app.domains
+        with mock.patch.object(self.app, 'domains', wraps=self.app.domains) \
+                as mock_domains:
+            mock_domains.configure_mock(**{
+                'get_new_unused_dispid': mock.Mock(return_value=42),
+                '__getitem__.side_effect': orig_domains.__getitem__,
+                '__iter__.side_effect': orig_domains.__iter__,
+                '__setitem__.side_effect': orig_domains.__setitem__,
+            })
+            vm = self.dispvm = self.app.add_new_vm(qubes.vm.dispvm.DispVM,
+                name='test-dispvm', template=self.appvm)
+            self.assertTrue(vm.events_enabled)
+            self.loop.run_until_complete(self.dispvm.create_on_disk())
+            # create new mock, so new template will get different volumes
+            self.app.pools['default'] = mock.Mock(**{
+                'init_volume.return_value.pool': 'default'})
+            template2 = qubes.vm.templatevm.TemplateVM(self.app, None,
+                qid=3, name=qubes.tests.VMPREFIX + 'template2')
+            self.assertTrue(template2.events_enabled)
+            self.app.domains[template2.name] = template2
+            self.app.domains[template2] = template2
+            app2 = qubes.vm.appvm.AppVM(self.app, None,
+                qid=4, name=qubes.tests.VMPREFIX + 'app2', template=template2)
+            self.assertTrue(app2.events_enabled)
+            app2.template_for_dispvms = True
+            self.app.domains[app2.name] = app2
+            self.app.domains[app2] = app2
+            self.dispvm.template = app2
+
+        self.assertIs(vm, self.dispvm)
+        self.assertFalse(vm.volume_config['root']['save_on_stop'])
+        self.assertTrue(vm.volume_config['root']['snap_on_start'])
+        self.assertNotEqual(vm.volume_config['root'].get('source', None),
+            self.template.volumes['root'].source)
+        self.assertNotEqual(vm.volume_config['root'].get('source', None),
+            self.appvm.volumes['root'].source)
+        self.assertNotEqual(vm.volume_config['private'].get('source', None),
+            self.appvm.volumes['private'].source)
+        self.assertEqual(vm.volume_config['root'].get('source', None),
+            app2.volumes['root'])
+        self.assertEqual(vm.volume_config['private'].get('source', None),
+            app2.volumes['private'])

qubes/vm/appvm.py

@@ -27,9 +27,9 @@
 import qubes.vm.mix.dvmtemplate
 from qubes.config import defaults
 
-def template_changed_update_storage(self, volume_config):
+def template_changed_update_storage(self):
     '''Update storage configuration for TemplateVM changes'''
-    for volume_name, conf in volume_config.items():
+    for volume_name, conf in self.volume_config.items():
         if conf.get('snap_on_start', False) and \
                 conf.get('source', None) is None:
             config = conf.copy()
@@ -128,4 +128,6 @@ def on_property_set_template(self, event, name, newvalue, oldvalue=None):
         ''' Adjust root (and possibly other snap_on_start=True) volume
         on template change.
         '''  # pylint: disable=unused-argument
-        template_changed_update_storage(self, self.default_volume_config)
+        template_changed_update_storage(self)
+        for vm in self.dispvms:
+            vm.on_property_set_template(event, name, newvalue, oldvalue)

qubes/vm/dispvm.py

@@ -65,28 +65,28 @@ def __init__(self, app, xml, *args, **kwargs):
                 'save_on_stop': False,
                 'rw': True,
                 'source': None,
-            },
+            }.copy(),
             'private': {
                 'name': 'private',
                 'snap_on_start': True,
                 'save_on_stop': False,
                 'rw': True,
                 'source': None,
-            },
+            }.copy(),
             'volatile': {
                 'name': 'volatile',
                 'snap_on_start': False,
                 'save_on_stop': False,
                 'rw': True,
                 'size': qubes.config.defaults['root_img_size'] +
                         qubes.config.defaults['private_img_size'],
-            },
+            }.copy(),
             'kernel': {
                 'name': 'kernel',
                 'snap_on_start': False,
                 'save_on_stop': False,
                 'rw': False,
-            }
+            }.copy()
         }
 
         template = kwargs.get('template', None)
@@ -165,7 +165,13 @@ def on_property_set_template(self, event, name, newvalue, oldvalue=None):
         ''' Adjust root (and possibly other snap_on_start=True) volume
         on template change.
         '''  # pylint: disable=unused-argument
-        qubes.vm.appvm.template_changed_update_storage(self, self.volume_config)
+        for volume_name, conf in self.volume_config.items():
+            if conf.get('snap_on_start', False) and \
+                    conf.get('source', None) is None:
+                config = conf.copy()
+                self.volume_config[volume_name] = config
+                self.storage.init_volume(volume_name, config)
+            qubes.vm.appvm.template_changed_update_storage(self)
 
     @qubes.events.handler('domain-shutdown')
     @asyncio.coroutine

qubes/vm/mix/dvmtemplate.py

@@ -50,17 +50,18 @@ def __on_pre_property_set_template(self, event, name, newvalue,
             oldvalue=None):
         # pylint: disable=unused-argument
         for vm in self.dispvms:
-            if vm.is_running:
+            running = vm.is_running()
+            assert type(running) is bool
+            if running:
                 raise qubes.exc.QubesVMNotHaltedError(self,
-                    'Cannot change template while there are running DispVMs'
+                    'Cannot change template while there are running DispVMs '
                     'based on this DVM template')
 
     @qubes.events.handler('property-set:template')
     def __on_property_set_template(self, event, name, newvalue,
             oldvalue=None):
         # pylint: disable=unused-argument
-        for vm in self.dispvms:
-            vm.on_property_set_template(event, name, newvalue, oldvalue)
+        pass
 
     @property
     def dispvms(self):

run-tests

@@ -1,7 +1,7 @@
 #!/bin/sh --
 
 set -eu
-sudo dnf -y install lvm2 python3-inotify python3-sphinx python3-docutils python3-PyYAML python3-jinja2 python3-lxml python3-pylint python3-coverage
+sudo dnf -y install lvm2 python3-inotify python3-sphinx python3-docutils python3-PyYAML python3-jinja2 python3-lxml python3-pylint python3-coverage btrfs-progs vim-common
 CLEANUP_LVM=
 if sudo --non-interactive $(dirname "$0")/ci/lvm-manage setup-lvm vg$$/pool; then
     export DEFAULT_LVM_POOL=vg$$/pool
@@ -12,7 +12,7 @@ fi
 : "${TESTPYTHONPATH:=test-packages}"
 
 if [ -d ../core-qrexec/qrexec ] && ! $PYTHON -c 'import qrexec' 2>/dev/null; then
-    PYTHONPATH="${PYTHONPATH}:../core-qrexec"
+    PYTHONPATH="${PYTHONPATH-}:../core-qrexec"
 fi
 
 PYTHONPATH="${TESTPYTHONPATH}:${PYTHONPATH}"