Skip to content

pkg/fileutil: fix F_OFD_ constants#12444

Merged
jingyih merged 1 commit intoetcd-io:masterfrom
kolyshkin:fix-lock
Nov 3, 2020
Merged

pkg/fileutil: fix F_OFD_ constants#12444
jingyih merged 1 commit intoetcd-io:masterfrom
kolyshkin:fix-lock

Conversation

@kolyshkin
Copy link
Copy Markdown
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 <[email protected]>
@@ -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}
Copy link
Copy Markdown

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
Copy Markdown
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.

Copy link
Copy Markdown

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
Copy Markdown
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
Copy Markdown

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

@joakim-tjernlund
Copy link
Copy Markdown

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

@kolyshkin
Copy link
Copy Markdown
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
Copy Markdown
Contributor Author

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

@kolyshkin
Copy link
Copy Markdown
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
Copy Markdown

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