Remove dependence on GetScsiUvmPath function #1742
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Apr 23, 2023
kevpar
force-pushed
the
getscsiuvmpath
branch
2 times, most recently
from
4575f4c
to
7305955
Compare
anmaxvl
approved these changes
May 1, 2023
Currently, when installing drivers on LCOW, we use GetScsiUvmPath to check if the VHD is already mounted, and if it is, we assume the drivers have already been installed, so we can skip doing it again. This check has a few problems: - It relies on GetScsiUvmPath, which assumes a single mount-point in the guest for a given VHD. This assumption is not safe to make in the face of future changes, where we could mount a device (or partitions on it) multiple times. - It assumes the disk has stayed attached the whole time after drivers were installed. This may be a safe assumption today, but can be fragile in the future. - It does not work in the case of a VHD containing multiple sets of drivers, or a VHD being changed/updated to newer content after first install. Again, this is safe given the current overall design today, but could break in the future. This change is still mostly a bandaid fix. Probably what is most correct is to track driver installation in something with state (the GCS) rather than using a separately invoked binary to do the in-guest install. However, this change does address the first issue above, removing the dependency on GetScsiUvmPath. I do this in the following way: - Change install-drivers to check if the overlay path exists already, and exit with a no-op if it does. This encodes the assumption that the overlay path will be consistent for a given driver set. - Change InstallDrivers in the shim to compute a V5 GUID from the VHD path, and use that as part of the overlay path given to the guest. This ensures there is a unique guest overlay path for each unique host driver VHD path. Signed-off-by: Kevin Parsons <kevpar@microsoft.com>
The WCOW-isolated SCSI mount process currently works as follows: - In resources_wcow.go, go through each mount on the OCI spec, and if it is a SCSI mount, add a mount to the UVM for it. - in hcsdoc_wcow.go, go through each mount on the OCI spec, use GetScsiUvmPath to determine the guest path it was mounted to, and add an entry to the container doc for it. This is quite hacky, as it relies on a 1:1 mapping between host VHDs and mounts in the guest, and also because it requires us to re-query information we've already been given. The SCSIMount object returned when we mounted to the guest can already tell us the guest path. This change resolves this problem by instead determing the set of guest mounts that should be added to the container doc at the time when the SCSI mounts are done, and saving it in the creation options. Then, when we construct the actual container doc, we just grab those mounts and add them in. Signed-off-by: Kevin Parsons <kevpar@microsoft.com>
msscotb
approved these changes
May 4, 2023
Merged
anmaxvl
pushed a commit
that referenced
this pull request
Oct 20, 2023
This PR updates our ADO fork to commits in hcsshim up to commit hash [7769a64](7769a64). This includes support for partitioned scsi devices and ensuring filesystem format for lcow scsi devices. Related work items: #1728, #1740, #1741, #1742, #1743, #1744, #1745, #1747, #1748, #1749, #1750, #1752, #1754, #1756, #1757, #1767, #1769, #1771, #1772, #1773, #1779
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is commit 3/6 in a chain. Recommended to review in order. If reviewing a later PR in the chain, you can view individual commits to see just what that PR changes.
GetScsiUvmPathencodes the unfortunate assumption that there is a single guest mount path for each SCSI device host path. This may be correct today, but there is no good reason for it to be treated as an invariant (mounting a SCSI device gives you back an object that tells you where it was mounted) and needs to not be true in the future (so we can do things like mounting multiple partitions from a device).These commits remove the usage of
GetScsiUvmPath, and replace it with just getting the guest path directly from the returnedSCSIMountobject.Please see individual commit messages for details.