Skip to content

feat: support loading cdrom modules before reading blkid info #6364

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.

Sign up for GitHub

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

Closed
smoser wants to merge 1 commit into from
Closed

feat: support loading cdrom modules before reading blkid info #6364

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 51 additions & 0 deletions tools/ds-identify
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,11 @@ DI_ON_NOTFOUND=""

DI_EC2_STRICT_ID_DEFAULT="true"

STATE_CDROM_PROBED=""
STATE_FLOPPY_PROBED=""

# PROBE_CDROM_MODULES - modules will be modprobed if /dev/cdrom does not exist.
PROBE_CDROM_MODULES=""
Copy link
Copy Markdown
Member

@holmanb holmanb Aug 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do you plan to set this environment variable?

_IS_IBM_CLOUD=""

error() {
Expand Down Expand Up @@ -304,6 +309,7 @@ ensure_sane_path() {
blkid_export() {
# call 'blkid -c /dev/null export', set DI_BLKID_EXPORT_OUT
cached "$DI_BLKID_EXPORT_OUT" && return 0
probe_cdrom
local out="" ret=0
out=$(blkid -c /dev/null -o export) && DI_BLKID_EXPORT_OUT="$out" || {
ret=$?
Expand Down Expand Up @@ -763,6 +769,51 @@ check_writable_seed_dir() {
check_seed_dir "$@"
}

debugts() {
local lvl="$1"
shift
[ "$lvl" -gt "${DI_DEBUG_LEVEL}" ] && return
read_uptime
debug "$lvl" "[up ${_RET}s]" "$@"
}

probe_cdrom() {
cached "${STATE_CDROM_PROBED}" && return "${STATE_CDROM_PROBED}"
if [ -z "$PROBE_CDROM_MODULES" ]; then
STATE_CDROM_PROBED=1
return 0
fi

local fpath=/dev/cdrom
if [ -b "$fpath" ]; then
STATE_CDROM_PROBED=1;
debug 1 "$fpath existed before probe_cdrom"
return 0;
fi

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:$?"
Copy link
Copy Markdown
Member

@holmanb holmanb Aug 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Systemd generators shouldn't require modifying system state.

done
[ -z "$modfails" ] ||
debug 1 "probe_cdrom modprobe had unsuccessful attempts: $modfails"

# 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
Copy link
Copy Markdown
Member

@holmanb holmanb Aug 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

debugts 1 "udevadm settle returned $?"
return 1
fi
debugts 1 "udev settled for $fpath"
fi

[ -b "$fpath" ]
STATE_CDROM_PROBED=$?
return "${STATE_CDROM_PROBED}"
}

probe_floppy() {
cached "${STATE_FLOPPY_PROBED}" && return "${STATE_FLOPPY_PROBED}"
local fpath=/dev/floppy
Expand Down