`nvme/039` assumes too much

[igaw]

nvme/039 started to fail with the current nvme-cli master. nvme-cli started to probe if the 64bit ioctl can be used with

	ret = ioctl(hdl->fd, NVME_IOCTL_ADMIN64_CMD, &dummy);
	if (ret > 0)
		hdl->ioctl64 = true;

and the tests does:

inject_access_denied_on_identify()
{
	# Inject a 'Access Denied' (0x286) status error on an
	# Identify admin command
	_nvme_enable_err_inject "$1" 0 100 1 0x286 1

	nvme admin-passthru /dev/"$1" --opcode=0x06 --data-len=4096 \
	    --cdw10=1 -r 2> /dev/null 1>&2

	_nvme_disable_err_inject "$1"

	if ${nvme_verbose_errors}; then
		last_dmesg 1 | grep "Access Denied (" | \
		    sed 's/nvme.*://g'
	else
		last_dmesg 1 | grep "Admin Cmd(" | \
		    sed 's/Admin Cmd/Identify/g' | \
		    sed 's/I\/O Error/Access Denied/g' | \
		    sed 's/nvme.*://g'
	fi
}

nvme/039 assumes after setting up the fault injection the next command will be the passthru command it issues. I don't think this is a very reliable approach IMO.

see linux-nvme/nvme-cli#3070

[kawasaki]

@igaw Thanks for the heads up.
@alanpeterad FYI.

My understanding about the failure is as follows:

• nvme-cli calls NVME_IOCTL_ADMIN64_CMD to check if 64 bit admin command is supported.
• nvme-cli was recently modified to provide dummy, zero-filled command block to the NVME_IOCTL_ADMIN64_CMD.
• Before the change, the error injected by nvme/039 for Identify command triggered the error in the Identify command as expected.
• After the change, the error injected by nvme/039 is no longer triggered in the Identify command. It is triggered for the dummy command to check the support of 64 bit admin command. Hence the test case fails.

My comments on this observation are as follows:

• I think the basic testing approach of current nvme/039 implementation is fine. To confirm that the nvme driver's error injection feature, I think it is natural to set up the error injection, trigger nvme command and check that the expected error was reported. IMO, current nvme/039 just implement it in straight-forward way, and I can not think of any other good ways (I can think of KUnit testing from kernel internal, but it will be rather complicated to specify test target device.)
• IMO, nvme-cli has two roles. The primary role is the tool to allow users to control NVMe devices in efficient manner. The secondary role is to support testing tool. The nvme groiup of blktests, and probably other test sets also, relies on the testing capability of nvme-cli command.
• My understanding is that the ioctl for 64bit admin command support check aims for the primary role. This is useful for users, and simplify nvme-cli command. On the other hand, it impacts the secondary role of nvme-cli command. For the testing purpose, it is desired to control each IOCTL in precise manner. But nvme-cli does not aim to control each IOCTL, right?
• I think we need to separated out the secondary purpose from nvme-cli. Probably, we can add a simple tool written in C under blktests's src/ directory, which can control each NVME IOCTL for nvme/039 (or other test cases in the future). We still can use nvme-cli for device set up and clean up. We will need the new tool just only for the limited part of nvme/039, hopefully.