From 691a6f4d8cdbdae11e624a6a1d479b0b1847aa36 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Sun, 6 Aug 2017 12:27:35 +0200 Subject: [PATCH] vm/dispvm: add auto_cleanup property, unify creating new DispVM Add auto_cleanup property, which remove DispVM after its shutdown - this is to unify DispVM handling - less places needing special handling after DispVM shutdown. New DispVM inherit all settings from respective AppVM. Move this from classmethod `DispVM.from_appvm()`, to DispVM constructor. This unify creating new DispVM with any other VM class. Notable exception are attached devices - because only one running VM can have a device attached, this would prevent second DispVM started from the same AppVM. If one need DispVM with some device attached, one can create DispVM with auto_cleanup=False. Such DispVM will still not have persistent storage (as any other DispVM). Tests included. QubesOS/qubes-issues#2974 --- qubes/tests/__init__.py | 1 + qubes/tests/vm/dispvm.py | 95 ++++++++++++++++++++++++++++++++++++++++ qubes/vm/dispvm.py | 65 ++++++++++++++++++++------- rpm_spec/core-dom0.spec | 1 + 4 files changed, 145 insertions(+), 17 deletions(-) create mode 100644 qubes/tests/vm/dispvm.py diff --git a/qubes/tests/__init__.py b/qubes/tests/__init__.py index 48b7a9d5..80b18eba 100644 --- a/qubes/tests/__init__.py +++ b/qubes/tests/__init__.py @@ -1003,6 +1003,7 @@ def load_tests(loader, tests, pattern): # pylint: disable=unused-argument 'qubes.tests.vm.mix.net', 'qubes.tests.vm.adminvm', 'qubes.tests.vm.appvm', + 'qubes.tests.vm.dispvm', 'qubes.tests.app', 'qubes.tests.tarwriter', 'qubes.tests.api', diff --git a/qubes/tests/vm/dispvm.py b/qubes/tests/vm/dispvm.py new file mode 100644 index 00000000..c0bb1ed7 --- /dev/null +++ b/qubes/tests/vm/dispvm.py @@ -0,0 +1,95 @@ +# -*- encoding: utf-8 -*- +# +# The Qubes OS Project, http://www.qubes-os.org +# +# Copyright (C) 2017 Marek Marczykowski-Górecki +# +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License along +# with this program; if not, see . + +import unittest.mock as mock + +import asyncio + +import qubes.vm.dispvm +import qubes.vm.appvm +import qubes.vm.templatevm +import qubes.tests +import qubes.tests.vm +import qubes.tests.vm.appvm + +class TestApp(qubes.tests.vm.TestApp): + def __init__(self): + super(TestApp, self).__init__() + self.qid_counter = 1 + + def add_new_vm(self, cls, **kwargs): + qid = self.qid_counter + self.qid_counter += 1 + vm = cls(self, None, qid=qid, **kwargs) + self.domains[vm.name] = vm + self.domains[vm] = vm + return vm + +class TC_00_DispVM(qubes.tests.QubesTestCase): + def setUp(self): + super(TC_00_DispVM, self).setUp() + self.app = TestApp() + self.app.save = mock.Mock() + self.app.pools['default'] = qubes.tests.vm.appvm.TestPool('default') + self.app.pools['linux-kernel'] = mock.Mock(**{ + 'init_volume.return_value.pool': 'linux-kernel'}) + self.app.vmm.offline_mode = True + self.template = self.app.add_new_vm(qubes.vm.templatevm.TemplateVM, + name='test-template', label='red') + self.appvm = self.app.add_new_vm(qubes.vm.appvm.AppVM, + name='test-vm', template=self.template, label='red') + self.app.domains[self.appvm.name] = self.appvm + self.app.domains[self.appvm] = self.appvm + + @asyncio.coroutine + def mock_coro(self, *args, **kwargs): + pass + + @mock.patch('os.symlink') + @mock.patch('os.makedirs') + @mock.patch('qubes.storage.Storage') + def test_000_from_appvm(self, mock_storage, mock_makedirs, mock_symlink): + mock_storage.return_value.create.side_effect = self.mock_coro + self.appvm.dispvm_allowed = True + orig_getitem = self.app.domains.__getitem__ + 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_getitem + }) + dispvm = self.loop.run_until_complete( + qubes.vm.dispvm.DispVM.from_appvm(self.appvm)) + mock_domains.get_new_unused_dispid.assert_called_once_with() + self.assertTrue(dispvm.name.startswith('disp')) + self.assertEqual(dispvm.template, self.appvm) + self.assertEqual(dispvm.label, self.appvm.label) + self.assertEqual(dispvm.label, self.appvm.label) + self.assertEqual(dispvm.auto_cleanup, True) + mock_makedirs.assert_called_once_with( + '/var/lib/qubes/appvms/' + dispvm.name, mode=0o775) + mock_symlink.assert_called_once_with( + '/usr/share/icons/hicolor/128x128/devices/appvm-red.png', + '/var/lib/qubes/appvms/{}/icon.png'.format(dispvm.name)) + + def test_001_from_appvm_reject_not_allowed(self): + with self.assertRaises(qubes.exc.QubesException): + dispvm = self.loop.run_until_complete( + qubes.vm.dispvm.DispVM.from_appvm(self.appvm)) diff --git a/qubes/vm/dispvm.py b/qubes/vm/dispvm.py index b9f6bf39..0e0285a2 100644 --- a/qubes/vm/dispvm.py +++ b/qubes/vm/dispvm.py @@ -39,7 +39,10 @@ class DispVM(qubes.vm.qubesvm.QubesVM): clone=False, doc='''Internal, persistent identifier of particular DispVM.''') - def __init__(self, *args, **kwargs): + auto_cleanup = qubes.property('auto_cleanup', type=bool, default=False, + doc='automatically remove this VM upon shutdown') + + def __init__(self, app, xml, *args, **kwargs): self.volume_config = { 'root': { 'name': 'root', @@ -70,10 +73,26 @@ class DispVM(qubes.vm.qubesvm.QubesVM): 'rw': False, } } - if 'name' not in kwargs and 'dispid' in kwargs: - kwargs['name'] = 'disp' + str(kwargs['dispid']) + template = kwargs.get('template', None) + if xml is None: + if 'dispid' not in kwargs: + kwargs['dispid'] = app.domains.get_new_unused_dispid() + if 'name' not in kwargs: + kwargs['name'] = 'disp' + str(kwargs['dispid']) + + # by default inherit properties from the DispVM template + proplist = [prop.__name__ for prop in template.property_list() + if prop.clone and prop.__name__ not in ['template']] + self_props = [prop.__name__ for prop in self.property_list()] + for prop in proplist: + if prop not in self_props: + continue + if prop not in kwargs and \ + not template.property_is_default(prop): + kwargs[prop] = getattr(template, prop) + if template is not None: # template is only passed if the AppVM is created, in other cases we # don't need to patch the volume_config because the config is @@ -86,11 +105,12 @@ class DispVM(qubes.vm.qubesvm.QubesVM): if 'vid' in self.volume_config[name]: del self.volume_config[name]['vid'] - # by default inherit label from the DispVM template - if 'label' not in kwargs: - kwargs['label'] = template.label + super(DispVM, self).__init__(app, xml, *args, **kwargs) - super(DispVM, self).__init__(*args, **kwargs) + if xml is None: + self.firewall.clone(template.firewall) + self.features.update(template.features) + self.tags.update(template.tags) @qubes.events.handler('domain-load') def on_domain_loaded(self, event): @@ -106,6 +126,19 @@ class DispVM(qubes.vm.qubesvm.QubesVM): raise qubes.exc.QubesValueError(self, 'Cannot change template of Disposable VM') + @asyncio.coroutine + def on_domain_shutdown_coro(self): + '''Coroutine for executing cleanup after domain shutdown. + + This override default action defined in QubesVM.on_domain_shutdown_coro + ''' + with (yield from self.startup_lock): + yield from self.storage.stop() + if self.auto_cleanup: + yield from self.remove_from_disk() + del self.app.domains[self] + self.app.save() + @classmethod @asyncio.coroutine def from_appvm(cls, appvm, **kwargs): @@ -127,18 +160,14 @@ class DispVM(qubes.vm.qubesvm.QubesVM): ''' if not appvm.dispvm_allowed: raise qubes.exc.QubesException( - 'Refusing to start DispVM out of this AppVM, because ' + 'Refusing to create DispVM out of this AppVM, because ' 'dispvm_allowed=False') app = appvm.app dispvm = app.add_new_vm( cls, - dispid=app.domains.get_new_unused_dispid(), - template=app.domains[appvm], + template=appvm, + auto_cleanup=True, **kwargs) - # exclude template - proplist = [prop for prop in dispvm.property_list() - if prop.clone and prop.__name__ not in ['template']] - dispvm.clone_properties(app.domains[appvm], proplist=proplist) yield from dispvm.create_on_disk() app.save() return dispvm @@ -155,6 +184,8 @@ class DispVM(qubes.vm.qubesvm.QubesVM): yield from self.kill() except qubes.exc.QubesVMNotStartedError: pass - yield from self.remove_from_disk() - del self.app.domains[self] - self.app.save() + # if auto_cleanup is set, this will be done automatically + if not self.auto_cleanup: + yield from self.remove_from_disk() + del self.app.domains[self] + self.app.save() diff --git a/rpm_spec/core-dom0.spec b/rpm_spec/core-dom0.spec index 35b5b8c4..a80857f2 100644 --- a/rpm_spec/core-dom0.spec +++ b/rpm_spec/core-dom0.spec @@ -326,6 +326,7 @@ fi %{python3_sitelib}/qubes/tests/vm/init.py %{python3_sitelib}/qubes/tests/vm/adminvm.py %{python3_sitelib}/qubes/tests/vm/appvm.py +%{python3_sitelib}/qubes/tests/vm/dispvm.py %{python3_sitelib}/qubes/tests/vm/qubesvm.py %dir %{python3_sitelib}/qubes/tests/vm/mix