-
Notifications
You must be signed in to change notification settings - Fork 263
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Pass disk handle for computestorage.FormatWritableLayerVhd on RS5 #1204
Conversation
378d627
to
8b76d52
Compare
On RS5 the HcsFormatWritableLayerVhd call expects to receive a disk handle. On 19h1+ you can pass a vhd handle and internally they will do the same work we're doing in this change to grab a disk handle to perform the format. Signed-off-by: Daniel Canter <[email protected]>
8b76d52
to
ee0f993
Compare
@helsaawy Wanna take a look at this one? :) |
Fix DetachVHD error check and assignment. Signed-off-by: Daniel Canter <[email protected]>
56bb91d
to
44b7ee9
Compare
lgtm 👍 |
if err != nil { | ||
return err | ||
} | ||
defer windows.CloseHandle(diskHandle) // nolint: errcheck |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we need to close this handle but we don't close the handle that's created if this section of code doesn't run?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't handle closing the vhd handle passed in as it's expected the caller will close it. This function owns this new disk handle essentially so we need to close it ourselves if we hit this path.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah okay, makes sense
Related work items: microsoft#1067, microsoft#1097, microsoft#1119, microsoft#1170, microsoft#1176, microsoft#1180, microsoft#1181, microsoft#1182, microsoft#1183, microsoft#1184, microsoft#1185, microsoft#1186, microsoft#1187, microsoft#1188, microsoft#1189, microsoft#1191, microsoft#1193, microsoft#1194, microsoft#1195, microsoft#1196, microsoft#1197, microsoft#1200, microsoft#1201, microsoft#1202, microsoft#1203, microsoft#1204, microsoft#1205, microsoft#1206, microsoft#1207, microsoft#1209, microsoft#1210, microsoft#1211, microsoft#1218, microsoft#1219, microsoft#1220, microsoft#1223
… RS5 (microsoft#1204)" This reverts commit aaf5db9. We'd added a change to FormatWritableLayerVhd to help the caller work around a breaking change in the OS, but this would actually cause a breaking change in our wrapper of it if the caller was already working around the issue themselves. To avoid this scenario, revert the commit that added the "friendly" behavior. Signed-off-by: Daniel Canter <[email protected]>
… RS5 (microsoft#1204)" This reverts commit aaf5db9. We'd added a change to FormatWritableLayerVhd to help the caller work around a breaking change in the OS, but this would actually cause a breaking change in our wrapper of it if the caller was already working around the issue themselves. To avoid this scenario, revert the commit that added the "friendly" behavior. Signed-off-by: Daniel Canter <[email protected]>
On RS5 the HcsFormatWritableLayerVhd call expects to receive a disk handle.
On 19h1+ you can pass a vhd handle and internally they will do the same
work we're doing in this change to grab a disk handle to perform the format.
Signed-off-by: Daniel Canter [email protected]