Autorun fixes #101
Merged
Autorun fixes #101
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 not obvious what gets logged where and when, document that. Signed-off-by: Marc Herbert <marc.herbert@intel.com>
This will be re-used by systemd fixes Signed-off-by: Marc Herbert <marc.herbert@intel.com>
Stop hacking symbolic links and use .preset files to make sure ssh, systemd-networkd and systemd-resolved are enabled by default. Use .preset files according to `man systemd.preset` and other official systemd documentation. Generally speaking, symbolic links are created and deleted by systemctl commands based on configuration files. So it's not clear whether the previous symbolic link creation really worked and when (most services were likely already enabled anyway in most configurations). Signed-off-by: Marc Herbert <marc.herbert@intel.com>
marc-hb
force-pushed
the
autorun-fixes
branch
2 times, most recently
from
2ca0424 to
2d3aa2c
Compare
- Stop messing with symbolic links and use systemd preset instead. Since Fedora 37, `systemctl preset-all` is run on the first boot (which is every boot unless run_qemu.sh --rw is specified). https://fedoraproject.org/wiki/Changes/Preset_All_Systemd_Units_on_First_Boot This destroys and reconfigures all symbolic links based on .preset files. So, switch to .preset files. Other recent distributions have most likely adopted the same method, see `man systemd-firstboot` etc. Even for the distributions that have not, mkosi v15 commit 84ec58cca8e3c also runs `systemctl preset-all` at build time anyway - whichever the distribution is. - Also drop "AllowIsolate" which is discouraged probably never made sense. - Simplify and fix the target dependency graph Signed-off-by: Marc Herbert <marc.herbert@intel.com>
Fail fast and print an error message instead. There's probably a dependency missing between --options but that's a different question. Signed-off-by: Marc Herbert <marc.herbert@intel.com>
We already power off in trap EXIT, no need to repeat it. Signed-off-by: Marc Herbert <marc.herbert@intel.com>
According to Dave Jiang, cxl_test skips over the hardware registers. Maybe it did not in 2021 when these lines were added to git? Signed-off-by: Marc Herbert <marc.herbert@intel.com>
|
Successfully tested |
marc-hb
force-pushed
the
autorun-fixes
branch
from
2d3aa2c to
04cf33c
Compare
|
Unrelated "tune2fs" error #47 on Ubuntu 22.04 in https://github.com/pmem/run_qemu/actions/runs/12824790402/job/35761549704?pr=101. Re-running. |
| @@ -45,4 +45,3 @@ dumpfile /root/ndctl/build/meson-logs/testlog.txt | |||
| echo "======= meson-test.log ========" > /dev/kmsg | |||
| dumpfile "$logfile" | |||
| echo "======= Done $0 ========" > /dev/kmsg | |||
| systemctl poweroff | |||
There was a problem hiding this comment.
I think this was needed to actually shutdown the guest fully and exit the qemu process (when I last ran/tested this) - without it, even after a successful completion of all the tests, we'd end up waiting for the timeout to kill it and exit. Does that not happen anymore?
There was a problem hiding this comment.
It does not happen because there is (more recently?) a trap EXIT, check the commit message.
There was a problem hiding this comment.
Ah I see - makes sense, the rest of it looks good.
There was a problem hiding this comment.
(more recently?)
Impossible to tell from the git log because both poweroff commands came in the same, very first commit.
Anyway I tested this multiple times and a single poweroff command is enough.
I found that because sometimes you want to LOCALLY comment it out for temporary testing and doing this twice was not convenient :-)
stellarhopper
approved these changes
Jan 21, 2025
marc-hb
added a commit
to marc-hb/run_qemu
that referenced
this pull request
Jan 23, 2025
QEMU devices are not required to run cxl tests, so let the user decide what they want. Autorun has very recently been fixed (commit 17d0473 and others in PR pmem#101) and had been broken for likely 1 or 2 years which means no one was using it which means it's a great time to alter the behavior. Signed-off-by: Marc Herbert <marc.herbert@intel.com>
stellarhopper
pushed a commit
that referenced
this pull request
Jan 24, 2025
QEMU devices are not required to run cxl tests, so let the user decide what they want. Autorun has very recently been fixed (commit 17d0473 and others in PR #101) and had been broken for likely 1 or 2 years which means no one was using it which means it's a great time to alter the behavior. Signed-off-by: Marc Herbert <marc.herbert@intel.com>
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.
Since Fedora 37,
systemctl preset-allis run on the first boot (whichis every boot unless run_qemu.sh --rw is specified).
https://fedoraproject.org/wiki/Changes/Preset_All_Systemd_Units_on_First_Boot
This destroys and reconfigures all symbolic links based on .preset
files. So, switch to .preset files.
Other recent distributions have most likely adopted the same method, see
man systemd-firstbootetc.Even for the distributions that have not, mkosi v15 commit 84ec58cca8e3c
also runs
systemctl preset-allat build time anyway - whichever thedistribution is.
Also drop "AllowIsolate" which is discouraged probably never made
sense.
Simplify and fix the target dependency graph