[PATCH V5 0/6] nvme-cli: Introduce nvme-status mapping with errno

[PATCH V5 0/6] nvme-cli: Introduce nvme-status mapping with errno

[Minwoo Im]

Hi,

+ The first three patches have been added in this series to make sure
the previous ununiformed return type and positions in the subcommands.
Some commands used to return the negative error values in case of
internal errors, but some didn't.  Also returning in the middle of the
subcommands are not proper for the errno-mapping which is introduced
here.  The first three patches are going to fix them first.

This patchset introduces nvme-status module to manage mapping
relationships between nvme error status and errno.  It cannot be
directly mapped in 1:1, but we can figure out what kind of errors
happended by the return value of nvme-cli.

NVMe status fields are 16bits to indicate, but UNIX return value from
main() will be parsed in 8bits so that we need to do something about
return value to indicate nvme error status.

This patch series has been tested with:
  - nvme pcie device controller/namespaces
  - nvme loop target with nvme-fabrics

The branch on github can be found at:
  https://github.com/minwooim/nvme-cli.git for-1.9/return-negative-errno-v5

Please review.
Thanks,

Changes to previous V4:
  - Add the first three patches before introducing errno mapping module.

Changes to previous V3:
  - Fix to return 0 when given error is 0 which means success. (Chaitanya)

Changes to previous V2:
  - do not overwrite the err local variable, instead returning the
    converted errno mapped directly.
  - return ECOMM in case of linux internal err which indicates the
    negative values from in the middle of the subcommand.

Changes to previous V1:
  - make switch-case inline in nvme-status (Chaitanya)

Minwoo Im (6):
  nvme: Do not return in the middle of the subcommand
  fabrics: Do not return in the middle of the subcommand
  nvme: Return negative error value for internal errors
  nvme-status: Introduce nvme status module to map errno
  nvme: Return errno mapped for nvme error status
  fabrics: Return errno mapped for fabrics error status

 Makefile      |   3 +-
 fabrics.c     |  45 ++--
 linux/nvme.h  |   6 +
 nvme-status.c | 155 +++++++++++
 nvme-status.h |  14 +
 nvme.c        | 699 ++++++++++++++++++++++++++++++--------------------
 6 files changed, 620 insertions(+), 302 deletions(-)
 create mode 100644 nvme-status.c
 create mode 100644 nvme-status.h

-- 
2.21.0

[PATCH V5 1/6] nvme: Do not return in the middle of the subcommand

[Minwoo Im]

To make nvme-cli subcommand return a mapped errno value to main(), it
should return the error status in a single place because it would be
great if the return statements and free operations are in an one shot
place.

This patch makes all the subcommands in nvme module return the error
which means internal error which should be in negative and nvme error
status which is in positive at the end of the subcommand.

Most of the changed parts are file descriptors which is returned from
parse_and_open() function.  The "fd" could be in a negative value so
that it needs to be mapped to a uniformed errno value which will be
applied by the next patches.

Cc: Keith Busch <kbusch at kernel.org>
Cc: Chaitanya Kulkarni <chaitanya.Kulkarni at wdc.com>
Signed-off-by: Minwoo Im <minwoo.im.dev at gmail.com>
---
 nvme.c | 425 +++++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 277 insertions(+), 148 deletions(-)

diff --git a/nvme.c b/nvme.c
index 9819fcb..88b2cfc 100644
--- a/nvme.c
+++ b/nvme.c
@@ -193,8 +193,10 @@ static int get_smart_log(int argc, char **argv, struct command *cmd, struct plug
 	};
 
 	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
