Skip to content
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

Support assigning devices into LCOW #1215

Merged

Conversation

katiewasnothere
Copy link
Contributor

In linux, there is no way to predetermine the dev device path since dev nodes are determined and created by the device driver. As a result, we need to search for the dev node that corresponds with a given device that we've already assigned in with vpci support.

Unfortunately, there is no great way to get the dev node information from the content in the sysfs path for the vmbus instance added for the assigned device, which we can easily determine. One option would be to query linux uevents for device node creation events. However, we determined that route was overly complex for today's needs. Instead, this PR adds simple logic to find pci assigned devices in LCOW and add them to the container spec. This works by first finding the full sysfs path of the vmbus instance and then looping through all /dev devices on the host UVM to find the device that links to that vmbus device.

Signed-off-by: Kathryn Baldauf [email protected]

@katiewasnothere katiewasnothere requested a review from a team as a code owner November 2, 2021 20:01
Comment on lines +34 to +46
case vpciDeviceIDTypeLegacy, vpciDeviceIDType:
// validate that the device is ready
fullPCIPath, err := pci.FindDeviceFullPath(ctx, d.ID)
if err != nil {
return errors.Wrapf(err, "failed to find device pci path for device %v", d)
}
// find the device node that links to the pci path we just got
dev, err := devicePathFromPCIPath(fullPCIPath)
if err != nil {
return errors.Wrapf(err, "failed to find dev node for device %v", d)
}
addLinuxDeviceToSpec(ctx, dev, spec, true)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need to handle an invalid device type?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose at this point we'd already validated it before passing it over to the guest

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well better question, don't need to handle the gpu case here*?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, gpu devices are handled through a different code path, see workload_container.go.

We could add an invalid error here but like you said, it's already validated at this point.

internal/hcsoci/devices.go Outdated Show resolved Hide resolved
Copy link
Contributor

@dcantah dcantah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, all comments are quick fixes

internal/hcsoci/devices.go Outdated Show resolved Hide resolved
internal/hcsoci/devices.go Outdated Show resolved Hide resolved
Signed-off-by: Kathryn Baldauf <[email protected]>
@katiewasnothere katiewasnothere merged commit 7646525 into microsoft:master Nov 30, 2021
@katiewasnothere katiewasnothere deleted the support_lcow_assigned_device branch November 30, 2021 00:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants