Merge "libvirt: Ensure device alias is present"

nova/tests/fixtures/libvirt.py

@@ -917,6 +917,10 @@ def _parse_disk_info(element):
         disk_info['target_dev'] = target.get('dev')
         disk_info['target_bus'] = target.get('bus')
 
+    alias = element.find('./alias')
+    if alias is not None:
+        disk_info['alias'] = alias.get('name')
+
     return disk_info
 
 
@@ -962,6 +966,10 @@ def _parse_nic_info(element):
     if target is not None:
         nic_info['target_dev'] = target.get('dev')
 
+    alias = element.find('./alias')
+    if alias is not None:
+        nic_info['alias'] = alias.get('name')
+
     return nic_info
 
 
@@ -979,6 +987,10 @@ def _parse_hostdev_info(element):
             hostdev_info['slot'] = address.get('slot')
             hostdev_info['function'] = address.get('function')
 
+    alias = element.find('./alias')
+    if alias is not None:
+        hostdev_info['alias'] = alias.get('name')
+
     return hostdev_info
 
 
@@ -1222,8 +1234,16 @@ class Domain(object):
         if device_nodes is not None:
             disks_info = []
             disks = device_nodes.findall('./disk')
-            for disk in disks:
-                disks_info += [_parse_disk_info(disk)]
+            for idx, disk in enumerate(disks):
+                disk_info = _parse_disk_info(disk)
+
+                alias = disk.find('./alias')
+                if alias is not None:
+                    disk_info['alias'] = alias.get('name')
+                else:
+                    disk_info['alias'] = f'disk{idx}'
+
+                disks_info.append(disk_info)
             devices['disks'] = disks_info
 
             # Manage shares
@@ -1235,7 +1255,7 @@ class Domain(object):
 
             nics_info = []
             nics = device_nodes.findall('./interface')
-            for nic in nics:
+            for idx, nic in enumerate(nics):
                 nic_info = {}
                 nic_info['type'] = nic.get('type')
 
@@ -1243,6 +1263,12 @@ class Domain(object):
                 if mac is not None:
                     nic_info['mac'] = mac.get('address')
 
+                alias = nic.find('./alias')
+                if alias is not None:
+                    nic_info['alias'] = alias.get('name')
+                else:
+                    nic_info['alias'] = f'net{idx}'
+
                 source = nic.find('./source')
                 if source is not None:
                     if nic_info['type'] == 'network':
@@ -1409,16 +1435,25 @@ class Domain(object):
         if xml.startswith("<disk"):
             disk_info = _parse_disk_info(etree.fromstring(xml))
             disk_info['_attached'] = True
+            if 'alias' not in disk_info:
+                idx = len(self._def['devices']['disks'])
+                disk_info['alias'] = f'disk{idx}'
             self._def['devices']['disks'] += [disk_info]
             result = True
         elif xml.startswith("<interface"):
             nic_info = _parse_nic_info(etree.fromstring(xml))
             nic_info['_attached'] = True
+            if 'alias' not in nic_info:
+                idx = len(self._def['devices']['nics'])
+                nic_info['alias'] = f'net{idx}'
             self._def['devices']['nics'] += [nic_info]
             result = True
         elif xml.startswith("<hostdev"):
             hostdev_info = _parse_hostdev_info(etree.fromstring(xml))
             hostdev_info['_attached'] = True
+            if 'alias' not in hostdev_info:
+                idx = len(self._def['devices']['hostdevs'])
+                hostdev_info['alias'] = f'hostdev{idx}'
             self._def['devices']['hostdevs'] += [hostdev_info]
             result = True
         else:
@@ -1518,7 +1553,9 @@ class Domain(object):
             strformat = """
     <disk type='%(type)s' device='%(device)s'>
       <driver name='%(driver_name)s' type='%(driver_type)s'/>
+      <alias name='%(alias)s'/>
       <source %(source_attr)s='%(source)s'"""
+
             if 'encryption_format' not in disk:
                 strformat += '/>'
             else:
@@ -1555,6 +1592,7 @@ class Domain(object):
                 # this branch covers kernel ovs interfaces
                 nics += '''<interface type='%(type)s'>
           <mac address='%(mac)s'/>
+          <alias name='%(alias)s'/>
           <target dev='tap274487d1-6%(func)s'/>
           <address type='pci' domain='0x0000' bus='0x00' slot='0x03'
                    function='0x%(func)s'/>
@@ -1563,6 +1601,7 @@ class Domain(object):
                 # this branch covers hardware offloaded ovs with vdpa
                 nics += '''<interface type='%(type)s'>
           <mac address='%(mac)s'/>
+          <alias name='%(alias)s'/>
           <source dev='%(source)s'/>
           <address type='pci' domain='0x0000' bus='0x00' slot='0x03'
                    function='0x%(func)s'/>
