Most of the tests in rust-vmm-ci are just Buildkite pipeline steps which makes it impossible to run them locally before submitting a PR.

One idea would be to have something similar to the coverage test we already have, but we need to think about a flexible design so that the same thing can be used in buildkite pipelines and still keep the nice individual step/rust command.

The current way of invoking clippy as part of the CI pipeline (cargo clippy --all -- -D warnings) does not check some parts of the code. It looks like the exhaustive list of relevant arguments is something along the lines of cargo clippy --workspace --bins --examples --tests --benches --all-targets --all-features -- -D warnings, with the caveat that --all-features might not make sense for all crates.

As kcov seems to work on arm64, I think it is the time to consider fully enable and test the ci coverage test of rust-vmm on arm64. @andreeaflorescu @michael2012z

FYI, please check the attached kcov_output dir for kvm-ioctls
kcov_output.zip

test_commit_format forever hangs when the repository is specified with git instead of https.
The reason it hangs is that doing git fetch in the docker container awaits user input to fetch from the unknown host (in this case github). A quick fix here is to run ssh-keyscan -t rsa github.com >> ~/.ssh/known_hosts before calling git fetch. This way at least the test will just fail, and not hang.

The second problem is that we need to forward the ssh keys on the host for git to work (and configure the ssh agent). No idea how to do that in a safe/sane way.

A recurrent problem we have in rust-vmm PRs is that we have unsigned commits. Let's make this part of the CI as it is easy to miss during reviews.

Formatting for code examples is only available in nightly for now. So we first need to resolve: rust-vmm/rust-vmm-container#36

I would propose we add a rustfmt.toml file as part of the test such that not all repositories need to be changed (also, cargo fmt will fail if you add some option in rustfmt.toml which is not supported in stable). The content of the file needs to be:

format_code_in_doc_comments = true

After running the test using cargo +nightly fmt, we need to delete the created file.

Some, if not all, crates will require long running tests for more relevant performance measurements, as well as stability and more. This might resolve into dedicated Buildkite pipelines for each crate and may not be centralized in rust-vmm-ci, but until that's ironed out, we'll be tracking the matter in one issue. We can follow up with per-crate issues if it resolves to that.

Add a dedicated Buildkite pipeline for performance tests for each crate that has such tests (alongside the "standard" CI pipeline). In the future, this can help separate performance runs from build / CI runs, and split tests on different machines according to our needs.

It's worth looking into critcmp as it seems easy to run and get a verdict from.

Long term, long duration performance tests will demand separate runs (nightly / on some other schedule). To begin with, we can add performance tests in the CI because:

• it's easier to add them there short term
• it will give us early indicators of how long they take
• it will help us assess the stability of results

The way test_bench works is that it checks weather there are any performance regressions introduced by running cargo bench on both the PR commits and on master.
This introduces an interesting problem because typically you add both the tests and the pipeline update to run those tests in the same PR. The PR will thus always fail because running cargo bench on master (when there are no tests) will fail with:

error: Unrecognized option: 'noplot'
error: bench failed

There is an obvious workaround:

• merge the performance tests first (including the Cargo.toml configuration that makes that error disappear)
• merge the pipeline (aka the update so that the tests run) in a following PR

I find this workaround to be pretty ugly because it requires tribal knowledge and can introduce confusion. Granted, it happens only the first time performance tests are added, but I still think it's pretty ugly.

Alternatively, what we can do is that we can only run cargo bench when the benchmarks exists.

I would propose the following change. Instead of running cargo bench --all-features -- --noplot --save-baseline {} and use the [lib] bench = false in Cargo.toml to avoid the problem specified here, we can use cargo bench --all-features --bench main -- --noplot --save-baseline {}. This implies that all benchmarks for all crates would need to be named main (or at least their entry point), which seems to be a lesser evil than having to merge first the tests and the Cargo.toml update and then the pipeline update. With this change then we can check if main benchmark is defined or not, and if it is not defined, we simply do not compare after with before.

The advantages of these proposal:

• the performance tests run the first time they're introduced; this way we can see if the performance tests actually work (there will be no comparison, which sounds reasonable)
• there is no need to have 2 PRs when you need to introduce the performance tests for the first time

WDYT?

