]> review.fuel-infra Code Review - openstack-build/cinder-build.git/commitdiff
Support mount options for NFS/GlusterFS volumes
authorEric Harney <eharney@redhat.com>
Fri, 10 May 2013 17:42:58 +0000 (13:42 -0400)
committerEric Harney <eharney@redhat.com>
Tue, 21 May 2013 13:33:13 +0000 (09:33 -0400)
This patch adds support for mount options for NFS/GlusterFS volumes.

To set options, append them to the line for the volume in the
nfs_shares_config or glusterfs_shares_config file:
 192.168.175.166:/testvol -o backupvolfile-server=192.168.175.177
 host1:/testvol2 -o backupvolfile-server=host2,rw,other_option

These options are used when mounting from the cinder-volume
service and the nova-compute service.  (There is an associated
nova-compute patch to enable the same.)

This is a useful change for GlusterFS because the
backupvolfile-server option enables a mount to succeed even if
the first server specified is offline.

Change-Id: I13bb0a930d6a05e803ccb50e6dc5d4b2b412ac11

cinder/tests/test_glusterfs.py
cinder/tests/test_nfs.py
cinder/volume/drivers/glusterfs.py
cinder/volume/drivers/nfs.py [changed mode: 0755->0644]

index a210d57cb141b32d450aaf9a2b6e608158e7d783..db2724383a035adce683022b6232180b5087bf6a 100644 (file)
@@ -49,6 +49,7 @@ class GlusterFsDriverTestCase(test.TestCase):
 
     TEST_EXPORT1 = 'glusterfs-host1:/export'
     TEST_EXPORT2 = 'glusterfs-host2:/export'
+    TEST_EXPORT2_OPTIONS = '-o backupvolfile-server=glusterfs-backup1'
     TEST_SIZE_IN_GB = 1
     TEST_MNT_POINT = '/mnt/glusterfs'
     TEST_MNT_POINT_BASE = '/mnt/test'
@@ -71,6 +72,7 @@ class GlusterFsDriverTestCase(test.TestCase):
         self.stubs = stubout.StubOutForTesting()
         self._driver = glusterfs.GlusterfsDriver(
                                     configuration=self._configuration)
+        self._driver.shares = {}
 
     def tearDown(self):
         self._mox.UnsetStubs()
@@ -272,19 +274,27 @@ class GlusterFsDriverTestCase(test.TestCase):
         mox = self._mox
         drv = self._driver
 
-        glusterfs.FLAGS.glusterfs_shares_config = self.TEST_SHARES_CONFIG_FILE
+        drv.configuration.glusterfs_shares_config = (
+            self.TEST_SHARES_CONFIG_FILE)
 
-        mox.StubOutWithMock(__builtin__, 'open')
+        mox.StubOutWithMock(drv, '_read_config_file')
         config_data = []
         config_data.append(self.TEST_EXPORT1)
         config_data.append('#' + self.TEST_EXPORT2)
+        config_data.append(self.TEST_EXPORT2 + ' ' + self.TEST_EXPORT2_OPTIONS)
         config_data.append('')
-        __builtin__.open(self.TEST_SHARES_CONFIG_FILE).AndReturn(config_data)
+        drv._read_config_file(self.TEST_SHARES_CONFIG_FILE).\
+            AndReturn(config_data)
         mox.ReplayAll()
 
-        shares = drv._load_shares_config()
+        drv._load_shares_config(drv.configuration.glusterfs_shares_config)
+
+        self.assertIn(self.TEST_EXPORT1, drv.shares)
+        self.assertIn(self.TEST_EXPORT2, drv.shares)
+        self.assertEqual(len(drv.shares), 2)
 
-        self.assertEqual([self.TEST_EXPORT1], shares)
+        self.assertEqual(drv.shares[self.TEST_EXPORT2],
+                         self.TEST_EXPORT2_OPTIONS)
 
         mox.VerifyAll()
 
@@ -312,8 +322,12 @@ class GlusterFsDriverTestCase(test.TestCase):
         mox = self._mox
         drv = self._driver
 
