Skip to content
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.

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

lib: Fix safe_mount() when exfat or ntfs are not using fuse #556

Closed
wants to merge 1 commit into from

Conversation

@kerneltoast
Copy link
Contributor

commented Aug 16, 2019

safe_mount() incorrectly assumes that when exfat or ntfs are attempted,
that they must be using fuse. However, ntfs and exfat can have proper
kernel drivers and thus mount successfully without the use of fuse. When
there is a proper kernel driver for ntfs or exfat, safe_mount()
attempts to use fuse and errors out.

Fix this by trying to mount the requested fs normally first, before
assuming that it needs fuse in the case of exfat and ntfs.

Signed-off-by: Sultan Alsawaf sultan@kerneltoast.com

@@ -726,6 +726,16 @@ int safe_mount(const char *file, const int lineno, void (*cleanup_fn)(void),
{
int rval;

rval = mount(source, target, filesystemtype, mountflags, data);
if (rval == -1) {

This comment has been minimized.

Copy link
@metan-ucw

metan-ucw Aug 16, 2019

Member

Shouldn't we do:

        if (!is_fuse(filesystemtype)) {
                tst_brkm(...)
        }

Here instead so that we carry on and get to the code that attempts to mount a fuse filesystem?

This comment has been minimized.

Copy link
@kerneltoast

kerneltoast Aug 16, 2019

Author Contributor

No, because the is_fuse() check only shows filesystems that might use fuse. There's no way to know if fuse is really needed unless we try to mount normally and fail.

This comment has been minimized.

Copy link
@metan-ucw

metan-ucw Aug 19, 2019

Member

Yes, but in a case that mounting normally fails we should try fuse, which we do not after you change, that is because the tst_brk() is used for fatal errors and causes the test to exit.

This comment has been minimized.

Copy link
@kerneltoast

kerneltoast Aug 19, 2019

Author Contributor

Thanks, I've updated the commit. How does it look now?

@kerneltoast kerneltoast force-pushed the kerneltoast:master branch from 33bb455 to 7c3f3aa Aug 19, 2019

@kerneltoast

This comment has been minimized.

Copy link
Contributor Author

commented Aug 21, 2019

I'd like to point out that the Travis build failure was caused by bandwidth issues with Ubuntu's update servers, not this pull request.

@metan-ucw

This comment has been minimized.

Copy link
Member

commented Aug 26, 2019

Yes, I'm aware, that unfortunately happens randomly with docker in travis...

Anyway this patch looks good to me, maybe just a small nit, I guess that it would be cleaner to rename the is_fuse() function to possibly_fuse() now.

lib: Fix safe_mount() when exfat or ntfs are not using fuse
safe_mount() incorrectly assumes that when exfat or ntfs are attempted,
that they must be using fuse. However, ntfs and exfat can have proper
kernel drivers and thus mount successfully without the use of fuse. When
there *is* a proper kernel driver for ntfs or exfat, safe_mount()
attempts to use fuse and errors out.

Fix this by trying to mount the requested fs normally first, before
assuming that it needs fuse in the case of exfat and ntfs.

Signed-off-by: Sultan Alsawaf <sultan@kerneltoast.com>

@kerneltoast kerneltoast force-pushed the kerneltoast:master branch from 7c3f3aa to d75a96f Aug 27, 2019

@kerneltoast

This comment has been minimized.

Copy link
Contributor Author

commented Aug 27, 2019

Yes, I'm aware, that unfortunately happens randomly with docker in travis...

Just wanted to make sure. :)

Anyway this patch looks good to me, maybe just a small nit, I guess that it would be cleaner to rename the is_fuse() function to possibly_fuse() now.

Done.

@metan-ucw

This comment has been minimized.

Copy link
Member

commented Aug 29, 2019

Applied, thanks.

@metan-ucw metan-ucw closed this Aug 29, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.