SMBFS: report each share as a pool

Cinder RemoteFS based drivers report total space capacity as a
sum of all individual share capacities.

For this reason, a volume backend may be considered for placing
a volume, even if none of the shares used by the backend can actually
store the requested volume.

This patch changes this behavior for the SMBFS drivers, treating
each share as a pool and repoting the stats accordingly.

This will also allow the scheduler to target a specific share.

The pool names will be configurable. If no pool name is explicitly
mapped to a share, the share name will be used instead as a pool name.

Partially Implements: blueprint remotefs-pools-support

Change-Id: I7ad82a051ba2732f2f7e497e795aff83efcfd428

cinder/tests/unit/windows/test_smbfs.py

@@ -131,6 +131,7 @@ class WindowsSmbFsTestCase(test.TestCase):
         mock_exists.return_value = share_config_exists
         fake_ensure_mounted = mock.MagicMock()
         self._smbfs_driver._ensure_shares_mounted = fake_ensure_mounted
+        self._smbfs_driver._setup_pool_mappings = mock.Mock()
         self._smbfs_driver.configuration = config
 
         if not (config.smbfs_shares_config and share_config_exists and
@@ -148,9 +149,36 @@ class WindowsSmbFsTestCase(test.TestCase):
             self.assertEqual({}, self._smbfs_driver.shares)
             fake_ensure_mounted.assert_called_once_with()
             mock_setup_alloc_data.assert_called_once_with()
+            self._smbfs_driver._setup_pool_mappings.assert_called_once_with()
 
         mock_check_os_platform.assert_called_once_with()
-        mock_remotefs_do_setup.assert_called_once_with(mock.sentinel.context)
+
+    def test_setup_pools(self):
+        pool_mappings = {
+            '//ip/share0': 'pool0',
+            '//ip/share1': 'pool1',
+        }
+        self._smbfs_driver.configuration.smbfs_pool_mappings = pool_mappings
+        self._smbfs_driver.shares = {
+            '//ip/share0': None,
+            '//ip/share1': None,
+            '//ip/share2': None
+        }
+
+        expected_pool_mappings = pool_mappings.copy()
+        expected_pool_mappings['//ip/share2'] = 'share2'
+
+        self._smbfs_driver._setup_pool_mappings()
+        self.assertEqual(expected_pool_mappings,
+                         self._smbfs_driver._pool_mappings)
+
+    def test_setup_pool_duplicates(self):
+        self._smbfs_driver.configuration.smbfs_pool_mappings = {
+            'share0': 'pool0',
+            'share1': 'pool0'
+        }
+        self.assertRaises(exception.SmbfsException,
+                          self._smbfs_driver._setup_pool_mappings)
 
     def test_initialize_connection(self):
         self._smbfs_driver.get_active_image_from_info = mock.Mock(
@@ -280,43 +308,6 @@ class WindowsSmbFsTestCase(test.TestCase):
             virtual_size_gb=self.volume.size,
             volume_exists=False)
 
-    def _test_find_share(self, existing_mounted_shares=True,
-                         eligible_shares=True):
-        if existing_mounted_shares:
-            mounted_shares = ('fake_share1', 'fake_share2', 'fake_share3')
-        else:
-            mounted_shares = None
-
-        self._smbfs_driver._mounted_shares = mounted_shares
-        self._smbfs_driver._is_share_eligible = mock.Mock(
-            return_value=eligible_shares)
-        self._smbfs_driver._get_total_allocated = mock.Mock(
-            side_effect=[3, 2, 1])
-
-        if not mounted_shares:
-            self.assertRaises(exception.SmbfsNoSharesMounted,
-                              self._smbfs_driver._find_share,
-                              self.volume)
-        elif not eligible_shares:
-            self.assertRaises(exception.SmbfsNoSuitableShareFound,
-                              self._smbfs_driver._find_share,
-                              self.volume)
-        else:
-            ret_value = self._smbfs_driver._find_share(
-                self.volume)
-            # The eligible share with the minimum allocated space
-            # will be selected
-            self.assertEqual('fake_share3', ret_value)
-
-    def test_find_share(self):
-        self._test_find_share()
-
-    def test_find_share_missing_mounted_shares(self):
-        self._test_find_share(existing_mounted_shares=False)
-
-    def test_find_share_missing_eligible_shares(self):
-        self._test_find_share(eligible_shares=False)
-
     def _test_is_share_eligible(self, capacity_info, volume_size):
         self._smbfs_driver._get_capacity_info = mock.Mock(
             return_value=[float(x << 30) for x in capacity_info])
@@ -841,3 +832,26 @@ class WindowsSmbFsTestCase(test.TestCase):
             self._FAKE_VOLUME_NAME, 'vhdx')
         drv._vhdutils.reconnect_parent_vhd.assert_called_once_with(
             self._FAKE_SNAPSHOT_PATH, self._FAKE_VOLUME_PATH)
+
+    def test_get_pool_name_from_share(self):
+        self._smbfs_driver._pool_mappings = {
+            mock.sentinel.share: mock.sentinel.pool}
+
+        pool = self._smbfs_driver._get_pool_name_from_share(
+            mock.sentinel.share)
+        self.assertEqual(mock.sentinel.pool, pool)
+
+    def test_get_share_from_pool_name(self):
+        self._smbfs_driver._pool_mappings = {
+            mock.sentinel.share: mock.sentinel.pool}
+
+        share = self._smbfs_driver._get_share_from_pool_name(
+            mock.sentinel.pool)
+        self.assertEqual(mock.sentinel.share, share)
+
+    def test_get_pool_name_from_share_exception(self):
+        self._smbfs_driver._pool_mappings = {}
+
+        self.assertRaises(exception.SmbfsException,
+                          self._smbfs_driver._get_share_from_pool_name,
+                          mock.sentinel.pool)

