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

vfio-ioctls: Remove conditional compilation to x86_64 for mshv feature #43

Merged
merged 1 commit into from
Apr 10, 2024

Conversation

jinankjain
Copy link
Contributor

@jinankjain jinankjain commented Apr 1, 2024

Summary of the PR

We are planning to add support for ARM64 on cloud hypervisor side. Thus, remove the conditional compilation for target_arch x86_64.

Requirements

Before submitting your PR, please make sure you addressed the following
requirements:

  • All commits in this PR have Signed-Off-By trailers (with
    git commit -s), and the commit message has max 60 characters for the
    summary and max 75 characters for each description line.
  • All added/changed functionality has a corresponding unit/integration
    test.
  • All added/changed public-facing functionality has entries in the "Upcoming
    Release" section of CHANGELOG.md (if no such section exists, please create one).
  • Any newly added unsafe code is properly documented.

@jinankjain jinankjain force-pushed the arm64 branch 2 times, most recently from fe08f2b to 1b61455 Compare April 1, 2024 12:38
@jinankjain
Copy link
Contributor Author

friendly ping for reviews? @sameo @sboeuf @jiangliu

Copy link
Collaborator

@sboeuf sboeuf left a comment

Choose a reason for hiding this comment

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

LGTM

@rbradford
Copy link
Contributor

Here is the dependency chain that leads to toml_edit being pulled in

toml_edit v0.21.1
└── proc-macro-crate v3.1.0
    └── num_enum_derive v0.7.2 (proc-macro)
        └── num_enum v0.7.2
            └── mshv-bindings v0.1.1 (https://github.com/rust-vmm/mshv?branch=main#a2695a5b)
                ├── mshv-ioctls v0.1.1 (https://github.com/rust-vmm/mshv?branch=main#a2695a5b)
                │   └── vfio-ioctls v0.2.0 (/home/rob/src/rust-vmm/vfio/crates/vfio-ioctls)
                └── vfio-ioctls v0.2.0 (/home/rob/src/rust-vmm/vfio/crates/vfio-ioctls)

Might be worth adjusting the features used in mshv-bindings when depending on num_enum crate (see illicitonion/num_enum#18) or maybe drop that depenency if it's not really required.

@jinankjain
Copy link
Contributor Author

num_enum was recently pulled in the latest commit of mshv: rust-vmm/mshv@a2695a5. Let me see if we can remove toml_edit as a dependency.

@jinankjain
Copy link
Contributor Author

I have a PR to fix the problem on MSHV side: rust-vmm/mshv#139

@jinankjain
Copy link
Contributor Author

I tested with the PR branch, and it seems to have fix the problem. Here is a link to test run: https://buildkite.com/rust-vmm/vfio-ioctls-ci/builds/253#018ebdaf-a57c-470c-ba10-3563d6ef2fc5

We are planning to add support for ARM64 on cloud hypervisor side. Thus,
remove the conditional compilation for target_arch x86_64.

Signed-off-by: Jinank Jain <[email protected]>
@jinankjain
Copy link
Contributor Author

@rbradford Fix in mshv is merged and the pipeline is green now.

@liuw liuw merged commit 4e937a5 into rust-vmm:main Apr 10, 2024
2 checks passed
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.

4 participants