From 9bf0cce11ece888028f3c1ff6c0408db0d461858 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Mon, 18 Nov 2019 23:43:19 +0100 Subject: [PATCH 1/2] tests: extend mock objects in QubesVM tests - allow TestQubesDB to be populated with initial data - support list() method - allow to register pre-created VM instance (useful for AdminVM, which don't accept setting qid) --- qubes/tests/vm/qubesvm.py | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/qubes/tests/vm/qubesvm.py b/qubes/tests/vm/qubesvm.py index 6cb55f55..2790cf5c 100644 --- a/qubes/tests/vm/qubesvm.py +++ b/qubes/tests/vm/qubesvm.py @@ -39,6 +39,7 @@ import shutil import qubes import qubes.exc import qubes.config +import qubes.devices import qubes.vm import qubes.vm.qubesvm @@ -79,8 +80,10 @@ class TestDeviceCollection(object): return self._list class TestQubesDB(object): - def __init__(self): + def __init__(self, data=None): self.data = {} + if data: + self.data = data def write(self, path, value): self.data[path] = value @@ -92,6 +95,12 @@ class TestQubesDB(object): else: self.data.pop(path, None) + def list(self, prefix): + return [key for key in self.data if key.startswith(prefix)] + + def close(self): + pass + class TestVM(object): # pylint: disable=too-few-public-methods app = TestApp() @@ -269,10 +278,11 @@ class QubesVMTestsMixin(object): pass super(QubesVMTestsMixin, self).tearDown() - def get_vm(self, name='test', cls=qubes.vm.qubesvm.QubesVM, **kwargs): - vm = cls(self.app, None, - qid=kwargs.pop('qid', 1), name=qubes.tests.VMPREFIX + name, - **kwargs) + def get_vm(self, name='test', cls=qubes.vm.qubesvm.QubesVM, vm=None, **kwargs): + if not vm: + vm = cls(self.app, None, + qid=kwargs.pop('qid', 1), name=qubes.tests.VMPREFIX + name, + **kwargs) self.app.domains[vm.qid] = vm self.app.domains[vm.uuid] = vm self.app.domains[vm.name] = vm From 6c7af109e548a22ca143c05f4e40aa4e7b5da3d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Mon, 18 Nov 2019 05:10:09 +0100 Subject: [PATCH 2/2] ext/block: prefer connecting cdrom as xvdd Only first 4 disks can be emulated as IDE disks by QEMU. Specifically, CDROM must be one of those first 4 disks, otherwise it will be ignored. This is especially important if one wants to boot the VM from that CDROM. Since xvdd normally is a kernel-related volume (boot image, modules) it makes perfect sense to re-use it for CDROM. It is either set for kernel volume (in which case, VM should boot from it and not the CDROM), or (possibly bootable) CDROM. This needs to be done in two places: - BlockExtension for dynamic attach - libvirt xen.xml - for before-boot attach In theory the latter would be enough, but it would be quite confusing that device will get different options depending on when it's attached (in addition to whether the kernel is set - introduced here). This all also means, xvdd not always is a "system disk". Adjust listing connected disks accordingly. --- qubes/ext/block.py | 18 ++- qubes/tests/devices_block.py | 56 +++++++-- qubes/tests/vm/qubesvm.py | 183 ++++++++++++++++++++++++++++ templates/libvirt/devices/block.xml | 3 + 4 files changed, 249 insertions(+), 11 deletions(-) diff --git a/qubes/ext/block.py b/qubes/ext/block.py index 13b163ed..7a0f9fa8 100644 --- a/qubes/ext/block.py +++ b/qubes/ext/block.py @@ -37,7 +37,9 @@ mode_re = re.compile(r"^[rw]$") AVAILABLE_FRONTENDS = ['xvd'+c for c in string.ascii_lowercase[8:]+string.ascii_lowercase[:8]] -SYSTEM_DISKS = ('xvda', 'xvdb', 'xvdc', 'xvdd') +SYSTEM_DISKS = ('xvda', 'xvdb', 'xvdc') +# xvdd is considered system disk only if vm.kernel is set +SYSTEM_DISKS_DOM0_KERNEL = SYSTEM_DISKS + ('xvdd',) class BlockDevice(qubes.devices.DeviceInfo): @@ -172,6 +174,9 @@ class BlockDeviceExtension(qubes.ext.Extension): if not vm.is_running(): return + system_disks = SYSTEM_DISKS + if getattr(vm, 'kernel', None): + system_disks = SYSTEM_DISKS_DOM0_KERNEL xml_desc = lxml.etree.fromstring(vm.libvirt_domain.XMLDesc()) for disk in xml_desc.findall('devices/disk'): @@ -187,7 +192,7 @@ class BlockDeviceExtension(qubes.ext.Extension): frontend_dev = target_node.get('dev') if not frontend_dev: continue - if frontend_dev in SYSTEM_DISKS: + if frontend_dev in system_disks: continue else: continue @@ -217,7 +222,7 @@ class BlockDeviceExtension(qubes.ext.Extension): yield (BlockDevice(backend_domain, ident), options) - def find_unused_frontend(self, vm): + def find_unused_frontend(self, vm, devtype='disk'): # pylint: disable=no-self-use '''Find unused block frontend device node for parameter''' @@ -227,6 +232,10 @@ class BlockDeviceExtension(qubes.ext.Extension): parsed_xml = lxml.etree.fromstring(xml) used = [target.get('dev', None) for target in parsed_xml.xpath("//domain/devices/disk/target")] + if devtype == 'cdrom' and 'xvdd' not in used: + # prefer 'xvdd' for CDROM if available; only first 4 disks are + # emulated in HVM, which means only those are bootable + return 'xvdd' for dev in AVAILABLE_FRONTENDS: if dev not in used: return dev @@ -269,7 +278,8 @@ class BlockDeviceExtension(qubes.ext.Extension): 'it'.format(device.backend_domain.name)) if 'frontend-dev' not in options: - options['frontend-dev'] = self.find_unused_frontend(vm) + options['frontend-dev'] = self.find_unused_frontend( + vm, options.get('devtype', 'disk')) vm.libvirt_domain.attachDevice( vm.app.env.get_template('libvirt/devices/block.xml').render( diff --git a/qubes/tests/devices_block.py b/qubes/tests/devices_block.py index bf0298d4..959ec31c 100644 --- a/qubes/tests/devices_block.py +++ b/qubes/tests/devices_block.py @@ -25,6 +25,15 @@ import jinja2 import qubes.tests import qubes.ext.block +modules_disk = ''' + + + + + + + +''' domain_xml_template = ''' @@ -66,13 +75,6 @@ domain_xml_template = ''' - - - - - - - {} @@ -486,6 +488,46 @@ class TC_00_Block(qubes.tests.QubesTestCase): '') vm.libvirt_domain.attachDevice.assert_called_once_with(device_xml) + def test_048_attach_cdrom_xvdi(self): + back_vm = TestVM(name='sys-usb', qdb={ + '/qubes-block-devices/sda': b'', + '/qubes-block-devices/sda/desc': b'Test device', + '/qubes-block-devices/sda/size': b'1024000', + '/qubes-block-devices/sda/mode': b'r', + }) + vm = TestVM({}, domain_xml=domain_xml_template.format(modules_disk)) + dev = qubes.ext.block.BlockDevice(back_vm, 'sda') + self.ext.on_device_pre_attached_block(vm, '', dev, {'devtype': 'cdrom'}) + device_xml = ( + '\n' + ' \n' + ' \n' + ' \n' + ' \n' + ' \n' + '') + vm.libvirt_domain.attachDevice.assert_called_once_with(device_xml) + + def test_048_attach_cdrom_xvdd(self): + back_vm = TestVM(name='sys-usb', qdb={ + '/qubes-block-devices/sda': b'', + '/qubes-block-devices/sda/desc': b'Test device', + '/qubes-block-devices/sda/size': b'1024000', + '/qubes-block-devices/sda/mode': b'r', + }) + vm = TestVM({}, domain_xml=domain_xml_template.format('')) + dev = qubes.ext.block.BlockDevice(back_vm, 'sda') + self.ext.on_device_pre_attached_block(vm, '', dev, {'devtype': 'cdrom'}) + device_xml = ( + '\n' + ' \n' + ' \n' + ' \n' + ' \n' + ' \n' + '') + vm.libvirt_domain.attachDevice.assert_called_once_with(device_xml) + def test_050_detach(self): back_vm = TestVM(name='sys-usb', qdb={ '/qubes-block-devices/sda': b'', diff --git a/qubes/tests/vm/qubesvm.py b/qubes/tests/vm/qubesvm.py index 2790cf5c..1c57695b 100644 --- a/qubes/tests/vm/qubesvm.py +++ b/qubes/tests/vm/qubesvm.py @@ -1209,6 +1209,189 @@ class TC_90_QubesVM(QubesVMTestsMixin, qubes.tests.QubesTestCase): self.assertXMLEqual(lxml.etree.XML(libvirt_xml), lxml.etree.XML(expected)) + def test_600_libvirt_xml_hvm_cdrom_boot(self): + expected = ''' + test-inst-test + 7db78950-c467-4863-94d1-af59806384ea + 400 + 400 + 2 + + + + + + + + + hvm + + hvmloader + + + + + + + + + + + destroy + destroy + destroy + + + + + + + + + + + + + + + + + + + ''' + my_uuid = '7db78950-c467-4863-94d1-af59806384ea' + qdb = { + '/qubes-block-devices/sda': b'', + '/qubes-block-devices/sda/desc': b'Test device', + '/qubes-block-devices/sda/size': b'1024000', + '/qubes-block-devices/sda/mode': b'r', + } + test_qdb = TestQubesDB(qdb) + dom0 = qubes.vm.adminvm.AdminVM(self.app, None) + dom0._qdb_connection = test_qdb + self.get_vm('dom0', vm=dom0) + vm = self.get_vm(uuid=my_uuid) + vm.netvm = None + vm.virt_mode = 'hvm' + vm.kernel = None + dom0.events_enabled = True + self.app.vmm.offline_mode = False + dev = qubes.devices.DeviceAssignment( + dom0, 'sda', + {'devtype': 'cdrom', 'read-only': 'yes'}, persistent=True) + self.loop.run_until_complete(vm.devices['block'].attach(dev)) + libvirt_xml = vm.create_config_file() + self.assertXMLEqual(lxml.etree.XML(libvirt_xml), + lxml.etree.XML(expected)) + + def test_600_libvirt_xml_hvm_cdrom_dom0_kernel_boot(self): + expected = ''' + test-inst-test + 7db78950-c467-4863-94d1-af59806384ea + 400 + 400 + 2 + + + + + + + + + hvm + + hvmloader + + + root=/dev/mapper/dmroot ro nomodeset console=hvc0 rd_NO_PLYMOUTH rd.plymouth.enable=0 plymouth.enable=0 nopat + + + + + + + + + destroy + destroy + destroy + + + + + + + + + + + + + + + + + + + + + + + + ''' + qdb = { + '/qubes-block-devices/sda': b'', + '/qubes-block-devices/sda/desc': b'Test device', + '/qubes-block-devices/sda/size': b'1024000', + '/qubes-block-devices/sda/mode': b'r', + } + test_qdb = TestQubesDB(qdb) + dom0 = qubes.vm.adminvm.AdminVM(self.app, None) + dom0._qdb_connection = test_qdb + my_uuid = '7db78950-c467-4863-94d1-af59806384ea' + vm = self.get_vm(uuid=my_uuid) + vm.netvm = None + vm.virt_mode = 'hvm' + with unittest.mock.patch('qubes.config.qubes_base_dir', + '/tmp/qubes-test'): + kernel_dir = '/tmp/qubes-test/vm-kernels/dummy' + os.makedirs(kernel_dir, exist_ok=True) + open(os.path.join(kernel_dir, 'vmlinuz'), 'w').close() + open(os.path.join(kernel_dir, 'initramfs'), 'w').close() + self.addCleanup(shutil.rmtree, '/tmp/qubes-test') + vm.kernel = 'dummy' + # tests for storage are later + vm.volumes['kernel'] = unittest.mock.Mock(**{ + 'kernels_dir': '/tmp/kernel', + 'block_device.return_value.domain': 'dom0', + 'block_device.return_value.script': None, + 'block_device.return_value.path': '/tmp/kernel/modules.img', + 'block_device.return_value.devtype': 'disk', + 'block_device.return_value.name': 'kernel', + }) + dom0.events_enabled = True + self.app.vmm.offline_mode = False + dev = qubes.devices.DeviceAssignment( + dom0, 'sda', + {'devtype': 'cdrom', 'read-only': 'yes'}, persistent=True) + self.loop.run_until_complete(vm.devices['block'].attach(dev)) + libvirt_xml = vm.create_config_file() + self.assertXMLEqual(lxml.etree.XML(libvirt_xml), + lxml.etree.XML(expected)) + def test_610_libvirt_xml_network(self): expected = ''' test-inst-test diff --git a/templates/libvirt/devices/block.xml b/templates/libvirt/devices/block.xml index a78dca34..add6cb3c 100644 --- a/templates/libvirt/devices/block.xml +++ b/templates/libvirt/devices/block.xml @@ -3,6 +3,9 @@ {%- if 'frontend-dev' in options %} + {%- elif options.get('devtype', 'disk') == 'cdrom' and not vm.kernel %} + + {%- else %} {% set i = i + 1 %}