Resolve conflicting mac address in resize

This patch fixes bug 1161226 and mac address conflicts when migrating or
resizing an instance using the PowerVM driver. This will ensure that if
a migration or resize is taking place on the same host, a temporary mac
address will be assigned to the original LPAR so there is no conflict on
the VIOS when a second LPAR is started with the same address.

Change-Id: I59db484defb789f3a7c0eddbd5ef9e92efc22419

3 files changed, 108 insertions, 1 deletions

diff --git a/nova/tests/test_powervm.py b/nova/tests/test_powervm.py
index 946fd15a8..0ba9dce9a 100644
--- a/nova/tests/test_powervm.py
+++ b/nova/tests/test_powervm.py
@@ -104,6 +104,18 @@ class FakeIVMOperator(object):
     def rename_lpar(self, old, new):
         pass
 
+    def set_lpar_mac_base_value(self, instance_name, mac):
+        pass
+
+    def get_logical_vol_size(self, diskname):
+        pass
+
+    def macs_for_instance(self, instance):
+        return set(['FA:98:64:2B:29:39'])
+
+    def run_vios_command(self, cmd):
+        pass
+
 
 class FakeBlockAdapter(powervm_blockdev.PowerVMLocalVolumeAdapter):
 
@@ -281,12 +293,16 @@ class PowerVMDriverTestCase(test.TestCase):
 
     def _test_finish_revert_migration_after_crash(self, backup_made, new_made):
         inst = {'name': 'foo'}
+        network_info = []
+        network_info.append({'address': 'fa:89:f0:8b:9b:39'})
 
         self.mox.StubOutWithMock(self.powervm_connection, 'instance_exists')
         self.mox.StubOutWithMock(self.powervm_connection._powervm, 'destroy')
         self.mox.StubOutWithMock(self.powervm_connection._powervm._operator,
                                  'rename_lpar')
         self.mox.StubOutWithMock(self.powervm_connection._powervm, 'power_on')
+        self.mox.StubOutWithMock(self.powervm_connection._powervm._operator,
+                                 'set_lpar_mac_base_value')
 
         self.powervm_connection.instance_exists('rsz_foo').AndReturn(
             backup_made)
@@ -297,11 +313,14 @@ class PowerVMDriverTestCase(test.TestCase):
                 self.powervm_connection._powervm.destroy('foo')
             self.powervm_connection._powervm._operator.rename_lpar('rsz_foo',
                                                                    'foo')
+        self.powervm_connection._powervm._operator.set_lpar_mac_base_value(
+                'foo', 'fa:89:f0:8b:9b:39')
         self.powervm_connection._powervm.power_on('foo')
 
         self.mox.ReplayAll()
 
-        self.powervm_connection.finish_revert_migration(inst, [])
+        self.powervm_connection.finish_revert_migration(inst, network_info,
+                                                    block_device_info=None)
 
     def test_finish_revert_migration_after_crash(self):
         self._test_finish_revert_migration_after_crash(True, True)
@@ -349,6 +368,62 @@ class PowerVMDriverTestCase(test.TestCase):
         expected_path = 'some/image/path/logical-vol-name_rsz.gz'
         self.assertEqual(file_path, expected_path)
 
+    def test_set_lpar_mac_base_value(self):
+        instance = self.instance
+        context = 'fake_context'
+        dest = '10.8.46.20'  # Some fake dest IP
+        instance_type = 'fake_instance_type'
+        network_info = []
+        network_info.append({'address': 'fa:89:f0:8b:9b:39'})
+        block_device_info = None
+        self.flags(powervm_mgr=dest)
+        fake_noop = lambda *args, **kwargs: None
+        fake_op = self.powervm_connection._powervm._operator
+        self.stubs.Set(fake_op, 'get_vhost_by_instance_id', fake_noop)
+        self.stubs.Set(fake_op, 'get_disk_name_by_vhost', fake_noop)
+        self.stubs.Set(self.powervm_connection._powervm, 'power_off',
+                       fake_noop)
+        self.stubs.Set(fake_op, 'get_logical_vol_size',
+                       lambda *args, **kwargs: '20')
+        self.stubs.Set(self.powervm_connection, '_get_resize_name', fake_noop)
+        self.stubs.Set(fake_op, 'rename_lpar', fake_noop)
+
+        def fake_migrate_disk(*args, **kwargs):
+            disk_info = {}
+            disk_info['fake_dict'] = 'some/file/path.gz'
+            return disk_info
+
+        def fake_set_lpar_mac_base_value(inst_name, mac, *args, **kwargs):
+            # get expected mac address from FakeIVM set
+            fake_ivm = FakeIVMOperator()
+            exp_mac = fake_ivm.macs_for_instance(inst_name).pop()
+            self.assertEqual(exp_mac, mac)
+
+        self.stubs.Set(self.powervm_connection._powervm, 'migrate_disk',
+                       fake_migrate_disk)
+        self.stubs.Set(fake_op, 'set_lpar_mac_base_value',
+                       fake_set_lpar_mac_base_value)
+        disk_info = self.powervm_connection.migrate_disk_and_power_off(
+                        context, instance,
+                        dest, instance_type, network_info, block_device_info)
+
+    def test_set_lpar_mac_base_value_command(self):
+        inst_name = 'some_instance'
+        mac = 'FA:98:64:2B:29:39'
+        exp_mac_str = mac[:-2].replace(':', '').lower()
+
+        def fake_run_vios_command(cmd, *args, **kwargs):
+            exp_cmd = ('chsyscfg -r lpar -i "name=%(inst_name)s, ',
+                       'virtual_eth_mac_base_value=%(exp_mac_str)s"' %
+                       locals())
+            assertEqual(exp_cmd, cmd)
+
+        self.stubs.Set(self.powervm_connection._powervm._operator,
+                       'run_vios_command', fake_run_vios_command)
+
+        fake_op = self.powervm_connection._powervm
+        fake_op._operator.set_lpar_mac_base_value(inst_name, mac)
+
     def test_migrate_build_scp_command(self):
         lv_name = 'logical-vol-name'
         src_host = 'compute_host_1'
diff --git a/nova/virt/powervm/driver.py b/nova/virt/powervm/driver.py
index c193111c8..603ee0b61 100755
--- a/nova/virt/powervm/driver.py
+++ b/nova/virt/powervm/driver.py
@@ -240,6 +240,16 @@ class PowerVMDriver(driver.ComputeDriver):
         diskname = pvm_op.get_disk_name_by_vhost(vhost)
 
         self._powervm.power_off(instance['name'], timeout=120)
+        # NOTE(ldbragst) Here we need to check if the resize or migrate is
+        # happening on the same host. If yes, then we need to assign a temp
+        # mac address to the source LPAR so we don't have a conflict when
+        # another LPAR is booted with the same mac address as the
+        # original LPAR
+        if src_host == dest:
+            macs = self.macs_for_instance(instance)
+            temp_mac = macs.pop()
+            self._powervm._operator.set_lpar_mac_base_value(instance['name'],
+                                                            temp_mac)
 
         disk_info = self._powervm.migrate_disk(
                 diskname, src_host, dest, CONF.powervm_img_remote_path,
@@ -307,6 +317,14 @@ class PowerVMDriver(driver.ComputeDriver):
 
         new_name = self._get_resize_name(instance['name'])
 
+        # NOTE(ldbragst) In the case of a resize_revert on the same host
+        # we reassign the original mac address, replacing the temp mac
+        # on the old instance that will be started
+        if (self._powervm.instance_exists(new_name) and
+                self._powervm.instance_exists(instance['name'])):
+            original_mac = network_info[0]['address']
+            self._powervm._operator.set_lpar_mac_base_value(instance['name'],
+                                                            original_mac)
         # Make sure we don't have a failed same-host migration still
         # hanging around
         if self.instance_exists(new_name):
diff --git a/nova/virt/powervm/operator.py b/nova/virt/powervm/operator.py
index 059935652..13012b700 100644
--- a/nova/virt/powervm/operator.py
+++ b/nova/virt/powervm/operator.py
@@ -761,6 +761,20 @@ class BaseOperator(object):
         command = 'rm %s' % file_path
         self.run_vios_command_as_root(command)
 
+    def set_lpar_mac_base_value(self, instance_name, mac):
+        """Set LPAR's property virtual_eth_mac_base_value
+
+        :param instance_name: name of the instance to be set
+        :param mac: mac of virtual ethernet
+        """
+        # NOTE(ldbragst) We only use the base mac value because the last
+        # byte is the slot id of the virtual NIC, which doesn't change.
+        mac_base_value = mac[:-2].replace(':', '')
+        cmd = ' '.join(['chsyscfg -r lpar -i',
+                        '"name=%s,' % instance_name,
+                        'virtual_eth_mac_base_value=%s"' % mac_base_value])
+        self.run_vios_command(cmd)
+
 
 class IVMOperator(BaseOperator):
     """Integrated Virtualization Manager (IVM) Operator.