coreos/coreos-assembler

what is the difference between dasd and metal4k on s390x?

dustymabe opened this issue · 9 comments

dustymabe commented

I'm trying to understand what are the fundamental differences between our dasd and metal4k artifacts on s390x. We build both dasd and metal4k for RHCOS but from looking at the code in the following two files and some recent logs from pipeline runs from RHCOS 4.15 I can't really find significant differences.

Here are the two files in COSA I'm comparing:

Here are the two logs from the recent 4.15 run I'm comparing:

The only real changes I see between the two runs is:

  1. for metal4k --no-x86-bios-bootloader is passed to create_disk.sh, but this is a no-op on s390x so it is harmless.
  2. for the dasd log vs. the metal4k log the install disk was vdb and not vdc. I don't think this is significant.

So that begs the question: what is the actual difference between the dasd image and the metal4k image on s390x? They both are raw disk images. They both have ignition.platform.id=metal.

What am I missing?

jlebon commented

I think you're right. The most likely scenario is that when I opened #1130 way back when, I was focused on the x86_64 case and didn't realize that in the s390x case, it duplicated with the dasd format we had already learned to output.

Ideally we could remove dasd from our schema. Though it'd be some work to figure out how safe that is. But maybe we could do it in e.g. 4.17 if we release note it?

A safer approach we could do now is to just treat cosa buildextend-dasd as an alias for cosa buildextend-metal4k and always write to both .images.metal4k and .images.dasd with identical content (i.e. .path pointing at the same artifact). The ART mirrors could symlink also.

dustymabe commented

I think you're right. The most likely scenario is that when I opened #1130 way back when, I was focused on the x86_64 case and didn't realize that in the s390x case, it duplicated with the dasd format we had already learned to output.

👍

Ideally we could remove dasd from our schema. Though it'd be some work to figure out how safe that is. But maybe we could do it in e.g. 4.17 if we release note it?

Yeah. Maybe we should open a card for this and have someone investigate. The ultimate goal here is to simplify things and dropping whole artifacts is a big win IMO.

A safer approach we could do now is to just treat cosa buildextend-dasd as an alias for cosa buildextend-metal4k and always write to both .images.metal4k and .images.dasd with identical content (i.e. .path pointing at the same artifact). The ART mirrors could symlink also.

I'm actually OK shipping it as is for now if we can take steps towards the longer term goal of removing it.

I was investigating this for OSBuild purposes and the answer for now is pretty much to just copy the metal4k disk image output to a new dasd output stage. It's just a reflink copy so shouldn't hurt in the pipeline.

nikita-dubrovskii commented

I guess DASD is still here for historical reasons. As i remember: rhcos prior to 4.4 had no metal4k , but zVM support. Starting with 4.4 work on new rust-coreos-installer begun, and to make live easier (PR for adding s390x support to coreos-installer was for a long time on review) both dasd and new metal4k images lived in parallel.

nikita-dubrovskii commented

Also thanks to usage of osmet images fetching separate dasd.s390x.raw became obsolete. Maybe it makes sense to completely drop it?

dustymabe commented

Also thanks to usage of osmet images fetching separate dasd.s390x.raw became obsolete. Maybe it makes sense to completely drop it?

Are you saying that absolutely nothing uses it today?

jlebon commented

I think it'd be hard to rule that out entirely. It's on the mirrors, so anyone could be downloading that image and plugging it into their install flow. I can certainly imagine people being confused and picking the dasd image over the metal4k one because it's more specific to what they think they need.

dustymabe commented

we removed dasd from our build artifacts in RHCOS. At some point in the future we should be able to remove the code that supports building them (or probably more likely: switch to using OSBuild where support was never added).

jbtrystram commented

If i understand correctly @dustymabe last comment, we can close this since #3800 was merged ?