From d91e881890933363550ffb9e114c6df4c32a4090 Mon Sep 17 00:00:00 2001
From: Mike Lowe <jomlowe@iu.edu>
Date: Mon, 6 Nov 2017 11:06:46 -0500
Subject: [PATCH] live-mig: keep disk device address same

During live migration disk devices are updated with the latest
block device mapping information for volumes.  Previously this
relied on libvirt to assign addresses in order after the already
assigned devices like the root disk had been accounted for.  In
the latest libvirt the unassigned devices are allocated first which
makes the root disk address double allocated causing the migration to
fail.  A running instance should never have the hardware addresses
of its disks changed mid flight.  While disk address changes during
live migration produce fatal errors for the operator it would likely
cause errors inside the instance and unexpected behavior if the device
addresses change during cold migrationt review.  With this disk addresses are no
longer updated with block device mapping information while every
other element of the disk definition for a volume is updated.

Closes-Bug: 1715569

Change-Id: I17af9848f4c0edcbcb101b30e45ca4afa93dcdbb
(cherry picked from commit b196857f04e41dde294eaacc2c1a991807ecc829)
---
 .../tests/unit/virt/libvirt/test_migration.py | 67 +++++++++++++++++++
 nova/virt/libvirt/migration.py                | 15 +++--
 2 files changed, 77 insertions(+), 5 deletions(-)

diff --git a/nova/tests/unit/virt/libvirt/test_migration.py b/nova/tests/unit/virt/libvirt/test_migration.py
index ddb7d23ca1..83abbd6386 100644
--- a/nova/tests/unit/virt/libvirt/test_migration.py
+++ b/nova/tests/unit/virt/libvirt/test_migration.py
@@ -243,6 +243,73 @@ class UtilityMigrationTestCase(test.NoDBTestCase):
                               'ip-1.2.3.4:3260-iqn.cde.67890.opst-lun-Z')
         self.assertThat(res, matchers.XMLMatches(new_xml))
 
+    def test_update_volume_xml_keeps_address(self):
+        # Now test to make sure address isn't altered for virtio-scsi and rbd
+        connection_info = {
+            'driver_volume_type': 'rbd',
+            'serial': 'd299a078-f0db-4993-bf03-f10fe44fd192',
+            'data': {
+                'access_mode': 'rw',
+                'secret_type': 'ceph',
+                'name': 'cinder-volumes/volume-d299a078',
+                'encrypted': False,
+                'discard': True,
+                'cluster_name': 'ceph',
+                'secret_uuid': '1a790a26-dd49-4825-8d16-3dd627cf05a9',
+                'qos_specs': None,
+                'auth_enabled': True,
+                'volume_id': 'd299a078-f0db-4993-bf03-f10fe44fd192',
+                'hosts': ['172.16.128.101', '172.16.128.121'],
+                'auth_username': 'cinder',
+                'ports': ['6789', '6789', '6789']}}
+        bdm = objects.LibvirtLiveMigrateBDMInfo(
+            serial='d299a078-f0db-4993-bf03-f10fe44fd192',
+            bus='scsi', type='disk', dev='sdc',
+            connection_info=connection_info)
+        data = objects.LibvirtLiveMigrateData(
+            target_connect_addr=None,
+            bdms=[bdm],
+            block_migration=False)
+        xml = """<domain>
+ <devices>
+    <disk type='network' device='disk'>
+      <driver name='qemu' type='raw' cache='writeback' discard='unmap'/>
+      <auth username='cinder'>
+        <secret type='ceph' uuid='1a790a26-dd49-4825-8d16-3dd627cf05a9'/>
+      </auth>
+      <source protocol='rbd' name='cinder-volumes/volume-d299a078'>
+        <host name='172.16.128.101' port='6789'/>
+        <host name='172.16.128.121' port='6789'/>
+      </source>
+      <backingStore/>
+      <target dev='sdb' bus='scsi'/>
+      <serial>d299a078-f0db-4993-bf03-f10fe44fd192</serial>
+      <alias name='scsi0-0-0-1'/>
+      <address type='drive' controller='0' bus='0' target='0' unit='1'/>
+    </disk>
+ </devices>
+</domain>"""
+        conf = vconfig.LibvirtConfigGuestDisk()
+        conf.source_device = bdm.type
+        conf.driver_name = "qemu"
+        conf.driver_format = "raw"
+        conf.driver_cache = "writeback"
+        conf.target_dev = bdm.dev
+        conf.target_bus = bdm.bus
+        conf.serial = bdm.connection_info.get('serial')
+        conf.source_type = "network"
+        conf.driver_discard = 'unmap'
+        conf.device_addr = vconfig.LibvirtConfigGuestDeviceAddressDrive()
+        conf.device_addr.controller = 0
+
+        get_volume_config = mock.MagicMock(return_value=conf)
+        doc = etree.fromstring(xml)
+        res = etree.tostring(migration._update_volume_xml(
+            doc, data, get_volume_config), encoding='unicode')
+        new_xml = xml.replace('sdb',
+                              'sdc')
+        self.assertThat(res, matchers.XMLMatches(new_xml))
+
     def test_update_perf_events_xml(self):
         data = objects.LibvirtLiveMigrateData(
             supported_perf_events=['cmt'])
diff --git a/nova/virt/libvirt/migration.py b/nova/virt/libvirt/migration.py
index aa9823928d..abe4e71623 100644
--- a/nova/virt/libvirt/migration.py
+++ b/nova/virt/libvirt/migration.py
@@ -161,15 +161,20 @@ def _update_volume_xml(xml_doc, migrate_data, get_volume_config):
                 # If source and destination have same item, update
                 # the item using destination value.
                 for item_dst in xml_doc2.findall(item_src.tag):
-                    disk_dev.remove(item_src)
-                    item_dst.tail = None
-                    disk_dev.insert(cnt, item_dst)
+                    if item_dst.tag != 'address':
+                        # hw address presented to guest must never change,
+                        # especially during live migration as it can be fatal
+                        disk_dev.remove(item_src)
+                        item_dst.tail = None
+                        disk_dev.insert(cnt, item_dst)
 
             # If destination has additional items, thses items should be
             # added here.
             for item_dst in list(xml_doc2):
-                item_dst.tail = None
-                disk_dev.insert(cnt, item_dst)
+                if item_dst.tag != 'address':
+                    # again, hw address presented to guest must never change
+                    item_dst.tail = None
+                    disk_dev.insert(cnt, item_dst)
     return xml_doc
 
 
-- 
2.24.1

