attach/detach command line interface changes broke nightly tests

[igaw]

It appears #2665 breaks our nightly builds:

https://github.com/linux-nvme/nvme-cli/actions/runs/12878797474/job/35905289626#step:8:761

@ikegami-t could you have a look?

BTW, be careful running these tests locally. These tests do write to the local hardware, e.g. /dev/nvme0n1

[ikegami-t]

Just confirmed the log then not sure why the device /dev/nvme1 failed to detach as not botu both chardev and blkdev and also /dev/nvme1 NS1 not failed but /dev/nvme1 NS2 failed but looks this not caused by the nvme-cli bug I think.

1. Build and install nvme-cli: nvme-cli 2.11 built and tested
   https://github.com/linux-nvme/nvme-cli/actions/runs/12878797474/job/35905289626#step:6:134

  Configuration
    git version   : "1.11.1-41-ga9f667a"

nvme-cli 2.11

  Paths
    prefixdir         : /usr/local

2. Run on device tests: Failed on detaching NS and also seems failed to detach NS2
   https://github.com/linux-nvme/nvme-cli/actions/runs/12878797474/job/35905289626#step:8:616

##### Creating 109
##### Attaching 109
##### Running IOs in 109
Testcase main ... /dev/nvme1 is not a block or character device
Usage: nvme detach-ns <device> [OPTIONS]

Detach the given namespace from the given controller; de-activates the given
namespace's ID. A namespace must be attached to a controller before IO
commands may be directed to that namespace.

Options:
  [  --verbose, -v ]                    --- Increase output verbosity
  [  --output-format=<FMT>, -o <FMT> ]  --- Output format: normal|json|binary
  [  --namespace-id=<NUM>, -n <NUM> ]   --- namespace to attach
  [  --controllers=<LIST>, -c <LIST> ]  --- optional comma-sep controller id
                                            list
  [  --timeout=<NUM>, -t <NUM> ]        --- timeout value, in milliseconds
FAIL
...
##### Detaching 1
#### Deleting 1
##### Detaching 2

teardown: ctrl: /dev/nvme1, ns1: /dev/nvme1n1, default_nsid: 1, flbas: 0

3. Code: the error messege is output as the device /dev/nvme1 not both chardev and blkdev
   

   nvme-cli/nvme.c

   Line 367 in 946029c

   nvme_show_error("%s is not a block or character device", devstr);

static int open_dev_direct(struct nvme_dev **devp, char *devstr, int flags, __u32 nsid)
{
...
	if (!is_chardev(dev) && !is_blkdev(dev)) {
		nvme_show_error("%s is not a block or character device", devstr);
		errno = ENODEV;
		err = -1;
		goto err_close;
	}

[ikegami-t]

Sorry just realized the error caused on master as tested by run-nightly-tests #88 https://github.com/linux-nvme/nvme-cli/actions/runs/12878797474.
Now I think the error caused by the commit 565b30e changes that changes to use the chardev /dev/nvme1 to the blkdev /dev/nvme1n2 (or /dev/nvme1n1) to use instead then the error happend.
Also I think that the blkdev not detected by the driver fstat() actually then the error caused (but probably the device itself is okay as not failed before the changes).
So can we add some delay time before detaching NS? (Also for attaching and it seems for the attaching currently just used the chardev so no error happend.)

(Added)
Seems the test can be added the validation to create NS to wait until the blkdev /dev/nvmen created.

[igaw]

As far I understand the situation, before your change the test would open the char device /dev/nvme0 and now it opens /dev/nvme0n1. Exactly, what the indent of your change was. Not sure why this tests worked in the past. I must miss something

Adding @keithbusch and @MaisenbacherD for awareness.

[ikegami-t]

@MaisenbacherD Can we add to check and weit wait the blkdev /dev/nvme*n* is created atucally actually into the test function create_and_validate_ns()? Since seems the test faiulre failure caused as the blkdev not created completely by the host driver before the NS detaching.

[igaw]

I think we need something like this

        if err == 0:
            # enumerate new namespace block device
            self.nvme_reset_ctrl()
            # check if new namespace block device exists
            err = 0 if stat.S_ISBLK(os.stat(self.ns1).st_mode) else 1

for the create_and_validate_ns method as we have in attach_ns

[ikegami-t]

Thank you! Noted it. By the way can we add aslo the -v option for the NS management command commands to check the blkdev and dev print info outputs as below?

tokunori@tokunori-desktop:~/nvme-cli$ nvme-build detach-ns /dev/nvme0 -n 2 -v
[sudo] tokunori のパスワード: 
blkdev: /dev/nvme0n2, fd: -1
dev: /dev/nvme0, fd: 3
warning: empty controller-id list will result in no actual change in namespace attachment
NVMe status: Invalid Command Opcode: A reserved coded value or an unsupported value in the command opcode field(0x1)
NS management and attachment not supported
tokunori@tokunori-desktop:~/nvme-cli$ 

[ikegami-t]

About the blkdev checking as you mentioned it is done in attach_ns already so it looks enough to check the blkdev exists. So still the cause is unkown then let consider more later.

[ikegami-t]

By the way do we need to execute the nvme_tests as root as same with nvme-cli? But failed to install nose2 after changed to su by sudo su then I can not execute the nvme_test as root for now. If any advice please let me know.

tokunori@tokunori-desktop:~/nvme-cli$ nose2 --verbose --start-dir tests nvme_create_max_ns_test
test_attach_detach_ns (nvme_create_max_ns_test.TestNVMeCreateMaxNS.test_attach_detach_ns)
Testcase main ... 
Using nvme binary 'nvme'

setup: ctrl: /dev/nvme0, ns1: /dev/nvme0n1, default_nsid: 1, flbas: 0

Usage: nvme id-ctrl <device> [OPTIONS]

Send an Identify Controller command to the given device and report
information about the specified controller in human-readable or binary
format. May also return vendor-specific controller attributes in hex-dump if
requested.

Options:
  [  --verbose, -v ]                    --- Increase output verbosity
  [  --output-format=<FMT>, -o <FMT> ]  --- Output format: normal|json|binary
  [  --vendor-specific, -V ]            --- dump binary vendor field
  [  --raw-binary, -b ]                 --- show identify in binary format
  [  --human-readable, -H ]             --- show identify in readable format
  [  --timeout=<NUM>, -t <NUM> ]        --- timeout value, in milliseconds
FAIL



======================================================================
FAIL: test_attach_detach_ns (nvme_create_max_ns_test.TestNVMeCreateMaxNS.test_attach_detach_ns)
Testcase main
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/tokunori/nvme-cli/tests/nvme_create_max_ns_test.py", line 53, in setUp
    self.nsze = int(self.get_ncap() /
                    ^^^^^^^^^^^^^^^
  File "/home/tokunori/nvme-cli/tests/nvme_test.py", line 285, in get_ncap
    return int(self.get_id_ctrl_field_value("tnvmcap"))
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/tokunori/nvme-cli/tests/nvme_test.py", line 301, in get_id_ctrl_field_value
    self.assertEqual(err, 0, "ERROR : reading id-ctrl failed")
AssertionError: 1 != 0 : ERROR : reading id-ctrl failed

----------------------------------------------------------------------
Ran 1 test in 0.079s

FAILED (failures=1)

[igaw]

Bummer, this would have been to easy.

Yes, this testsuite needs to be run as root and as noted before it will write to the physical disk /dev/nvme0n1. So by very careful here. This was also the reason why these tests are disabled by default.

[ikegami-t]

Okay I will do retry to execute by the root later since failed to install nose2 after executed 'sudo su' earlier.

[ikegami-t]

Sorry I could not find the cause of error as looks no error in both nvme-cli and nvme_test but just pushed the fix PR #2671 for this issue to avoid as partially reverted the 565b30e changes cause caused this issue.

[igaw]

I'd rather revert the change instead stacking stuff on top of each other. I'll try to find some time to look into it. If I don't figure out I'll revert the offending change.