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

Jump to bottom

zed.d/pool_import-led.sh fallback path terminally broken #11934

Closed
nabijaczleweli opened this issue Apr 23, 2021 · 1 comment
Closed

zed.d/pool_import-led.sh fallback path terminally broken #11934

nabijaczleweli opened this issue Apr 23, 2021 · 1 comment
Labels
Status: Triage Needed New issue which needs to be triaged Type: Defect Incorrect behavior (e.g. crash, hang)

Comments

@nabijaczleweli
Copy link
Contributor

Describe the problem you're observing

See cmd/zed/zed.d/zed.d/pool_import-led.sh:

	cmd='echo led_token=$(cat "$VDEV_ENC_SYSFS_PATH/fault"),"$VDEV_ENC_SYSFS_PATH",'
	out=$($ZPOOL status -vc "$cmd" "$pool" | grep 'led_token=')

This will simply never work: -c takes script names nowadays, and silently ignores this argument (if only because it has a slash in it).

Describe how to reproduce the problem

This appears to date back to the original implementation by @tonyhutter in b291029 (Fri Feb 10 16:09:45 2017 -0800), and the commands used to be run using popen(3), which passes them through /bin/sh -c. This has obviously changed, but the ZEDLET hasn't, and, according to the comment at the top, is now broken for pool_import events, which are the only ones that hit this fallback.
@nabijaczleweli nabijaczleweli added Status: Triage Needed New issue which needs to be triaged Type: Defect Incorrect behavior (e.g. crash, hang) labels Apr 23, 2021
@nabijaczleweli
Copy link
Contributor Author

Oh, I missed the $ZPOOL_SCRIPTS_PATH bit in the manpage; should be easy enough to produce a patch, then

nabijaczleweli added a commit to nabijaczleweli/zfs that referenced this issue Apr 23, 2021
@nabijaczleweli
zed.d/pool_import-led.sh: fix for current zpool scripts
95e3898 
This requires an executable /tmp, but oh well

Also minor clean-up with folding state_to_val() into a case and
unrolling the lesser-available seq into numbers

Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#11934
@nabijaczleweli nabijaczleweli mentioned this issue Apr 23, 2021
Closed
13 tasks
nabijaczleweli added a commit to nabijaczleweli/zfs that referenced this issue Apr 24, 2021
@nabijaczleweli
zed.d/pool_import-led.sh: fix for current zpool scripts
5f89f4e 
This requires an executable /tmp, but oh well

Also minor clean-up with folding state_to_val() into a case and
unrolling the lesser-available seq into numbers

Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#11934
nabijaczleweli added a commit to nabijaczleweli/zfs that referenced this issue Apr 24, 2021
@nabijaczleweli
zed.d/pool_import-led.sh: fix for current zpool scripts
64635da 
This requires an executable /tmp, but oh well

Also minor clean-up with folding state_to_val() into a case and
unrolling the lesser-available seq into numbers

Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#11934
nabijaczleweli added a commit to nabijaczleweli/zfs that referenced this issue Apr 24, 2021
@nabijaczleweli
zed.d/pool_import-led.sh: fix for current zpool scripts
e5207de 
This requires an executable /tmp, but oh well

Also minor clean-up with folding state_to_val() into a case,
unrolling the lesser-available seq into numbers,
and documentation comments

Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#11934
nabijaczleweli added a commit to nabijaczleweli/zfs that referenced this issue Apr 26, 2021
@nabijaczleweli
zed.d/pool_import-led.sh: fix for current zpool scripts
11902ed 
This requires an executable /tmp, but oh well

Also minor clean-up with folding state_to_val() into a case,
unrolling the lesser-available seq into numbers,
and documentation comments

Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#11934
nabijaczleweli added a commit to nabijaczleweli/zfs that referenced this issue Apr 26, 2021
@nabijaczleweli
zed.d/pool_import-led.sh: fix for current zpool scripts
9db7bfd 
This requires an executable /tmp, but oh well

Also minor clean-up with folding state_to_val() into a case,
unrolling the lesser-available seq into numbers,
ignoring vdev states we don't care about,
and documentation comments

Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#11934
nabijaczleweli added a commit to nabijaczleweli/zfs that referenced this issue Apr 26, 2021
@nabijaczleweli
zed.d/pool_import-led.sh: fix for current zpool scripts
b97b26c 
This requires an executable /tmp, but oh well

