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 <[email protected]> > --- > 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 >
