fix(tests/nvme/056): Update ynl path

[nuclearcat]

The ynl Python code has been moved to a dedicated directory in Linux kernel commit ab88c2b3739a. Update the test to reflect this new location.

[kawasaki]

@nuclearcat Thank you for catching this!

The kernel commit which moved the python scripts is in v6.14-rc1 kernel. Can we make the test case work with the kernels older than v6.14-rc1 also? This will help to users who want to run the test case with v6.13.X kernels.

A helper function to get the cli script would be useful. FYI, here I share what is in my mind (totally untested).

diff --git a/tests/nvme/056 b/tests/nvme/056
index bdf0d67..57d758d 100755
--- a/tests/nvme/056
+++ b/tests/nvme/056
@@ -36,17 +36,32 @@ requires() {
        have_iface
 }
 
+CLI_PATH=tools/net/ynl/pyynl/cli.py
+OLD_CLI_PATH=tools/net/ynl/cli.py
+
+get_cli() {
+       local cli="${KERNELSRC}/${CLI_PATH}"
+
+       if [[ ! -e $cli ]]; then
+               cli="${KERNELSRC}/${OLD_CLI_PATH}"
+               if [[ ! -e $cli ]]; then
+                       SKIP_REASONS+=("Kernel sources do not have cli.py under tools/net/ynl")
+                       return 1
+               fi
+       fi
+
+       echo "$cli"
+}
+
 have_netlink_cli() {
        local cli
-       cli="${KERNELSRC}/tools/net/ynl/pyynl/cli.py"
 
-       if ! [ -f "$cli" ]; then
-               SKIP_REASONS+=("Kernel sources do not have tools/net/ynl/pyynl/cli.py")
+       if ! cli=$(get_cli); then
                return 1
        fi
 
        if ! "$cli" -h &> /dev/null; then
-               SKIP_REASONS+=("Cannot run the kernel tools/net/ynl/pyynl/cli.py")
+               SKIP_REASONS+=("Cannot run the kernel tool ${cli}")
                return 1;
        fi
 
@@ -69,7 +84,7 @@ set_conditions() {
 }
 
 netlink_cli() {
-       "${KERNELSRC}/tools/net/ynl/pyynl/cli.py" \
+       $(get_cli) \
                --spec "${KERNELSRC}/Documentation/netlink/specs/ulp_ddp.yaml" \
                "$@"
 }

[nuclearcat]

Thank you for your suggestion about supporting older kernels for this test case.
I'd like to clarify that the feature being tested is still pending inclusion in the kernel as patches in netdev and hasn't been merged yet. Since the functionality doesn't exist in any released kernel version (including v6.13.X or v6.14-rc1), the test will fail regardless of script location or kernel version.
The test case is being prepared in anticipation of the feature being merged soon. Once the patches are accepted, the test will begin working with whichever kernel version includes the merged feature.
Adding compatibility with older kernels wouldn't be helpful in this particular case, as the test is specifically verifying functionality that simply doesn't exist yet in any released kernel.
I can submit a separate PR once the feature is included in the kernel, which will add proper version checking to verify the minimal kernel version required. This would ensure the test gracefully handles execution on kernels that don't support the feature.

[kawasaki]

Ah, thanks for the clarification. I understand that there is no need to support the older kernel versions. I'll wait for the key kernel commit will be merged. I'm ok both to reuse this PR, or to create a separate PR.

Let me leave a comment about the kernel version check. When a blktests test case requires a specific kernel feature, it is not ideal to refer to the kernel version to check if the requirement is fulfilled, since such check is fragile. It is the better to check something visible from userland for such check: sysfs attribute file existence/value or module parameter existence check. Some test cases refer to kernel version with _have_kver() function, but it is the last resort.

Thanks!

[vigneshraman]

@kawasaki, if the kernel commit is merged, can we pick this PR? The reason I'm asking is we are using a patched version of blktests in kernelci.

[kawasaki]

@kawasaki, if the kernel commit is merged, can we pick this PR? The reason I'm asking is we are using a patched version of blktests in kernelci.

@nuclearcat Yes, I think so. The code change of this PR looks good to me. The commit subject prefix is rather different from other blktests commits. I think "nvme/056: Update ynl path" is simpler and enough. I can fold-in this change when I pick up the commit (Or you can update this PR).

As the the kernel side commit, could you share current status? Is there outlook to be included in any rcX tag?

[nuclearcat]

I will update commit, but still it is unclear when patches will be accepted. I will ask authors.