nvme: Always set result field for passthru cmd

Modify admin passthru so that result field of passthru struct is always
populated. This should be safe since dw0 is either reserved or contains
command specific info. This is specifically meant for the namespace
management command when attempting to create a namespace. As per spec:
"Dword 0 of the completion queue entry contains the Namespace Identifier
created.". So for nvme cli and perhaps other application to see what is
the id of the namespace created there needs to be a way to pass the
information back.

Signed-off-by: Ahriben Gonzalez <ahribeng@gmail.com>
Change-Id: Ide4effc126ad9eedac95b0700dd65041ed4b35b1
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/10633
Community-CI: Broadcom CI <spdk-ci.pdl@broadcom.com>
Community-CI: Mellanox Build Bot
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Tomasz Zawadzki <tomasz.zawadzki@intel.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>

lib/nvme/nvme_cuse.c

@@ -99,26 +99,21 @@ cuse_nvme_passthru_cmd_cb(void *arg, const struct spdk_nvme_cpl *cpl)
 	struct cuse_io_ctx *ctx = arg;
 	struct iovec out_iov[2];
 	struct spdk_nvme_cpl _cpl;
+	int out_iovcnt = 0;
 	uint16_t status_field = cpl->status_raw >> 1; /* Drop out phase bit */
 
-	if (ctx->data_transfer == SPDK_NVME_DATA_HOST_TO_CONTROLLER ||
+	memcpy(&_cpl, cpl, sizeof(struct spdk_nvme_cpl));
-	    ctx->data_transfer == SPDK_NVME_DATA_NONE) {
+	out_iov[out_iovcnt].iov_base = &_cpl.cdw0;
-		fuse_reply_ioctl_iov(ctx->req, status_field, NULL, 0);
+	out_iov[out_iovcnt].iov_len = sizeof(_cpl.cdw0);
-	} else {
+	out_iovcnt += 1;
-		memcpy(&_cpl, cpl, sizeof(struct spdk_nvme_cpl));
 
-		out_iov[0].iov_base = &_cpl.cdw0;
+	if (ctx->data_transfer == SPDK_NVME_DATA_CONTROLLER_TO_HOST && ctx->data_len > 0) {
-		out_iov[0].iov_len = sizeof(_cpl.cdw0);
+		out_iov[out_iovcnt].iov_base = ctx->data;
-
+		out_iov[out_iovcnt].iov_len = ctx->data_len;
-		if (ctx->data_len > 0) {
+		out_iovcnt += 1;
-			out_iov[1].iov_base = ctx->data;
-			out_iov[1].iov_len = ctx->data_len;
-			fuse_reply_ioctl_iov(ctx->req, status_field, out_iov, 2);
-		} else {
-			fuse_reply_ioctl_iov(ctx->req, status_field, out_iov, 1);
-		}
 	}
 
+	fuse_reply_ioctl_iov(ctx->req, status_field, out_iov, out_iovcnt);
 	cuse_io_ctx_free(ctx);
 }
 
