Skip to content

daemon.initCgroupsPath fix unhandled error in recursion#39704

Closed
thaJeztah wants to merge 1 commit intomoby:masterfrom
thaJeztah:daemon_handle_error
Closed

daemon.initCgroupsPath fix unhandled error in recursion#39704
thaJeztah wants to merge 1 commit intomoby:masterfrom
thaJeztah:daemon_handle_error

Conversation

@thaJeztah
Copy link
Member

This was added in 56f77d5 (#23430), but I don't see a mention of errors being deliberately ignored here, so this might just have been missed during review

This was added in 56f77d5, but I
don't see a mention of errors being deliberately ignored here,
so this might just have been missed during review

Signed-off-by: Sebastiaan van Stijn <[email protected]>
@thaJeztah
Copy link
Member Author

@kolyshkin @crosbymichael ptal

Copy link
Contributor

@kolyshkin kolyshkin left a comment

Choose a reason for hiding this comment

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

Seems reasonable

LGTM

// for the period and runtime as this limits what the children can be set to.
daemon.initCgroupsPath(filepath.Dir(path))
if err := daemon.initCgroupsPath(filepath.Dir(path)); err != nil {
return err
Copy link
Contributor

Choose a reason for hiding this comment

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

please wrap

Copy link
Contributor

Choose a reason for hiding this comment

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

if not here, then in initCgroupsPath

Copy link
Member Author

Choose a reason for hiding this comment

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

This is initCgroupsPath; it's recursing. The final error is "wrapped" (well, fmt.Errorf - we should change that) here;

moby/daemon/oci_linux.go

Lines 793 to 795 in 3042254

if err := daemon.initCgroupsPath(parentPath); err != nil {
return fmt.Errorf("linux init cgroups path: %v", err)
}

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm talking about package boundaries like lines 1564, 1583 and 1586

Copy link
Member

Choose a reason for hiding this comment

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

ping @thaJeztah

@thaJeztah thaJeztah added the kind/refactor PR's that refactor, or clean-up code label Oct 10, 2019
@AkihiroSuda
Copy link
Member

ping @thaJeztah

@kolyshkin
Copy link
Contributor

should be obsoleted by #41016 (which was updated after reading review comments above)

@thaJeztah thaJeztah deleted the daemon_handle_error branch July 16, 2020 19:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/daemon Core Engine kind/refactor PR's that refactor, or clean-up code status/2-code-review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants