From c17b634913628e655a9b447613d5ef679b833a87 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Tue, 16 Jan 2018 21:32:15 +0100 Subject: [PATCH 01/10] tests: clear PCIDevice cache after each test This is yet another place where references to VM objects contribute to object leaks. --- qubes/tests/__init__.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/qubes/tests/__init__.py b/qubes/tests/__init__.py index 8c1deb7b..a9553791 100644 --- a/qubes/tests/__init__.py +++ b/qubes/tests/__init__.py @@ -60,6 +60,7 @@ import qubes.config import qubes.devices import qubes.events import qubes.exc +import qubes.ext.pci import qubes.vm.standalonevm import qubes.vm.templatevm @@ -378,6 +379,7 @@ class QubesTestCase(unittest.TestCase): self.loop = asyncio.get_event_loop() self.addCleanup(self.cleanup_loop) + self.addCleanup(qubes.ext.pci._cache_get.cache_clear) def cleanup_gc(self): gc.collect() From 06e82eccb0d4200742109ab78dc4a6c5a1efdcca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Tue, 16 Jan 2018 21:39:22 +0100 Subject: [PATCH 02/10] tests: add run_service and qrexec_policy wrappers to ExtraTestCase Provide same API as in core2, especially without exposing asyncio usage. This allows qubes-usb-proxy and qubes-split-gpg tests to run. --- qubes/tests/extra.py | 45 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/qubes/tests/extra.py b/qubes/tests/extra.py index a60cdfab..9208cab6 100644 --- a/qubes/tests/extra.py +++ b/qubes/tests/extra.py @@ -84,6 +84,27 @@ class VMWrapper(object): stderr=subprocess.PIPE if passio_stderr else None)) return ProcessWrapper(p, self._loop) + def run_service(self, service, wait=True, input=None, user=None, + passio_popen=False, + passio_stderr=False, **kwargs): + if wait: + try: + if isinstance(input, str): + input = input.encode() + self._loop.run_until_complete( + self._vm.run_service_for_stdio(service, + input=input, user=user)) + except subprocess.CalledProcessError as err: + return err.returncode + return 0 + elif passio_popen: + p = self._loop.run_until_complete(self._vm.run_service(service, + user=user, + stdin=subprocess.PIPE, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE if passio_stderr else None)) + return ProcessWrapper(p, self._loop) + class ExtraTestCase(qubes.tests.SystemTestCase): @@ -137,6 +158,30 @@ class ExtraTestCase(qubes.tests.SystemTestCase): """ self.init_networking() + def qrexec_policy(self, service, source, destination, allow=True): + """ + Allow qrexec calls for duration of the test + :param service: service name + :param source: source VM name + :param destination: destination VM name + :return: + """ + + def add_remove_rule(add=True): + with open('/etc/qubes-rpc/policy/{}'.format(service), 'r+') as policy: + policy_rules = policy.readlines() + rule = "{} {} {}\n".format(source, destination, + 'allow' if allow else 'deny') + if add: + policy_rules.insert(0, rule) + else: + policy_rules.remove(rule) + policy.truncate(0) + policy.seek(0) + policy.write(''.join(policy_rules)) + add_remove_rule(add=True) + self.addCleanup(add_remove_rule, add=False) + def load_tests(loader, tests, pattern): for entry in pkg_resources.iter_entry_points('qubes.tests.extra'): From 4d59f883a0f2323c24b6e7347644a6d48b0808da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Tue, 16 Jan 2018 21:41:38 +0100 Subject: [PATCH 03/10] tests: minor fixes - FD leak - switch to xterm to test also on minimal template --- qubes/tests/integ/backup.py | 5 ++++- qubes/tests/integ/vm_qrexec_gui.py | 6 ++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/qubes/tests/integ/backup.py b/qubes/tests/integ/backup.py index 52ddced9..cf1576d7 100644 --- a/qubes/tests/integ/backup.py +++ b/qubes/tests/integ/backup.py @@ -90,7 +90,10 @@ class BackupTestsMixin(object): block_size = 4096 self.log.debug("Filling %s" % path) - f = open(path, 'wb+') + try: + f = open(path, 'rb+') + except FileNotFoundError: + f = open(path, 'wb+') if size is None: f.seek(0, 2) size = f.tell() diff --git a/qubes/tests/integ/vm_qrexec_gui.py b/qubes/tests/integ/vm_qrexec_gui.py index e6ca70e1..e818c8cd 100644 --- a/qubes/tests/integ/vm_qrexec_gui.py +++ b/qubes/tests/integ/vm_qrexec_gui.py @@ -925,15 +925,17 @@ int main(int argc, char **argv) { # it is important to have some changing content there, to generate # content update events (aka damage notify) proc = yield from self.testvm1.run( - 'gnome-terminal --full-screen -e top') + 'xterm -maximized -e top') # help xdotool a little... yield from asyncio.sleep(2) + if proc.returncode is not None: + self.fail('xterm failed to start') # get window ID winid = (yield from asyncio.get_event_loop().run_in_executor(None, subprocess.check_output, ['xdotool', 'search', '--sync', '--onlyvisible', '--class', - self.testvm1.name + ':.*erminal'])).decode() + self.testvm1.name + ':xterm'])).decode() xprop = yield from asyncio.get_event_loop().run_in_executor(None, subprocess.check_output, ['xprop', '-notype', '-id', winid, '_QUBES_VMWINDOWID']) From 7905783861feca067c985c23b59a2e430127b4b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Tue, 16 Jan 2018 21:42:20 +0100 Subject: [PATCH 04/10] qubesvm: PVH minor improvements - use capital letters in acronyms in documentation to match upstream documentation. - refuse to start a PVH with without kernel set - provide meaningful error message --- qubes/vm/qubesvm.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/qubes/vm/qubesvm.py b/qubes/vm/qubesvm.py index 1d73f7ec..dd66ab3a 100644 --- a/qubes/vm/qubesvm.py +++ b/qubes/vm/qubesvm.py @@ -391,8 +391,8 @@ class QubesVM(qubes.vm.mix.net.NetVMMixin, qubes.vm.BaseVM): virt_mode = qubes.property('virt_mode', type=str, setter=_setter_virt_mode, default=_default_virt_mode, - doc='''Virtualisation mode: full virtualisation ("hvm"), - or paravirtualisation ("pv"), or hybrid ("pvh")''') + doc='''Virtualisation mode: full virtualisation ("HVM"), + or paravirtualisation ("PV"), or hybrid ("PVH")''') installed_by_rpm = qubes.property('installed_by_rpm', type=bool, setter=qubes.property.bool, @@ -869,6 +869,9 @@ class QubesVM(qubes.vm.mix.net.NetVMMixin, qubes.vm.BaseVM): qmemman_client = None try: + if self.virt_mode == 'pvh' and self.kernel is None: + raise qubes.exc.QubesException( + 'virt_mode PVH require kernel to be set') yield from self.storage.verify() if self.netvm is not None: From f2b9be360735d08bf052e79fb0b418e3b1e36af3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Wed, 17 Jan 2018 15:23:22 +0100 Subject: [PATCH 05/10] tests: one more missing virt_mode=hvm --- qubes/tests/integ/basic.py | 1 + 1 file changed, 1 insertion(+) diff --git a/qubes/tests/integ/basic.py b/qubes/tests/integ/basic.py index 99ca131a..ed8745fb 100644 --- a/qubes/tests/integ/basic.py +++ b/qubes/tests/integ/basic.py @@ -91,6 +91,7 @@ class TC_00_Basic(qubes.tests.SystemTestCase): self.vm = self.app.add_new_vm('StandaloneVM', label='red', name=vmname) self.loop.run_until_complete(self.vm.create_on_disk()) self.vm.kernel = None + self.vm.virt_mode = 'hvm' iso_path = self.create_bootable_iso() # start the VM using qvm-start tool, to test --cdrom option there From dce3b609b479fb979ebd77a59b186f74e2a4153a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Wed, 17 Jan 2018 15:23:39 +0100 Subject: [PATCH 06/10] qubesvm: do not try to define libvirt object in offline mode The idea is to not touch libvirt at all. --- qubes/app.py | 3 ++- qubes/vm/qubesvm.py | 3 +++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/qubes/app.py b/qubes/app.py index 06a1a41d..48561bd1 100644 --- a/qubes/app.py +++ b/qubes/app.py @@ -483,7 +483,8 @@ class VMCollection(object): raise qubes.exc.QubesVMNotHaltedError(vm) self.app.fire_event('domain-pre-delete', pre_event=True, vm=vm) try: - vm.libvirt_domain.undefine() + if vm.libvirt_domain: + vm.libvirt_domain.undefine() # pylint: disable=protected-access vm._libvirt_domain = None except libvirt.libvirtError as e: diff --git a/qubes/vm/qubesvm.py b/qubes/vm/qubesvm.py index dd66ab3a..493ef397 100644 --- a/qubes/vm/qubesvm.py +++ b/qubes/vm/qubesvm.py @@ -570,6 +570,9 @@ class QubesVM(qubes.vm.mix.net.NetVMMixin, qubes.vm.BaseVM): if self._libvirt_domain is not None: return self._libvirt_domain + if self.app.vmm.offline_mode: + return None + # XXX _update_libvirt_domain? try: self._libvirt_domain = self.app.vmm.libvirt_conn.lookupByUUID( From ca41ca66cdcb04f260bd6bee74bd1a65f19dc6fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Thu, 18 Jan 2018 16:47:55 +0100 Subject: [PATCH 07/10] qmemman: fix early crash clear_outdated_error_markers crashes if memory stats are not retrieved yet. In practice it crashes at the very first call during daemon startup, making the whole qmemman unusable. This fixes bf4306b815a5c0e84ebcb8b58a3449fbac962f88 qmemman: clear "not responding" flags when VM require more memory QubesOS/qubes-issues#3265 --- qubes/qmemman/__init__.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/qubes/qmemman/__init__.py b/qubes/qmemman/__init__.py index a4540eaf..1c7ff780 100644 --- a/qubes/qmemman/__init__.py +++ b/qubes/qmemman/__init__.py @@ -140,6 +140,8 @@ class SystemState(object): def clear_outdated_error_markers(self): # Clear outdated errors for i in self.domdict.keys(): + if self.domdict[i].mem_used is None: + continue # clear markers excluding VM from memory balance, if: # - VM have responded to previous request (with some safety margin) # - VM request more memory than it has assigned From 46177c7c9f18dc42f462f7aca9d0f14ca1fec33c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Thu, 18 Jan 2018 16:50:24 +0100 Subject: [PATCH 08/10] qmemman: do not close stdout/stderr in daemon mode Allow exceptions to be logged to syslog/journald --- qubes/tools/qmemmand.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/qubes/tools/qmemmand.py b/qubes/tools/qmemmand.py index 24ae5f9d..640e8051 100644 --- a/qubes/tools/qmemmand.py +++ b/qubes/tools/qmemmand.py @@ -242,10 +242,6 @@ def main(): ha_file.setFormatter( logging.Formatter('%(asctime)s %(name)s[%(process)d]: %(message)s')) logging.root.addHandler(ha_stderr) - else: - # close io - sys.stdout.close() - sys.stderr.close() sys.stdin.close() From edbfd3843e1189757f7f20af5c873ad0a3b2ce94 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Thu, 18 Jan 2018 16:51:05 +0100 Subject: [PATCH 09/10] tests: make tests.extra.VMWrapper hashable Allow using VMWrapper as dict key, same as QubesVM. --- qubes/tests/extra.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/qubes/tests/extra.py b/qubes/tests/extra.py index 9208cab6..98fc6e71 100644 --- a/qubes/tests/extra.py +++ b/qubes/tests/extra.py @@ -62,6 +62,9 @@ class VMWrapper(object): def __eq__(self, other): return self._vm == other + def __hash__(self): + return hash(self._vm) + def start(self): return self._loop.run_until_complete(self._vm.start()) From b245bbca6ff53d67f3dd117f6e90deb2133626bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Thu, 18 Jan 2018 16:52:01 +0100 Subject: [PATCH 10/10] tests: update PCI devices tests for core3 API - use asyncio where needed - attach now takes DeviceAssignment, not DeviceInfo - PCI ident have ':' replaced with '_' --- qubes/tests/integ/devices_pci.py | 70 ++++++++++++++++++++------------ 1 file changed, 43 insertions(+), 27 deletions(-) diff --git a/qubes/tests/integ/devices_pci.py b/qubes/tests/integ/devices_pci.py index 484202a1..d0344134 100644 --- a/qubes/tests/integ/devices_pci.py +++ b/qubes/tests/integ/devices_pci.py @@ -37,7 +37,10 @@ class TC_00_Devices_PCI(qubes.tests.SystemTestCase): if self._testMethodName not in ['test_000_list']: pcidev = os.environ['QUBES_TEST_PCIDEV'] self.dev = self.app.domains[0].devices['pci'][pcidev] - self.assignment = qubes.devices.DeviceAssignment(backend_domain=self.dev.backend_domain, ident=self.dev.ident, persistent=True) + self.assignment = qubes.devices.DeviceAssignment( + backend_domain=self.dev.backend_domain, + ident=self.dev.ident, + persistent=True) if isinstance(self.dev, qubes.devices.UnknownDevice): self.skipTest('Specified device {} does not exists'.format(pcidev)) self.init_default_template() @@ -45,7 +48,8 @@ class TC_00_Devices_PCI(qubes.tests.SystemTestCase): name=self.make_vm_name('vm'), label='red', ) - self.vm.create_on_disk() + self.loop.run_until_complete( + self.vm.create_on_disk()) self.vm.features['pci-no-strict-reset/' + pcidev] = True self.app.save() @@ -57,11 +61,12 @@ class TC_00_Devices_PCI(qubes.tests.SystemTestCase): l.split(' (')[0].split(' ', 1) for l in p.communicate()[0].decode().splitlines()) for dev in self.app.domains[0].devices['pci']: + lspci_ident = dev.ident.replace('_', ':') self.assertIsInstance(dev, qubes.ext.pci.PCIDevice) self.assertEqual(dev.backend_domain, self.app.domains[0]) - self.assertIn(dev.ident, actual_devices) - self.assertEqual(dev.description, actual_devices[dev.ident]) - actual_devices.pop(dev.ident) + self.assertIn(lspci_ident, actual_devices) + self.assertEqual(dev.description, actual_devices[lspci_ident]) + actual_devices.pop(lspci_ident) if actual_devices: self.fail('Not all devices listed, missing: {}'.format( @@ -76,7 +81,8 @@ class TC_00_Devices_PCI(qubes.tests.SystemTestCase): def test_010_attach_offline_persistent(self): dev_col = self.vm.devices['pci'] self.assertDeviceNotInCollection(self.dev, dev_col) - dev_col.attach(self.assignment) + self.loop.run_until_complete( + dev_col.attach(self.assignment)) self.app.save() self.assertNotIn(self.dev, dev_col.attached()) self.assertIn(self.dev, dev_col.persistent()) @@ -84,12 +90,11 @@ class TC_00_Devices_PCI(qubes.tests.SystemTestCase): self.assertIn(self.dev, dev_col.assignments(persistent=True)) self.assertNotIn(self.dev, dev_col.assignments(persistent=False)) - - self.vm.start() + self.loop.run_until_complete(self.vm.start()) self.assertIn(self.dev, dev_col.attached()) - p = self.vm.run('lspci', passio_popen=True) - (stdout, _) = p.communicate() + (stdout, _) = self.loop.run_until_complete( + self.vm.run_for_stdio('lspci')) self.assertIn(self.dev.description, stdout.decode()) @@ -98,14 +103,17 @@ class TC_00_Devices_PCI(qubes.tests.SystemTestCase): self.assertDeviceNotInCollection(self.dev, dev_col) self.assignment.persistent = False with self.assertRaises(qubes.exc.QubesVMNotRunningError): - dev_col.attach(self.assignment) + self.loop.run_until_complete( + dev_col.attach(self.assignment)) def test_020_attach_online_persistent(self): - self.vm.start() + self.loop.run_until_complete( + self.vm.start()) dev_col = self.vm.devices['pci'] self.assertDeviceNotInCollection(self.dev, dev_col) - dev_col.attach(self.assignment) + self.loop.run_until_complete( + dev_col.attach(self.assignment)) self.assertIn(self.dev, dev_col.attached()) self.assertIn(self.dev, dev_col.persistent()) @@ -115,39 +123,46 @@ class TC_00_Devices_PCI(qubes.tests.SystemTestCase): # give VM kernel some time to discover new device time.sleep(1) - p = self.vm.run('lspci', passio_popen=True) - (stdout, _) = p.communicate() + (stdout, _) = self.loop.run_until_complete( + self.vm.run_for_stdio('lspci')) self.assertIn(self.dev.description, stdout.decode()) def test_021_persist_detach_online_fail(self): dev_col = self.vm.devices['pci'] self.assertDeviceNotInCollection(self.dev, dev_col) - dev_col.attach(self.assignment) + self.loop.run_until_complete( + dev_col.attach(self.assignment)) self.app.save() - self.vm.start() + self.loop.run_until_complete( + self.vm.start()) with self.assertRaises(qubes.exc.QubesVMNotHaltedError): - self.vm.devices['pci'].detach(self.assignment) + self.loop.run_until_complete( + self.vm.devices['pci'].detach(self.assignment)) def test_030_persist_attach_detach_offline(self): dev_col = self.vm.devices['pci'] self.assertDeviceNotInCollection(self.dev, dev_col) - dev_col.attach(self.assignment) + self.loop.run_until_complete( + dev_col.attach(self.assignment)) self.app.save() self.assertNotIn(self.dev, dev_col.attached()) self.assertIn(self.dev, dev_col.persistent()) self.assertIn(self.dev, dev_col.assignments()) self.assertIn(self.dev, dev_col.assignments(persistent=True)) self.assertNotIn(self.dev, dev_col.assignments(persistent=False)) - dev_col.detach(self.assignment) + self.loop.run_until_complete( + dev_col.detach(self.assignment)) self.assertDeviceNotInCollection(self.dev, dev_col) def test_031_attach_detach_online_temp(self): dev_col = self.vm.devices['pci'] - self.vm.start() + self.loop.run_until_complete( + self.vm.start()) self.assignment.persistent = False self.assertDeviceNotInCollection(self.dev, dev_col) - dev_col.attach(self.assignment) + self.loop.run_until_complete( + dev_col.attach(self.assignment)) self.assertIn(self.dev, dev_col.attached()) self.assertNotIn(self.dev, dev_col.persistent()) @@ -159,13 +174,14 @@ class TC_00_Devices_PCI(qubes.tests.SystemTestCase): # give VM kernel some time to discover new device time.sleep(1) - p = self.vm.run('lspci', passio_popen=True) - (stdout, _) = p.communicate() + (stdout, _) = self.loop.run_until_complete( + self.vm.run_for_stdio('lspci')) self.assertIn(self.dev.description, stdout.decode()) - dev_col.detach(self.assignment) + self.loop.run_until_complete( + dev_col.detach(self.assignment)) self.assertDeviceNotInCollection(self.dev, dev_col) - p = self.vm.run('lspci', passio_popen=True) - (stdout, _) = p.communicate() + (stdout, _) = self.loop.run_until_complete( + self.vm.run_for_stdio('lspci')) self.assertNotIn(self.dev.description, stdout.decode())