-
Notifications
You must be signed in to change notification settings - Fork 168
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
rework osbuild s390x; drop dead code from recent cleanups #3920
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
It was determined that this was the exact same as the 4k disk image for s390x so we stopped building it. Let's remove the dead references from our code base. Closes coreos#3766
This is dead code now since d37958a.
Since d37958a this file that gets created is no longer used and gets overwritten by the file created from the OSBuild call.
Since d37958a this code is now only used in the s390x case. Let's move it down and colocate it with the rest of the related code.
Mostly just trying to make it a little cleaner and eliminate the extra extra_target_device_opts long string.
$path and $img were highly related. Here I rename $path to $imgpath and $img to $imgname to be more explicit and correct some usages. Previously they were being used interchangably because it just so happens `${PWD}/file` and `./file` point to the same place. Here I also stop using `${path}.tmp` everywhere and just use $imgpath because I don't really think the `.tmp` and the intermediate `finalize-artifact "${path}.tmp" "${path}"` are still needed.
These aren't used in the runvm-osbuild flow, but were for create_disk.sh, which was dropped in d37958a.
We shouldn't need to pass extra parameters around for qemu-secex and should be able to just ask for that "platform" to be produced from OSBuild without specifying more. This commit removes a bunch of extra variables that get set and passed around and also refactors the osbuild manifests to have qemu-secex treated more like the other platforms that we have in its own qemu-secex.ipp.yaml file. This commit also introduces a symlink cmd-buildextend-qemu-secex that points to cmd-buildextend-metal too (in addition to the existing cmd-buildextend-secex symlink). Since `qemu-secex` is the ID that is used in the meta.json I think we should try to stick with it more.
That allows making directories on mount:// targets. osbuild/osbuild#1904
Loogs good. Will test it today. |
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.
LGTM.
rhcos-418.94.202411040804-0-qemu-secex.s390x.qcow2
passed tests
nikita-dubrovskii
approved these changes
Nov 4, 2024
Thanks @nikita-dubrovskii |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I'm starting to work through how we can get away from our 1:1 mapping of
cosa
command to artifact generated (i.e.cosa buildextend-metal
yields ametal.raw
image) and can move more towards a generic command where we runosbuild
and generate many artifacts in one invocation. These are some cleanups that will help us get towards that goal eventually.See individual commit messages.