Skip to content
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

Add test for ANA support #114

Closed
wants to merge 4 commits into from
Closed

Conversation

hreinecke
Copy link
Contributor

This small patchset adds a test for ANA support.
In doing so I also had to update the loop transport to enable more than one port.
But with that the issues mentioned in #113 are gone.

@kawasaki
Copy link
Collaborator

kawasaki commented Mar 3, 2023

I tried to run tests with the changes on kernel version 6.2, and found that number of nvme test cases fail. For example, nvme/004 fail after the commit "nvme: enable ANA support".

nvme/004 (test nvme and nvmet UUID NS descriptors)           [failed]
    runtime  1.776s  ...  0.598s
    --- tests/nvme/004.out      2023-02-28 19:15:13.425727236 +0900
    +++ /home/shin/kts/kernel-test-suite/src/blktests/results/nodev/nvme/004.out.bad    2023-03-03 09:12:21.674526288 +0900
    @@ -1,5 +1,5 @@
     Running nvme/004
    -91fdba0d-f87b-4c25-b80f-db7be1418b9e
    -uuid.91fdba0d-f87b-4c25-b80f-db7be1418b9e
    +323d5abb-ff8e-4a12-99ad-128e15baef41
    +uuid.323d5abb-ff8e-4a12-99ad-128e15baef41
     NQN:blktests-subsystem-1 disconnected 1 controller(s)
     Test complete

@kawasaki
Copy link
Collaborator

kawasaki commented Mar 3, 2023

As for the commit message of the second patch, it would be helpful to note that ANA is the acronym of Asymmetric Namespace Access that NVMe spec 1.4 introduced. (If I'm wrong, please correct me)

Copy link
Collaborator

@kawasaki kawasaki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hreinecke Thanks for the work. I reviewed and did test run, then made some feedbacks.

One more nit comment: the first three patches have various commit title prefixes "nvme", "nvme/rc" or "test/nvme". but they all modify test/nvme/rc file. The past commits do not have consistent prefix, but I think "nvme/rc" is the good choice.

tests/nvme/rc Outdated Show resolved Hide resolved
tests/nvme/rc Outdated Show resolved Hide resolved
The 'loop' transport allows for several distinct ports if
the transport address is set to a number.

Signed-off-by: Hannes Reinecke <[email protected]>
Update functions to support ANA (Asymmetric Namespace Access).

Signed-off-by: Hannes Reinecke <[email protected]>
When creating more than one port we need to ensure that the
transport address differs, otherwise we cannot connect to it.
So automatically increment the transport address (for loop)
or the trsvcid (for tcp) to create distinct transport addresses.

Signed-off-by: Hannes Reinecke <[email protected]>
Create two ports on the subsystem and connect to both of them,
then run a fio job in the background and switch the ANA state
of these ports to check that I/O runs during state changes.

Signed-off-by: Hannes Reinecke <[email protected]>
@kawasaki
Copy link
Collaborator

kawasaki commented Mar 7, 2023

@hreinecke Thanks for reworking the patches. Today, I ran the test case nvme/047 and observed it fails.

nvme/047 (test ANA optimized/transitioning/inaccessible support) [failed]
    runtime  44.260s  ...  45.509s
    --- tests/nvme/047.out      2023-03-07 10:07:13.131098972 +0900
    +++ /home/shin/kts/kernel-test-suite/src/blktests/results/nodev/nvme/047.out.bad    2023-03-07 13:54:20.111062268 +0900
    @@ -1,28 +1,71 @@
     Running nvme/047
     ANA state failover
    -1: grpid 1 state inaccessible
    -1: grpid 2 state optimized
    -2: grpid 1 state optimized
    -2: grpid 2 state inaccessible
    +cat: /sys/class/nvme/nvme7/nvme7n1/ana_grpid: No such file or directory
    ...
    (Run 'diff -u tests/nvme/047.out /home/shin/kts/kernel-test-suite/src/blktests/results/nodev/nvme/047.out.bad' to see the entire diff)

It looks that the sysfs attribute file ana_grpid is not found under /sys/class/nvme/nvmeX/nvmeXn1. I wonder what I'm missing to make the test case pass. I use kernel v6.2 and nvme-cli v2.2.1. Do I need any changes on top of these?

One nit comment is shellcheck warning. The change below for the last patch will avoid it.

diff --git a/tests/nvme/047 b/tests/nvme/047
index 0853d76..401f06e 100755
--- a/tests/nvme/047
+++ b/tests/nvme/047
@@ -58,7 +58,7 @@ execute_ana_test() {
        local port1=$3
        local port2=$4
        local mount_dir="/mnt/blktests"
-       local nproc=$(nproc)
+       local nproc
 
        switch_nvmet_anagroup "${port1}" "${port2}" "failover"
 
@@ -71,6 +71,7 @@ execute_ana_test() {
 
        _xfs_mkfs_and_mount "/dev/${nvmedev}n2" "${mount_dir}/ns2" >> "${FULL}" 2>&1
    
+       nproc=$(nproc)
        if (( nproc > 8 )); then
                nproc=8
        fi

@hreinecke
Copy link
Contributor Author

And we can now retire this in favour of #146

@hreinecke hreinecke closed this Oct 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants