Update console for runc integration#14
Conversation
|
|
||
| func (m *master) Name() string { | ||
| return "" | ||
| } |
There was a problem hiding this comment.
not particularly sure about this. Now that i think a bit more, maybe exposing File() *os.File is better for cross-platform. I think in go its always "/dev/stdin".
There was a problem hiding this comment.
@mlaventure would have a good idea what the windows side needs. its actually very complex because they use all 3 stdio for the console ( i think )
There was a problem hiding this comment.
On windows, the console can only be made from os.Std{in,out,err} and not an arbitrary file descriptor like on linux, so the returned value here doesn't matter.
This is due to the fact that a process can only be associated with one Console on windows and usually that's already the case when it's created (I guess that's the reason from the lack of AllocConsole()/GetStdHandle() use in the code - it was ported over from the docker codebase -)
I guess you could just return "console" here.
when sending the console over i.e. (unix.Sendfd in runc), we also use name as auxiliary data in the payload. This exposes `Name()` function. I think this should be enough, maybe a bit leaner than exposing `File` For windows, name is an empty string. Not sure if this is correct but i dont know what are other options. Signed-off-by: Daniel Dao <dqminh89@gmail.com>
similar to opencontainers/runc#1479, we want to use this for console's consumer so they dont have to create their own implementation. Signed-off-by: Daniel Dao <dqminh89@gmail.com>
|
LGTM |
Still a work in progress while im making changes in runc too ( but i think this are all we need ). Opening this a bit early for some API reviews first.
With this change, we should be able to remove all console related code in runc.