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

@@ -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

@@ -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 = '''<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 %}