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

pkg/fileutil: fix F_OFD_ constants #12444

Merged
merged 1 commit into from
Nov 3, 2020
Merged

Conversation

kolyshkin
Copy link
Contributor

Use golang.org/x/sys/unix for F_OFD_* constants.

This fixes the issue that F_OFD_GETLK was defined incorrectly,
resulting in bugs such as moby/moby#31182

Closes: #12440

Use golang.org/x/sys/unix for F_OFD_* constants.

This fixes the issue that F_OFD_GETLK was defined incorrectly,
resulting in bugs such as moby/moby#31182

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
@@ -50,7 +45,7 @@ var (
func init() {
// use open file descriptor locks if the system supports it
getlk := syscall.Flock_t{Type: syscall.F_RDLCK}

Choose a reason for hiding this comment

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

Why do you need to use dynamic probing for OFD locks ? This file is for Linux only.
Are OFD locks new in Linux?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

F_OFD_ locks are there since Linux kernel 3.15.

Choose a reason for hiding this comment

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

Do you still support 3.15 and older?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you still support 3.15 and older?

@joakim-tjernlund I do not know about that, I am just a contributor here. Given the fact that this package is used by other software, I think it make sense to support older kernels.

@jingyih jingyih merged commit 64e048b into etcd-io:master Nov 3, 2020
@joakim-tjernlund
Copy link

Can this fix be cherry-picked into stable 19 branch too ?

@joakim-tjernlund
Copy link

Ping ? This bug has been around for years already. Please add fix to the release branches.

@kolyshkin
Copy link
Contributor Author

kolyshkin commented Dec 14, 2020

Can this fix be cherry-picked into stable 19 branch too ?

I am not sure what do you mean by 19 branch, but I have just created a backport PR for release-3.4 branch here: #12551, as well as 3.3 (see below). Feel free to backport to older branches, too (not sure if those are still maintained).

@kolyshkin
Copy link
Contributor Author

Backport to 3.3 (using the original #12440) here: #12552

@kolyshkin
Copy link
Contributor Author

Backport to 3.2 (using the original #12440) here: #12553

Not sure it is maintained; adding just in case.

@joakim-tjernlund
Copy link

thanks, I guess this is more than enough.

with branch 19 I incorrectly referred to the docker daemon which is at 19.03.14 here.
I guess it uses one of the above branches?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants