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.

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

zed.d/pool_import-led.sh: fix for current zpool scripts (and a few bonus touchups) #11935

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion cmd/zed/zed.d/data-notify.sh
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ zed_rate_limit "${rate_limit_tag}" || exit 3

umask 077
note_subject="ZFS ${ZEVENT_SUBCLASS} error for ${ZEVENT_POOL} on $(hostname)"
note_pathname="${TMPDIR:="/tmp"}/$(basename -- "$0").${ZEVENT_EID}.$$"
note_pathname="$(mktemp)"
behlendorf marked this conversation as resolved.
Show resolved Hide resolved
{
echo "ZFS has detected a data error:"
echo
Expand Down
2 changes: 1 addition & 1 deletion cmd/zed/zed.d/generic-notify.sh
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ umask 077
pool_str="${ZEVENT_POOL:+" for ${ZEVENT_POOL}"}"
host_str=" on $(hostname)"
note_subject="ZFS ${ZEVENT_SUBCLASS} event${pool_str}${host_str}"
note_pathname="${TMPDIR:="/tmp"}/$(basename -- "$0").${ZEVENT_EID}.$$"
note_pathname="$(mktemp)"
{
echo "ZFS has posted the following event:"
echo
Expand Down
4 changes: 2 additions & 2 deletions cmd/zed/zed.d/history_event-zfs-list-cacher.sh.in
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@ FSLIST="${FSLIST_DIR}/${ZEVENT_POOL}"
. "${ZED_ZEDLET_DIR}/zed-functions.sh"

[ "$ZEVENT_SUBCLASS" != "history_event" ] && exit 0
zed_check_cmd "${ZFS}" sort diff grep
zed_check_cmd "${ZFS}" sort diff

# If we are acting on a snapshot, we have nothing to do
printf '%s' "${ZEVENT_HISTORY_DSNAME}" | grep '@' && exit 0
[ "${ZEVENT_HISTORY_DSNAME%@*}" = "${ZEVENT_HISTORY_DSNAME}" ] || exit 0

# We obtain a lock on zfs-list to avoid any simultaneous writes.
# If we run into trouble, log and drop the lock
Expand Down
2 changes: 1 addition & 1 deletion cmd/zed/zed.d/scrub_finish-notify.sh
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ fi

umask 077
note_subject="ZFS ${ZEVENT_SUBCLASS} event for ${ZEVENT_POOL} on $(hostname)"
note_pathname="${TMPDIR:="/tmp"}/$(basename -- "$0").${ZEVENT_EID}.$$"
note_pathname="$(mktemp)"
{
echo "ZFS has finished a ${action}:"
echo
Expand Down
90 changes: 45 additions & 45 deletions cmd/zed/zed.d/statechange-led.sh
Original file line number Diff line number Diff line change
@@ -1,21 +1,21 @@
#!/bin/sh
#
# Turn off/on the VDEV's enclosure fault LEDs when the pool's state changes.
# Turn off/on vdevs' enclosure fault LEDs when their pool's state changes.
#
# Turn the VDEV's fault LED on if it becomes FAULTED, DEGRADED or UNAVAIL.
# Turn the LED off when it's back ONLINE again.
# Turn a vdev's fault LED on if it becomes FAULTED, DEGRADED or UNAVAIL.
# Turn its LED off when it's back ONLINE again.
Comment on lines +5 to +6
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the point of this change?

Copy link
Contributor Author

@nabijaczleweli nabijaczleweli Apr 29, 2021

Choose a reason for hiding this comment

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

This can manipulate many fault LEDs at a time and describes a general rule, also making terminology consistent with the rest

#
# This script run in two basic modes:
#
# 1. If $ZEVENT_VDEV_ENC_SYSFS_PATH and $ZEVENT_VDEV_STATE_STR are set, then
# only set the LED for that particular VDEV. This is the case for statechange
# only set the LED for that particular vdev. This is the case for statechange
# events and some vdev_* events.
#
# 2. If those vars are not set, then check the state of all VDEVs in the pool
# 2. If those vars are not set, then check the state of all vdevs in the pool
# and set the LEDs accordingly. This is the case for pool_import events.
#
# Note that this script requires that your enclosure be supported by the
# Linux SCSI enclosure services (ses) driver. The script will do nothing
# Linux SCSI Enclosure services (SES) driver. The script will do nothing
# if you have no enclosure, or if your enclosure isn't supported.
#
# Exit codes:
Expand Down Expand Up @@ -59,18 +59,22 @@ check_and_set_led()
file="$1"
val="$2"

if [ -z "$val" ]; then
return 0
fi

if [ ! -e "$file" ] ; then
return 3
fi

# If another process is accessing the LED when we attempt to update it,
# the update will be lost so retry until the LED actually changes or we
# timeout.
for _ in $(seq 1 5); do
for _ in 1 2 3 4 5; do
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the point of this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As noted: seq is a very spotty GNU extension (and also not checked for at the top), so when not available this resolves to for _ in ; do which I assume you can guess why is less than optimal. (Plus, it's literally five numbers, do you really need to spawn a process for it?)

Copy link
Contributor

Choose a reason for hiding this comment

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

Has checkbashisms said that?

Copy link
Contributor Author

@nabijaczleweli nabijaczleweli Apr 30, 2021

Choose a reason for hiding this comment

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

Hm, I've misremembered apparently, according to FreeBSD's seq(1) it's had a tumultuous history (as ported from GNU shellutils and/or P9) and should be available on most recent fully-configured UNIXes, but I've also definitely used systems without it in recent years.
Plus, if the portability to small systems doesn't appeal to you, then the performance gains of not using a process to generate 5 numbers that are, in sum, shorter than the process substitution definitely will.

and dunno, haven't run checkbashisms, but it probably won't

# We want to check the current state first, since writing to the
# 'fault' entry always causes a SES command, even if the
# current state is already what you want.
current=$(cat "${file}")
read -r current < "${file}"

# On some enclosures if you write 1 to fault, and read it back,
# it will return 2. Treat all non-zero values as 1 for
Expand All @@ -85,47 +89,52 @@ check_and_set_led()
else
break
fi
done
done
}

state_to_val()
{
state="$1"
if [ "$state" = "FAULTED" ] || [ "$state" = "DEGRADED" ] || \
[ "$state" = "UNAVAIL" ] ; then
echo 1
elif [ "$state" = "ONLINE" ] ; then
echo 0
fi
case "$state" in
FAULTED|DEGRADED|UNAVAIL)
echo 1
;;
ONLINE)
behlendorf marked this conversation as resolved.
Show resolved Hide resolved
echo 0
;;
esac
}

# process_pool ([pool])
# process_pool (pool)
#
# Iterate through a pool (or pools) and set the VDEV's enclosure slot LEDs to
# the VDEV's state.
# Iterate through a pool and set the vdevs' enclosure slot LEDs to
# those vdevs' state.
#
# Arguments
# pool: Optional pool name. If not specified, iterate though all pools.
# pool: Pool name.
#
# Return
# 0 on success, 3 on missing sysfs path
#
process_pool()
{
pool="$1"
rc=0

# Lookup all the current LED values and paths in parallel
#shellcheck disable=SC2016
cmd='echo led_token=$(cat "$VDEV_ENC_SYSFS_PATH/fault"),"$VDEV_ENC_SYSFS_PATH",'
out=$($ZPOOL status -vc "$cmd" "$pool" | grep 'led_token=')

#shellcheck disable=SC2034
echo "$out" | while read -r vdev state read write chksum therest; do
# The output will be the vdevs only (from "grep '/dev/'"):
#
# U45 ONLINE 0 0 0 /dev/sdk 0
# U46 ONLINE 0 0 0 /dev/sdm 0
# U47 ONLINE 0 0 0 /dev/sdn 0
# U50 ONLINE 0 0 0 /dev/sdbn 0
#
ZPOOL_SCRIPTS_AS_ROOT=1 $ZPOOL status -c upath,fault_led "$pool" | grep '/dev/' | (
rc=0
while read -r vdev state _ _ _ therest; do
# Read out current LED value and path
tmp=$(echo "$therest" | sed 's/^.*led_token=//g')
vdev_enc_sysfs_path=$(echo "$tmp" | awk -F ',' '{print $2}')
current_val=$(echo "$tmp" | awk -F ',' '{print $1}')
# Get dev name (like 'sda')
dev=$(basename "$(echo "$therest" | awk '{print $(NF-1)}')")
vdev_enc_sysfs_path=$(realpath "/sys/class/block/$dev/device/enclosure_device"*)
current_val=$(echo "$therest" | awk '{print $NF}')

if [ "$current_val" != "0" ] ; then
current_val=1
Expand All @@ -137,36 +146,27 @@ process_pool()
fi

if [ ! -e "$vdev_enc_sysfs_path/fault" ] ; then
#shellcheck disable=SC2030
rc=1
rc=3
zed_log_msg "vdev $vdev '$file/fault' doesn't exist"
continue;
continue
fi

val=$(state_to_val "$state")

if [ "$current_val" = "$val" ] ; then
# LED is already set correctly
continue;
continue
fi

if ! check_and_set_led "$vdev_enc_sysfs_path/fault" "$val"; then
rc=1
rc=3
fi

done

#shellcheck disable=SC2031
if [ "$rc" = "0" ] ; then
return 0
else
# We didn't see a sysfs entry that we wanted to set
return 3
fi
exit "$rc"; )
}

if [ -n "$ZEVENT_VDEV_ENC_SYSFS_PATH" ] && [ -n "$ZEVENT_VDEV_STATE_STR" ] ; then
# Got a statechange for an individual VDEV
# Got a statechange for an individual vdev
val=$(state_to_val "$ZEVENT_VDEV_STATE_STR")
vdev=$(basename "$ZEVENT_VDEV_PATH")
check_and_set_led "$ZEVENT_VDEV_ENC_SYSFS_PATH/fault" "$val"
Expand Down
2 changes: 1 addition & 1 deletion cmd/zed/zed.d/statechange-notify.sh
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ fi

umask 077
note_subject="ZFS device fault for pool ${ZEVENT_POOL_GUID} on $(hostname)"
note_pathname="${TMPDIR:="/tmp"}/$(basename -- "$0").${ZEVENT_EID}.$$"
note_pathname="$(mktemp)"
{
if [ "${ZEVENT_VDEV_STATE_STR}" = "FAULTED" ] ; then
echo "The number of I/O errors associated with a ZFS device exceeded"
Expand Down
2 changes: 1 addition & 1 deletion cmd/zed/zed.d/trim_finish-notify.sh
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ zed_check_cmd "${ZPOOL}" || exit 9

umask 077
note_subject="ZFS ${ZEVENT_SUBCLASS} event for ${ZEVENT_POOL} on $(hostname)"
note_pathname="${TMPDIR:="/tmp"}/$(basename -- "$0").${ZEVENT_EID}.$$"
note_pathname="$(mktemp)"
{
echo "ZFS has finished a trim:"
echo
Expand Down
8 changes: 3 additions & 5 deletions cmd/zed/zed.d/zed-functions.sh
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ zed_notify_pushbullet()
#
# Notification via Slack Webhook <https://api.slack.com/incoming-webhooks>.
# The Webhook URL (ZED_SLACK_WEBHOOK_URL) identifies this client to the
# Slack channel.
# Slack channel.
#
# Requires awk, curl, and sed executables to be installed in the standard PATH.
#
Expand Down Expand Up @@ -511,10 +511,8 @@ zed_guid_to_pool()
return
fi

guid=$(printf "%llu" "$1")
if [ -n "$guid" ] ; then
$ZPOOL get -H -ovalue,name guid | awk '$1=='"$guid"' {print $2}'
fi
guid="$(printf "%u" "$1")"
$ZPOOL get -H -ovalue,name guid | awk '$1 == '"$guid"' {print $2; exit}'
}

# zed_exit_if_ignoring_this_event
Expand Down
2 changes: 1 addition & 1 deletion lib/libzutil/os/linux/zutil_device_path_os.c
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,7 @@ dm_get_underlying_path(const char *dm_name)
free(tmp);
free(realp);

if (!path) {
if (!path && first_path) {
/*
* None of the underlying paths had a link back to their
* enclosure devices. Throw up out hands and return the first
Expand Down