The template mandates 50/72 limit. I think it is very easy to go over the 50 characters limit for the summary line.

https://github.com/rust-vmm/.github/blob/25304ffcedb6e7ecdfa77655deeace5a86b6a6b0/pull_request_template.md?plain=1#L11

We need to be able to select the hypervisor for certain pipeline (for example, for kvm-ioctls, the hypervisor needs to be kvm). Right now the EC2 hosts don't specify a hypervisor, while the mshv hosts do. This needs to be configured on the host, so it's not directly related to this repository.

Nevertheless, we do need to update the code in this crate as well because we need some way of selecting the hypervisor at the crate level and or at the test level. When using the tests defined in rust-vmm-ci, we need to be able to override the agent tags. There is already some functionality to override tags using environment variables, so we should experiment with that as well.

Depends on rust-vmm/rust-vmm-container#30

Add a test to check that the commit message has an appropriate format:

• it has a summary no longer than 50 chars (we can also adjust this number if we think it's too small)
• each line of the body is less than 72 chars

          Setting `export PKG_CONFIG_ALLOW_CROSS=1` was enough to get through that, but:

1. I can't build pipewire with musl, it complains stdbool.h is missing. I tried installing musl-dev and setting the library path to musl's dir in .cargo/config.toml:

   [target.x86_64-unknown-linux-musl]
   linker = "ld"
   rustflags = ["-Ctarget-feature=-crt-static", "-Clink-self-contained=on", "-L/usr/lib/x86_64-linux-musl", "-Clink-args=--dynamic-linker /lib/ld-musl-x86_64.so.1"]

   but no dice. Any ideas?

2. Disabling default features with --no-default-features gives me errors from rust-vmm crates:

   Compiling vhost-user-backend v0.10.1
error[E0277]: the trait bound `<M as GuestAddressSpace>::M: vm_memory::guest_memory::GuestMemory` is not satisfied
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/vhost-user-backend-0.10.1/src/vring.rs:145:23
    |
145 |             .add_used(self.mem.memory().deref(), desc_index, len)
    |              -------- ^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `vm_memory::guest_memory::GuestMemory` is not implemented for `<M as GuestAddressSpace>::M`
    |              |
    |              required by a bound introduced by this call
    |
note: required by a bound in `add_used`
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/virtio-queue-0.9.1/src/lib.rs:193:20
    |
193 |     fn add_used<M: GuestMemory>(&mut self, mem: &M, head_index: u16, len: u32)
    |                    ^^^^^^^^^^^ required by this bound in `QueueT::add_used`
help: consider further restricting the associated type
    |
143 |     pub fn add_used(&mut self, desc_index: u16, len: u32) -> Result<(), VirtQueError> where <M as GuestAddressSpace>::M: vm_memory::guest_memory::GuestMemory {
    |                                                                                       +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++

error[E0277]: the trait bound `<M as GuestAddressSpace>::M: vm_memory::guest_memory::GuestMemory` is not satisfied
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/vhost-user-backend-0.10.1/src/vring.rs:159:40
    |
159 |         self.queue.enable_notification(self.mem.memory().deref())
    |                    ------------------- ^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `vm_memory::guest_memory::GuestMemory` is not implemented for `<M as GuestAddressSpace>::M`
    |                    |
    |                    required by a bound introduced by this call
    |
note: required by a bound in `enable_notification`
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/virtio-queue-0.9.1/src/lib.rs:201:31
    |
201 |     fn enable_notification<M: GuestMemory>(&mut self, mem: &M) -> Result<bool, Error>;
    |                               ^^^^^^^^^^^ required by this bound in `QueueT::enable_notification`
help: consider further restricting the associated type
    |
158 |     pub fn enable_notification(&mut self) -> Result<bool, VirtQueError> where <M as GuestAddressSpace>::M: vm_memory::guest_memory::GuestMemory {
    |                                                                         +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++

error[E0277]: the trait bound `<M as GuestAddressSpace>::M: vm_memory::guest_memory::GuestMemory` is not satisfied
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/vhost-user-backend-0.10.1/src/vring.rs:164:41
    |
164 |         self.queue.disable_notification(self.mem.memory().deref())
    |                    -------------------- ^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `vm_memory::guest_memory::GuestMemory` is not implemented for `<M as GuestAddressSpace>::M`
    |                    |
    |                    required by a bound introduced by this call
    |
note: required by a bound in `disable_notification`
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/virtio-queue-0.9.1/src/lib.rs:204:32
    |
204 |     fn disable_notification<M: GuestMemory>(&mut self, mem: &M) -> Result<(), Error>;
    |                                ^^^^^^^^^^^ required by this bound in `QueueT::disable_notification`
help: consider further restricting the associated type
    |
163 |     pub fn disable_notification(&mut self) -> Result<(), VirtQueError> where <M as GuestAddressSpace>::M: vm_memory::guest_memory::GuestMemory {
    |                                                                        +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++

error[E0277]: the trait bound `<M as GuestAddressSpace>::M: vm_memory::guest_memory::GuestMemory` is not satisfied
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/vhost-user-backend-0.10.1/src/vring.rs:169:39
    |
169 |         self.queue.needs_notification(self.mem.memory().deref())
    |                    ------------------ ^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `vm_memory::guest_memory::GuestMemory` is not implemented for `<M as GuestAddressSpace>::M`
    |                    |
    |                    required by a bound introduced by this call
    |
note: required by a bound in `needs_notification`
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/virtio-queue-0.9.1/src/lib.rs:212:30
    |
212 |     fn needs_notification<M: GuestMemory>(&mut self, mem: &M) -> Result<bool, Error>;
    |                              ^^^^^^^^^^^ required by this bound in `QueueT::needs_notification`
help: consider further restricting the associated type
    |
168 |     pub fn needs_notification(&mut self) -> Result<bool, VirtQueError> where <M as GuestAddressSpace>::M: vm_memory::guest_memory::GuestMemory {
    |                                                                        +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++

error[E0308]: mismatched types
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/vhost-user-backend-0.10.1/src/vring.rs:185:41
    |
185 |             .try_set_desc_table_address(GuestAddress(desc_table))?;
    |              -------------------------- ^^^^^^^^^^^^^^^^^^^^^^^^ expected `GuestAddress`, found a different `GuestAddress`
    |              |
    |              arguments to this method are incorrect
    |
    = note: `GuestAddress` and `GuestAddress` have similar names, but are actually distinct types
note: `GuestAddress` is defined in crate `vm_memory`
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/vm-memory-0.12.2/src/guest_memory.rs:109:1
    |
109 | pub struct GuestAddress(pub u64);
    | ^^^^^^^^^^^^^^^^^^^^^^^
note: `GuestAddress` is defined in crate `vm_memory`
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/vm-memory-0.13.1/src/guest_memory.rs:111:1
    |
111 | pub struct GuestAddress(pub u64);
    | ^^^^^^^^^^^^^^^^^^^^^^^
    = note: perhaps two different versions of crate `vm_memory` are being used?
note: method defined here
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/virtio-queue-0.9.1/src/queue.rs:132:12
    |
132 |     pub fn try_set_desc_table_address(&mut self, desc_table: GuestAddress) -> Result<(), Error> {
    |            ^^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0308]: mismatched types
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/vhost-user-backend-0.10.1/src/vring.rs:187:41
    |
187 |             .try_set_avail_ring_address(GuestAddress(avail_ring))?;
    |              -------------------------- ^^^^^^^^^^^^^^^^^^^^^^^^ expected `GuestAddress`, found a different `GuestAddress`
    |              |
    |              arguments to this method are incorrect
    |
    = note: `GuestAddress` and `GuestAddress` have similar names, but are actually distinct types
note: `GuestAddress` is defined in crate `vm_memory`
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/vm-memory-0.12.2/src/guest_memory.rs:109:1
    |
109 | pub struct GuestAddress(pub u64);
    | ^^^^^^^^^^^^^^^^^^^^^^^
note: `GuestAddress` is defined in crate `vm_memory`
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/vm-memory-0.13.1/src/guest_memory.rs:111:1
    |
111 | pub struct GuestAddress(pub u64);
    | ^^^^^^^^^^^^^^^^^^^^^^^
    = note: perhaps two different versions of crate `vm_memory` are being used?
note: method defined here
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/virtio-queue-0.9.1/src/queue.rs:147:12
    |
147 |     pub fn try_set_avail_ring_address(&mut self, avail_ring: GuestAddress) -> Result<(), Error> {
    |            ^^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0308]: mismatched types
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/vhost-user-backend-0.10.1/src/vring.rs:189:40
    |
189 |             .try_set_used_ring_address(GuestAddress(used_ring))
    |              ------------------------- ^^^^^^^^^^^^^^^^^^^^^^^ expected `GuestAddress`, found a different `GuestAddress`
    |              |
    |              arguments to this method are incorrect
    |
    = note: `GuestAddress` and `GuestAddress` have similar names, but are actually distinct types
note: `GuestAddress` is defined in crate `vm_memory`
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/vm-memory-0.12.2/src/guest_memory.rs:109:1
    |
109 | pub struct GuestAddress(pub u64);
    | ^^^^^^^^^^^^^^^^^^^^^^^
note: `GuestAddress` is defined in crate `vm_memory`
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/vm-memory-0.13.1/src/guest_memory.rs:111:1
    |
111 | pub struct GuestAddress(pub u64);
    | ^^^^^^^^^^^^^^^^^^^^^^^
    = note: perhaps two different versions of crate `vm_memory` are being used?
note: method defined here
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/virtio-queue-0.9.1/src/queue.rs:161:12
    |
161 |     pub fn try_set_used_ring_address(&mut self, used_ring: GuestAddress) -> Result<(), Error> {
    |            ^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0277]: the trait bound `<M as GuestAddressSpace>::M: vm_memory::guest_memory::GuestMemory` is not satisfied
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/vhost-user-backend-0.10.1/src/vring.rs:210:23
    |
210 |             .used_idx(self.mem.memory().deref(), Ordering::Relaxed)
    |              -------- ^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `vm_memory::guest_memory::GuestMemory` is not implemented for `<M as GuestAddressSpace>::M`
    |              |
    |              required by a bound introduced by this call
    |
note: required by a bound in `used_idx`
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/virtio-queue-0.9.1/src/lib.rs:190:20
    |
190 |     fn used_idx<M: GuestMemory>(&self, mem: &M, order: Ordering) -> Result<Wrapping<u16>, Error>;
    |                    ^^^^^^^^^^^ required by this bound in `QueueT::used_idx`
help: consider further restricting the associated type
    |
208 |     fn queue_used_idx(&self) -> Result<u16, VirtQueError> where <M as GuestAddressSpace>::M: vm_memory::guest_memory::GuestMemory {
    |                                                           +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++

Some errors have detailed explanations: E0277, E0308.
For more information about an error, try `rustc --explain E0277`.
error: could not compile `vhost-user-backend` (lib) due to 8 previous errors

Originally posted by @epilys in rust-vmm/vhost-device#493 (comment)

According to this comment, breaking changes related to cargo audit (which may turn into errors in time, see this thread and this PR) can be caught earlier if we run cargo audit with --deny-warnings enabled.

If we choose to enable this flag, cargo audit will also fail if we expose in Cargo.toml a version that was yanked. An example would be vm-superio: 0.1.0 was yanked, but master still points to 0.1.0 in Cargo.toml (with other commits on top). Basically, after we yank a version, cargo audit --deny-warning will force us to release a new version on the branch on which we are running the test if the toml file is pointing to the yanked version (which sounds pretty reasonable).

The rust-vmm-ci provides the common tests to be run by all rust-vmm crates. The problem is that we have a few things that should be configurable.

One example is the possibility to configure the devices passed when running the unit tests. For example, vhost tests need access to the vdpa device, but adding it explicitly in the rust-vmm-ci excessively constrains the CI of all other components when this is not in fact needed (i.e. all other components need to run on a host that has access to the vdpa device, and the vdpa device needs to be passed to the docker image when running the tests). More details about this here: #64

At the moment we're running the kcov command with --all flag. While this is a good default as we typically want to run all the tests in a workspace, there are situations in which we just want to run the tests for the current crate. This is the case for crates that we want to publish as part of the same workspace.

Running ./rust-vmm-ci/test_run.py locally fails for vhost-device repository.

Commands I ran:

$ docker run --device=/dev/kvm -it --security-opt seccomp=unconfined --volume /home/vireshk/vhost-device:/vhost-device --volume ~/.ssh:/root/.ssh rustvmm/dev:v13

cd vhost-device; ./rust-vmm-ci/test_run.py

Failure Message:

test_commit-format (main.TestsContainer) ... =============================================================================================== test session starts ===============================================================================================
platform linux -- Python 3.8.10, pytest-6.2.5, py-1.10.0, pluggy-1.0.0
rootdir: /vhost-device
plugins: timeout-1.4.2
collected 1 item

rust-vmm-ci/integration_tests/test_commit_format.py F [100%]

==================================================================================================== FAILURES =====================================================================================================
_______________________________________________________________________________________________ test_commit_format ________________________________________________________________________________________________

def test_commit_format():
    """
    Checks commit message format for the current PR's commits.

    Checks if commit messages follow the 50/72 git commit rule
    [https://www.midori-global.com/blog/2018/04/02/git-50-72-rule]
    and if commits are signed.
    """
    # Fetch the upstream repository.
    fetch_base_cmd = "git fetch {} {}".format(REMOTE, BASE_BRANCH)
    try:
        subprocess.run(fetch_base_cmd, shell=True, check=True)
    except subprocess.CalledProcessError:

      raise NameError(

            "The name of the base branch or remote is invalid. "
            "See test documentation for more details."
        ) from None

E NameError: The name of the base branch or remote is invalid. See test documentation for more details.

rust-vmm-ci/integration_tests/test_commit_format.py:42: NameError
---------------------------------------------------------------------------------------------- Captured stderr call -----------------------------------------------------------------------------------------------
Bad owner or permissions on /root/.ssh/config
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.
============================================================================================= short test summary info =============================================================================================
FAILED rust-vmm-ci/integration_tests/test_commit_format.py::test_commit_format - NameError: The name of the base branch or remote is invalid. See test documentation for more details.
================================================================================================ 1 failed in 0.03s ================================================================================================
ERROR

According to the publishing documentation¹, CODEOWNERS and the gatekeper team must be owners of a crate published on crates.io in addition to the original crate authors. We can add a check that runs for every crate located in a repository that verifies the ownership status is correct with the crates.io API².

Example:

Find all crates:

for manifest_file in $(find . -type f -name Cargo.toml | grep --invert-match "/target/" | grep --invert-match "/fuzz/"); do
  NAME=$(grep --max-count=1 name "${manifest_file}" | sed -e 's/name = "\(.\+\)"/\1/')
  if ! [ -z "${NAME}" ]; then
    echo ${NAME}
  fi
done

Check owner for a crate:

CRATE_NAME="$1"

echo "INFO: querying https://crates.io/api/v1/crates/${CRATE_NAME}/owners"

OWNERS_REPLY=$(curl --silent "https://crates.io/api/v1/crates/${CRATE_NAME}/owners")

echo "INFO: API reply was ${OWNERS_REPLY}"

case "${OWNERS_REPLY}" in
  *'{"errors":[{"detail":"Not Found"}]}'* ) echo "INFO: Crate ${CRATE_NAME} was not found on crates.io and is assumed unpublished." ; exit 0 ;; # Crate is not published.
esac

ERROR=""

case "${OWNERS_REPLY}" in
  *github:rust-vmm:gatekeepers* ) echo "OK: rust-vmm:gatekeepers is an owner." ;;
  * ) echo "ERROR: rust-vmm:gatekeepers team must be in ${CRATE_NAME}'s owners." ; ERROR="yes";;
esac

for owner in $(sed -e '/^#.\+$/d' -e 's/^[*]\s*//' -e 's/[@]//g' < CODEOWNERS); do
  case "${OWNERS_REPLY}" in
    *${owner}* )
      echo "OK: ${owner} is an owner."
      ;;
    * )
      echo "ERROR: ${owner} is not an owner." ; ERROR="yes"
  esac
done

if ! [ -z "${ERROR}" ]; then
  echo "Missing crate owners for ${CRATE_NAME}, please add them."
  exit 1
fi

Running it for virtio-bindings outputs:

INFO: querying https://crates.io/api/v1/crates/virtio-bindings/owners
INFO: API reply was {"users":[{"avatar":"https://avatars.githubusercontent.com/u/1043863?v=4","id":35756,"kind":"user","login":"sameo","name":"Samuel Ortiz","url":"https://github.com/sameo"},{"avatar":"https://avatars.githubusercontent.com/u/2815944?v=4","id":37110,"kind":"user","login":"andreeaflorescu","name":"Andreea Florescu","url":"https://github.com/andreeaflorescu"},{"avatar":"https://avatars.githubusercontent.com/u/8278356?v=4","id":53759,"kind":"user","login":"epilys","name":"Manos Pitsidianakis","url":"https://github.com/epilys"},{"avatar":"https://avatars.githubusercontent.com/u/115481277?v=4","id":186043,"kind":"user","login":"roypat","name":"Patrick Roy","url":"https://github.com/roypat"},{"avatar":"https://avatars.githubusercontent.com/u/46028664?v=4","id":1481,"kind":"team","login":"github:rust-vmm:gatekeepers","name":"gatekeepers","url":"https://github.com/rust-vmm"}]}
OK: rust-vmm:gatekeepers is an owner.
ERROR: alexandruag is not an owner.
OK: andreeaflorescu is an owner.
ERROR: jiangliu is not an owner.
ERROR: slp is not an owner.
ERROR: stsquad is not an owner.
OK: epilys is an owner.
Missing crate owners for virtio-bindings, please add them.

At a minimum it should check that commits are signed.

It would also be nice that each commit in this repository runs all tests in a repository. This would be useful to catch bugs such as the one fixed by #48

The commit format of depandabot does not confirm to the 50/72 rule of commits. Since this is not in our control, we should add an exception for it.

Log example: https://buildkite.com/rust-vmm/linux-loader-ci/builds/277#7213c428-ba53-4604-a230-7d997e274c3a

Not sure if we also want to keep the debug mode, but we should run the tests using --release.

We should make sure we don't introduce dependencies in rust-vmm crates that have incompatible licenses. We can check the licenses of all crates using cargo license or some other tool.

We shouldn't have too many crates that we depend on, but still it is worth checking cargo audit as part of our CI.

Detail here: https://github.com/RustSec/cargo-audit

When computing the coverage percentage, we should exclude the tests directory which included the Rust integration tests.

As a workaround, we can use the exclude_path field in the coverage_config_x86_64.json and coverage_config_aarch64.json and add "tests" to it.

A permanent solution is to add "tests" to the exlude_pattern:

Currently the rust-vmm CI runs carg test --all-features see:

This doesn't work for certain cases e.g. rust-vmm/vhost#218 (comment).

To test all features while supporting configuration for exclusive features unittests-gnu should run cargo test-all-features using https://github.com/frewsxcv/cargo-all-features.

This test is taking a dependency on Buildkite specific infrastructure. Specifically, it expect the following environment variables to be set in order to run the test: BUILDKITE_PULL_REQUEST_BASE_BRANCH and BUILDKITE_REPO.

We need to find good defaults for those environment variables in case they're not set so that we can run the tests locally. The challenge here comes from identifying a good default for the BUILDKITE_PULL_REQUEST_BASE_BRANCH as this is the branch we use for figuring out which commits are part of the PR.

From a short round of thinking, I could not find a perfect solution. A meh solution would be to always assume that the base branch is the master of the configured origin remote. This is a meh solution because it implies that the developer updates their local master branch from the upstream master branch so that the diff in commits is truly what is on the local branch from which they wish to open a PR. This assumes good practices (for example PRs are never opened from the master branch).

Other solutions are welcomed.

Running kcov on aarch64 doesn't seem to work as expected.

For the same code on x86 vs aarch64 we get differences from 1-4% which seems a lot for medium sized code bases (>10k lines of code).

We should either fix the kcov problem or remove the test altogether.

For rust-vmm/vm-memory#19 I would like to use optionally the integer atomic types from Rust 1.34. Right now I would have to keep them out of CI (and not use --all-features in the pipeline files), it would be great if you could update the Rust image that buildkite pulls.

To unblock rust-vmm/vhost#33, several conditions must be met:

1. CI kernel version should be >= 5.7 (currently 5.4)
2. vhost vdpa kernel modules should be loaded (see rust-vmm/vhost#33 (comment))
3. the /dev/vhost-vdpa-0 device should be exposed to the container (see rust-vmm/vhost#33 (comment))

1 and 2 can be fixed by updating the AMIs for the CI runners. This issue tracks step 3, for which we need to update the Buildkite pipeline so that the steps that involve running the unit tests can access the device. The device needs to be passed in the Buildkite yaml as devices: [ "/dev/vhost-vdpa-0" ] (see documentation).

Some paths are out of date in section "Adaptive Coverage".
Some error messages, like the one below, have out-of-date path too.
AssertionError: Coverage is increased from 67.9 to 69.6. Please update the coverage in tests/coverage.

Add a test using cargo-semver-checks to enforce semver.

This will require:

• Adding cargo-semver-checks to https://github.com/rust-vmm/rust-vmm-container
• Adding a test here (e.g. #134)
• Updating https://github.com/rust-vmm/community/blob/main/docs/crate_release.md to match new workflow.

We've recently hit an issue where an advisory was issued for an unmaintained crate (atty - advisory here) that several rust-vmm crates depend on. Since there's no apparent way to fix the issue (unresponsive maintainer), we need to allow for certain advisories such as this one to be skipped in the cargo audit test. Otherwise, CI will fail and block.

In test_bench, we now expect all benchmarks to be named main. This is because we are running the benchmarks as follows:
cargo bench --bench main --all-features -- --noplot --save-baseline {}.

Instead of harcoding the name to be main, we can parse the Cargo.toml file to check what is the name of the benchmarks.
We need to look for the bench target in Cargo.toml:

[[bench]]
name = "main"

To be able to eventually delete the static pipeline from rust-vmm-ci we need to update the CI for the existing components to use the autogenerate script instead (as specified in the readme).

This issue tracks the progress per component:

• vm-fdt
• vhost-device
• kvm-bindings
• crate-template (N/A)
• io-rate-limiter (N/A)
• mshv
• seccompiler
• vhost-user-backend
• vfio-ioctls
• vm-memory
• vm-virtio
• vm-device
• linux-loader
• vhost
• event-manager
• vm-superio
• vmm-sys-util
• virtio-bindings
• kvm-ioctls
• vmm-reference
• vfio-bindings
• vm-allocator (N/A)

Right now we only support selecting a buildkite agent to run the job based on the os (either windows or linux) and the platform tag (x86_64.metal or arm.metal). We need to add support for another agent tag as well so that the mshv CI can use the autogeneration tool to generate the tests.

Here is an example of the tags: https://github.com/rust-vmm/mshv/blob/1a9ca01801e78f8ddc2630660cbab3bc3300e39e/.buildkite/pipeline.yml#L43

The code that needs to be updated to support this is: https://github.com/rust-vmm/rust-vmm-ci/blob/main/.buildkite/autogenerate_pipeline.py#L211. We need to do a get for hypevisor, and set it in the agents tags. The code should be similar to the way we set the platform.

How about we add --profile test to clippy:

cargo clippy --all --profile test -- -D warnings

There is a ]missing in the README in the performance tests section for the URL of critcmp.

For example the coverage test will not work on workspaces because we are using the --features flag which is invalid in workspaces and only allowed for packages.

We need to create gitignore file first.

In test_bench we are doing a git checkout on the upstream master branch to check the diff in performance between the PR and the current master. This is problematic because if test_bench is not the last test to be run, all other tests will run on master instead of the PR branch.

We should make sure to properly clean up after the test (at all times, even when the test fails).

Make sure that there are no spaces in the JSON field that defines the features, otherwise we can encounter failures when running kcov.

Example of failure: https://buildkite.com/rust-vmm/mshv-ci/builds/7#cd3a2709-ad84-484a-9fef-ab41da00b7c8

The cargo audit, and the performance test are failing on arm because the tools were not installed due to OOM when building the container (see rust-vmm/rust-vmm-container#79). The fix in rust-vmm-container is merged, but we need to pull the latest container version here as well.

We should add a step in the pipeline that cleans up disk space by running docker system prune.

When running the integration tests with profile devel, we can leave the kcov_output directory so that it is easier to inspect the coverage report.
Alternatively, we can add a --no-cleanup so we don't leave lingering files on the system without the user being aware.

Right now the coverage test is not running for all features, but just the default ones.

Ideally, we should find a way to run coverage on all features (similar to what we have for unit tests with --all-features flag). The --all-features flag does not work with cargo kcov.