Also minor clean-up with folding state_to_val() into a case,
unrolling the lesser-available seq into numbers,
ignoring vdev states we don't care about,
and documentation comments

Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#11934
tonyhutter added a commit to tonyhutter/zfs that referenced this issue Apr 28, 2021
@tonyhutter
zed: Remove dead code from statechange-led.sh
f1fbc38 
statechange-led.sh calls process_pool() if either $ZEVENT_VDEV_ENC_SYSFS_PATH
or $ZEVENT_VDEV_STATE_STR are not set by zed.  That should never happen,
and process_pool() will never get called.  Furthermore, process_pool() is
totally broken (openzfs#11934).  This patch removes process_pool().

Fixes: openzfs#11934
Signed-off-by: Tony Hutter <[email protected]>
tonyhutter added a commit to tonyhutter/zfs that referenced this issue Apr 28, 2021
@tonyhutter
zed: Remove dead code from statechange-led.sh
75e1cf7 
statechange-led.sh calls process_pool() if either $ZEVENT_VDEV_ENC_SYSFS_PATH
or $ZEVENT_VDEV_STATE_STR are not set by zed.  That should never happen,
and thus process_pool() will never get called.  Furthermore, process_pool() is
totally broken (openzfs#11934).  This patch removes process_pool().

Fixes: openzfs#11934
Signed-off-by: Tony Hutter <[email protected]>
@tonyhutter tonyhutter mentioned this issue Apr 28, 2021
Closed
13 tasks
nabijaczleweli added a commit to nabijaczleweli/zfs that referenced this issue Apr 29, 2021
@nabijaczleweli
zed.d/pool_import-led.sh: fix for current zpool scripts
985372e 
Also minor clean-up with folding state_to_val() into a case,
unrolling the lesser-available seq into numbers,
ignoring vdev states we don't care about,
and documentation comments

Suggested-by: Tony Hutter <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#11934
nabijaczleweli added a commit to nabijaczleweli/zfs that referenced this issue Apr 29, 2021
@nabijaczleweli
zed.d/pool_import-led.sh: fix for current zpool scripts
f2c943a 
Also minor clean-up with folding state_to_val() into a case,
unrolling the lesser-available seq into numbers,
ignoring vdev states we don't care about,
and documentation comments

Suggested-by: Tony Hutter <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#11934
nabijaczleweli added a commit to nabijaczleweli/zfs that referenced this issue Apr 29, 2021
@nabijaczleweli
zed.d/pool_import-led.sh: fix for current zpool scripts
f8dfe14 
Also minor clean-up with folding state_to_val() into a case,
unrolling the lesser-available seq into numbers,
ignoring vdev states we don't care about,
and documentation comments

Suggested-by: Tony Hutter <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#11934
nabijaczleweli added a commit to nabijaczleweli/zfs that referenced this issue Apr 29, 2021
@nabijaczleweli
zed.d/pool_import-led.sh: fix for current zpool scripts
d2d7f3c 
Also minor clean-up with folding state_to_val() into a case,
unrolling the lesser-available seq into numbers,
ignoring vdev states we don't care about,
and documentation comments

Suggested-by: Tony Hutter <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#11934
nabijaczleweli added a commit to nabijaczleweli/zfs that referenced this issue Apr 29, 2021
@nabijaczleweli
zed.d/pool_import-led.sh: fix for current zpool scripts
153336d 
Also minor clean-up with folding state_to_val() into a case,
unrolling the lesser-available seq into numbers,
ignoring vdev states we don't care about,
and documentation comments

Signed-off-by: Tony Hutter <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#11934
nabijaczleweli added a commit to nabijaczleweli/zfs that referenced this issue Apr 29, 2021
@nabijaczleweli
zed.d/pool_import-led.sh: fix for current zpool scripts
46b243a 
Also minor clean-up with folding state_to_val() into a case,
unrolling the lesser-available seq into numbers,
ignoring vdev states we don't care about,
and documentation comments

Signed-off-by: Tony Hutter <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#11934
behlendorf pushed a commit to behlendorf/zfs that referenced this issue May 10, 2021
@nabijaczleweli @behlendorf
zed.d/pool_import-led.sh: fix for current zpool scripts
9c5256a 
Also minor clean-up with folding state_to_val() into a case,
unrolling the lesser-available seq into numbers,
ignoring vdev states we don't care about,
and documentation comments

Signed-off-by: Tony Hutter <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#11934
Closes openzfs#11935
sempervictus pushed a commit to sempervictus/zfs that referenced this issue May 31, 2021
@nabijaczleweli
zed.d/pool_import-led.sh: fix for current zpool scripts
c16a50d 
Also minor clean-up with folding state_to_val() into a case,
unrolling the lesser-available seq into numbers,
ignoring vdev states we don't care about,
and documentation comments

Signed-off-by: Tony Hutter <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#11934
Closes openzfs#11935
tonyhutter pushed a commit to tonyhutter/zfs that referenced this issue Nov 10, 2021
@nabijaczleweli @tonyhutter
zed.d/pool_import-led.sh: fix for current zpool scripts
370c931 
Also minor clean-up with folding state_to_val() into a case,
unrolling the lesser-available seq into numbers,
ignoring vdev states we don't care about,
and documentation comments

Signed-off-by: Tony Hutter <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#11934
Closes openzfs#11935
tonyhutter pushed a commit to tonyhutter/zfs that referenced this issue Nov 13, 2021
@nabijaczleweli @tonyhutter
zed.d/pool_import-led.sh: fix for current zpool scripts
a908830 
Also minor clean-up with folding state_to_val() into a case,
unrolling the lesser-available seq into numbers,
ignoring vdev states we don't care about,
and documentation comments

Signed-off-by: Tony Hutter <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#11934
Closes openzfs#11935
tonyhutter pushed a commit to tonyhutter/zfs that referenced this issue Nov 13, 2021
@nabijaczleweli @tonyhutter
zed.d/pool_import-led.sh: fix for current zpool scripts
5863581 
Also minor clean-up with folding state_to_val() into a case,
unrolling the lesser-available seq into numbers,
ignoring vdev states we don't care about,
and documentation comments

Signed-off-by: Tony Hutter <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#11934
Closes openzfs#11935
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Status: Triage Needed New issue which needs to be triaged Type: Defect Incorrect behavior (e.g. crash, hang)
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

zed: Remove dead code from statechange-led.sh tonyhutter/zfs
1 participant
@nabijaczleweli