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: EpollEvent struct incorrect for s390x #16021

Closed
mundaym opened this issue Jun 9, 2016 · 4 comments
Closed

syscall: EpollEvent struct incorrect for s390x #16021

mundaym opened this issue Jun 9, 2016 · 4 comments
Milestone

Comments

@mundaym
Copy link
Member

@mundaym mundaym commented Jun 9, 2016

This was fixed for ppc64/ppc64le in #15135. The struct is missing a padding field between the Events and Fd fields.

I think the syscall.EpollEvent struct is wrong for all 64-bit architectures except amd64 (which uses the packed attribute and therefore has no alignment padding) and ppc64/ppc64le (which was fixed).

https://github.com/docker/containerd currently uses the syscall.EpollEvent struct directly for some operations. Presumably they should be using golang.org/x/sys/unix (which uses the correct padding on arm64 and s390x) instead however I'd like to fix this anyway since it is a very small change.

CL to follow.

@ianlancetaylor ianlancetaylor added this to the Go1.7Maybe milestone Jun 9, 2016
@gopherbot
Copy link

@gopherbot gopherbot commented Jun 9, 2016

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

@mwhudson
Copy link
Contributor

@mwhudson mwhudson commented Jun 9, 2016

Am I going to want to fix this on s390x (and arm64 I guess?) for the version of Go in Ubuntu 16.04 before I try to build docker/containerd there? Do you have a test case, or are containerd's own tests enough?

@mundaym
Copy link
Member Author

@mundaym mundaym commented Jun 9, 2016

I think you will need to yes. You might also want to pick up 0324a3f if you do an update (this also came up during docker testing).

The issue manifests itself as docker-containerd hanging when trying to run the CI tests, so I guess you won't be able to get the test suite to start without this change.

I don't think you'll need the change for arm64 - they appear to have worked around this issue by re-implementing the epoll calls using cgo (!): docker-archive/containerd@6299489.

@gopherbot gopherbot closed this in 5f3eb43 Jun 10, 2016
@mwhudson
Copy link
Contributor

@mwhudson mwhudson commented Jun 10, 2016

Thanks, I've got both those fixes in 16.10 now and have started the process for getting them into 16.04.

mundaym added a commit to linux-on-ibm-z/go that referenced this issue Jun 17, 2016
Fixes golang#16021.

Change-Id: I55df38bbccd2641abcb54704115002a9aa04325d
Reviewed-on: https://go-review.googlesource.com/23962
Run-TryBot: Michael Munday <munday@ca.ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
@golang golang locked and limited conversation to collaborators Jun 10, 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.