Skip to content
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

Always use NVMe datadisk on Yellow if it's present on first boot #3686

Merged
merged 1 commit into from
Nov 21, 2024
Merged

Always use NVMe datadisk on Yellow if it's present on first boot #3686

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
63 changes: 51 additions & 12 deletions buildroot-external/rootfs-overlay/usr/libexec/haos-data-disk-detach
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#!/bin/sh
# shellcheck disable=SC1091

# Find root using rdev command
rootpart=$(rdev | cut -f 1 -d ' ')
Expand All @@ -9,15 +10,53 @@ sleep 10s

datapartitions=$(blkid --match-token LABEL="hassos-data" --output device)

for datapart in ${datapartitions}
do
datadev=$(lsblk -no pkname "${datapart}")

# If major does not match our root device major, it is an external data
# disk. Rename to make sure it gets ignored.
if [ "$rootdev" != "$datadev" ]
then
echo "Found external data disk device on ${datapart}, mark it disabled..."
e2label "${datapart}" hassos-data-dis
fi
done
. /etc/os-release

disable_data_partition() {
e2label "${1}" hassos-data-dis
}
Comment on lines +15 to +17
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add error handling in disable_data_partition function

The disable_data_partition function does not handle possible errors from the e2label command. If e2label fails (e.g., due to a corrupted filesystem or permission issues), the script will continue without disabling the data partition, which could lead to inconsistent system states.

Consider adding error handling to check the exit status of e2label and take appropriate action if it fails. Apply this diff to enhance the function:

 disable_data_partition() {
     e2label "${1}" hassos-data-dis
+    if [ $? -ne 0 ]; then
+        echo "Error: Failed to disable data partition ${1}" >&2
+        # Optionally, exit or handle the error as needed
+    fi
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
disable_data_partition() {
e2label "${1}" hassos-data-dis
}
disable_data_partition() {
e2label "${1}" hassos-data-dis
if [ $? -ne 0 ]; then
echo "Error: Failed to disable data partition ${1}" >&2
# Optionally, exit or handle the error as needed
fi
}


if [ "$VARIANT_ID" = "yellow" ]; then
emmc_data_partition=""
nvme_data_partition=""

for datapart in ${datapartitions}; do
datadev=$(lsblk -no pkname "${datapart}")

case "${datadev}" in
mmc*)
# Data partition on internal eMMC
if [ "$rootdev" = "$datadev" ]; then
emmc_data_partition="${datapart}"
fi
;;
nvme0*)
# Data partition on first NVMe disk
nvme_data_partition="${datapart}"
;;
*)
# Disable all other data disks as normally
if [ "$rootdev" != "$datadev" ]; then
echo "Found extra external data disk device on ${datapart}, marking it disabled..."
disable_data_partition "${datapart}"
fi
;;
esac
done

if [ -n "${emmc_data_partition}" ] && [ -n "${nvme_data_partition}" ]; then
echo "Found both eMMC and NVMe data disk devices, marking eMMC as disabled"
disable_data_partition "${emmc_data_partition}"
fi
else
for datapart in ${datapartitions}; do
datadev=$(lsblk -no pkname "${datapart}")

# If major does not match our root device major, it is an external data
# disk. Rename to make sure it gets ignored.
if [ "$rootdev" != "$datadev" ]; then
echo "Found external data disk device on ${datapart}, marking it disabled..."
disable_data_partition "${datapart}"
fi
done
fi