nvme-cli: finish the move of vendor specific commands to plugins

nvme-cli: finish the move of vendor specific commands to plugins

[Christoph Hellwig]

Move the remaining Intel specific support code from the core to the
Intel plugin.

[PATCH 1/4] move show_intel_smart_log to intel-nvme.c

[Christoph Hellwig]

To fit in with the rest of the plugin infrastructure.

Signed-off-by: Christoph Hellwig <hch at lst.de>
---
 intel-nvme.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
 nvme-print.c | 48 ------------------------------------------------
 nvme-print.h |  1 -
 3 files changed, 50 insertions(+), 49 deletions(-)

diff --git a/intel-nvme.c b/intel-nvme.c
index df0155a..dc13475 100644
--- a/intel-nvme.c
+++ b/intel-nvme.c
@@ -48,6 +48,56 @@ static int id_ctrl(int argc, char **argv, struct command *cmd, struct plugin *pl
 	return __id_ctrl(argc, argv, cmd, plugin, intel_id_ctrl);
 }
 
+static void show_intel_smart_log(struct nvme_additional_smart_log *smart,
+		unsigned int nsid, const char *devname)
+{
+	printf("Additional Smart Log for NVME device:%s namespace-id:%x\n",
+		devname, nsid);
+	printf("key                               normalized raw\n");
+	printf("program_fail_count              : %3d%%       %"PRIu64"\n",
+		smart->program_fail_cnt.norm,
+		int48_to_long(smart->program_fail_cnt.raw));
+	printf("erase_fail_count                : %3d%%       %"PRIu64"\n",
+		smart->erase_fail_cnt.norm,
+		int48_to_long(smart->erase_fail_cnt.raw));
+	printf("wear_leveling                   : %3d%%       min: %u, max: %u, avg: %u\n",
+		smart->wear_leveling_cnt.norm,
+		le16_to_cpu(smart->wear_leveling_cnt.wear_level.min),
+		le16_to_cpu(smart->wear_leveling_cnt.wear_level.max),
+		le16_to_cpu(smart->wear_leveling_cnt.wear_level.avg));
+	printf("end_to_end_error_detection_count: %3d%%       %"PRIu64"\n",
+		smart->e2e_err_cnt.norm,
+		int48_to_long(smart->e2e_err_cnt.raw));
+	printf("crc_error_count                 : %3d%%       %"PRIu64"\n",
+		smart->crc_err_cnt.norm,
+		int48_to_long(smart->crc_err_cnt.raw));
+	printf("timed_workload_media_wear       : %3d%%       %.3f%%\n",
+		smart->timed_workload_media_wear.norm,
+		((float)int48_to_long(smart->timed_workload_media_wear.raw)) / 1024);
+	printf("timed_workload_host_reads       : %3d%%       %"PRIu64"%%\n",
+		smart->timed_workload_host_reads.norm,
+		int48_to_long(smart->timed_workload_host_reads.raw));
+	printf("timed_workload_timer            : %3d%%       %"PRIu64" min\n",
+		smart->timed_workload_timer.norm,
+		int48_to_long(smart->timed_workload_timer.raw));
+	printf("thermal_throttle_status         : %3d%%       %u%%, cnt: %u\n",
+		smart->thermal_throttle_status.norm,
+		smart->thermal_throttle_status.thermal_throttle.pct,
+		smart->thermal_throttle_status.thermal_throttle.count);
+	printf("retry_buffer_overflow_count     : %3d%%       %"PRIu64"\n",
+		smart->retry_buffer_overflow_cnt.norm,
+		int48_to_long(smart->retry_buffer_overflow_cnt.raw));
+	printf("pll_lock_loss_count             : %3d%%       %"PRIu64"\n",
+		smart->pll_lock_loss_cnt.norm,
+		int48_to_long(smart->pll_lock_loss_cnt.raw));
+	printf("nand_bytes_written              : %3d%%       sectors: %"PRIu64"\n",
+		smart->nand_bytes_written.norm,
+		int48_to_long(smart->nand_bytes_written.raw));
+	printf("host_bytes_written              : %3d%%       sectors: %"PRIu64"\n",
+		smart->host_bytes_written.norm,
+		int48_to_long(smart->host_bytes_written.raw));
+}
+
 static int get_additional_smart_log(int argc, char **argv, struct command *cmd, struct plugin *plugin)
 {
 	struct nvme_additional_smart_log smart_log;
diff --git a/nvme-print.c b/nvme-print.c
index 6f40303..4eddbd4 100644
--- a/nvme-print.c
+++ b/nvme-print.c
@@ -814,54 +814,6 @@ void show_smart_log(struct nvme_smart_log *smart, unsigned int nsid, const char
 	}
 }
 
-void show_intel_smart_log(struct nvme_additional_smart_log *smart, unsigned int nsid, const char *devname)
-{
-	printf("Additional Smart Log for NVME device:%s namespace-id:%x\n", devname, nsid);
-	printf("key                               normalized raw\n");
-	printf("program_fail_count              : %3d%%       %"PRIu64"\n",
-		smart->program_fail_cnt.norm,
-		int48_to_long(smart->program_fail_cnt.raw));
-	printf("erase_fail_count                : %3d%%       %"PRIu64"\n",
-		smart->erase_fail_cnt.norm,
-		int48_to_long(smart->erase_fail_cnt.raw));
-	printf("wear_leveling                   : %3d%%       min: %u, max: %u, avg: %u\n",
-		smart->wear_leveling_cnt.norm,
-		le16_to_cpu(smart->wear_leveling_cnt.wear_level.min),
-		le16_to_cpu(smart->wear_leveling_cnt.wear_level.max),
-		le16_to_cpu(smart->wear_leveling_cnt.wear_level.avg));
-	printf("end_to_end_error_detection_count: %3d%%       %"PRIu64"\n",
-		smart->e2e_err_cnt.norm,
-		int48_to_long(smart->e2e_err_cnt.raw));
-	printf("crc_error_count                 : %3d%%       %"PRIu64"\n",
-		smart->crc_err_cnt.norm,
-		int48_to_long(smart->crc_err_cnt.raw));
-	printf("timed_workload_media_wear       : %3d%%       %.3f%%\n",
-		smart->timed_workload_media_wear.norm,
-		((float)int48_to_long(smart->timed_workload_media_wear.raw)) / 1024);
-	printf("timed_workload_host_reads       : %3d%%       %"PRIu64"%%\n",
-		smart->timed_workload_host_reads.norm,
-		int48_to_long(smart->timed_workload_host_reads.raw));
-	printf("timed_workload_timer            : %3d%%       %"PRIu64" min\n",
-		smart->timed_workload_timer.norm,
-		int48_to_long(smart->timed_workload_timer.raw));
-	printf("thermal_throttle_status         : %3d%%       %u%%, cnt: %u\n",
-		smart->thermal_throttle_status.norm,
-		smart->thermal_throttle_status.thermal_throttle.pct,
-		smart->thermal_throttle_status.thermal_throttle.count);
-	printf("retry_buffer_overflow_count     : %3d%%       %"PRIu64"\n",
-		smart->retry_buffer_overflow_cnt.norm,
-		int48_to_long(smart->retry_buffer_overflow_cnt.raw));
-	printf("pll_lock_loss_count             : %3d%%       %"PRIu64"\n",
-		smart->pll_lock_loss_cnt.norm,
-		int48_to_long(smart->pll_lock_loss_cnt.raw));
-	printf("nand_bytes_written              : %3d%%       sectors: %"PRIu64"\n",
-		smart->nand_bytes_written.norm,
-		int48_to_long(smart->nand_bytes_written.raw));
-	printf("host_bytes_written              : %3d%%       sectors: %"PRIu64"\n",
-		smart->host_bytes_written.norm,
-		int48_to_long(smart->host_bytes_written.raw));
-}
-
 char *nvme_feature_to_string(int feature)
 {
 	switch (feature) {
diff --git a/nvme-print.h b/nvme-print.h
index f5e4c8f..cd1c9e7 100644
--- a/nvme-print.h
+++ b/nvme-print.h
@@ -23,7 +23,6 @@ void show_nvme_id_ns(struct nvme_id_ns *ns, unsigned int flags);
 void show_nvme_resv_report(struct nvme_reservation_status *status);
 void show_lba_range(struct nvme_lba_range_type *lbrt, int nr_ranges);
 void show_error_log(struct nvme_error_log_page *err_log, int entries, const char *devname);
-void show_intel_smart_log(struct nvme_additional_smart_log *smart, unsigned int nsid, const char *devname);
 void show_smart_log(struct nvme_smart_log *smart, unsigned int nsid, const char *devname);
 void show_fw_log(struct nvme_firmware_log_page *fw_log, const char *devname);
 void show_ctrl_registers(void *bar, unsigned int mode);
-- 
2.11.0

[PATCH 2/4] remove json_add_smart_log

[Christoph Hellwig]

This is the json variant of the Intel additional smart log, but it
appears to be entirely unused.  If we want to bring it back it should
be moved to intel-nvme.c, though.

Signed-off-by: Christoph Hellwig <hch at lst.de>
---
 nvme-print.c | 53 -----------------------------------------------------
 nvme-print.h |  2 --
 2 files changed, 55 deletions(-)

diff --git a/nvme-print.c b/nvme-print.c
index 4eddbd4..a86902e 100644
--- a/nvme-print.c
+++ b/nvme-print.c
@@ -1408,59 +1408,6 @@ void json_fw_log(struct nvme_firmware_log_page *fw_log, const char *devname)
 	json_free_object(root);
 }
 
-void json_add_smart_log(struct nvme_additional_smart_log *smart,
-			unsigned int nsid, const char *devname)
-{
-	struct json_object *root;
-	struct json_object *data;
-	char fmt[128];
-
-	root = json_create_object();
-	data = json_create_object();
-
-	json_object_add_value_int(data, "Program Fail Count",
-				  int48_to_long(smart->program_fail_cnt.raw));
-	json_object_add_value_int(data, "Erase Fail Count",
-				  int48_to_long(smart->erase_fail_cnt.raw));
-	json_object_add_value_int(data, "Wear Leveling Min",
-				  le16toh(smart->wear_leveling_cnt.wear_level.min));
-	json_object_add_value_int(data, "Wear Leveling Max",
-				  le16toh(smart->wear_leveling_cnt.wear_level.max));
-	json_object_add_value_int(data, "Wear Leveling Avg",
-				  le16toh(smart->wear_leveling_cnt.wear_level.avg));
-	json_object_add_value_int(data, "End-to-end Error Detection Count",
-				  int48_to_long(smart->e2e_err_cnt.raw));
-	json_object_add_value_int(data, "CRC Error Count",
-				  int48_to_long(smart->crc_err_cnt.raw));
-	json_object_add_value_float(data, "Timed Workload Media Wear",
-				    ((long double)int48_to_long(smart->timed_workload_media_wear.raw)) / 1024);
-
-	json_object_add_value_int(data, "Timed Workload Host Reads",
-				  int48_to_long(smart->timed_workload_host_reads.raw));
-	json_object_add_value_int(data, "Timed Workload Timer",
-				  int48_to_long(smart->timed_workload_timer.raw));
-	snprintf(fmt, sizeof(fmt), "%u%%",
-		 smart->thermal_throttle_status.thermal_throttle.pct);
-	json_object_add_value_string(data, "Thermal Throttle status Percentage",
-				     fmt);
-	json_object_add_value_int(data, "Thermal Throttle Status Count",
-				  smart->thermal_throttle_status.thermal_throttle.count);
-	json_object_add_value_int(data, "Retry Buffer Overflow Count",
-				  int48_to_long(smart->retry_buffer_overflow_cnt.raw));
-	json_object_add_value_int(data, "PLL Lock Loss Count",
-				  int48_to_long(smart->pll_lock_loss_cnt.raw));
-	json_object_add_value_int(data, "Nand Bytes Written",
-				  int48_to_long(smart->nand_bytes_written.raw));
-	json_object_add_value_int(data, "Host Bytes Written",
-				  int48_to_long(smart->host_bytes_written.raw));
-
-	snprintf(fmt, sizeof(fmt), "Additional Smart Log for %s", devname);
-	json_object_add_value_object(root, fmt, data);
-	json_print_object(root, NULL);
-	json_free_object(data);
-	json_free_object(root);
-}
-
 void json_smart_log(struct nvme_smart_log *smart, unsigned int nsid, const char *devname)
 {
 	struct json_object *root;
diff --git a/nvme-print.h b/nvme-print.h
index cd1c9e7..0502d0d 100644
--- a/nvme-print.h
+++ b/nvme-print.h
@@ -37,8 +37,6 @@ void json_nvme_id_ns(struct nvme_id_ns *ns, unsigned int flags);
 void json_nvme_resv_report(struct nvme_reservation_status *status);
 void json_error_log(struct nvme_error_log_page *err_log, int entries, const char *devname);
 void json_smart_log(struct nvme_smart_log *smart, unsigned int nsid, const char *devname);
-void json_add_smart_log(struct nvme_additional_smart_log *smart,
-			unsigned int nsid, const char *devname);
 void json_fw_log(struct nvme_firmware_log_page *fw_log, const char *devname);
 void json_print_list_items(struct list_item *items, unsigned amnt);
 
-- 
2.11.0

[PATCH 3/4] remove nvme_intel_smart_log

[Christoph Hellwig]

and just inline it into the only caller.

Signed-off-by: Christoph Hellwig <hch at lst.de>
---
 intel-nvme.c | 3 ++-
 nvme-ioctl.c | 7 -------
 nvme-ioctl.h | 2 --
 3 files changed, 2 insertions(+), 10 deletions(-)

diff --git a/intel-nvme.c b/intel-nvme.c
index dc13475..3446115 100644
--- a/intel-nvme.c
+++ b/intel-nvme.c
@@ -123,7 +123,8 @@ static int get_additional_smart_log(int argc, char **argv, struct command *cmd,
 
 	fd = parse_and_open(argc, argv, desc, command_line_options, &cfg, sizeof(cfg));
 
-	err = nvme_intel_smart_log(fd, cfg.namespace_id, &smart_log);
+	err = nvme_get_log(fd, cfg.namespace_id, 0xca, sizeof(smart_log),
+			&smart_log);
 	if (!err) {
 		if (!cfg.raw_binary)
 			show_intel_smart_log(&smart_log, cfg.namespace_id, devicename);
diff --git a/nvme-ioctl.c b/nvme-ioctl.c
index b74cdc8..a22399a 100644
--- a/nvme-ioctl.c
+++ b/nvme-ioctl.c
@@ -397,13 +397,6 @@ int nvme_smart_log(int fd, __u32 nsid, struct nvme_smart_log *smart_log)
 	return nvme_get_log(fd, nsid, NVME_LOG_SMART, sizeof(*smart_log), smart_log);
 }
 
-int nvme_intel_smart_log(int fd, __u32 nsid,
-			 struct nvme_additional_smart_log *intel_smart_log)
-{
-	return nvme_get_log(fd, nsid, 0xca, sizeof(*intel_smart_log),
-			intel_smart_log);
-}
-
 int nvme_discovery_log(int fd, struct nvmf_disc_rsp_page_hdr *log, __u32 size)
 {
 	return nvme_get_log(fd, 0, NVME_LOG_DISC, size, log);
diff --git a/nvme-ioctl.h b/nvme-ioctl.h
index 434b29e..3beddf8 100644
--- a/nvme-ioctl.h
+++ b/nvme-ioctl.h
@@ -83,8 +83,6 @@ int nvme_fw_log(int fd, struct nvme_firmware_log_page *fw_log);
 int nvme_error_log(int fd, __u32 nsid, int entries,
 		   struct nvme_error_log_page *err_log);
 int nvme_smart_log(int fd, __u32 nsid, struct nvme_smart_log *smart_log);
-int nvme_intel_smart_log(int fd, __u32 nsid,
-			 struct nvme_additional_smart_log *intel_smart_log);
 int nvme_discovery_log(int fd, struct nvmf_disc_rsp_page_hdr *log, __u32 size);
 
 int nvme_feature(int fd, __u8 opcode, __u32 nsid, __u32 cdw10,
-- 
2.11.0

[PATCH 4/4] move struct nvme_additional_smart_log to intel-nvme.c

[Christoph Hellwig]

It's a vendor-specific structure only used by the intel plugin, so
move it there.

Signed-off-by: Christoph Hellwig <hch at lst.de>
---
 intel-nvme.c | 38 ++++++++++++++++++++++++++++++++++++++
 nvme.h       | 38 --------------------------------------
 2 files changed, 38 insertions(+), 38 deletions(-)

diff --git a/intel-nvme.c b/intel-nvme.c
index 3446115..8ff506c 100644
--- a/intel-nvme.c
+++ b/intel-nvme.c
@@ -21,6 +21,44 @@
 #define CREATE_CMD
 #include "intel-nvme.h"
 
+#pragma pack(push,1)
+struct nvme_additional_smart_log_item {
+	__u8			key;
+	__u8			_kp[2];
+	__u8			norm;
+	__u8			_np;
+	union {
+		__u8		raw[6];
+		struct wear_level {
+			__le16	min;
+			__le16	max;
+			__le16	avg;
+		} wear_level ;
+		struct thermal_throttle {
+			__u8	pct;
+			__u32	count;
+		} thermal_throttle;
+	};
+	__u8			_rp;
+};
+#pragma pack(pop)
+
+struct nvme_additional_smart_log {
+	struct nvme_additional_smart_log_item	program_fail_cnt;
+	struct nvme_additional_smart_log_item	erase_fail_cnt;
+	struct nvme_additional_smart_log_item	wear_leveling_cnt;
+	struct nvme_additional_smart_log_item	e2e_err_cnt;
+	struct nvme_additional_smart_log_item	crc_err_cnt;
+	struct nvme_additional_smart_log_item	timed_workload_media_wear;
+	struct nvme_additional_smart_log_item	timed_workload_host_reads;
+	struct nvme_additional_smart_log_item	timed_workload_timer;
+	struct nvme_additional_smart_log_item	thermal_throttle_status;
+	struct nvme_additional_smart_log_item	retry_buffer_overflow_cnt;
+	struct nvme_additional_smart_log_item	pll_lock_loss_cnt;
+	struct nvme_additional_smart_log_item	nand_bytes_written;
+	struct nvme_additional_smart_log_item	host_bytes_written;
+};
+
 static void intel_id_ctrl(__u8 *vs, struct json_object *root)
 {
 	char bl[9];
diff --git a/nvme.h b/nvme.h
index fb2a3a3..397a1a5 100644
--- a/nvme.h
+++ b/nvme.h
@@ -55,44 +55,6 @@ enum {
 	NVME_ID_CNS_CTRL_LIST		= 0x13,
 };
 
-#pragma pack(push,1)
-struct nvme_additional_smart_log_item {
-	__u8			key;
-	__u8			_kp[2];
-	__u8			norm;
-	__u8			_np;
-	union {
-		__u8		raw[6];
-		struct wear_level {
-			__le16	min;
-			__le16	max;
-			__le16	avg;
-		} wear_level ;
-		struct thermal_throttle {
-			__u8	pct;
-			__u32	count;
-		} thermal_throttle;
-	};
-	__u8			_rp;
-};
-#pragma pack(pop)
-
-struct nvme_additional_smart_log {
-	struct nvme_additional_smart_log_item	program_fail_cnt;
-	struct nvme_additional_smart_log_item	erase_fail_cnt;
-	struct nvme_additional_smart_log_item	wear_leveling_cnt;
-	struct nvme_additional_smart_log_item	e2e_err_cnt;
-	struct nvme_additional_smart_log_item	crc_err_cnt;
-	struct nvme_additional_smart_log_item	timed_workload_media_wear;
-	struct nvme_additional_smart_log_item	timed_workload_host_reads;
-	struct nvme_additional_smart_log_item	timed_workload_timer;
-	struct nvme_additional_smart_log_item	thermal_throttle_status;
-	struct nvme_additional_smart_log_item	retry_buffer_overflow_cnt;
-	struct nvme_additional_smart_log_item	pll_lock_loss_cnt;
-	struct nvme_additional_smart_log_item	nand_bytes_written;
-	struct nvme_additional_smart_log_item	host_bytes_written;
-};
-
 struct nvme_host_mem_buffer {
 	__u32			hsize;
 	__u32			hmdlal;
-- 
2.11.0

nvme-cli: finish the move of vendor specific commands to plugins

[Keith Busch]

On Mon, Mar 13, 2017@05:10:50PM -0600, Christoph Hellwig wrote:
> Move the remaining Intel specific support code from the core to the
> Intel plugin.

Looks good, all applied. Thanks!

nvme-cli: finish the move of vendor specific commands to plugins

[Sagi Grimberg]

FWIW, looks good to me too,

Reviewed-by: Sagi Grimberg <sagi at grimberg.me>