Skip to content

fix flistd cleanup #41

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Eslam-Nawara merged 4 commits into from
Jul 31, 2025
Merged

fix flistd cleanup #41

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0e29282
force unmount and remove in flist clean up if the path is not mountpo…
Eslam-Nawara Jul 10, 2025
bb225bf
log debug message if failed to remove
Eslam-Nawara Jul 10, 2025
9c702e7
use f.system instead of syscall
Eslam-Nawara Jul 13, 2025
ca2c822
rename force clean up function
Eslam-Nawara Jul 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 24 additions & 1 deletion pkg/flist/cleanup.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,9 +139,32 @@ func (f *flistModule) cleanUnusedMounts() error {
}

if err := os.Remove(path); err != nil {
log.Error().Err(err).Msgf("failed to clean mountpoint %s", path)
log.Debug().Msgf("failed to remove path, trying to forcibly clean up : %+v", path)
if err := f.forceRemoveBrokenMounts(path); err != nil {
log.Error().Err(err).Msgf("failed to clean mountpoint %s", path)
}
}
}

return nil
}

// forceRemoveBrokenMounts tries to force the clean up of broken mounts by attempting to unmount them first
func (f *flistModule) forceRemoveBrokenMounts(path string) error {
// try normal unmount first
err := f.system.Unmount(path, 0)
if err != nil {
log.Warn().Err(err).Msgf("normal unmount failed for %s, trying lazy unmount", path)

// try lazy unmount
err = f.system.Unmount(path, syscall.MNT_DETACH)
Copy link
Copy Markdown
Collaborator

@ashraffouda ashraffouda Jul 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is not force unmount that is lazy unmount which is something diff from the method name
so I suggest u try to unmount if not try force unmount if u really want to force unmount
err := syscall.Unmount("/mnt/my-mount", syscall.MNT_FORCE)
or change method name to reflect u r doing lazy unmount but sure this needs some testing to check if force unmount will slove it or not

Copy link
Copy Markdown
Contributor Author

@Eslam-Nawara Eslam-Nawara Jul 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

force umount is not supported for the used fs, the force will be ignored, so the safe solution is to use lazy mount, the function name meant to reflect that we forcibly do the clean up, will updated the name to reflect that

if err != nil {
return errors.Wrapf(err, "lazy unmount also failed for %s", path)
}
}
// try to remove the path after unmount
if err := os.RemoveAll(path); err != nil {
return errors.Wrapf(err, "failed to remove mountpoint %s", path)
}
return nil
}
Loading