-	if (fd < 0)
-		return fd;
+	if (fd < 0) {
+		err = fd;
+		goto ret;
+	}
 
 	fmt = validate_output_format(cfg.output_format);
 	if (fmt < 0) {
@@ -220,6 +222,7 @@ static int get_smart_log(int argc, char **argv, struct command *cmd, struct plug
  close_fd:
 	close(fd);
 
+ ret:
 	return err;
 }
 
@@ -249,8 +252,10 @@ static int get_ana_log(int argc, char **argv, struct command *cmd,
 	};
 
 	fd = parse_and_open(argc, argv, desc, command_line_options, NULL, 0);
-	if (fd < 0)
-		return fd;
+	if (fd < 0) {
+		err = fd;
+		goto ret;
+	}
 
 	fmt = validate_output_format(cfg.output_format);
 	if (fmt < 0) {
@@ -292,6 +297,7 @@ static int get_ana_log(int argc, char **argv, struct command *cmd,
 	free(ana_log);
 close_fd:
 	close(fd);
+ret:
 	return err;
 }
 
@@ -330,8 +336,10 @@ static int get_telemetry_log(int argc, char **argv, struct command *cmd, struct
 	};
 
 	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
-	if (fd < 0)
-		return fd;
+	if (fd < 0) {
+		err = fd;
+		goto ret;
+	}
 
 	if (!cfg.file_name) {
 		fprintf(stderr, "Please provide an output file!\n");
@@ -419,6 +427,7 @@ static int get_telemetry_log(int argc, char **argv, struct command *cmd, struct
 	free(page_log);
  close_fd:
 	close(fd);
+ ret:
 	return err;
 }
 
@@ -449,8 +458,10 @@ static int get_endurance_log(int argc, char **argv, struct command *cmd, struct
 	};
 
 	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
-	if (fd < 0)
-		return fd;
+	if (fd < 0) {
+		err = fd;
+		goto ret;
+	}
 
 	fmt = validate_output_format(cfg.output_format);
 	if (fmt < 0) {
@@ -473,6 +484,7 @@ static int get_endurance_log(int argc, char **argv, struct command *cmd, struct
 
  close_fd:
 	close(fd);
+ ret:
 	return err;
 }
 
@@ -505,8 +517,10 @@ static int get_effects_log(int argc, char **argv, struct command *cmd, struct pl
 	};
 
 	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
-	if (fd < 0)
-		return fd;
+	if (fd < 0) {
+		err = fd;
+		goto ret;
+	}
 
 	fmt = validate_output_format(cfg.output_format);
 	if (fmt < 0) {
@@ -535,6 +549,7 @@ static int get_effects_log(int argc, char **argv, struct command *cmd, struct pl
 
  close_fd:
 	close(fd);
+ ret:
 	return err;
 }
 
@@ -567,8 +582,10 @@ static int get_error_log(int argc, char **argv, struct command *cmd, struct plug
 	};
 
 	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
-	if (fd < 0)
-		return fd;
+	if (fd < 0) {
+		err = fd;
+		goto ret;
+	}
 
 	fmt = validate_output_format(cfg.output_format);
 	if (fmt < 0) {
@@ -619,6 +636,7 @@ static int get_error_log(int argc, char **argv, struct command *cmd, struct plug
 
  close_fd:
 	close(fd);
+ ret:
 	return err;
 }
 
@@ -646,8 +664,10 @@ static int get_fw_log(int argc, char **argv, struct command *cmd, struct plugin
 	};
 
 	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
-	if (fd < 0)
-		return fd;
+	if (fd < 0) {
+		err = fd;
+		goto ret;
+	}
 
 	fmt = validate_output_format(cfg.output_format);
 	if (fmt < 0) {
@@ -673,6 +693,7 @@ static int get_fw_log(int argc, char **argv, struct command *cmd, struct plugin
 
  close_fd:
 	close(fd);
+ ret:
 	return err;
 }
 
@@ -701,8 +722,10 @@ static int get_changed_ns_list_log(int argc, char **argv, struct command *cmd, s
 	};
 
 	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
-	if (fd < 0)
-		return fd;
+	if (fd < 0) {
+		err = fd;
+		goto ret;
+	}
 
 	fmt = validate_output_format(cfg.output_format);
 	if (fmt < 0) {
@@ -728,7 +751,7 @@ static int get_changed_ns_list_log(int argc, char **argv, struct command *cmd, s
 
  close_fd:
 	close(fd);
-
+ ret:
 	return err;
 }
 
@@ -780,8 +803,10 @@ static int get_log(int argc, char **argv, struct command *cmd, struct plugin *pl
 	};
 
 	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
-	if (fd < 0)
-		return fd;
+	if (fd < 0) {
+		err = fd;
+		goto ret;
+	}
 
 	if (cfg.aen) {
 		cfg.log_len = 4096;
@@ -828,6 +853,7 @@ static int get_log(int argc, char **argv, struct command *cmd, struct plugin *pl
 
  close_fd:
 	close(fd);
+ ret:
 	return err;
 }
 
@@ -860,8 +886,10 @@ static int sanitize_log(int argc, char **argv, struct command *command, struct p
 	};
 
 	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
-	if (fd < 0)
-		return fd;
+	if (fd < 0) {
+		ret = fd;
+		goto ret;
+	}
 
 	fmt = validate_output_format(cfg.output_format);
 	if (fmt < 0) {
@@ -890,7 +918,7 @@ static int sanitize_log(int argc, char **argv, struct command *command, struct p
 
  close_fd:
 	close(fd);
-
+ ret:
 	return ret;
 }
 
@@ -919,8 +947,10 @@ static int list_ctrl(int argc, char **argv, struct command *cmd, struct plugin *
 	};
 
 	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
-	if (fd < 0)
-		return fd;
+	if (fd < 0) {
+		err = fd;
+		goto ret;
+	}
 
 	if (posix_memalign((void *)&cntlist, getpagesize(), 0x1000)) {
 		fprintf(stderr, "can not allocate controller list payload\n");
@@ -944,7 +974,7 @@ static int list_ctrl(int argc, char **argv, struct command *cmd, struct plugin *
 
 close_fd:
 	close(fd);
-
+ret:
 	return err;
 }
 
@@ -973,8 +1003,10 @@ static int list_ns(int argc, char **argv, struct command *cmd, struct plugin *pl
 	};
 
 	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
-	if (fd < 0)
-		return fd;
+	if (fd < 0) {
+		err = fd;
+		goto ret;
+	}
 
 	err = nvme_identify_ns_list(fd, cfg.namespace_id, !!cfg.all, ns_list);
 	if (!err) {
@@ -988,7 +1020,7 @@ static int list_ns(int argc, char **argv, struct command *cmd, struct plugin *pl
 		perror("id namespace list");
 
 	close(fd);
-
+ ret:
 	return err;
 }
 
@@ -1033,8 +1065,10 @@ static int delete_ns(int argc, char **argv, struct command *cmd, struct plugin *
 	};
 
 	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
-	if (fd < 0)
-		return fd;
+	if (fd < 0) {
+		err = fd;
+		goto ret;
+	}
 
 	if (S_ISBLK(nvme_stat.st_mode)) {
 		cfg.namespace_id = get_nsid(fd);
@@ -1060,7 +1094,7 @@ static int delete_ns(int argc, char **argv, struct command *cmd, struct plugin *
 
  close_fd:
 	close(fd);
-
+ ret:
 	return err;
 }
 
@@ -1088,8 +1122,10 @@ static int nvme_attach_ns(int argc, char **argv, int attach, const char *desc, s
 	};
 
 	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
-	if (fd < 0)
-		return fd;
+	if (fd < 0) {
+		err = fd;
+		goto ret;
+	}
 
 	if (!cfg.namespace_id) {
 		fprintf(stderr, "%s: namespace-id parameter required\n",
@@ -1125,7 +1161,7 @@ static int nvme_attach_ns(int argc, char **argv, int attach, const char *desc, s
 
  close_fd:
 	close(fd);
-
+ ret:
 	return err;
 }
 
@@ -1195,8 +1231,10 @@ static int create_ns(int argc, char **argv, struct command *cmd, struct plugin *
 	};
 
 	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
-	if (fd < 0)
-		return fd;
+	if (fd < 0) {
+		err = fd;
+		goto ret;
+	}
 
 	if (cfg.flbas != 0xff && cfg.bs != 0x00) {
 		fprintf(stderr,
@@ -1253,7 +1291,7 @@ static int create_ns(int argc, char **argv, struct command *cmd, struct plugin *
 
 close_fd:
 	close(fd);
-
+ret:
 	return err;
 }
 
@@ -1691,7 +1729,7 @@ static int list_subsys(int argc, char **argv, struct command *cmd,
 
 	ret = argconfig_parse(argc, argv, desc, opts, &cfg, sizeof(cfg));
 	if (ret < 0)
-		return ret;
+		goto ret;
 
 	devicename = NULL;
 	if (optind < argc) {
@@ -1703,29 +1741,30 @@ static int list_subsys(int argc, char **argv, struct command *cmd,
 			   &cfg.namespace_id) != 2) {
 			fprintf(stderr, "%s is not a NVMe namespace device\n",
 				argv[optind]);
-			return -EINVAL;
+			ret = -EINVAL;
+			goto ret;
 		}
 		sprintf(path, "/sys/block/%s/device", devicename);
 		subsysnqn = get_nvme_subsnqn(path);
 		if (!subsysnqn) {
 			fprintf(stderr, "Cannot read subsys NQN from %s\n",
 				devicename);
-			return -EINVAL;
+			ret = -EINVAL;
+			goto ret;
 		}
 		optind++;
 	}
 
 	if (ret < 0) {
 		argconfig_print_help(desc, opts);
-		if (subsysnqn)
-			free(subsysnqn);
-		return ret;
+		goto free;
 	}
 	fmt = validate_output_format(cfg.output_format);
 	if (fmt != JSON && fmt != NORMAL) {
 		if (subsysnqn)
 			free(subsysnqn);
-		return -EINVAL;
+		ret = -EINVAL;
+		goto free;
 	}
 
 	slist = get_subsys_list(&subcnt, subsysnqn, cfg.namespace_id);
@@ -1736,8 +1775,11 @@ static int list_subsys(int argc, char **argv, struct command *cmd,
 		show_nvme_subsystem_list(slist, subcnt);
 
 	free_subsys_list(slist, subcnt);
+ free:
 	if (subsysnqn)
 		free(subsysnqn);
+
+ ret:
 	return ret;
 }
 
@@ -1809,17 +1851,20 @@ static int list(int argc, char **argv, struct command *cmd, struct plugin *plugi
 
 	ret = argconfig_parse(argc, argv, desc, opts, &cfg, sizeof(cfg));
 	if (ret < 0)
-		return ret;
+		goto ret;
 
 	fmt = validate_output_format(cfg.output_format);
 
-	if (fmt != JSON && fmt != NORMAL)
-		return -EINVAL;
+	if (fmt != JSON && fmt != NORMAL) {
+		ret = -EINVAL;
+		goto ret;
+	}
 
 	n = scandir(dev, &devices, scan_dev_filter, alphasort);
 	if (n < 0) {
 		fprintf(stderr, "no NVMe device(s) detected.\n");
-		return n;
+		ret = n;
+		goto ret;
 	}
 
 	list_items = calloc(n, sizeof(*list_items));
@@ -1867,7 +1912,7 @@ static int list(int argc, char **argv, struct command *cmd, struct plugin *plugi
 	for (i = 0; i < n; i++)
 		free(devices[i]);
 	free(devices);
-
+ ret:
 	return ret;
 }
 
@@ -1905,8 +1950,10 @@ int __id_ctrl(int argc, char **argv, struct command *cmd, struct plugin *plugin,
 	};
 
 	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
-	if (fd < 0)
-		return fd;
+	if (fd < 0) {
+		err = fd;
+		goto ret;
+	}
 
 	fmt = validate_output_format(cfg.output_format);
 	if (fmt < 0) {
@@ -1941,7 +1988,7 @@ int __id_ctrl(int argc, char **argv, struct command *cmd, struct plugin *plugin,
 
  close_fd:
 	close(fd);
-
+ ret:
 	return err;
 }
 
@@ -1978,8 +2025,10 @@ static int ns_descs(int argc, char **argv, struct command *cmd, struct plugin *p
 	};
 
 	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
-	if (fd < 0)
-		return fd;
+	if (fd < 0) {
+		err = fd;
+		goto ret;
+	}
 
 	fmt = validate_output_format(cfg.output_format);
 	if (fmt < 0) {
@@ -2022,7 +2071,7 @@ static int ns_descs(int argc, char **argv, struct command *cmd, struct plugin *p
 
  close_fd:
 	close(fd);
-
+ ret:
 	return err;
 }
 
@@ -2066,8 +2115,10 @@ static int id_ns(int argc, char **argv, struct command *cmd, struct plugin *plug
 	};
 
 	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
-	if (fd < 0)
-		return fd;
+	if (fd < 0) {
+		err = fd;
+		goto ret;
+	}
 
 	fmt = validate_output_format(cfg.output_format);
 	if (fmt < 0) {
@@ -2109,7 +2160,7 @@ static int id_ns(int argc, char **argv, struct command *cmd, struct plugin *plug
 
  close_fd:
 	close(fd);
-
+ ret:
 	return err;
 }
 
@@ -2140,8 +2191,10 @@ static int id_nvmset(int argc, char **argv, struct command *cmd, struct plugin *
 	};
 
 	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
-	if (fd < 0)
-		return fd;
+	if (fd < 0) {
+		err = fd;
+		goto ret;
+	}
 
 	fmt = validate_output_format(cfg.output_format);
 	if (fmt < 0) {
@@ -2167,13 +2220,13 @@ static int id_nvmset(int argc, char **argv, struct command *cmd, struct plugin *
 
  close_fd:
 	close(fd);
-
+ ret:
 	return err;
 }
 
 static int get_ns_id(int argc, char **argv, struct command *cmd, struct plugin *plugin)
 {
-	int nsid, fd;
+	int err = 0, nsid, fd;
 	const char *desc = "Get namespce ID of a the block device.";
 
 	const struct argconfig_commandline_options command_line_options[] = {
@@ -2181,18 +2234,23 @@ static int get_ns_id(int argc, char **argv, struct command *cmd, struct plugin *
 	};
 
 	fd = parse_and_open(argc, argv, desc, command_line_options, NULL, 0);
-	if (fd < 0)
-		return fd;
+	if (fd < 0) {
+		err = fd;
+		goto ret;
+	}
+
 	nsid = nvme_get_nsid(fd);
 	if (nsid <= 0) {
 		perror(devicename);
-		close(fd);
-		return errno;
+		err = errno;
+		goto close_fd;
 	}
 	printf("%s: namespace-id:%d\n", devicename, nsid);
 
+ close_fd:
 	close(fd);
-	return 0;
+ ret:
+	return err;
 }
 
 static int virtual_mgmt(int argc, char **argv, struct command *cmd, struct plugin *plugin)
@@ -2240,8 +2298,10 @@ static int virtual_mgmt(int argc, char **argv, struct command *cmd, struct plugi
 	};
 
 	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
-	if (fd < 0)
-		return fd;
+	if (fd < 0) {
+		err = fd;
+		goto ret;
+	}
 
 	cfg.cdw10 = cfg.cntlid << 16;
 	cfg.cdw10 = cfg.cdw10 | (cfg.rt << 8);
@@ -2256,6 +2316,7 @@ static int virtual_mgmt(int argc, char **argv, struct command *cmd, struct plugi
 		perror("virt-mgmt");
 
 	close(fd);
+ ret:
 	return err;
 
 }
@@ -2293,8 +2354,10 @@ static int list_secondary_ctrl(int argc, char **argv, struct command *cmd, struc
 	};
 
 	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
-	if (fd < 0)
-		return fd;
+	if (fd < 0) {
+		err = fd;
+		goto ret;
+	}
 
 	fmt = validate_output_format(cfg.output_format);
 	if (fmt < 0) {
@@ -2332,7 +2395,7 @@ static int list_secondary_ctrl(int argc, char **argv, struct command *cmd, struc
 
 close_fd:
 	close(fd);
-
+ret:
 	return err;
 }
 
@@ -2366,8 +2429,10 @@ static int device_self_test(int argc, char **argv, struct command *cmd, struct p
 	};
 
 	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
-	if (fd < 0)
-		return fd;
+	if (fd < 0) {
+		err = fd;
+		goto ret;
+	}
 
 	err = nvme_self_test_start(fd, cfg.namespace_id, cfg.cdw10);
 	if (!err) {
@@ -2381,6 +2446,7 @@ static int device_self_test(int argc, char **argv, struct command *cmd, struct p
 		perror("Device self-test");
 
 	close(fd);
+ ret:
 	return err;
 }
 
@@ -2406,8 +2472,10 @@ static int self_test_log(int argc, char **argv, struct command *cmd, struct plug
 	};
 
 	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
-	if (fd < 0)
-		return fd;
+	if (fd < 0) {
+		err = fd;
+		goto ret;
+	}
 
 	fmt = validate_output_format(cfg.output_format);
 	if (fmt < 0) {
@@ -2436,7 +2504,7 @@ static int self_test_log(int argc, char **argv, struct command *cmd, struct plug
 
  close_fd:
 	close(fd);
-
+ ret:
 	return err;
 }
 
@@ -2492,8 +2560,10 @@ static int get_feature(int argc, char **argv, struct command *cmd, struct plugin
 	};
 
 	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
-	if (fd < 0)
-		return fd;
+	if (fd < 0) {
+		err = fd;
+		goto ret;
+	}
 
 	if (cfg.sel > 7) {
 		fprintf(stderr, "invalid 'select' param:%d\n", cfg.sel);
@@ -2567,7 +2637,7 @@ static int get_feature(int argc, char **argv, struct command *cmd, struct plugin
 
  close_fd:
 	close(fd);
-
+ ret:
 	return err;
 }
 
@@ -2609,8 +2679,10 @@ static int fw_download(int argc, char **argv, struct command *cmd, struct plugin
 	};
 
 	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
-	if (fd < 0)
-		return fd;
+	if (fd < 0) {
+		err = fd;
+		goto ret;
+	}
 
 	fw_fd = open(cfg.fw, O_RDONLY);
 	cfg.offset <<= 2;
@@ -2672,7 +2744,7 @@ static int fw_download(int argc, char **argv, struct command *cmd, struct plugin
 	close(fw_fd);
  close_fd:
 	close(fd);
-
+ ret:
 	return err;
 }
 
@@ -2718,8 +2790,10 @@ static int fw_commit(int argc, char **argv, struct command *cmd, struct plugin *
 	};
 
 	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
-	if (fd < 0)
-		return fd;
+	if (fd < 0) {
+		err = fd;
+		goto ret;
+	}
 
 	if (cfg.slot > 7) {
 		fprintf(stderr, "invalid slot:%d\n", cfg.slot);
@@ -2765,7 +2839,7 @@ static int fw_commit(int argc, char **argv, struct command *cmd, struct plugin *
 
  close_fd:
 	close(fd);
-
+ ret:
 	return err;
 }
 
@@ -2779,21 +2853,22 @@ static int subsystem_reset(int argc, char **argv, struct command *cmd, struct pl
 	};
 
 	fd = parse_and_open(argc, argv, desc, command_line_options, NULL, 0);
-	if (fd < 0)
-		return fd;
+	if (fd < 0) {
+		err = fd;
+		goto ret;
+	}
 
 	err = nvme_subsystem_reset(fd);
 	if (err < 0) {
-		close(fd);
 		if (errno == ENOTTY)
 			fprintf(stderr,
 				"Subsystem-reset: NVM Subsystem Reset not supported.\n");
 		else
 			perror("Subsystem-reset");
-		return errno;
 	}
 
 	close(fd);
+ ret:
 	return err;
 }
 
@@ -2807,17 +2882,17 @@ static int reset(int argc, char **argv, struct command *cmd, struct plugin *plug
 	};
 
 	fd = parse_and_open(argc, argv, desc, command_line_options, NULL, 0);
-	if (fd < 0)
-		return fd;
+	if (fd < 0) {
+		err = fd;
+		goto ret;
+	}
 
 	err = nvme_reset_controller(fd);
-	if (err < 0) {
-		close(fd);
+	if (err < 0)
 		perror("Reset");
-		return errno;
-	}
 
 	close(fd);
+ ret:
 	return err;
 }
 
@@ -2831,17 +2906,17 @@ static int ns_rescan(int argc, char **argv, struct command *cmd, struct plugin *
 	};
 
 	fd = parse_and_open(argc, argv, desc, command_line_options, NULL, 0);
-	if (fd < 0)
-		return fd;
+	if (fd < 0) {
+		err = fd;
+		goto ret;
+	}
 
 	err = nvme_ns_rescan(fd);
-	if (err < 0) {
-		close(fd);
+	if (err < 0)
 		perror("Namespace Rescan");
-		return errno;
-	}
 
 	close(fd);
+ ret:
 	return err;
 }
 
@@ -2887,8 +2962,10 @@ static int sanitize(int argc, char **argv, struct command *cmd, struct plugin *p
 	};
 
 	fd = parse_and_open(argc, argv, desc, command_line_options, NULL, 0);
-	if (fd < 0)
-		return fd;
+	if (fd < 0) {
+		ret = fd;
+		goto ret;
+	}
 
 	switch (cfg.sanact) {
 	case NVME_SANITIZE_ACT_CRYPTO_ERASE:
@@ -2933,7 +3010,7 @@ static int sanitize(int argc, char **argv, struct command *cmd, struct plugin *p
 
  close_fd:
 	close(fd);
-
+ ret:
 	return ret;
 }
 
@@ -2965,8 +3042,10 @@ static int show_registers(int argc, char **argv, struct command *cmd, struct plu
 	};
 
 	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
-	if (fd < 0)
-		return fd;
+	if (fd < 0) {
+		err = fd;
+		goto ret;
+	}
 
 	fmt = validate_output_format(cfg.output_format);
 	if (fmt < 0) {
@@ -3006,7 +3085,7 @@ static int show_registers(int argc, char **argv, struct command *cmd, struct plu
 
  close_fd:
 	close(fd);
-
+ ret:
 	return err;
 }
 
@@ -3038,8 +3117,10 @@ static int get_property(int argc, char **argv, struct command *cmd, struct plugi
 	};
 
 	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
-	if (fd < 0)
-		return fd;
+	if (fd < 0) {
+		err = fd;
+		goto ret;
+	}
 
 	if (cfg.offset == -1) {
 		fprintf(stderr, "offset required param");
@@ -3058,7 +3139,7 @@ static int get_property(int argc, char **argv, struct command *cmd, struct plugi
 
  close_fd:
 	close(fd);
-
+ ret:
 	return err;
 }
 
@@ -3087,8 +3168,10 @@ static int set_property(int argc, char **argv, struct command *cmd, struct plugi
 	};
 
 	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
-	if (fd < 0)
-		return fd;
+	if (fd < 0) {
+		err = fd;
+		goto ret;
+	}
 
 	if (cfg.offset == -1) {
 		fprintf(stderr, "offset required param");
@@ -3113,7 +3196,7 @@ static int set_property(int argc, char **argv, struct command *cmd, struct plugi
 
  close_fd:
 	close(fd);
-
+ ret:
 	return err;
 }
 
@@ -3173,8 +3256,10 @@ static int format(int argc, char **argv, struct command *cmd, struct plugin *plu
 	};
 
 	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
-	if (fd < 0)
-		return fd;
+	if (fd < 0) {
+		err = fd;
+		goto ret;
+	}
 
 	if (cfg.lbaf != 0xff && cfg.bs !=0) {
 		fprintf(stderr,
@@ -3274,7 +3359,7 @@ static int format(int argc, char **argv, struct command *cmd, struct plugin *plu
 
  close_fd:
 	close(fd);
-
+ ret:
 	return err;
 }
 
@@ -3332,8 +3417,10 @@ static int set_feature(int argc, char **argv, struct command *cmd, struct plugin
 	};
 
 	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
-	if (fd < 0)
-		return fd;
+	if (fd < 0) {
+		err = fd;
+		goto ret;
+	}
 
 	if (!cfg.feature_id) {
 		fprintf(stderr, "feature-id required param\n");
@@ -3394,6 +3481,7 @@ static int set_feature(int argc, char **argv, struct command *cmd, struct plugin
 		free(buf);
  close_fd:
 	close(fd);
+ ret:
 	return err;
 }
 
@@ -3443,8 +3531,10 @@ static int sec_send(int argc, char **argv, struct command *cmd, struct plugin *p
 	};
 
 	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
-	if (fd < 0)
-		return fd;
+	if (fd < 0) {
+		err = fd;
+		goto ret;
+	}
 
 	sec_fd = open(cfg.file, O_RDONLY);
 	if (sec_fd < 0) {
@@ -3490,6 +3580,7 @@ static int sec_send(int argc, char **argv, struct command *cmd, struct plugin *p
 	close(sec_fd);
  close_fd:
 	close(fd);
+ ret:
 	return err;
 }
 
@@ -3550,8 +3641,10 @@ static int dir_send(int argc, char **argv, struct command *cmd, struct plugin *p
 	};
 
 	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
-	if (fd < 0)
-		return fd;
+	if (fd < 0) {
+		err = fd;
+		goto ret;
+	}
 
 	switch (cfg.dtype) {
 	case NVME_DIR_IDENTIFY:
@@ -3641,6 +3734,7 @@ free:
 		free(buf);
 close_fd:
 	close(fd);
+ret:
 	return err;
 }
 
@@ -3673,8 +3767,10 @@ static int write_uncor(int argc, char **argv, struct command *cmd, struct plugin
 	};
 
 	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
-	if (fd < 0)
-		return fd;
+	if (fd < 0) {
+		err = fd;
+		goto ret;
+	}
 
 	if (!cfg.namespace_id) {
 		cfg.namespace_id = get_nsid(fd);
@@ -3695,7 +3791,7 @@ static int write_uncor(int argc, char **argv, struct command *cmd, struct plugin
 
 close_fd:
 	close(fd);
-
+ret:
 	return err;
 }
 
@@ -3753,8 +3849,10 @@ static int write_zeroes(int argc, char **argv, struct command *cmd, struct plugi
 	};
 
 	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
-	if (fd < 0)
-		return fd;
+	if (fd < 0) {
+		err = fd;
+		goto ret;
+	}
 
 	if (cfg.prinfo > 0xf) {
 		err = EINVAL;
@@ -3787,6 +3885,7 @@ static int write_zeroes(int argc, char **argv, struct command *cmd, struct plugi
 
  close_fd:
 	close(fd);
+ ret:
 	return err;
 }
 
@@ -3847,8 +3946,10 @@ static int dsm(int argc, char **argv, struct command *cmd, struct plugin *plugin
 	};
 
 	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
-	if (fd < 0)
-		return fd;
+	if (fd < 0) {
+		err = fd;
+		goto ret;
+	}
 
 	nc = argconfig_parse_comma_sep_array(cfg.ctx_attrs, ctx_attrs, ARRAY_SIZE(ctx_attrs));
 	nb = argconfig_parse_comma_sep_array(cfg.blocks, nlbs, ARRAY_SIZE(nlbs));
@@ -3887,6 +3988,7 @@ static int dsm(int argc, char **argv, struct command *cmd, struct plugin *plugin
 
  close_fd:
 	close(fd);
+ ret:
 	return err;
 }
 
@@ -3914,8 +4016,10 @@ static int flush(int argc, char **argv, struct command *cmd, struct plugin *plug
 	};
 
 	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
-	if (fd < 0)
-		return fd;
+	if (fd < 0) {
+		err = fd;
+		goto ret;
+	}
 
 	if (S_ISBLK(nvme_stat.st_mode)) {
 		cfg.namespace_id = get_nsid(fd);
@@ -3934,6 +4038,7 @@ static int flush(int argc, char **argv, struct command *cmd, struct plugin *plug
 		printf("NVMe Flush: success\n");
 close_fd:
 	close(fd);
+ret:
 	return err;
 }
 
@@ -3981,8 +4086,10 @@ static int resv_acquire(int argc, char **argv, struct command *cmd, struct plugi
 	};
 
 	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
-	if (fd < 0)
-		return fd;
+	if (fd < 0) {
+		err = fd;
+		goto ret;
+	}
 
 	if (!cfg.namespace_id) {
 		cfg.namespace_id = get_nsid(fd);
@@ -4008,6 +4115,7 @@ static int resv_acquire(int argc, char **argv, struct command *cmd, struct plugi
 
  close_fd:
 	close(fd);
+ ret:
 	return err;
 }
 
@@ -4052,8 +4160,10 @@ static int resv_register(int argc, char **argv, struct command *cmd, struct plug
 	};
 
 	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
-	if (fd < 0)
-		return fd;
+	if (fd < 0) {
+		err = fd;
+		goto ret;
+	}
 
 	if (!cfg.namespace_id) {
 		cfg.namespace_id = get_nsid(fd);
@@ -4085,6 +4195,7 @@ static int resv_register(int argc, char **argv, struct command *cmd, struct plug
 
  close_fd:
 	close(fd);
+ ret:
 	return err;
 }
 
@@ -4131,8 +4242,10 @@ static int resv_release(int argc, char **argv, struct command *cmd, struct plugi
 	};
 
 	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
-	if (fd < 0)
-		return fd;
+	if (fd < 0) {
+		err = fd;
+		goto ret;
+	}
 
 	if (!cfg.namespace_id) {
 		cfg.namespace_id = get_nsid(fd);
@@ -4158,6 +4271,7 @@ static int resv_release(int argc, char **argv, struct command *cmd, struct plugi
 
  close_fd:
 	close(fd);
+ ret:
 	return err;
 }
 
@@ -4201,8 +4315,10 @@ static int resv_report(int argc, char **argv, struct command *cmd, struct plugin
 	};
 
 	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
-	if (fd < 0)
-		return fd;
+	if (fd < 0) {
+		err = fd;
+		goto ret;
+	}
 
 	fmt = validate_output_format(cfg.output_format);
 	if (fmt < 0) {
@@ -4252,6 +4368,7 @@ static int resv_report(int argc, char **argv, struct command *cmd, struct plugin
 
  close_fd:
 	close(fd);
+ ret:
 	return err;
 }
 
@@ -4345,8 +4462,10 @@ static int submit_io(int opcode, char *command, const char *desc,
 	};
 
 	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
-	if (fd < 0)
-		return fd;
+	if (fd < 0) {
+		err = fd;
+		goto ret;
+	}
 
 	dfd = mfd = opcode & 1 ? STDIN_FILENO : STDOUT_FILENO;
 	if (cfg.prinfo > 0xf) {
@@ -4497,6 +4616,7 @@ static int submit_io(int opcode, char *command, const char *desc,
 	close(dfd);
  close_fd:
 	close(fd);
+ ret:
 	return err;
 }
 
@@ -4571,8 +4691,10 @@ static int sec_recv(int argc, char **argv, struct command *cmd, struct plugin *p
 	};
 
 	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
-	if (fd < 0)
-		return fd;
+	if (fd < 0) {
+		err = fd;
+		goto ret;
+	}
 
 	if (cfg.size) {
 		if (posix_memalign(&sec_buf, getpagesize(), cfg.size)) {
@@ -4603,6 +4725,7 @@ static int sec_recv(int argc, char **argv, struct command *cmd, struct plugin *p
 
  close_fd:
 	close(fd);
+ ret:
 	return err;
 }
 
@@ -4656,8 +4779,10 @@ static int dir_receive(int argc, char **argv, struct command *cmd, struct plugin
 	};
 
 	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
-	if (fd < 0)
-		return fd;
+	if (fd < 0) {
+		err = fd;
+		goto ret;
+	}
 
 	switch (cfg.dtype) {
 	case NVME_DIR_IDENTIFY:
@@ -4733,6 +4858,7 @@ free:
 		free(buf);
 close_fd:
 	close(fd);
+ret:
 	return err;
 }
 
@@ -4837,8 +4963,10 @@ static int passthru(int argc, char **argv, int ioctl_cmd, const char *desc, stru
 	};
 
 	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
-	if (fd < 0)
-		return fd;
+	if (fd < 0) {
+		err = fd;
+		goto ret;
+	}
 
 	if (strlen(cfg.input_file)){
 		wfd = open(cfg.input_file, O_RDONLY,
@@ -4933,6 +5061,7 @@ static int passthru(int argc, char **argv, int ioctl_cmd, const char *desc, stru
 		close(wfd);
  close_fd:
 	close(fd);
+ ret:
 	return err;
 }
 
-- 
2.21.0

[PATCH V5 2/6] fabrics: Do not return in the middle of the subcommand

[Minwoo Im]

This patch also makes fabrics module to not return the internal error
status in the middle of the subcommands to support uniformed mapped
error value which will be introduced in the next patches.

Cc: Keith Busch <kbusch at kernel.org>
Cc: Chaitanya Kulkarni <chaitanya.Kulkarni at wdc.com>
Signed-off-by: Minwoo Im <minwoo.im.dev at gmail.com>
---
 fabrics.c | 31 +++++++++++++++++++------------
 1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/fabrics.c b/fabrics.c
index 511de06..469b4a1 100644
--- a/fabrics.c
+++ b/fabrics.c
@@ -984,20 +984,23 @@ int discover(const char *desc, int argc, char **argv, bool connect)
 	ret = argconfig_parse(argc, argv, desc, command_line_options, &cfg,
 			sizeof(cfg));
 	if (ret)
-		return ret;
+		goto ret;
 
 	cfg.nqn = NVME_DISC_SUBSYS_NAME;
 
 	if (!cfg.transport && !cfg.traddr) {
-		return discover_from_conf_file(desc, argstr,
+		ret = discover_from_conf_file(desc, argstr,
 				command_line_options, connect);
 	} else {
 		ret = build_options(argstr, BUF_SIZE);
 		if (ret)
-			return ret;
+			goto ret;
 
-		return do_discover(argstr, connect);
+		ret = do_discover(argstr, connect);
 	}
+
+ ret:
+	return ret;
 }
 
 int connect(const char *desc, int argc, char **argv)
@@ -1029,21 +1032,23 @@ int connect(const char *desc, int argc, char **argv)
 	ret = argconfig_parse(argc, argv, desc, command_line_options, &cfg,
 			sizeof(cfg));
 	if (ret)
-		return ret;
+		goto ret;
 
 	ret = build_options(argstr, BUF_SIZE);
 	if (ret)
-		return ret;
+		goto ret;
 
 	if (!cfg.nqn) {
 		fprintf(stderr, "need a -n argument\n");
-		return -EINVAL;
+		ret = -EINVAL;
+		goto ret;
 	}
 
 	instance = add_ctrl(argstr);
 	if (instance < 0)
-		return instance;
-	return 0;
+		ret = instance;
+ ret:
+	return ret;
 }
 
 static int scan_sys_nvme_filter(const struct dirent *d)
@@ -1148,11 +1153,12 @@ int disconnect(const char *desc, int argc, char **argv)
 	ret = argconfig_parse(argc, argv, desc, command_line_options, &cfg,
 			sizeof(cfg));
 	if (ret)
-		return ret;
+		goto ret;
 
 	if (!cfg.nqn && !cfg.device) {
 		fprintf(stderr, "need a -n or -d argument\n");
-		return -EINVAL;
+		ret = -EINVAL;
+		goto ret;
 	}
 
 	if (cfg.nqn) {
@@ -1174,6 +1180,7 @@ int disconnect(const char *desc, int argc, char **argv)
 				cfg.device);
 	}
 
+ ret:
 	return ret;
 }
 
@@ -1188,7 +1195,7 @@ int disconnect_all(const char *desc, int argc, char **argv)
 	ret = argconfig_parse(argc, argv, desc, command_line_options, &cfg,
 			sizeof(cfg));
 	if (ret)
-		return ret;
+		goto out;
 
 	slist = get_subsys_list(&subcnt, NULL, NVME_NSID_ALL);
 	for (i = 0; i < subcnt; i++) {
-- 
2.21.0

[PATCH V5 3/6] nvme: Return negative error value for internal errors

[Minwoo Im]

To support the mapping of error values which will be introduced in the
next patches, All the internal errors caused in the middle of the
subcommands preparation or teardown should be returned as a negative
value to make it distinguished from nvme error status which is positive.

This patch makes all the internal error values which used to be returned
as a positive value to be returned as a negative value.

Cc: Keith Busch <kbusch at kernel.org>
Cc: Chaitanya Kulkarni <chaitanya.Kulkarni at wdc.com>
Signed-off-by: Minwoo Im <minwoo.im.dev at gmail.com>
---
 nvme.c | 170 ++++++++++++++++++++++++++++-----------------------------
 1 file changed, 85 insertions(+), 85 deletions(-)

diff --git a/nvme.c b/nvme.c
index 88b2cfc..a8d382a 100644
--- a/nvme.c
+++ b/nvme.c
@@ -343,7 +343,7 @@ static int get_telemetry_log(int argc, char **argv, struct command *cmd, struct
 
 	if (!cfg.file_name) {
 		fprintf(stderr, "Please provide an output file!\n");
-		err = EINVAL;
+		err = -EINVAL;
 		goto close_fd;
 	}
 
@@ -353,7 +353,7 @@ static int get_telemetry_log(int argc, char **argv, struct command *cmd, struct
 	if (!hdr || !page_log) {
 		fprintf(stderr, "Failed to allocate %zu bytes for log: %s\n",
 				bs, strerror(errno));
-		err = ENOMEM;
+		err = -ENOMEM;
 		goto free_mem;
 	}
 	memset(hdr, 0, bs);
@@ -393,7 +393,7 @@ static int get_telemetry_log(int argc, char **argv, struct command *cmd, struct
 		break;
 	default:
 		fprintf(stderr, "Invalid data area requested");
-		err = EINVAL;
+		err = -EINVAL;
 		goto close_output;
 	}
 
@@ -597,7 +597,7 @@ static int get_error_log(int argc, char **argv, struct command *cmd, struct plug
 
 	if (!cfg.log_entries) {
 		fprintf(stderr, "non-zero log-entries is required param\n");
-		err = EINVAL;
+		err = -EINVAL;
 		goto close_fd;
 	}
 
@@ -606,7 +606,7 @@ static int get_error_log(int argc, char **argv, struct command *cmd, struct plug
 		perror("identify controller");
 	else if (err) {
 		fprintf(stderr, "could not identify controller\n");
-		err = ENODEV;
+		err = -ENODEV;
 	} else {
 		struct nvme_error_log_page *err_log;
 
@@ -614,7 +614,7 @@ static int get_error_log(int argc, char **argv, struct command *cmd, struct plug
 		err_log = calloc(cfg.log_entries, sizeof(struct nvme_error_log_page));
 		if (!err_log) {
 			fprintf(stderr, "could not alloc buffer for error log\n");
-			err = ENOMEM;
+			err = -ENOMEM;
 			goto close_fd;
 		}
 
@@ -815,13 +815,13 @@ static int get_log(int argc, char **argv, struct command *cmd, struct plugin *pl
 
 	if (cfg.log_id > 0xff) {
 		fprintf(stderr, "Invalid log identifier: %d. Valid range: 0-255\n", cfg.log_id);
-		err = EINVAL;
+		err = -EINVAL;
 		goto close_fd;
 	}
 
 	if (!cfg.log_len) {
 		fprintf(stderr, "non-zero log-len is required param\n");
-		err = EINVAL;
+		err = -EINVAL;
 	} else {
 		unsigned char *log;
 
@@ -829,7 +829,7 @@ static int get_log(int argc, char **argv, struct command *cmd, struct plugin *pl
 		if (!log) {
 			fprintf(stderr, "could not alloc buffer for log: %s\n",
 					strerror(errno));
-			err = EINVAL;
+			err = -EINVAL;
 			goto close_fd;
 		}
 
@@ -1073,13 +1073,13 @@ static int delete_ns(int argc, char **argv, struct command *cmd, struct plugin *
 	if (S_ISBLK(nvme_stat.st_mode)) {
 		cfg.namespace_id = get_nsid(fd);
 		if (cfg.namespace_id == 0) {
-			err = EINVAL;
+			err = -EINVAL;
 			goto close_fd;
 		}
 	} else if (!cfg.namespace_id) {
 		fprintf(stderr, "%s: namespace-id parameter required\n",
 						cmd->name);
-		err = EINVAL;
+		err = -EINVAL;
 		goto close_fd;
 	}
 
@@ -1130,7 +1130,7 @@ static int nvme_attach_ns(int argc, char **argv, int attach, const char *desc, s
 	if (!cfg.namespace_id) {
 		fprintf(stderr, "%s: namespace-id parameter required\n",
 						cmd->name);
-		err = EINVAL;
+		err = -EINVAL;
 		goto close_fd;
 	}
 
@@ -1140,7 +1140,7 @@ static int nvme_attach_ns(int argc, char **argv, int attach, const char *desc, s
     if (num == -1) {
 		fprintf(stderr, "%s: controller id list is required\n",
 						cmd->name);
-		err = EINVAL;
+		err = -EINVAL;
 		goto close_fd;
     }
 
@@ -2040,14 +2040,14 @@ static int ns_descs(int argc, char **argv, struct command *cmd, struct plugin *p
 	if (!cfg.namespace_id) {
 		cfg.namespace_id = get_nsid(fd);
 		if (cfg.namespace_id == 0) {
-			err = EINVAL;
+			err = -EINVAL;
 			goto close_fd;
 		}
 	}
 
 	if (posix_memalign(&nsdescs, getpagesize(), 0x1000)) {
 		fprintf(stderr, "can not allocate controller list payload\n");
-		err = ENOMEM;
+		err = -ENOMEM;
 		goto close_fd;
 	}
 
@@ -2134,7 +2134,7 @@ static int id_ns(int argc, char **argv, struct command *cmd, struct plugin *plug
 	if (!cfg.namespace_id && S_ISBLK(nvme_stat.st_mode)) {
 		cfg.namespace_id = get_nsid(fd);
 		if (cfg.namespace_id == 0) {
-			err = EINVAL;
+			err = -EINVAL;
 			goto close_fd;
 		}
 	}
@@ -2567,12 +2567,12 @@ static int get_feature(int argc, char **argv, struct command *cmd, struct plugin
 
 	if (cfg.sel > 7) {
 		fprintf(stderr, "invalid 'select' param:%d\n", cfg.sel);
-		err = EINVAL;
+		err = -EINVAL;
 		goto close_fd;
 	}
 	if (!cfg.feature_id) {
 		fprintf(stderr, "feature-id required param\n");
-		err = EINVAL;
+		err = -EINVAL;
 		goto close_fd;
 	}
 
@@ -2606,7 +2606,7 @@ static int get_feature(int argc, char **argv, struct command *cmd, struct plugin
 	if (cfg.data_len) {
 		if (posix_memalign(&buf, getpagesize(), cfg.data_len)) {
 			fprintf(stderr, "can not allocate feature payload\n");
-			err = ENOMEM;
+			err = -ENOMEM;
 			goto close_fd;
 		}
 		memset(buf, 0, cfg.data_len);
@@ -2689,7 +2689,7 @@ static int fw_download(int argc, char **argv, struct command *cmd, struct plugin
 	if (fw_fd < 0) {
 		fprintf(stderr, "Failed to open firmware file %s: %s\n",
 				cfg.fw, strerror(errno));
-		err = EINVAL;
+		err = -EINVAL;
 		goto close_fd;
 	}
 
@@ -2702,12 +2702,12 @@ static int fw_download(int argc, char **argv, struct command *cmd, struct plugin
 	fw_size = sb.st_size;
 	if (fw_size & 0x3) {
 		fprintf(stderr, "Invalid size:%d for f/w image\n", fw_size);
-		err = EINVAL;
+		err = -EINVAL;
 		goto close_fw_fd;
 	}
 	if (posix_memalign(&fw_buf, getpagesize(), fw_size)) {
 		fprintf(stderr, "No memory for f/w size:%d\n", fw_size);
-		err = ENOMEM;
+		err = -ENOMEM;
 		goto close_fw_fd;
 	}
 
@@ -2797,17 +2797,17 @@ static int fw_commit(int argc, char **argv, struct command *cmd, struct plugin *
 
 	if (cfg.slot > 7) {
 		fprintf(stderr, "invalid slot:%d\n", cfg.slot);
-		err = EINVAL;
+		err = -EINVAL;
 		goto close_fd;
 	}
 	if (cfg.action > 7 || cfg.action == 4 || cfg.action == 5) {
 		fprintf(stderr, "invalid action:%d\n", cfg.action);
-		err = EINVAL;
+		err = -EINVAL;
 		goto close_fd;
 	}
 	if (cfg.bpid > 1) {
 		fprintf(stderr, "invalid boot partition id:%d\n", cfg.bpid);
-		err = EINVAL;
+		err = -EINVAL;
 		goto close_fd;
 	}
 
@@ -2975,14 +2975,14 @@ static int sanitize(int argc, char **argv, struct command *cmd, struct plugin *p
 		break;
 	default:
 		fprintf(stderr, "Invalid Sanitize Action\n");
-		ret = EINVAL;
+		ret = -EINVAL;
 		goto close_fd;
 	}
 
 	if (cfg.sanact == NVME_SANITIZE_ACT_EXIT) {
 	       if (cfg.ause || cfg.no_dealloc) {
 			fprintf(stderr, "SANACT is Exit Failure Mode\n");
-			ret = EINVAL;
+			ret = -EINVAL;
 			goto close_fd;
 	       }
 	}
@@ -2990,13 +2990,13 @@ static int sanitize(int argc, char **argv, struct command *cmd, struct plugin *p
 	if (cfg.sanact == NVME_SANITIZE_ACT_OVERWRITE) {
 		if (cfg.owpass > 16) {
 			fprintf(stderr, "OWPASS out of range [0-16]\n");
-			ret = EINVAL;
+			ret = -EINVAL;
 			goto close_fd;
 		}
 	} else {
 		if (cfg.owpass || cfg.oipbp || cfg.ovrpat) {
 			fprintf(stderr, "SANACT is not Overwrite\n");
-			ret = EINVAL;
+			ret = -EINVAL;
 			goto close_fd;
 		}
 	}
@@ -3057,7 +3057,7 @@ static int show_registers(int argc, char **argv, struct command *cmd, struct plu
 
 	if (cfg.human_readable && fmt != NORMAL) {
 		fprintf(stderr, "Only --output-format=normal supports -H\n");
-		err = EINVAL;
+		err = -EINVAL;
 		goto close_fd;
 	}
 
@@ -3069,7 +3069,7 @@ static int show_registers(int argc, char **argv, struct command *cmd, struct plu
 			err = 0;
 	}
 	if (!bar) {
-		err = ENODEV;
+		err = -ENODEV;
 		goto close_fd;
 	}
 
@@ -3124,7 +3124,7 @@ static int get_property(int argc, char **argv, struct command *cmd, struct plugi
 
 	if (cfg.offset == -1) {
 		fprintf(stderr, "offset required param");
-		err = EINVAL;
+		err = -EINVAL;
 		goto close_fd;
 	}
 
@@ -3175,12 +3175,12 @@ static int set_property(int argc, char **argv, struct command *cmd, struct plugi
 
 	if (cfg.offset == -1) {
 		fprintf(stderr, "offset required param");
-		err = EINVAL;
+		err = -EINVAL;
 		goto close_fd;
 	}
 	if (cfg.value == -1) {
 		fprintf(stderr, "value required param");
-		err = EINVAL;
+		err = -EINVAL;
 		goto close_fd;
 	}
 
@@ -3279,7 +3279,7 @@ static int format(int argc, char **argv, struct command *cmd, struct plugin *plu
 	if (S_ISBLK(nvme_stat.st_mode)) {
 		cfg.namespace_id = get_nsid(fd);
 		if (cfg.namespace_id == 0) {
-			err = EINVAL;
+			err = -EINVAL;
 			goto close_fd;
 		}
 	}
@@ -3320,27 +3320,27 @@ static int format(int argc, char **argv, struct command *cmd, struct plugin *plu
 	/* ses & pi checks set to 7 for forward-compatibility */
 	if (cfg.ses > 7) {
 		fprintf(stderr, "invalid secure erase settings:%d\n", cfg.ses);
-		err = EINVAL;
+		err = -EINVAL;
 		goto close_fd;
 	}
 	if (cfg.lbaf > 15) {
 		fprintf(stderr, "invalid lbaf:%d\n", cfg.lbaf);
-		err = EINVAL;
+		err = -EINVAL;
 		goto close_fd;
 	}
 	if (cfg.pi > 7) {
 		fprintf(stderr, "invalid pi:%d\n", cfg.pi);
-		err = EINVAL;
+		err = -EINVAL;
 		goto close_fd;
 	}
 	if (cfg.pil > 1) {
 		fprintf(stderr, "invalid pil:%d\n", cfg.pil);
-		err = EINVAL;
+		err = -EINVAL;
 		goto close_fd;
 	}
 	if (cfg.ms > 1) {
 		fprintf(stderr, "invalid ms:%d\n", cfg.ms);
-		err = EINVAL;
+		err = -EINVAL;
 		goto close_fd;
 	}
 
@@ -3424,7 +3424,7 @@ static int set_feature(int argc, char **argv, struct command *cmd, struct plugin
 
 	if (!cfg.feature_id) {
 		fprintf(stderr, "feature-id required param\n");
-		err = EINVAL;
+		err = -EINVAL;
 		goto close_fd;
 	}
 	if (cfg.feature_id == NVME_FEAT_LBA_RANGE)
@@ -3432,7 +3432,7 @@ static int set_feature(int argc, char **argv, struct command *cmd, struct plugin
 	if (cfg.data_len) {
 		if (posix_memalign(&buf, getpagesize(), cfg.data_len)) {
 			fprintf(stderr, "can not allocate feature payload\n");
-			err = ENOMEM;
+			err = -ENOMEM;
 			goto close_fd;
 		}
 		memset(buf, 0, cfg.data_len);
@@ -3444,7 +3444,7 @@ static int set_feature(int argc, char **argv, struct command *cmd, struct plugin
 			if (ffd <= 0) {
 				fprintf(stderr, "Failed to open file %s: %s\n",
 						cfg.file, strerror(errno));
-				err = EINVAL;
+				err = -EINVAL;
 				goto free;
 			}
 		}
@@ -3540,7 +3540,7 @@ static int sec_send(int argc, char **argv, struct command *cmd, struct plugin *p
 	if (sec_fd < 0) {
 		fprintf(stderr, "Failed to open %s: %s\n",
 				cfg.file, strerror(errno));
-		err = EINVAL;
+		err = -EINVAL;
 		goto close_fd;
 	}
 
@@ -3553,7 +3553,7 @@ static int sec_send(int argc, char **argv, struct command *cmd, struct plugin *p
 	sec_size = sb.st_size;
 	if (posix_memalign(&sec_buf, getpagesize(), sec_size)) {
 		fprintf(stderr, "No memory for security size:%d\n", sec_size);
-		err = ENOMEM;
+		err = -ENOMEM;
 		goto close_sec_fd;
 	}
 
@@ -3652,14 +3652,14 @@ static int dir_send(int argc, char **argv, struct command *cmd, struct plugin *p
 		case NVME_DIR_SND_ID_OP_ENABLE:
 			if (!cfg.ttype) {
 				fprintf(stderr, "target-dir required param\n");
-				err = EINVAL;
+				err = -EINVAL;
 				goto close_fd;
 			}
 			dw12 = cfg.ttype << 8 | cfg.endir;
 			break;
 		default:
 			fprintf(stderr, "invalid directive operations for Identify Directives\n");
-			err = EINVAL;
+			err = -EINVAL;
 			goto close_fd;
 		}
 		break;
@@ -3670,20 +3670,20 @@ static int dir_send(int argc, char **argv, struct command *cmd, struct plugin *p
 			break;
 		default:
 			fprintf(stderr, "invalid directive operations for Streams Directives\n");
-			err = EINVAL;
+			err = -EINVAL;
 			goto close_fd;
 		}
 		break;
 	default:
 		fprintf(stderr, "invalid directive type\n");
-		err = EINVAL;
+		err = -EINVAL;
 		goto close_fd;
 	}
 
 
 	if (cfg.data_len) {
 		if (posix_memalign(&buf, getpagesize(), cfg.data_len)) {
-			err = ENOMEM;
+			err = -ENOMEM;
 			goto close_fd;
 		}
 		memset(buf, 0, cfg.data_len);
@@ -3695,7 +3695,7 @@ static int dir_send(int argc, char **argv, struct command *cmd, struct plugin *p
 			if (ffd <= 0) {
 				fprintf(stderr, "Failed to open file %s: %s\n",
 						cfg.file, strerror(errno));
-				err = EINVAL;
+				err = -EINVAL;
 				goto free;
 			}
 		}
@@ -3775,7 +3775,7 @@ static int write_uncor(int argc, char **argv, struct command *cmd, struct plugin
 	if (!cfg.namespace_id) {
 		cfg.namespace_id = get_nsid(fd);
 		if (cfg.namespace_id == 0) {
-			err = EINVAL;
+			err = -EINVAL;
 			goto close_fd;
 		}
 	}
@@ -3855,7 +3855,7 @@ static int write_zeroes(int argc, char **argv, struct command *cmd, struct plugi
 	}
 
 	if (cfg.prinfo > 0xf) {
-		err = EINVAL;
+		err = -EINVAL;
 		goto close_fd;
 	}
 
@@ -3869,7 +3869,7 @@ static int write_zeroes(int argc, char **argv, struct command *cmd, struct plugi
 	if (!cfg.namespace_id) {
 		cfg.namespace_id = get_nsid(fd);
 		if (cfg.namespace_id == 0) {
-			err = EINVAL;
+			err = -EINVAL;
 			goto close_fd;
 		}
 	}
@@ -3957,14 +3957,14 @@ static int dsm(int argc, char **argv, struct command *cmd, struct plugin *plugin
 	nr = max(nc, max(nb, ns));
 	if (!nr || nr > 256) {
 		fprintf(stderr, "No range definition provided\n");
-		err = EINVAL;
+		err = -EINVAL;
 		goto close_fd;
 	}
 
 	if (!cfg.namespace_id) {
 		cfg.namespace_id = get_nsid(fd);
 		if (cfg.namespace_id == 0) {
-			err = EINVAL;
+			err = -EINVAL;
 			goto close_fd;
 		}
 	}
@@ -3974,7 +3974,7 @@ static int dsm(int argc, char **argv, struct command *cmd, struct plugin *plugin
 	dsm = nvme_setup_dsm_range((__u32 *)ctx_attrs, (__u32 *)nlbs, (__u64 *)slbas, nr);
 	if (!dsm) {
 		fprintf(stderr, "failed to allocate data set payload\n");
-		err = ENOMEM;
+		err = -ENOMEM;
 		goto close_fd;
 	}
 
@@ -4024,7 +4024,7 @@ static int flush(int argc, char **argv, struct command *cmd, struct plugin *plug
 	if (S_ISBLK(nvme_stat.st_mode)) {
 		cfg.namespace_id = get_nsid(fd);
 		if (cfg.namespace_id == 0) {
-			err = EINVAL;
+			err = -EINVAL;
 			goto close_fd;
 		}
 	}
@@ -4094,13 +4094,13 @@ static int resv_acquire(int argc, char **argv, struct command *cmd, struct plugi
 	if (!cfg.namespace_id) {
 		cfg.namespace_id = get_nsid(fd);
 		if (cfg.namespace_id == 0) {
-			err = EINVAL;
+			err = -EINVAL;
 			goto close_fd;
 		}
 	}
 	if (cfg.racqa > 7) {
 		fprintf(stderr, "invalid racqa:%d\n", cfg.racqa);
-		err = EINVAL;
+		err = -EINVAL;
 		goto close_fd;
 	}
 
@@ -4168,19 +4168,19 @@ static int resv_register(int argc, char **argv, struct command *cmd, struct plug
 	if (!cfg.namespace_id) {
 		cfg.namespace_id = get_nsid(fd);
 		if (cfg.namespace_id == 0) {
-			err = EINVAL;
+			err = -EINVAL;
 			goto close_fd;
 		}
 	}
 	if (cfg.cptpl > 3) {
 		fprintf(stderr, "invalid cptpl:%d\n", cfg.cptpl);
-		err = EINVAL;
+		err = -EINVAL;
 		goto close_fd;
 	}
 
 	if (cfg.rrega > 7) {
 		fprintf(stderr, "invalid rrega:%d\n", cfg.rrega);
-		err = EINVAL;
+		err = -EINVAL;
 		goto close_fd;
 	}
 
@@ -4250,13 +4250,13 @@ static int resv_release(int argc, char **argv, struct command *cmd, struct plugi
 	if (!cfg.namespace_id) {
 		cfg.namespace_id = get_nsid(fd);
 		if (cfg.namespace_id == 0) {
-			err = EINVAL;
+			err = -EINVAL;
 			goto close_fd;
 		}
 	}
 	if (cfg.rrela > 7) {
 		fprintf(stderr, "invalid rrela:%d\n", cfg.rrela);
-		err = EINVAL;
+		err = -EINVAL;
 		goto close_fd;
 	}
 
@@ -4331,7 +4331,7 @@ static int resv_report(int argc, char **argv, struct command *cmd, struct plugin
 	if (!cfg.namespace_id) {
 		cfg.namespace_id = get_nsid(fd);
 		if (cfg.namespace_id == 0) {
-			err = EINVAL;
+			err = -EINVAL;
 			goto close_fd;
 		}
 	}
@@ -4344,7 +4344,7 @@ static int resv_report(int argc, char **argv, struct command *cmd, struct plugin
 
 	if (posix_memalign((void **)&status, getpagesize(), size)) {
 		fprintf(stderr, "No memory for resv report:%d\n", size);
-		err = ENOMEM;
+		err = -ENOMEM;
 		goto close_fd;
 	}
 	memset(status, 0, size);
@@ -4469,7 +4469,7 @@ static int submit_io(int opcode, char *command, const char *desc,
 
 	dfd = mfd = opcode & 1 ? STDIN_FILENO : STDOUT_FILENO;
 	if (cfg.prinfo > 0xf) {
-		err = EINVAL;
+		err = -EINVAL;
 		goto close_fd;
 	}
 
@@ -4483,7 +4483,7 @@ static int submit_io(int opcode, char *command, const char *desc,
 		if (cfg.dtype > 0xf) {
 			fprintf(stderr, "Invalid directive type, %x\n",
 				cfg.dtype);
-			err = EINVAL;
+			err = -EINVAL;
 			goto close_fd;
 		}
 		control |= cfg.dtype << 4;
@@ -4494,7 +4494,7 @@ static int submit_io(int opcode, char *command, const char *desc,
 		dfd = open(cfg.data, flags, mode);
 		if (dfd < 0) {
 			perror(cfg.data);
-			err = EINVAL;
+			err = -EINVAL;
 			goto close_fd;
 		}
 		mfd = dfd;
@@ -4503,14 +4503,14 @@ static int submit_io(int opcode, char *command, const char *desc,
 		mfd = open(cfg.metadata, flags, mode);
 		if (mfd < 0) {
 			perror(cfg.metadata);
-			err = EINVAL;
+			err = -EINVAL;
 			goto close_dfd;
 		}
 	}
 
 	if (!cfg.data_size)	{
 		fprintf(stderr, "data size not provided\n");
-		err = EINVAL;
+		err = -EINVAL;
 		goto close_mfd;
 	}
 
@@ -4527,7 +4527,7 @@ static int submit_io(int opcode, char *command, const char *desc,
 
 	if (posix_memalign(&buffer, getpagesize(), buffer_size)) {
 		fprintf(stderr, "can not allocate io payload\n");
-		err = ENOMEM;
+		err = -ENOMEM;
 		goto close_mfd;
 	}
 	memset(buffer, 0, buffer_size);
@@ -4537,7 +4537,7 @@ static int submit_io(int opcode, char *command, const char *desc,
 		if (!mbuffer) {
 			fprintf(stderr, "can not allocate io metadata "
 					"payload: %s\n", strerror(errno));
-			err = ENOMEM;
+			err = -ENOMEM;
 			goto free_buffer;
 		}
 	}
@@ -4594,12 +4594,12 @@ static int submit_io(int opcode, char *command, const char *desc,
 		if (!(opcode & 1) && write(dfd, (void *)buffer, cfg.data_size) < 0) {
 			fprintf(stderr, "write: %s: failed to write buffer to output file\n",
 					strerror(errno));
-			err = EINVAL;
+			err = -EINVAL;
 		} else if (!(opcode & 1) && cfg.metadata_size &&
 				write(mfd, (void *)mbuffer, cfg.metadata_size) < 0) {
 			fprintf(stderr, "write: %s: failed to write meta-data buffer to output file\n",
 					strerror(errno));
-			err = EINVAL;
+			err = -EINVAL;
 		} else
 			fprintf(stderr, "%s: Success\n", command);
 	}
@@ -4700,7 +4700,7 @@ static int sec_recv(int argc, char **argv, struct command *cmd, struct plugin *p
 		if (posix_memalign(&sec_buf, getpagesize(), cfg.size)) {
 			fprintf(stderr, "No memory for security size:%d\n",
 								cfg.size);
-			err = ENOMEM;
+			err = -ENOMEM;
 			goto close_fd;
 		}
 	}
@@ -4793,7 +4793,7 @@ static int dir_receive(int argc, char **argv, struct command *cmd, struct plugin
 			break;
 		default:
 			fprintf(stderr, "invalid directive operations for Identify Directives\n");
-			err = EINVAL;
+			err = -EINVAL;
 			goto close_fd;
 		}
 		break;
@@ -4812,19 +4812,19 @@ static int dir_receive(int argc, char **argv, struct command *cmd, struct plugin
 			break;
 		default:
 			fprintf(stderr, "invalid directive operations for Streams Directives\n");
-			err = EINVAL;
+			err = -EINVAL;
 			goto close_fd;
 		}
 		break;
 	default:
 		fprintf(stderr, "invalid directive type\n");
-		err = EINVAL;
+		err = -EINVAL;
 		goto close_fd;
 	}
 
 	if (cfg.data_len) {
 		if (posix_memalign(&buf, getpagesize(), cfg.data_len)) {
-			err = ENOMEM;
+			err = -ENOMEM;
 			goto close_fd;
 		}
 		memset(buf, 0, cfg.data_len);
@@ -4973,7 +4973,7 @@ static int passthru(int argc, char **argv, int ioctl_cmd, const char *desc, stru
 			   S_IRUSR | S_IRGRP | S_IROTH);
 		if (wfd < 0) {
 			perror(cfg.input_file);
-			err = EINVAL;
+			err = -EINVAL;
 			goto close_fd;
 		}
 	}
@@ -4983,21 +4983,21 @@ static int passthru(int argc, char **argv, int ioctl_cmd, const char *desc, stru
 		if (!metadata) {
 			fprintf(stderr, "can not allocate metadata "
 					"payload: %s\n", strerror(errno));
-			err = ENOMEM;
+			err = -ENOMEM;
 			goto close_wfd;
 		}
 	}
 	if (cfg.data_len) {
 		if (posix_memalign(&data, getpagesize(), cfg.data_len)) {
 			fprintf(stderr, "can not allocate data payload\n");
-			err = ENOMEM;
+			err = -ENOMEM;
 			goto free_metadata;
 		}
 
 		memset(data, cfg.prefill, cfg.data_len);
 		if (!cfg.read && !cfg.write) {
 			fprintf(stderr, "data direction not given\n");
-			err = EINVAL;
+			err = -EINVAL;
 			goto free_data;
 		} else if (cfg.write) {
 			if (read(wfd, data, cfg.data_len) < 0) {
-- 
2.21.0

[PATCH V5 4/6] nvme-status: Introduce nvme status module to map errno

[Minwoo Im]

Background:
  It's not enough to return the nvme status value in main() because it's
allowed to be in 8bits, but nvme status is indicated in 16bits.  So we
has not been able to figure out what kind of nvme status has been
returned by return value.

  This patch introduces nvme-status module that manages mapping between
nvme status and errno.  It's not possible to make 1:1 mapping relations,
but we can map it as a groups.

All the internal errors which has been returned in a negative value will
be returned with ECOMM that indicates communication errors.  In this
case, we can see what happened via stderr.

Cc: Keith Busch <kbusch at kernel.org>
Cc: Chaitanya Kulkarni <chaitanya.Kulkarni at wdc.com>
Signed-off-by: Minwoo Im <minwoo.im.dev at gmail.com>
---
 Makefile      |   3 +-
 linux/nvme.h  |   6 ++
 nvme-status.c | 155 ++++++++++++++++++++++++++++++++++++++++++++++++++
 nvme-status.h |  14 +++++
 4 files changed, 177 insertions(+), 1 deletion(-)
 create mode 100644 nvme-status.c
 create mode 100644 nvme-status.h

diff --git a/Makefile b/Makefile
index ebb6b75..e0c0fe0 100644
--- a/Makefile
+++ b/Makefile
@@ -32,7 +32,8 @@ override CFLAGS += -DNVME_VERSION='"$(NVME_VERSION)"'
 NVME_DPKG_VERSION=1~`lsb_release -sc`
 
 OBJS := argconfig.o suffix.o parser.o nvme-print.o nvme-ioctl.o \
-	nvme-lightnvm.o fabrics.o json.o nvme-models.o plugin.o
+	nvme-lightnvm.o fabrics.o json.o nvme-models.o plugin.o \
+	nvme-status.o
 
 PLUGIN_OBJS :=					\
 	plugins/intel/intel-nvme.o		\
diff --git a/linux/nvme.h b/linux/nvme.h
index 69f287e..9963e90 100644
--- a/linux/nvme.h
+++ b/linux/nvme.h
@@ -1319,6 +1319,12 @@ static inline bool nvme_is_write(struct nvme_command *cmd)
 	return cmd->common.opcode & 1;
 }
 
+enum {
+	NVME_SCT_GENERIC		= 0x0,
+	NVME_SCT_CMD_SPECIFIC		= 0x1,
+	NVME_SCT_MEDIA			= 0x2,
+};
+
 enum {
 	/*
 	 * Generic Command Status:
diff --git a/nvme-status.c b/nvme-status.c
new file mode 100644
index 0000000..c69ff3f
--- /dev/null
+++ b/nvme-status.c
@@ -0,0 +1,155 @@
+#include <linux/types.h>
+#include <stdbool.h>
+#include <errno.h>
+
+#include "nvme.h"
+#include "nvme-status.h"
+
+static inline __u8 nvme_generic_status_to_errno(__u16 status)
+{
+	switch (status) {
+	case NVME_SC_INVALID_OPCODE:
+	case NVME_SC_INVALID_FIELD:
+	case NVME_SC_INVALID_NS:
+	case NVME_SC_SGL_INVALID_LAST:
+	case NVME_SC_SGL_INVALID_COUNT:
+	case NVME_SC_SGL_INVALID_DATA:
+	case NVME_SC_SGL_INVALID_METADATA:
+	case NVME_SC_SGL_INVALID_TYPE:
+	case NVME_SC_SGL_INVALID_OFFSET:
+	case NVME_SC_SGL_INVALID_SUBTYPE:
+		return EINVAL;
+	case NVME_SC_CMDID_CONFLICT:
+		return EADDRINUSE;
+	case NVME_SC_DATA_XFER_ERROR:
+	case NVME_SC_INTERNAL:
+	case NVME_SC_SANITIZE_FAILED:
+		return EIO;
+	case NVME_SC_POWER_LOSS:
+	case NVME_SC_ABORT_REQ:
+	case NVME_SC_ABORT_QUEUE:
+	case NVME_SC_FUSED_FAIL:
+	case NVME_SC_FUSED_MISSING:
+		return EWOULDBLOCK;
+	case NVME_SC_CMD_SEQ_ERROR:
+		return EILSEQ;
+	case NVME_SC_SANITIZE_IN_PROGRESS:
+		return EINPROGRESS;
+	case NVME_SC_NS_WRITE_PROTECTED:
+	case NVME_SC_NS_NOT_READY:
+	case NVME_SC_RESERVATION_CONFLICT:
+		return EACCES;
+	case NVME_SC_LBA_RANGE:
+		return EREMOTEIO;
+	case NVME_SC_CAP_EXCEEDED:
+		return ENOSPC;
+	}
+
+	return EIO;
+}
+
+static inline __u8 nvme_cmd_specific_status_to_errno(__u16 status)
+{
+	switch (status) {
+	case NVME_SC_CQ_INVALID:
+	case NVME_SC_QID_INVALID:
+	case NVME_SC_QUEUE_SIZE:
+	case NVME_SC_FIRMWARE_SLOT:
+	case NVME_SC_FIRMWARE_IMAGE:
+	case NVME_SC_INVALID_VECTOR:
+	case NVME_SC_INVALID_LOG_PAGE:
+	case NVME_SC_INVALID_FORMAT:
+	case NVME_SC_INVALID_QUEUE:
+	case NVME_SC_NS_INSUFFICIENT_CAP:
+	case NVME_SC_NS_ID_UNAVAILABLE:
+	case NVME_SC_CTRL_LIST_INVALID:
+	case NVME_SC_BAD_ATTRIBUTES:
+	case NVME_SC_INVALID_PI:
+		return EINVAL;
+	case NVME_SC_ABORT_LIMIT:
+	case NVME_SC_ASYNC_LIMIT:
+		return EDQUOT;
+	case NVME_SC_FW_NEEDS_CONV_RESET:
+	case NVME_SC_FW_NEEDS_SUBSYS_RESET:
+	case NVME_SC_FW_NEEDS_MAX_TIME:
+		return ERESTART;
+	case NVME_SC_FEATURE_NOT_SAVEABLE:
+	case NVME_SC_FEATURE_NOT_CHANGEABLE:
+	case NVME_SC_FEATURE_NOT_PER_NS:
+	case NVME_SC_FW_ACTIVATE_PROHIBITED:
+	case NVME_SC_NS_IS_PRIVATE:
+	case NVME_SC_BP_WRITE_PROHIBITED:
+	case NVME_SC_READ_ONLY:
+		return EPERM;
+	case NVME_SC_OVERLAPPING_RANGE:
+	case NVME_SC_NS_NOT_ATTACHED:
+		return ENOSPC;
+	case NVME_SC_NS_ALREADY_ATTACHED:
+		return EALREADY;
+	case NVME_SC_THIN_PROV_NOT_SUPP:
+	case NVME_SC_ONCS_NOT_SUPPORTED:
+		return EOPNOTSUPP;
+	}
+
+	return EIO;
+}
+
+static inline __u8 nvme_fabrics_status_to_errno(__u16 status)
+{
+	switch (status) {
+	case NVME_SC_CONNECT_FORMAT:
+	case NVME_SC_CONNECT_INVALID_PARAM:
+		return EINVAL;
+	case NVME_SC_CONNECT_CTRL_BUSY:
+		return EBUSY;
+	case NVME_SC_CONNECT_RESTART_DISC:
+		return ERESTART;
+	case NVME_SC_CONNECT_INVALID_HOST:
+		return ECONNREFUSED;
+	case NVME_SC_DISCOVERY_RESTART:
+		return EAGAIN;
+	case NVME_SC_AUTH_REQUIRED:
+		return EPERM;
+	}
+
+	return EIO;
+}
+
+/*
+ * nvme_status_to_errno - It converts given status to errno mapped
+ * @status: >= 0 for nvme status field in completion queue entry,
+ *          < 0 for linux internal errors
+ * @fabrics: true if given status is for fabrics
+ *
+ * Notes: This function will convert a given status to an errno mapped
+ */
+__u8 nvme_status_to_errno(int status, bool fabrics)
+{
+	__u8 sct;
+
+	if (!status)
+		return 0;
+
+	if (status < 0)
+		return ECOMM;
+
+	/*
+	 * The actual status code is enough with masking 0xff, but we need to
+	 * cover status code type which is 3bits with 0x7ff.
+	 */
+	status &= 0x7ff;
+
+	sct = nvme_status_type(status);
+	if (sct == NVME_SCT_GENERIC)
+		return nvme_generic_status_to_errno(status);
+	else if (sct == NVME_SCT_CMD_SPECIFIC && !fabrics)
+		return nvme_cmd_specific_status_to_errno(status);
+	else if (sct == NVME_SCT_CMD_SPECIFIC && fabrics)
+		return nvme_fabrics_status_to_errno(status);
+
+	/*
+	 * Media, integrity related status, and the others will be mapped to
+	 * EIO.
+	 */
+	return EIO;
+}
diff --git a/nvme-status.h b/nvme-status.h
new file mode 100644
index 0000000..92a474c
--- /dev/null
+++ b/nvme-status.h
@@ -0,0 +1,14 @@
+#include <linux/types.h>
+#include <stdbool.h>
+
+/*
+ * nvme_status_type - It gives SCT(Status Code Type) in status field in
+ *                    completion queue entry.
+ * @status: status field located at DW3 in completion queue entry
+ */
+static inline __u8 nvme_status_type(__u16 status)
+{
+	return (status & 0x700) >> 8;
+}
+
+__u8 nvme_status_to_errno(int status, bool fabrics);
-- 
2.21.0

[PATCH V5 5/6] nvme: Return errno mapped for nvme error status

[Minwoo Im]

If ioctl module has returned a nvme error status, it will be returned
by being converted to a mapped errno value which has been provided by
previous commits.

Cc: Keith Busch <kbusch at kernel.org>
Cc: Chaitanya Kulkarni <chaitanya.Kulkarni at wdc.com>
Signed-off-by: Minwoo Im <minwoo.im.dev at gmail.com>
---
 nvme.c | 106 ++++++++++++++++++++++++++++-----------------------------
 1 file changed, 53 insertions(+), 53 deletions(-)

diff --git a/nvme.c b/nvme.c
index a8d382a..f3d003b 100644
--- a/nvme.c
+++ b/nvme.c
@@ -48,6 +48,7 @@
 #include "common.h"
 #include "nvme-print.h"
 #include "nvme-ioctl.h"
+#include "nvme-status.h"
 #include "nvme-lightnvm.h"
 #include "plugin.h"
 
@@ -223,7 +224,7 @@ static int get_smart_log(int argc, char **argv, struct command *cmd, struct plug
 	close(fd);
 
  ret:
-	return err;
+	return nvme_status_to_errno(err, false);
 }
 
 static int get_ana_log(int argc, char **argv, struct command *cmd,
@@ -298,7 +299,7 @@ static int get_ana_log(int argc, char **argv, struct command *cmd,
 close_fd:
 	close(fd);
 ret:
-	return err;
+	return nvme_status_to_errno(err, false);
 }
 
 static int get_telemetry_log(int argc, char **argv, struct command *cmd, struct plugin *plugin)
@@ -428,7 +429,7 @@ static int get_telemetry_log(int argc, char **argv, struct command *cmd, struct
  close_fd:
 	close(fd);
  ret:
-	return err;
+	return nvme_status_to_errno(err, false);
 }
 
 static int get_endurance_log(int argc, char **argv, struct command *cmd, struct plugin *plugin)
@@ -485,7 +486,7 @@ static int get_endurance_log(int argc, char **argv, struct command *cmd, struct
  close_fd:
 	close(fd);
  ret:
-	return err;
+	return nvme_status_to_errno(err, false);
 }
 
 static int get_effects_log(int argc, char **argv, struct command *cmd, struct plugin *plugin)
@@ -550,7 +551,7 @@ static int get_effects_log(int argc, char **argv, struct command *cmd, struct pl
  close_fd:
 	close(fd);
  ret:
-	return err;
+	return nvme_status_to_errno(err, false);
 }
 
 static int get_error_log(int argc, char **argv, struct command *cmd, struct plugin *plugin)
@@ -637,7 +638,7 @@ static int get_error_log(int argc, char **argv, struct command *cmd, struct plug
  close_fd:
 	close(fd);
  ret:
-	return err;
+	return nvme_status_to_errno(err, false);
 }
 
 static int get_fw_log(int argc, char **argv, struct command *cmd, struct plugin *plugin)
@@ -694,7 +695,7 @@ static int get_fw_log(int argc, char **argv, struct command *cmd, struct plugin
  close_fd:
 	close(fd);
  ret:
-	return err;
+	return nvme_status_to_errno(err, false);
 }
 
 static int get_changed_ns_list_log(int argc, char **argv, struct command *cmd, struct plugin *plugin)
@@ -752,7 +753,7 @@ static int get_changed_ns_list_log(int argc, char **argv, struct command *cmd, s
  close_fd:
 	close(fd);
  ret:
-	return err;
+	return nvme_status_to_errno(err, false);
 }
 
 static int get_log(int argc, char **argv, struct command *cmd, struct plugin *plugin)
@@ -854,7 +855,7 @@ static int get_log(int argc, char **argv, struct command *cmd, struct plugin *pl
  close_fd:
 	close(fd);
  ret:
-	return err;
+	return nvme_status_to_errno(err, false);
 }
 
 static int sanitize_log(int argc, char **argv, struct command *command, struct plugin *plugin)
@@ -919,7 +920,7 @@ static int sanitize_log(int argc, char **argv, struct command *command, struct p
  close_fd:
 	close(fd);
  ret:
-	return ret;
+	return nvme_status_to_errno(ret, false);
 }
 
 static int list_ctrl(int argc, char **argv, struct command *cmd, struct plugin *plugin)
@@ -975,7 +976,7 @@ static int list_ctrl(int argc, char **argv, struct command *cmd, struct plugin *
 close_fd:
 	close(fd);
 ret:
-	return err;
+	return nvme_status_to_errno(err, false);
 }
 
 static int list_ns(int argc, char **argv, struct command *cmd, struct plugin *plugin)
@@ -1021,7 +1022,7 @@ static int list_ns(int argc, char **argv, struct command *cmd, struct plugin *pl
 
 	close(fd);
  ret:
-	return err;
+	return nvme_status_to_errno(err, false);
 }
 
 static int get_nsid(int fd)
@@ -1095,7 +1096,7 @@ static int delete_ns(int argc, char **argv, struct command *cmd, struct plugin *
  close_fd:
 	close(fd);
  ret:
-	return err;
+	return nvme_status_to_errno(err, false);
 }
 
 static int nvme_attach_ns(int argc, char **argv, int attach, const char *desc, struct command *cmd)
@@ -1162,7 +1163,7 @@ static int nvme_attach_ns(int argc, char **argv, int attach, const char *desc, s
  close_fd:
 	close(fd);
  ret:
-	return err;
+	return nvme_status_to_errno(err, false);
 }
 
 static int attach_ns(int argc, char **argv, struct command *cmd, struct plugin *plugin)
@@ -1292,7 +1293,7 @@ static int create_ns(int argc, char **argv, struct command *cmd, struct plugin *
 close_fd:
 	close(fd);
 ret:
-	return err;
+	return nvme_status_to_errno(err, false);
 }
 
 char *nvme_char_from_block(char *block)
@@ -1780,7 +1781,7 @@ static int list_subsys(int argc, char **argv, struct command *cmd,
 		free(subsysnqn);
 
  ret:
-	return ret;
+	return nvme_status_to_errno(ret, false);
 }
 
 static int get_nvme_info(int fd, struct list_item *item, const char *node)
@@ -1913,7 +1914,7 @@ static int list(int argc, char **argv, struct command *cmd, struct plugin *plugi
 		free(devices[i]);
 	free(devices);
  ret:
-	return ret;
+	return nvme_status_to_errno(ret, false);
 }
 
 int __id_ctrl(int argc, char **argv, struct command *cmd, struct plugin *plugin, void (*vs)(__u8 *vs, struct json_object *root))
@@ -1989,7 +1990,7 @@ int __id_ctrl(int argc, char **argv, struct command *cmd, struct plugin *plugin,
  close_fd:
 	close(fd);
  ret:
-	return err;
+	return nvme_status_to_errno(err, false);
 }
 
 static int id_ctrl(int argc, char **argv, struct command *cmd, struct plugin *plugin)
@@ -2072,7 +2073,7 @@ static int ns_descs(int argc, char **argv, struct command *cmd, struct plugin *p
  close_fd:
 	close(fd);
  ret:
-	return err;
+	return nvme_status_to_errno(err, false);
 }
 
 static int id_ns(int argc, char **argv, struct command *cmd, struct plugin *plugin)
@@ -2161,7 +2162,7 @@ static int id_ns(int argc, char **argv, struct command *cmd, struct plugin *plug
  close_fd:
 	close(fd);
  ret:
-	return err;
+	return nvme_status_to_errno(err, false);
 }
 
 static int id_nvmset(int argc, char **argv, struct command *cmd, struct plugin *plugin)
@@ -2221,7 +2222,7 @@ static int id_nvmset(int argc, char **argv, struct command *cmd, struct plugin *
  close_fd:
 	close(fd);
  ret:
-	return err;
+	return nvme_status_to_errno(err, false);
 }
 
 static int get_ns_id(int argc, char **argv, struct command *cmd, struct plugin *plugin)
@@ -2250,7 +2251,7 @@ static int get_ns_id(int argc, char **argv, struct command *cmd, struct plugin *
  close_fd:
 	close(fd);
  ret:
-	return err;
+	return nvme_status_to_errno(err, false);
 }
 
 static int virtual_mgmt(int argc, char **argv, struct command *cmd, struct plugin *plugin)
@@ -2317,8 +2318,7 @@ static int virtual_mgmt(int argc, char **argv, struct command *cmd, struct plugi
 
 	close(fd);
  ret:
-	return err;
-
+	return nvme_status_to_errno(err, false);
 }
 
 static int list_secondary_ctrl(int argc, char **argv, struct command *cmd, struct plugin *plugin)
@@ -2396,7 +2396,7 @@ static int list_secondary_ctrl(int argc, char **argv, struct command *cmd, struc
 close_fd:
 	close(fd);
 ret:
-	return err;
+	return nvme_status_to_errno(err, false);
 }
 
 static int device_self_test(int argc, char **argv, struct command *cmd, struct plugin *plugin)
@@ -2447,7 +2447,7 @@ static int device_self_test(int argc, char **argv, struct command *cmd, struct p
 
 	close(fd);
  ret:
-	return err;
+	return nvme_status_to_errno(err, false);
 }
 
 static int self_test_log(int argc, char **argv, struct command *cmd, struct plugin *plugin)
@@ -2505,7 +2505,7 @@ static int self_test_log(int argc, char **argv, struct command *cmd, struct plug
  close_fd:
 	close(fd);
  ret:
-	return err;
+	return nvme_status_to_errno(err, false);
 }
 
 static int get_feature(int argc, char **argv, struct command *cmd, struct plugin *plugin)
@@ -2638,7 +2638,7 @@ static int get_feature(int argc, char **argv, struct command *cmd, struct plugin
  close_fd:
 	close(fd);
  ret:
-	return err;
+	return nvme_status_to_errno(err, false);
 }
 
 static int fw_download(int argc, char **argv, struct command *cmd, struct plugin *plugin)
@@ -2745,7 +2745,7 @@ static int fw_download(int argc, char **argv, struct command *cmd, struct plugin
  close_fd:
 	close(fd);
  ret:
-	return err;
+	return nvme_status_to_errno(err, false);
 }
 
 static char *nvme_fw_status_reset_type(__u32 status)
@@ -2840,7 +2840,7 @@ static int fw_commit(int argc, char **argv, struct command *cmd, struct plugin *
  close_fd:
 	close(fd);
  ret:
-	return err;
+	return nvme_status_to_errno(err, false);
 }
 
 static int subsystem_reset(int argc, char **argv, struct command *cmd, struct plugin *plugin)
@@ -2869,7 +2869,7 @@ static int subsystem_reset(int argc, char **argv, struct command *cmd, struct pl
 
 	close(fd);
  ret:
-	return err;
+	return nvme_status_to_errno(err, false);
 }
 
 static int reset(int argc, char **argv, struct command *cmd, struct plugin *plugin)
@@ -2893,7 +2893,7 @@ static int reset(int argc, char **argv, struct command *cmd, struct plugin *plug
 
 	close(fd);
  ret:
-	return err;
+	return nvme_status_to_errno(err, false);
 }
 
 static int ns_rescan(int argc, char **argv, struct command *cmd, struct plugin *plugin)
@@ -2917,7 +2917,7 @@ static int ns_rescan(int argc, char **argv, struct command *cmd, struct plugin *
 
 	close(fd);
  ret:
-	return err;
+	return nvme_status_to_errno(err, false);
 }
 
 static int sanitize(int argc, char **argv, struct command *cmd, struct plugin *plugin)
@@ -3011,7 +3011,7 @@ static int sanitize(int argc, char **argv, struct command *cmd, struct plugin *p
  close_fd:
 	close(fd);
  ret:
-	return ret;
+	return nvme_status_to_errno(ret, false);
 }
 
 static int show_registers(int argc, char **argv, struct command *cmd, struct plugin *plugin)
@@ -3086,7 +3086,7 @@ static int show_registers(int argc, char **argv, struct command *cmd, struct plu
  close_fd:
 	close(fd);
  ret:
-	return err;
+	return nvme_status_to_errno(err, false);
 }
 
 static int get_property(int argc, char **argv, struct command *cmd, struct plugin *plugin)
@@ -3140,7 +3140,7 @@ static int get_property(int argc, char **argv, struct command *cmd, struct plugi
  close_fd:
 	close(fd);
  ret:
-	return err;
+	return nvme_status_to_errno(err, false);
 }
 
 static int set_property(int argc, char **argv, struct command *cmd, struct plugin *plugin)
@@ -3197,7 +3197,7 @@ static int set_property(int argc, char **argv, struct command *cmd, struct plugi
  close_fd:
 	close(fd);
  ret:
-	return err;
+	return nvme_status_to_errno(err, false);
 }
 
 static int format(int argc, char **argv, struct command *cmd, struct plugin *plugin)
@@ -3360,7 +3360,7 @@ static int format(int argc, char **argv, struct command *cmd, struct plugin *plu
  close_fd:
 	close(fd);
  ret:
-	return err;
+	return nvme_status_to_errno(err, false);
 }
 
 static int set_feature(int argc, char **argv, struct command *cmd, struct plugin *plugin)
@@ -3482,7 +3482,7 @@ static int set_feature(int argc, char **argv, struct command *cmd, struct plugin
  close_fd:
 	close(fd);
  ret:
-	return err;
+	return nvme_status_to_errno(err, false);
 }
 
 static int sec_send(int argc, char **argv, struct command *cmd, struct plugin *plugin)
@@ -3581,7 +3581,7 @@ static int sec_send(int argc, char **argv, struct command *cmd, struct plugin *p
  close_fd:
 	close(fd);
  ret:
-	return err;
+	return nvme_status_to_errno(err, false);
 }
 
 static int dir_send(int argc, char **argv, struct command *cmd, struct plugin *plugin)
@@ -3735,7 +3735,7 @@ free:
 close_fd:
 	close(fd);
 ret:
-	return err;
+	return nvme_status_to_errno(err, false);
 }
 
 static int write_uncor(int argc, char **argv, struct command *cmd, struct plugin *plugin)
@@ -3792,7 +3792,7 @@ static int write_uncor(int argc, char **argv, struct command *cmd, struct plugin
 close_fd:
 	close(fd);
 ret:
-	return err;
+	return nvme_status_to_errno(err, false);
 }
 
 static int write_zeroes(int argc, char **argv, struct command *cmd, struct plugin *plugin)
@@ -3886,7 +3886,7 @@ static int write_zeroes(int argc, char **argv, struct command *cmd, struct plugi
  close_fd:
 	close(fd);
  ret:
-	return err;
+	return nvme_status_to_errno(err, false);
 }
 
 static int dsm(int argc, char **argv, struct command *cmd, struct plugin *plugin)
@@ -3989,7 +3989,7 @@ static int dsm(int argc, char **argv, struct command *cmd, struct plugin *plugin
  close_fd:
 	close(fd);
  ret:
-	return err;
+	return nvme_status_to_errno(err, false);
 }
 
 static int flush(int argc, char **argv, struct command *cmd, struct plugin *plugin)
@@ -4039,7 +4039,7 @@ static int flush(int argc, char **argv, struct command *cmd, struct plugin *plug
 close_fd:
 	close(fd);
 ret:
-	return err;
+	return nvme_status_to_errno(err, false);
 }
 
 static int resv_acquire(int argc, char **argv, struct command *cmd, struct plugin *plugin)
@@ -4116,7 +4116,7 @@ static int resv_acquire(int argc, char **argv, struct command *cmd, struct plugi
  close_fd:
 	close(fd);
  ret:
-	return err;
+	return nvme_status_to_errno(err, false);
 }
 
 static int resv_register(int argc, char **argv, struct command *cmd, struct plugin *plugin)
@@ -4196,7 +4196,7 @@ static int resv_register(int argc, char **argv, struct command *cmd, struct plug
  close_fd:
 	close(fd);
  ret:
-	return err;
+	return nvme_status_to_errno(err, false);
 }
 
 static int resv_release(int argc, char **argv, struct command *cmd, struct plugin *plugin)
@@ -4272,7 +4272,7 @@ static int resv_release(int argc, char **argv, struct command *cmd, struct plugi
  close_fd:
 	close(fd);
  ret:
-	return err;
+	return nvme_status_to_errno(err, false);
 }
 
 static int resv_report(int argc, char **argv, struct command *cmd, struct plugin *plugin)
@@ -4369,7 +4369,7 @@ static int resv_report(int argc, char **argv, struct command *cmd, struct plugin
  close_fd:
 	close(fd);
  ret:
-	return err;
+	return nvme_status_to_errno(err, false);
 }
 
 static int submit_io(int opcode, char *command, const char *desc,
@@ -4617,7 +4617,7 @@ static int submit_io(int opcode, char *command, const char *desc,
  close_fd:
 	close(fd);
  ret:
-	return err;
+	return nvme_status_to_errno(err, false);
 }
 
 static int compare(int argc, char **argv, struct command *cmd, struct plugin *plugin)
@@ -4726,7 +4726,7 @@ static int sec_recv(int argc, char **argv, struct command *cmd, struct plugin *p
  close_fd:
 	close(fd);
  ret:
-	return err;
+	return nvme_status_to_errno(err, false);
 }
 
 static int dir_receive(int argc, char **argv, struct command *cmd, struct plugin *plugin)
@@ -4859,7 +4859,7 @@ free:
 close_fd:
 	close(fd);
 ret:
-	return err;
+	return nvme_status_to_errno(err, false);
 }
 
 static int passthru(int argc, char **argv, int ioctl_cmd, const char *desc, struct command *cmd)
@@ -5062,7 +5062,7 @@ static int passthru(int argc, char **argv, int ioctl_cmd, const char *desc, stru
  close_fd:
 	close(fd);
  ret:
-	return err;
+	return nvme_status_to_errno(err, false);
 }
 
 static int io_passthru(int argc, char **argv, struct command *cmd, struct plugin *plugin)
-- 
2.21.0

[PATCH V5 6/6] fabrics: Return errno mapped for fabrics error status

[Minwoo Im]

If discover has been failed due to a nvme status, it will be returned to
main() with mapped value for fabrics get log page command.

Now connect command related status cannot be added in this patch because
kernel is not currently returning the nvme status, it's instead
returning -EIO if fails.  errno for connect command can be added once
kernel is ready to return the proper value for nvme status.

Cc: Keith Busch <kbusch at kernel.org>
Cc: Chaitanya Kulkarni <chaitanya.Kulkarni at wdc.com>
Signed-off-by: Minwoo Im <minwoo.im.dev at gmail.com>
---
 fabrics.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/fabrics.c b/fabrics.c
index 469b4a1..8dae627 100644
--- a/fabrics.c
+++ b/fabrics.c
@@ -35,6 +35,7 @@
 
 #include "parser.h"
 #include "nvme-ioctl.h"
+#include "nvme-status.h"
 #include "fabrics.h"
 
 #include "nvme.h"
@@ -295,7 +296,7 @@ enum {
 };
 
 static int nvmf_get_log_page_discovery(const char *dev_path,
-		struct nvmf_disc_rsp_page_hdr **logp, int *numrec)
+		struct nvmf_disc_rsp_page_hdr **logp, int *numrec, int *status)
 {
 	struct nvmf_disc_rsp_page_hdr *log;
 	unsigned int hdr_size;
@@ -400,6 +401,7 @@ out_free_log:
 out_close:
 	close(fd);
 out:
+	*status = nvme_status_to_errno(error, true);
 	return error;
 }
 
@@ -849,6 +851,7 @@ static int do_discover(char *argstr, bool connect)
 	struct nvmf_disc_rsp_page_hdr *log = NULL;
 	char *dev_name;
 	int instance, numrec = 0, ret, err;
+	int status = 0;
 
 	instance = add_ctrl(argstr);
 	if (instance < 0)
@@ -856,7 +859,7 @@ static int do_discover(char *argstr, bool connect)
 
 	if (asprintf(&dev_name, "/dev/nvme%d", instance) < 0)
 		return -errno;
-	ret = nvmf_get_log_page_discovery(dev_name, &log, &numrec);
+	ret = nvmf_get_log_page_discovery(dev_name, &log, &numrec, &status);
 	free(dev_name);
 	err = remove_ctrl(instance);
 	if (err)
@@ -874,9 +877,11 @@ static int do_discover(char *argstr, bool connect)
 	case DISC_GET_NUMRECS:
 		fprintf(stderr,
 			"Get number of discovery log entries failed.\n");
+		ret = status;
 		break;
 	case DISC_GET_LOG:
 		fprintf(stderr, "Get discovery log entries failed.\n");
+		ret = status;
 		break;
 	case DISC_NO_LOG:
 		fprintf(stdout, "No discovery log entries to fetch.\n");
@@ -885,6 +890,7 @@ static int do_discover(char *argstr, bool connect)
 	case DISC_NOT_EQUAL:
 		fprintf(stderr,
 		"Numrec values of last two get discovery log page not equal\n");
+		ret = DISC_OK;
 		break;
 	default:
 		fprintf(stderr, "Get discovery log page failed: %d\n", ret);
@@ -1000,7 +1006,7 @@ int discover(const char *desc, int argc, char **argv, bool connect)
 	}
 
  ret:
-	return ret;
+	return nvme_status_to_errno(ret, true);
 }
 
 int connect(const char *desc, int argc, char **argv)
@@ -1048,7 +1054,7 @@ int connect(const char *desc, int argc, char **argv)
 	if (instance < 0)
 		ret = instance;
  ret:
-	return ret;
+	return nvme_status_to_errno(ret, true);
 }
 
 static int scan_sys_nvme_filter(const struct dirent *d)
@@ -1181,7 +1187,7 @@ int disconnect(const char *desc, int argc, char **argv)
 	}
 
  ret:
-	return ret;
+	return nvme_status_to_errno(ret, true);
 }
 
 int disconnect_all(const char *desc, int argc, char **argv)
@@ -1210,5 +1216,5 @@ int disconnect_all(const char *desc, int argc, char **argv)
 		}
 	}
 out:
-	return ret;
+	return nvme_status_to_errno(ret, true);
 }
-- 
2.21.0

[PATCH V5 1/6] nvme: Do not return in the middle of the subcommand

[Chaitanya Kulkarni]

On 5/23/19 7:58 AM, Minwoo Im wrote:
> To make nvme-cli subcommand return a mapped errno value to main(), it
> should return the error status in a single place because it would be
> great if the return statements and free operations are in an one shot
> place.
> 
> This patch makes all the subcommands in nvme module return the error
> which means internal error which should be in negative and nvme error
> status which is in positive at the end of the subcommand.
> 
> Most of the changed parts are file descriptors which is returned from
> parse_and_open() function.  The "fd" could be in a negative value so
> that it needs to be mapped to a uniformed errno value which will be
> applied by the next patches.
> 
> Cc: Keith Busch <kbusch at kernel.org>
> Cc: Chaitanya Kulkarni <chaitanya.Kulkarni at wdc.com>
> Signed-off-by: Minwoo Im <minwoo.im.dev at gmail.com>
> ---
>   nvme.c | 425 +++++++++++++++++++++++++++++++++++++--------------------
>   1 file changed, 277 insertions(+), 148 deletions(-)
> 
> diff --git a/nvme.c b/nvme.c
> index 9819fcb..88b2cfc 100644
> --- a/nvme.c
> +++ b/nvme.c
> @@ -193,8 +193,10 @@ static int get_smart_log(int argc, char **argv, struct command *cmd, struct plug
>   	};
>   
>   	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
> -	if (fd < 0)
> -		return fd;
> +	if (fd < 0) {
> +		err = fd;
> +		goto ret;
> +	}
>   
>   	fmt = validate_output_format(cfg.output_format);
>   	if (fmt < 0) {
> @@ -220,6 +222,7 @@ static int get_smart_log(int argc, char **argv, struct command *cmd, struct plug
>    close_fd:
>   	close(fd);
>   
> + ret:
There is space here before return.
>   	return err;
>   }
>   
> @@ -249,8 +252,10 @@ static int get_ana_log(int argc, char **argv, struct command *cmd,
>   	};
>   
>   	fd = parse_and_open(argc, argv, desc, command_line_options, NULL, 0);
> -	if (fd < 0)
> -		return fd;
> +	if (fd < 0) {
> +		err = fd;
> +		goto ret;
> +	}
>   
>   	fmt = validate_output_format(cfg.output_format);
>   	if (fmt < 0) {
> @@ -292,6 +297,7 @@ static int get_ana_log(int argc, char **argv, struct command *cmd,
>   	free(ana_log);
>   close_fd:
>   	close(fd);
> +ret:
And there is no space here, please use consistent coding style. WIll 
send other comments soon.
>   	return err;
>   }
>   
> @@ -330,8 +336,10 @@ static int get_telemetry_log(int argc, char **argv, struct command *cmd, struct
>   	};
>   
>   	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
> -	if (fd < 0)
> -		return fd;
> +	if (fd < 0) {
> +		err = fd;
> +		goto ret;
> +	}
>   
>   	if (!cfg.file_name) {
>   		fprintf(stderr, "Please provide an output file!\n");
> @@ -419,6 +427,7 @@ static int get_telemetry_log(int argc, char **argv, struct command *cmd, struct
>   	free(page_log);
>    close_fd:
>   	close(fd);
> + ret:
>   	return err;
>   }
>   
> @@ -449,8 +458,10 @@ static int get_endurance_log(int argc, char **argv, struct command *cmd, struct
>   	};
>   
>   	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
> -	if (fd < 0)
> -		return fd;
> +	if (fd < 0) {
> +		err = fd;
> +		goto ret;
> +	}
>   
>   	fmt = validate_output_format(cfg.output_format);
>   	if (fmt < 0) {
> @@ -473,6 +484,7 @@ static int get_endurance_log(int argc, char **argv, struct command *cmd, struct
>   
>    close_fd:
>   	close(fd);
> + ret:
>   	return err;
>   }
>   
> @@ -505,8 +517,10 @@ static int get_effects_log(int argc, char **argv, struct command *cmd, struct pl
>   	};
>   
>   	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
> -	if (fd < 0)
> -		return fd;
> +	if (fd < 0) {
> +		err = fd;
> +		goto ret;
> +	}
>   
>   	fmt = validate_output_format(cfg.output_format);
>   	if (fmt < 0) {
> @@ -535,6 +549,7 @@ static int get_effects_log(int argc, char **argv, struct command *cmd, struct pl
>   
>    close_fd:
>   	close(fd);
> + ret:
>   	return err;
>   }
>   
> @@ -567,8 +582,10 @@ static int get_error_log(int argc, char **argv, struct command *cmd, struct plug
>   	};
>   
>   	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
> -	if (fd < 0)
> -		return fd;
> +	if (fd < 0) {
> +		err = fd;
> +		goto ret;
> +	}
>   
>   	fmt = validate_output_format(cfg.output_format);
>   	if (fmt < 0) {
> @@ -619,6 +636,7 @@ static int get_error_log(int argc, char **argv, struct command *cmd, struct plug
>   
>    close_fd:
>   	close(fd);
> + ret:
>   	return err;
>   }
>   
> @@ -646,8 +664,10 @@ static int get_fw_log(int argc, char **argv, struct command *cmd, struct plugin
>   	};
>   
>   	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
> -	if (fd < 0)
> -		return fd;
> +	if (fd < 0) {
> +		err = fd;
> +		goto ret;
> +	}
>   
>   	fmt = validate_output_format(cfg.output_format);
>   	if (fmt < 0) {
> @@ -673,6 +693,7 @@ static int get_fw_log(int argc, char **argv, struct command *cmd, struct plugin
>   
>    close_fd:
>   	close(fd);
> + ret:
>   	return err;
>   }
>   
> @@ -701,8 +722,10 @@ static int get_changed_ns_list_log(int argc, char **argv, struct command *cmd, s
>   	};
>   
>   	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
> -	if (fd < 0)
> -		return fd;
> +	if (fd < 0) {
> +		err = fd;
> +		goto ret;
> +	}
>   
>   	fmt = validate_output_format(cfg.output_format);
>   	if (fmt < 0) {
> @@ -728,7 +751,7 @@ static int get_changed_ns_list_log(int argc, char **argv, struct command *cmd, s
>   
>    close_fd:
>   	close(fd);
> -
> + ret:
>   	return err;
>   }
>   
> @@ -780,8 +803,10 @@ static int get_log(int argc, char **argv, struct command *cmd, struct plugin *pl
>   	};
>   
>   	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
> -	if (fd < 0)
> -		return fd;
> +	if (fd < 0) {
> +		err = fd;
> +		goto ret;
> +	}
>   
>   	if (cfg.aen) {
>   		cfg.log_len = 4096;
> @@ -828,6 +853,7 @@ static int get_log(int argc, char **argv, struct command *cmd, struct plugin *pl
>   
>    close_fd:
>   	close(fd);
> + ret:
>   	return err;
>   }
>   
> @@ -860,8 +886,10 @@ static int sanitize_log(int argc, char **argv, struct command *command, struct p
>   	};
>   
>   	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
> -	if (fd < 0)
> -		return fd;
> +	if (fd < 0) {
> +		ret = fd;
> +		goto ret;
> +	}
>   
>   	fmt = validate_output_format(cfg.output_format);
>   	if (fmt < 0) {
> @@ -890,7 +918,7 @@ static int sanitize_log(int argc, char **argv, struct command *command, struct p
>   
>    close_fd:
>   	close(fd);
> -
> + ret:
>   	return ret;
>   }
>   
> @@ -919,8 +947,10 @@ static int list_ctrl(int argc, char **argv, struct command *cmd, struct plugin *
>   	};
>   
>   	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
> -	if (fd < 0)
> -		return fd;
> +	if (fd < 0) {
> +		err = fd;
> +		goto ret;
> +	}
>   
>   	if (posix_memalign((void *)&cntlist, getpagesize(), 0x1000)) {
>   		fprintf(stderr, "can not allocate controller list payload\n");
> @@ -944,7 +974,7 @@ static int list_ctrl(int argc, char **argv, struct command *cmd, struct plugin *
>   
>   close_fd:
>   	close(fd);
> -
> +ret:
>   	return err;
>   }
>   
> @@ -973,8 +1003,10 @@ static int list_ns(int argc, char **argv, struct command *cmd, struct plugin *pl
>   	};
>   
>   	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
> -	if (fd < 0)
> -		return fd;
> +	if (fd < 0) {
> +		err = fd;
> +		goto ret;
> +	}
>   
>   	err = nvme_identify_ns_list(fd, cfg.namespace_id, !!cfg.all, ns_list);
>   	if (!err) {
> @@ -988,7 +1020,7 @@ static int list_ns(int argc, char **argv, struct command *cmd, struct plugin *pl
>   		perror("id namespace list");
>   
>   	close(fd);
> -
> + ret:
>   	return err;
>   }
>   
> @@ -1033,8 +1065,10 @@ static int delete_ns(int argc, char **argv, struct command *cmd, struct plugin *
>   	};
>   
>   	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
> -	if (fd < 0)
> -		return fd;
> +	if (fd < 0) {
> +		err = fd;
> +		goto ret;
> +	}
>   
>   	if (S_ISBLK(nvme_stat.st_mode)) {
>   		cfg.namespace_id = get_nsid(fd);
> @@ -1060,7 +1094,7 @@ static int delete_ns(int argc, char **argv, struct command *cmd, struct plugin *
>   
>    close_fd:
>   	close(fd);
> -
> + ret:
>   	return err;
>   }
>   
> @@ -1088,8 +1122,10 @@ static int nvme_attach_ns(int argc, char **argv, int attach, const char *desc, s
>   	};
>   
>   	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
> -	if (fd < 0)
> -		return fd;
> +	if (fd < 0) {
> +		err = fd;
> +		goto ret;
> +	}
>   
>   	if (!cfg.namespace_id) {
>   		fprintf(stderr, "%s: namespace-id parameter required\n",
> @@ -1125,7 +1161,7 @@ static int nvme_attach_ns(int argc, char **argv, int attach, const char *desc, s
>   
>    close_fd:
>   	close(fd);
> -
> + ret:
>   	return err;
>   }
>   
> @@ -1195,8 +1231,10 @@ static int create_ns(int argc, char **argv, struct command *cmd, struct plugin *
>   	};
>   
>   	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
> -	if (fd < 0)
> -		return fd;
> +	if (fd < 0) {
> +		err = fd;
> +		goto ret;
> +	}
>   
>   	if (cfg.flbas != 0xff && cfg.bs != 0x00) {
>   		fprintf(stderr,
> @@ -1253,7 +1291,7 @@ static int create_ns(int argc, char **argv, struct command *cmd, struct plugin *
>   
>   close_fd:
>   	close(fd);
> -
> +ret:
>   	return err;
>   }
>   
> @@ -1691,7 +1729,7 @@ static int list_subsys(int argc, char **argv, struct command *cmd,
>   
>   	ret = argconfig_parse(argc, argv, desc, opts, &cfg, sizeof(cfg));
>   	if (ret < 0)
> -		return ret;
> +		goto ret;
>   
>   	devicename = NULL;
>   	if (optind < argc) {
> @@ -1703,29 +1741,30 @@ static int list_subsys(int argc, char **argv, struct command *cmd,
>   			   &cfg.namespace_id) != 2) {
>   			fprintf(stderr, "%s is not a NVMe namespace device\n",
>   				argv[optind]);
> -			return -EINVAL;
> +			ret = -EINVAL;
> +			goto ret;
>   		}
>   		sprintf(path, "/sys/block/%s/device", devicename);
>   		subsysnqn = get_nvme_subsnqn(path);
>   		if (!subsysnqn) {
>   			fprintf(stderr, "Cannot read subsys NQN from %s\n",
>   				devicename);
> -			return -EINVAL;
> +			ret = -EINVAL;
> +			goto ret;
>   		}
>   		optind++;
>   	}
>   
>   	if (ret < 0) {
>   		argconfig_print_help(desc, opts);
> -		if (subsysnqn)
> -			free(subsysnqn);
> -		return ret;
> +		goto free;
>   	}
>   	fmt = validate_output_format(cfg.output_format);
>   	if (fmt != JSON && fmt != NORMAL) {
>   		if (subsysnqn)
>   			free(subsysnqn);
> -		return -EINVAL;
> +		ret = -EINVAL;
> +		goto free;
>   	}
>   
>   	slist = get_subsys_list(&subcnt, subsysnqn, cfg.namespace_id);
> @@ -1736,8 +1775,11 @@ static int list_subsys(int argc, char **argv, struct command *cmd,
>   		show_nvme_subsystem_list(slist, subcnt);
>   
>   	free_subsys_list(slist, subcnt);
> + free:
>   	if (subsysnqn)
>   		free(subsysnqn);
> +
> + ret:
>   	return ret;
>   }
>   
> @@ -1809,17 +1851,20 @@ static int list(int argc, char **argv, struct command *cmd, struct plugin *plugi
>   
>   	ret = argconfig_parse(argc, argv, desc, opts, &cfg, sizeof(cfg));
>   	if (ret < 0)
> -		return ret;
> +		goto ret;
>   
>   	fmt = validate_output_format(cfg.output_format);
>   
> -	if (fmt != JSON && fmt != NORMAL)
> -		return -EINVAL;
> +	if (fmt != JSON && fmt != NORMAL) {
> +		ret = -EINVAL;
> +		goto ret;
> +	}
>   
>   	n = scandir(dev, &devices, scan_dev_filter, alphasort);
>   	if (n < 0) {
>   		fprintf(stderr, "no NVMe device(s) detected.\n");
> -		return n;
> +		ret = n;
> +		goto ret;
>   	}
>   
>   	list_items = calloc(n, sizeof(*list_items));
> @@ -1867,7 +1912,7 @@ static int list(int argc, char **argv, struct command *cmd, struct plugin *plugi
>   	for (i = 0; i < n; i++)
>   		free(devices[i]);
>   	free(devices);
> -
> + ret:
>   	return ret;
>   }
>   
> @@ -1905,8 +1950,10 @@ int __id_ctrl(int argc, char **argv, struct command *cmd, struct plugin *plugin,
>   	};
>   
>   	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
> -	if (fd < 0)
> -		return fd;
> +	if (fd < 0) {
> +		err = fd;
> +		goto ret;
> +	}
>   
>   	fmt = validate_output_format(cfg.output_format);
>   	if (fmt < 0) {
> @@ -1941,7 +1988,7 @@ int __id_ctrl(int argc, char **argv, struct command *cmd, struct plugin *plugin,
>   
>    close_fd:
>   	close(fd);
> -
> + ret:
>   	return err;
>   }
>   
> @@ -1978,8 +2025,10 @@ static int ns_descs(int argc, char **argv, struct command *cmd, struct plugin *p
>   	};
>   
>   	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
> -	if (fd < 0)
> -		return fd;
> +	if (fd < 0) {
> +		err = fd;
> +		goto ret;
> +	}
>   
>   	fmt = validate_output_format(cfg.output_format);
>   	if (fmt < 0) {
> @@ -2022,7 +2071,7 @@ static int ns_descs(int argc, char **argv, struct command *cmd, struct plugin *p
>   
>    close_fd:
>   	close(fd);
> -
> + ret:
>   	return err;
>   }
>   
> @@ -2066,8 +2115,10 @@ static int id_ns(int argc, char **argv, struct command *cmd, struct plugin *plug
>   	};
>   
>   	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
> -	if (fd < 0)
> -		return fd;
> +	if (fd < 0) {
> +		err = fd;
> +		goto ret;
> +	}
>   
>   	fmt = validate_output_format(cfg.output_format);
>   	if (fmt < 0) {
> @@ -2109,7 +2160,7 @@ static int id_ns(int argc, char **argv, struct command *cmd, struct plugin *plug
>   
>    close_fd:
>   	close(fd);
> -
> + ret:
>   	return err;
>   }
>   
> @@ -2140,8 +2191,10 @@ static int id_nvmset(int argc, char **argv, struct command *cmd, struct plugin *
>   	};
>   
>   	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
> -	if (fd < 0)
> -		return fd;
> +	if (fd < 0) {
> +		err = fd;
> +		goto ret;
> +	}
>   
>   	fmt = validate_output_format(cfg.output_format);
>   	if (fmt < 0) {
> @@ -2167,13 +2220,13 @@ static int id_nvmset(int argc, char **argv, struct command *cmd, struct plugin *
>   
>    close_fd:
>   	close(fd);
> -
> + ret:
>   	return err;
>   }
>   
>   static int get_ns_id(int argc, char **argv, struct command *cmd, struct plugin *plugin)
>   {
> -	int nsid, fd;
> +	int err = 0, nsid, fd;
>   	const char *desc = "Get namespce ID of a the block device.";
>   
>   	const struct argconfig_commandline_options command_line_options[] = {
> @@ -2181,18 +2234,23 @@ static int get_ns_id(int argc, char **argv, struct command *cmd, struct plugin *
>   	};
>   
>   	fd = parse_and_open(argc, argv, desc, command_line_options, NULL, 0);
> -	if (fd < 0)
> -		return fd;
> +	if (fd < 0) {
> +		err = fd;
> +		goto ret;
> +	}
> +
>   	nsid = nvme_get_nsid(fd);
>   	if (nsid <= 0) {
>   		perror(devicename);
> -		close(fd);
> -		return errno;
> +		err = errno;
> +		goto close_fd;
>   	}
>   	printf("%s: namespace-id:%d\n", devicename, nsid);
>   
> + close_fd:
>   	close(fd);
> -	return 0;
> + ret:
> +	return err;
>   }
>   
>   static int virtual_mgmt(int argc, char **argv, struct command *cmd, struct plugin *plugin)
> @@ -2240,8 +2298,10 @@ static int virtual_mgmt(int argc, char **argv, struct command *cmd, struct plugi
>   	};
>   
>   	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
> -	if (fd < 0)
> -		return fd;
> +	if (fd < 0) {
> +		err = fd;
> +		goto ret;
> +	}
>   
>   	cfg.cdw10 = cfg.cntlid << 16;
>   	cfg.cdw10 = cfg.cdw10 | (cfg.rt << 8);
> @@ -2256,6 +2316,7 @@ static int virtual_mgmt(int argc, char **argv, struct command *cmd, struct plugi
>   		perror("virt-mgmt");
>   
>   	close(fd);
> + ret:
>   	return err;
>   
>   }
> @@ -2293,8 +2354,10 @@ static int list_secondary_ctrl(int argc, char **argv, struct command *cmd, struc
>   	};
>   
>   	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
> -	if (fd < 0)
> -		return fd;
> +	if (fd < 0) {
> +		err = fd;
> +		goto ret;
> +	}
>   
>   	fmt = validate_output_format(cfg.output_format);
>   	if (fmt < 0) {
> @@ -2332,7 +2395,7 @@ static int list_secondary_ctrl(int argc, char **argv, struct command *cmd, struc
>   
>   close_fd:
>   	close(fd);
> -
> +ret:
>   	return err;
>   }
>   
> @@ -2366,8 +2429,10 @@ static int device_self_test(int argc, char **argv, struct command *cmd, struct p
>   	};
>   
>   	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
> -	if (fd < 0)
> -		return fd;
> +	if (fd < 0) {
> +		err = fd;
> +		goto ret;
> +	}
>   
>   	err = nvme_self_test_start(fd, cfg.namespace_id, cfg.cdw10);
>   	if (!err) {
> @@ -2381,6 +2446,7 @@ static int device_self_test(int argc, char **argv, struct command *cmd, struct p
>   		perror("Device self-test");
>   
>   	close(fd);
> + ret:
>   	return err;
>   }
>   
> @@ -2406,8 +2472,10 @@ static int self_test_log(int argc, char **argv, struct command *cmd, struct plug
>   	};
>   
>   	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
> -	if (fd < 0)
> -		return fd;
> +	if (fd < 0) {
> +		err = fd;
> +		goto ret;
> +	}
>   
>   	fmt = validate_output_format(cfg.output_format);
>   	if (fmt < 0) {
> @@ -2436,7 +2504,7 @@ static int self_test_log(int argc, char **argv, struct command *cmd, struct plug
>   
>    close_fd:
>   	close(fd);
> -
> + ret:
>   	return err;
>   }
>   
> @@ -2492,8 +2560,10 @@ static int get_feature(int argc, char **argv, struct command *cmd, struct plugin
>   	};
>   
>   	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
> -	if (fd < 0)
> -		return fd;
> +	if (fd < 0) {
> +		err = fd;
> +		goto ret;
> +	}
>   
>   	if (cfg.sel > 7) {
>   		fprintf(stderr, "invalid 'select' param:%d\n", cfg.sel);
> @@ -2567,7 +2637,7 @@ static int get_feature(int argc, char **argv, struct command *cmd, struct plugin
>   
>    close_fd:
>   	close(fd);
> -
> + ret:
>   	return err;
>   }
>   
> @@ -2609,8 +2679,10 @@ static int fw_download(int argc, char **argv, struct command *cmd, struct plugin
>   	};
>   
>   	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
> -	if (fd < 0)
> -		return fd;
> +	if (fd < 0) {
> +		err = fd;
> +		goto ret;
> +	}
>   
>   	fw_fd = open(cfg.fw, O_RDONLY);
>   	cfg.offset <<= 2;
> @@ -2672,7 +2744,7 @@ static int fw_download(int argc, char **argv, struct command *cmd, struct plugin
>   	close(fw_fd);
>    close_fd:
>   	close(fd);
> -
> + ret:
>   	return err;
>   }
>   
> @@ -2718,8 +2790,10 @@ static int fw_commit(int argc, char **argv, struct command *cmd, struct plugin *
>   	};
>   
>   	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
> -	if (fd < 0)
> -		return fd;
> +	if (fd < 0) {
> +		err = fd;
> +		goto ret;
> +	}
>   
>   	if (cfg.slot > 7) {
>   		fprintf(stderr, "invalid slot:%d\n", cfg.slot);
> @@ -2765,7 +2839,7 @@ static int fw_commit(int argc, char **argv, struct command *cmd, struct plugin *
>   
>    close_fd:
>   	close(fd);
> -
> + ret:
>   	return err;
>   }
>   
> @@ -2779,21 +2853,22 @@ static int subsystem_reset(int argc, char **argv, struct command *cmd, struct pl
>   	};
>   
>   	fd = parse_and_open(argc, argv, desc, command_line_options, NULL, 0);
> -	if (fd < 0)
> -		return fd;
> +	if (fd < 0) {
> +		err = fd;
> +		goto ret;
> +	}
>   
>   	err = nvme_subsystem_reset(fd);
>   	if (err < 0) {
> -		close(fd);
>   		if (errno == ENOTTY)
>   			fprintf(stderr,
>   				"Subsystem-reset: NVM Subsystem Reset not supported.\n");
>   		else
>   			perror("Subsystem-reset");
> -		return errno;
>   	}
>   
>   	close(fd);
> + ret:
>   	return err;
>   }
>   
> @@ -2807,17 +2882,17 @@ static int reset(int argc, char **argv, struct command *cmd, struct plugin *plug
>   	};
>   
>   	fd = parse_and_open(argc, argv, desc, command_line_options, NULL, 0);
> -	if (fd < 0)
> -		return fd;
> +	if (fd < 0) {
> +		err = fd;
> +		goto ret;
> +	}
>   
>   	err = nvme_reset_controller(fd);
> -	if (err < 0) {
> -		close(fd);
> +	if (err < 0)
>   		perror("Reset");
> -		return errno;
> -	}
>   
>   	close(fd);
> + ret:
>   	return err;
>   }
>   
> @@ -2831,17 +2906,17 @@ static int ns_rescan(int argc, char **argv, struct command *cmd, struct plugin *
>   	};
>   
>   	fd = parse_and_open(argc, argv, desc, command_line_options, NULL, 0);
> -	if (fd < 0)
> -		return fd;
> +	if (fd < 0) {
> +		err = fd;
> +		goto ret;
> +	}
>   
>   	err = nvme_ns_rescan(fd);
> -	if (err < 0) {
> -		close(fd);
> +	if (err < 0)
>   		perror("Namespace Rescan");
> -		return errno;
> -	}
>   
>   	close(fd);
> + ret:
>   	return err;
>   }
>   
> @@ -2887,8 +2962,10 @@ static int sanitize(int argc, char **argv, struct command *cmd, struct plugin *p
>   	};
>   
>   	fd = parse_and_open(argc, argv, desc, command_line_options, NULL, 0);
> -	if (fd < 0)
> -		return fd;
> +	if (fd < 0) {
> +		ret = fd;
> +		goto ret;
> +	}
>   
>   	switch (cfg.sanact) {
>   	case NVME_SANITIZE_ACT_CRYPTO_ERASE:
> @@ -2933,7 +3010,7 @@ static int sanitize(int argc, char **argv, struct command *cmd, struct plugin *p
>   
>    close_fd:
>   	close(fd);
> -
> + ret:
>   	return ret;
>   }
>   
> @@ -2965,8 +3042,10 @@ static int show_registers(int argc, char **argv, struct command *cmd, struct plu
>   	};
>   
>   	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
> -	if (fd < 0)
> -		return fd;
> +	if (fd < 0) {
> +		err = fd;
> +		goto ret;
> +	}
>   
>   	fmt = validate_output_format(cfg.output_format);
>   	if (fmt < 0) {
> @@ -3006,7 +3085,7 @@ static int show_registers(int argc, char **argv, struct command *cmd, struct plu
>   
>    close_fd:
>   	close(fd);
> -
> + ret:
>   	return err;
>   }
>   
> @@ -3038,8 +3117,10 @@ static int get_property(int argc, char **argv, struct command *cmd, struct plugi
>   	};
>   
>   	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
> -	if (fd < 0)
> -		return fd;
> +	if (fd < 0) {
> +		err = fd;
> +		goto ret;
> +	}
>   
>   	if (cfg.offset == -1) {
>   		fprintf(stderr, "offset required param");
> @@ -3058,7 +3139,7 @@ static int get_property(int argc, char **argv, struct command *cmd, struct plugi
>   
>    close_fd:
>   	close(fd);
> -
> + ret:
>   	return err;
>   }
>   
> @@ -3087,8 +3168,10 @@ static int set_property(int argc, char **argv, struct command *cmd, struct plugi
>   	};
>   
>   	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
> -	if (fd < 0)
> -		return fd;
> +	if (fd < 0) {
> +		err = fd;
> +		goto ret;
> +	}
>   
>   	if (cfg.offset == -1) {
>   		fprintf(stderr, "offset required param");
> @@ -3113,7 +3196,7 @@ static int set_property(int argc, char **argv, struct command *cmd, struct plugi
>   
>    close_fd:
>   	close(fd);
> -
> + ret:
>   	return err;
>   }
>   
> @@ -3173,8 +3256,10 @@ static int format(int argc, char **argv, struct command *cmd, struct plugin *plu
>   	};
>   
>   	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
> -	if (fd < 0)
> -		return fd;
> +	if (fd < 0) {
> +		err = fd;
> +		goto ret;
> +	}
>   
>   	if (cfg.lbaf != 0xff && cfg.bs !=0) {
>   		fprintf(stderr,
> @@ -3274,7 +3359,7 @@ static int format(int argc, char **argv, struct command *cmd, struct plugin *plu
>   
>    close_fd:
>   	close(fd);
> -
> + ret:
>   	return err;
>   }
>   
> @@ -3332,8 +3417,10 @@ static int set_feature(int argc, char **argv, struct command *cmd, struct plugin
>   	};
>   
>   	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
> -	if (fd < 0)
> -		return fd;
> +	if (fd < 0) {
> +		err = fd;
> +		goto ret;
> +	}
>   
>   	if (!cfg.feature_id) {
>   		fprintf(stderr, "feature-id required param\n");
> @@ -3394,6 +3481,7 @@ static int set_feature(int argc, char **argv, struct command *cmd, struct plugin
>   		free(buf);
>    close_fd:
>   	close(fd);
> + ret:
>   	return err;
>   }
>   
> @@ -3443,8 +3531,10 @@ static int sec_send(int argc, char **argv, struct command *cmd, struct plugin *p
>   	};
>   
>   	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
> -	if (fd < 0)
> -		return fd;
> +	if (fd < 0) {
> +		err = fd;
> +		goto ret;
> +	}
>   
>   	sec_fd = open(cfg.file, O_RDONLY);
>   	if (sec_fd < 0) {
> @@ -3490,6 +3580,7 @@ static int sec_send(int argc, char **argv, struct command *cmd, struct plugin *p
>   	close(sec_fd);
>    close_fd:
>   	close(fd);
> + ret:
>   	return err;
>   }
>   
> @@ -3550,8 +3641,10 @@ static int dir_send(int argc, char **argv, struct command *cmd, struct plugin *p
>   	};
>   
>   	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
> -	if (fd < 0)
> -		return fd;
> +	if (fd < 0) {
> +		err = fd;
> +		goto ret;
> +	}
>   
>   	switch (cfg.dtype) {
>   	case NVME_DIR_IDENTIFY:
> @@ -3641,6 +3734,7 @@ free:
>   		free(buf);
>   close_fd:
>   	close(fd);
> +ret:
>   	return err;
>   }
>   
> @@ -3673,8 +3767,10 @@ static int write_uncor(int argc, char **argv, struct command *cmd, struct plugin
>   	};
>   
>   	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
> -	if (fd < 0)
> -		return fd;
> +	if (fd < 0) {
> +		err = fd;
> +		goto ret;
> +	}
>   
>   	if (!cfg.namespace_id) {
>   		cfg.namespace_id = get_nsid(fd);
> @@ -3695,7 +3791,7 @@ static int write_uncor(int argc, char **argv, struct command *cmd, struct plugin
>   
>   close_fd:
>   	close(fd);
> -
> +ret:
>   	return err;
>   }
>   
> @@ -3753,8 +3849,10 @@ static int write_zeroes(int argc, char **argv, struct command *cmd, struct plugi
>   	};
>   
>   	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
> -	if (fd < 0)
> -		return fd;
> +	if (fd < 0) {
> +		err = fd;
> +		goto ret;
> +	}
>   
>   	if (cfg.prinfo > 0xf) {
>   		err = EINVAL;
> @@ -3787,6 +3885,7 @@ static int write_zeroes(int argc, char **argv, struct command *cmd, struct plugi
>   
>    close_fd:
>   	close(fd);
> + ret:
>   	return err;
>   }
>   
> @@ -3847,8 +3946,10 @@ static int dsm(int argc, char **argv, struct command *cmd, struct plugin *plugin
>   	};
>   
>   	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
> -	if (fd < 0)
> -		return fd;
> +	if (fd < 0) {
> +		err = fd;
> +		goto ret;
> +	}
>   
>   	nc = argconfig_parse_comma_sep_array(cfg.ctx_attrs, ctx_attrs, ARRAY_SIZE(ctx_attrs));
>   	nb = argconfig_parse_comma_sep_array(cfg.blocks, nlbs, ARRAY_SIZE(nlbs));
> @@ -3887,6 +3988,7 @@ static int dsm(int argc, char **argv, struct command *cmd, struct plugin *plugin
>   
>    close_fd:
>   	close(fd);
> + ret:
>   	return err;
>   }
>   
> @@ -3914,8 +4016,10 @@ static int flush(int argc, char **argv, struct command *cmd, struct plugin *plug
>   	};
>   
>   	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
> -	if (fd < 0)
> -		return fd;
> +	if (fd < 0) {
> +		err = fd;
> +		goto ret;
> +	}
>   
>   	if (S_ISBLK(nvme_stat.st_mode)) {
>   		cfg.namespace_id = get_nsid(fd);
> @@ -3934,6 +4038,7 @@ static int flush(int argc, char **argv, struct command *cmd, struct plugin *plug
>   		printf("NVMe Flush: success\n");
>   close_fd:
>   	close(fd);
> +ret:
>   	return err;
>   }
>   
> @@ -3981,8 +4086,10 @@ static int resv_acquire(int argc, char **argv, struct command *cmd, struct plugi
>   	};
>   
>   	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
> -	if (fd < 0)
> -		return fd;
> +	if (fd < 0) {
> +		err = fd;
> +		goto ret;
> +	}
>   
>   	if (!cfg.namespace_id) {
>   		cfg.namespace_id = get_nsid(fd);
> @@ -4008,6 +4115,7 @@ static int resv_acquire(int argc, char **argv, struct command *cmd, struct plugi
>   
>    close_fd:
>   	close(fd);
> + ret:
>   	return err;
>   }
>   
> @@ -4052,8 +4160,10 @@ static int resv_register(int argc, char **argv, struct command *cmd, struct plug
>   	};
>   
>   	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
> -	if (fd < 0)
> -		return fd;
> +	if (fd < 0) {
> +		err = fd;
> +		goto ret;
> +	}
>   
>   	if (!cfg.namespace_id) {
>   		cfg.namespace_id = get_nsid(fd);
> @@ -4085,6 +4195,7 @@ static int resv_register(int argc, char **argv, struct command *cmd, struct plug
>   
>    close_fd:
>   	close(fd);
> + ret:
>   	return err;
>   }
>   
> @@ -4131,8 +4242,10 @@ static int resv_release(int argc, char **argv, struct command *cmd, struct plugi
>   	};
>   
>   	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
> -	if (fd < 0)
> -		return fd;
> +	if (fd < 0) {
> +		err = fd;
> +		goto ret;
> +	}
>   
>   	if (!cfg.namespace_id) {
>   		cfg.namespace_id = get_nsid(fd);
> @@ -4158,6 +4271,7 @@ static int resv_release(int argc, char **argv, struct command *cmd, struct plugi
>   
>    close_fd:
>   	close(fd);
> + ret:
>   	return err;
>   }
>   
> @@ -4201,8 +4315,10 @@ static int resv_report(int argc, char **argv, struct command *cmd, struct plugin
>   	};
>   
>   	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
> -	if (fd < 0)
> -		return fd;
> +	if (fd < 0) {
> +		err = fd;
> +		goto ret;
> +	}
>   
>   	fmt = validate_output_format(cfg.output_format);
>   	if (fmt < 0) {
> @@ -4252,6 +4368,7 @@ static int resv_report(int argc, char **argv, struct command *cmd, struct plugin
>   
>    close_fd:
>   	close(fd);
> + ret:
>   	return err;
>   }
>   
> @@ -4345,8 +4462,10 @@ static int submit_io(int opcode, char *command, const char *desc,
>   	};
>   
>   	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
> -	if (fd < 0)
> -		return fd;
> +	if (fd < 0) {
> +		err = fd;
> +		goto ret;
> +	}
>   
>   	dfd = mfd = opcode & 1 ? STDIN_FILENO : STDOUT_FILENO;
>   	if (cfg.prinfo > 0xf) {
> @@ -4497,6 +4616,7 @@ static int submit_io(int opcode, char *command, const char *desc,
>   	close(dfd);
>    close_fd:
>   	close(fd);
> + ret:
>   	return err;
>   }
>   
> @@ -4571,8 +4691,10 @@ static int sec_recv(int argc, char **argv, struct command *cmd, struct plugin *p
>   	};
>   
>   	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
> -	if (fd < 0)
> -		return fd;
> +	if (fd < 0) {
> +		err = fd;
> +		goto ret;
> +	}
>   
>   	if (cfg.size) {
>   		if (posix_memalign(&sec_buf, getpagesize(), cfg.size)) {
> @@ -4603,6 +4725,7 @@ static int sec_recv(int argc, char **argv, struct command *cmd, struct plugin *p
>   
>    close_fd:
>   	close(fd);
> + ret:
>   	return err;
>   }
>   
> @@ -4656,8 +4779,10 @@ static int dir_receive(int argc, char **argv, struct command *cmd, struct plugin
>   	};
>   
>   	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
> -	if (fd < 0)
> -		return fd;
> +	if (fd < 0) {
> +		err = fd;
> +		goto ret;
> +	}
>   
>   	switch (cfg.dtype) {
>   	case NVME_DIR_IDENTIFY:
> @@ -4733,6 +4858,7 @@ free:
>   		free(buf);
>   close_fd:
>   	close(fd);
> +ret:
>   	return err;
>   }
>   
> @@ -4837,8 +4963,10 @@ static int passthru(int argc, char **argv, int ioctl_cmd, const char *desc, stru
>   	};
>   
>   	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
> -	if (fd < 0)
> -		return fd;
> +	if (fd < 0) {
> +		err = fd;
> +		goto ret;
> +	}
>   
>   	if (strlen(cfg.input_file)){
>   		wfd = open(cfg.input_file, O_RDONLY,
> @@ -4933,6 +5061,7 @@ static int passthru(int argc, char **argv, int ioctl_cmd, const char *desc, stru
>   		close(wfd);
>    close_fd:
>   	close(fd);
> + ret:
>   	return err;
>   }
>   
> 

[PATCH V5 1/6] nvme: Do not return in the middle of the subcommand

[Minwoo Im]

Some functions have label with an whitespace, but some don't.   I didn't
wanted to involve those kind of clean-up things in this series.  I just made labels
with its own function's previuos style, if there's not label, I put an whitespace
prior to label.

I was thinking that it could be cleaned-up at once later.  Do you really think that
those kind of clean-up things are involved in this commit?

Thanks for your review.
Chaitanya,

[PATCH V5 1/6] nvme: Do not return in the middle of the subcommand

[Chaitanya Kulkarni]

On 5/23/19 6:41 PM, Minwoo Im wrote:
> Some functions have label with an whitespace, but some don't.   I didn't
> wanted to involve those kind of clean-up things in this series.  I just made labels
> with its own function's previuos style, if there's not label, I put an whitespace
> prior to label.
>
> I was thinking that it could be cleaned-up at once later.  Do you really think that
> those kind of clean-up things are involved in this commit?

We should at least try and use the default style irrespective of the
inheriting existing style

which will trigger more cleanups. Begin said that now the question is
what is default style ?

I'd just stick to non space labels as that is something been consistent
with kernel code.

>
> Thanks for your review.
> Chaitanya,
>

[PATCH V5 2/6] fabrics: Do not return in the middle of the subcommand

[Chaitanya Kulkarni]

This patch overall looks good to me. Some code level consistency is needed.

On 5/23/19 7:58 AM, Minwoo Im wrote:
> This patch also makes fabrics module to not return the internal error
> status in the middle of the subcommands to support uniformed mapped
> error value which will be introduced in the next patches.
>
> Cc: Keith Busch <kbusch at kernel.org>
> Cc: Chaitanya Kulkarni <chaitanya.Kulkarni at wdc.com>
> Signed-off-by: Minwoo Im <minwoo.im.dev at gmail.com>
> ---
>  fabrics.c | 31 +++++++++++++++++++------------
>  1 file changed, 19 insertions(+), 12 deletions(-)
>
> diff --git a/fabrics.c b/fabrics.c
> index 511de06..469b4a1 100644
> --- a/fabrics.c
> +++ b/fabrics.c
> @@ -984,20 +984,23 @@ int discover(const char *desc, int argc, char **argv, bool connect)
>  	ret = argconfig_parse(argc, argv, desc, command_line_options, &cfg,
>  			sizeof(cfg));
>  	if (ret)
> -		return ret;
> +		goto ret;
>  
>  	cfg.nqn = NVME_DISC_SUBSYS_NAME;
>  
>  	if (!cfg.transport && !cfg.traddr) {
> -		return discover_from_conf_file(desc, argstr,
> +		ret = discover_from_conf_file(desc, argstr,
>  				command_line_options, connect);
>  	} else {
>  		ret = build_options(argstr, BUF_SIZE);
>  		if (ret)
> -			return ret;
> +			goto ret;
>  
> -		return do_discover(argstr, connect);
> +		ret = do_discover(argstr, connect);
>  	}
> +
> + ret:

Particularly in this file you can see label named "out"  being used and
is also prefixed.

Can we make sure this change is consistent with the rest of the code?

> +	return ret;
>  }
>  
>  int connect(const char *desc, int argc, char **argv)
> @@ -1029,21 +1032,23 @@ int connect(const char *desc, int argc, char **argv)
>  	ret = argconfig_parse(argc, argv, desc, command_line_options, &cfg,
>  			sizeof(cfg));
>  	if (ret)
> -		return ret;
> +		goto ret;
>  
>  	ret = build_options(argstr, BUF_SIZE);
>  	if (ret)
> -		return ret;
> +		goto ret;
>  
>  	if (!cfg.nqn) {
>  		fprintf(stderr, "need a -n argument\n");
> -		return -EINVAL;
> +		ret = -EINVAL;
> +		goto ret;
>  	}
>  
>  	instance = add_ctrl(argstr);
>  	if (instance < 0)
> -		return instance;
> -	return 0;
> +		ret = instance;
> + ret:
Same here.
> +	return ret;
>  }
>  
>  static int scan_sys_nvme_filter(const struct dirent *d)
> @@ -1148,11 +1153,12 @@ int disconnect(const char *desc, int argc, char **argv)
>  	ret = argconfig_parse(argc, argv, desc, command_line_options, &cfg,
>  			sizeof(cfg));
>  	if (ret)
> -		return ret;
> +		goto ret;
>  
>  	if (!cfg.nqn && !cfg.device) {
>  		fprintf(stderr, "need a -n or -d argument\n");
> -		return -EINVAL;
> +		ret = -EINVAL;
> +		goto ret;
>  	}
>  
>  	if (cfg.nqn) {
> @@ -1174,6 +1180,7 @@ int disconnect(const char *desc, int argc, char **argv)
>  				cfg.device);
>  	}
>  
> + ret:
>  	
Same here.
> return ret;
>  }
>  
> @@ -1188,7 +1195,7 @@ int disconnect_all(const char *desc, int argc, char **argv)
>  	ret = argconfig_parse(argc, argv, desc, command_line_options, &cfg,
>  			sizeof(cfg));
>  	if (ret)
> -		return ret;
> +		goto out;
>  
>  	slist = get_subsys_list(&subcnt, NULL, NVME_NSID_ALL);
>  	for (i = 0; i < subcnt; i++) {

[PATCH V5 3/6] nvme: Return negative error value for internal errors

[Chaitanya Kulkarni]

Looks good to me.

Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>
On 5/23/19 7:58 AM, Minwoo Im wrote:
> To support the mapping of error values which will be introduced in the
> next patches, All the internal errors caused in the middle of the
> subcommands preparation or teardown should be returned as a negative
> value to make it distinguished from nvme error status which is positive.
>
> This patch makes all the internal error values which used to be returned
> as a positive value to be returned as a negative value.
>
> Cc: Keith Busch <kbusch at kernel.org>
> Cc: Chaitanya Kulkarni <chaitanya.Kulkarni at wdc.com>
> Signed-off-by: Minwoo Im <minwoo.im.dev at gmail.com>
> ---
>  nvme.c | 170 ++++++++++++++++++++++++++++-----------------------------
>  1 file changed, 85 insertions(+), 85 deletions(-)
>
> diff --git a/nvme.c b/nvme.c
> index 88b2cfc..a8d382a 100644
> --- a/nvme.c
> +++ b/nvme.c
> @@ -343,7 +343,7 @@ static int get_telemetry_log(int argc, char **argv, struct command *cmd, struct
>  
>  	if (!cfg.file_name) {
>  		fprintf(stderr, "Please provide an output file!\n");
> -		err = EINVAL;
> +		err = -EINVAL;
>  		goto close_fd;
>  	}
>  
> @@ -353,7 +353,7 @@ static int get_telemetry_log(int argc, char **argv, struct command *cmd, struct
>  	if (!hdr || !page_log) {
>  		fprintf(stderr, "Failed to allocate %zu bytes for log: %s\n",
>  				bs, strerror(errno));
> -		err = ENOMEM;
> +		err = -ENOMEM;
>  		goto free_mem;
>  	}
>  	memset(hdr, 0, bs);
> @@ -393,7 +393,7 @@ static int get_telemetry_log(int argc, char **argv, struct command *cmd, struct
>  		break;
>  	default:
>  		fprintf(stderr, "Invalid data area requested");
> -		err = EINVAL;
> +		err = -EINVAL;
>  		goto close_output;
>  	}
>  
> @@ -597,7 +597,7 @@ static int get_error_log(int argc, char **argv, struct command *cmd, struct plug
>  
>  	if (!cfg.log_entries) {
>  		fprintf(stderr, "non-zero log-entries is required param\n");
> -		err = EINVAL;
> +		err = -EINVAL;
>  		goto close_fd;
>  	}
>  
> @@ -606,7 +606,7 @@ static int get_error_log(int argc, char **argv, struct command *cmd, struct plug
>  		perror("identify controller");
>  	else if (err) {
>  		fprintf(stderr, "could not identify controller\n");
> -		err = ENODEV;
> +		err = -ENODEV;
>  	} else {
>  		struct nvme_error_log_page *err_log;
>  
> @@ -614,7 +614,7 @@ static int get_error_log(int argc, char **argv, struct command *cmd, struct plug
>  		err_log = calloc(cfg.log_entries, sizeof(struct nvme_error_log_page));
>  		if (!err_log) {
>  			fprintf(stderr, "could not alloc buffer for error log\n");
> -			err = ENOMEM;
> +			err = -ENOMEM;
>  			goto close_fd;
>  		}
>  
> @@ -815,13 +815,13 @@ static int get_log(int argc, char **argv, struct command *cmd, struct plugin *pl
>  
>  	if (cfg.log_id > 0xff) {
>  		fprintf(stderr, "Invalid log identifier: %d. Valid range: 0-255\n", cfg.log_id);
> -		err = EINVAL;
> +		err = -EINVAL;
>  		goto close_fd;
>  	}
>  
>  	if (!cfg.log_len) {
>  		fprintf(stderr, "non-zero log-len is required param\n");
> -		err = EINVAL;
> +		err = -EINVAL;
>  	} else {
>  		unsigned char *log;
>  
> @@ -829,7 +829,7 @@ static int get_log(int argc, char **argv, struct command *cmd, struct plugin *pl
>  		if (!log) {
>  			fprintf(stderr, "could not alloc buffer for log: %s\n",
>  					strerror(errno));
> -			err = EINVAL;
> +			err = -EINVAL;
>  			goto close_fd;
>  		}
>  
> @@ -1073,13 +1073,13 @@ static int delete_ns(int argc, char **argv, struct command *cmd, struct plugin *
>  	if (S_ISBLK(nvme_stat.st_mode)) {
>  		cfg.namespace_id = get_nsid(fd);
>  		if (cfg.namespace_id == 0) {
> -			err = EINVAL;
> +			err = -EINVAL;
>  			goto close_fd;
>  		}
>  	} else if (!cfg.namespace_id) {
>  		fprintf(stderr, "%s: namespace-id parameter required\n",
>  						cmd->name);
> -		err = EINVAL;
> +		err = -EINVAL;
>  		goto close_fd;
>  	}
>  
> @@ -1130,7 +1130,7 @@ static int nvme_attach_ns(int argc, char **argv, int attach, const char *desc, s
>  	if (!cfg.namespace_id) {
>  		fprintf(stderr, "%s: namespace-id parameter required\n",
>  						cmd->name);
> -		err = EINVAL;
> +		err = -EINVAL;
>  		goto close_fd;
>  	}
>  
> @@ -1140,7 +1140,7 @@ static int nvme_attach_ns(int argc, char **argv, int attach, const char *desc, s
>      if (num == -1) {
>  		fprintf(stderr, "%s: controller id list is required\n",
>  						cmd->name);
> -		err = EINVAL;
> +		err = -EINVAL;
>  		goto close_fd;
>      }
>  
> @@ -2040,14 +2040,14 @@ static int ns_descs(int argc, char **argv, struct command *cmd, struct plugin *p
>  	if (!cfg.namespace_id) {
>  		cfg.namespace_id = get_nsid(fd);
>  		if (cfg.namespace_id == 0) {
> -			err = EINVAL;
> +			err = -EINVAL;
>  			goto close_fd;
>  		}
>  	}
>  
>  	if (posix_memalign(&nsdescs, getpagesize(), 0x1000)) {
>  		fprintf(stderr, "can not allocate controller list payload\n");
> -		err = ENOMEM;
> +		err = -ENOMEM;
>  		goto close_fd;
>  	}
>  
> @@ -2134,7 +2134,7 @@ static int id_ns(int argc, char **argv, struct command *cmd, struct plugin *plug
>  	if (!cfg.namespace_id && S_ISBLK(nvme_stat.st_mode)) {
>  		cfg.namespace_id = get_nsid(fd);
>  		if (cfg.namespace_id == 0) {
> -			err = EINVAL;
> +			err = -EINVAL;
>  			goto close_fd;
>  		}
>  	}
> @@ -2567,12 +2567,12 @@ static int get_feature(int argc, char **argv, struct command *cmd, struct plugin
>  
>  	if (cfg.sel > 7) {
>  		fprintf(stderr, "invalid 'select' param:%d\n", cfg.sel);
> -		err = EINVAL;
> +		err = -EINVAL;
>  		goto close_fd;
>  	}
>  	if (!cfg.feature_id) {
>  		fprintf(stderr, "feature-id required param\n");
> -		err = EINVAL;
> +		err = -EINVAL;
>  		goto close_fd;
>  	}
>  
> @@ -2606,7 +2606,7 @@ static int get_feature(int argc, char **argv, struct command *cmd, struct plugin
>  	if (cfg.data_len) {
>  		if (posix_memalign(&buf, getpagesize(), cfg.data_len)) {
>  			fprintf(stderr, "can not allocate feature payload\n");
> -			err = ENOMEM;
> +			err = -ENOMEM;
>  			goto close_fd;
>  		}
>  		memset(buf, 0, cfg.data_len);
> @@ -2689,7 +2689,7 @@ static int fw_download(int argc, char **argv, struct command *cmd, struct plugin
>  	if (fw_fd < 0) {
>  		fprintf(stderr, "Failed to open firmware file %s: %s\n",
>  				cfg.fw, strerror(errno));
> -		err = EINVAL;
> +		err = -EINVAL;
>  		goto close_fd;
>  	}
>  
> @@ -2702,12 +2702,12 @@ static int fw_download(int argc, char **argv, struct command *cmd, struct plugin
>  	fw_size = sb.st_size;
>  	if (fw_size & 0x3) {
>  		fprintf(stderr, "Invalid size:%d for f/w image\n", fw_size);
> -		err = EINVAL;
> +		err = -EINVAL;
>  		goto close_fw_fd;
>  	}
>  	if (posix_memalign(&fw_buf, getpagesize(), fw_size)) {
>  		fprintf(stderr, "No memory for f/w size:%d\n", fw_size);
> -		err = ENOMEM;
> +		err = -ENOMEM;
>  		goto close_fw_fd;
>  	}
>  
> @@ -2797,17 +2797,17 @@ static int fw_commit(int argc, char **argv, struct command *cmd, struct plugin *
>  
>  	if (cfg.slot > 7) {
>  		fprintf(stderr, "invalid slot:%d\n", cfg.slot);
> -		err = EINVAL;
> +		err = -EINVAL;
>  		goto close_fd;
>  	}
>  	if (cfg.action > 7 || cfg.action == 4 || cfg.action == 5) {
>  		fprintf(stderr, "invalid action:%d\n", cfg.action);
> -		err = EINVAL;
> +		err = -EINVAL;
>  		goto close_fd;
>  	}
>  	if (cfg.bpid > 1) {
>  		fprintf(stderr, "invalid boot partition id:%d\n", cfg.bpid);
> -		err = EINVAL;
> +		err = -EINVAL;
>  		goto close_fd;
>  	}
>  
> @@ -2975,14 +2975,14 @@ static int sanitize(int argc, char **argv, struct command *cmd, struct plugin *p
>  		break;
>  	default:
>  		fprintf(stderr, "Invalid Sanitize Action\n");
> -		ret = EINVAL;
> +		ret = -EINVAL;
>  		goto close_fd;
>  	}
>  
>  	if (cfg.sanact == NVME_SANITIZE_ACT_EXIT) {
>  	       if (cfg.ause || cfg.no_dealloc) {
>  			fprintf(stderr, "SANACT is Exit Failure Mode\n");
> -			ret = EINVAL;
> +			ret = -EINVAL;
>  			goto close_fd;
>  	       }
>  	}
> @@ -2990,13 +2990,13 @@ static int sanitize(int argc, char **argv, struct command *cmd, struct plugin *p
>  	if (cfg.sanact == NVME_SANITIZE_ACT_OVERWRITE) {
>  		if (cfg.owpass > 16) {
>  			fprintf(stderr, "OWPASS out of range [0-16]\n");
> -			ret = EINVAL;
> +			ret = -EINVAL;
>  			goto close_fd;
>  		}
>  	} else {
>  		if (cfg.owpass || cfg.oipbp || cfg.ovrpat) {
>  			fprintf(stderr, "SANACT is not Overwrite\n");
> -			ret = EINVAL;
> +			ret = -EINVAL;
>  			goto close_fd;
>  		}
>  	}
> @@ -3057,7 +3057,7 @@ static int show_registers(int argc, char **argv, struct command *cmd, struct plu
>  
>  	if (cfg.human_readable && fmt != NORMAL) {
>  		fprintf(stderr, "Only --output-format=normal supports -H\n");
> -		err = EINVAL;
> +		err = -EINVAL;
>  		goto close_fd;
>  	}
>  
> @@ -3069,7 +3069,7 @@ static int show_registers(int argc, char **argv, struct command *cmd, struct plu
>  			err = 0;
>  	}
>  	if (!bar) {
> -		err = ENODEV;
> +		err = -ENODEV;
>  		goto close_fd;
>  	}
>  
> @@ -3124,7 +3124,7 @@ static int get_property(int argc, char **argv, struct command *cmd, struct plugi
>  
>  	if (cfg.offset == -1) {
>  		fprintf(stderr, "offset required param");
> -		err = EINVAL;
> +		err = -EINVAL;
>  		goto close_fd;
>  	}
>  
> @@ -3175,12 +3175,12 @@ static int set_property(int argc, char **argv, struct command *cmd, struct plugi
>  
>  	if (cfg.offset == -1) {
>  		fprintf(stderr, "offset required param");
> -		err = EINVAL;
> +		err = -EINVAL;
>  		goto close_fd;
>  	}
>  	if (cfg.value == -1) {
>  		fprintf(stderr, "value required param");
> -		err = EINVAL;
> +		err = -EINVAL;
>  		goto close_fd;
>  	}
>  
> @@ -3279,7 +3279,7 @@ static int format(int argc, char **argv, struct command *cmd, struct plugin *plu
>  	if (S_ISBLK(nvme_stat.st_mode)) {
>  		cfg.namespace_id = get_nsid(fd);
>  		if (cfg.namespace_id == 0) {
> -			err = EINVAL;
> +			err = -EINVAL;
>  			goto close_fd;
>  		}
>  	}
> @@ -3320,27 +3320,27 @@ static int format(int argc, char **argv, struct command *cmd, struct plugin *plu
>  	/* ses & pi checks set to 7 for forward-compatibility */
>  	if (cfg.ses > 7) {
>  		fprintf(stderr, "invalid secure erase settings:%d\n", cfg.ses);
> -		err = EINVAL;
> +		err = -EINVAL;
>  		goto close_fd;
>  	}
>  	if (cfg.lbaf > 15) {
>  		fprintf(stderr, "invalid lbaf:%d\n", cfg.lbaf);
> -		err = EINVAL;
> +		err = -EINVAL;
>  		goto close_fd;
>  	}
>  	if (cfg.pi > 7) {
>  		fprintf(stderr, "invalid pi:%d\n", cfg.pi);
> -		err = EINVAL;
> +		err = -EINVAL;
>  		goto close_fd;
>  	}
>  	if (cfg.pil > 1) {
>  		fprintf(stderr, "invalid pil:%d\n", cfg.pil);
> -		err = EINVAL;
> +		err = -EINVAL;
>  		goto close_fd;
>  	}
>  	if (cfg.ms > 1) {
>  		fprintf(stderr, "invalid ms:%d\n", cfg.ms);
> -		err = EINVAL;
> +		err = -EINVAL;
>  		goto close_fd;
>  	}
>  
> @@ -3424,7 +3424,7 @@ static int set_feature(int argc, char **argv, struct command *cmd, struct plugin
>  
>  	if (!cfg.feature_id) {
>  		fprintf(stderr, "feature-id required param\n");
> -		err = EINVAL;
> +		err = -EINVAL;
>  		goto close_fd;
>  	}
>  	if (cfg.feature_id == NVME_FEAT_LBA_RANGE)
> @@ -3432,7 +3432,7 @@ static int set_feature(int argc, char **argv, struct command *cmd, struct plugin
>  	if (cfg.data_len) {
>  		if (posix_memalign(&buf, getpagesize(), cfg.data_len)) {
>  			fprintf(stderr, "can not allocate feature payload\n");
> -			err = ENOMEM;
> +			err = -ENOMEM;
>  			goto close_fd;
>  		}
>  		memset(buf, 0, cfg.data_len);
> @@ -3444,7 +3444,7 @@ static int set_feature(int argc, char **argv, struct command *cmd, struct plugin
>  			if (ffd <= 0) {
>  				fprintf(stderr, "Failed to open file %s: %s\n",
>  						cfg.file, strerror(errno));
> -				err = EINVAL;
> +				err = -EINVAL;
>  				goto free;
>  			}
>  		}
> @@ -3540,7 +3540,7 @@ static int sec_send(int argc, char **argv, struct command *cmd, struct plugin *p
>  	if (sec_fd < 0) {
>  		fprintf(stderr, "Failed to open %s: %s\n",
>  				cfg.file, strerror(errno));
> -		err = EINVAL;
> +		err = -EINVAL;
>  		goto close_fd;
>  	}
>  
> @@ -3553,7 +3553,7 @@ static int sec_send(int argc, char **argv, struct command *cmd, struct plugin *p
>  	sec_size = sb.st_size;
>  	if (posix_memalign(&sec_buf, getpagesize(), sec_size)) {
>  		fprintf(stderr, "No memory for security size:%d\n", sec_size);
> -		err = ENOMEM;
> +		err = -ENOMEM;
>  		goto close_sec_fd;
>  	}
>  
> @@ -3652,14 +3652,14 @@ static int dir_send(int argc, char **argv, struct command *cmd, struct plugin *p
>  		case NVME_DIR_SND_ID_OP_ENABLE:
>  			if (!cfg.ttype) {
>  				fprintf(stderr, "target-dir required param\n");
> -				err = EINVAL;
> +				err = -EINVAL;
>  				goto close_fd;
>  			}
>  			dw12 = cfg.ttype << 8 | cfg.endir;
>  			break;
>  		default:
>  			fprintf(stderr, "invalid directive operations for Identify Directives\n");
> -			err = EINVAL;
> +			err = -EINVAL;
>  			goto close_fd;
>  		}
>  		break;
> @@ -3670,20 +3670,20 @@ static int dir_send(int argc, char **argv, struct command *cmd, struct plugin *p
>  			break;
>  		default:
>  			fprintf(stderr, "invalid directive operations for Streams Directives\n");
> -			err = EINVAL;
> +			err = -EINVAL;
>  			goto close_fd;
>  		}
>  		break;
>  	default:
>  		fprintf(stderr, "invalid directive type\n");
> -		err = EINVAL;
> +		err = -EINVAL;
>  		goto close_fd;
>  	}
>  
>  
>  	if (cfg.data_len) {
>  		if (posix_memalign(&buf, getpagesize(), cfg.data_len)) {
> -			err = ENOMEM;
> +			err = -ENOMEM;
>  			goto close_fd;
>  		}
>  		memset(buf, 0, cfg.data_len);
> @@ -3695,7 +3695,7 @@ static int dir_send(int argc, char **argv, struct command *cmd, struct plugin *p
>  			if (ffd <= 0) {
>  				fprintf(stderr, "Failed to open file %s: %s\n",
>  						cfg.file, strerror(errno));
> -				err = EINVAL;
> +				err = -EINVAL;
>  				goto free;
>  			}
>  		}
> @@ -3775,7 +3775,7 @@ static int write_uncor(int argc, char **argv, struct command *cmd, struct plugin
>  	if (!cfg.namespace_id) {
>  		cfg.namespace_id = get_nsid(fd);
>  		if (cfg.namespace_id == 0) {
> -			err = EINVAL;
> +			err = -EINVAL;
>  			goto close_fd;
>  		}
>  	}
> @@ -3855,7 +3855,7 @@ static int write_zeroes(int argc, char **argv, struct command *cmd, struct plugi
>  	}
>  
>  	if (cfg.prinfo > 0xf) {
> -		err = EINVAL;
> +		err = -EINVAL;
>  		goto close_fd;
>  	}
>  
> @@ -3869,7 +3869,7 @@ static int write_zeroes(int argc, char **argv, struct command *cmd, struct plugi
>  	if (!cfg.namespace_id) {
>  		cfg.namespace_id = get_nsid(fd);
>  		if (cfg.namespace_id == 0) {
> -			err = EINVAL;
> +			err = -EINVAL;
>  			goto close_fd;
>  		}
>  	}
> @@ -3957,14 +3957,14 @@ static int dsm(int argc, char **argv, struct command *cmd, struct plugin *plugin
>  	nr = max(nc, max(nb, ns));
>  	if (!nr || nr > 256) {
>  		fprintf(stderr, "No range definition provided\n");
> -		err = EINVAL;
> +		err = -EINVAL;
>  		goto close_fd;
>  	}
>  
>  	if (!cfg.namespace_id) {
>  		cfg.namespace_id = get_nsid(fd);
>  		if (cfg.namespace_id == 0) {
> -			err = EINVAL;
> +			err = -EINVAL;
>  			goto close_fd;
>  		}
>  	}
> @@ -3974,7 +3974,7 @@ static int dsm(int argc, char **argv, struct command *cmd, struct plugin *plugin
>  	dsm = nvme_setup_dsm_range((__u32 *)ctx_attrs, (__u32 *)nlbs, (__u64 *)slbas, nr);
>  	if (!dsm) {
>  		fprintf(stderr, "failed to allocate data set payload\n");
> -		err = ENOMEM;
> +		err = -ENOMEM;
>  		goto close_fd;
>  	}
>  
> @@ -4024,7 +4024,7 @@ static int flush(int argc, char **argv, struct command *cmd, struct plugin *plug
>  	if (S_ISBLK(nvme_stat.st_mode)) {
>  		cfg.namespace_id = get_nsid(fd);
>  		if (cfg.namespace_id == 0) {
> -			err = EINVAL;
> +			err = -EINVAL;
>  			goto close_fd;
>  		}
>  	}
> @@ -4094,13 +4094,13 @@ static int resv_acquire(int argc, char **argv, struct command *cmd, struct plugi
>  	if (!cfg.namespace_id) {
>  		cfg.namespace_id = get_nsid(fd);
>  		if (cfg.namespace_id == 0) {
> -			err = EINVAL;
> +			err = -EINVAL;
>  			goto close_fd;
>  		}
>  	}
>  	if (cfg.racqa > 7) {
>  		fprintf(stderr, "invalid racqa:%d\n", cfg.racqa);
> -		err = EINVAL;
> +		err = -EINVAL;
>  		goto close_fd;
>  	}
>  
> @@ -4168,19 +4168,19 @@ static int resv_register(int argc, char **argv, struct command *cmd, struct plug
>  	if (!cfg.namespace_id) {
>  		cfg.namespace_id = get_nsid(fd);
>  		if (cfg.namespace_id == 0) {
> -			err = EINVAL;
> +			err = -EINVAL;
>  			goto close_fd;
>  		}
>  	}
>  	if (cfg.cptpl > 3) {
>  		fprintf(stderr, "invalid cptpl:%d\n", cfg.cptpl);
> -		err = EINVAL;
> +		err = -EINVAL;
>  		goto close_fd;
>  	}
>  
>  	if (cfg.rrega > 7) {
>  		fprintf(stderr, "invalid rrega:%d\n", cfg.rrega);
> -		err = EINVAL;
> +		err = -EINVAL;
>  		goto close_fd;
>  	}
>  
> @@ -4250,13 +4250,13 @@ static int resv_release(int argc, char **argv, struct command *cmd, struct plugi
>  	if (!cfg.namespace_id) {
>  		cfg.namespace_id = get_nsid(fd);
>  		if (cfg.namespace_id == 0) {
> -			err = EINVAL;
> +			err = -EINVAL;
>  			goto close_fd;
>  		}
>  	}
>  	if (cfg.rrela > 7) {
>  		fprintf(stderr, "invalid rrela:%d\n", cfg.rrela);
> -		err = EINVAL;
> +		err = -EINVAL;
>  		goto close_fd;
>  	}
>  
> @@ -4331,7 +4331,7 @@ static int resv_report(int argc, char **argv, struct command *cmd, struct plugin
>  	if (!cfg.namespace_id) {
>  		cfg.namespace_id = get_nsid(fd);
>  		if (cfg.namespace_id == 0) {
> -			err = EINVAL;
> +			err = -EINVAL;
>  			goto close_fd;
>  		}
>  	}
> @@ -4344,7 +4344,7 @@ static int resv_report(int argc, char **argv, struct command *cmd, struct plugin
>  
>  	if (posix_memalign((void **)&status, getpagesize(), size)) {
>  		fprintf(stderr, "No memory for resv report:%d\n", size);
> -		err = ENOMEM;
> +		err = -ENOMEM;
>  		goto close_fd;
>  	}
>  	memset(status, 0, size);
> @@ -4469,7 +4469,7 @@ static int submit_io(int opcode, char *command, const char *desc,
>  
>  	dfd = mfd = opcode & 1 ? STDIN_FILENO : STDOUT_FILENO;
>  	if (cfg.prinfo > 0xf) {
> -		err = EINVAL;
> +		err = -EINVAL;
>  		goto close_fd;
>  	}
>  
> @@ -4483,7 +4483,7 @@ static int submit_io(int opcode, char *command, const char *desc,
>  		if (cfg.dtype > 0xf) {
>  			fprintf(stderr, "Invalid directive type, %x\n",
>  				cfg.dtype);
> -			err = EINVAL;
> +			err = -EINVAL;
>  			goto close_fd;
>  		}
>  		control |= cfg.dtype << 4;
> @@ -4494,7 +4494,7 @@ static int submit_io(int opcode, char *command, const char *desc,
>  		dfd = open(cfg.data, flags, mode);
>  		if (dfd < 0) {
>  			perror(cfg.data);
> -			err = EINVAL;
> +			err = -EINVAL;
>  			goto close_fd;
>  		}
>  		mfd = dfd;
> @@ -4503,14 +4503,14 @@ static int submit_io(int opcode, char *command, const char *desc,
>  		mfd = open(cfg.metadata, flags, mode);
>  		if (mfd < 0) {
>  			perror(cfg.metadata);
> -			err = EINVAL;
> +			err = -EINVAL;
>  			goto close_dfd;
>  		}
>  	}
>  
>  	if (!cfg.data_size)	{
>  		fprintf(stderr, "data size not provided\n");
> -		err = EINVAL;
> +		err = -EINVAL;
>  		goto close_mfd;
>  	}
>  
> @@ -4527,7 +4527,7 @@ static int submit_io(int opcode, char *command, const char *desc,
>  
>  	if (posix_memalign(&buffer, getpagesize(), buffer_size)) {
>  		fprintf(stderr, "can not allocate io payload\n");
> -		err = ENOMEM;
> +		err = -ENOMEM;
>  		goto close_mfd;
>  	}
>  	memset(buffer, 0, buffer_size);
> @@ -4537,7 +4537,7 @@ static int submit_io(int opcode, char *command, const char *desc,
>  		if (!mbuffer) {
>  			fprintf(stderr, "can not allocate io metadata "
>  					"payload: %s\n", strerror(errno));
> -			err = ENOMEM;
> +			err = -ENOMEM;
>  			goto free_buffer;
>  		}
>  	}
> @@ -4594,12 +4594,12 @@ static int submit_io(int opcode, char *command, const char *desc,
>  		if (!(opcode & 1) && write(dfd, (void *)buffer, cfg.data_size) < 0) {
>  			fprintf(stderr, "write: %s: failed to write buffer to output file\n",
>  					strerror(errno));
> -			err = EINVAL;
> +			err = -EINVAL;
>  		} else if (!(opcode & 1) && cfg.metadata_size &&
>  				write(mfd, (void *)mbuffer, cfg.metadata_size) < 0) {
>  			fprintf(stderr, "write: %s: failed to write meta-data buffer to output file\n",
>  					strerror(errno));
> -			err = EINVAL;
> +			err = -EINVAL;
>  		} else
>  			fprintf(stderr, "%s: Success\n", command);
>  	}
> @@ -4700,7 +4700,7 @@ static int sec_recv(int argc, char **argv, struct command *cmd, struct plugin *p
>  		if (posix_memalign(&sec_buf, getpagesize(), cfg.size)) {
>  			fprintf(stderr, "No memory for security size:%d\n",
>  								cfg.size);
> -			err = ENOMEM;
> +			err = -ENOMEM;
>  			goto close_fd;
>  		}
>  	}
> @@ -4793,7 +4793,7 @@ static int dir_receive(int argc, char **argv, struct command *cmd, struct plugin
>  			break;
>  		default:
>  			fprintf(stderr, "invalid directive operations for Identify Directives\n");
> -			err = EINVAL;
> +			err = -EINVAL;
>  			goto close_fd;
>  		}
>  		break;
> @@ -4812,19 +4812,19 @@ static int dir_receive(int argc, char **argv, struct command *cmd, struct plugin
>  			break;
>  		default:
>  			fprintf(stderr, "invalid directive operations for Streams Directives\n");
> -			err = EINVAL;
> +			err = -EINVAL;
>  			goto close_fd;
>  		}
>  		break;
>  	default:
>  		fprintf(stderr, "invalid directive type\n");
> -		err = EINVAL;
> +		err = -EINVAL;
>  		goto close_fd;
>  	}
>  
>  	if (cfg.data_len) {
>  		if (posix_memalign(&buf, getpagesize(), cfg.data_len)) {
> -			err = ENOMEM;
> +			err = -ENOMEM;
>  			goto close_fd;
>  		}
>  		memset(buf, 0, cfg.data_len);
> @@ -4973,7 +4973,7 @@ static int passthru(int argc, char **argv, int ioctl_cmd, const char *desc, stru
>  			   S_IRUSR | S_IRGRP | S_IROTH);
>  		if (wfd < 0) {
>  			perror(cfg.input_file);
> -			err = EINVAL;
> +			err = -EINVAL;
>  			goto close_fd;
>  		}
>  	}
> @@ -4983,21 +4983,21 @@ static int passthru(int argc, char **argv, int ioctl_cmd, const char *desc, stru
>  		if (!metadata) {
>  			fprintf(stderr, "can not allocate metadata "
>  					"payload: %s\n", strerror(errno));
> -			err = ENOMEM;
> +			err = -ENOMEM;
>  			goto close_wfd;
>  		}
>  	}
>  	if (cfg.data_len) {
>  		if (posix_memalign(&data, getpagesize(), cfg.data_len)) {
>  			fprintf(stderr, "can not allocate data payload\n");
> -			err = ENOMEM;
> +			err = -ENOMEM;
>  			goto free_metadata;
>  		}
>  
>  		memset(data, cfg.prefill, cfg.data_len);
>  		if (!cfg.read && !cfg.write) {
>  			fprintf(stderr, "data direction not given\n");
> -			err = EINVAL;
> +			err = -EINVAL;
>  			goto free_data;
>  		} else if (cfg.write) {
>  			if (read(wfd, data, cfg.data_len) < 0) {

[PATCH V5 4/6] nvme-status: Introduce nvme status module to map errno

[Chaitanya Kulkarni]

Looks good to me.

Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>
On 5/23/19 7:58 AM, Minwoo Im wrote:
> Background:
>   It's not enough to return the nvme status value in main() because it's
> allowed to be in 8bits, but nvme status is indicated in 16bits.  So we
> has not been able to figure out what kind of nvme status has been
> returned by return value.
>
>   This patch introduces nvme-status module that manages mapping between
> nvme status and errno.  It's not possible to make 1:1 mapping relations,
> but we can map it as a groups.
>
> All the internal errors which has been returned in a negative value will
> be returned with ECOMM that indicates communication errors.  In this
> case, we can see what happened via stderr.
>
> Cc: Keith Busch <kbusch at kernel.org>
> Cc: Chaitanya Kulkarni <chaitanya.Kulkarni at wdc.com>
> Signed-off-by: Minwoo Im <minwoo.im.dev at gmail.com>
> ---
>  Makefile      |   3 +-
>  linux/nvme.h  |   6 ++
>  nvme-status.c | 155 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  nvme-status.h |  14 +++++
>  4 files changed, 177 insertions(+), 1 deletion(-)
>  create mode 100644 nvme-status.c
>  create mode 100644 nvme-status.h
>
> diff --git a/Makefile b/Makefile
> index ebb6b75..e0c0fe0 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -32,7 +32,8 @@ override CFLAGS += -DNVME_VERSION='"$(NVME_VERSION)"'
>  NVME_DPKG_VERSION=1~`lsb_release -sc`
>  
>  OBJS := argconfig.o suffix.o parser.o nvme-print.o nvme-ioctl.o \
> -	nvme-lightnvm.o fabrics.o json.o nvme-models.o plugin.o
> +	nvme-lightnvm.o fabrics.o json.o nvme-models.o plugin.o \
> +	nvme-status.o
>  
>  PLUGIN_OBJS :=					\
>  	plugins/intel/intel-nvme.o		\
> diff --git a/linux/nvme.h b/linux/nvme.h
> index 69f287e..9963e90 100644
> --- a/linux/nvme.h
> +++ b/linux/nvme.h
> @@ -1319,6 +1319,12 @@ static inline bool nvme_is_write(struct nvme_command *cmd)
>  	return cmd->common.opcode & 1;
>  }
>  
> +enum {
> +	NVME_SCT_GENERIC		= 0x0,
> +	NVME_SCT_CMD_SPECIFIC		= 0x1,
> +	NVME_SCT_MEDIA			= 0x2,
> +};
> +
>  enum {
>  	/*
>  	 * Generic Command Status:
> diff --git a/nvme-status.c b/nvme-status.c
> new file mode 100644
> index 0000000..c69ff3f
> --- /dev/null
> +++ b/nvme-status.c
> @@ -0,0 +1,155 @@
> +#include <linux/types.h>
> +#include <stdbool.h>
> +#include <errno.h>
> +
> +#include "nvme.h"
> +#include "nvme-status.h"
> +
> +static inline __u8 nvme_generic_status_to_errno(__u16 status)
> +{
> +	switch (status) {
> +	case NVME_SC_INVALID_OPCODE:
> +	case NVME_SC_INVALID_FIELD:
> +	case NVME_SC_INVALID_NS:
> +	case NVME_SC_SGL_INVALID_LAST:
> +	case NVME_SC_SGL_INVALID_COUNT:
> +	case NVME_SC_SGL_INVALID_DATA:
> +	case NVME_SC_SGL_INVALID_METADATA:
> +	case NVME_SC_SGL_INVALID_TYPE:
> +	case NVME_SC_SGL_INVALID_OFFSET:
> +	case NVME_SC_SGL_INVALID_SUBTYPE:
> +		return EINVAL;
> +	case NVME_SC_CMDID_CONFLICT:
> +		return EADDRINUSE;
> +	case NVME_SC_DATA_XFER_ERROR:
> +	case NVME_SC_INTERNAL:
> +	case NVME_SC_SANITIZE_FAILED:
> +		return EIO;
> +	case NVME_SC_POWER_LOSS:
> +	case NVME_SC_ABORT_REQ:
> +	case NVME_SC_ABORT_QUEUE:
> +	case NVME_SC_FUSED_FAIL:
> +	case NVME_SC_FUSED_MISSING:
> +		return EWOULDBLOCK;
> +	case NVME_SC_CMD_SEQ_ERROR:
> +		return EILSEQ;
> +	case NVME_SC_SANITIZE_IN_PROGRESS:
> +		return EINPROGRESS;
> +	case NVME_SC_NS_WRITE_PROTECTED:
> +	case NVME_SC_NS_NOT_READY:
> +	case NVME_SC_RESERVATION_CONFLICT:
> +		return EACCES;
> +	case NVME_SC_LBA_RANGE:
> +		return EREMOTEIO;
> +	case NVME_SC_CAP_EXCEEDED:
> +		return ENOSPC;
> +	}
> +
> +	return EIO;
> +}
> +
> +static inline __u8 nvme_cmd_specific_status_to_errno(__u16 status)
> +{
> +	switch (status) {
> +	case NVME_SC_CQ_INVALID:
> +	case NVME_SC_QID_INVALID:
> +	case NVME_SC_QUEUE_SIZE:
> +	case NVME_SC_FIRMWARE_SLOT:
> +	case NVME_SC_FIRMWARE_IMAGE:
> +	case NVME_SC_INVALID_VECTOR:
> +	case NVME_SC_INVALID_LOG_PAGE:
> +	case NVME_SC_INVALID_FORMAT:
> +	case NVME_SC_INVALID_QUEUE:
> +	case NVME_SC_NS_INSUFFICIENT_CAP:
> +	case NVME_SC_NS_ID_UNAVAILABLE:
> +	case NVME_SC_CTRL_LIST_INVALID:
> +	case NVME_SC_BAD_ATTRIBUTES:
> +	case NVME_SC_INVALID_PI:
> +		return EINVAL;
> +	case NVME_SC_ABORT_LIMIT:
> +	case NVME_SC_ASYNC_LIMIT:
> +		return EDQUOT;
> +	case NVME_SC_FW_NEEDS_CONV_RESET:
> +	case NVME_SC_FW_NEEDS_SUBSYS_RESET:
> +	case NVME_SC_FW_NEEDS_MAX_TIME:
> +		return ERESTART;
> +	case NVME_SC_FEATURE_NOT_SAVEABLE:
> +	case NVME_SC_FEATURE_NOT_CHANGEABLE:
> +	case NVME_SC_FEATURE_NOT_PER_NS:
> +	case NVME_SC_FW_ACTIVATE_PROHIBITED:
> +	case NVME_SC_NS_IS_PRIVATE:
> +	case NVME_SC_BP_WRITE_PROHIBITED:
> +	case NVME_SC_READ_ONLY:
> +		return EPERM;
> +	case NVME_SC_OVERLAPPING_RANGE:
> +	case NVME_SC_NS_NOT_ATTACHED:
> +		return ENOSPC;
> +	case NVME_SC_NS_ALREADY_ATTACHED:
> +		return EALREADY;
> +	case NVME_SC_THIN_PROV_NOT_SUPP:
> +	case NVME_SC_ONCS_NOT_SUPPORTED:
> +		return EOPNOTSUPP;
> +	}
> +
> +	return EIO;
> +}
> +
> +static inline __u8 nvme_fabrics_status_to_errno(__u16 status)
> +{
> +	switch (status) {
> +	case NVME_SC_CONNECT_FORMAT:
> +	case NVME_SC_CONNECT_INVALID_PARAM:
> +		return EINVAL;
> +	case NVME_SC_CONNECT_CTRL_BUSY:
> +		return EBUSY;
> +	case NVME_SC_CONNECT_RESTART_DISC:
> +		return ERESTART;
> +	case NVME_SC_CONNECT_INVALID_HOST:
> +		return ECONNREFUSED;
> +	case NVME_SC_DISCOVERY_RESTART:
> +		return EAGAIN;
> +	case NVME_SC_AUTH_REQUIRED:
> +		return EPERM;
> +	}
> +
> +	return EIO;
> +}
> +
> +/*
> + * nvme_status_to_errno - It converts given status to errno mapped
> + * @status: >= 0 for nvme status field in completion queue entry,
> + *          < 0 for linux internal errors
> + * @fabrics: true if given status is for fabrics
> + *
> + * Notes: This function will convert a given status to an errno mapped
> + */
> +__u8 nvme_status_to_errno(int status, bool fabrics)
> +{
> +	__u8 sct;
> +
> +	if (!status)
> +		return 0;
> +
> +	if (status < 0)
> +		return ECOMM;
> +
> +	/*
> +	 * The actual status code is enough with masking 0xff, but we need to
> +	 * cover status code type which is 3bits with 0x7ff.
> +	 */
> +	status &= 0x7ff;
> +
> +	sct = nvme_status_type(status);
> +	if (sct == NVME_SCT_GENERIC)
> +		return nvme_generic_status_to_errno(status);
> +	else if (sct == NVME_SCT_CMD_SPECIFIC && !fabrics)
> +		return nvme_cmd_specific_status_to_errno(status);
> +	else if (sct == NVME_SCT_CMD_SPECIFIC && fabrics)
> +		return nvme_fabrics_status_to_errno(status);
> +
> +	/*
> +	 * Media, integrity related status, and the others will be mapped to
> +	 * EIO.
> +	 */
> +	return EIO;
> +}
> diff --git a/nvme-status.h b/nvme-status.h
> new file mode 100644
> index 0000000..92a474c
> --- /dev/null
> +++ b/nvme-status.h
> @@ -0,0 +1,14 @@
> +#include <linux/types.h>
> +#include <stdbool.h>
> +
> +/*
> + * nvme_status_type - It gives SCT(Status Code Type) in status field in
> + *                    completion queue entry.
> + * @status: status field located at DW3 in completion queue entry
> + */
> +static inline __u8 nvme_status_type(__u16 status)
> +{
> +	return (status & 0x700) >> 8;
> +}
> +
> +__u8 nvme_status_to_errno(int status, bool fabrics);

[PATCH V5 5/6] nvme: Return errno mapped for nvme error status

[Chaitanya Kulkarni]

Looks good to me.

Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>

On 5/23/19 7:58 AM, Minwoo Im wrote:
> If ioctl module has returned a nvme error status, it will be returned
> by being converted to a mapped errno value which has been provided by
> previous commits.
>
> Cc: Keith Busch <kbusch at kernel.org>
> Cc: Chaitanya Kulkarni <chaitanya.Kulkarni at wdc.com>
> Signed-off-by: Minwoo Im <minwoo.im.dev at gmail.com>
> ---
>  nvme.c | 106 ++++++++++++++++++++++++++++-----------------------------
>  1 file changed, 53 insertions(+), 53 deletions(-)
>
> diff --git a/nvme.c b/nvme.c
> index a8d382a..f3d003b 100644
> --- a/nvme.c
> +++ b/nvme.c
> @@ -48,6 +48,7 @@
>  #include "common.h"
>  #include "nvme-print.h"
>  #include "nvme-ioctl.h"
> +#include "nvme-status.h"
>  #include "nvme-lightnvm.h"
>  #include "plugin.h"
>  
> @@ -223,7 +224,7 @@ static int get_smart_log(int argc, char **argv, struct command *cmd, struct plug
>  	close(fd);
>  
>   ret:
> -	return err;
> +	return nvme_status_to_errno(err, false);
>  }
>  
>  static int get_ana_log(int argc, char **argv, struct command *cmd,
> @@ -298,7 +299,7 @@ static int get_ana_log(int argc, char **argv, struct command *cmd,
>  close_fd:
>  	close(fd);
>  ret:
> -	return err;
> +	return nvme_status_to_errno(err, false);
>  }
>  
>  static int get_telemetry_log(int argc, char **argv, struct command *cmd, struct plugin *plugin)
> @@ -428,7 +429,7 @@ static int get_telemetry_log(int argc, char **argv, struct command *cmd, struct
>   close_fd:
>  	close(fd);
>   ret:
> -	return err;
> +	return nvme_status_to_errno(err, false);
>  }
>  
>  static int get_endurance_log(int argc, char **argv, struct command *cmd, struct plugin *plugin)
> @@ -485,7 +486,7 @@ static int get_endurance_log(int argc, char **argv, struct command *cmd, struct
>   close_fd:
>  	close(fd);
>   ret:
> -	return err;
> +	return nvme_status_to_errno(err, false);
>  }
>  
>  static int get_effects_log(int argc, char **argv, struct command *cmd, struct plugin *plugin)
> @@ -550,7 +551,7 @@ static int get_effects_log(int argc, char **argv, struct command *cmd, struct pl
>   close_fd:
>  	close(fd);
>   ret:
> -	return err;
> +	return nvme_status_to_errno(err, false);
>  }
>  
>  static int get_error_log(int argc, char **argv, struct command *cmd, struct plugin *plugin)
> @@ -637,7 +638,7 @@ static int get_error_log(int argc, char **argv, struct command *cmd, struct plug
>   close_fd:
>  	close(fd);
>   ret:
> -	return err;
> +	return nvme_status_to_errno(err, false);
>  }
>  
>  static int get_fw_log(int argc, char **argv, struct command *cmd, struct plugin *plugin)
> @@ -694,7 +695,7 @@ static int get_fw_log(int argc, char **argv, struct command *cmd, struct plugin
>   close_fd:
>  	close(fd);
>   ret:
> -	return err;
> +	return nvme_status_to_errno(err, false);
>  }
>  
>  static int get_changed_ns_list_log(int argc, char **argv, struct command *cmd, struct plugin *plugin)
> @@ -752,7 +753,7 @@ static int get_changed_ns_list_log(int argc, char **argv, struct command *cmd, s
>   close_fd:
>  	close(fd);
>   ret:
> -	return err;
> +	return nvme_status_to_errno(err, false);
>  }
>  
>  static int get_log(int argc, char **argv, struct command *cmd, struct plugin *plugin)
> @@ -854,7 +855,7 @@ static int get_log(int argc, char **argv, struct command *cmd, struct plugin *pl
>   close_fd:
>  	close(fd);
>   ret:
> -	return err;
> +	return nvme_status_to_errno(err, false);
>  }
>  
>  static int sanitize_log(int argc, char **argv, struct command *command, struct plugin *plugin)
> @@ -919,7 +920,7 @@ static int sanitize_log(int argc, char **argv, struct command *command, struct p
>   close_fd:
>  	close(fd);
>   ret:
> -	return ret;
> +	return nvme_status_to_errno(ret, false);
>  }
>  
>  static int list_ctrl(int argc, char **argv, struct command *cmd, struct plugin *plugin)
> @@ -975,7 +976,7 @@ static int list_ctrl(int argc, char **argv, struct command *cmd, struct plugin *
>  close_fd:
>  	close(fd);
>  ret:
> -	return err;
> +	return nvme_status_to_errno(err, false);
>  }
>  
>  static int list_ns(int argc, char **argv, struct command *cmd, struct plugin *plugin)
> @@ -1021,7 +1022,7 @@ static int list_ns(int argc, char **argv, struct command *cmd, struct plugin *pl
>  
>  	close(fd);
>   ret:
> -	return err;
> +	return nvme_status_to_errno(err, false);
>  }
>  
>  static int get_nsid(int fd)
> @@ -1095,7 +1096,7 @@ static int delete_ns(int argc, char **argv, struct command *cmd, struct plugin *
>   close_fd:
>  	close(fd);
>   ret:
> -	return err;
> +	return nvme_status_to_errno(err, false);
>  }
>  
>  static int nvme_attach_ns(int argc, char **argv, int attach, const char *desc, struct command *cmd)
> @@ -1162,7 +1163,7 @@ static int nvme_attach_ns(int argc, char **argv, int attach, const char *desc, s
>   close_fd:
>  	close(fd);
>   ret:
> -	return err;
> +	return nvme_status_to_errno(err, false);
>  }
>  
>  static int attach_ns(int argc, char **argv, struct command *cmd, struct plugin *plugin)
> @@ -1292,7 +1293,7 @@ static int create_ns(int argc, char **argv, struct command *cmd, struct plugin *
>  close_fd:
>  	close(fd);
>  ret:
> -	return err;
> +	return nvme_status_to_errno(err, false);
>  }
>  
>  char *nvme_char_from_block(char *block)
> @@ -1780,7 +1781,7 @@ static int list_subsys(int argc, char **argv, struct command *cmd,
>  		free(subsysnqn);
>  
>   ret:
> -	return ret;
> +	return nvme_status_to_errno(ret, false);
>  }
>  
>  static int get_nvme_info(int fd, struct list_item *item, const char *node)
> @@ -1913,7 +1914,7 @@ static int list(int argc, char **argv, struct command *cmd, struct plugin *plugi
>  		free(devices[i]);
>  	free(devices);
>   ret:
> -	return ret;
> +	return nvme_status_to_errno(ret, false);
>  }
>  
>  int __id_ctrl(int argc, char **argv, struct command *cmd, struct plugin *plugin, void (*vs)(__u8 *vs, struct json_object *root))
> @@ -1989,7 +1990,7 @@ int __id_ctrl(int argc, char **argv, struct command *cmd, struct plugin *plugin,
>   close_fd:
>  	close(fd);
>   ret:
> -	return err;
> +	return nvme_status_to_errno(err, false);
>  }
>  
>  static int id_ctrl(int argc, char **argv, struct command *cmd, struct plugin *plugin)
> @@ -2072,7 +2073,7 @@ static int ns_descs(int argc, char **argv, struct command *cmd, struct plugin *p
>   close_fd:
>  	close(fd);
>   ret:
> -	return err;
> +	return nvme_status_to_errno(err, false);
>  }
>  
>  static int id_ns(int argc, char **argv, struct command *cmd, struct plugin *plugin)
> @@ -2161,7 +2162,7 @@ static int id_ns(int argc, char **argv, struct command *cmd, struct plugin *plug
>   close_fd:
>  	close(fd);
>   ret:
> -	return err;
> +	return nvme_status_to_errno(err, false);
>  }
>  
>  static int id_nvmset(int argc, char **argv, struct command *cmd, struct plugin *plugin)
> @@ -2221,7 +2222,7 @@ static int id_nvmset(int argc, char **argv, struct command *cmd, struct plugin *
>   close_fd:
>  	close(fd);
>   ret:
> -	return err;
> +	return nvme_status_to_errno(err, false);
>  }
>  
>  static int get_ns_id(int argc, char **argv, struct command *cmd, struct plugin *plugin)
> @@ -2250,7 +2251,7 @@ static int get_ns_id(int argc, char **argv, struct command *cmd, struct plugin *
>   close_fd:
>  	close(fd);
>   ret:
> -	return err;
> +	return nvme_status_to_errno(err, false);
>  }
>  
>  static int virtual_mgmt(int argc, char **argv, struct command *cmd, struct plugin *plugin)
> @@ -2317,8 +2318,7 @@ static int virtual_mgmt(int argc, char **argv, struct command *cmd, struct plugi
>  
>  	close(fd);
>   ret:
> -	return err;
> -
> +	return nvme_status_to_errno(err, false);
>  }
>  
>  static int list_secondary_ctrl(int argc, char **argv, struct command *cmd, struct plugin *plugin)
> @@ -2396,7 +2396,7 @@ static int list_secondary_ctrl(int argc, char **argv, struct command *cmd, struc
>  close_fd:
>  	close(fd);
>  ret:
> -	return err;
> +	return nvme_status_to_errno(err, false);
>  }
>  
>  static int device_self_test(int argc, char **argv, struct command *cmd, struct plugin *plugin)
> @@ -2447,7 +2447,7 @@ static int device_self_test(int argc, char **argv, struct command *cmd, struct p
>  
>  	close(fd);
>   ret:
> -	return err;
> +	return nvme_status_to_errno(err, false);
>  }
>  
>  static int self_test_log(int argc, char **argv, struct command *cmd, struct plugin *plugin)
> @@ -2505,7 +2505,7 @@ static int self_test_log(int argc, char **argv, struct command *cmd, struct plug
>   close_fd:
>  	close(fd);
>   ret:
> -	return err;
> +	return nvme_status_to_errno(err, false);
>  }
>  
>  static int get_feature(int argc, char **argv, struct command *cmd, struct plugin *plugin)
> @@ -2638,7 +2638,7 @@ static int get_feature(int argc, char **argv, struct command *cmd, struct plugin
>   close_fd:
>  	close(fd);
>   ret:
> -	return err;
> +	return nvme_status_to_errno(err, false);
>  }
>  
>  static int fw_download(int argc, char **argv, struct command *cmd, struct plugin *plugin)
> @@ -2745,7 +2745,7 @@ static int fw_download(int argc, char **argv, struct command *cmd, struct plugin
>   close_fd:
>  	close(fd);
>   ret:
> -	return err;
> +	return nvme_status_to_errno(err, false);
>  }
>  
>  static char *nvme_fw_status_reset_type(__u32 status)
> @@ -2840,7 +2840,7 @@ static int fw_commit(int argc, char **argv, struct command *cmd, struct plugin *
>   close_fd:
>  	close(fd);
>   ret:
> -	return err;
> +	return nvme_status_to_errno(err, false);
>  }
>  
>  static int subsystem_reset(int argc, char **argv, struct command *cmd, struct plugin *plugin)
> @@ -2869,7 +2869,7 @@ static int subsystem_reset(int argc, char **argv, struct command *cmd, struct pl
>  
>  	close(fd);
>   ret:
> -	return err;
> +	return nvme_status_to_errno(err, false);
>  }
>  
>  static int reset(int argc, char **argv, struct command *cmd, struct plugin *plugin)
> @@ -2893,7 +2893,7 @@ static int reset(int argc, char **argv, struct command *cmd, struct plugin *plug
>  
>  	close(fd);
>   ret:
> -	return err;
> +	return nvme_status_to_errno(err, false);
>  }
>  
>  static int ns_rescan(int argc, char **argv, struct command *cmd, struct plugin *plugin)
> @@ -2917,7 +2917,7 @@ static int ns_rescan(int argc, char **argv, struct command *cmd, struct plugin *
>  
>  	close(fd);
>   ret:
> -	return err;
> +	return nvme_status_to_errno(err, false);
>  }
>  
>  static int sanitize(int argc, char **argv, struct command *cmd, struct plugin *plugin)
> @@ -3011,7 +3011,7 @@ static int sanitize(int argc, char **argv, struct command *cmd, struct plugin *p
>   close_fd:
>  	close(fd);
>   ret:
> -	return ret;
> +	return nvme_status_to_errno(ret, false);
>  }
>  
>  static int show_registers(int argc, char **argv, struct command *cmd, struct plugin *plugin)
> @@ -3086,7 +3086,7 @@ static int show_registers(int argc, char **argv, struct command *cmd, struct plu
>   close_fd:
>  	close(fd);
>   ret:
> -	return err;
> +	return nvme_status_to_errno(err, false);
>  }
>  
>  static int get_property(int argc, char **argv, struct command *cmd, struct plugin *plugin)
> @@ -3140,7 +3140,7 @@ static int get_property(int argc, char **argv, struct command *cmd, struct plugi
>   close_fd:
>  	close(fd);
>   ret:
> -	return err;
> +	return nvme_status_to_errno(err, false);
>  }
>  
>  static int set_property(int argc, char **argv, struct command *cmd, struct plugin *plugin)
> @@ -3197,7 +3197,7 @@ static int set_property(int argc, char **argv, struct command *cmd, struct plugi
>   close_fd:
>  	close(fd);
>   ret:
> -	return err;
> +	return nvme_status_to_errno(err, false);
>  }
>  
>  static int format(int argc, char **argv, struct command *cmd, struct plugin *plugin)
> @@ -3360,7 +3360,7 @@ static int format(int argc, char **argv, struct command *cmd, struct plugin *plu
>   close_fd:
>  	close(fd);
>   ret:
> -	return err;
> +	return nvme_status_to_errno(err, false);
>  }
>  
>  static int set_feature(int argc, char **argv, struct command *cmd, struct plugin *plugin)
> @@ -3482,7 +3482,7 @@ static int set_feature(int argc, char **argv, struct command *cmd, struct plugin
>   close_fd:
>  	close(fd);
>   ret:
> -	return err;
> +	return nvme_status_to_errno(err, false);
>  }
>  
>  static int sec_send(int argc, char **argv, struct command *cmd, struct plugin *plugin)
> @@ -3581,7 +3581,7 @@ static int sec_send(int argc, char **argv, struct command *cmd, struct plugin *p
>   close_fd:
>  	close(fd);
>   ret:
> -	return err;
> +	return nvme_status_to_errno(err, false);
>  }
>  
>  static int dir_send(int argc, char **argv, struct command *cmd, struct plugin *plugin)
> @@ -3735,7 +3735,7 @@ free:
>  close_fd:
>  	close(fd);
>  ret:
> -	return err;
> +	return nvme_status_to_errno(err, false);
>  }
>  
>  static int write_uncor(int argc, char **argv, struct command *cmd, struct plugin *plugin)
> @@ -3792,7 +3792,7 @@ static int write_uncor(int argc, char **argv, struct command *cmd, struct plugin
>  close_fd:
>  	close(fd);
>  ret:
> -	return err;
> +	return nvme_status_to_errno(err, false);
>  }
>  
>  static int write_zeroes(int argc, char **argv, struct command *cmd, struct plugin *plugin)
> @@ -3886,7 +3886,7 @@ static int write_zeroes(int argc, char **argv, struct command *cmd, struct plugi
>   close_fd:
>  	close(fd);
>   ret:
> -	return err;
> +	return nvme_status_to_errno(err, false);
>  }
>  
>  static int dsm(int argc, char **argv, struct command *cmd, struct plugin *plugin)
> @@ -3989,7 +3989,7 @@ static int dsm(int argc, char **argv, struct command *cmd, struct plugin *plugin
>   close_fd:
>  	close(fd);
>   ret:
> -	return err;
> +	return nvme_status_to_errno(err, false);
>  }
>  
>  static int flush(int argc, char **argv, struct command *cmd, struct plugin *plugin)
> @@ -4039,7 +4039,7 @@ static int flush(int argc, char **argv, struct command *cmd, struct plugin *plug
>  close_fd:
>  	close(fd);
>  ret:
> -	return err;
> +	return nvme_status_to_errno(err, false);
>  }
>  
>  static int resv_acquire(int argc, char **argv, struct command *cmd, struct plugin *plugin)
> @@ -4116,7 +4116,7 @@ static int resv_acquire(int argc, char **argv, struct command *cmd, struct plugi
>   close_fd:
>  	close(fd);
>   ret:
> -	return err;
> +	return nvme_status_to_errno(err, false);
>  }
>  
>  static int resv_register(int argc, char **argv, struct command *cmd, struct plugin *plugin)
> @@ -4196,7 +4196,7 @@ static int resv_register(int argc, char **argv, struct command *cmd, struct plug
>   close_fd:
>  	close(fd);
>   ret:
> -	return err;
> +	return nvme_status_to_errno(err, false);
>  }
>  
>  static int resv_release(int argc, char **argv, struct command *cmd, struct plugin *plugin)
> @@ -4272,7 +4272,7 @@ static int resv_release(int argc, char **argv, struct command *cmd, struct plugi
>   close_fd:
>  	close(fd);
>   ret:
> -	return err;
> +	return nvme_status_to_errno(err, false);
>  }
>  
>  static int resv_report(int argc, char **argv, struct command *cmd, struct plugin *plugin)
> @@ -4369,7 +4369,7 @@ static int resv_report(int argc, char **argv, struct command *cmd, struct plugin
>   close_fd:
>  	close(fd);
>   ret:
> -	return err;
> +	return nvme_status_to_errno(err, false);
>  }
>  
>  static int submit_io(int opcode, char *command, const char *desc,
> @@ -4617,7 +4617,7 @@ static int submit_io(int opcode, char *command, const char *desc,
>   close_fd:
>  	close(fd);
>   ret:
> -	return err;
> +	return nvme_status_to_errno(err, false);
>  }
>  
>  static int compare(int argc, char **argv, struct command *cmd, struct plugin *plugin)
> @@ -4726,7 +4726,7 @@ static int sec_recv(int argc, char **argv, struct command *cmd, struct plugin *p
>   close_fd:
>  	close(fd);
>   ret:
> -	return err;
> +	return nvme_status_to_errno(err, false);
>  }
>  
>  static int dir_receive(int argc, char **argv, struct command *cmd, struct plugin *plugin)
> @@ -4859,7 +4859,7 @@ free:
>  close_fd:
>  	close(fd);
>  ret:
> -	return err;
> +	return nvme_status_to_errno(err, false);
>  }
>  
>  static int passthru(int argc, char **argv, int ioctl_cmd, const char *desc, struct command *cmd)
> @@ -5062,7 +5062,7 @@ static int passthru(int argc, char **argv, int ioctl_cmd, const char *desc, stru
>   close_fd:
>  	close(fd);
>   ret:
> -	return err;
> +	return nvme_status_to_errno(err, false);
>  }
>  
>  static int io_passthru(int argc, char **argv, struct command *cmd, struct plugin *plugin)

[PATCH V5 2/6] fabrics: Do not return in the middle of the subcommand

[Minwoo Im]

Hi Chaitanya,

Thanks for your review.  That sounds absolutely right.
Will prepare V6	 patches with it.

Thanks,

[PATCH V5 1/6] nvme: Do not return in the middle of the subcommand

[Minwoo Im]

> We should at least try and use the default style irrespective of the
> inheriting existing style
> 
> which will trigger more cleanups. Begin said that now the question is
> what is default style ?

Actually, I wanted to ask what the default style is to myself  ;).

> I'd just stick to non space labels as that is something been consistent
> with kernel code.

I think following kernel code style would be better than what these
patches are doing here.

Will prepare V6 series with it.

Thanks,

[PATCH V5 1/6] nvme: Do not return in the middle of the subcommand

[Minwoo Im]

On 19-06-02 23:03:52, Chaitanya Kulkarni wrote:
> On 5/23/19 6:41 PM, Minwoo Im wrote:
> > Some functions have label with an whitespace, but some don't.   I didn't
> > wanted to involve those kind of clean-up things in this series.  I just made labels
> > with its own function's previuos style, if there's not label, I put an whitespace
> > prior to label.
> >
> > I was thinking that it could be cleaned-up at once later.  Do you really think that
> > those kind of clean-up things are involved in this commit?
> 
> We should at least try and use the default style irrespective of the
> inheriting existing style
> 
> which will trigger more cleanups. Begin said that now the question is
> what is default style ?
> 
> I'd just stick to non space labels as that is something been consistent
> with kernel code.

Hmm... Actually I just tried to do all the added "ret" labels with
non-whitespace in front of it.  But it seems that it's not looking good
though because it just follows the existing style in each functions.
The new label which has been added without any exsiting labels in that
function are good to be updated into default(?) style.

In commit point of view, if a single label breaks the existing labels
style in that function might make the function dirty at the time of
commit.

I would prefer to follow the existing function's style, and for the case
of new label in a function, it will follow the default sytle which is
non-space label.

Inconsistency among the functions are not good, of course.  It should be
cleaned-up later, I guess.

Thanks,