Skip to content
This repository has been archived by the owner on Dec 20, 2021. It is now read-only.

Add MSHV support #22

Merged
merged 4 commits into from
Sep 7, 2021
Merged

Add MSHV support #22

merged 4 commits into from
Sep 7, 2021

Conversation

liuw
Copy link
Member

@liuw liuw commented Sep 2, 2021

These two patches add device fd support for MSHV to vfio-ioctls.

Note that patch is still needs one modification to point to rust-vmm's repository. It is pointing to a local path for MSHV. I will change it once rust-vmm/mshv#11 is merged.

Signed-off-by: Wei Liu <liuwe@microsoft.com>
@liuw liuw marked this pull request as ready for review September 3, 2021 14:18
@liuw
Copy link
Member Author

liuw commented Sep 3, 2021

@andreeaflorescu @lauralt

The pipeline failed due to mutually exclusive features (kvm and mshv shouldn't be enabled at the same time). Rust does not support for that use case. What's your thought on augmenting the pipeline for this use case?

I think what I need is:

  • Create a customized pipeline.
  • Disable some tests in the common pipeline.

@jiangliu @sameo @sboeuf

It would be good if you can have a look at the change before I take on the work to fix the pipeline. If adding mutually exclusive features is not popular I will need to come up with something else.

@andreeaflorescu
Copy link
Member

@liuw this was a problem that I was facing when implementing mutually exclusive features. The way I fixed it is to choose a default feature when both are specified. Here is the commit that was doing that: rust-vmm/kvm-bindings@190dbaa

I think it makes sense to not have both features enabled at the same time. At the CI level, you'll also need a custom pipeline to run cargo test & cargo build with the non-default feature.

src/vfio_device.rs Outdated Show resolved Hide resolved
src/vfio_device.rs Outdated Show resolved Hide resolved
Rename KvmSetDeviceAttr to SetDeviceAttr and modify private helper
functions to work with both hypervisors.

Signed-off-by: Wei Liu <liuwe@microsoft.com>
Generally speaking functions take RawFd should be marked as unsafe.
device_add_group and device_del group are such functions.

Instead of marks them as unsafe, we can make these two functions take a
reference to VfioGroup. This is far more elegant.

Signed-off-by: Wei Liu <liuwe@microsoft.com>
@liuw
Copy link
Member Author

liuw commented Sep 6, 2021

@andreeaflorescu I added a cunstom-tests.json to this project. The CI is not picking it up though. I suppose buildkite needs to be configured somehow?

@andreeaflorescu
Copy link
Member

@andreeaflorescu I added a cunstom-tests.json to this project. The CI is not picking it up though. I suppose buildkite needs to be configured somehow?

Yes, we need to configure it in Buildkite. Once we do the configuration no other PR will pass the tests until we merge this one. Should we configure it now or do you want to merge anything else before?

Rust does not support mutually exclusive features. Buildkite enables all
features. These two factors combined causes pipeline failures.

Turn the feature gates in code a bit. Whenever "kvm" is specify, surface
KVM's definitions. Make available MSHV if and only if "mshv" is
specified.

Add custom tests for MSHV.

Signed-off-by: Wei Liu <liuwe@microsoft.com>
@liuw
Copy link
Member Author

liuw commented Sep 6, 2021

@andreeaflorescu I added a cunstom-tests.json to this project. The CI is not picking it up though. I suppose buildkite needs to be configured somehow?

Yes, we need to configure it in Buildkite. Once we do the configuration no other PR will pass the tests until we merge this one. Should we configure it now or do you want to merge anything else before?

The other in flight PR is your dependabot PR, I suppose it should be rather easy to adapt to the new pipeline configuration?

If you can do it now that would be great, but I'm not sure how you want the other one to be handled.

@andreeaflorescu
Copy link
Member

The other PR can wait, we can merge this one first if there are not other PRs in flight. I'll setup buildkite today.

Copy link
Member

@andreeaflorescu andreeaflorescu left a comment

Choose a reason for hiding this comment

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

I added the Buildkite configuration and now the custom tests are also running: https://buildkite.com/rust-vmm/vfio-ioctls-ci/builds/58#404de176-1fbc-421d-b126-f82a9f18d1b3

.buildkite/custom-tests.json Show resolved Hide resolved
@jiangliu jiangliu merged commit a8ee64b into rust-vmm:main Sep 7, 2021
@liuw liuw deleted the mshv-support branch September 7, 2021 11:44
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants