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

syscall: Fsid.X__val not exported on s390x #17298

Closed
mundaym opened this issue Sep 30, 2016 · 4 comments
Closed

syscall: Fsid.X__val not exported on s390x #17298

mundaym opened this issue Sep 30, 2016 · 4 comments

Comments

@mundaym
Copy link
Member

@mundaym mundaym commented Sep 30, 2016

Kubernetes recently added https://github.com/pkg/sftp as a dependency. The sftp package uses the X__val member in the Fsid struct which we currently replace with _ on s390x using mkpost.go.

The usage is here: https://github.com/pkg/sftp/blob/8197a2e580736b78d704be0fc47b2324c0591a32/server_statvfs_linux.go#L19.

Since this variable is used on other architectures we should expose it so that sftp will compile (although it may need an endianness fix too). I will send a CL.

@mundaym
Copy link
Member Author

@mundaym mundaym commented Sep 30, 2016

@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Sep 30, 2016

They should just not use the built-in syscall package.

Also, that field is kinda disgusting and the comment is very encouraging:

  Fsid:  uint64(uint64(stat.Fsid.X__val[1])<<32 | uint64(stat.Fsid.X__val[0])), // endianness?

Do they even need that field? They don't seem to know what it even is.

Deleting that line might be a better fix?

@gopherbot
Copy link

@gopherbot gopherbot commented Sep 30, 2016

CL https://golang.org/cl/30130 mentions this issue.

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Sep 30, 2016

You have to love the statfs man page on GNU/Linux: "Nobody knows what f_fsid is supposed to contain."

But it does go on to say "The general idea is that f_fsid contains some random stuff such that the pair (f_fsid,ino) uniquely determines a file."

davecheney added a commit to pkg/sftp that referenced this issue Sep 30, 2016
Fixes golang/go#17298

This field is not present in all versions of the syscall package.
davecheney added a commit to pkg/sftp that referenced this issue Sep 30, 2016
Fixes golang/go#17298

This field is not present in all versions of the syscall package.
techjacker added a commit to techjacker/sftp that referenced this issue Dec 5, 2016
Fixes golang/go#17298

This field is not present in all versions of the syscall package.
gopherbot pushed a commit that referenced this issue Jan 13, 2017
mkpost.go replaces all variables prefixed with 'X_' with '_' on s390x
because most of them do not need to be exposed. X__val is being used
by a third party library so it turns out we do need to expose it on
s390x (it is already exposed on all other Linux architectures).

Fixes #17298 and updates #18632.

Change-Id: Ic03463229a5f75ca41a4a4b50300da4b4d892d45
Reviewed-on: https://go-review.googlesource.com/30130
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@golang golang locked and limited conversation to collaborators Sep 30, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.