From 55e9df75d0dd97bbf1eece1e0a39bbc645d49df2 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Thu, 7 Mar 2019 17:03:59 +0100 Subject: [PATCH 1/4] Clean tmp directory if it not empty --- src/yunohost/backup.py | 64 +++++++++++++++++++++++++----------------- 1 file changed, 38 insertions(+), 26 deletions(-) diff --git a/src/yunohost/backup.py b/src/yunohost/backup.py index f9505fb66..98758a57a 100644 --- a/src/yunohost/backup.py +++ b/src/yunohost/backup.py @@ -326,10 +326,19 @@ class BackupManager(): if not os.path.isdir(self.work_dir): filesystem.mkdir(self.work_dir, 0o750, parents=True, uid='admin') elif self.is_tmp_work_dir: - logger.debug("temporary directory for backup '%s' already exists", + + logger.debug("temporary directory for backup '%s' already exists... attempting to clean it", self.work_dir) - # FIXME May be we should clean the workdir here - raise YunohostError('backup_output_directory_not_empty') + + # Try to recursively unmount stuff (from a previously failed backup ?) + if _recursive_umount(self.work_dir) > 0: + raise YunohostError('backup_output_directory_not_empty') + else: + # If umount succeeded, remove the directory (we checked that + # we're in /home/yunohost.backup/tmp so that should be okay... + # c.f. method clean() which also does this) + filesystem.rm(self.work_dir, True, True) + filesystem.mkdir(self.work_dir, 0o750, parents=True, uid='admin') # # Backup target management # @@ -1514,34 +1523,12 @@ class BackupMethod(object): directories of the working directories """ if self.need_mount(): - if self._recursive_umount(self.work_dir) > 0: + if _recursive_umount(self.work_dir) > 0: raise YunohostError('backup_cleaning_failed') if self.manager.is_tmp_work_dir: filesystem.rm(self.work_dir, True, True) - def _recursive_umount(self, directory): - """ - Recursively umount sub directories of a directory - - Args: - directory -- a directory path - """ - mount_lines = subprocess.check_output("mount").split("\n") - - points_to_umount = [line.split(" ")[2] - for line in mount_lines - if len(line) >= 3 and line.split(" ")[2].startswith(directory)] - ret = 0 - for point in reversed(points_to_umount): - ret = subprocess.call(["umount", point]) - if ret != 0: - ret = 1 - logger.warning(m18n.n('backup_cleaning_failed', point)) - continue - - return ret - def _check_is_enough_free_space(self): """ Check free space in repository or output directory before to backup @@ -2011,6 +1998,7 @@ def backup_create(name=None, description=None, methods=[], # Check that output directory is empty if os.path.isdir(output_directory) and no_compress and \ os.listdir(output_directory): + raise YunohostError('backup_output_directory_not_empty') elif no_compress: raise YunohostError('backup_output_directory_required') @@ -2315,6 +2303,30 @@ def _call_for_each_path(self, callback, csv_path=None): callback(self, row['source'], row['dest']) +def _recursive_umount(directory): + """ + Recursively umount sub directories of a directory + + Args: + directory -- a directory path + """ + mount_lines = subprocess.check_output("mount").split("\n") + + points_to_umount = [line.split(" ")[2] + for line in mount_lines + if len(line) >= 3 and line.split(" ")[2].startswith(directory)] + + ret = 0 + for point in reversed(points_to_umount): + ret = subprocess.call(["umount", point]) + if ret != 0: + ret = 1 + logger.warning(m18n.n('backup_cleaning_failed', point)) + continue + + return ret + + def free_space_in_directory(dirpath): stat = os.statvfs(dirpath) return stat.f_frsize * stat.f_bavail From 135ac60d94d81e35ce790d0c0e7d7d037a490e03 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Thu, 7 Mar 2019 17:04:40 +0100 Subject: [PATCH 2/4] Clarify arguments usage --- src/yunohost/backup.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/yunohost/backup.py b/src/yunohost/backup.py index 98758a57a..5608c7478 100644 --- a/src/yunohost/backup.py +++ b/src/yunohost/backup.py @@ -337,7 +337,7 @@ class BackupManager(): # If umount succeeded, remove the directory (we checked that # we're in /home/yunohost.backup/tmp so that should be okay... # c.f. method clean() which also does this) - filesystem.rm(self.work_dir, True, True) + filesystem.rm(self.work_dir, recursive=True, force=True) filesystem.mkdir(self.work_dir, 0o750, parents=True, uid='admin') # @@ -913,7 +913,7 @@ class RestoreManager(): ret = subprocess.call(["umount", self.work_dir]) if ret != 0: logger.warning(m18n.n('restore_cleaning_failed')) - filesystem.rm(self.work_dir, True, True) + filesystem.rm(self.work_dir, recursive=True, force=True) # # Restore target manangement # From 7f779ddd1cf60ba808f5ca0569c121be5fb75e62 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Thu, 7 Mar 2019 17:10:01 +0100 Subject: [PATCH 3/4] This ambiguity about 'ret' definitely looks like a bug ... --- src/yunohost/backup.py | 10 +++++----- src/yunohost/tests/test_backuprestore.py | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/yunohost/backup.py b/src/yunohost/backup.py index 5608c7478..039944264 100644 --- a/src/yunohost/backup.py +++ b/src/yunohost/backup.py @@ -331,7 +331,7 @@ class BackupManager(): self.work_dir) # Try to recursively unmount stuff (from a previously failed backup ?) - if _recursive_umount(self.work_dir) > 0: + if not _recursive_umount(self.work_dir): raise YunohostError('backup_output_directory_not_empty') else: # If umount succeeded, remove the directory (we checked that @@ -1523,7 +1523,7 @@ class BackupMethod(object): directories of the working directories """ if self.need_mount(): - if _recursive_umount(self.work_dir) > 0: + if not _recursive_umount(self.work_dir): raise YunohostError('backup_cleaning_failed') if self.manager.is_tmp_work_dir: @@ -2316,15 +2316,15 @@ def _recursive_umount(directory): for line in mount_lines if len(line) >= 3 and line.split(" ")[2].startswith(directory)] - ret = 0 + everything_went_fine = True for point in reversed(points_to_umount): ret = subprocess.call(["umount", point]) if ret != 0: - ret = 1 + everything_went_fine = False logger.warning(m18n.n('backup_cleaning_failed', point)) continue - return ret + return everything_went_fine def free_space_in_directory(dirpath): diff --git a/src/yunohost/tests/test_backuprestore.py b/src/yunohost/tests/test_backuprestore.py index 14c479d9a..73728240f 100644 --- a/src/yunohost/tests/test_backuprestore.py +++ b/src/yunohost/tests/test_backuprestore.py @@ -571,7 +571,7 @@ def test_backup_binds_are_readonly(monkeypatch): assert "Read-only file system" in output - if self._recursive_umount(self.work_dir) > 0: + if not _recursive_umount(self.work_dir): raise Exception("Backup cleaning failed !") self.clean() From 1d37dd4fa28a7529dd438f1aaa544d2551894b0e Mon Sep 17 00:00:00 2001 From: "ljf (zamentur)" Date: Wed, 13 Mar 2019 13:32:50 +0100 Subject: [PATCH 4/4] [fix] Missing import --- src/yunohost/tests/test_backuprestore.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/yunohost/tests/test_backuprestore.py b/src/yunohost/tests/test_backuprestore.py index 73728240f..893925e83 100644 --- a/src/yunohost/tests/test_backuprestore.py +++ b/src/yunohost/tests/test_backuprestore.py @@ -10,7 +10,7 @@ from moulinette import m18n from moulinette.core import init_authenticator from yunohost.app import app_install, app_remove, app_ssowatconf from yunohost.app import _is_installed -from yunohost.backup import backup_create, backup_restore, backup_list, backup_info, backup_delete +from yunohost.backup import backup_create, backup_restore, backup_list, backup_info, backup_delete, _recursive_umount from yunohost.domain import _get_maindomain from yunohost.utils.error import YunohostError