@@ -1572,6 +1611,7 @@ class Domain(object):
             elif 'source' in nic:
                 nics += '''<interface type='%(type)s'>
           <mac address='%(mac)s'/>
+          <alias name='%(alias)s'/>
           <source %(type)s='%(source)s'/>
           <target dev='tap274487d1-6%(func)s'/>
           <address type='pci' domain='0x0000' bus='0x00' slot='0x03'
@@ -1584,6 +1624,7 @@ class Domain(object):
                 # the fixture we hard code it.
                 nics += '''<interface type='%(type)s'>
           <mac address='%(mac)s'/>
+          <alias name='%(alias)s'/>
           <source dev='fake_pf_interface_name' mode='passthrough'>
               <address type='pci' domain='0x0000' bus='0x81' slot='0x00'
                    function='0x%(func)s'/>

nova/tests/unit/virt/libvirt/test_driver.py

@@ -10726,17 +10726,22 @@ class LibvirtConnTestCase(test.NoDBTestCase,
         drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
         instance = objects.Instance(**self.test_instance)
         volume_id = uuids.volume
-        alias_xml = '<alias name="%s"/>' % vconfig.make_libvirt_device_alias(
-            volume_id)
+        if use_alias:
+            alias = vconfig.make_libvirt_device_alias(volume_id)
+        else:
+            # if we haven't created our own alias, libvirt will generate one
+            # for us
+            alias = 'virtio-disk0'
+
         mock_xml_with_disk = """<domain>
   <devices>
     <disk type='file'>
-      %(alias)s
+      <alias name="%(alias)s"/>
       <source file='/path/to/fake-volume'/>
       <target dev='vdc' bus='virtio'/>
     </disk>
   </devices>
-</domain>""" % {'alias': use_alias and alias_xml or ''}
+</domain>""" % {'alias': alias}
         mock_xml_without_disk = """<domain>
   <devices>
   </devices>
@@ -10781,10 +10786,10 @@ class LibvirtConnTestCase(test.NoDBTestCase,
 
                 mock_get_domain.assert_called_with(instance)
                 xml = """<disk type="file" device="disk">
-                            %(alias)s
+                            <alias name="%(alias)s"/>
                             <source file="/path/to/fake-volume"/>
                             <target bus="virtio" dev="vdc"/>
-                        </disk>""" % {'alias': use_alias and alias_xml or ''}
+                        </disk>""" % {'alias': alias}
                 # we expect two separate detach calls
                 self.assertEqual(2, mock_dom.detachDeviceFlags.call_count)
                 # one for the persistent domain
@@ -26094,6 +26099,7 @@ class LibvirtDriverTestCase(test.NoDBTestCase, TraitsComparisonMixin):
                     <devices>
                         <interface type='bridge'>
                             <mac address='52:54:00:f6:35:8f'/>
+                            <alias name='bridge0'/>
                             <model type='virtio'/>
                             <source bridge='br0'/>
                             <target dev='tap12345678'/>
@@ -26109,6 +26115,7 @@ class LibvirtDriverTestCase(test.NoDBTestCase, TraitsComparisonMixin):
         expected_cfg.parse_str("""
             <interface type='bridge'>
               <mac address='52:54:00:f6:35:8f'/>
+              <alias name='bridge0'/>
               <model type='virtio'/>
               <source bridge='br0'/>
               <target dev='tap12345678'/>
@@ -26250,6 +26257,7 @@ class LibvirtDriverTestCase(test.NoDBTestCase, TraitsComparisonMixin):
                     <devices>
                         <interface type='bridge'>
                             <mac address='52:54:00:f6:35:8f'/>
+                            <alias name='bridge0'/>
                             <model type='virtio'/>
                             <source bridge='br0'/>
                             <target dev='tap12345678'/>
@@ -26258,6 +26266,7 @@ class LibvirtDriverTestCase(test.NoDBTestCase, TraitsComparisonMixin):
                         </interface>
                         <interface type='bridge'>
                             <mac address='52:54:00:f6:35:8f'/>
+                            <alias name='bridge1'/>
                             <model type='virtio'/>
                             <source bridge='br1'/>
                             <target dev='tap87654321'/>
@@ -26272,6 +26281,7 @@ class LibvirtDriverTestCase(test.NoDBTestCase, TraitsComparisonMixin):
         expected.parse_str("""
             <interface type='bridge'>
                 <mac address='52:54:00:f6:35:8f'/>
+                <alias name='bridge0'/>
                 <model type='virtio'/>
                 <source bridge='br0'/>
                 <target dev='tap12345678'/>

nova/virt/libvirt/driver.py

@@ -2673,6 +2673,12 @@ class LibvirtDriver(driver.ComputeDriver):
             synchronously.
         :raises DeviceDetachFailed: if libvirt sent DeviceRemovalFailedEvent
         """
+        if dev.alias is None:
+            # our event handler needs a device alias, and we should never get
+            # here without one
+            msg = 'Device %s has no alias. This should not happen.'
+            raise exception.InternalError(msg % dev)
+
         # So we will issue an detach to libvirt and we will wait for an
         # event from libvirt about the result. We need to set up the event
         # handling before the detach to avoid missing the event if libvirt