feat: support loading cdrom modules before reading blkid info#6364
feat: support loading cdrom modules before reading blkid info#6364smoser wants to merge 1 commit into
Conversation
If scsi cdrom module is not built in, then cdrom device might not be present before blkid is run. The result would be that cdrom based datasources (ConfigDrive, NoCloud, OVF) may not be found. Add support for local setting of modification of PROBE_CDROM_MODULES varible. Each module in this list will be modprobed and then udevadm will be settled afterwards. Other things here: 1. fix for probe_floppy path that would fail with unset variable. 2. add debugts function for debug with timestamp. Upstream PR canonical#6364
smoser
force-pushed
the
fix/load-cdrom-modules-before-blkid
branch
2 times, most recently
from
5070e1e to
aa9cca4
Compare
| STATE_FLOPPY_PROBED="" | ||
|
|
||
| # PROBE_CDROM_MODULES - modules will be modprobed if /dev/cdrom does not exist. | ||
| PROBE_CDROM_MODULES="" |
There was a problem hiding this comment.
How do you plan to set this environment variable?
holmanb
left a comment
holmanb
left a comment
There was a problem hiding this comment.
I'm curious: why isn't a more standard approach such as initrd / initramfs or a kernel command line argument preferred? The proposed changes do not fit the expectations of systemd generators (see comments).
If scsi cdrom module is not built in, then cdrom device might not be present before blkid is run.
Which distro is this seen this on?
| debugts 1 "attempting modprobe of ${PROBE_CDROM_MODULES}" | ||
| local m="" modfails="" | ||
| for m in ${PROBE_CDROM_MODULES}; do | ||
| modprobe -b "$m" >/dev/null 2>&1 || modfails="${modfails:+$modfails }$m:$?" |
There was a problem hiding this comment.
Systemd generators shouldn't require modifying system state.
| # Some Linux distros/non-Linux OSes may not have udev | ||
| if command -v udevadm >/dev/null 2>&1; then | ||
| debugts 1 "udev settling for $fpath" | ||
| if ! udevadm settle "--exit-if-exists=$fpath"; then |
There was a problem hiding this comment.
This solution looks incorrect. To my knowledge the kernel makes no guarantees that loading a module will cause an associated uevent to be queued before the driver is done loading. This might work sometimes, but I don't think that there are any guarantees that it will always work. Am I missing something?
Also, generators are supposed to be fast and this operation is functionally a wait.
|
Hello! Thank you for this proposed change to cloud-init. This pull request is now marked as stale as it has not seen any activity in 14 days. If no activity occurs within the next 7 days, this pull request will automatically close. If you are waiting for code review and you are seeing this message, apologies! Please reply, tagging TheRealFalcon, and he will ensure that someone takes a look soon. (If the pull request is closed and you would like to continue working on it, please do tag TheRealFalcon to reopen it.) |
github-actions
Bot
added
the
stale-pr
2 participants
If scsi cdrom module is not built in, then cdrom device might not be present before blkid is run. The result would be that cdrom based datasources (ConfigDrive, NoCloud, OVF) may not be found.
Add support for local setting of modification of PROBE_CDROM_MODULES varible. Each module in this list will be modprobed and then udevadm will be settled afterwards.
Other things here:
--
tests/unittests/cloudinit/example.pyshould be tested bytests/unittests/test_example.pytox -e py3tox -e docs.Proposed Commit Message
Additional Context
Test Steps
Merge type