From d91e881890933363550ffb9e114c6df4c32a4090 Mon Sep 17 00:00:00 2001 From: Mike Lowe 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 ddb7d23ca15a..83abbd638635 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 = """ + + + + + + + + + + + + + d299a078-f0db-4993-bf03-f10fe44fd192 + +
+ + +""" + 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 aa9823928de1..abe4e7162338 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