Browse Source

[ZFSOnLinux] Retry unmounting old datasets during manage

Add a retry loop to ensure the dataset being renamed
is cleanly unmounted before the rename operation.

Change-Id: Ie506f237010c415ee9f0d64abbefd5854f776a5f
Closes-Bug: #1785180
(cherry picked from commit d7c01efb44)
(cherry picked from commit 6d8f47d3c3)
(cherry picked from commit 820638ccef)
tags/5.0.2^0
Goutham Pacha Ravi 9 months ago
parent
commit
efbc62d9b2

+ 39
- 15
manila/share/drivers/zfsonlinux/driver.py View File

@@ -724,15 +724,6 @@ class ZFSonLinuxShareDriver(zfs_utils.ExecuteMixin, driver.ShareDriver):
724 724
             "username": self.configuration.zfs_ssh_username,
725 725
             "host": self.service_ip,
726 726
         }
727
-        self.private_storage.update(
728
-            share["id"], {
729
-                "entity_type": "share",
730
-                "dataset_name": new_dataset_name,
731
-                "ssh_cmd": ssh_cmd,  # used in replication
732
-                "pool_name": actual_pool_name,  # used in replication
733
-                "used_options": " ".join(options),
734
-            }
735
-        )
736 727
 
737 728
         # Perform checks on requested dataset
738 729
         if actual_pool_name != scheduled_pool_name:
@@ -760,13 +751,25 @@ class ZFSonLinuxShareDriver(zfs_utils.ExecuteMixin, driver.ShareDriver):
760 751
                     "dataset": old_dataset_name,
761 752
                     "zpool": actual_pool_name})
762 753
 
763
-        # Rename dataset
764
-        out, err = self.execute("sudo", "mount")
765
-        if "%s " % old_dataset_name in out:
766
-            self.zfs_with_retry("umount", "-f", old_dataset_name)
767
-            time.sleep(1)
754
+        # Unmount the dataset before attempting to rename and mount
755
+        try:
756
+            self._unmount_share_with_retry(old_dataset_name)
757
+        except exception.ZFSonLinuxException:
758
+            msg = _("Unable to unmount share before renaming and re-mounting.")
759
+            raise exception.ZFSonLinuxException(message=msg)
760
+
761
+        # Rename the dataset and mount with new name
768 762
         self.zfs_with_retry("rename", old_dataset_name, new_dataset_name)
769
-        self.zfs("mount", new_dataset_name)
763
+
764
+        try:
765
+            self.zfs("mount", new_dataset_name)
766
+        except exception.ProcessExecutionError:
767
+            # Workaround for bug/1785180
768
+            out, err = self.zfs("mount")
769
+            mounted = any([new_dataset_name in mountedfs
770
+                           for mountedfs in out.splitlines()])
771
+            if not mounted:
772
+                raise
770 773
 
771 774
         # Apply options to dataset
772 775
         for option in options:
@@ -776,6 +779,16 @@ class ZFSonLinuxShareDriver(zfs_utils.ExecuteMixin, driver.ShareDriver):
776 779
         export_locations = self._get_share_helper(
777 780
             share["share_proto"]).get_exports(new_dataset_name)
778 781
 
782
+        self.private_storage.update(
783
+            share["id"], {
784
+                "entity_type": "share",
785
+                "dataset_name": new_dataset_name,
786
+                "ssh_cmd": ssh_cmd,  # used in replication
787
+                "pool_name": actual_pool_name,  # used in replication
788
+                "used_options": " ".join(options),
789
+            }
790
+        )
791
+
779 792
         return {"size": share["size"], "export_locations": export_locations}
780 793
 
781 794
     def unmanage(self, share):
@@ -836,6 +849,17 @@ class ZFSonLinuxShareDriver(zfs_utils.ExecuteMixin, driver.ShareDriver):
836 849
         """Unmanage dataset snapshot."""
