From 22b8ae0975187a847fc201a750f05be0af9818a4 Mon Sep 17 00:00:00 2001 From: Huan Xie Date: Wed, 24 Aug 2016 02:25:10 +0000 Subject: [PATCH] XenAPI: Fix VM live-migrate with iSCSI SR volume With XenServer 7.0 (XCP 2.1.0), VM live migration with iSCSI volume will fail due to no relax_xsm_sr_check in /etc/xapi.conf in Dom0. But since XS7.0, the default value of relax_xsm_sr_check is true if not configured in /etc/xapi.conf in Dom0. This fix is to add checks on XCP version, if it's equal or greater than 2.1.0, will treat relax_xsm_sr_check=true if not found in /etc/xapi.conf Change-Id: I88d1d384ab7587c428e517d184258bb517dfb4ab Closes-bug: #1610932 --- .../unit/virt/xenapi/client/test_session.py | 72 +++++++++++++++++-- nova/tests/unit/virt/xenapi/stubs.py | 5 +- nova/tests/unit/virt/xenapi/test_vmops.py | 15 ---- nova/tests/unit/virt/xenapi/test_xenapi.py | 18 ++--- nova/virt/xenapi/client/session.py | 28 +++++++- nova/virt/xenapi/vmops.py | 16 +---- 6 files changed, 110 insertions(+), 44 deletions(-) diff --git a/nova/tests/unit/virt/xenapi/client/test_session.py b/nova/tests/unit/virt/xenapi/client/test_session.py index f9b490eca974..10daef99fa38 100644 --- a/nova/tests/unit/virt/xenapi/client/test_session.py +++ b/nova/tests/unit/virt/xenapi/client/test_session.py @@ -25,15 +25,16 @@ from nova.virt.xenapi.client import session class SessionTestCase(stubs.XenAPITestBaseNoDB): + @mock.patch.object(session.XenAPISession, '_get_platform_version') @mock.patch.object(session.XenAPISession, '_create_session') @mock.patch.object(session.XenAPISession, '_get_product_version_and_brand') @mock.patch.object(session.XenAPISession, '_verify_plugin_version') def test_session_passes_version(self, mock_verify, mock_version, - create_session): + create_session, mock_platform_version): sess = mock.Mock() create_session.return_value = sess mock_version.return_value = ('version', 'brand') - + mock_platform_version.return_value = (2, 1, 0) session.XenAPISession('http://someserver', 'username', 'password') expected_version = '%s %s %s' % (version.vendor_string(), @@ -43,21 +44,25 @@ class SessionTestCase(stubs.XenAPITestBaseNoDB): expected_version, 'OpenStack') + @mock.patch.object(session.XenAPISession, '_get_platform_version') @mock.patch('eventlet.timeout.Timeout') @mock.patch.object(session.XenAPISession, '_create_session') @mock.patch.object(session.XenAPISession, '_get_product_version_and_brand') @mock.patch.object(session.XenAPISession, '_verify_plugin_version') def test_session_login_with_timeout(self, mock_verify, mock_version, - create_session, mock_timeout): + create_session, mock_timeout, + mock_platform_version): self.flags(connection_concurrent=2, group='xenserver') sess = mock.Mock() create_session.return_value = sess mock_version.return_value = ('version', 'brand') + mock_platform_version.return_value = (2, 1, 0) session.XenAPISession('http://someserver', 'username', 'password') self.assertEqual(2, sess.login_with_password.call_count) self.assertEqual(2, mock_timeout.call_count) + @mock.patch.object(session.XenAPISession, '_get_platform_version') @mock.patch('eventlet.timeout.Timeout') @mock.patch.object(session.XenAPISession, '_create_session') @mock.patch.object(session.XenAPISession, '_get_product_version_and_brand') @@ -66,7 +71,8 @@ class SessionTestCase(stubs.XenAPITestBaseNoDB): @mock.patch.object(session.XenAPISession, '_get_host_ref') def test_session_raises_exception(self, mock_ref, mock_uuid, mock_verify, mock_version, - create_session, mock_timeout): + create_session, mock_timeout, + mock_platform_version): import XenAPI self.flags(connection_concurrent=2, group='xenserver') sess = mock.Mock() @@ -76,11 +82,69 @@ class SessionTestCase(stubs.XenAPITestBaseNoDB): sess.login_with_password.side_effect = [ XenAPI.Failure(['HOST_IS_SLAVE', 'master']), None, None] mock_version.return_value = ('version', 'brand') + mock_platform_version.return_value = (2, 1, 0) session.XenAPISession('http://slave', 'username', 'password') self.assertEqual(3, sess.login_with_password.call_count) self.assertEqual(3, mock_timeout.call_count) + @mock.patch.object(session.XenAPISession, 'call_plugin') + @mock.patch.object(session.XenAPISession, '_get_software_version') + @mock.patch.object(session.XenAPISession, '_verify_plugin_version') + @mock.patch.object(session.XenAPISession, '_create_session') + def test_relax_xsm_sr_check_true(self, mock_create_session, + mock_verify_plugin_version, + mock_get_software_version, + mock_call_plugin): + sess = mock.Mock() + mock_create_session.return_value = sess + mock_get_software_version.return_value = {'product_version': '6.5.0', + 'product_brand': 'XenServer', + 'platform_version': '1.9.0'} + # mark relax-xsm-sr-check=True in /etc/xapi.conf + mock_call_plugin.return_value = "True" + xenapi_sess = session.XenAPISession( + 'http://someserver', 'username', 'password') + self.assertTrue(xenapi_sess.is_xsm_sr_check_relaxed()) + + @mock.patch.object(session.XenAPISession, 'call_plugin') + @mock.patch.object(session.XenAPISession, '_get_software_version') + @mock.patch.object(session.XenAPISession, '_verify_plugin_version') + @mock.patch.object(session.XenAPISession, '_create_session') + def test_relax_xsm_sr_check_XS65_missing(self, mock_create_session, + mock_verify_plugin_version, + mock_get_software_version, + mock_call_plugin): + sess = mock.Mock() + mock_create_session.return_value = sess + mock_get_software_version.return_value = {'product_version': '6.5.0', + 'product_brand': 'XenServer', + 'platform_version': '1.9.0'} + # mark no relax-xsm-sr-check setting in /etc/xapi.conf + mock_call_plugin.return_value = "" + xenapi_sess = session.XenAPISession( + 'http://someserver', 'username', 'password') + self.assertFalse(xenapi_sess.is_xsm_sr_check_relaxed()) + + @mock.patch.object(session.XenAPISession, 'call_plugin') + @mock.patch.object(session.XenAPISession, '_get_software_version') + @mock.patch.object(session.XenAPISession, '_verify_plugin_version') + @mock.patch.object(session.XenAPISession, '_create_session') + def test_relax_xsm_sr_check_XS7_missing(self, mock_create_session, + mock_verify_plugin_version, + mock_get_software_version, + mock_call_plugin): + sess = mock.Mock() + mock_create_session.return_value = sess + mock_get_software_version.return_value = {'product_version': '7.0.0', + 'product_brand': 'XenServer', + 'platform_version': '2.1.0'} + # mark no relax-xsm-sr-check in /etc/xapi.conf + mock_call_plugin.return_value = "" + xenapi_sess = session.XenAPISession( + 'http://someserver', 'username', 'password') + self.assertTrue(xenapi_sess.is_xsm_sr_check_relaxed()) + class ApplySessionHelpersTestCase(stubs.XenAPITestBaseNoDB): def setUp(self): diff --git a/nova/tests/unit/virt/xenapi/stubs.py b/nova/tests/unit/virt/xenapi/stubs.py index aa8cd33b9a53..af9489a6f209 100644 --- a/nova/tests/unit/virt/xenapi/stubs.py +++ b/nova/tests/unit/virt/xenapi/stubs.py @@ -56,12 +56,15 @@ def stubout_instance_snapshot(stubs): def stubout_session(stubs, cls, product_version=(5, 6, 2), - product_brand='XenServer', **opt_args): + product_brand='XenServer', platform_version=(1, 9, 0), + **opt_args): """Stubs out methods from XenAPISession.""" stubs.Set(session.XenAPISession, '_create_session', lambda s, url: cls(url, **opt_args)) stubs.Set(session.XenAPISession, '_get_product_version_and_brand', lambda s: (product_version, product_brand)) + stubs.Set(session.XenAPISession, '_get_platform_version', + lambda s: platform_version) def stubout_get_this_vm_uuid(stubs): diff --git a/nova/tests/unit/virt/xenapi/test_vmops.py b/nova/tests/unit/virt/xenapi/test_vmops.py index 1768e0ca5309..ba7ac10da408 100644 --- a/nova/tests/unit/virt/xenapi/test_vmops.py +++ b/nova/tests/unit/virt/xenapi/test_vmops.py @@ -135,21 +135,6 @@ class VMOpsTestCase(VMOpsTestBase): def test_finish_revert_migration_after_crash_before_backup(self): self._test_finish_revert_migration_after_crash(False, False) - def test_xsm_sr_check_relaxed_cached(self): - self.make_plugin_call_count = 0 - - def fake_make_plugin_call(plugin, method, **args): - self.make_plugin_call_count = self.make_plugin_call_count + 1 - return "true" - - self.stubs.Set(self._vmops, "_make_plugin_call", - fake_make_plugin_call) - - self.assertTrue(self._vmops._is_xsm_sr_check_relaxed()) - self.assertTrue(self._vmops._is_xsm_sr_check_relaxed()) - - self.assertEqual(self.make_plugin_call_count, 1) - @mock.patch.object(vm_utils, 'lookup', return_value=None) def test_get_vm_opaque_ref_raises_instance_not_found(self, mock_lookup): instance = {"name": "dummy"} diff --git a/nova/tests/unit/virt/xenapi/test_xenapi.py b/nova/tests/unit/virt/xenapi/test_xenapi.py index de92ae4887e9..babce6c31aaa 100644 --- a/nova/tests/unit/virt/xenapi/test_xenapi.py +++ b/nova/tests/unit/virt/xenapi/test_xenapi.py @@ -3512,10 +3512,11 @@ class XenAPILiveMigrateTestCase(stubs.XenAPITestBaseNoDB): self.stubs.Set(self.conn._vmops, "_get_iscsi_srs", fake_get_iscsi_srs) - def fake_make_plugin_call(plugin, method, **args): - return "true" - self.stubs.Set(self.conn._vmops, "_make_plugin_call", - fake_make_plugin_call) + def fake_is_xsm_sr_check_relaxed(): + return True + self.stubs.Set(self.conn._vmops._session, + 'is_xsm_sr_check_relaxed', + fake_is_xsm_sr_check_relaxed) dest_check_data = objects.XenapiLiveMigrateData( block_migration=True, @@ -3539,10 +3540,11 @@ class XenAPILiveMigrateTestCase(stubs.XenAPITestBaseNoDB): self.stubs.Set(self.conn._vmops, "_get_iscsi_srs", fake_get_iscsi_srs) - def fake_make_plugin_call(plugin, method, **args): - return {'returncode': 'error', 'message': 'Plugin not found'} - self.stubs.Set(self.conn._vmops, "_make_plugin_call", - fake_make_plugin_call) + def fake_is_xsm_sr_check_relaxed(): + return False + self.stubs.Set(self.conn._vmops._session, + 'is_xsm_sr_check_relaxed', + fake_is_xsm_sr_check_relaxed) self.assertRaises(exception.MigrationError, self.conn.check_can_live_migrate_source, diff --git a/nova/virt/xenapi/client/session.py b/nova/virt/xenapi/client/session.py index 1be804fd94b5..566fd150c494 100644 --- a/nova/virt/xenapi/client/session.py +++ b/nova/virt/xenapi/client/session.py @@ -94,8 +94,9 @@ class XenAPISession(object): self.host_ref = self._get_host_ref() self.product_version, self.product_brand = \ self._get_product_version_and_brand() - self._verify_plugin_version() + self.platform_version = self._get_platform_version() + self._cached_xsm_sr_relaxed = None apply_session_helpers(self) @@ -174,6 +175,15 @@ class XenAPISession(object): return product_version, product_brand + def _get_platform_version(self): + """Return a tuple of (major, minor, rev) for the host version""" + software_version = self._get_software_version() + platform_version_str = software_version.get('platform_version', + '0.0.0') + platform_version = versionutils.convert_version_to_tuple( + platform_version_str) + return platform_version + def _get_software_version(self): return self.call_xenapi('host.get_software_version', self.host_ref) @@ -365,3 +375,19 @@ class XenAPISession(object): yield conn finally: conn.close() + + def is_xsm_sr_check_relaxed(self): + if self._cached_xsm_sr_relaxed is None: + config_value = self.call_plugin('config_file', 'get_val', + key='relax-xsm-sr-check') + if not config_value: + version_str = '.'.join(str(v) for v in self.platform_version) + if versionutils.is_compatible('2.1.0', version_str, + same_major=False): + self._cached_xsm_sr_relaxed = True + else: + self._cached_xsm_sr_relaxed = False + else: + self._cached_xsm_sr_relaxed = config_value.lower() == 'true' + + return self._cached_xsm_sr_relaxed diff --git a/nova/virt/xenapi/vmops.py b/nova/virt/xenapi/vmops.py index a50314e2dbff..e9b51f5403bb 100644 --- a/nova/virt/xenapi/vmops.py +++ b/nova/virt/xenapi/vmops.py @@ -2216,20 +2216,6 @@ class VMOps(object): # block migration work will be able to resolve this return dest_check_data - def _is_xsm_sr_check_relaxed(self): - try: - return self.cached_xsm_sr_relaxed - except AttributeError: - config_value = None - try: - config_value = self._make_plugin_call('config_file.py', - 'get_val', - key='relax-xsm-sr-check') - except Exception: - LOG.exception(_LE('Plugin config_file get_val failed')) - self.cached_xsm_sr_relaxed = config_value == "true" - return self.cached_xsm_sr_relaxed - def check_can_live_migrate_source(self, ctxt, instance_ref, dest_check_data): """Check if it's possible to execute live migration on the source side. @@ -2243,7 +2229,7 @@ class VMOps(object): if len(self._get_iscsi_srs(ctxt, instance_ref)) > 0: # XAPI must support the relaxed SR check for live migrating with # iSCSI VBDs - if not self._is_xsm_sr_check_relaxed(): + if not self._session.is_xsm_sr_check_relaxed(): raise exception.MigrationError(reason=_('XAPI supporting ' 'relax-xsm-sr-check=true required'))