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 <devel@shambarger.net>

diff --git a/sys-utils/fstrim.c b/sys-utils/fstrim.c
index 88333e91db86557b8dd148d22f8bca1956a9f586..0def35f824c19eb5bf6a89ac8913e6ac8cb15a4f 100644 (file)
--- a/sys-utils/fstrim.c
+++ b/sys-utils/fstrim.c
@@ -233,8 +233,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(
@@ -320,6 +329,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);
@@ -342,19 +353,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) */
@@ -362,20 +360,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++;
 
                /*