Rename 'dispvm_allowed' to 'template_for_dispvms'
'dispvm_allowed' name was confusing, because it suggested being able to spawn new DispVMs, not being a template for DispVM. Fixes QubesOS/qubes-issues#3047
This commit is contained in:
		
							parent
							
								
									c247ddff72
								
							
						
					
					
						commit
						99bd193688
					
				| @ -46,7 +46,8 @@ class QubesInternalAPI(qubes.api.AbstractQubesAPI): | ||||
|             domain.name: { | ||||
|                 'tags': list(domain.tags), | ||||
|                 'type': domain.__class__.__name__, | ||||
|                 'dispvm_allowed': getattr(domain, 'dispvm_allowed', False), | ||||
|                 'template_for_dispvms': | ||||
|                     getattr(domain, 'template_for_dispvms', False), | ||||
|                 'default_dispvm': (str(domain.default_dispvm) if | ||||
|                     getattr(domain, 'default_dispvm', None) else None), | ||||
|                 'icon': str(domain.label.icon), | ||||
|  | ||||
| @ -2102,7 +2102,7 @@ class TC_00_VMs(AdminAPITestCase): | ||||
|     @unittest.mock.patch('qubes.storage.Storage.create') | ||||
|     def test_640_vm_create_disposable(self, mock_storage): | ||||
|         mock_storage.side_effect = self.dummy_coro | ||||
|         self.vm.dispvm_allowed = True | ||||
|         self.vm.template_for_dispvms = True | ||||
|         retval = self.call_mgmt_func(b'admin.vm.CreateDisposable', | ||||
|                 b'test-vm1') | ||||
|         self.assertTrue(retval.startswith('disp')) | ||||
| @ -2115,7 +2115,7 @@ class TC_00_VMs(AdminAPITestCase): | ||||
|     @unittest.mock.patch('qubes.storage.Storage.create') | ||||
|     def test_641_vm_create_disposable_default(self, mock_storage): | ||||
|         mock_storage.side_effect = self.dummy_coro | ||||
|         self.vm.dispvm_allowed = True | ||||
|         self.vm.template_for_dispvms = True | ||||
|         self.app.default_dispvm = self.vm | ||||
|         retval = self.call_mgmt_func(b'admin.vm.CreateDisposable', | ||||
|                 b'dom0') | ||||
|  | ||||
| @ -20,6 +20,8 @@ | ||||
| 
 | ||||
| from unittest import mock | ||||
| 
 | ||||
| import lxml.etree | ||||
| 
 | ||||
| import qubes.storage | ||||
| import qubes.tests | ||||
| import qubes.tests.vm.qubesvm | ||||
| @ -118,3 +120,26 @@ class TC_90_AppVM(qubes.tests.vm.qubesvm.QubesVMTestsMixin, | ||||
|             mock_power.return_value = 'Running' | ||||
|             with self.assertRaises(qubes.exc.QubesVMNotHaltedError): | ||||
|                 vm.template = self.template | ||||
| 
 | ||||
|     def test_500_property_migrate_template_for_dispvms(self): | ||||
|         xml_template = ''' | ||||
|         <domain class="AppVM" id="domain-1"> | ||||
|             <properties> | ||||
|                 <property name="qid">1</property> | ||||
|                 <property name="name">testvm</property> | ||||
|                 <property name="label" ref="label-1" /> | ||||
|                 <property name="dispvm_allowed">{value}</property> | ||||
|             </properties> | ||||
|         </domain> | ||||
|         ''' | ||||
|         xml = lxml.etree.XML(xml_template.format(value='True')) | ||||
|         vm = qubes.vm.appvm.AppVM(self.app, xml) | ||||
|         self.assertEqual(vm.template_for_dispvms, True) | ||||
|         with self.assertRaises(AttributeError): | ||||
|             vm.dispvm_allowed | ||||
| 
 | ||||
|         xml = lxml.etree.XML(xml_template.format(value='False')) | ||||
|         vm = qubes.vm.appvm.AppVM(self.app, xml) | ||||
|         self.assertEqual(vm.template_for_dispvms, False) | ||||
|         with self.assertRaises(AttributeError): | ||||
|             vm.dispvm_allowed | ||||
|  | ||||
| @ -67,7 +67,7 @@ class TC_00_DispVM(qubes.tests.QubesTestCase): | ||||
|     @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 | ||||
|         self.appvm.template_for_dispvms = True | ||||
|         orig_getitem = self.app.domains.__getitem__ | ||||
|         with mock.patch.object(self.app, 'domains', wraps=self.app.domains) \ | ||||
|                 as mock_domains: | ||||
| @ -78,7 +78,7 @@ class TC_00_DispVM(qubes.tests.QubesTestCase): | ||||
|             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.name, 'disp42') | ||||
|         self.assertEqual(dispvm.template, self.appvm) | ||||
|         self.assertEqual(dispvm.label, self.appvm.label) | ||||
|         self.assertEqual(dispvm.label, self.appvm.label) | ||||
| @ -95,7 +95,7 @@ class TC_00_DispVM(qubes.tests.QubesTestCase): | ||||
|                 qubes.vm.dispvm.DispVM.from_appvm(self.appvm)) | ||||
| 
 | ||||
