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

rust-vmm / vfio

14 stars 20 forks source link

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

Closed jinankjain closed 4 months ago

jinankjain commented 5 months ago

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:

[x] 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.
[x] All added/changed functionality has a corresponding unit/integration test.
[x] All added/changed public-facing functionality has entries in the "Upcoming Release" section of CHANGELOG.md (if no such section exists, please create one).
[x] Any newly added unsafe code is properly documented.

jinankjain commented 4 months ago

friendly ping for reviews? @sameo @sboeuf @jiangliu

rbradford commented 4 months ago

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 https://github.com/illicitonion/num_enum/issues/18) or maybe drop that depenency if it's not really required.

jinankjain commented 4 months ago

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

jinankjain commented 4 months ago

I have a PR to fix the problem on MSHV side: https://github.com/rust-vmm/mshv/pull/139

jinankjain commented 4 months ago

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

jinankjain commented 4 months ago

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