@@ -206,52 +201,58 @@ cuse_nvme_passthru_cmd(fuse_req_t req, int cmd, void *arg,
 {
 	struct nvme_passthru_cmd *passthru_cmd;
 	struct iovec in_iov[2], out_iov[2];
+	int in_iovcnt = 0, out_iovcnt = 0;
+	const void *dptr = NULL;
+	enum spdk_nvme_data_transfer data_transfer;
 
-	in_iov[0].iov_base = (void *)arg;
+	in_iov[in_iovcnt].iov_base = (void *)arg;
-	in_iov[0].iov_len = sizeof(*passthru_cmd);
+	in_iov[in_iovcnt].iov_len = sizeof(*passthru_cmd);
+	in_iovcnt += 1;
 	if (in_bufsz == 0) {
-		fuse_reply_ioctl_retry(req, in_iov, 1, NULL, 0);
+		fuse_reply_ioctl_retry(req, in_iov, in_iovcnt, NULL, out_iovcnt);
 		return;
 	}
 
 	passthru_cmd = (struct nvme_passthru_cmd *)in_buf;
+	data_transfer = spdk_nvme_opc_get_data_transfer(passthru_cmd->opcode);
 
-	switch (spdk_nvme_opc_get_data_transfer(passthru_cmd->opcode)) {
+	if (data_transfer == SPDK_NVME_DATA_HOST_TO_CONTROLLER) {
-	case SPDK_NVME_DATA_HOST_TO_CONTROLLER:
+		/* Make data pointer accessible (RO) */
 		if (passthru_cmd->addr != 0) {
-			in_iov[1].iov_base = (void *)passthru_cmd->addr;
+			in_iov[in_iovcnt].iov_base = (void *)passthru_cmd->addr;
-			in_iov[1].iov_len = passthru_cmd->data_len;
+			in_iov[in_iovcnt].iov_len = passthru_cmd->data_len;
-			if (in_bufsz == sizeof(*passthru_cmd)) {
+			in_iovcnt += 1;
-				fuse_reply_ioctl_retry(req, in_iov, 2, NULL, 0);
-				return;
-			}
-			cuse_nvme_passthru_cmd_send(req, passthru_cmd, in_buf + sizeof(*passthru_cmd), cmd);
-		} else {
-			cuse_nvme_passthru_cmd_send(req, passthru_cmd, NULL, cmd);
 		}
-		return;
+	}
-	case SPDK_NVME_DATA_NONE:
+
-	case SPDK_NVME_DATA_CONTROLLER_TO_HOST:
+	/* Always make result field writable regardless of data transfer bits */
-		if (out_bufsz == 0) {
+	out_iov[out_iovcnt].iov_base = &((struct nvme_passthru_cmd *)arg)->result;
-			out_iov[0].iov_base = &((struct nvme_passthru_cmd *)arg)->result;
+	out_iov[out_iovcnt].iov_len = sizeof(uint32_t);
-			out_iov[0].iov_len = sizeof(uint32_t);
+	out_iovcnt += 1;
-			if (passthru_cmd->data_len > 0) {
+
-				out_iov[1].iov_base = (void *)passthru_cmd->addr;
+	if (data_transfer == SPDK_NVME_DATA_CONTROLLER_TO_HOST) {
-				out_iov[1].iov_len = passthru_cmd->data_len;
+		if (passthru_cmd->data_len > 0) {
-				fuse_reply_ioctl_retry(req, in_iov, 1, out_iov, 2);
+			out_iov[out_iovcnt].iov_base = (void *)passthru_cmd->addr;
-			} else {
+			out_iov[out_iovcnt].iov_len = passthru_cmd->data_len;
-				fuse_reply_ioctl_retry(req, in_iov, 1, out_iov, 1);
+			out_iovcnt += 1;
-			}
-			return;
 		}
+	}
 
-		cuse_nvme_passthru_cmd_send(req, passthru_cmd, NULL, cmd);
+	if (out_bufsz == 0) {
-
+		fuse_reply_ioctl_retry(req, in_iov, in_iovcnt, out_iov, out_iovcnt);
 		return;
-	case SPDK_NVME_DATA_BIDIRECTIONAL:
+	}
+
+	if (data_transfer == SPDK_NVME_DATA_BIDIRECTIONAL) {
 		fuse_reply_err(req, EINVAL);
 		return;
 	}
+
+	if (data_transfer == SPDK_NVME_DATA_HOST_TO_CONTROLLER) {
+		dptr = (passthru_cmd->addr == 0) ? NULL : in_buf + sizeof(*passthru_cmd);
+	}
+
+	cuse_nvme_passthru_cmd_send(req, passthru_cmd, dptr, cmd);
 }
 
 static void

test/nvme/cuse/nvme_ns_manage_cuse.sh

@@ -51,13 +51,24 @@ function reset_nvme_if_aer_unsupported() {
 	fi
 }
 
+function remove_all_namespaces() {
+	info_print "delete all namespaces"
+	active_nsids=$($NVME_CMD list-ns ${nvme_dev} | cut -f2 -d:)
+	# Cant globally detach all namespaces ... must do so one by one
+	for n in ${active_nsids}; do
+		info_print "removing nsid=${n}"
+		$NVME_CMD detach-ns ${nvme_dev} -n ${n} -c 0 || true
+		$NVME_CMD delete-ns ${nvme_dev} -n ${n} || true
+	done
+}
+
 function clean_up() {
 	$rootdir/scripts/setup.sh reset
 
 	# This assumes every NVMe controller contains single namespace,
 	# encompassing Total NVM Capacity and formatted as 512 block size.
 	# 512 block size is needed for test/vhost/vhost_boot.sh to
-	# successfully run.
+	# succesfully run.
 
 	tnvmcap=$($NVME_CMD id-ctrl ${nvme_dev} | grep tnvmcap | cut -d: -f2)
 	blksize=512
@@ -65,10 +76,9 @@ function clean_up() {
 	size=$((tnvmcap / blksize))
 
 	echo "Restoring $nvme_dev..."
-	$NVME_CMD detach-ns ${nvme_dev} -n 0xffffffff -c 0 || true
+	remove_all_namespaces
-	$NVME_CMD delete-ns ${nvme_dev} -n 0xffffffff || true
+	nsid=$($NVME_CMD create-ns ${nvme_dev} -s ${size} -c ${size} -b ${blksize} | grep -o 'nsid:[0-9].*' | cut -f2 -d:)
-	$NVME_CMD create-ns ${nvme_dev} -s ${size} -c ${size} -b ${blksize}
+	$NVME_CMD attach-ns ${nvme_dev} -n ${nsid} -c 0
-	$NVME_CMD attach-ns ${nvme_dev} -n 1 -c 0
 	$NVME_CMD reset ${nvme_dev}
 
 	$rootdir/scripts/setup.sh
@@ -81,9 +91,7 @@ function info_print() {
 }
 
 # Prepare controller
-info_print "delete all namespaces"
+remove_all_namespaces
-$NVME_CMD detach-ns ${nvme_dev} -n 0xffffffff -c 0 || true
-$NVME_CMD delete-ns ${nvme_dev} -n 0xffffffff || true
 
 reset_nvme_if_aer_unsupported ${nvme_dev}
 sleep 1
@@ -99,64 +107,52 @@ waitforlisten $spdk_tgt_pid
 $rpc_py bdev_nvme_attach_controller -b Nvme0 -t PCIe -a ${bdf}
 $rpc_py bdev_nvme_cuse_register -n Nvme0
 
-sleep 1
+ctrlr="/dev/spdk/nvme0"
-[[ -c /dev/spdk/nvme0 ]]
 
-for dev in /dev/spdk/nvme0n*; do
+sleep 1
+[[ -c $ctrlr ]]
+
+for dev in "${ctrlr}"n*; do
 	[[ ! -c ${dev} ]]
 done
-
-info_print "create ns: nsze=10000 ncap=10000 flbias=0"
-$NVME_CMD create-ns /dev/spdk/nvme0 -s 10000 -c 10000 -f 0
-
-info_print "attach ns: nsid=1 controller=0"
-$NVME_CMD attach-ns /dev/spdk/nvme0 -n 1 -c 0
-
-reset_nvme_if_aer_unsupported /dev/spdk/nvme0
 sleep 1
+nsids=()
 
-[[ -c /dev/spdk/nvme0n1 ]]
+for i in {1..2}; do
+	info_print "create ns: nsze=10000 ncap=10000 flbias=0"
+	nsid=$($NVME_CMD create-ns ${ctrlr} -s 10000 -c 10000 -f 0 | grep -o 'nsid:[0-9].*' | cut -f2 -d:)
+	nsids+=(${nsid})
+	info_print "attach ns: nsid=${nsid} controller=0"
+	$NVME_CMD attach-ns ${ctrlr} -n ${nsid} -c 0
 
-info_print "create ns: nsze=10000 ncap=10000 flbias=0"
+	reset_nvme_if_aer_unsupported ${ctrlr}
-$NVME_CMD create-ns /dev/spdk/nvme0 -s 10000 -c 10000 -f 0
+	sleep 1
 
-info_print "attach ns: nsid=2 controller=0"
+	[[ -c "${ctrlr}n${nsid}" ]]
-$NVME_CMD attach-ns /dev/spdk/nvme0 -n 2 -c 0
+done
 
-reset_nvme_if_aer_unsupported /dev/spdk/nvme0
+for n in "${nsids[@]}"; do
-sleep 1
+	info_print "detach ns: nsid=${n} controller=0"
+	$NVME_CMD detach-ns ${ctrlr} -n ${n} -c 0 || true
 
-[[ -c /dev/spdk/nvme0n2 ]]
+	info_print "delete ns: nsid=${n}"
+	$NVME_CMD delete-ns ${ctrlr} -n ${n} || true
 
-info_print "detach ns: nsid=2 controller=0"
+	reset_nvme_if_aer_unsupported ${ctrlr}
-$NVME_CMD detach-ns /dev/spdk/nvme0 -n 2 -c 0 || true
+	sleep 1
 
-info_print "delete ns: nsid=2"
+	[[ ! -c "${ctrlr}n${n}" ]]
-$NVME_CMD delete-ns /dev/spdk/nvme0 -n 2 || true
+done
-
-reset_nvme_if_aer_unsupported /dev/spdk/nvme0
-sleep 1
-
-[[ ! -c /dev/spdk/nvme0n2 ]]
-
-info_print "detach ns: nsid=1 controller=0"
-$NVME_CMD detach-ns /dev/spdk/nvme0 -n 1 -c 0 || true
-
-info_print "delete ns: nsid=1"
-$NVME_CMD delete-ns /dev/spdk/nvme0 -n 1 || true
-
-reset_nvme_if_aer_unsupported /dev/spdk/nvme0
-sleep 1
 
 # Here we should not have any cuse devices
-for dev in /dev/spdk/nvme0n*; do
+for dev in "${ctrlr}"n*; do
 	[[ ! -c ${dev} ]]
 done
 
 $rpc_py bdev_nvme_detach_controller Nvme0
 
 sleep 1
-[[ ! -c /dev/spdk/nvme0 ]]
+[[ ! -c ${ctrlr} ]]
 
 trap - SIGINT SIGTERM EXIT
 killprocess $spdk_tgt_pid