On 6/5/19 7:12 AM, Chaitanya Kulkarni wrote:
> Overall it looks good to me, couple of nits, can be ignored for now.
>
> Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni@xxxxxxx>
>
> On 5/30/19 6:59 PM, Shin'ichiro Kawasaki wrote:
>> As a preparation for the zone mapping test case, add several helper
>> functions. _find_last_sequential_zone() and
>> _find_sequential_zone_in_middle() help to select test target zones.
>> _test_dev_is_logical() checks TEST_DEV is the valid test target.
>> _test_dev_has_dm_map() helps to check that the dm target is linear or
>> flakey. _get_dev_container_and_sector() helps to get the container device
>> and sector mappings.
>> ---
>> tests/zbd/rc | 133 +++++++++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 133 insertions(+)
>>
>> diff --git a/tests/zbd/rc b/tests/zbd/rc
>> index 5f04c84..792b83d 100644
>> --- a/tests/zbd/rc
>> +++ b/tests/zbd/rc
>> @@ -193,6 +193,42 @@ _find_first_sequential_zone() {
>> return 1
>> }
>>
>> +_find_last_sequential_zone() {
>> + for ((idx = REPORTED_COUNT - 1; idx > 0; idx--)); do
>> + if ((ZONE_TYPES[idx] == ZONE_TYPE_SEQ_WRITE_REQUIRED)); then
>> + echo "${idx}"
>> + return 0
>> + fi
>> + done
>> +
>> + echo "-1"
>> + return 1
>> +}
>> +
>> +# Try to find a sequential required zone between given two zone indices
>> +_find_sequential_zone_in_middle() {
>> + local -i s=${1}
>> + local -i e=${2}
> nit:- do we need to validate the s and e before we use ?
Yes. Will add the checks in the v3 patch. Thanks.
>> + local -i idx=$(((s + e) / 2))
>> + local -i i=1
>> +
> nit:- Is there a reason for while ? we can also get away with for loop
> right ?
My understanding is that ZBC and ZAC allow to place conventional zones between
the sequential required zones 's' and 'e'. The loop is required to check zone
types to find out a sequential required zone, not a conventional zone in case
such devices get available in the future.
>> + while ((idx != s && idx != e)); do
>> + if ((ZONE_TYPES[idx] == ZONE_TYPE_SEQ_WRITE_REQUIRED)); then
>> + echo "${idx}"
>> + return 0
>> + fi
>> + if ((i%2 == 0)); then
>> + $((idx += i))
>> + else
>> + $((idx -= i))
>> + fi
>> + $((i++))
>> + done
>> +
>> + echo "-1"
>> + return 1
>> +}
>> +
>> # Search zones and find two contiguous sequential required zones.
>> # Return index of the first zone of the found two zones.
>> # Call _get_blkzone_report() beforehand.
>> @@ -210,3 +246,100 @@ _find_two_contiguous_seq_zones() {
>> echo "Contiguous sequential write required zones not found"
>> return 1
>> }
>> +
>> +_test_dev_is_dm() {
>> + if [[ ! -r "${TEST_DEV_SYSFS}/dm/name" ]]; then
>> + SKIP_REASON="$TEST_DEV is not device-mapper"
>> + return 1
>> + fi
>> + return 0
>> +}
>> +
>> +_test_dev_is_logical() {
>> + if ! _test_dev_is_partition && ! _test_dev_is_dm; then
>> + SKIP_REASON="$TEST_DEV is not a logical device"
>> + return 1
>> + fi
>> + return 0
>> +}
>> +
>> +_test_dev_has_dm_map() {
>> + local target_type=${1}
>> + local dm_name
>> +
>> + dm_name=$(<"${TEST_DEV_SYSFS}/dm/name")
>> + if ! dmsetup status "${dm_name}" | grep -qe "${target_type}"; then
>> + SKIP_REASON="$TEST_DEV does not have ${target_type} map"
>> + return 1
>> + fi
>> + if dmsetup status "${dm_name}" | grep -v "${target_type}"; then
>> + SKIP_REASON="$TEST_DEV has map other than ${target_type}"
>> + return 1
>> + fi
>> + return 0
>> +}
>> +
>> +# Get device file path from the device ID "major:minor".
>> +_get_dev_path_by_id() {
>> + for d in /sys/block/*; do
>> + if [[ ! -r "${d}/dev" ]]; then
>> + continue
>> + fi
>> + if [[ "${1}" == "$(<"${d}/dev")" ]]; then
>> + echo "/dev/${d##*/}"
>> + return 0
>> + fi
>> + done
>> + return 1
>> +}
>> +
>> +# Given sector of TEST_DEV, return the device which contain the sector and
>> +# corresponding sector of the container device.
>> +_get_dev_container_and_sector() {
>> + local -i sector=${1}
>> + local cont_dev
>> + local -i offset
>> + local -a tbl_line
>> +
>> + if _test_dev_is_partition; then
>> + offset=$(<"${TEST_DEV_PART_SYSFS}/start")
>> + cont_dev=$(_get_dev_path_by_id "$(<"${TEST_DEV_SYSFS}/dev")")
>> + echo "${cont_dev}" "$((offset + sector))"
>> + return 0
>> + fi
>> +
>> + if ! _test_dev_is_dm; then
>> + echo "${TEST_DEV} is not a logical device"
>> + return 1
>> + fi
>> + if ! _test_dev_has_dm_map linear &&
>> + ! _test_dev_has_dm_map flakey; then
>> + echo -n "dm mapping test other than linear/flakey is"
>> + echo "not implemented"
>> + return 1
>> + fi
>> +
>> + # Parse dm table lines for dm-linear or dm-flakey target
>> + while read -r -a tbl_line; do
>> + local -i map_start=${tbl_line[0]}
>> + local -i map_end=$((tbl_line[0] + tbl_line[1]))
>> +
>> + if ((sector < map_start)) || (((map_end) <= sector)); then
>> + continue
>> + fi
>> +
>> + offset=${tbl_line[4]}
>> + if ! cont_dev=$(_get_dev_path_by_id "${tbl_line[3]}"); then
>> + echo -n "Cannot access to container device: "
>> + echo "${tbl_line[3]}"
>> + return 1
>> + fi
>> +
>> + echo "${cont_dev}" "$((offset + sector - map_start))"
>> + return 0
>> +
>> + done < <(dmsetup table "$(<"${TEST_DEV_SYSFS}/dm/name")")
>> +
>> + echo -n "Cannot find container device of ${TEST_DEV}"
>> + return 1
>> +}
>
>
>
--
Best Regards,
Shin'ichiro Kawasaki
