jacobsa/fuse

fuse.E* numerous missing constants

DamonBlais opened this issue · 4 comments

DamonBlais commented

As you said you would prefer one lump report instead of isolated issues, here is a (non-exhaustive) list. Implementation requires more than just defining constants, hence the different issues pertaining to each. The rationale behind each is also completely different.

The following system error codes are missing as constants in the fuse package

  • EACCESS
  • EBUSY - for network filesystems
  • EAGAIN - for network filesystems
  • ENODATA
  • EISDIR - for OpenFile on directories
  • EPERM
  • ERANGE - for directory listings (see: #53 for details)
  • EEXDEV
  • EBADF - for close(2) and friends (see #55 for details)
  • EROFS - for read-only filesystems (see #56 for details)
jacobsa commented
DamonBlais commented

There is a large inconsistency with some error codes being defined in the fuse package, where each code listed above is not defined here. This would also lead one to believe they are not being correctly utilized within the package (giving no need to define them, previously.)

Codes that ARE defined include:

  • fuse.EIO
  • fuse.ENOSYS
  • fuse.EEXIST
  • fuse.ENOENT
  • fuse.ENOATTR
  • fuse.ENOTDIR
  • fuse.ENOTEMPTY

I had noticed EINVAL used in many incorrect places, which would be resolved by correctly implementing each of the above error codes where they should be expected, as described in the kernel man pages. The incorrect usage has caused me (and others) various troubleshooting headaches, as anything we created using this package performed erratically when running filesystem testing tools.

jacobsa commented

I don't know what you mean by "incorrect usage" -- these constants are there for your use, not for use by the package itself. Is the package itself using them somewhere incorrectly?

In any case, I think this is the relevant comment:

These may be treated specially by Connection.Reply.

The package only defines those that it needs to behave specially for. Ideally it would be consistent and define all or none of them, but I don't see how that's possible given that the set of error codes may differ across platforms.

DamonBlais commented

these constants are there for your use

Here are just three (of many) examples of where we might need these constants to correctly implement a filesystem:

  1. Responding to SetXattr on bad names (missing or incorrect prefix)

The Linux man pages state under ERRORS for setxattr(2) that one must reply with ENOTSUP (which is not semantically the same as fuse.ENOSYS) when attempting to set an attribute with a prefix not defined in the specification. You can test this on existing filesystems (to see how they respond) with the setfattr tool found in the attr system package.

  1. Responding to ReleaseFileHandle on already closed or stale file handle

The Linux man pages state under ERRORS for close(2) and close(3) that one must reply with EBADF when the file handle passed in " isn't a valid file descriptor. "

  1. Responding to OpenFile on a directory

The Linux man pages state under ERRORS for open(2) that one must reply with EISDIR in one of the multiple conditions. Here is a quote of the relevant passage from the pages included in Ubuntu 18.04 for reference:

       EISDIR pathname refers to a directory and the access requested
              involved writing (that is, O_WRONLY or O_RDWR is set).

       EISDIR pathname refers to an existing directory, O_TMPFILE and one of
              O_WRONLY or O_RDWR were specified in flags, but this kernel
              version does not provide the O_TMPFILE functionality.

Ideally it would be consistent and define all or none of them, but I don't see how that's possible given that the set of error codes may differ across platforms.

As per conditional compilation

# +build linux darwin
// ... platform specific constants

If we disagree on the motive for implementing these constants, then I'll gladly concede using these constants for using the ones in the syscall package.