Merge remote-tracking branch 'origin/pr/296'

* origin/pr/296:
  ext/block: prefer connecting cdrom as xvdd
  tests: extend mock objects in QubesVM tests

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 <target dev=.../>
         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(

qubes/tests/devices_block.py

@@ -25,6 +25,15 @@ import jinja2
 import qubes.tests
 import qubes.ext.block
 
+modules_disk = '''
+    <disk type='block' device='disk'>
+      <driver name='phy'/>
+      <source dev='/var/lib/qubes/vm-kernels/4.4.55-11/modules.img'/>
+      <backingStore/>
+      <target dev='xvdd' bus='xen'/>
+      <readonly/>
+    </disk>
+'''
 
 domain_xml_template = '''
 <domain type='xen' id='9'>
@@ -66,13 +75,6 @@ domain_xml_template = '''
       <backingStore/>
       <target dev='xvdc' bus='xen'/>
     </disk>
-    <disk type='block' device='disk'>
-      <driver name='phy'/>
-      <source dev='/var/lib/qubes/vm-kernels/4.4.55-11/modules.img'/>
-      <backingStore/>
-      <target dev='xvdd' bus='xen'/>
-      <readonly/>
-    </disk>
     {}
     <interface type='ethernet'>
       <mac address='00:16:3e:5e:6c:06'/>
@@ -486,6 +488,46 @@ class TC_00_Block(qubes.tests.QubesTestCase):
             '</disk>')
         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 = (
+            '<disk type="block" device="cdrom">\n'
+            '    <driver name="phy" />\n'
+            '    <source dev="/dev/sda" />\n'
+            '        <target dev="xvdi" />\n'
+            '        <readonly />\n'
+            '    <backenddomain name="sys-usb" />\n'
+            '</disk>')
+        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 = (
+            '<disk type="block" device="cdrom">\n'
+            '    <driver name="phy" />\n'
+            '    <source dev="/dev/sda" />\n'
+            '        <target dev="xvdd" />\n'
+            '        <readonly />\n'
+            '    <backenddomain name="sys-usb" />\n'
+            '</disk>')
+        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'',

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
@@ -1199,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 = '''<domain type="xen">
+        <name>test-inst-test</name>
+        <uuid>7db78950-c467-4863-94d1-af59806384ea</uuid>
+        <memory unit="MiB">400</memory>
+        <currentMemory unit="MiB">400</currentMemory>
+        <vcpu placement="static">2</vcpu>
+        <cpu mode='host-passthrough'>
+            <!-- disable nested HVM -->
+            <feature name='vmx' policy='disable'/>
+            <feature name='svm' policy='disable'/>
+            <!-- disable SMAP inside VM, because of Linux bug -->
+            <feature name='smap' policy='disable'/>
+        </cpu>
+        <os>
+            <type arch="x86_64" machine="xenfv">hvm</type>
+                <!--
+                     For the libxl backend libvirt switches between OVMF (UEFI)
+                     and SeaBIOS based on the loader type. This has nothing to
+                     do with the hvmloader binary.
+                -->
+            <loader type="rom">hvmloader</loader>
+            <boot dev="cdrom" />
+            <boot dev="hd" />
+        </os>
+        <features>
+            <pae/>
+            <acpi/>
+            <apic/>
+            <viridian/>
+        </features>
+        <clock offset="variable" adjustment="0" basis="localtime" />
+        <on_poweroff>destroy</on_poweroff>
+        <on_reboot>destroy</on_reboot>
+        <on_crash>destroy</on_crash>
+        <devices>
+            <disk type="block" device="cdrom">
+                <driver name="phy" />
+                <source dev="/dev/sda" />
+                <!-- prefer xvdd for CDROM -->
+                <target dev="xvdd" />
+                <readonly/>
+            </disk>
+            <!-- server_ip is the address of stubdomain. It hosts it's own DNS server. -->
+            <emulator type="stubdom-linux" />
+            <input type="tablet" bus="usb"/>
+            <video>
+                <model type="vga"/>
+            </video>
+            <graphics type="qubes"/>
+            <console type="pty">
+                <target type="xen" port="0"/>
+            </console>
+        </devices>
+        </domain>
+        '''
+        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 = '''<domain type="xen">
+        <name>test-inst-test</name>
+        <uuid>7db78950-c467-4863-94d1-af59806384ea</uuid>
+        <memory unit="MiB">400</memory>
+        <currentMemory unit="MiB">400</currentMemory>
+        <vcpu placement="static">2</vcpu>
+        <cpu mode='host-passthrough'>
+            <!-- disable nested HVM -->
+            <feature name='vmx' policy='disable'/>
+            <feature name='svm' policy='disable'/>
+            <!-- disable SMAP inside VM, because of Linux bug -->
+            <feature name='smap' policy='disable'/>
+        </cpu>
+        <os>
+            <type arch="x86_64" machine="xenfv">hvm</type>
+                <!--
+                     For the libxl backend libvirt switches between OVMF (UEFI)
+                     and SeaBIOS based on the loader type. This has nothing to
+                     do with the hvmloader binary.
+                -->
+            <loader type="rom">hvmloader</loader>
+            <boot dev="cdrom" />
+            <boot dev="hd" />
+            <cmdline>root=/dev/mapper/dmroot ro nomodeset console=hvc0 rd_NO_PLYMOUTH rd.plymouth.enable=0 plymouth.enable=0 nopat</cmdline>
+        </os>
+        <features>
+            <pae/>
+            <acpi/>
+            <apic/>
+            <viridian/>
+        </features>
+        <clock offset="variable" adjustment="0" basis="localtime" />
+        <on_poweroff>destroy</on_poweroff>
+        <on_reboot>destroy</on_reboot>
+        <on_crash>destroy</on_crash>
+        <devices>
+            <disk type="block" device="disk">
+                <driver name="phy" />
+                <source dev="/tmp/kernel/modules.img" />
+                <target dev="xvdd" />
+                <backenddomain name="dom0" />
+            </disk>
+            <disk type="block" device="cdrom">
+                <driver name="phy" />
+                <source dev="/dev/sda" />
+                <target dev="xvdi" />
+                <readonly/>
+            </disk>
+            <!-- server_ip is the address of stubdomain. It hosts it's own DNS server. -->
+            <emulator type="stubdom-linux" />
+            <input type="tablet" bus="usb"/>
+            <video>
+                <model type="vga"/>
+            </video>
+            <graphics type="qubes"/>
+            <console type="pty">
+                <target type="xen" port="0"/>
+            </console>
+        </devices>
+        </domain>
+        '''
+        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 = '''<domain type="xen">
         <name>test-inst-test</name>

templates/libvirt/devices/block.xml

@@ -3,6 +3,9 @@
     <source dev="{{ device.device_node }}" />
     {%- if 'frontend-dev' in options %}
         <target dev="{{ options.get('frontend-dev') }}" />
+    {%- elif options.get('devtype', 'disk') == 'cdrom' and not vm.kernel %}
+        <!-- prefer xvdd for CDROM -->
+        <target dev="xvdd" />
     {%- else %}
         <target dev="xvd{{dd[i]}}" />
         {% set i = i + 1 %}