Skip to content

EOPNOTSUPP can be returned if the filesystem does not support xattrs#5344

Merged
MichaelEischer merged 4 commits intorestic:masterfrom
gregoster:xattr_eopnotsupp
Sep 5, 2025
Merged

EOPNOTSUPP can be returned if the filesystem does not support xattrs#5344
MichaelEischer merged 4 commits intorestic:masterfrom
gregoster:xattr_eopnotsupp

Conversation

@gregoster
Copy link
Copy Markdown
Contributor

@gregoster gregoster commented Apr 3, 2025

What does this PR change? What problem does it solve?

EOPNOTSUPP is returned on NetBSD on non-FFSv2ea filesystems, and possibly on other OSes with filesystems which do not support xattrs. This patch ignores EOPNOTSUPP as an error for xattr.

Was the change previously discussed in an issue or on the forum?

#5341

Checklist

  • I have added tests for all code changes.
  • I have added documentation for relevant changes (in the manual).
  • There's a new file in changelog/unreleased/ that describes the changes for our users (see template).
  • I'm done! This pull request is ready for review.

Comment on lines +56 to +57
// ENOATTR instead of ENOTSUP. Ignore "Operation not supported"
// as well.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ENOTSUP is "Operation not supported"; EOPNOTSUPP is "Operation not supported on socket". So I think this comment should call out NetBSD for being weird:

Suggested change
// ENOATTR instead of ENOTSUP. Ignore "Operation not supported"
// as well.
// ENOATTR instead of ENOTSUP. NetBSD can return EOPNOTSUPP.

Copy link
Copy Markdown
Member

@MichaelEischer MichaelEischer Jun 2, 2025

Choose a reason for hiding this comment

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

From the Linux errno manpage xD . Although, I agree that NetBSD shows some rather unusual behavior here.

(ENOTSUP and EOPNOTSUPP have the same value on Linux, but according to POSIX.1 these error values should be distinct.)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The xattr package for go:
https://github.com/pkg/xattr/blob/master/xattr_test.go
uses both ENOTSUP and EOPNOTSUPP as well. FreeBSD (I think) will be the same as NetBSD in terms of returning EOPNOTSUPP. Not sure about Darwin.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

EOPNOTSUPP seems indeed to be a BSD thing: https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man3/acl_set.3.html . Although, restic has supported xattrs on darwin, freebsd, linux and solaris for quite a while, and there haven't been any reports regarding that error code. But feel free to replace NetBSD with BSD in the above suggestion.

Copy link
Copy Markdown
Member

@MichaelEischer MichaelEischer left a comment

Choose a reason for hiding this comment

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

See comment above

@MichaelEischer
Copy link
Copy Markdown
Member

@gregoster are you still interested in completing the PR?

@gregoster
Copy link
Copy Markdown
Contributor Author

@gregoster are you still interested in completing the PR?

Yes, I'd still like this completed. I've merged in the suggested wording changes. Is there anything else required?

Copy link
Copy Markdown
Member

@MichaelEischer MichaelEischer left a comment

Choose a reason for hiding this comment

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

LGTM.

I've rebased the PR to get rid of the merge commit.

@MichaelEischer MichaelEischer enabled auto-merge (squash) September 5, 2025 17:23
On Linux, ENOTSUP and EOPNOTSUPP have the same value such that Go does
not allow adding them to the same case.
@MichaelEischer MichaelEischer merged commit 839c38b into restic:master Sep 5, 2025
11 checks passed
MichaelEischer pushed a commit to MichaelEischer/restic that referenced this pull request Sep 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants