-
Notifications
You must be signed in to change notification settings - Fork 32
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
Introduce riscv64 CI container #106
Introduce riscv64 CI container #106
Conversation
530d01b
to
43b9cf3
Compare
43b9cf3
to
fd1b0ba
Compare
Very cool!! Could you do a draft PR to rust-vmm-ci with the changes needed for this? :o |
Surely, I have the required work done in |
Introduce logic necessary for generating YAML needed by BuildKite, which are designed to work with image introduced in rust-vmm/rust-vmm-container#106. Signed-off-by: Ruoqing He <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, this looks very reasonable to me, modulo some minor comments.
One thing we need to keep in mind though (and this might be more of a comment for rust-vmm/rust-vmm-ci#159 and not a blocker here) is that we need a way to selectively enable the riscv64 CI checks on a per repository basis. Currently, if we merged this PR and rust-vmm-ci#159, it'd enable rustv64 checks on all repositories. I suspect that for a bunch of them, it will not work immediately (e.g. the rootfs compiled here doesn't contain all of the vhost-user stuffs, I suspect). The fact that not all rust-vmm repos are initially supported is fine - we can iterate as we go, just start with kvm-bindings and kvm-ioctls, and add whatever is needed for the other repositories as we actually add riscv64 support to them. But it does mean that we'll need some sort of "repository allowlist", maybe some sort of .platforms
file in the repo root that lists for which platforms CI should be enabled?
cc @stefano-garzarella @stsquad for opinions on this :)
ff88ffa
to
364a81b
Compare
Very reasonable, that means we also need logic in that python script of detecting which repository it's running in. Possibly by looking at Or should the |
I'm somewhat inclined to have it in each repository (with the default being x86 + arm for repos that don't have the file), as that will allow us to just enable the riscv64 CI via a PR to the repository on which we want to enable it (instead of making a change in rust-vmm-ci and then waiting for dependabot to propagate it), but I'd like to hear some more opinions on this |
364a81b
to
ae34d59
Compare
Introduce logic necessary for generating YAML needed by BuildKite, which are designed to work with image introduced in rust-vmm/rust-vmm-container#106. Signed-off-by: Ruoqing He <[email protected]>
Yeah +1 on this!
I was also thinking something like this, it should be easy to implement and also to activate. |
6377783
to
e498223
Compare
Introduce logic necessary for generating YAML needed by BuildKite, which are designed to work with image introduced in rust-vmm/rust-vmm-container#106. Signed-off-by: Ruoqing He <[email protected]>
d5e3ac4
to
f11bcae
Compare
f11bcae
to
2c5cdf9
Compare
Introduce logic necessary for generating YAML needed by BuildKite, which are designed to work with image introduced in rust-vmm/rust-vmm-container#106. Signed-off-by: Ruoqing He <[email protected]>
Introduce logic necessary for generating YAML needed by BuildKite, which are designed to work with image introduced in rust-vmm/rust-vmm-container#106. Signed-off-by: Ruoqing He <[email protected]>
Introduce logic necessary for generating YAML needed by BuildKite, which are designed to work with image introduced in rust-vmm/rust-vmm-container#106. Signed-off-by: Ruoqing He <[email protected]>
2c5cdf9
to
cbba1ba
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just minor things, the rest LGTM.
cbba1ba
to
5bbd928
Compare
Introduce logic necessary for generating YAML needed by BuildKite, which are designed to work with image introduced in rust-vmm/rust-vmm-container#106. Signed-off-by: Ruoqing He <[email protected]>
@TimePrinciple LGTM, please can you rebase? |
Add build scripts for v6.10 riscv64 kernel, qemu-system-riscv64 and opensbi required to boot qemu-system inside docker container. With this approach, we are able to run tests inside qemu-system, while preserving the original output as much as possbile with ssh. This is the third draft proceeds rust-vmm#101, rust-vmm#104. It is expected to be replaced by the second draft rust-vmm#104 in the future which standardise `riscv64`. Signed-off-by: Ruoqing He <[email protected]>
5bbd928
to
97d421d
Compare
Yes, already rebased :) |
Introduce logic necessary for generating YAML needed by BuildKite, which are designed to work with image introduced in rust-vmm/rust-vmm-container#106. The container version is updated to v39 to enable CI on RISC-V platform. Signed-off-by: Ruoqing He <[email protected]>
Introduce logic necessary for generating YAML needed by BuildKite, which are designed to work with image introduced in rust-vmm/rust-vmm-container#106. The container version is updated to v40 to enable CI on RISC-V platform. Signed-off-by: Ruoqing He <[email protected]>
@TimePrinciple our workflow is failing signing the riscv image: https://github.com/rust-vmm/rust-vmm-container/actions/runs/10581126625/job/29317670656 Can you take a look? |
Ops, I just saw the new #112 PR for that, thanks! |
Introduce logic necessary for generating YAML needed by BuildKite, which are designed to work with image introduced in rust-vmm/rust-vmm-container#106. The container version is updated to v44 to enable CI on RISC-V platform. Signed-off-by: Ruoqing He <[email protected]>
Introduce logic necessary for generating YAML needed by BuildKite, which are designed to work with image introduced in rust-vmm/rust-vmm-container#106. The container version is updated to v44 to enable CI on RISC-V platform. Signed-off-by: Ruoqing He <[email protected]>
Summary of the PR
This work was inspired by the work done by @endeneer in PR #91, and is the third draft proceeds #101, #104. It is expected to be replaced by #104 in the future.
Add build scripts for v6.10 riscv64 kernel, qemu-system-riscv64, opensbi and rootfs required to boot qemu-system. And an entrypoint to forward the commands accepted to qemu-system inside the container.
With this approach, we are able to run tests inside qemu-system, while preserving the original output as much as possbile with ssh.
Requirements
Before submitting your PR, please make sure you addressed the following
requirements:
git commit -s
), and the commit message has max 60 characters for thesummary and max 75 characters for each description line.
test.
Release" section of CHANGELOG.md (if no such section exists, please create one).
unsafe
code is properly documented.