Add --device support for Windows#1290
Add --device support for Windows#1290jterry75 wants to merge 1 commit intodocker:masterfrom jterry75:add_device_support
Conversation
|
See: moby/moby:37638 for daemon side PR |
Codecov Report
@@ Coverage Diff @@
## master #1290 +/- ##
=======================================
Coverage 55.14% 55.14%
=======================================
Files 289 290 +1
Lines 19371 19371
=======================================
Hits 10683 10683
Misses 7997 7997
Partials 691 691 |
|
Think there's a typo on an error string. Otherwise code LGTM (not a maintainer in this repo) |
|
@thaJeztah - Is there anything I can help with to get this in? |
Adds support for --device in Windows. This must take the form of:
--device='class/{clsid}'
Signed-off-by: Justin Terry (VM) <juterry@microsoft.com>
|
@jhowardmsft - This is the client side. PTAL |
|
LGTM (not a maintainer) |
|
@thaJeztah Can this be merged now the daemon side is in? |
| @@ -0,0 +1,115 @@ | |||
| // +build !windows | |||
There was a problem hiding this comment.
🤔 Not compiling this on Windows means that starting a Linux container (Docker Desktop for Windows, or a Windows client connecting to a remote Linux daemon) will no longer be functional; I guess that's not what we want 😅
There was a problem hiding this comment.
How do you suggest I determine when to use what parser for the device assignment? I would have to ping the end node to determine if its a Windows or Linux host first?
There was a problem hiding this comment.
Take a look at cli\command\cli.go L212, function initializeFromClient. You can pull the server OS out of cli.serverInfo.OSType. IIRC every CLI command starts with a _ping API.
There was a problem hiding this comment.
Yes, unfortunately it gets complicated fast to take mixed environments into account 😞. I guess we should keep the separate functions for windows and linux (to keep it clean), and add a wrapper function that calls one of them, based on what the target platform is
There was a problem hiding this comment.
If possible (not sure that is in this case (haven't looked in depth)), we try to reduce validation on the client side to a minimum, and leave it to the daemon to return an error.
|
Closing in favor of #1606 |
Adds support for --device in Windows. This must take the form of:
--device='class/{clsid}'
Signed-off-by: Justin Terry (VM) juterry@microsoft.com
@jhowardmsft - FYI