|     def test_010_create_direct(self): | ||||
|         self.appvm.dispvm_allowed = True | ||||
|         self.appvm.template_for_dispvms = True | ||||
|         orig_getitem = self.app.domains.__getitem__ | ||||
|         with mock.patch.object(self.app, 'domains', wraps=self.app.domains) \ | ||||
|                 as mock_domains: | ||||
| @ -113,7 +113,7 @@ class TC_00_DispVM(qubes.tests.QubesTestCase): | ||||
|         self.assertEqual(dispvm.auto_cleanup, False) | ||||
| 
 | ||||
|     def test_011_create_direct_generate_name(self): | ||||
|         self.appvm.dispvm_allowed = True | ||||
|         self.appvm.template_for_dispvms = True | ||||
|         orig_getitem = self.app.domains.__getitem__ | ||||
|         with mock.patch.object(self.app, 'domains', wraps=self.app.domains) \ | ||||
|                 as mock_domains: | ||||
|  | ||||
| @ -36,7 +36,7 @@ class AppVM(qubes.vm.qubesvm.QubesVM): | ||||
|                                 vmclass=qubes.vm.templatevm.TemplateVM, | ||||
|                                 doc='Template, on which this AppVM is based.') | ||||
| 
 | ||||
|     dispvm_allowed = qubes.property('dispvm_allowed', | ||||
|     template_for_dispvms = qubes.property('template_for_dispvms', | ||||
|         type=bool, | ||||
|         default=False, | ||||
|         doc='Should this VM be allowed to start as Disposable VM') | ||||
| @ -72,6 +72,15 @@ class AppVM(qubes.vm.qubesvm.QubesVM): | ||||
|         } | ||||
| 
 | ||||
|     def __init__(self, app, xml, **kwargs): | ||||
|         # migrate renamed properties | ||||
|         if xml is not None: | ||||
|             node_dispvm_allowed = xml.find( | ||||
|                 './properties/property[@name=\'dispvm_allowed\']') | ||||
|             if node_dispvm_allowed is not None: | ||||
|                 kwargs['template_for_dispvms'] = \ | ||||
|                     qubes.property.bool(None, None, node_dispvm_allowed.text) | ||||
|                 node_dispvm_allowed.getparent().remove(node_dispvm_allowed) | ||||
| 
 | ||||
|         self.volume_config = copy.deepcopy(self.default_volume_config) | ||||
|         template = kwargs.get('template', None) | ||||
| 
 | ||||
|  | ||||
| @ -79,10 +79,10 @@ class DispVM(qubes.vm.qubesvm.QubesVM): | ||||
|         if xml is None: | ||||
|             assert template is not None | ||||
| 
 | ||||
|             if not template.dispvm_allowed: | ||||
|             if not template.template_for_dispvms: | ||||
|                 raise qubes.exc.QubesValueError( | ||||
|                     'template for DispVM ({}) needs to have ' | ||||
|                     'dispvm_allowed=True'.format(template.name)) | ||||
|                     'template_for_dispvms=True'.format(template.name)) | ||||
| 
 | ||||
|             if 'dispid' not in kwargs: | ||||
|                 kwargs['dispid'] = app.domains.get_new_unused_dispid() | ||||
| @ -161,10 +161,10 @@ class DispVM(qubes.vm.qubesvm.QubesVM): | ||||
|         This method modifies :file:`qubes.xml` file. | ||||
|         The qube returned is not started. | ||||
|         ''' | ||||
|         if not appvm.dispvm_allowed: | ||||
|         if not appvm.template_for_dispvms: | ||||
|             raise qubes.exc.QubesException( | ||||
|                 'Refusing to create DispVM out of this AppVM, because ' | ||||
|                 'dispvm_allowed=False') | ||||
|                 'template_for_appvms=False') | ||||
|         app = appvm.app | ||||
|         dispvm = app.add_new_vm( | ||||
|             cls, | ||||
| @ -198,9 +198,9 @@ class DispVM(qubes.vm.qubesvm.QubesVM): | ||||
|         # pylint: disable=arguments-differ | ||||
| 
 | ||||
|         # sanity check, if template_for_dispvm got changed in the meantime | ||||
|         if not self.template.dispvm_allowed: | ||||
|         if not self.template.template_for_dispvms: | ||||
|             raise qubes.exc.QubesException( | ||||
|                 'template for DispVM ({}) needs to have ' | ||||
|                 'dispvm_allowed=True'.format(self.template.name)) | ||||
|                 'template_for_dispvms=True'.format(self.template.name)) | ||||
| 
 | ||||
|         yield from super(DispVM, self).start(**kwargs) | ||||
|  | ||||
| @ -80,7 +80,7 @@ def verify_target_value(system_info, value): | ||||
|         if dispvm_base not in system_info['domains']: | ||||
|             return False | ||||
|         dispvm_base_info = system_info['domains'][dispvm_base] | ||||
|         return bool(dispvm_base_info['dispvm_allowed']) | ||||
|         return bool(dispvm_base_info['template_for_dispvms']) | ||||
|     else: | ||||
|         return value in system_info['domains'] | ||||
| 
 | ||||
| @ -307,13 +307,13 @@ class PolicyRule(object): | ||||
|             for name, domain in system_info['domains'].items(): | ||||
|                 if name != 'dom0': | ||||
|                     yield name | ||||
|                 if domain['dispvm_allowed']: | ||||
|                 if domain['template_for_dispvms']: | ||||
|                     yield '$dispvm:' + name | ||||
|             yield '$dispvm' | ||||
|         elif self.target.startswith('$dispvm:'): | ||||
|             dispvm_base = self.target.split(':', 1)[1] | ||||
|             try: | ||||
|                 if system_info['domains'][dispvm_base]['dispvm_allowed']: | ||||
|                 if system_info['domains'][dispvm_base]['template_for_dispvms']: | ||||
|                     yield self.target | ||||
|             except KeyError: | ||||
|                 # TODO log a warning? | ||||
| @ -685,7 +685,7 @@ def get_system_info(): | ||||
|        - `<domain name>`: | ||||
|           - tags: list of tags | ||||
|           - type: domain type | ||||
|           - dispvm_allowed: should DispVM based on this VM be allowed | ||||
|           - template_for_dispvms: should DispVM based on this VM be allowed | ||||
|           - default_dispvm: name of default AppVM for DispVMs started from here | ||||
| 
 | ||||
|     ''' | ||||
|  | ||||
| @ -115,7 +115,8 @@ def main(args=None): | ||||
|             icons = {name: system_info['domains'][name]['icon'] | ||||
|                 for name in system_info['domains'].keys()} | ||||
|             for dispvm_base in system_info['domains']: | ||||
|                 if not system_info['domains'][dispvm_base]['dispvm_allowed']: | ||||
|                 if not (system_info['domains'][dispvm_base] | ||||
|                         ['template_for_dispvms']): | ||||
|                     continue | ||||
|                 dispvm_api_name = '$dispvm:' + dispvm_base | ||||
|                 icons[dispvm_api_name] = \ | ||||
|  | ||||
| @ -87,7 +87,7 @@ def main(args=None): | ||||
|     targets = list(system_info['domains'].keys()) | ||||
|     targets.append('$dispvm') | ||||
|     targets.extend('$dispvm:' + dom for dom in system_info['domains'] | ||||
|         if system_info['domains'][dom]['dispvm_allowed']) | ||||
|         if system_info['domains'][dom]['template_for_dispvms']) | ||||
| 
 | ||||
|     connections = set() | ||||
| 
 | ||||
|  | ||||
| @ -34,55 +34,55 @@ system_info = { | ||||
|             'tags': ['dom0-tag'], | ||||
|             'type': 'AdminVM', | ||||
|             'default_dispvm': 'default-dvm', | ||||
|             'dispvm_allowed': False, | ||||
|             'template_for_dispvms': False, | ||||
|         }, | ||||
|         'test-vm1': { | ||||
|             'tags': ['tag1', 'tag2'], | ||||
|             'type': 'AppVM', | ||||
|             'default_dispvm': 'default-dvm', | ||||
|             'dispvm_allowed': False, | ||||
|             'template_for_dispvms': False, | ||||
|         }, | ||||
|         'test-vm2': { | ||||
|             'tags': ['tag2'], | ||||
|             'type': 'AppVM', | ||||
|             'default_dispvm': 'default-dvm', | ||||
|             'dispvm_allowed': False, | ||||
|             'template_for_dispvms': False, | ||||
|         }, | ||||
|         'test-vm3': { | ||||
|             'tags': [], | ||||
|             'type': 'AppVM', | ||||
|             'default_dispvm': 'default-dvm', | ||||
|             'dispvm_allowed': True, | ||||
|             'template_for_dispvms': True, | ||||
|         }, | ||||
|         'default-dvm': { | ||||
|             'tags': [], | ||||
|             'type': 'AppVM', | ||||
|             'default_dispvm': 'default-dvm', | ||||
|             'dispvm_allowed': True, | ||||
|             'template_for_dispvms': True, | ||||
|         }, | ||||
|         'test-invalid-dvm': { | ||||
|             'tags': ['tag1', 'tag2'], | ||||
|             'type': 'AppVM', | ||||
|             'default_dispvm': 'test-vm1', | ||||
|             'dispvm_allowed': False, | ||||
|             'template_for_dispvms': False, | ||||
|         }, | ||||
|         'test-no-dvm': { | ||||
|             'tags': ['tag1', 'tag2'], | ||||
|             'type': 'AppVM', | ||||
|             'default_dispvm': None, | ||||
|             'dispvm_allowed': False, | ||||
|             'template_for_dispvms': False, | ||||
|         }, | ||||
|         'test-template': { | ||||
|             'tags': ['tag1', 'tag2'], | ||||
|             'type': 'TemplateVM', | ||||
|             'default_dispvm': 'default-dvm', | ||||
|             'dispvm_allowed': False, | ||||
|             'template_for_dispvms': False, | ||||
|         }, | ||||
|         'test-standalone': { | ||||
|             'tags': ['tag1', 'tag2'], | ||||
|             'type': 'StandaloneVM', | ||||
|             'default_dispvm': 'default-dvm', | ||||
|             'dispvm_allowed': False, | ||||
|             'template_for_dispvms': False, | ||||
|         }, | ||||
|     } | ||||
| } | ||||
| @ -268,10 +268,10 @@ class TC_00_PolicyRule(qubes.tests.QubesTestCase): | ||||
|         self.assertFalse(is_match_single(system_info, '$default', 'test-vm1')) | ||||
|         self.assertFalse(is_match_single(system_info, '$tag:tag1', 'test-vm3')) | ||||
|         self.assertFalse(is_match_single(system_info, '$anyvm', 'no-such-vm')) | ||||
|         # test-vm1.dispvm_allowed=False | ||||
|         # test-vm1.template_for_dispvms=False | ||||
|         self.assertFalse(is_match_single(system_info, | ||||
|             '$anyvm', '$dispvm:test-vm1')) | ||||
|         # test-vm1.dispvm_allowed=False | ||||
|         # test-vm1.template_for_dispvms=False | ||||
|         self.assertFalse(is_match_single(system_info, | ||||
|             '$dispvm:test-vm1', '$dispvm:test-vm1')) | ||||
|         self.assertFalse(is_match_single(system_info, '$anyvm', 'dom0')) | ||||
|  | ||||
| @ -41,10 +41,10 @@ class TC_00_qrexec_policy(qubes.tests.QubesTestCase): | ||||
|         self.system_info_mock = self.system_info_patch.start() | ||||
| 
 | ||||
|         self.system_info = { | ||||
|             'domains': {'dom0': {'icon': 'black', 'dispvm_allowed': False}, | ||||
|                 'test-vm1': {'icon': 'red', 'dispvm_allowed': False}, | ||||
|                 'test-vm2': {'icon': 'red', 'dispvm_allowed': False}, | ||||
|                 'test-vm3': {'icon': 'green', 'dispvm_allowed': True}, }} | ||||
|             'domains': {'dom0': {'icon': 'black', 'template_for_dispvms': False}, | ||||
|                 'test-vm1': {'icon': 'red', 'template_for_dispvms': False}, | ||||
|                 'test-vm2': {'icon': 'red', 'template_for_dispvms': False}, | ||||
|                 'test-vm3': {'icon': 'green', 'template_for_dispvms': True}, }} | ||||
|         self.system_info_mock.return_value = self.system_info | ||||
| 
 | ||||
|         self.dbus_patch = unittest.mock.patch('pydbus.SystemBus') | ||||
|  | ||||
		Loading…
	
		Reference in New Issue
	
	Block a user
	 Marek Marczykowski-Górecki
						Marek Marczykowski-Górecki