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

Use (char *) for fsname and subtype #20

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
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
11 changes: 6 additions & 5 deletions src/runtime/runtime.c
Original file line number Diff line number Diff line change
Expand Up @@ -901,12 +901,13 @@ int fusefs_main(int argc, char* argv[], void (* mounted)(void)) {

int err;
sqfs_ll* ll;

struct fuse_opt fuse_opts[] = {
{"offset=%zu", offsetof(sqfs_opts, offset), 0},
{"timeout=%u", offsetof(sqfs_opts, idle_timeout_secs), 0},
{"fsname=%s", "squashfuse", 0},
{"subtype=%s", "squashfuse", 0},
FUSE_OPT_END
{"offset=%zu", offsetof(sqfs_opts, offset), 0},
{"timeout=%u", offsetof(sqfs_opts, idle_timeout_secs), 0},
{"fsname=%s", (char *)"squashfuse", 0},
{"subtype=%s", (char *)"squashfuse", 0},
FUSE_OPT_END
Comment on lines +906 to +910
Copy link

@danpla danpla Oct 28, 2023

Choose a reason for hiding this comment

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

According to the documentation of fuse_opt and the signature of fuse_opt_parse, the "squashfuse" strings should probably be a part of the struct passed to fuse_opt_parse.

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure how to do this, would you mind sending a pull request?

Copy link

Choose a reason for hiding this comment

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

I can try, but I need some context. I found probonopd/static-tools#31, which suggests that fsname and fstype were added so that mount would print:

some.AppImage on /tmp... type fuse.squashfuse  ...

instead of:

some.AppImage on /tmp/... type fuse.some.AppImage  ...

Is that right? Currently, mount prints the latter.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, we want fuse.squashfuse.

Copy link

Choose a reason for hiding this comment

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

So this means that at least on Linux we shouldn't set fsname, because it will also change the first string (some.AppImage) to squashfuse, giving:

squashfuse on /tmp/... type fuse.squashfuse  ...

But I'm not sure about FreeBSD, because it seems that it doesn't have the subtype option (see AppImage/AppImageKit#1139 and https://github.com/libfuse/libfuse/blob/a466241b45d1dd0bf685513bdeefd6448b63beb6/lib/helper.c#L195).

Copy link
Member Author

Choose a reason for hiding this comment

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

On FreeBSD, fuse is ok (it is the same for other fuse filesystems).

Copy link

Choose a reason for hiding this comment

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

Or has this been fixed in freebsd/freebsd-src@128a1db806d?

Copy link

@danpla danpla Oct 28, 2023

Choose a reason for hiding this comment

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

Looks like setting only subtype=squashfuse implicitly sets fsname; mount gives:

squashfuse on /tmp/... type fuse.squashfuse  ...

So if we want to keep the AppImage name as the filesystem source (i.e. some.AppImage /tmp/... type fuse.squashfuse), we should set it explicitly via fsname.

};

struct fuse_lowlevel_ops sqfs_ll_ops;
Expand Down