From c76c1e5d7d3b043549f69c8dc8d6b878b1db0231 Mon Sep 17 00:00:00 2001 From: Scott Shambarger Date: Thu, 12 May 2022 16:27:26 -0700 Subject: fstrim: Remove all skipped entries before de-duplication When processing fstab entries, de-duplication is performed based on the source before all tests on the target have been checked, resulting in some entries being skipped when a removed duplicate with a different target would not have been. The fix is to move all the target checks before the source de-duplication. Addresses: #1686 Signed-off-by: Scott Shambarger Addresses: https://bugzilla.redhat.com/show_bug.cgi?id=2165981 --- sys-utils/fstrim.c | 53 ++++++++++++++++++++++++++-------------------- 1 file changed, 30 insertions(+), 23 deletions(-) diff --git a/sys-utils/fstrim.c b/sys-utils/fstrim.c index 88397f0ec..0b05e590e 100644 --- a/sys-utils/fstrim.c +++ b/sys-utils/fstrim.c @@ -230,8 +230,17 @@ static int is_unwanted_fs(struct libmnt_fs *fs, const char *tgt) return 1; rc = fstatfs(fd, &vfs) != 0 || vfs.f_type == STATFS_AUTOFS_MAGIC; close(fd); + if (rc) + return 1; - return rc; + /* FITRIM on read-only filesystem can fail, and it can fail */ + if (access(tgt, W_OK) != 0) { + if (errno == EROFS) + return 1; + if (errno == EACCES) + return 1; + } + return 0; } static int uniq_fs_target_cmp( @@ -317,6 +326,8 @@ static int fstrim_all_from_file(struct fstrim_control *ctl, const char *filename while (mnt_table_next_fs(tab, itr, &fs) == 0) { const char *src = mnt_fs_get_srcpath(fs), *tgt = mnt_fs_get_target(fs); + char *path; + int rc = 1; if (!tgt || is_unwanted_fs(fs, tgt)) { mnt_table_remove_fs(tab, fs); @@ -339,19 +350,6 @@ static int fstrim_all_from_file(struct fstrim_control *ctl, const char *filename mnt_table_remove_fs(tab, fs); continue; } - } - - /* de-duplicate by source */ - mnt_table_uniq_fs(tab, MNT_UNIQ_FORWARD, uniq_fs_source_cmp); - - mnt_reset_iter(itr, MNT_ITER_BACKWARD); - - /* Do FITRIM */ - while (mnt_table_next_fs(tab, itr, &fs) == 0) { - const char *src = mnt_fs_get_srcpath(fs), - *tgt = mnt_fs_get_target(fs); - char *path; - int rc = 1; /* Is it really accessible mountpoint? Not all mountpoints are * accessible (maybe over mounted by another filesystem) */ @@ -359,20 +357,29 @@ static int fstrim_all_from_file(struct fstrim_control *ctl, const char *filename if (path && streq_paths(path, tgt)) rc = 0; free(path); - if (rc) + if (rc) { + mnt_table_remove_fs(tab, fs); continue; /* overlaying mount */ - - /* FITRIM on read-only filesystem can fail, and it can fail */ - if (access(tgt, W_OK) != 0) { - if (errno == EROFS) - continue; - if (errno == EACCES) - continue; } if (!is_directory(tgt, 1) || - !has_discard(src, &wholedisk)) + !has_discard(src, &wholedisk)) { + mnt_table_remove_fs(tab, fs); continue; + } + } + + /* de-duplicate by source */ + mnt_table_uniq_fs(tab, MNT_UNIQ_FORWARD, uniq_fs_source_cmp); + + mnt_reset_iter(itr, MNT_ITER_BACKWARD); + + /* Do FITRIM */ + while (mnt_table_next_fs(tab, itr, &fs) == 0) { + const char *src = mnt_fs_get_srcpath(fs), + *tgt = mnt_fs_get_target(fs); + int rc; + cnt++; /* -- 2.39.1