-
Notifications
You must be signed in to change notification settings - Fork 93
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
s390x: add support for zFCP SCSI and ECKD DASD devices on zVM and LPAR #61
Conversation
Goes along with : coreos/coreos-assembler#732 |
e84442d
to
b99a115
Compare
Should be ready by now. |
Aha ! I just discovered that |
Let's put this on hold until coreos/coreos-assembler#738 get merged. We might need to make many changes with it coming in. |
df40bd8
to
d83c8eb
Compare
d83c8eb
to
e65a492
Compare
e65a492
to
345d697
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.
This is coming together nicely! @tuan-hoang1, Please let us know when you're ready for a second review pass.
5d657ea
to
07ebfa2
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.
This looks good to me. I'd like to have verified before merging it in though. Was this PR tested to install both s390x and x86?
Yes, please help me to test these changes. I only tested in s390x till now, because mostly the code are for s390x. The only global change that might affect x86 is the |
cc @arithx for 2nd review pass of existing unresolved discussion. |
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.
It looks reasonable to me. I didn't test myself though. Once there is a second 👍 this can merge.
Added a few more folks to help get a second set of eyes for review. |
cc @Prashanth684 @dbenoit17 @andymcc who have been using this code in RHCOS on z/VM for some time now. Last commit was for David's question on what image can be used to install KVM as bare metal disk. |
@@ -723,8 +724,14 @@ write_image_to_dasd() { | |||
|
|||
# the first 2 tracks of the ECKD DASD are reserved | |||
first_track=2 | |||
boot_partition=($(fdisk -b 4096 -o DEVICE,START,SECTORS -l ${imagefile} | grep "${imagefile}1" | awk '{print $2,$3}')) | |||
root_partition=($(fdisk -b 4096 -o DEVICE,START,SECTORS -l ${imagefile} | grep "${imagefile}4" | awk '{print $2,$3}')) | |||
# in RHCOS4.2 using anaconda, we have root partition at index 2 while in FCOS it's 4 |
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.
I don't think this is a good thing to do when RHCOS 4.3 goes out - which matches FCOS. But for the time being, I don't have time/other way to do it.
I will test this against the latest commit and follow-up. Otherwise looks good to me so far. |
Tested by building local FCOS and RHCOS with installer ISO with commits and bare metal raw image. |
Unless @darkmuggle or @miabbott disagrees I think this should merge. |
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.
Not an expert with s390x, but it appears this code has been well tested/reviewed. 👍
I have tested with all but the abf14cf commit and everything works on s390x as expected. |
@tuan-hoang1 any objection to me merging this? |
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.
I don't see anything substantive other than stylistic nits.
There are cases where lsblk cannot get the output right away. Thus output= and UUID= would be empty making the loop to be useless. Put lsblk inside the loop too.
Also small change in sed-ing the sha256 signature
Significant changes include : - zFCP SCSI devices cannot be dd directly from qemu raw image, but dd each partition. Thus we have to extract the image, read and apply partition table accordingly, then dd partitions. Also remount tmpfs according to the size of decompressed image. Another good approach is using `sgdisk -R` which copies the whole GPT disk. - dracut-cmdline-ask.service is required on s390x, adding it at boot time (via generator) rather than hardcode in .target file affecting other arches. - write_zipl_bootloader(): chreipl to target disk at the end of the installation so in the next reboot, that disk will be in first priority.
In FCOS, root partition is always at index 4. Even with coreos-assembler commit d5299ad6d5caf3479aeeac10ce0879a48815f0a4, it's still at index 4 even though there are only 2 partitions. In RHCOS4.2, anaconda only creates 2 partitions, boot and root. Also change the rest of hard-coded mountpoints to variables, which was missed earlier in a3052c5.
In coreos/fedora-coreos-tracker#305 we decided that, we want to write s390x opts to BLS files to persist after second boot. Respect all networking opts that are provided. This aligns with what write_networking_opts() function does.
Only the last instance of multiple instances of, for example, rd.znet= was parsed. Now we catch 'em all.
At first the assumption was, using bare metal raw image to install on zFCP SCSI disks on z/VM and LPAR. For KVM, users would be expected to use qemu/openstack images to boot instead of using coreos-installer to install on disk. Basically the bare metal raw image that works on zFCP SCSI disk would be working just fine for qemu's raw disk, we just didn't let this happen due to above assumption. Now we remove this restriction.
abf14cf
to
868e38d
Compare
Thanks @dbenoit17 @zonggen for testing ! |
zipl records need to be updated, because ignition.firstboot is burned into target disk during coreos-installer As a short-term solution for: coreos#84 Depends on: ibm-s390-linux/s390-tools#71 ibm-s390-linux/s390-tools#74 Related: coreos/coreos-installer#61 coreos/coreos-assembler#780 (cherry picked from commit 38af701)
zipl records need to be updated, because ignition.firstboot is burned into target disk during coreos-installer As a short-term solution for: #84 Depends on: ibm-s390-linux/s390-tools#71 ibm-s390-linux/s390-tools#74 Related: coreos/coreos-installer#61 coreos/coreos-assembler#780 (cherry picked from commit 38af701)
s390x: add support for zFCP SCSI and ECKD DASD devices on zVM and LPAR
No description provided.