cinder/volume/drivers/windows/smbfs.py

@@ -67,6 +67,12 @@ volume_opts = [
     cfg.StrOpt('smbfs_mount_point_base',
                default=r'C:\OpenStack\_mnt',
                help=('Base dir containing mount points for smbfs shares.')),
+    cfg.DictOpt('smbfs_pool_mappings',
+                default={},
+                help=('Mappings between share locations and pool names. '
+                      'If not specified, the share names will be used as '
+                      'pool names. Example: '
+                      '//addr/share:pool_name,//addr/share2:pool_name2')),
 ]
 
 CONF = cfg.CONF
@@ -93,7 +99,8 @@ def update_allocation_data(delete=False):
 
 
 @interface.volumedriver
-class WindowsSmbfsDriver(remotefs_drv.RemoteFSSnapDriver):
+class WindowsSmbfsDriver(remotefs_drv.RemoteFSPoolMixin,
+                         remotefs_drv.RemoteFSSnapDriver):
     VERSION = VERSION
 
     driver_volume_type = 'smbfs'
@@ -171,6 +178,29 @@ class WindowsSmbfsDriver(remotefs_drv.RemoteFSSnapDriver):
         self.shares = {}  # address : options
         self._ensure_shares_mounted()
         self._setup_allocation_data()
+        self._setup_pool_mappings()
+
+    def _setup_pool_mappings(self):
+        self._pool_mappings = self.configuration.smbfs_pool_mappings
+
+        pools = list(self._pool_mappings.values())
+        duplicate_pools = set([pool for pool in pools
+                               if pools.count(pool) > 1])
+        if duplicate_pools:
+            msg = _("Found multiple mappings for pools %(pools)s. "
+                    "Requested pool mappings: %(pool_mappings)s")
+            raise exception.SmbfsException(
+                msg % dict(pools=duplicate_pools,
+                           pool_mappings=self._pool_mappings))
+
+        shares_missing_mappings = (
+            set(self.shares).difference(set(self._pool_mappings)))
+        for share in shares_missing_mappings:
+            msg = ("No pool name was requested for share %(share)s "
+                   "Using the share name instead.")
+            LOG.warning(msg, dict(share=share))
+
+            self._pool_mappings[share] = self._get_share_name(share)
 
     @remotefs_drv.locked_volume_id_operation
     def initialize_connection(self, volume, connector):
@@ -244,40 +274,6 @@ class WindowsSmbfsDriver(remotefs_drv.RemoteFSSnapDriver):
         total_allocated = share_alloc_data.get('total_allocated', 0) << 30
         return float(total_allocated)
 
-    def _find_share(self, volume):
-        """Choose SMBFS share among available ones for given volume size.
-
-        For instances with more than one share that meets the criteria, the
-        share with the least "allocated" space will be selected.
-
-        :param volume: the volume to be created.
-        """
-        if not self._mounted_shares:
-            raise exception.SmbfsNoSharesMounted()
-
-        target_share = None
-        target_share_reserved = 0
-
-        for smbfs_share in self._mounted_shares:
-            if not self._is_share_eligible(smbfs_share, volume.size):
-                continue
-            total_allocated = self._get_total_allocated(smbfs_share)
-            if target_share is not None:
-                if target_share_reserved > total_allocated:
-                    target_share = smbfs_share
-                    target_share_reserved = total_allocated
-            else:
-                target_share = smbfs_share
-                target_share_reserved = total_allocated
-
-        if target_share is None:
-            raise exception.SmbfsNoSuitableShareFound(
-                volume_size=volume.size)
-
-        LOG.debug('Selected %s as target smbfs share.', target_share)
-
-        return target_share
-
     def _is_share_eligible(self, smbfs_share, volume_size_in_gib):
         """Verifies SMBFS share is eligible to host volume with given size.
 
@@ -658,3 +654,22 @@ class WindowsSmbfsDriver(remotefs_drv.RemoteFSSnapDriver):
                                    volume_path)
         self._vhdutils.resize_vhd(volume_path, volume_size * units.Gi,
                                   is_file_max_size=False)
+
+    def _get_share_name(self, share):
+        return share.replace('/', '\\').lstrip('\\').split('\\', 1)[1]
+
+    def _get_pool_name_from_share(self, share):
+        return self._pool_mappings[share]
+
+    def _get_share_from_pool_name(self, pool_name):
+        mappings = {pool: share
+                    for share, pool in self._pool_mappings.items()}
+        share = mappings.get(pool_name)
+
+        if not share:
+            msg = _("Could not find any share for pool %(pool_name)s. "
+                    "Pool mappings: %(pool_mappings)s.")
+            raise exception.SmbfsException(
+                msg % dict(pool_name=pool_name,
+                           pool_mappings=self._pool_mappings))
+        return share

releasenotes/notes/smbfs-pools-support-bc43c653cfb1a34f.yaml

@@ -0,0 +1,5 @@
+---
+features:
+  - |
+    The SMBFS driver now exposes share information to the scheduler via pools.
+    The pool names are configurable, defaulting to the share names.