-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Added auto_online_001_pos test as apart of ZED/FMA tests in the ZTS #5774
Conversation
@dpquigl, thanks for your PR! By analyzing the history of the files in this pull request, we identified @behlendorf, @loli10K and @ahrens to be potential reviewers. |
if [[ -z $proc_name ]]; then | ||
echo '- - -' > $host/scan | ||
else | ||
NAME=`cat $host/proc_name` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: It's preferable to use $() instead of ``.
# First checks state of disk. Test will fail if disk is not properly onlined | ||
# or offlined. Online is a full rescan of SCSI disks with rescan-scsi-bus.sh | ||
# tool. Ubuntu and CentoOS distros currently supported - need to add supporting | ||
# package if distro is different. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment is no longer accurate and should be updated.
ss="${slave_dir}/state" | ||
sd="${slave_dir}/delete" | ||
log_must eval "$ECHO "offline" > ${ss}" | ||
log_must eval "$ECHO "1" > ${sd}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The quoting here doesn't look right. How about just using single quotes around the value.
log_must eval "$ECHO 'offline' > ${ss}"
log_must eval "$ECHO '1' > ${sd}"
dev_state="/sys/block/$disk/device/state" | ||
dev_delete="/sys/block/$disk/device/delete" | ||
log_must eval "$ECHO "offline" > ${dev_state}" | ||
log_must eval "$ECHO "1" > ${dev_delete}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comment above
elif [[ $state == "online" ]]; then | ||
#force a full rescan | ||
log_must scan_scsi_hosts | ||
$SLEEP 5 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is a block_device_wait
call here insufficient?
log_must $TOUCH ${ZEDLET_DIR}/zed.pid | ||
log_must $CHMOD 666 ${ZEDLET_DIR}/zed.pid | ||
log_must $TOUCH ${ZEDLET_DIR}/state | ||
log_must $CHMOD 666 ${ZEDLET_DIR}/state |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do this files needs to be created as part of setup.sh
? The zed should create them when it's started.
fi | ||
else | ||
log_fail "ZED already running" | ||
fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd suggest using pgrep -x zed
here to simplify things, it also helps avoid assuming install paths. Something like this:
# Verify the ZED is not already running.
$PGREP -x zed >/dev/null && log_fail "ZED already running"
log_note "Starting ZED" | ||
#run ZED in the background and redirect foreground logging output to zedlog | ||
log_must eval "$ZED -vF -d $ZEDLET_DIR -p $ZEDLET_DIR/zed.pid -s" \ | ||
"$ZEDLET_DIR/state 2>${ZEDLET_DIR}/zedlog&" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we get a space before the trailing &
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can see us wanting to move this functionality in to a fault.shlib
at some point. For example, it would be nice to be able to start/stop the zed and sighup it to reread it's zedlets. But we can do this when it's needed.
fi | ||
|
||
#add some data to the pool | ||
log_must $TRUNCATE -s $FSIZE /$pool/data |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
$MKFILE $FSIZE $TESTPOOL/data
might be a better way to do this since then you won't get a sparse file.
log_must $TRUNCATE -s $FSIZE /$pool/data | ||
|
||
#cache file import | ||
log_must $ZPOOL set cachefile=/etc/zfs/zpool.cache $pool |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We shouldn't use the standard cache file location since we might overwrite a real cache file, this should be under tmp.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm good with this. But we'll probably find we want to move the zed setup/cleanup function outside the fault directory since there are other test cases which will require the zed. For example, zpool_expand_001_pos
and zpool_expand_003_neg
which test auto-expand will require it.
do | ||
log_must $ZPOOL export -F $TESTPOOL | ||
|
||
$LS /sys/block/$offline_disk/device/scsi_device |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Debugging?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whoops that is correct. I'll remove that. Or if you want you can remove it before merging.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
Automated auto-online test to go along with ZED FMA integration (PR 4673) auto_online_001.pos works with real devices (sd- and mpath) and with non-real block devices (loop) by adding a scsi_debug device to the pool Note: In order for test group to run, ZED must not currently be running. Kernel 3.16.37 or higher needed for scsi_debug to work properly If timeout occurs on test using a scsi_debug device (error noticed on Ubuntu system), a reboot might be needed in order for test to pass. (more investigation into this) Also suppressed output from is_real_device/is_loop_device/is_mpath_device - was making the log file very cluttered with useless error messages "ie /dev/mapper/sdc is not a block device" from previous patch Signed-off-by: Sydney Vanda <[email protected]> Reviewed-by: Don Brady <[email protected]>
Also suppressed output from is_real_device/is_loop_device/is_mpath_device -
was making the log file very cluttered with useless error messages
"ie /dev/mapper/sdc is not a block device"
Auto-online works with real devices (sd- and mpath).
Multipath functionality needs more testing with real multipath set-up.
Signed-off-by: Sydney Vanda [email protected]
Signed-off-by: David Quigley [email protected]
Reviewed-by: Don Brady [email protected]