From: Navneet Singh Date: Tue, 4 Feb 2014 11:29:30 +0000 (+0530) Subject: Brick support for pNFS X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=57c6e2f7a1b9d528fab36c14d0f1526f07d53f91;p=openstack-build%2Fcinder-build.git Brick support for pNFS This change implements a retry mechanism during mounting nfs share. It tries mounting using pnfs if no specific nfs version is specified in the nfs options. In case of any failure during mounting it falls back to regular nfs mount which will internally try to mount with highest supported version. If nfs version is specified in options then regular mount will be allowed. Change-Id: I502b375548707ce7021cbacf1cd4f5532804aeb3 Implements: blueprint pnfs --- diff --git a/cinder/brick/remotefs/remotefs.py b/cinder/brick/remotefs/remotefs.py old mode 100644 new mode 100755 index 8239d9e72..c68e18b88 --- a/cinder/brick/remotefs/remotefs.py +++ b/cinder/brick/remotefs/remotefs.py @@ -17,6 +17,8 @@ import hashlib import os +import re +import six from cinder.brick import exception from cinder.openstack.common.gettextutils import _ @@ -38,6 +40,7 @@ class RemoteFsClient(object): raise exception.InvalidParameterValue( err=_('nfs_mount_point_base required')) self._mount_options = kwargs.get('nfs_mount_options', None) + self._check_nfs_options() elif mount_type == "glusterfs": self._mount_base = kwargs.get('glusterfs_mount_point_base', None) if not self._mount_base: @@ -78,22 +81,87 @@ class RemoteFsClient(object): mounts[mnt_point] = device return mounts - def mount(self, nfs_share, flags=None): - """Mount NFS share.""" - mount_path = self.get_mount_point(nfs_share) + def mount(self, share, flags=None): + """Mount given share.""" + mount_path = self.get_mount_point(share) if mount_path in self._read_mounts(): LOG.info(_('Already mounted: %s') % mount_path) return self._execute('mkdir', '-p', mount_path, check_exit_code=0) - - mnt_cmd = ['mount', '-t', self._mount_type] - if self._mount_options is not None: - mnt_cmd.extend(['-o', self._mount_options]) + if self._mount_type == 'nfs': + self._mount_nfs(share, mount_path, flags) + else: + self._do_mount(self._mount_type, share, mount_path, + self._mount_options, flags) + + def _do_mount(self, mount_type, share, mount_path, mount_options=None, + flags=None): + """Mounts share based on the specified params.""" + mnt_cmd = ['mount', '-t', mount_type] + if mount_options is not None: + mnt_cmd.extend(['-o', mount_options]) if flags is not None: mnt_cmd.extend(flags) - mnt_cmd.extend([nfs_share, mount_path]) + mnt_cmd.extend([share, mount_path]) self._execute(*mnt_cmd, root_helper=self.root_helper, run_as_root=True, check_exit_code=0) + + def _mount_nfs(self, nfs_share, mount_path, flags=None): + """Mount nfs share using present mount types.""" + mnt_errors = {} + + # This loop allows us to first try to mount with NFS 4.1 for pNFS + # support but falls back to mount NFS 4 or NFS 3 if either the client + # or server do not support it. + for mnt_type in sorted(self._nfs_mount_type_opts.keys(), reverse=True): + options = self._nfs_mount_type_opts[mnt_type] + try: + self._do_mount('nfs', nfs_share, mount_path, options, flags) + LOG.debug(_('Mounted %(sh)s using %(mnt_type)s.') + % {'sh': nfs_share, 'mnt_type': mnt_type}) + return + except Exception as e: + mnt_errors[mnt_type] = six.text_type(e) + LOG.debug(_('Failed to do %s mount.'), mnt_type) + raise exception.BrickException(_("NFS mount failed for share %(sh)s." + "Error - %(error)s") + % {'sh': nfs_share, + 'error': mnt_errors}) + + def _check_nfs_options(self): + """Checks and prepares nfs mount type options.""" + self._nfs_mount_type_opts = {'nfs': self._mount_options} + nfs_vers_opt_patterns = ['^nfsvers', '^vers', '^v[\d]'] + for opt in nfs_vers_opt_patterns: + if self._option_exists(self._mount_options, opt): + return + + # pNFS requires NFS 4.1. The mount.nfs4 utility does not automatically + # negotiate 4.1 support, we have to ask for it by specifying two + # options: vers=4 and minorversion=1. + pnfs_opts = self._update_option(self._mount_options, 'vers', '4') + pnfs_opts = self._update_option(pnfs_opts, 'minorversion', '1') + self._nfs_mount_type_opts['pnfs'] = pnfs_opts + + def _option_exists(self, options, opt_pattern): + """Checks if the option exists in nfs options and returns position.""" + options = [x.strip() for x in options.split(',')] if options else [] + pos = 0 + for opt in options: + pos = pos + 1 + if re.match(opt_pattern, opt, flags=0): + return pos + return 0 + + def _update_option(self, options, option, value=None): + """Update option if exists else adds it and returns new options.""" + opts = [x.strip() for x in options.split(',')] if options else [] + pos = self._option_exists(options, option) + if pos: + opts.pop(pos - 1) + opt = '%s=%s' % (option, value) if value else option + opts.append(opt) + return ",".join(opts) if len(opts) > 1 else opts[0] diff --git a/cinder/tests/brick/test_brick_connector.py b/cinder/tests/brick/test_brick_connector.py index d97709155..010e5448b 100644 --- a/cinder/tests/brick/test_brick_connector.py +++ b/cinder/tests/brick/test_brick_connector.py @@ -179,8 +179,8 @@ class ISCSIConnectorTestCase(ConnectorTestCase): ('iscsiadm -m node -T %s -p %s --login' % (iqn, location)), ('iscsiadm -m node -T %s -p %s --op update' - ' -n node.startup -v automatic' % (iqn, - location)), + ' -n node.startup -v automatic' + % (iqn, location)), ('iscsiadm -m node --rescan'), ('iscsiadm -m session --rescan'), ('tee -a /sys/block/sdb/device/delete'), @@ -579,7 +579,8 @@ class RemoteFsConnectorTestCase(ConnectorTestCase): 'export': self.TEST_DEV, 'name': '9c592d52-ce47-4263-8c21-4ecf3c029cdb'} self.connector = connector.RemoteFsConnector( - 'nfs', root_helper='sudo', nfs_mount_point_base='/mnt/test') + 'nfs', root_helper='sudo', nfs_mount_point_base='/mnt/test', + nfs_mount_options='vers=3') def tearDown(self): self.mox.VerifyAll() @@ -594,7 +595,7 @@ class RemoteFsConnectorTestCase(ConnectorTestCase): check_exit_code=0).AndReturn(("", "")) client._execute('mkdir', '-p', self.TEST_PATH, check_exit_code=0).AndReturn(("", "")) - client._execute('mount', '-t', 'nfs', + client._execute('mount', '-t', 'nfs', '-o', 'vers=3', self.TEST_DEV, self.TEST_PATH, root_helper='sudo', run_as_root=True, check_exit_code=0).AndReturn(("", "")) diff --git a/cinder/tests/brick/test_brick_remotefs.py b/cinder/tests/brick/test_brick_remotefs.py index 322e97aeb..a352948aa 100644 --- a/cinder/tests/brick/test_brick_remotefs.py +++ b/cinder/tests/brick/test_brick_remotefs.py @@ -13,8 +13,10 @@ # License for the specific language governing permissions and limitations # under the License. +import mock import mox +from cinder.brick import exception from cinder.brick.remotefs import remotefs from cinder.openstack.common import log as logging from cinder import test @@ -53,7 +55,8 @@ class BrickRemoteFsTestCase(test.TestCase): client._execute('mount', check_exit_code=0).AndReturn(("", "")) client._execute('mkdir', '-p', self.TEST_MNT_POINT, check_exit_code=0).AndReturn(("", "")) - client._execute('mount', '-t', 'nfs', self.TEST_EXPORT, + client._execute('mount', '-t', 'nfs', '-o', 'vers=4,minorversion=1', + self.TEST_EXPORT, self.TEST_MNT_POINT, root_helper='sudo', run_as_root=True, check_exit_code=0).AndReturn(("", "")) @@ -63,6 +66,81 @@ class BrickRemoteFsTestCase(test.TestCase): mox.VerifyAll() + def test_mount_nfs_with_specific_vers(self): + opts = ['vers=2,nointr', 'nfsvers=3,lock', 'nolock,v2', 'v4.0'] + for opt in opts: + client = remotefs.RemoteFsClient( + 'nfs', 'sudo', nfs_mount_point_base=self.TEST_MNT_BASE, + nfs_mount_options=opt) + + client._read_mounts = mock.Mock(return_value=[]) + client._execute = mock.Mock(return_value=True) + + client.mount(self.TEST_EXPORT) + client._execute.assert_any_call('mkdir', '-p', self.TEST_MNT_POINT, + check_exit_code=0) + client._execute.assert_any_call('mount', '-t', 'nfs', '-o', + opt, self.TEST_EXPORT, + self.TEST_MNT_POINT, + root_helper='sudo', + run_as_root=True, + check_exit_code=0) + + def test_mount_nfs_with_fallback_no_vers(self): + def execute(*args, **kwargs): + if 'mkdir' in args: + return True + elif 'mount' in args: + if 'lock,nointr,vers=4,minorversion=1' in args: + raise Exception() + else: + return True + else: + self.fail(_("Unexpected call to _execute.")) + + opts = 'lock,nointr' + client = remotefs.RemoteFsClient( + 'nfs', 'sudo', nfs_mount_point_base=self.TEST_MNT_BASE, + nfs_mount_options=opts) + + client._read_mounts = mock.Mock(return_value=[]) + client._execute = mock.Mock(wraps=execute) + + client.mount(self.TEST_EXPORT) + client._execute.assert_any_call('mkdir', '-p', self.TEST_MNT_POINT, + check_exit_code=0) + client._execute.assert_any_call('mount', '-t', 'nfs', '-o', + 'lock,nointr,vers=4,minorversion=1', + self.TEST_EXPORT, + self.TEST_MNT_POINT, + root_helper='sudo', + run_as_root=True, + check_exit_code=0) + client._execute.assert_any_call('mount', '-t', 'nfs', '-o', + 'lock,nointr', + self.TEST_EXPORT, + self.TEST_MNT_POINT, + root_helper='sudo', + run_as_root=True, + check_exit_code=0) + + def test_mount_nfs_with_fallback_all_fail(self): + def execute(*args, **kwargs): + if 'mkdir' in args: + return True + else: + raise Exception(_("mount failed.")) + + opts = 'lock,nointr' + client = remotefs.RemoteFsClient( + 'nfs', 'sudo', nfs_mount_point_base=self.TEST_MNT_BASE, + nfs_mount_options=opts) + + client._read_mounts = mock.Mock(return_value=[]) + client._execute = mock.Mock(wraps=execute) + self.assertRaises(exception.BrickException, client.mount, + self.TEST_EXPORT) + def test_mount_nfs_should_not_remount(self): mox = self._mox client = self._nfsclient