config: Add Windows Devices to Schema #976
Conversation
config-windows.md
Outdated
|
|
||
| Each entry has the following structure: | ||
|
|
||
| * **`id`** *(string, REQUIRED)* - specifies the device for which the runtime must make available in the container. |
There was a problem hiding this comment.
"for which" -> "which". Also "must" -> "MUST".
config-windows.md
Outdated
| Each entry has the following structure: | ||
|
|
||
| * **`id`** *(string, REQUIRED)* - specifies the device for which the runtime must make available in the container. | ||
| * **`id_type`** *(string, REQUIRED)* - tells the runtime how to interpret `id`. Today, Windows only supports a value of `class`, which identifies `id` as a device class GUID. |
There was a problem hiding this comment.
Is there a doc link with more background on device class GUIDs? Maybe this? It would be nice to give readers a way to unpack that jargon.
config-windows.md
Outdated
| ] | ||
| ``` | ||
|
|
||
| All devices on the host that ascribe to the declared device class GUID interface will be made available in the container. |
There was a problem hiding this comment.
This line seems redundant with the earlier "MUST be available in the container". Can we drop this line?
There was a problem hiding this comment.
Yes, that sounds reasonable.
schema/defs-windows.json
Outdated
| "type": "object", | ||
| "required": [ | ||
| "id", | ||
| "id_type" |
There was a problem hiding this comment.
I think you should define these as strings as well as marking them required. Alternatively, id_type could be an enum.
config-windows.md
Outdated
| ### Example | ||
|
|
||
| ```json | ||
| "devices": [ |
There was a problem hiding this comment.
nit: The other examples in config-windows.md include the "windows" object to which this is an entry. What you have here, on the other hand, is closer to the example approach taken in config-linux.md, where you only show the key being defined in this subsection. I don't know if the maintainers have a preference, but personally I think consistency within this file is more important. And ideally we'd have consistency across the whole spec, so we wouldn't have to make these decisions for each new example ;).
Signed-off-by: Craig Wilhite <crwilhit@microsoft.com> This adds Windows Device support to the OCI runtime-spec: code, JSON schema and markdown documentation. This is a first step in our approach to lighting up access to host devices for Windows Server containers. Devices in Windows implement any number of interface class GUIDs. By configuring a container with a device interface class GUID, we will plug all devices on the host that implement the interface class GUID into the device namespace of the container. While this approach does not allow for the speciication of a single, particular device, our schema definition is defined to be flexible; we interpret the 'id' of the device passed in based upon the 'id_type'.
|
@wking - fixed from suggestions. |
There was a problem hiding this comment.
I have no Windows experience, so I can't comment on whether this is actually useful or the best modelling for the functionality. But as far as internal-consistency with spec conventions goes, 840d343 looks good to me.
I don't have a preference on this myself, but some maintainers might prefer camelCase idType (or just type?) to id_type. See #859 for some previous Windows / camelCase discussion.
Signed-off-by: Craig Wilhite <crwilhit@microsoft.com>
|
Bumping this thread to see status on getting this PR pulled in. |
config-windows.md
Outdated
| Each entry has the following structure: | ||
|
|
||
| * **`id`** *(string, REQUIRED)* - specifies the device which the runtime MUST make available in the container. | ||
| * **`id_type`** *(string, REQUIRED)* - tells the runtime how to interpret `id`. Today, Windows only supports a value of `class`, which identifies `id` as a [device interface class GUID][interfaceGUID]. |
There was a problem hiding this comment.
Are there going to be other types other than class?
Also this should be idType not id_type
There was a problem hiding this comment.
I also wonder if this is really flexible enough for future needs -- will all future resources that might count as devices have some kind of ID? (I guess it's Windows we're talking about, so everything has a GUID of some kind?)
Being able to attach all sound devices to a container is neat, but if you want something like raw hard drive or USB drive access you'd have to attach all drives, which isn't great, right? (I'm guessing that's why idType is included here -- so id could instead point to the GUID of an individual device at some point in the future?)
There was a problem hiding this comment.
@crosbymichael Thanks for catching the typo, let me commit a fix. In the future, we'll certainly have more types.
That's correct, @tianon. Each device in Windows is guaranteed to have a unique device instance path ID. In the future it would be easy to extend so that id= and idType="instance" or some other label. Given this, I think what we have is extensible to give finer granularity in defining a device for a Windows container in the future.
|
after @crosbymichael feedback, LGTM |
Signed-off-by: Craig Wilhite <crwilhit@microsoft.com>
|
LGTM |
1 similar comment
|
LGTM |
Signed-off-by: Craig Wilhite crwilhit@microsoft.com
This adds Windows Device support to the OCI runtime-spec: code, JSON schema and markdown documentation. This is a first step in our approach to lighting up access to host devices for Windows Server containers.
Devices in Windows implement any number of interface class GUIDs. By configuring a container with a device interface class GUID, we will plug all devices on the host that implement the interface class GUID
into the device namespace of the container.
While this approach does not allow for the speciication of a single, particular device, our schema definition is defined to be flexible; we interpret the 'id' of the device passed in based upon the 'id_type'.