-        mox.StubOutWithMock(drv, '_load_shares_config')
-        drv._load_shares_config().AndReturn([self.TEST_EXPORT1])
+        mox.StubOutWithMock(drv, '_read_config_file')
+        config_data = []
+        config_data.append(self.TEST_EXPORT1)
+        drv._read_config_file(self.TEST_SHARES_CONFIG_FILE).\
+            AndReturn(config_data)
+
         mox.StubOutWithMock(drv, '_ensure_share_mounted')
         drv._ensure_share_mounted(self.TEST_EXPORT1)
 
@@ -331,8 +345,12 @@ class GlusterFsDriverTestCase(test.TestCase):
         mox = self._mox
         drv = self._driver
 
-        mox.StubOutWithMock(drv, '_load_shares_config')
-        drv._load_shares_config().AndReturn([self.TEST_EXPORT1])
+        mox.StubOutWithMock(drv, '_read_config_file')
+        config_data = []
+        config_data.append(self.TEST_EXPORT1)
+        drv._read_config_file(self.TEST_SHARES_CONFIG_FILE).\
+            AndReturn(config_data)
+
         mox.StubOutWithMock(drv, '_ensure_share_mounted')
         drv._ensure_share_mounted(self.TEST_EXPORT1).AndRaise(Exception())
 
index 2427554912f536a3d7d5760cc3effb128c623eff..4c87fef8ff7f86903c9820af1f937ce3d8a183c2 100644 (file)
@@ -109,6 +109,7 @@ class NfsDriverTestCase(test.TestCase):
 
     TEST_NFS_EXPORT1 = 'nfs-host1:/export'
     TEST_NFS_EXPORT2 = 'nfs-host2:/export'
+    TEST_NFS_EXPORT2_OPTIONS = '-o intr'
     TEST_SIZE_IN_GB = 1
     TEST_MNT_POINT = '/mnt/nfs'
     TEST_MNT_POINT_BASE = '/mnt/test'
@@ -128,6 +129,7 @@ class NfsDriverTestCase(test.TestCase):
         self.configuration.nfs_disk_util = 'df'
         self.configuration.nfs_sparsed_volumes = True
         self._driver = nfs.NfsDriver(configuration=self.configuration)
+        self._driver.shares = {}
 
     def tearDown(self):
         self._mox.UnsetStubs()
@@ -327,19 +329,27 @@ class NfsDriverTestCase(test.TestCase):
         mox = self._mox
         drv = self._driver
 
-        self.configuration.nfs_shares_config = self.TEST_SHARES_CONFIG_FILE
+        drv.configuration.nfs_shares_config = self.TEST_SHARES_CONFIG_FILE
 
-        mox.StubOutWithMock(__builtin__, 'open')
+        mox.StubOutWithMock(drv, '_read_config_file')
         config_data = []
         config_data.append(self.TEST_NFS_EXPORT1)
         config_data.append('#' + self.TEST_NFS_EXPORT2)
         config_data.append('')
-        __builtin__.open(self.TEST_SHARES_CONFIG_FILE).AndReturn(config_data)
+        config_data.append(self.TEST_NFS_EXPORT2 + ' ' +
+                           self.TEST_NFS_EXPORT2_OPTIONS)
+        drv._read_config_file(self.TEST_SHARES_CONFIG_FILE).\
+            AndReturn(config_data)
         mox.ReplayAll()
 
-        shares = drv._load_shares_config()
+        drv._load_shares_config(drv.configuration.nfs_shares_config)
+
+        self.assertIn(self.TEST_NFS_EXPORT1, drv.shares)
+        self.assertIn(self.TEST_NFS_EXPORT2, drv.shares)
+        self.assertEqual(len(drv.shares), 2)
 
-        self.assertEqual([self.TEST_NFS_EXPORT1], shares)
+        self.assertEqual(drv.shares[self.TEST_NFS_EXPORT2],
+                         self.TEST_NFS_EXPORT2_OPTIONS)
 
         mox.VerifyAll()
 
