On Wed, Feb 20, 2019 at 05:12:29PM +0900, Shin'ichiro Kawasaki wrote:
> When partition devices are specified in TEST_DEV, TEST_DEV_SYSFS
> variable points to the sysfs paths of holder devices of the partition
> devices (e.g., /sys/block/sda). This sysfs path is different from the
> sysfs path of the partition devices (e.g., /sys/block/sda/sda1). For
> example, size parameters exist in both the holder device sysfs and
> the partition device sysfs with different values.
>
> To allow test cases to access sysfs path of the partition devices,
> add TEST_DEV_PART_SYSFS variable. TEST_DEV_SYSFS is set as is to refer
> the sysfs path of the holder devices. If the TEST_DEV is not a partition
> device, an empty string is set to the TEST_DEV_PART_SYSFS variable.
>
> Change _find_sysfs_dir() function to return the holder device sysfs as
> well as the partition device sysfs. The function obtains the canonical
> sysfs path, and if the device is a partition device, the function cut the
> last device name in the canonical sysfs path to obtain the holder device
> sysfs path.
This makes sense. A couple of small tweaks below.
> Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@xxxxxxx>
> ---
> check | 51 ++++++++++++++++++++++++++++++++++-----------------
> new | 16 ++++++++++++++--
> 2 files changed, 48 insertions(+), 19 deletions(-)
>
> diff --git a/check b/check
> index f41ecba..e45b34f 100755
> --- a/check
> +++ b/check
> @@ -442,13 +442,19 @@ _run_test() {
> _warning "$TEST_NAME: fallback_device call failure"
> return 0
> fi
> - if ! sysfs_dir="$(_find_sysfs_dir "$test_dev")"; then
> +
> + local dirs
> + local sysfs_dir
> + local part_sysfs_dir
> + if ! dirs=$(_find_sysfs_dir "$test_dev") ; then
> _warning "$TEST_NAME: could not find sysfs directory for ${test_dev}"
> cleanup_fallback_device
> return 0
> fi
> + read -r sysfs_dir part_sysfs_dir < <(echo "${dirs}")
Let's rename _find_sysfs_dir to find_sysfs_dirs and make it set
TEST_DEV_SYSFS_DIRS["$test_dev"] and
TEST_DEV_PART_SYSFS_DIRS["$test_dev"] itself instead of returning the
string.
> TEST_DEVS=( "${test_dev}" )
> TEST_DEV_SYSFS_DIRS["$test_dev"]="$sysfs_dir"
> + TEST_DEV_PART_SYSFS_DIRS["$test_dev"]="$part_sysfs_dir"
> FALLBACK_DEVICE=1
> fi
>
> @@ -464,6 +470,7 @@ _run_test() {
> local ret=0
> for TEST_DEV in "${TEST_DEVS[@]}"; do
> TEST_DEV_SYSFS="${TEST_DEV_SYSFS_DIRS["$TEST_DEV"]}"
> + TEST_DEV_PART_SYSFS="${TEST_DEV_PART_SYSFS_DIRS["$TEST_DEV"]}"
> if (( !CAN_BE_ZONED )) && _test_dev_is_zoned; then
> SKIP_REASON="${TEST_DEV} is a zoned block device"
> _output_notrun "$TEST_NAME => $(basename "$TEST_DEV")"
> @@ -483,6 +490,7 @@ _run_test() {
> if (( FALLBACK_DEVICE )); then
> cleanup_fallback_device
> unset TEST_DEV_SYSFS_DIRS["${TEST_DEVS[0]}"]
> + unset TEST_DEV_PART_SYSFS_DIRS["${TEST_DEVS[0]}"]
> TEST_DEVS=()
> fi
>
> @@ -507,6 +515,8 @@ _run_group() {
> for i in "${!TEST_DEVS[@]}"; do
> TEST_DEV="${TEST_DEVS[$i]}"
> TEST_DEV_SYSFS="${TEST_DEV_SYSFS_DIRS["$TEST_DEV"]}"
> + # shellcheck disable=SC2034
> + TEST_DEV_PART_SYSFS="${TEST_DEV_PART_SYSFS_DIRS["$TEST_DEV"]}"
> if ! group_device_requires; then
> _output_notrun "${group}/*** => $(basename "$TEST_DEV")"
> unset TEST_DEVS["$i"]
> @@ -529,28 +539,31 @@ _run_group() {
>
> _find_sysfs_dir() {
> local test_dev="$1"
> + local sysfs_path
> local major=$((0x$(stat -L -c '%t' "$test_dev")))
> local minor=$((0x$(stat -L -c '%T' "$test_dev")))
> - local dev="$major:$minor"
> + local sysdev_path="/sys/dev/block/${major}:${minor}"
>
> - local block_dir part_dir
> - for block_dir in /sys/block/*; do
> - if [[ $(cat "${block_dir}/dev") = "$dev" ]]; then
> - echo "$block_dir"
> - return
> - fi
> - for part_dir in "$block_dir"/*; do
> - if [[ -r ${part_dir}/dev && $(cat "${part_dir}/dev") = "$dev" ]]; then
> - echo "$block_dir"
> - return
> - fi
> - done
> - done
> + # Get the canonical sysfs path
> + if ! sysfs_path=/sys/dev/block/$(readlink "${sysdev_path}"); then
sysfs_path="$(realpath "/sys/dev/block/${major}:${minor}")" is a bit
shorter, does that still work?
> + return 1
> + fi
>
> - return 1
> + if [[ -r "${sysfs_path}"/partition ]]; then
> + # If the device is a partition device, cut the last device name
> + # of the canonical sysfs path to access to the sysfs of its
> + # holder device.
> + # e.g. .../block/sda/sda1 -> ...block/sda
> + # Return both the holder device sysfs path and the partition
> + # device sysfs path.
> + echo "${sysfs_path%/*}" "${sysfs_path}"
> + else
> + echo "${sysfs_path}" ""
> + fi
> }
>
> declare -A TEST_DEV_SYSFS_DIRS
> +declare -A TEST_DEV_PART_SYSFS_DIRS
> _check() {
> # shellcheck disable=SC2034
> SRCDIR="$(realpath src)"
> @@ -563,11 +576,15 @@ _check() {
> _error "${test_dev} is not a block device"
> fi
>
> + local dirs
> local sysfs_dir
> - if ! sysfs_dir="$(_find_sysfs_dir "$test_dev")"; then
> + local part_sysfs_dir
> + if ! dirs=$(_find_sysfs_dir "$test_dev") ; then
> _error "could not find sysfs directory for ${test_dev}"
> fi
> + read -r sysfs_dir part_sysfs_dir < <(echo "${dirs}")
> TEST_DEV_SYSFS_DIRS["$test_dev"]="$sysfs_dir"
> + TEST_DEV_PART_SYSFS_DIRS["$test_dev"]="$part_sysfs_dir"
> done
>
> local test_name group prev_group
> diff --git a/new b/new
> index d7d5f7c..24c066d 100755
> --- a/new
> +++ b/new
> @@ -80,7 +80,10 @@ group_requires() {
> # should return non-zero and set the \$SKIP_REASON variable. \$TEST_DEV is the
> # full path of the block device (e.g., /dev/nvme0n1 or /dev/sda1), and
> # \$TEST_DEV_SYSFS is the sysfs path of the disk (not the partition, e.g.,
> -# /sys/block/nvme0n1 or /sys/block/sda).
> +# /sys/block/nvme0n1 or /sys/block/sda). If the target device is a partition
> +# device, \$TEST_DEV_PART_SYSFS is the sysfs path of the partition device
> +# (e.g., /sys/block/nvme0n1/nvme0n1p1 or /sys/block/sda/sda1). Otherwise,
> +# \$TEST_DEV_PART_SYSFS is an empty string.
> #
> # Usually, group_device_requires() just needs to check that the test device is
> # the right type of hardware or supports any necessary features using the
> @@ -165,7 +168,10 @@ DESCRIPTION=""
> # set the \$SKIP_REASON variable. \$TEST_DEV is the full path of the block
> # device (e.g., /dev/nvme0n1 or /dev/sda1), and \$TEST_DEV_SYSFS is the sysfs
> # path of the disk (not the partition, e.g., /sys/block/nvme0n1 or
> -# /sys/block/sda).
> +# /sys/block/sda). If the target device is a partition device,
> +# \$TEST_DEV_PART_SYSFS is the sysfs path of the partition device (e.g.,
> +# /sys/block/nvme0n1/nvme0n1p1 or /sys/block/sda/sda1). Otherwise,
> +# \$TEST_DEV_PART_SYSFS is an empty string.
> #
> # Usually, device_requires() just needs to check that the test device is the
> # right type of hardware or supports any necessary features using the
> @@ -207,6 +213,12 @@ DESCRIPTION=""
> # - \$TEST_DEV_SYSFS -- the sysfs directory of the device (e.g.,
> # /sys/block/sda). In general, you should use the
> # _test_dev_queue_{get,set} helpers.
> +# If the device is a partition device, the sysfs
> +# directory of its holder device is set.
> +# - \$TEST_DEV_PART_SYSFS -- the sysfs directory of the device if the device
> +# is a partition device (e.g.,
> +# /sys/block/sda/sda1). Empty string is set if
> +# the device is not a partition device.
> test() {
> echo "Running \${TEST_NAME}"
>
> --
> 2.20.1
>
