scsi: Support optional guest path for mount, add tests, refcount fix #2278
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
|
I think you're missing the AddVirtualDisk call in a few places, particularly the runhcs create-scratch call, some functional tests, and the uvmboot tool. |
kiashok
approved these changes
Sep 27, 2024
Currently the SCSI mount manager will generate a new path for each new mount, based on a format string that it is instantiated with. However, it turns out some code in the GCS (e.g. sandbox mounts) assumes that the container scratch is mounted at a certain path. The long-term best solution here is probably to pass what paths to use explicitly to the GCS, but that would be more impactful. We need a more contained fix. This commit addresses the issue by allowing an optional guest path to be given for a SCSI mount. The mount manager has been changed as follows: - If a guest path is not supplied: The mount can re-use (refcount++) any existing mount with the same controller/lun/options. If a new mount is created, the mount manager will generate a path for it. - If a guest path is supplied: The mount can re-use (refcount++) any existing mount with the same controller/lun/guestpath/options. If a new mount is created, the mount manager will use the supplied path for it. Accordingly, code calling into the mount manager has been updated to pass an empty string for the guest path. The exception to this is the LCOW layer mounting code, which will pass an explicit guest path for the scratch disk. As far as I know, WCOW does not depend on a specific path for its scratch disk. Signed-off-by: Kevin Parsons <kevpar@microsoft.com>
kevpar
force-pushed
the
scsi-mount-fix-2
branch
from
78d08f4
to
6830384
Compare
Thanks, fixed |
Actually might still need to fix tests. I think find references in gopls might have missed those |
kevpar
force-pushed
the
scsi-mount-fix-2
branch
2 times, most recently
from
cdc61ef
to
a596691
Compare
Adds various tests to the SCSI manager code. As part of this testing, a bug tracking attachment refcounts was also found. The attachment refcount is increased every time a new top-level request comes in, even if an existing mount is re-used. This means that the attachment refcount should also be decremented every time one is released, even if the mount refcount is not at 0. Signed-off-by: Kevin Parsons <kevpar@microsoft.com>
kevpar
force-pushed
the
scsi-mount-fix-2
branch
from
a596691
to
3b7c087
Compare
|
@katiewasnothere any concerns once CI passes? |
# for free
to join this conversation on GitHub.
Already have an account?
# 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.
Adds support for callers to
internal/uvm/scsi.Managerto specify a path where the disk will be mounted in the guest.Adds unit tests for SCSI manager behavior.
Fixes a refcount bug found through the tests.
Details in the individual commits.