837 850
         self.private_storage.delete(snapshot_instance["snapshot_id"])
838 851
 
852
+    @utils.retry(exception.ZFSonLinuxException)
853
+    def _unmount_share_with_retry(self, share_name):
854
+        out, err = self.execute("sudo", "mount")
855
+        if "%s " % share_name not in out:
856
+            return
857
+        self.zfs_with_retry("umount", "-f", share_name)
858
+        out, err = self.execute("sudo", "mount")
859
+        if "%s " % share_name in out:
860
+            raise exception.ZFSonLinuxException(
861
+                _("Unable to unmount dataset %s"), share_name)
862
+
839 863
     def _get_replication_snapshot_prefix(self, replica):
840 864
         """Returns replica-based snapshot prefix."""
841 865
         replication_snapshot_prefix = "%s_%s" % (

+ 71
- 7
manila/tests/share/drivers/zfsonlinux/test_driver.py View File

@@ -1138,7 +1138,7 @@ class ZFSonLinuxShareDriverTestCase(test.TestCase):
1138 1138
             zfs_driver.share_types,
1139 1139
             'get_extra_specs_from_share',
1140 1140
             mock.Mock(return_value={}))
1141
-        mock_sleep = self.mock_object(zfs_driver.time, "sleep")
1141
+        self.mock_object(zfs_driver.time, "sleep")
1142 1142
         mock__get_dataset_name = self.mock_object(
1143 1143
             self.driver, "_get_dataset_name",
1144 1144
             mock.Mock(return_value=new_dataset_name))
@@ -1148,11 +1148,17 @@ class ZFSonLinuxShareDriverTestCase(test.TestCase):
1148 1148
             mock.Mock(return_value=("fake_out", "fake_error")))
1149 1149
         mock_zfs_with_retry = self.mock_object(self.driver, "zfs_with_retry")
1150 1150
 
1151
-        mock_execute = self.mock_object(self.driver, "execute")
1151
+        mock_execute_side_effects = [
1152
+            ("%s " % old_dataset_name, "fake_err")
1153
+            if mount_exists else ("foo", "bar")
1154
+        ] * 3
1152 1155
         if mount_exists:
