doc: stage1-implementors-guide.md: clarify pid vs ppid #2397

alban · 2016-04-08T11:47:43Z

Clarify that only one file must be written among pid / ppid.

Reword the description. With the kvm flavor, the pid 1 of the container
was not visible on the host. The new wording does not make that
assumption.

/cc @jellonek @jonboulle @s-urbaniak PTAL

Related to: #2389

jellonek · 2016-04-08T12:02:56Z

Documentation/devel/stage1-implementors-guide.md

@@ -50,10 +50,10 @@ An alternative stage 1 could forego systemd-nspawn and systemd altogether, or re
 All that is required is an executable at the place indicated by the `coreos.com/rkt/stage1/run` entrypoint that knows how to apply the pod manifest and prepared ACI file-systems to good effect.

 The resolved entrypoint must inform rkt of its PID for the benefit of `rkt enter`.
-Stage 1 must write the host PIDs of the pod's process #1 and that process's parent to these two files, respectively:
+Stage 1 implementors have two options for doing so; only one must be implemented:


Only? Or maybe "at least". But i'm not sure if it's true - in kvm flavor i was writing into ppid file, and this led to hang in endless loop, in entering process.

It was waiting for a child process of lkvm. But that child process does not exist.

I think that "only" is better. If both "pid" and "ppid" exist, which one is stage0 supposed to use?

Without "pid" entering process hangs. So this "only" still leads to mistake.

This was #2389, fixed by #2396. But still, only one of the two files are written by stage1.

We should clarify in the code here that the ordering is not important and only one of them must be supplied (currently the code reads as if it is favouring "pid", but if you factor in the raciness of the function then we can consider that ordering to be arbitrary).

Please either fix that in this PR or file a follow up and land this as-is

PR updated.

Clarify that only one file must be written among pid / ppid. Reword the description. With the kvm flavor, the pid 1 of the container was not visible on the host. The new wording does not make that assumption.

jonboulle · 2016-04-26T13:01:39Z

@jellonek as our lead other implementer - does this make sense to you now?

jonboulle · 2016-04-28T12:39:13Z

@jellonek please let us know if not and follow up

alban added component/stage1 kind/documentation labels Apr 8, 2016

jellonek reviewed Apr 8, 2016
View reviewed changes

alban force-pushed the alban/ppid-or-pid branch from 0c53237 to 2343d0a Compare April 18, 2016 13:51

alban added the needs/review label Apr 19, 2016

alban added this to the v1.5.0 milestone Apr 24, 2016

jonboulle removed the needs/review label Apr 26, 2016

alban force-pushed the alban/ppid-or-pid branch from 2343d0a to 0e8debe Compare April 26, 2016 12:18

doc: stage1-implementors-guide.md: clarify pid vs ppid

0e8debe

Clarify that only one file must be written among pid / ppid. Reword the description. With the kvm flavor, the pid 1 of the container was not visible on the host. The new wording does not make that assumption.

jonboulle merged commit fc3600e into rkt:master Apr 28, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

doc: stage1-implementors-guide.md: clarify pid vs ppid #2397

doc: stage1-implementors-guide.md: clarify pid vs ppid #2397

doc: stage1-implementors-guide.md: clarify pid vs ppid #2397

doc: stage1-implementors-guide.md: clarify pid vs ppid #2397

Conversation

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment