Skip to content

Extract cgroups utilities to own submodule.#3310

Merged
crosbymichael merged 1 commit intomoby:masterfrom
pnasrat:cgroups-subpackage
Jan 6, 2014
Merged

Extract cgroups utilities to own submodule.#3310
crosbymichael merged 1 commit intomoby:masterfrom
pnasrat:cgroups-subpackage

Conversation

@pnasrat
Copy link
Contributor

@pnasrat pnasrat commented Dec 21, 2013

Initial extraction of methods, no functionality changes or additions.

@pnasrat
Copy link
Contributor Author

pnasrat commented Dec 21, 2013

@crosbymichael this is just a start there is clearly a lot more we can do to put in decent structure and naming and testing. In part I wanted to ask should this actually be resources/... then having cgroups as an implementor.

@tianon
Copy link
Member

tianon commented Dec 21, 2013

I wonder if we should replace that /proc/mounts scanning with @crosbymichael's new mountinfo reading stuff.

@pnasrat
Copy link
Contributor Author

pnasrat commented Dec 22, 2013

@tianon @crosbymichael it seems reasonable I'll ammend to support the needed search and update this once done.

@creack
Copy link
Contributor

creack commented Dec 23, 2013

I love it :), but +1 @tianon. Can't wait to merge this!

@pnasrat
Copy link
Contributor Author

pnasrat commented Dec 24, 2013

Does this now need to go under pkg? Initial attempt to use mountinfo @crosbymichael

@creack
Copy link
Contributor

creack commented Dec 24, 2013

@pnasrat Yes, please move it to pkg

@creack
Copy link
Contributor

creack commented Dec 27, 2013

@pnasrat Please, can you rebase?

@pnasrat
Copy link
Contributor Author

pnasrat commented Dec 27, 2013

Yeah I just need to make some changes then push - probably tomorrow.

On 26 December 2013 19:24, Guillaume J. Charmes notifications@github.comwrote:

@pnasrat https://github.com/pnasrat Please, can you rebase?


Reply to this email directly or view it on GitHubhttps://github.com//pull/3310#issuecomment-31241539
.

@pnasrat
Copy link
Contributor Author

pnasrat commented Dec 27, 2013

Integration tests are failing - hold off review until I ping this PR again.

@pnasrat
Copy link
Contributor Author

pnasrat commented Dec 27, 2013

Logic was broken in rebasing, fixed but want to add some unit tests.

@pnasrat
Copy link
Contributor Author

pnasrat commented Dec 27, 2013

@creack @crosbymichael @tianon PTAL.

@pnasrat
Copy link
Contributor Author

pnasrat commented Dec 27, 2013

@pnasrat
Copy link
Contributor Author

pnasrat commented Dec 31, 2013

Rebased again - @creack @crosbymichael @tianon

@tianon
Copy link
Member

tianon commented Dec 31, 2013

+1 - looks reasonable to me

Copy link
Contributor

Choose a reason for hiding this comment

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

You don't need var here for only one declaration

Use mountinfo rather than for cgroups parsing.
Make helper method private and change name.
Makes method naming more explicit rather than GetThisCgroup.
Use upstream term subsystem rather than cgroupType.
@pnasrat
Copy link
Contributor Author

pnasrat commented Jan 2, 2014

@crosbymichael addressed your line comments.

@crosbymichael
Copy link
Contributor

LGTM

@vieux
Copy link
Contributor

vieux commented Jan 6, 2014

LGTM, ping @crosbymichael

crosbymichael added a commit that referenced this pull request Jan 6, 2014
Extract cgroups utilities to own submodule.
@crosbymichael crosbymichael merged commit 4f31141 into moby:master Jan 6, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants