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.

Sign up for GitHub

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

Jump to bottom

loader: Re-organize module layout #197

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

loader: Re-organize module layout #197

wants to merge 3 commits into from
+28 −68

Conversation

TimePrinciple
Copy link
Contributor

Summary of the PR

loader: Re-organize module layout

Re-organize modules of loader to follow image format, enable reuse of
code across different architectures.

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.

@TimePrinciple TimePrinciple force-pushed the repurpose-loader branch 4 times, most recently from 85b10c9 to c1cf33f Compare October 17, 2024 03:13
@TimePrinciple
Copy link
Contributor Author

Should we consider modify our CI to ignore changes not committed, or change our bench test code to skip download, use local binary?

This setup is confusing while having a binary in our repo and download it from internet when testing 😵‍💫

@roypat
Copy link
Collaborator

roypat commented Oct 17, 2024

Should we consider modify our CI to ignore changes not committed, or change our bench test code to skip download, use local binary?

This setup is confusing while having a binary in our repo and download it from internet when testing 😵‍💫

I agree its a confusing/annoying, but I'm not sure about the license implications of including the kernel binary in the repository, so let's keep it the way it is for now :/

@TimePrinciple
Copy link
Contributor Author

Should we consider modify our CI to ignore changes not committed, or change our bench test code to skip download, use local binary?
This setup is confusing while having a binary in our repo and download it from internet when testing 😵‍💫

I agree its a confusing/annoying, but I'm not sure about the license implications of including the kernel binary in the repository, so let's keep it the way it is for now :/

I don't know why after re-organizing the layout makes the bzimage binary different, see https://buildkite.com/rust-vmm/linux-loader-ci/builds/764#01929877-c73e-4d57-9a9c-1bb5ef78e105

In that run, I haven't remove the binary :( but it's changed somehow after the test downloaded it

@TimePrinciple
loader: Re-organize module layout
5f59395 
Re-organize modules of `loader` to follow image format, enable reuse of
code across different architectures. Its interface is unchanged.

Signed-off-by: Ruoqing He <heruoqing@iscas.ac.cn>
@TimePrinciple TimePrinciple force-pushed the repurpose-loader branch 3 times, most recently from 51e1a11 to a099c14 Compare October 20, 2024 14:21
@TimePrinciple
configurator: Re-organize module layout
df0098a 
Code in `fdt.rs` do not contain architecture specific code as of
`aarch64`, it's only used by `aarch64`, move it one level up to allow
reusing on riscv64 platform.

Signed-off-by: Ruoqing He <heruoqing@iscas.ac.cn>
@TimePrinciple TimePrinciple force-pushed the repurpose-loader branch 2 times, most recently from cc8dec1 to d5b8810 Compare October 20, 2024 14:49
@TimePrinciple
benches: Re-organize module layout
073b198 
Code in `mod.rs` of `aarch64` do not contain architecture specific code
as of `aarch64`, it's only used by `aarch64` to run benchmark tests,
move it one level up and rename it to `fdt.rs` to allow reusing on
riscv64 platform.

Signed-off-by: Ruoqing He <heruoqing@iscas.ac.cn>
@TimePrinciple
Copy link
Contributor Author

Should we consider modify our CI to ignore changes not committed, or change our bench test code to skip download, use local binary?
This setup is confusing while having a binary in our repo and download it from internet when testing 😵‍💫

I agree its a confusing/annoying, but I'm not sure about the license implications of including the kernel binary in the repository, so let's keep it the way it is for now :/

I don't know why after re-organizing the layout makes the bzimage binary different, see https://buildkite.com/rust-vmm/linux-loader-ci/builds/764#01929877-c73e-4d57-9a9c-1bb5ef78e105

In that run, I haven't remove the binary :( but it's changed somehow after the test downloaded it

Update: fixed by @roypat in rust-vmm/rust-vmm-ci#169

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sameo sameo Awaiting requested review from sameo sameo is a code owner

@rbradford rbradford Awaiting requested review from rbradford rbradford is a code owner

@roypat roypat Awaiting requested review from roypat roypat is a code owner

@andreitraistaru andreitraistaru Awaiting requested review from andreitraistaru andreitraistaru is a code owner

@ShadowCurse ShadowCurse Awaiting requested review from ShadowCurse ShadowCurse is a code owner

At least 2 approving reviews are required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@TimePrinciple @roypat