stage1/kvm: Avoid writing misleading subcgroup #3107
Conversation
ebe1709 to
b4227f5
Compare
| @@ -581,7 +582,12 @@ func stage1() int { | |||
| return 1 | |||
| } | |||
|
|
|||
| args, env, err := getArgsEnv(p, flavor, debug, n, insecureOptions) | |||
| canMachinedRegister := false | |||
| if flavor != "kvm" { | |||
There was a problem hiding this comment.
How does fly fit into this PR? Should it be taken into account here?
There was a problem hiding this comment.
fly neither writes a file nor does machined registration (this init.go code is not called for it and there is no analogue in the stage1_fly/run/run.go code).
|
I see this is not trying to tackle the kvm-machined registration issue but just accommodating cgroups info for api-service. This is not what I was thinking about initially, but it makes sense too as it can help decoupling api-service from machined on the host. What I'm not completely sure at this point is how does "fly" (and everything else without |
|
Good call on the title being misleading, I'll fix that. No other stage1s attempt to do machined registration, so their cgroups will be accurate and there will be no such race. We do just read I don't think we have a good answer for what to do if an external system moves our cgroup after we're running in general (e.g. writing our ppid to the In this case, it's a stage1 responsibility because the external-cgroup-fiddling is at the behest of internal stage1 code, and any other stage1 that triggers a change to its cgroup can use the file similarly. |
Ref rkt#2664 The `init.go` code does not run nspawn with --register=true for kvm, but does write a subcgroup file of that form. This makes it a little saner, and also simplifies it down to only one dbus call rather than calling it several times in different places.
b4227f5 to
0a2760f
Compare
|
Are your concerns addressed, @lucab? Does anyone else want to weigh in on whether this seems right? |
|
Yes, it seems fine. I'm leaving some time for @iaguis to express any concerns on this till next Monday, and then merge. |
|
It already looked good to me before and I think we can just land it at this point, dealing with concerns/fallouts (if any) later. |
Ref #2664
This also documents the existing behavior as an optional stage1 feature/contract. Ref #3072.
The
init.gocode does not run nspawn with --register=true for kvm, butdoes write a subcgroup file of that form.
This makes it a little saner, and also simplifies it down to only one
dbus call rather than calling it several times in different places.