From edcaed537a118b0ee8eb6e62f11630e459947635 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Mon, 2 Oct 2017 20:43:01 +0200 Subject: [PATCH] Always use QubesVM objects, instead of AppVM/TemplateVM etc Very few calls at client side really needs VM class name. So, even in non-blind mode use just QubesVM class, to avoid strange cases depending on blind mode being enabled or not. Then, have VM class name in 'klass' property. If known at object creation time, cache it, otherwise query qubesd at first access. --- qubesadmin/app.py | 16 +++++++++------- qubesadmin/backup/restore.py | 2 +- qubesadmin/tests/__init__.py | 1 + qubesadmin/tests/app.py | 29 ++++++++++++++++++++--------- qubesadmin/tools/__init__.py | 2 +- qubesadmin/tools/qvm_check.py | 3 +-- qubesadmin/tools/qvm_ls.py | 25 +++++++++++-------------- qubesadmin/tools/qvm_start.py | 2 +- qubesadmin/tools/qvm_start_gui.py | 4 ++-- qubesadmin/vm/__init__.py | 11 ++++++++++- 10 files changed, 57 insertions(+), 38 deletions(-) diff --git a/qubesadmin/app.py b/qubesadmin/app.py index 1a88fe5..4f03824 100644 --- a/qubesadmin/app.py +++ b/qubesadmin/app.py @@ -86,12 +86,14 @@ class VMCollection(object): if not self.app.blind_mode and item not in self: raise KeyError(item) if item not in self._vm_objects: - if self.app.blind_mode: - cls = qubesadmin.vm.QubesVM - else: - cls = qubesadmin.utils.get_entry_point_one(VM_ENTRY_POINT, - self._vm_list[item]['class']) - self._vm_objects[item] = cls(self.app, item) + cls = qubesadmin.vm.QubesVM + # provide class name to constructor, if already cached (which can be + # done by 'item not in self' check above, unless blind_mode is + # enabled + klass = None + if self._vm_list and item in self._vm_list: + klass = self._vm_list[item]['class'] + self._vm_objects[item] = cls(self.app, item, klass=klass) return self._vm_objects[item] def __contains__(self, item): @@ -312,7 +314,7 @@ class QubesBase(qubesadmin.base.PropertyHolder): src_vm = self.domains[src_vm] if new_cls is None: - new_cls = src_vm.__class__.__name__ + new_cls = src_vm.klass template = getattr(src_vm, 'template', None) if template is not None: diff --git a/qubesadmin/backup/restore.py b/qubesadmin/backup/restore.py index 99c1037..ab27bcb 100644 --- a/qubesadmin/backup/restore.py +++ b/qubesadmin/backup/restore.py @@ -1456,7 +1456,7 @@ class BackupRestore(object): except KeyError: host_template = None present_on_host = (host_template and - isinstance(host_template, qubesadmin.vm.TemplateVM)) + host_template.klass == 'TemplateVM') present_in_backup = (template_name in restore_info.keys() and restore_info[template_name].good_to_go and restore_info[template_name].vm.klass == diff --git a/qubesadmin/tests/__init__.py b/qubesadmin/tests/__init__.py index d803a9d..d6a2309 100644 --- a/qubesadmin/tests/__init__.py +++ b/qubesadmin/tests/__init__.py @@ -29,6 +29,7 @@ import qubesadmin.app class TestVM(object): def __init__(self, name, **kwargs): self.name = name + self.klass = 'TestVM' for key, value in kwargs.items(): setattr(self, key, value) diff --git a/qubesadmin/tests/app.py b/qubesadmin/tests/app.py index b0a3dc6..f01b4f6 100644 --- a/qubesadmin/tests/app.py +++ b/qubesadmin/tests/app.py @@ -94,7 +94,7 @@ class TC_00_VMCollection(qubesadmin.tests.QubesTestCase): b'test-vm2 class=AppVM state=Running\n' values = self.app.domains.values() for obj in values: - self.assertIsInstance(obj, qubesadmin.vm.AppVM) + self.assertIsInstance(obj, qubesadmin.vm.QubesVM) self.assertEqual(set([vm.name for vm in values]), set(['test-vm', 'test-vm2'])) self.assertAllCalled() @@ -122,6 +122,17 @@ class TC_00_VMCollection(qubesadmin.tests.QubesTestCase): self.assertNotIn('test-non-existent', self.app.domains) self.assertAllCalled() + def test_009_getitem_cache_class(self): + self.app.expected_calls[('dom0', 'admin.vm.List', None, None)] = \ + b'0\x00test-vm class=AppVM state=Running\n' + try: + vm = self.app.domains['test-vm'] + self.assertEqual(vm.name, 'test-vm') + self.assertEqual(vm.klass, 'AppVM') + except KeyError: + self.fail('VM not found in collection') + self.assertAllCalled() + class TC_10_QubesBase(qubesadmin.tests.QubesTestCase): def test_010_new_simple(self): @@ -131,7 +142,7 @@ class TC_10_QubesBase(qubesadmin.tests.QubesTestCase): b'0\x00new-vm class=AppVM state=Running\n' vm = self.app.add_new_vm('AppVM', 'new-vm', 'red') self.assertEqual(vm.name, 'new-vm') - self.assertEqual(vm.__class__.__name__, 'AppVM') + self.assertEqual(vm.klass, 'AppVM') self.assertAllCalled() def test_011_new_template(self): @@ -141,7 +152,7 @@ class TC_10_QubesBase(qubesadmin.tests.QubesTestCase): b'0\x00new-template class=TemplateVM state=Running\n' vm = self.app.add_new_vm('TemplateVM', 'new-template', 'red') self.assertEqual(vm.name, 'new-template') - self.assertEqual(vm.__class__.__name__, 'TemplateVM') + self.assertEqual(vm.klass, 'TemplateVM') self.assertAllCalled() def test_012_new_template_based(self): @@ -151,7 +162,7 @@ class TC_10_QubesBase(qubesadmin.tests.QubesTestCase): b'0\x00new-vm class=AppVM state=Running\n' vm = self.app.add_new_vm('AppVM', 'new-vm', 'red', 'some-template') self.assertEqual(vm.name, 'new-vm') - self.assertEqual(vm.__class__.__name__, 'AppVM') + self.assertEqual(vm.klass, 'AppVM') self.assertAllCalled() def test_013_new_objects_params(self): @@ -165,7 +176,7 @@ class TC_10_QubesBase(qubesadmin.tests.QubesTestCase): vm = self.app.add_new_vm(self.app.get_vm_class('AppVM'), 'new-vm', self.app.get_label('red'), self.app.domains['some-template']) self.assertEqual(vm.name, 'new-vm') - self.assertEqual(vm.__class__.__name__, 'AppVM') + self.assertEqual(vm.klass, 'AppVM') self.assertAllCalled() def test_014_new_pool(self): @@ -175,7 +186,7 @@ class TC_10_QubesBase(qubesadmin.tests.QubesTestCase): b'0\x00new-vm class=AppVM state=Running\n' vm = self.app.add_new_vm('AppVM', 'new-vm', 'red', pool='some-pool') self.assertEqual(vm.name, 'new-vm') - self.assertEqual(vm.__class__.__name__, 'AppVM') + self.assertEqual(vm.klass, 'AppVM') self.assertAllCalled() def test_015_new_pools(self): @@ -187,7 +198,7 @@ class TC_10_QubesBase(qubesadmin.tests.QubesTestCase): vm = self.app.add_new_vm('AppVM', 'new-vm', 'red', pools={'private': 'some-pool', 'volatile': 'other-pool'}) self.assertEqual(vm.name, 'new-vm') - self.assertEqual(vm.__class__.__name__, 'AppVM') + self.assertEqual(vm.klass, 'AppVM') self.assertAllCalled() def test_016_new_template_based_default(self): @@ -198,7 +209,7 @@ class TC_10_QubesBase(qubesadmin.tests.QubesTestCase): vm = self.app.add_new_vm('AppVM', 'new-vm', 'red', template=qubesadmin.DEFAULT) self.assertEqual(vm.name, 'new-vm') - self.assertEqual(vm.__class__.__name__, 'AppVM') + self.assertEqual(vm.klass, 'AppVM') self.assertAllCalled() def test_020_get_label(self): @@ -422,7 +433,7 @@ class TC_10_QubesBase(qubesadmin.tests.QubesTestCase): new_vm = self.app.clone_vm('test-vm', 'new-name', new_cls='StandaloneVM') self.assertEqual(new_vm.name, 'new-name') - self.assertEqual(new_vm.__class__.__name__, 'StandaloneVM') + self.assertEqual(new_vm.klass, 'StandaloneVM') self.assertAllCalled() def test_035_clone_fail(self): diff --git a/qubesadmin/tools/__init__.py b/qubesadmin/tools/__init__.py index ec46e3a..76067bd 100644 --- a/qubesadmin/tools/__init__.py +++ b/qubesadmin/tools/__init__.py @@ -156,7 +156,7 @@ class VmNameAction(QubesAction): namespace.domains = [ vm for vm in app.domains - if not isinstance(vm, qubesadmin.vm.AdminVM) and + if not vm.klass == 'AdminVM' and vm.name not in namespace.exclude ] else: diff --git a/qubesadmin/tools/qvm_check.py b/qubesadmin/tools/qvm_check.py index 9d0fc3c..07c4b0a 100644 --- a/qubesadmin/tools/qvm_check.py +++ b/qubesadmin/tools/qvm_check.py @@ -64,8 +64,7 @@ def main(args=None, app=None): print_msg(paused, "is paused", "are paused") return 0 if paused else 1 elif args.template: - template = [vm for vm in domains if isinstance(vm, - qubesadmin.vm.TemplateVM)] + template = [vm for vm in domains if vm.klass == 'TemplateVM'] if args.verbose: print_msg(template, "is a template", "are templates") return 0 if template else 1 diff --git a/qubesadmin/tools/qvm_ls.py b/qubesadmin/tools/qvm_ls.py index 860b15c..6485554 100644 --- a/qubesadmin/tools/qvm_ls.py +++ b/qubesadmin/tools/qvm_ls.py @@ -214,19 +214,16 @@ class FlagsColumn(Column): When it is HVM (optimised VM), the letter is capital. ''' - if isinstance(vm, qubesadmin.vm.AdminVM): - return '0' - - ret = None - # TODO right order, depending on inheritance - if isinstance(vm, qubesadmin.vm.TemplateVM): - ret = 't' - if isinstance(vm, qubesadmin.vm.AppVM): - ret = 'a' - if isinstance(vm, qubesadmin.vm.StandaloneVM): - ret = 's' - if isinstance(vm, qubesadmin.vm.DispVM): - ret = 'd' + type_codes = { + 'AdminVM': '0', + 'TemplateVM': 't', + 'AppVM': 'a', + 'StandaloneVM': 's', + 'DispVM': 'd', + } + ret = type_codes.get(vm.klass, None) + if ret == '0': + return ret if ret is not None: if getattr(vm, 'virt_mode', 'pv') == 'hvm': @@ -338,7 +335,7 @@ Column('STATE', doc='Current power state.') Column('CLASS', - attr=(lambda vm: type(vm).__name__), + attr=(lambda vm: vm.klass), doc='Class of the qube.') diff --git a/qubesadmin/tools/qvm_start.py b/qubesadmin/tools/qvm_start.py index 070ab83..42f8474 100644 --- a/qubesadmin/tools/qvm_start.py +++ b/qubesadmin/tools/qvm_start.py @@ -109,7 +109,7 @@ def get_drive_assignment(app, drive_str): 'Existing block device identifier needed when running from ' 'outside of dom0 (see qvm-block)') try: - if isinstance(backend_domain, qubesadmin.vm.AdminVM): + if backend_domain.klass == 'AdminVM': loop_name = subprocess.check_output( ['sudo', 'losetup', '-f', '--show', ident]) else: diff --git a/qubesadmin/tools/qvm_start_gui.py b/qubesadmin/tools/qvm_start_gui.py index bba5030..e47d8f2 100644 --- a/qubesadmin/tools/qvm_start_gui.py +++ b/qubesadmin/tools/qvm_start_gui.py @@ -296,7 +296,7 @@ class GUILauncher(object): '''Send monitor layout to all (running) VMs''' monitor_layout = get_monitor_layout() for vm in self.app.domains: - if isinstance(vm, qubesadmin.vm.AdminVM): + if vm.klass == 'AdminVM': continue if vm.is_running(): if not vm.features.check_with_template('gui', True): @@ -325,7 +325,7 @@ class GUILauncher(object): monitor_layout = get_monitor_layout() self.app.domains.clear_cache() for vm in self.app.domains: - if isinstance(vm, qubesadmin.vm.AdminVM): + if vm.klass == 'AdminVM': continue if not vm.features.check_with_template('gui', True): continue diff --git a/qubesadmin/vm/__init__.py b/qubesadmin/vm/__init__.py index 1d0fe1c..a22d65c 100644 --- a/qubesadmin/vm/__init__.py +++ b/qubesadmin/vm/__init__.py @@ -46,9 +46,10 @@ class QubesVM(qubesadmin.base.PropertyHolder): firewall = None - def __init__(self, app, name): + def __init__(self, app, name, klass=None): super(QubesVM, self).__init__(app, 'admin.vm.property.', name) self._volumes = None + self._klass = klass self.log = logging.getLogger(name) self.tags = qubesadmin.tags.Tags(self) self.features = qubesadmin.features.Features(self) @@ -314,6 +315,14 @@ class QubesVM(qubesadmin.base.PropertyHolder): e.cmd = command raise e + @property + def klass(self): + ''' Qube class ''' + # use cached value if available + if self._klass is None: + # pylint: disable=no-member + self._klass = super(QubesVM, self).klass + return self._klass # pylint: disable=abstract-method class AdminVM(QubesVM):