fstest: have CreateSocket actually create a socket #143
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tiborvass
force-pushed
the
fix-sockets
branch
5 times, most recently
from
9a89c42 to
461ecee
Compare
ijc
approved these changes
Aug 21, 2019
| if err := unix.Bind(fd, sa); err != nil { | ||
| return err | ||
| } | ||
| return os.Chmod(fullPath, perm) |
Contributor
There was a problem hiding this comment.
If this fails you return an error but the socket is left on disk. I suspect this isn't a problem because the caller is using this to populate some sort of temp dir which it will clean up in its entirety on error, but something to be aware of I guess.
Contributor
Author
There was a problem hiding this comment.
I think it's fine, but I'll let @dmcgowan decide
Using syscall package to create socket file, because net.Listen's Close() method removes the socket file and not closing the listener would result in leaking an fd. Signed-off-by: Tibor Vass <tibor@docker.com>
Contributor
Author
|
Ping @crosbymichael |
Member
|
LGTM |
thaJeztah
added a commit
to thaJeztah/docker
that referenced
this pull request
Oct 24, 2019
full diff: containerd/continuity@f2a389a...75bee3e - containerd/continuity#141 Add src string to copyDirectory error message - containerd/continuity#143 fstest: have CreateSocket actually create a socket Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
thaJeztah
added a commit
to thaJeztah/docker
that referenced
this pull request
Jan 13, 2020
…ce1fc84c4c4 full diff: containerd/continuity@f2a389a...26c1120 - containerd/continuity#109 Add OpenBSD support for fs subpackage - containerd/continuity#141 Add src string to copyDirectory error message - containerd/continuity#143 fstest: have CreateSocket actually create a socket - containerd/continuity#144 Support Go Modules - containerd/continuity#147 xattr lost when copying directory - containerd/continuity#148 fs: don't convert syscall.Timespec to unix.Timespec directly (doesn't work with gccgo) Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
docker-jenkins
pushed a commit
to docker-archive/docker-ce
that referenced
this pull request
Jan 13, 2020
…ce1fc84c4c4 full diff: containerd/continuity@f2a389a...26c1120 - containerd/continuity#109 Add OpenBSD support for fs subpackage - containerd/continuity#141 Add src string to copyDirectory error message - containerd/continuity#143 fstest: have CreateSocket actually create a socket - containerd/continuity#144 Support Go Modules - containerd/continuity#147 xattr lost when copying directory - containerd/continuity#148 fs: don't convert syscall.Timespec to unix.Timespec directly (doesn't work with gccgo) Signed-off-by: Sebastiaan van Stijn <github@gone.nl> Upstream-commit: 67a0695e8b355c7a1c3382b7165f97a2bafc5f9b Component: engine
thaJeztah
added a commit
to thaJeztah/cli
that referenced
this pull request
Feb 12, 2020
…ce1fc84c4c4 full diff: containerd/continuity@f2a389a...26c1120 full diff: containerd/continuity@f2a389a...26c1120 - containerd/continuity#109 Add OpenBSD support for fs subpackage - containerd/continuity#141 Add src string to copyDirectory error message - containerd/continuity#143 fstest: have CreateSocket actually create a socket - containerd/continuity#144 Support Go Modules - containerd/continuity#147 xattr lost when copying directory - containerd/continuity#148 fs: don't convert syscall.Timespec to unix.Timespec directly (doesn't work with gccgo) Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
thaJeztah
added a commit
to thaJeztah/cli
that referenced
this pull request
Feb 12, 2020
…ce1fc84c4c4 full diff: containerd/continuity@f2a389a...26c1120 full diff: containerd/continuity@f2a389a...26c1120 - containerd/continuity#109 Add OpenBSD support for fs subpackage - containerd/continuity#141 Add src string to copyDirectory error message - containerd/continuity#143 fstest: have CreateSocket actually create a socket - containerd/continuity#144 Support Go Modules - containerd/continuity#147 xattr lost when copying directory - containerd/continuity#148 fs: don't convert syscall.Timespec to unix.Timespec directly (doesn't work with gccgo) Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
docker-jenkins
pushed a commit
to docker-archive/docker-ce
that referenced
this pull request
Feb 13, 2020
…ce1fc84c4c4 full diff: containerd/continuity@f2a389a...26c1120 full diff: containerd/continuity@f2a389a...26c1120 - containerd/continuity#109 Add OpenBSD support for fs subpackage - containerd/continuity#141 Add src string to copyDirectory error message - containerd/continuity#143 fstest: have CreateSocket actually create a socket - containerd/continuity#144 Support Go Modules - containerd/continuity#147 xattr lost when copying directory - containerd/continuity#148 fs: don't convert syscall.Timespec to unix.Timespec directly (doesn't work with gccgo) Signed-off-by: Sebastiaan van Stijn <github@gone.nl> Upstream-commit: 507e6b61cf82059ce412ccdb531b0e9402d2d2a8 Component: cli
eiffel-fl
pushed a commit
to eiffel-fl/cli
that referenced
this pull request
Jul 28, 2020
…ce1fc84c4c4 full diff: containerd/continuity@f2a389a...26c1120 full diff: containerd/continuity@f2a389a...26c1120 - containerd/continuity#109 Add OpenBSD support for fs subpackage - containerd/continuity#141 Add src string to copyDirectory error message - containerd/continuity#143 fstest: have CreateSocket actually create a socket - containerd/continuity#144 Support Go Modules - containerd/continuity#147 xattr lost when copying directory - containerd/continuity#148 fs: don't convert syscall.Timespec to unix.Timespec directly (doesn't work with gccgo) Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Using syscall package to create socket file, because
net.Listen's Close() method removes the socket file
and not closing the listener would result in leaking an fd.
Signed-off-by: Tibor Vass tibor@docker.com
Ping @ijc @dmcgowan
More discussion is needed to know how to deal with sockets in continuity but I don't want that to hold back this PR. For the record, in BuildKit we think it is better to convert it to a regular empty file, as both for regular and socket file types, the existence of the file should cause errors when trying to bind to it again. Creating a regular file ensures a more consistent behavior of programs in that matter.