Skip to content

libcontainer/notify_linux*: adjust the file mode#2837

Closed
KentaTada wants to merge 1 commit intoopencontainers:mainfrom
KentaTada:refactor-mode-of-notifylinux
Closed

libcontainer/notify_linux*: adjust the file mode#2837
KentaTada wants to merge 1 commit intoopencontainers:mainfrom
KentaTada:refactor-mode-of-notifylinux

Conversation

@KentaTada
Copy link
Copy Markdown
Contributor

This commit adjusts the file mode to use the latest golang style
In addition to that, I changed those modes from 0700 to 0600
as same as #2636

Related to #2625

Signed-off-by: Kenta Tada Kenta.Tada@sony.com

Comment thread libcontainer/notify_linux.go Outdated
eventControlPath := filepath.Join(cgDir, "cgroup.event_control")
data := fmt.Sprintf("%d %d %s", eventfd.Fd(), evFile.Fd(), arg)
if err := ioutil.WriteFile(eventControlPath, []byte(data), 0700); err != nil {
if err := ioutil.WriteFile(eventControlPath, []byte(data), 0o600); err != nil {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

  1. In fact it does not matter what the mode here is, as the file exists (and it can't possibly be created as this is cgroupfs).

  2. Might make sense to switch to fscommon.WriteFile here.

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.

I'll try to switch to fscommon.WriteFile.
It's a great way.

@kolyshkin
Copy link
Copy Markdown
Contributor

Would be nice to switch to fscommon.WriteFile here; it will eliminate the filemode arg.

@KentaTada KentaTada force-pushed the refactor-mode-of-notifylinux branch from 368fc5d to 90375b0 Compare March 15, 2021 05:50
@thaJeztah
Copy link
Copy Markdown
Member

Wondering if these should be moved into libcontainer/cgroups (not as part of this PR)

Comment thread libcontainer/notify_linux.go Outdated
const cgEvCtlFile = "cgroup.event_control"
data := fmt.Sprintf("%d %d %s", eventfd.Fd(), evFile.Fd(), arg)
if err := ioutil.WriteFile(eventControlPath, []byte(data), 0700); err != nil {
if err := fscommon.WriteFile(cgDir, "cgroup.event_control", data); err != nil {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This should be

Suggested change
if err := fscommon.WriteFile(cgDir, "cgroup.event_control", data); err != nil {
if err := fscommon.WriteFile(cgDir, cgEvCtlFile, data); err != nil {

@KentaTada KentaTada force-pushed the refactor-mode-of-notifylinux branch from 90375b0 to debb300 Compare July 5, 2021 02:54
This commit adjusts the file mode to use the latest golang style
In addition to that, I changed those modes from 0700 to 0600
as same as opencontainers#2636

Related to opencontainers#2625

Signed-off-by: Kenta Tada <Kenta.Tada@sony.com>
@KentaTada KentaTada force-pushed the refactor-mode-of-notifylinux branch from debb300 to a41b75d Compare July 5, 2021 03:14
@KentaTada
Copy link
Copy Markdown
Contributor Author

I'm not familiar with the current implementations but I think openFile() checks whether the actual cgropfs is used.
And so this change could not pass the test.
I should modify the test.

@@ -23,10 +23,10 @@ func testMemoryNotification(t *testing.T, evName string, notify notifyFunc, targ
}
evFile := filepath.Join(memoryPath, evName)
eventPath := filepath.Join(memoryPath, "cgroup.event_control")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should this also use the const? (cgEvCtlFile)

@kolyshkin
Copy link
Copy Markdown
Contributor

Since this file already exists, the mode argument is ignored, so there is nothing to fix here.

@kolyshkin kolyshkin closed this Jun 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants