From 19f7e256a55bd9d44af485177fe8b4b431e0b5a9 Mon Sep 17 00:00:00 2001 From: Yaguang Tang Date: Mon, 9 Mar 2026 22:39:56 +0800 Subject: [PATCH] Fix generic driver NFS share deletion 'target is busy' error When deleting NFS shares from generic driver service VMs, the operation can fail with 'target is busy' because NFS exports are not unexported before unmount is attempted. This patch fixes the issue by: 1. Implementing NFSHelper.remove_exports() to actively unexport all NFS clients via 'exportfs -u' before the share is unmounted. Previously this method was a no-op. 2. Adding a lazy unmount fallback in GenericShareDriver._unmount_device() that detects 'is busy' errors and retries with 'umount -l' to allow the unmount to succeed even when NFS kernel state hasn't fully drained. Assisted-By: Copilot <223556219+Copilot@users.noreply.github.com> Change-Id: I9e31a0e082c50ab24b3f01522aba7bb1fb2eaa64 Signed-off-by: Yaguang Tang (cherry picked from commit a244262d8609d715a5046431d90a6a4063284c40) (cherry picked from commit cc2e903b7e08b5baae14abc1c2946e0b28afadf3) --- manila/share/drivers/generic.py | 13 ++++- manila/share/drivers/helpers.py | 28 ++++++++- manila/tests/share/drivers/test_generic.py | 31 ++++++++++ manila/tests/share/drivers/test_helpers.py | 58 +++++++++++++++++++ ...-deletion-busy-error-4cee8e87e98fa73b.yaml | 9 +++ 5 files changed, 137 insertions(+), 2 deletions(-) create mode 100644 releasenotes/notes/fix-generic-driver-nfs-share-deletion-busy-error-4cee8e87e98fa73b.yaml diff --git a/manila/share/drivers/generic.py b/manila/share/drivers/generic.py index 4b02601333..61eda3186a 100644 --- a/manila/share/drivers/generic.py +++ b/manila/share/drivers/generic.py @@ -400,7 +400,18 @@ class GenericShareDriver(driver.ExecuteMixin, driver.ShareDriver): "'%(server)s'.", log_data) unmount_cmd = ['sudo', 'umount', mount_path, '&&', 'sudo', 'rmdir', mount_path] - self._ssh_exec(server_details, unmount_cmd) + try: + self._ssh_exec(server_details, unmount_cmd) + except exception.ProcessExecutionError as e: + if e.stderr and 'is busy' in e.stderr.lower(): + LOG.warning("Device '%(path)s' is busy on server " + "'%(server)s', retrying with lazy " + "unmount.", log_data) + lazy_cmd = ['sudo', 'umount', '-l', mount_path, + '&&', 'sudo', 'rmdir', mount_path] + self._ssh_exec(server_details, lazy_cmd) + else: + raise self._remove_mount_permanently(share.id, server_details) else: LOG.warning("Mount point '%(path)s' does not exist on " diff --git a/manila/share/drivers/helpers.py b/manila/share/drivers/helpers.py index 9d68653e78..deb7a26415 100644 --- a/manila/share/drivers/helpers.py +++ b/manila/share/drivers/helpers.py @@ -219,7 +219,33 @@ class NFSHelper(NASHelperBase): LOG.error(e.stderr) def remove_exports(self, server, share_name): - """Remove exports.""" + """Remove NFS exports for the given share. + + Unexports all clients from the share path using exportfs -u before + the share is unmounted. This prevents 'device busy' errors during + unmount by ensuring no NFS clients hold references to the export. + """ + local_path = os.path.join(self.configuration.share_mount_path, + share_name) + out, __ = self._ssh_exec(server, ['sudo', 'exportfs']) + hosts = self.get_host_list(out, local_path) + for host in hosts: + parsed_host = self._get_parsed_address_or_cidr(host) + try: + self._ssh_exec(server, ['sudo', 'exportfs', '-u', + ':'.join((parsed_host, local_path))]) + except exception.ProcessExecutionError as e: + if "could not find" in e.stderr.lower(): + LOG.debug("Export %(host)s:%(path)s was already " + "removed.", {'host': parsed_host, + 'path': local_path}) + else: + LOG.warning("Failed to unexport %(host)s:%(path)s: " + "%(error)s", + {'host': parsed_host, 'path': local_path, + 'error': e.stderr}) + if hosts: + self._sync_nfs_temp_and_perm_files(server) @nfs_synchronized def update_access(self, server, share_name, access_rules, add_rules, diff --git a/manila/tests/share/drivers/test_generic.py b/manila/tests/share/drivers/test_generic.py index f80ddff1ce..2e901eda64 100644 --- a/manila/tests/share/drivers/test_generic.py +++ b/manila/tests/share/drivers/test_generic.py @@ -434,6 +434,37 @@ class GenericShareDriverTestCase(test.TestCase): mount_path, self.server) generic.LOG.warning.assert_called_once_with(mock.ANY, mock.ANY) + def test_unmount_device_busy_lazy_fallback(self): + mount_path = '/fake/mount/path' + self.mock_object(self._driver, '_is_device_mounted', + mock.Mock(return_value=True)) + self.mock_object(self._driver, '_remove_mount_permanently') + self.mock_object(self._driver, '_get_mount_path', + mock.Mock(return_value=mount_path)) + + def fake_ssh_exec(server, cmd): + if cmd == ['sudo', 'umount', mount_path, '&&', + 'sudo', 'rmdir', mount_path]: + raise exception.ProcessExecutionError( + stderr='umount: /fake/mount/path: target is busy') + return ('', '') + + self.mock_object(self._driver, '_ssh_exec', + mock.Mock(side_effect=fake_ssh_exec)) + + self._driver._unmount_device(self.share, self.server) + + self._driver._ssh_exec.assert_has_calls([ + mock.call(self.server, + ['sudo', 'umount', mount_path, + '&&', 'sudo', 'rmdir', mount_path]), + mock.call(self.server, + ['sudo', 'umount', '-l', mount_path, + '&&', 'sudo', 'rmdir', mount_path]), + ]) + self._driver._remove_mount_permanently.assert_called_once_with( + self.share.id, self.server) + def test_is_device_mounted_true(self): volume = {'mountpoint': 'fake_mount_point', 'id': 'fake_id'} mount_path = '/fake/mount/path' diff --git a/manila/tests/share/drivers/test_helpers.py b/manila/tests/share/drivers/test_helpers.py index 2f29c44906..1031a37df0 100644 --- a/manila/tests/share/drivers/test_helpers.py +++ b/manila/tests/share/drivers/test_helpers.py @@ -433,6 +433,64 @@ class NFSHelperTestCase(test.TestCase): fake_maintenance_path] ) + def test_remove_exports(self): + local_path = os.path.join(CONF.share_mount_path, self.share_name) + exec_result = ( + '%(path)s\n\t\t1.1.1.1\n' + '%(path)s\n\t\t2.2.2.2' % {'path': local_path} + ) + + def fake_ssh_exec(*args, **kwargs): + if args[1] == ['sudo', 'exportfs']: + return (exec_result, '') + return ('', '') + + self.mock_object(self._helper, '_ssh_exec', + mock.Mock(side_effect=fake_ssh_exec)) + self.mock_object(self._helper, '_sync_nfs_temp_and_perm_files') + + self._helper.remove_exports(self.server, self.share_name) + + self._helper._ssh_exec.assert_has_calls([ + mock.call(self.server, ['sudo', 'exportfs']), + mock.call(self.server, ['sudo', 'exportfs', '-u', + ':'.join(['1.1.1.1', local_path])]), + mock.call(self.server, ['sudo', 'exportfs', '-u', + ':'.join(['2.2.2.2', local_path])]), + ]) + self._helper._sync_nfs_temp_and_perm_files.assert_called_once_with( + self.server) + + def test_remove_exports_no_active_exports(self): + self.mock_object(self._helper, '_ssh_exec', + mock.Mock(return_value=('', ''))) + self.mock_object(self._helper, '_sync_nfs_temp_and_perm_files') + + self._helper.remove_exports(self.server, self.share_name) + + self._helper._ssh_exec.assert_called_once_with( + self.server, ['sudo', 'exportfs']) + self._helper._sync_nfs_temp_and_perm_files.assert_not_called() + + def test_remove_exports_already_removed(self): + local_path = os.path.join(CONF.share_mount_path, self.share_name) + exec_result = '%s\n\t\t1.1.1.1' % local_path + + def fake_ssh_exec(*args, **kwargs): + if args[1] == ['sudo', 'exportfs']: + return (exec_result, '') + raise exception.ProcessExecutionError( + stderr='could not find export') + + self.mock_object(self._helper, '_ssh_exec', + mock.Mock(side_effect=fake_ssh_exec)) + self.mock_object(self._helper, '_sync_nfs_temp_and_perm_files') + + self._helper.remove_exports(self.server, self.share_name) + + self._helper._sync_nfs_temp_and_perm_files.assert_called_once_with( + self.server) + @ddt.ddt class CIFSHelperIPAccessTestCase(test.TestCase): diff --git a/releasenotes/notes/fix-generic-driver-nfs-share-deletion-busy-error-4cee8e87e98fa73b.yaml b/releasenotes/notes/fix-generic-driver-nfs-share-deletion-busy-error-4cee8e87e98fa73b.yaml new file mode 100644 index 0000000000..59080e0806 --- /dev/null +++ b/releasenotes/notes/fix-generic-driver-nfs-share-deletion-busy-error-4cee8e87e98fa73b.yaml @@ -0,0 +1,9 @@ +--- +fixes: + - | + Fixed a bug in the generic share driver where deleting NFS shares + from service VMs could fail with a 'target is busy' error. The + NFS exports are now properly unexported via ``exportfs -u`` before + the share is unmounted. Additionally, a lazy unmount fallback + (``umount -l``) has been added to handle cases where the NFS + kernel state has not fully drained at unmount time.