Windows: OCI process struct and console size to uint
#26579
Conversation
lowenna
force-pushed
the
jjh/ociprocess
branch
4 times, most recently
from
0fd4f5f
to
a3701ad
Compare
lowenna
changed the title
process struct and console to uint
lowenna
changed the title
process struct and console to uintprocess struct and console size to uint
|
@mlaventure @tonistiigi @stevvooe PTAL. Thanks. |
| @@ -313,7 +313,7 @@ type HostConfig struct { | |||
| Runtime string `json:",omitempty"` // Runtime to use with this container | |||
|
|
|||
| // Applicable to Windows | |||
| ConsoleSize [2]int // Initial console size | |||
| ConsoleSize [2]uint // Initial console size | |||
There was a problem hiding this comment.
Wish ConsoleSize would have been 2 fields initially, but changing it now would break compatibility.
Could you specify in the comment which order they are supposed to be in the array? (i.e. is it Height, Width or Width, Height)
There was a problem hiding this comment.
Actually, thinking about this, I don't have to deal with back compat on Windows yet. I'll change it to a structure.
There was a problem hiding this comment.
@mlaventure Updated to use type Box as per this comment in the matching OCI spec PR (opencontainers/runtime-spec#563 (comment)). Also updated it to ConsoleSize as per opencontainers/runtime-spec#563 (comment), dropping Initial. I'll open a follow-up PR to update the HCSShim APIs to also use a structure and re-vendor with the changes once this one is in.
lowenna
force-pushed
the
jjh/ociprocess
branch
2 times, most recently
from
415a19a
to
b8b60f9
Compare
Signed-off-by: John Howard <jhoward@microsoft.com>
lowenna
force-pushed
the
jjh/ociprocess
branch
from
b8b60f9
to
5377442
Compare
|
all green, merging |
|
@jhowardmsft @mlaventure @tonistiigi @thaJeztah this commit broke backward compatibility - old users cannot post anymore an |
|
we should check API version for this 537744#diff-49c513a9f9836642d75d7f8478e8f521R316 |
|
@runcom, yes it was in one of my comment. But, if I'm not mistaken, the Windows support is only going to be considered "stable" once Windows Server 2016 RTM is released. |
|
Oh, damned. Just realized this is part of the common |
:) wanna take a peek at this? I'm going ahead and fix it if I can |
|
Ugh. I can fix unless you have something already in progress @runcom? |
|
Oh hang on, there's no need to fix this, unless it's broken Linux? Windows compatibility doesn't matter. |
@jhowardmsft it's broken on linux as well :( I'm trying to find some time to fix this tomorrow morning - buried into something |
|
Don't worry - I'll fix shortly. Sorry for not thinking about that. |
Windows: OCI `process` struct and console size to uint
Signed-off-by: John Howard jhoward@microsoft.com
Convergence closer to OCI compliance - this modifies the Process struct definition in the hacked OCI spec used on Windows to match the existing structure from https://github.com/opencontainers/runtime-spec/blob/master/specs-go/config.go#L30-L54, but also adding the initialConsoleSize field which is required on Windows submitted to OCI as opencontainers/runtime-spec#563.
As a result of opencontainers/runtime-spec#563 (comment), have changed the docker codebase to use uint instead of int for the console size throughout.
There is one further update which will be required (and simplify the code by no longer needing to cast from uint to int in the Windows libcontainerd files) after microsoft/hcsshim#67 is merged, and a revendor of https://github.com/Microsoft/hcsshim is done in docker.
There are no functional changes in the PR.