run_qemu.sh: print warning that --ndctl-build never worked incrementally #76
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
The update_existing_rootfs() function is part of a hidden shortcut that does not run mkosi. It had a tentative feature that wanted to invoke rsync to update `ndctl` sources in the existing image. That ndctl shortcut never actually did anything! - Replace that broken rsync with a warning in case anyone thought this was working. - Do not print the warning when `$ndctl` is undefined or ill-defined because --ndctl-build is ON by default and `$ndctl` is in practice used as the actual ndctl "switch". 1. This feature apparently _never worked_ and always left outdated sources _silently_. That's because the mount point requires root access, which made the `test [ -d qbuild/mnt/root/ndctl ]` always return false - - without any message! 2. Even if that `rsync` had worked (with the appropriate "sudose"), sources would still have not been _compiled_; there was no attempt to compile. This would have left binaries out of date with sources inside the image, been inconsistent with building from scratch and would have been very confusing and dangerous too. Compiling and installing requires the container/chroot which is currently available "for free" when building from scratch. But it is not straight-forward in the shortcut case. Speaking of containers, support for mkosi v15 and above in the other, main ndctl deployment code path (from scratch or with `-r img`) requires container-related changes in mkosi.postinst. See v15 commit systemd/mkosi@9b626c647037bc8a). These unrelated and upcoming changes will hopefully make it easier to finally implement this feature. Signed-off-by: Marc Herbert <marc.herbert@intel.com>
There are some allusions to this incremental build in the --help text but it's very indirect. People who just run the same command consecutively and don't pay attention to the logs will not notice the difference between a build from a scratch and incremental. I mean, they will see that it's much faster and that some caching is happening but mkosi does a lot of caching too, so who knows? And who cares. Caching should be functionally the same - otherwise it's a bug. Hence the warning. This warning would have saved my former self about 0.5 day of trial and error. |
stellarhopper
approved these changes
Dec 11, 2024
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.
The update_existing_rootfs() function is part of a hidden shortcut that does not run mkosi. It had a tentative feature that wanted to invoke rsync to update
ndctlsources in the existing image. That ndctl shortcut never actually did anything!$ndctlis undefined or ill-defined because --ndctl-build is ON by default and$ndctlis in practice used as the actual ndctl "switch".This feature apparently never worked and always left outdated sources silently. That's because the mount point requires root access, which made the
test [ -d qbuild/mnt/root/ndctl ]always return false - without any message!Even if that
rsynchad worked (with the appropriate "sudose"), sources would still have not been compiled; there was no attempt to compile. This would have left binaries out of date with sources inside the image, been inconsistent with building from scratch and would have been very confusing and dangerous too.Compiling and installing requires the container/chroot which is currently available "for free" when building from scratch. But it is not straight-forward in the shortcut case.
Speaking of containers, support for mkosi v15 and above in the other, main ndctl deployment code path (from scratch or with
-r img) requires container-related changes in mkosi.postinst. See v15 commit systemd/mkosi@9b626c647037bc8a). These unrelated and upcoming changes will hopefully make it easier to finally implement this feature.