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

WIP: s390x: run chreipl only on zVM #1274

Closed
wants to merge 1 commit into from
Closed

WIP: s390x: run chreipl only on zVM #1274

wants to merge 1 commit into from

Conversation

nikita-dubrovskii
Copy link
Contributor

There is no need to re-IPL disk under QEMU/KVM

@nikita-dubrovskii nikita-dubrovskii changed the title s390x: run chreipl only on zVM WIP: s390x: run chreipl only on zVM Aug 22, 2023
@nikita-dubrovskii nikita-dubrovskii mentioned this pull request Aug 22, 2023
Open
dustymabe
dustymabe reviewed Aug 22, 2023
View reviewed changes
src/s390x/zipl.rs
@@ -28,8 +28,11 @@ use tempfile::{Builder, NamedTempFile};

/// Sets the boot device to `dev` using `chreipl`.
Copy link
Member

Choose a reason for hiding this comment

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

Could we add a comment here to explain why we only want to do this on zvm? As someone who doesn't know anything about s390x it would probably help me understand. What are the other platfomrs and what will systemd-detect-virt return on s390x that isn't zVM?

@nikita-dubrovskii
Copy link
Contributor Author

I'm inclined to close this PR. But some details worth to mention here:
When we install Linux on zVM we always use so-called reader devices (with kernel,ramfs,cmdline) to boot, once system installation is done and zipl updated the bootloader, we can mark dasd / scsi as next boot target.
Qemu doesn't require but still supports chreipl, as it has bootindex option.
And here comes a problem - kola uses iso image to install VM and doesn't change boot order (bootindex=1) after installation, so CDROM is always the 1st boot device. So let's just keep this functionality

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

@dustymabe dustymabe dustymabe left review comments

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
@nikita-dubrovskii @dustymabe