1153
-            mock_execute.return_value = "%s " % old_dataset_name, "fake_err"
1154
-        else:
1155
-            mock_execute.return_value = ("foo", "bar")
1156
+            # After three retries, assume the mount goes away
1157
+            mock_execute_side_effects.append((("foo", "bar")))
1158
+        mock_execute = self.mock_object(
1159
+            self.driver, "execute",
1160
+            mock.Mock(side_effect=iter(mock_execute_side_effects)))
1161
+
1156 1162
         mock_parse_zfs_answer = self.mock_object(
1157 1163
             self.driver,
1158 1164
             "parse_zfs_answer",
@@ -1175,9 +1181,11 @@ class ZFSonLinuxShareDriverTestCase(test.TestCase):
1175 1181
         self.assertEqual(
1176 1182
             mock_helper.return_value.get_exports.return_value,
1177 1183
             result["export_locations"])
1184
+        mock_execute.assert_called_with("sudo", "mount")
1178 1185
         if mount_exists:
1179
-            mock_sleep.assert_called_once_with(1)
1180
-        mock_execute.assert_called_once_with("sudo", "mount")
1186
+            self.assertEqual(4, mock_execute.call_count)
1187
+        else:
1188
+            self.assertEqual(1, mock_execute.call_count)
1181 1189
         mock_parse_zfs_answer.assert_called_once_with(mock_zfs.return_value[0])
1182 1190
         if driver_options.get("size"):
1183 1191
             self.assertFalse(mock_get_zfs_option.called)
@@ -1259,6 +1267,62 @@ class ZFSonLinuxShareDriverTestCase(test.TestCase):
1259 1267
         mock_parse_zfs_answer.assert_called_once_with(mock_zfs.return_value[0])
1260 1268
         mock_get_extra_specs_from_share.assert_called_once_with(share)
1261 1269
 
1270
+    def test_manage_unmount_exception(self):
1271
+        old_ds_name = "foopool/path/to/old/dataset/name"
1272
+        new_ds_name = "foopool/path/to/new/dataset/name"
1273
+        share = {
1274
+            "id": "fake_share_instance_id",
1275
+            "share_id": "fake_share_id",
1276
+            "export_locations": [{"path": "1.1.1.1:/%s" % old_ds_name}],
1277
+            "host": "foobackend@foohost#foopool",
1278
+            "share_proto": "NFS",
1279
+        }
1280
+
1281
+        mock_get_extra_specs_from_share = self.mock_object(
1282
+            zfs_driver.share_types,
1283
+            'get_extra_specs_from_share',
1284
+            mock.Mock(return_value={}))
1285
+        self.mock_object(zfs_driver.time, "sleep")
1286
+        mock__get_dataset_name = self.mock_object(
1287
+            self.driver, "_get_dataset_name",
1288
+            mock.Mock(return_value=new_ds_name))
1289
+        mock_helper = self.mock_object(self.driver, "_get_share_helper")
1290
+        mock_zfs = self.mock_object(
1291
+            self.driver, "zfs",
1292
+            mock.Mock(return_value=("fake_out", "fake_error")))
1293
+        mock_zfs_with_retry = self.mock_object(self.driver, "zfs_with_retry")
1294
+
1295
+        # 10 Retries, would mean 20 calls to check the mount still exists
1296
+        mock_execute_side_effects = [("%s " % old_ds_name, "fake_err")] * 21
1297
+        mock_execute = self.mock_object(
1298
+            self.driver, "execute",
1299
+            mock.Mock(side_effect=mock_execute_side_effects))
1300
+
1301
+        mock_parse_zfs_answer = self.mock_object(
1302
+            self.driver,
1303
+            "parse_zfs_answer",
1304
+            mock.Mock(return_value=[
1305
+                {"NAME": "some_other_dataset_1"},
1306
+                {"NAME": old_ds_name},
1307
+                {"NAME": "some_other_dataset_2"},
1308
+            ]))
1309
+        mock_get_zfs_option = self.mock_object(
1310
+            self.driver, 'get_zfs_option', mock.Mock(return_value="4G"))
1311
+
1312
+        self.assertRaises(exception.ZFSonLinuxException,
1313
+                          self.driver.manage_existing,
1314
+                          share, {'size': 10})
1315
+
1316
+        self.assertFalse(mock_helper.return_value.get_exports.called)
1317
+        mock_zfs_with_retry.assert_called_with("umount", "-f", old_ds_name)
1318
+        mock_execute.assert_called_with("sudo", "mount")
1319
+        self.assertEqual(10, mock_zfs_with_retry.call_count)
1320
+        self.assertEqual(20, mock_execute.call_count)
1321
+        mock_parse_zfs_answer.assert_called_once_with(mock_zfs.return_value[0])
1322
+        self.assertFalse(mock_get_zfs_option.called)
1323
+        mock__get_dataset_name.assert_called_once_with(share)
1324
+        mock_get_extra_specs_from_share.assert_called_once_with(share)
1325
+
1262 1326
     def test_unmanage(self):
1263 1327
         share = {'id': 'fake_share_id'}
1264 1328
         self.mock_object(self.driver.private_storage, 'delete')

+ 6
- 0
releasenotes/notes/bug-1785180-zfsonlinux-retry-unmounting-during-manage-872cf46313c5a4ff.yaml View File

@@ -0,0 +1,6 @@
1
+---
2
+fixes:
3
+  - |
4
+    The ZFSOnLinux driver now retries unmounting zfs shares
5
+    to perform the manage operation. See `Launchpad bug 1785180
6
+    <https://bugs.launchpad.net/manila/+bug/1785180>`_ for details.

Loading…
Cancel
Save