project-machine / puzzlefs

Apache License 2.0
380 stars 18 forks source link

Daemonize mount command #46

Closed ariel-miculas closed 1 year ago

hallyn commented 1 year ago

Hm, not sure whether this iunrelated to this patch, but when I do

strace -f ../target/debug/puzzlefs mount oci first xxx

I see a

205348 chdir("/" <unfinished ...>
205347 <... exit_group resumed>)        = ?
205348 <... chdir resumed>)             = 0

so then I get

openat(AT_FDCWD, "./oci/index.json", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
write(2, "Error: ", 7)                  = 7
write(2, "fs error: ", 10)              = 10
write(2, "No such file or directory", 25) = 25

Is the daemonization library doing the chdir("/")? That's unfortunate, but if so then you should probably account for that by recording the initial cwd and either joining relative pathnames to it, or chdiring back to it, right?

ariel-miculas commented 1 year ago

You're right, the daemon does chdir("/"), which is probably a good idea https://stackoverflow.com/a/3095624/3512536

chdir("/") to ensure that our process doesn't keep any directory in use. Failure to do this could make it so that an administrator couldn't unmount a filesystem, because it was our current directory. [Equivalently, we could change to any directory containing files important to the daemon's operation.]

So I'd rather change the code to use absolute paths instead of relative paths to the oci dir.

hallyn commented 1 year ago

Hm, I tried

sudo ../target/debug/puzzlefs mount oci first xxx

but now, even as root, I'm seeing:

d????????? ? ?     ?         ?            ? xxx

A second problem is that a second puzzlefs mount command fails when trying to create /tmp/puzzlefs.out again.

ariel-miculas commented 1 year ago
  1. Are you using an older puzzlefs image? If so, can you try building a new image with this branch and try mounting that one? The reason is that I made some changes to the wire format in https://github.com/anuvu/puzzlefs/commit/3c7d9500
  2. This should be fixed by cleaning up the log files when the daemon exits, otherwise alternating between sudo and non-sudo runs will prevent the daemon from starting
ariel-miculas commented 1 year ago

Log files cannot be cleaned up when the daemon exits because that would defeat the purpose (we want to see the errors if the mount command fails), so there are the following options:

  1. ignore logs and set stdout/stderr to /dev/null (this is how sshfs handles it)
  2. generate log files under XDG_DATA_HOME/puzzlefs/puzzlefs_pid.{out,err} and never delete them
  3. write logs to /dev/log (possibly capturing stdout and writing it to /dev/log):
    • ignore the messages from libfuse and only log the os error using syslog*; this has the advantage of not having to capture stdout
    • there's a fuse_set_log_func but it's only available in libfuse3 (missing in libfuse2) and it would require switching from fuse to fuser since the former only links to libfuse2; anyway, this requires changes in the rust fuse(r?) library to export this function; this would have the advantage of not having to capture stdout and using a custom logging function
    • use a systemd service, this would redirect stdout to syslog for us, but it requires creating a unit file and it's probably overkill
    • patch daemonize crate to accept a pipe and use logger or a separate thread to read from the pipe and write to syslog
    • use fopencookie as described in redirecting-stderr-to-syslog; there doesn't seem to be any rust bindings available so this wouldn't be trivial to implement

*e.g. instead of

fuse: mountpoint is not empty
fuse: if you are sure this is safe, use the 'nonempty' mount option
Error: fs error: Invalid argument (os error 22)

Caused by:
    Invalid argument (os error 22)

we would only get

Error: fs error: Invalid argument (os error 22)
hallyn commented 1 year ago
  1. Are you using an older puzzlefs image? If so, can you try building a new image with this branch and try mounting that one? The reason is that I made some changes to the wire format in 3c7d9500

Thank you, yes, that fixed that. There is an issue with a symbolic link in the mounted image. I'll probably create a new issue for that.

This PR seems good, I'll approve it, you can decide whether to merge it and handle logging in a differnet PR, or wait until you update logging in this PR.

ariel-miculas commented 1 year ago

I prefer handling logging in another PR, since there's one more issue related to this: https://github.com/anuvu/puzzlefs/issues/25

ariel-miculas commented 1 year ago

I don't have rights for merging, @hallyn can you please merge this?