From a3d451157684ba9054086537d726fc2d0c61d7ab Mon Sep 17 00:00:00 2001 From: Tomas Winkler Date: Wed, 23 Apr 2025 18:34:34 +0300 Subject: [PATCH 1/3] wdc: add wdc_is_zn350() wrapper Add wdc_is_zn350() wrapper to replace repeating open code condition (device_id == WDC_NVME_ZN350_DEV_ID || device_id == WDC_NVME_ZN350_DEV_ID_1) Signed-off-by: Tomas Winkler --- plugins/wdc/wdc-nvme.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/plugins/wdc/wdc-nvme.c b/plugins/wdc/wdc-nvme.c index 45f3590115..b5d80f780d 100644 --- a/plugins/wdc/wdc-nvme.c +++ b/plugins/wdc/wdc-nvme.c @@ -1576,6 +1576,12 @@ static bool wdc_is_sn650_e1l(__u32 device_id) return false; } +static bool wdc_is_zn350(__u32 device_id) +{ + return (device_id == WDC_NVME_ZN350_DEV_ID || + device_id == WDC_NVME_ZN350_DEV_ID_1); +} + static bool needs_c2_log_page_check(__u32 device_id) { if ((wdc_is_sn640(device_id)) || @@ -2734,7 +2740,7 @@ static bool get_dev_mgment_cbs_data(nvme_root_t r, struct nvme_dev *dev, (int *)&uuid_index) && wdc_is_sn640_3(device_id))) { found = get_dev_mgmt_log_page_lid_data(dev, cbs_data, lid, log_id, uuid_index); - } else if (device_id == WDC_NVME_ZN350_DEV_ID || device_id == WDC_NVME_ZN350_DEV_ID_1) { + } else if (wdc_is_zn350(device_id)) { uuid_index = 0; found = get_dev_mgmt_log_page_lid_data(dev, cbs_data, lid, log_id, uuid_index); } else { @@ -10815,8 +10821,7 @@ static int wdc_log_page_directory(int argc, char **argv, struct command *command ret = wdc_get_pci_ids(r, dev, &device_id, &read_vendor_id); - log_id = (device_id == WDC_NVME_ZN350_DEV_ID || - device_id == WDC_NVME_ZN350_DEV_ID_1) ? + log_id = wdc_is_zn350(device_id) ? WDC_NVME_GET_DEV_MGMNT_LOG_PAGE_ID_C8 : WDC_NVME_GET_DEV_MGMNT_LOG_PAGE_ID; From bb9c1fa25d59d5166732062ce6d50e3324e6d7ee Mon Sep 17 00:00:00 2001 From: Tomas Winkler Date: Wed, 23 Apr 2025 19:36:24 +0300 Subject: [PATCH 2/3] wdc: make char pointer arguments and variables const where appropriate 1. Refactor function signatures to use `const char *` for char pointer arguments that are not intended to be modified. 2. Constify pointers to constant strings. This improves code clarity and prevents accidental modifications to read-only data. Signed-off-by: Tomas Winkler --- plugins/wdc/wdc-nvme.c | 101 +++++++++++++++++++++------------------- plugins/wdc/wdc-utils.c | 6 +-- plugins/wdc/wdc-utils.h | 6 +-- 3 files changed, 59 insertions(+), 54 deletions(-) diff --git a/plugins/wdc/wdc-nvme.c b/plugins/wdc/wdc-nvme.c index b5d80f780d..d7f50f57ee 100644 --- a/plugins/wdc/wdc-nvme.c +++ b/plugins/wdc/wdc-nvme.c @@ -923,15 +923,16 @@ struct __packed feature_latency_monitor { }; static int wdc_get_serial_name(struct nvme_dev *dev, char *file, size_t len, const char *suffix); -static int wdc_create_log_file(char *file, __u8 *drive_log_data, __u32 drive_log_length); +static int wdc_create_log_file(const char *file, const __u8 *drive_log_data, + __u32 drive_log_length); static int wdc_do_clear_dump(struct nvme_dev *dev, __u8 opcode, __u32 cdw12); -static int wdc_do_dump(struct nvme_dev *dev, __u32 opcode, __u32 data_len, __u32 cdw12, char *file, - __u32 xfer_size); +static int wdc_do_dump(struct nvme_dev *dev, __u32 opcode, __u32 data_len, __u32 cdw12, + const char *file, __u32 xfer_size); static int wdc_do_crash_dump(struct nvme_dev *dev, char *file, int type); -static int wdc_crash_dump(struct nvme_dev *dev, char *file, int type); +static int wdc_crash_dump(struct nvme_dev *dev, const char *file, int type); static int wdc_get_crash_dump(int argc, char **argv, struct command *command, struct plugin *plugin); -static int wdc_do_drive_log(struct nvme_dev *dev, char *file); +static int wdc_do_drive_log(struct nvme_dev *dev, const char *file); static int wdc_drive_log(int argc, char **argv, struct command *command, struct plugin *plugin); static const char *wdc_purge_mon_status_to_string(__u32 status); static int wdc_purge(int argc, char **argv, struct command *command, struct plugin *plugin); @@ -955,7 +956,7 @@ static int wdc_namespace_resize(int argc, char **argv, struct command *command, static int wdc_do_namespace_resize(struct nvme_dev *dev, __u32 nsid, __u32 op_option); static int wdc_reason_identifier(int argc, char **argv, struct command *command, struct plugin *plugin); -static int wdc_do_get_reason_id(struct nvme_dev *dev, char *file, int log_id); +static int wdc_do_get_reason_id(struct nvme_dev *dev, const char *file, int log_id); static int wdc_save_reason_id(struct nvme_dev *dev, __u8 *rsn_ident, int size); static int wdc_clear_reason_id(struct nvme_dev *dev); static int wdc_log_page_directory(int argc, char **argv, struct command *command, @@ -2150,8 +2151,8 @@ static int wdc_get_serial_name(struct nvme_dev *dev, char *file, size_t len, return 0; } -static int wdc_create_log_file(char *file, __u8 *drive_log_data, - __u32 drive_log_length) +static int wdc_create_log_file(const char *file, const __u8 *drive_log_data, + __u32 drive_log_length) { int fd; int ret; @@ -3022,7 +3023,7 @@ static __u32 wdc_dump_dui_data_v2(int fd, __u32 dataLen, __u64 offset, __u8 *dum } static int wdc_do_dump(struct nvme_dev *dev, __u32 opcode, __u32 data_len, - __u32 cdw12, char *file, __u32 xfer_size) + __u32 cdw12, const char *file, __u32 xfer_size) { int ret = 0; __u8 *dump_data; @@ -3081,7 +3082,7 @@ static int wdc_do_dump(struct nvme_dev *dev, __u32 opcode, __u32 data_len, } static int wdc_do_dump_e6(int fd, __u32 opcode, __u32 data_len, - __u32 cdw12, char *file, __u32 xfer_size, __u8 *log_hdr) + __u32 cdw12, char *file, __u32 xfer_size, __u8 *log_hdr) { int ret = 0; __u8 *dump_data; @@ -3155,8 +3156,8 @@ static int wdc_do_dump_e6(int fd, __u32 opcode, __u32 data_len, return ret; } -static int wdc_do_cap_telemetry_log(struct nvme_dev *dev, char *file, - __u32 bs, int type, int data_area) +static int wdc_do_cap_telemetry_log(struct nvme_dev *dev, const char *file, + __u32 bs, int type, int data_area) { struct nvme_telemetry_log *log; size_t full_size = 0; @@ -3798,9 +3799,9 @@ static int wdc_cap_diag(int argc, char **argv, struct command *command, struct plugin *plugin) { nvme_root_t r; - char *desc = "Capture Diagnostics Log."; - char *file = "Output file pathname."; - char *size = "Data retrieval transfer size."; + const char *desc = "Capture Diagnostics Log."; + const char *file = "Output file pathname."; + const char *size = "Data retrieval transfer size."; __u64 capabilities = 0; char f[PATH_MAX] = {0}; struct nvme_dev *dev; @@ -4091,7 +4092,7 @@ static int wdc_do_sn730_get_and_tar(int fd, char *outputName) return ret; } -static int dump_internal_logs(struct nvme_dev *dev, char *dir_name, int verbose) +static int dump_internal_logs(struct nvme_dev *dev, const char *dir_name, int verbose) { char file_path[PATH_MAX]; void *telemetry_log; @@ -4190,14 +4191,17 @@ static int dump_internal_logs(struct nvme_dev *dev, char *dir_name, int verbose) static int wdc_vs_internal_fw_log(int argc, char **argv, struct command *command, struct plugin *plugin) { - char *desc = "Internal Firmware Log."; - char *file = "Output file pathname."; - char *size = "Data retrieval transfer size."; - char *data_area = "Data area to retrieve up to. Currently only supported on the SN340, SN640, SN730, and SN840 devices."; - char *file_size = "Output file size. Currently only supported on the SN340 device."; - char *offset = "Output file data offset. Currently only supported on the SN340 device."; - char *type = "Telemetry type - NONE, HOST, or CONTROLLER. Currently only supported on the SN530, SN640, SN730, SN740, SN810, SN840 and ZN350 devices."; - char *verbose = "Display more debug messages."; + const char *desc = "Internal Firmware Log."; + const char *file = "Output file pathname."; + const char *size = "Data retrieval transfer size."; + const char *data_area = + "Data area to retrieve up to. Currently only supported on the SN340, SN640, SN730, and SN840 devices."; + const char *file_size = "Output file size. Currently only supported on the SN340 device."; + const char *offset = + "Output file data offset. Currently only supported on the SN340 device."; + const char *type = + "Telemetry type - NONE, HOST, or CONTROLLER Currently only supported on the SN530, SN640, SN730, SN740, SN810, SN840 and ZN350 devices."; + const char *verbose = "Display more debug messages."; char f[PATH_MAX] = {0}; char fb[PATH_MAX/2] = {0}; char fileSuffix[PATH_MAX] = {0}; @@ -4527,7 +4531,7 @@ static int wdc_do_crash_dump(struct nvme_dev *dev, char *file, int type) return ret; } -static int wdc_crash_dump(struct nvme_dev *dev, char *file, int type) +static int wdc_crash_dump(struct nvme_dev *dev, const char *file, int type) { char f[PATH_MAX] = {0}; const char *dump_type; @@ -4549,7 +4553,7 @@ static int wdc_crash_dump(struct nvme_dev *dev, char *file, int type) return ret; } -static int wdc_do_drive_log(struct nvme_dev *dev, char *file) +static int wdc_do_drive_log(struct nvme_dev *dev, const char *file) { int ret; __u8 *drive_log_data; @@ -4694,8 +4698,8 @@ static int wdc_get_crash_dump(int argc, char **argv, struct command *command, static int wdc_get_pfail_dump(int argc, char **argv, struct command *command, struct plugin *plugin) { - char *desc = "Get Pfail Crash Dump."; - char *file = "Output file pathname."; + const char *desc = "Get Pfail Crash Dump."; + const char *file = "Output file pathname."; __u64 capabilities = 0; struct nvme_dev *dev; struct config { @@ -5133,7 +5137,7 @@ static void wdc_print_latency_monitor_log_json(struct wdc_ssd_latency_monitor_lo { int i, j; char buf[128]; - char *operation[3] = {"Read", "Write", "Trim"}; + const char *operation[3] = {"Read", "Write", "Trim"}; struct json_object *root = json_create_object(); json_object_add_value_int(root, "Feature Status", log_data->feature_status); @@ -5956,7 +5960,7 @@ static void wdc_print_fw_act_history_log_normal(__u8 *data, int num_entries, __u16 oldestEntryIdx = 0, entryIdx = 0; uint64_t timestamp; __u64 timestamp_sec; - char *null_fw = "--------"; + const char *null_fw = "--------"; memset((void *)time_str, '\0', 100); @@ -8959,7 +8963,7 @@ static int wdc_do_clear_pcie_correctable_errors_fid(int fd) static int wdc_clear_pcie_correctable_errors(int argc, char **argv, struct command *command, struct plugin *plugin) { - char *desc = "Clear PCIE Correctable Errors."; + const char *desc = "Clear PCIE Correctable Errors."; __u64 capabilities = 0; struct nvme_dev *dev; nvme_root_t r; @@ -9002,7 +9006,7 @@ static int wdc_clear_pcie_correctable_errors(int argc, char **argv, struct comma static int wdc_drive_status(int argc, char **argv, struct command *command, struct plugin *plugin) { - char *desc = "Get Drive Status."; + const char *desc = "Get Drive Status."; struct nvme_dev *dev; int ret = 0; bool uuid_found = false; @@ -9145,7 +9149,7 @@ static int wdc_drive_status(int argc, char **argv, struct command *command, static int wdc_clear_assert_dump(int argc, char **argv, struct command *command, struct plugin *plugin) { - char *desc = "Clear Assert Dump Present Status."; + const char *desc = "Clear Assert Dump Present Status."; struct nvme_dev *dev; int ret = -1; nvme_root_t r; @@ -9455,7 +9459,7 @@ static int wdc_do_clear_fw_activate_history_fid(int fd) static int wdc_clear_fw_activate_history(int argc, char **argv, struct command *command, struct plugin *plugin) { - char *desc = "Clear FW activate history table."; + const char *desc = "Clear FW activate history table."; __u64 capabilities = 0; struct nvme_dev *dev; nvme_root_t r; @@ -9491,10 +9495,10 @@ static int wdc_clear_fw_activate_history(int argc, char **argv, struct command * static int wdc_vs_telemetry_controller_option(int argc, char **argv, struct command *command, struct plugin *plugin) { - char *desc = "Disable/Enable Controller Option of the Telemetry Log Page."; - char *disable = "Disable controller option of the telemetry log page."; - char *enable = "Enable controller option of the telemetry log page."; - char *status = "Displays the current state of the controller initiated log page."; + const char *desc = "Disable/Enable Controller Option of the Telemetry Log Page."; + const char *disable = "Disable controller option of the telemetry log page."; + const char *enable = "Enable controller option of the telemetry log page."; + const char *status = "Displays the current state of the controller initiated log page."; __u64 capabilities = 0; struct nvme_dev *dev; nvme_root_t r; @@ -9883,7 +9887,8 @@ static int wdc_fetch_log_file_from_device(struct nvme_dev *dev, __u32 fileId, return ret; } -static int wdc_de_get_dump_trace(struct nvme_dev *dev, char *filePath, __u16 binFileNameLen, char *binFileName) +static int wdc_de_get_dump_trace(struct nvme_dev *dev, const char *filePath, __u16 binFileNameLen, + const char *binFileName) { int ret = WDC_STATUS_FAILURE; __u8 *readBuffer = NULL; @@ -10292,8 +10297,8 @@ static int wdc_do_drive_essentials(nvme_root_t r, struct nvme_dev *dev, static int wdc_drive_essentials(int argc, char **argv, struct command *command, struct plugin *plugin) { - char *desc = "Capture Drive Essentials."; - char *dirName = "Output directory pathname."; + const char *desc = "Capture Drive Essentials."; + const char *dirName = "Output directory pathname."; char d[PATH_MAX] = {0}; char k[PATH_MAX] = {0}; __u64 capabilities = 0; @@ -10307,7 +10312,7 @@ static int wdc_drive_essentials(int argc, char **argv, struct command *command, }; struct config cfg = { - .dirName = NULL, + .dirName = NULL, }; OPT_ARGS(opts) = { @@ -10936,7 +10941,7 @@ static int wdc_get_drive_reason_id(struct nvme_dev *dev, char *drive_reason_id, int ret; int res_len = 0; struct nvme_id_ctrl ctrl; - char *reason_id_str = "reason_id"; + const char *reason_id_str = "reason_id"; i = sizeof(ctrl.sn) - 1; j = sizeof(ctrl.mn) - 1; @@ -11056,7 +11061,7 @@ static int wdc_dump_telemetry_hdr(struct nvme_dev *dev, int log_id, struct nvme_ return ret; } -static int wdc_do_get_reason_id(struct nvme_dev *dev, char *file, int log_id) +static int wdc_do_get_reason_id(struct nvme_dev *dev, const char *file, int log_id) { int ret; struct nvme_telemetry_log *log_hdr; @@ -12260,10 +12265,10 @@ static int wdc_cloud_boot_SSD_version(int argc, char **argv, struct command *com static int wdc_enc_get_log(int argc, char **argv, struct command *command, struct plugin *plugin) { - char *desc = "Get Enclosure Log."; - char *file = "Output file pathname."; - char *size = "Data retrieval transfer size."; - char *log = "Enclosure Log Page ID."; + const char *desc = "Get Enclosure Log."; + const char *file = "Output file pathname."; + const char *size = "Data retrieval transfer size."; + const char *log = "Enclosure Log Page ID."; struct nvme_dev *dev; FILE *output_fd; int xfer_size = 0; diff --git a/plugins/wdc/wdc-utils.c b/plugins/wdc/wdc-utils.c index 70a04a4524..a585d4f9d4 100644 --- a/plugins/wdc/wdc-utils.c +++ b/plugins/wdc/wdc-utils.c @@ -97,7 +97,7 @@ int wdc_UtilsGetTime(PUtilsTimeInfo timeInfo) return WDC_STATUS_SUCCESS; } -int wdc_UtilsCreateDir(char *path) +int wdc_UtilsCreateDir(const char *path) { int retStatus; int status = WDC_STATUS_SUCCESS; @@ -118,7 +118,7 @@ int wdc_UtilsCreateDir(char *path) return status; } -int wdc_WriteToFile(char *fileName, char *buffer, unsigned int bufferLen) +int wdc_WriteToFile(const char *fileName, const char *buffer, unsigned int bufferLen) { int status = WDC_STATUS_SUCCESS; FILE *file; @@ -150,7 +150,7 @@ int wdc_WriteToFile(char *fileName, char *buffer, unsigned int bufferLen) * 1 if the pcSrc string is lexically higher than pcDst or * -1 if the pcSrc string is lexically lower than pcDst. */ -int wdc_UtilsStrCompare(char *pcSrc, char *pcDst) +int wdc_UtilsStrCompare(const char *pcSrc, const char *pcDst) { while ((toupper(*pcSrc) == toupper(*pcDst)) && (*pcSrc != '\0')) { pcSrc++; diff --git a/plugins/wdc/wdc-utils.h b/plugins/wdc/wdc-utils.h index 6f536fc581..8e25ee0fe9 100644 --- a/plugins/wdc/wdc-utils.h +++ b/plugins/wdc/wdc-utils.h @@ -73,9 +73,9 @@ typedef struct _UtilsTimeInfo int wdc_UtilsSnprintf(char *buffer, unsigned int sizeOfBuffer, const char *format, ...); void wdc_UtilsDeleteCharFromString(char* buffer, int buffSize, char charToRemove); int wdc_UtilsGetTime(PUtilsTimeInfo timeInfo); -int wdc_UtilsStrCompare(char *pcSrc, char *pcDst); -int wdc_UtilsCreateDir(char *path); -int wdc_WriteToFile(char *fileName, char *buffer, unsigned int bufferLen); +int wdc_UtilsStrCompare(const char *pcSrc, const char *pcDst); +int wdc_UtilsCreateDir(const char *path); +int wdc_WriteToFile(const char *fileName, const char *buffer, unsigned int bufferLen); void wdc_StrFormat(char *formatter, size_t fmt_sz, char *tofmt, size_t tofmtsz); bool wdc_CheckUuidListSupport(struct nvme_dev *dev, struct nvme_id_uuid_list *uuid_list); From bc9fb970cd7de43c49fe4933d1eca9c71c6796c5 Mon Sep 17 00:00:00 2001 From: Tomas Winkler Date: Wed, 23 Apr 2025 18:44:05 +0300 Subject: [PATCH 3/3] wdc: use libnvme's nvme_uuid_find() for UUID lookup Replace the internal implementation for finding specific UUIDs with the `nvme_uuid_find()` function provided by libnvme. This change simplifies the code and improves maintainability by leveraging a well-tested utility function from the library. While the internal implementation might have offered slightly better performance as we looped only once in few cases the clarity and reduced code duplication are preferred. Signed-off-by: Tomas Winkler --- plugins/wdc/wdc-nvme.c | 131 +++++++++++++++++++--------------------- plugins/wdc/wdc-utils.c | 49 --------------- plugins/wdc/wdc-utils.h | 8 +-- 3 files changed, 62 insertions(+), 126 deletions(-) diff --git a/plugins/wdc/wdc-nvme.c b/plugins/wdc/wdc-nvme.c index d7f50f57ee..3d03aeb094 100644 --- a/plugins/wdc/wdc-nvme.c +++ b/plugins/wdc/wdc-nvme.c @@ -524,17 +524,16 @@ enum NVME_FEATURE_IDENTIFIERS { }; /* WDC UUID value */ -const uint8_t WDC_UUID[] = { - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x2d, 0xb9, 0x8c, 0x52, 0x0c, 0x4c, - 0x5a, 0x15, 0xab, 0xe6, 0x33, 0x29, 0x9a, 0x70, 0xdf, 0xd0 +static const __u8 WDC_UUID[NVME_UUID_LEN] = { + 0x2d, 0xb9, 0x8c, 0x52, 0x0c, 0x4c, 0x5a, 0x15, + 0xab, 0xe6, 0x33, 0x29, 0x9a, 0x70, 0xdf, 0xd0 }; + /* WDC_UUID value for SN640_3 devices */ -const uint8_t WDC_UUID_SN640_3[] = { - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, - 0x11, 0x11, 0x22, 0x22, 0x22, 0x22, 0x22, 0x22, 0x22, 0x22 +static const __u8 WDC_UUID_SN640_3[NVME_UUID_LEN] = { + 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, + 0x22, 0x22, 0x22, 0x22, 0x22, 0x22, 0x22, 0x22 }; enum WDC_DRIVE_ESSENTIAL_TYPE { @@ -1582,7 +1581,6 @@ static bool wdc_is_zn350(__u32 device_id) return (device_id == WDC_NVME_ZN350_DEV_ID || device_id == WDC_NVME_ZN350_DEV_ID_1); } - static bool needs_c2_log_page_check(__u32 device_id) { if ((wdc_is_sn640(device_id)) || @@ -2668,9 +2666,11 @@ static bool get_dev_mgment_data(nvme_root_t r, struct nvme_dev *dev, void **data) { bool found = false; - *data = NULL; __u32 device_id = 0, vendor_id = 0; int uuid_index = 0; + struct nvme_id_uuid_list uuid_list; + + *data = NULL; /* The wdc_get_pci_ids function could fail when drives are connected * via a PCIe switch. Therefore, the return code is intentionally @@ -2680,24 +2680,22 @@ static bool get_dev_mgment_data(nvme_root_t r, struct nvme_dev *dev, */ wdc_get_pci_ids(r, dev, &device_id, &vendor_id); - if ((wdc_FindUuidIndex(dev, - (struct nvme_id_uuid_list_entry *)WDC_UUID, - (int *)&uuid_index)) || - (wdc_FindUuidIndex(dev, - (struct nvme_id_uuid_list_entry *)WDC_UUID_SN640_3, - (int *)&uuid_index) && - wdc_is_sn640_3(device_id))) { - found = get_dev_mgmt_log_page_data(dev, data, uuid_index); - } else { - if (!uuid_index && needs_c2_log_page_check(device_id)) { - /* In certain devices that don't support UUID lists, there are multiple - * definitions of the C2 logpage. In those cases, the code - * needs to try two UUID indexes and use an identification algorithm - * to determine which is returning the correct log page data. - */ - uuid_index = 1; - } + memset(&uuid_list, 0, sizeof(struct nvme_id_uuid_list)); + if (wdc_CheckUuidListSupport(dev, &uuid_list)) { + uuid_index = nvme_uuid_find(&uuid_list, WDC_UUID); + if (uuid_index < 0 && wdc_is_sn640_3(device_id)) + uuid_index = nvme_uuid_find(&uuid_list, WDC_UUID_SN640_3); + if (uuid_index > 0) + found = get_dev_mgmt_log_page_data(dev, data, uuid_index); + } else if (needs_c2_log_page_check(device_id)) { + /* In certain devices that don't support UUID lists, there are multiple + * definitions of the C2 logpage. In those cases, the code + * needs to try two UUID indexes and use an identification algorithm + * to determine which is returning the correct log page data. + */ + + uuid_index = 1; found = get_dev_mgmt_log_page_data(dev, data, uuid_index); if (!found) { @@ -2717,11 +2715,12 @@ static bool get_dev_mgment_cbs_data(nvme_root_t r, struct nvme_dev *dev, __u8 log_id, void **cbs_data) { bool found = false; - __u8 uuid_ix = 0; __u8 lid = 0; - *cbs_data = NULL; __u32 device_id = 0, vendor_id = 0; int uuid_index = 0; + struct nvme_id_uuid_list uuid_list; + + *cbs_data = NULL; /* The wdc_get_pci_ids function could fail when drives are connected * via a PCIe switch. Therefore, the return code is intentionally @@ -2733,43 +2732,41 @@ static bool get_dev_mgment_cbs_data(nvme_root_t r, struct nvme_dev *dev, lid = WDC_NVME_GET_DEV_MGMNT_LOG_PAGE_ID; - if ((wdc_FindUuidIndex(dev, - (struct nvme_id_uuid_list_entry *)WDC_UUID, - (int *)&uuid_index)) || - (wdc_FindUuidIndex(dev, - (struct nvme_id_uuid_list_entry *)WDC_UUID_SN640_3, - (int *)&uuid_index) && - wdc_is_sn640_3(device_id))) { - found = get_dev_mgmt_log_page_lid_data(dev, cbs_data, lid, log_id, uuid_index); + memset(&uuid_list, 0, sizeof(struct nvme_id_uuid_list)); + if (wdc_CheckUuidListSupport(dev, &uuid_list)) { + uuid_index = nvme_uuid_find(&uuid_list, WDC_UUID); + if (uuid_index < 0 && wdc_is_sn640_3(device_id)) + uuid_index = nvme_uuid_find(&uuid_list, WDC_UUID_SN640_3); + + if (uuid_index < 0) + found = get_dev_mgmt_log_page_lid_data(dev, cbs_data, lid, + log_id, uuid_index); + } else if (wdc_is_zn350(device_id)) { uuid_index = 0; found = get_dev_mgmt_log_page_lid_data(dev, cbs_data, lid, log_id, uuid_index); - } else { - if (!uuid_index && needs_c2_log_page_check(device_id)) { - /* In certain devices that don't support UUID lists, there are multiple - * definitions of the C2 logpage. In those cases, the code - * needs to try two UUID indexes and use an identification algorithm - * to determine which is returning the correct log page data. - */ - uuid_ix = 1; - } - - found = get_dev_mgmt_log_page_lid_data(dev, cbs_data, lid, log_id, uuid_ix); - + } else if (needs_c2_log_page_check(device_id)) { + /* In certain devices that don't support UUID lists, there are multiple + * definitions of the C2 logpage. In those cases, the code + * needs to try two UUID indexes and use an identification algorithm + * to determine which is returning the correct log page data. + */ + uuid_index = 1; + found = get_dev_mgmt_log_page_lid_data(dev, cbs_data, lid, log_id, uuid_index); if (!found) { /* not found with uuid = 1 try with uuid = 0 */ - uuid_ix = 0; + uuid_index = 0; fprintf(stderr, "Not found, requesting log page with uuid_index %d\n", uuid_index); - found = get_dev_mgmt_log_page_lid_data(dev, cbs_data, lid, log_id, uuid_ix); + found = get_dev_mgmt_log_page_lid_data(dev, cbs_data, lid, log_id, + uuid_index); } } return found; } - static int wdc_get_supported_log_pages(struct nvme_dev *dev, struct nvme_supported_log_pages *supported, int uuid_index) @@ -9009,7 +9006,6 @@ static int wdc_drive_status(int argc, char **argv, struct command *command, const char *desc = "Get Drive Status."; struct nvme_dev *dev; int ret = 0; - bool uuid_found = false; int uuid_index; nvme_root_t r; void *dev_mng_log = NULL; @@ -9020,6 +9016,7 @@ static int wdc_drive_status(int argc, char **argv, struct command *command, __u32 assert_status = 0xFFFFFFFF; __u32 thermal_status = 0xFFFFFFFF; __u64 capabilities = 0; + struct nvme_id_uuid_list uuid_list; OPT_ARGS(opts) = { OPT_END() @@ -9040,12 +9037,12 @@ static int wdc_drive_status(int argc, char **argv, struct command *command, uuid_index = 0; /* Find the WDC UUID index */ - uuid_found = wdc_FindUuidIndex(dev, - (struct nvme_id_uuid_list_entry *)WDC_UUID, - (int *)&uuid_index); + memset(&uuid_list, 0, sizeof(struct nvme_id_uuid_list)); + if (wdc_CheckUuidListSupport(dev, &uuid_list)) + uuid_index = nvme_uuid_find(&uuid_list, WDC_UUID); - if (!uuid_found) - /* WD UUID not found, use default uuid index - 0 */ + /* WD UUID not found, use default uuid index - 0 */ + if (uuid_index < 0) uuid_index = 0; /* verify the 0xC2 Device Manageability log page is supported */ @@ -10781,7 +10778,6 @@ static int wdc_log_page_directory(int argc, char **argv, struct command *command __u32 device_id, read_vendor_id; bool uuid_supported = false; struct nvme_id_uuid_list uuid_list; - bool uuid_found = false; struct config { char *output_format; @@ -10814,7 +10810,6 @@ static int wdc_log_page_directory(int argc, char **argv, struct command *command fprintf(stderr, "ERROR: WDC: unsupported device for this command\n"); ret = -1; } else { - memset(&uuid_list, 0, sizeof(struct nvme_id_uuid_list)); if (wdc_CheckUuidListSupport(dev, &uuid_list)) uuid_supported = true; @@ -10831,20 +10826,16 @@ static int wdc_log_page_directory(int argc, char **argv, struct command *command WDC_NVME_GET_DEV_MGMNT_LOG_PAGE_ID; if (!wdc_is_sn861(device_id)) { - /* Find the WDC UUID index */ - uuid_found = wdc_FindUuidIndex(dev, - (struct nvme_id_uuid_list_entry *)WDC_UUID, - (int *)&uuid_index); + if (uuid_supported) + uuid_index = nvme_uuid_find(&uuid_list, WDC_UUID); - if (!uuid_found) - /* WD UUID not found, use default uuid index - 0 */ + /* WD UUID not found, use default uuid index - 0 */ + if (uuid_index < 0) uuid_index = 0; /* verify the 0xC2 Device Manageability log page is supported */ - if (wdc_nvme_check_supported_log_page(r, dev, - log_id, uuid_index) == false) { - fprintf(stderr, - "%s: ERROR: WDC: 0x%x Log Page not supported\n", + if (!wdc_nvme_check_supported_log_page(r, dev, log_id, uuid_index)) { + fprintf(stderr, "%s: ERROR: WDC: 0x%x Log Page not supported\n", __func__, log_id); ret = -1; goto out; diff --git a/plugins/wdc/wdc-utils.c b/plugins/wdc/wdc-utils.c index a585d4f9d4..31884990b7 100644 --- a/plugins/wdc/wdc-utils.c +++ b/plugins/wdc/wdc-utils.c @@ -29,13 +29,6 @@ #include "nvme-print.h" #include "wdc-utils.h" -/* UUID field with value of 0 indicates end of UUID List*/ -const uint8_t UUID_END[] = { - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 -}; - int wdc_UtilsSnprintf(char *buffer, unsigned int sizeOfBuffer, const char *format, ...) { int res = 0; @@ -196,45 +189,3 @@ bool wdc_CheckUuidListSupport(struct nvme_dev *dev, struct nvme_id_uuid_list *uu return false; } - -bool wdc_UuidEqual(struct nvme_id_uuid_list_entry *entry1, struct nvme_id_uuid_list_entry *entry2) -{ - return !memcmp(entry1->uuid, entry2->uuid, NVME_UUID_LEN); -} - -bool wdc_FindUuidIndex(struct nvme_dev *dev, - struct nvme_id_uuid_list_entry *uuid_entry, - int *uuid_index) -{ - bool uuid_found = false; - int index = 0; - struct nvme_id_uuid_list uuid_list; - - *uuid_index = 0; - - memset(&uuid_list, 0, sizeof(struct nvme_id_uuid_list)); - if (wdc_CheckUuidListSupport(dev, &uuid_list)) { - struct nvme_id_uuid_list_entry *uuid_list_entry_ptr = - (struct nvme_id_uuid_list_entry *)&uuid_list.entry[0]; - - while (index <= NVME_ID_UUID_LIST_MAX && - !wdc_UuidEqual(uuid_list_entry_ptr, - (struct nvme_id_uuid_list_entry *)UUID_END)) { - - if (wdc_UuidEqual(uuid_list_entry_ptr, uuid_entry)) { - uuid_found = true; - break; - } - - index++; - uuid_list_entry_ptr = - (struct nvme_id_uuid_list_entry *)&uuid_list.entry[index]; - } - - if (uuid_found) - *uuid_index = index + 1; - } - - return uuid_found; -} - diff --git a/plugins/wdc/wdc-utils.h b/plugins/wdc/wdc-utils.h index 8e25ee0fe9..545f0675f8 100644 --- a/plugins/wdc/wdc-utils.h +++ b/plugins/wdc/wdc-utils.h @@ -77,10 +77,4 @@ int wdc_UtilsStrCompare(const char *pcSrc, const char *pcDst); int wdc_UtilsCreateDir(const char *path); int wdc_WriteToFile(const char *fileName, const char *buffer, unsigned int bufferLen); void wdc_StrFormat(char *formatter, size_t fmt_sz, char *tofmt, size_t tofmtsz); -bool wdc_CheckUuidListSupport(struct nvme_dev *dev, - struct nvme_id_uuid_list *uuid_list); -bool wdc_UuidEqual(struct nvme_id_uuid_list_entry *entry1, - struct nvme_id_uuid_list_entry *entry2); -bool wdc_FindUuidIndex(struct nvme_dev *dev, - struct nvme_id_uuid_list_entry *uuid_entry, - int *uuid_index); +bool wdc_CheckUuidListSupport(struct nvme_dev *dev, struct nvme_id_uuid_list *uuid_list);