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

* origin/pr/313:
  Fix overlapping block device names

qubes/tests/vm/qubesvm.py

@@ -1469,6 +1469,150 @@ class TC_90_QubesVM(QubesVMTestsMixin, qubes.tests.QubesTestCase):
                     extra_ip='<ip address="{}::a89:1" family=\'ipv6\'/>'.format(
                         qubes.config.qubes_ipv6_prefix.replace(':0000', '')))))
 
+    def test_615_libvirt_xml_block_devices(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="disk">
+                <driver name="phy" />
+                <source dev="/dev/loop0" />
+                <target dev="xvda" />
+                <backenddomain name="dom0" />
+                <script path="/tmp/script" />
+            </disk>
+            <disk type="block" device="disk">
+                <driver name="phy" />
+                <source dev="/dev/loop1" />
+                <target dev="xvde" />
+                <backenddomain name="dom0" />
+            </disk>
+            <disk type="block" device="disk">
+                <driver name="phy" />
+                <source dev="/dev/loop2" />
+                <target dev="xvdf" />
+                <backenddomain name="dom0" />
+            </disk>
+
+            <disk type="block" device="disk">
+                <driver name="phy" />
+                <source dev="/dev/sdb" />
+                <target dev="xvdl" />
+            </disk>
+            <disk type="block" device="cdrom">
+                <driver name="phy" />
+                <source dev="/dev/sda" />
+                <!-- prefer xvdd for CDROM -->
+                <target dev="xvdd" />
+            </disk>
+            <disk type="block" device="disk">
+                <driver name="phy" />
+                <source dev="/dev/loop0" />
+                <target dev="xvdi" />
+                <backenddomain name="backend0" />
+            </disk>
+            <disk type="block" device="disk">
+                <driver name="phy" />
+                <source dev="/dev/loop0" />
+                <target dev="xvdj" />
+                <backenddomain name="backend1" />
+            </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'
+        vm = self.get_vm(uuid=my_uuid)
+        vm.netvm = None
+        vm.virt_mode = 'hvm'
+        vm.volumes['root'] = unittest.mock.Mock(**{
+            'block_device.return_value.name': 'root',
+            'block_device.return_value.path': '/dev/loop0',
+            'block_device.return_value.devtype': 'disk',
+            'block_device.return_value.domain': 'dom0',
+            'block_device.return_value.script': '/tmp/script',
+        })
+        vm.volumes['other'] = unittest.mock.Mock(**{
+            'block_device.return_value.name': 'other',
+            'block_device.return_value.path': '/dev/loop1',
+            'block_device.return_value.devtype': 'disk',
+            'block_device.return_value.domain': 'dom0',
+            'block_device.return_value.script': None,
+        })
+        vm.volumes['other2'] = unittest.mock.Mock(**{
+            'block_device.return_value.name': 'other',
+            'block_device.return_value.path': '/dev/loop2',
+            'block_device.return_value.devtype': 'disk',
+            'block_device.return_value.domain': 'dom0',
+            'block_device.return_value.script': None,
+        })
+        assignments = [
+            unittest.mock.Mock(**{
+                'options': {'frontend-dev': 'xvdl'},
+                'device.device_node': '/dev/sdb',
+                'device.backend_domain.name': 'dom0',
+            }),
+            unittest.mock.Mock(**{
+                'options': {'devtype': 'cdrom'},
+                'device.device_node': '/dev/sda',
+                'device.backend_domain.name': 'dom0',
+            }),
+            unittest.mock.Mock(**{
+                'options': {'read-only': True},
+                'device.device_node': '/dev/loop0',
+                'device.backend_domain.name': 'backend0',
+            }),
+            unittest.mock.Mock(**{
+                'options': {},
+                'device.device_node': '/dev/loop0',
+                'device.backend_domain.name': 'backend1',
+            }),
+        ]
+        vm.devices['block'].assignments = lambda persistent: assignments
+        libvirt_xml = vm.create_config_file()
+        self.assertXMLEqual(lxml.etree.XML(libvirt_xml),
+            lxml.etree.XML(expected))
+
     @unittest.mock.patch('qubes.utils.get_timezone')
     @unittest.mock.patch('qubes.utils.urandom')
     @unittest.mock.patch('qubes.vm.qubesvm.QubesVM.untrusted_qdb')

templates/libvirt/devices/block.xml

@@ -7,8 +7,8 @@
         <!-- prefer xvdd for CDROM -->
         <target dev="xvdd" />
     {%- else %}
-        <target dev="xvd{{dd[i]}}" />
+        <target dev="xvd{{dd[counter.i]}}" />
-        {% set i = i + 1 %}
+        {% if counter.update({'i': counter.i + 1}) %}{% endif %}
     {%- endif %}
 
     {%- if options.get('read-only', 'no') == 'yes' %}

templates/libvirt/xen.xml

@@ -96,7 +96,15 @@
 
     <devices>
         {% block devices %}
-            {% set i = 0 %}
+            {#
+                HACK: The letter counter is implemented in this way because
+                Jinja does not allow you to increment variables in a loop
+                anymore. As of Jinja 2.10, we will be able to replace this
+                with:
+                {% set counter = namespace(i=0) %}
+                {% set counter.i = counter.i + 1 %}
+            #}
+            {% set counter = {'i': 0} %}
             {# TODO Allow more volumes out of the box #}
             {% set dd = ['e', 'f', 'g', 'h', 'i', 'j', 'k', 'l', 'm', 'n', 'o', 'p',
                 'q', 'r', 's', 't', 'u', 'v', 'w', 'x', 'y']
@@ -114,8 +122,8 @@
                     {% elif device.name == 'kernel' %}
                         <target dev="xvdd" />
                     {% else %}
-                        <target dev="xvd{{dd[i]}}" />
+                        <target dev="xvd{{dd[counter.i]}}" />
-                        {% set i = i + 1 %}
+                        {% if counter.update({'i': counter.i + 1}) %}{% endif %}
                     {% endif %}
 
                     {% if not device.rw %}
@@ -133,7 +141,7 @@
             {% endfor %}
 
             {# start external devices from xvdi #}
-            {% set i = 4 %}
+            {% set counter = {'i': 4} %}
             {% for assignment in vm.devices.block.assignments(True) %}
                 {% set device = assignment.device %}
                 {% set options = assignment.options %}