@@ -366,9 +376,14 @@ class NfsDriverTestCase(test.TestCase):
         mox = self._mox
         drv = self._driver
 
-        mox.StubOutWithMock(drv, '_load_shares_config')
-        drv._load_shares_config().AndReturn([self.TEST_NFS_EXPORT1])
+        mox.StubOutWithMock(drv, '_read_config_file')
+        config_data = []
+        config_data.append(self.TEST_NFS_EXPORT1)
+        drv._read_config_file(self.TEST_SHARES_CONFIG_FILE).\
+            AndReturn(config_data)
+
         mox.StubOutWithMock(drv, '_ensure_share_mounted')
+        drv.configuration.nfs_shares_config = self.TEST_SHARES_CONFIG_FILE
         drv._ensure_share_mounted(self.TEST_NFS_EXPORT1)
 
         mox.ReplayAll()
@@ -385,9 +400,14 @@ class NfsDriverTestCase(test.TestCase):
         mox = self._mox
         drv = self._driver
 
-        mox.StubOutWithMock(drv, '_load_shares_config')
-        drv._load_shares_config().AndReturn([self.TEST_NFS_EXPORT1])
+        mox.StubOutWithMock(drv, '_read_config_file')
+        config_data = []
+        config_data.append(self.TEST_NFS_EXPORT1)
+        drv._read_config_file(self.TEST_SHARES_CONFIG_FILE).\
+            AndReturn(config_data)
+
         mox.StubOutWithMock(drv, '_ensure_share_mounted')
+        drv.configuration.nfs_shares_config = self.TEST_SHARES_CONFIG_FILE
         drv._ensure_share_mounted(self.TEST_NFS_EXPORT1).AndRaise(Exception())
 
         mox.ReplayAll()
index 21f3c30c9013adbf130a835cd2af65442b4d3554..0f1dfd7be66b37ebafbad12c21d1bd867898cae6 100644 (file)
@@ -72,6 +72,8 @@ class GlusterfsDriver(nfs.RemoteFsDriver):
             LOG.warn(msg)
             raise exception.GlusterfsException(msg)
 
+        self.shares = {}
+
         try:
             self._execute('mount.glusterfs', check_exit_code=False)
         except OSError as exc:
@@ -132,6 +134,8 @@ class GlusterfsDriver(nfs.RemoteFsDriver):
         """Allow connection to connector and return connection info."""
         data = {'export': volume['provider_location'],
                 'name': volume['name']}
+        if volume['provider_location'] in self.shares:
+            data['options'] = self.shares[volume['provider_location']]
         return {
             'driver_volume_type': 'glusterfs',
             'data': data
@@ -160,7 +164,9 @@ class GlusterfsDriver(nfs.RemoteFsDriver):
            locally."""
         self._mounted_shares = []
 
-        for share in self._load_shares_config():
+        self._load_shares_config(self.configuration.glusterfs_shares_config)
+
+        for share in self.shares.keys():
             try:
                 self._ensure_share_mounted(share)
                 self._mounted_shares.append(share)
@@ -169,14 +175,9 @@ class GlusterfsDriver(nfs.RemoteFsDriver):
 
         LOG.debug('Available shares %s' % str(self._mounted_shares))
 
-    def _load_shares_config(self):
-        return [share.strip() for share
-                in open(self.configuration.glusterfs_shares_config)
-                if share and not share.startswith('#')]
-
     def _ensure_share_mounted(self, glusterfs_share):
         """Mount GlusterFS share.
-        :param glusterfs_share:
+        :param glusterfs_share: string
         """
         mount_path = self._get_mount_point_for_share(glusterfs_share)
         self._mount_glusterfs(glusterfs_share, mount_path, ensure=True)
@@ -239,9 +240,13 @@ class GlusterfsDriver(nfs.RemoteFsDriver):
         """Mount GlusterFS share to mount path."""
         self._execute('mkdir', '-p', mount_path)
 
+        command = ['mount', '-t', 'glusterfs', glusterfs_share,
+                   mount_path]
+        if self.shares.get(glusterfs_share) is not None:
+            command.extend(self.shares[glusterfs_share].split())
+
         try:
-            self._execute('mount', '-t', 'glusterfs', glusterfs_share,
-                          mount_path, run_as_root=True)
+            self._execute(*command, run_as_root=True)
         except exception.ProcessExecutionError as exc:
             if ensure and 'already mounted' in exc.stderr:
                 LOG.warn(_("%s is already mounted"), glusterfs_share)
old mode 100755 (executable)
new mode 100644 (file)
index 5914ed7..7b04a79
@@ -124,6 +124,36 @@ class RemoteFsDriver(driver.VolumeDriver):
                                   image_meta,
                                   self.local_path(volume))
 
+    def _read_config_file(self, config_file):
+        # Returns list of lines in file
+        with open(config_file) as f:
+            return f.readlines()
+
+    def _load_shares_config(self, share_file):
+        self.shares = {}
+
+        for share in self._read_config_file(share_file):
+            # A configuration line may be either:
+            #  host:/vol_name
+            # or
+            #  host:/vol_name -o options=123,rw --other
+            if not share.strip():
+                # Skip blank or whitespace-only lines
+                continue
+            if share.startswith('#'):
+                continue
+
+            share_info = share.split(' ', 1)
+            # results in share_info =
+            #  [ 'address:/vol', '-o options=123,rw --other' ]
+
+            share_address = share_info[0].strip()
+            share_opts = share_info[1].strip() if len(share_info) > 1 else None
+
+            self.shares[share_address] = share_opts
+
+        LOG.debug("shares loaded: %s", self.shares)
+
 
 class NfsDriver(RemoteFsDriver):
     """NFS based cinder driver. Creates file on NFS share for using it
@@ -147,6 +177,8 @@ class NfsDriver(RemoteFsDriver):
             LOG.warn(msg)
             raise exception.NfsException(msg)
 
+        self.shares = {}  # address : options
+
         try:
             self._execute('mount.nfs', check_exit_code=False)
         except OSError as exc:
@@ -202,6 +234,8 @@ class NfsDriver(RemoteFsDriver):
         """Allow connection to connector and return connection info."""
         data = {'export': volume['provider_location'],
                 'name': volume['name']}
+        if volume['provider_location'] in self.shares:
+            data['options'] = self.shares[volume['provider_location']]
         return {
             'driver_volume_type': 'nfs',
             'data': data
@@ -229,7 +263,9 @@ class NfsDriver(RemoteFsDriver):
         """Look for NFS shares in the flags and tries to mount them locally"""
         self._mounted_shares = []
 
-        for share in self._load_shares_config():
+        self._load_shares_config(self.configuration.nfs_shares_config)
+
+        for share in self.shares.keys():
             try:
                 self._ensure_share_mounted(share)
                 self._mounted_shares.append(share)
@@ -238,14 +274,9 @@ class NfsDriver(RemoteFsDriver):
 
         LOG.debug('Available shares %s' % str(self._mounted_shares))
 
-    def _load_shares_config(self):
-        return [share.strip() for share in
-                open(self.configuration.nfs_shares_config)
-                if share and not share.startswith('#')]
-
     def _ensure_share_mounted(self, nfs_share):
         """Mount NFS share
-        :param nfs_share:
+        :param nfs_share: string
         """
         mount_path = self._get_mount_point_for_share(nfs_share)
         self._mount_nfs(nfs_share, mount_path, ensure=True)
@@ -312,6 +343,8 @@ class NfsDriver(RemoteFsDriver):
         nfs_cmd = ['mount', '-t', 'nfs']
         if self.configuration.nfs_mount_options is not None:
             nfs_cmd.extend(['-o', self.configuration.nfs_mount_options])
+        if self.shares.get(nfs_share) is not None:
+            nfs_cmd.extend(self.shares[nfs_share].split())
         nfs_cmd.extend([nfs_share, mount_path])
 
         try: