Skip to content

Commit d83b03e

Browse files
tomas-winkler-sndkigaw
tomas-winkler-sndk
authored and
igaw
committed
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 <[email protected]>
1 parent da94d43 commit d83b03e

File tree

3 files changed

+62
-126
lines changed

3 files changed

+62
-126
lines changed

‎plugins/wdc/wdc-nvme.c‎

Lines changed: 61 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -524,17 +524,16 @@ enum NVME_FEATURE_IDENTIFIERS {
524524
};
525525

526526
/* WDC UUID value */
527-
const uint8_t WDC_UUID[] = {
528-
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
529-
0x00, 0x00, 0x00, 0x00, 0x00, 0x2d, 0xb9, 0x8c, 0x52, 0x0c, 0x4c,
530-
0x5a, 0x15, 0xab, 0xe6, 0x33, 0x29, 0x9a, 0x70, 0xdf, 0xd0
527+
static const __u8 WDC_UUID[NVME_UUID_LEN] = {
528+
0x2d, 0xb9, 0x8c, 0x52, 0x0c, 0x4c, 0x5a, 0x15,
529+
0xab, 0xe6, 0x33, 0x29, 0x9a, 0x70, 0xdf, 0xd0
531530
};
532531

532+
533533
/* WDC_UUID value for SN640_3 devices */
534-
const uint8_t WDC_UUID_SN640_3[] = {
535-
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
536-
0x00, 0x00, 0x00, 0x00, 0x00, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11,
537-
0x11, 0x11, 0x22, 0x22, 0x22, 0x22, 0x22, 0x22, 0x22, 0x22
534+
static const __u8 WDC_UUID_SN640_3[NVME_UUID_LEN] = {
535+
0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11,
536+
0x22, 0x22, 0x22, 0x22, 0x22, 0x22, 0x22, 0x22
538537
};
539538

540539
enum WDC_DRIVE_ESSENTIAL_TYPE {
@@ -1582,7 +1581,6 @@ static bool wdc_is_zn350(__u32 device_id)
15821581
return (device_id == WDC_NVME_ZN350_DEV_ID ||
15831582
device_id == WDC_NVME_ZN350_DEV_ID_1);
15841583
}
1585-
15861584
static bool needs_c2_log_page_check(__u32 device_id)
15871585
{
15881586
if ((wdc_is_sn640(device_id)) ||
@@ -2668,9 +2666,11 @@ static bool get_dev_mgment_data(nvme_root_t r, struct nvme_dev *dev,
26682666
void **data)
26692667
{
26702668
bool found = false;
2671-
*data = NULL;
26722669
__u32 device_id = 0, vendor_id = 0;
26732670
int uuid_index = 0;
2671+
struct nvme_id_uuid_list uuid_list;
2672+
2673+
*data = NULL;
26742674

26752675
/* The wdc_get_pci_ids function could fail when drives are connected
26762676
* 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,
26802680
*/
26812681
wdc_get_pci_ids(r, dev, &device_id, &vendor_id);
26822682

2683-
if ((wdc_FindUuidIndex(dev,
2684-
(struct nvme_id_uuid_list_entry *)WDC_UUID,
2685-
(int *)&uuid_index)) ||
2686-
(wdc_FindUuidIndex(dev,
2687-
(struct nvme_id_uuid_list_entry *)WDC_UUID_SN640_3,
2688-
(int *)&uuid_index) &&
2689-
wdc_is_sn640_3(device_id))) {
2690-
found = get_dev_mgmt_log_page_data(dev, data, uuid_index);
2691-
} else {
2692-
if (!uuid_index && needs_c2_log_page_check(device_id)) {
2693-
/* In certain devices that don't support UUID lists, there are multiple
2694-
* definitions of the C2 logpage. In those cases, the code
2695-
* needs to try two UUID indexes and use an identification algorithm
2696-
* to determine which is returning the correct log page data.
2697-
*/
2698-
uuid_index = 1;
2699-
}
2683+
memset(&uuid_list, 0, sizeof(struct nvme_id_uuid_list));
2684+
if (wdc_CheckUuidListSupport(dev, &uuid_list)) {
2685+
uuid_index = nvme_uuid_find(&uuid_list, WDC_UUID);
2686+
if (uuid_index < 0 && wdc_is_sn640_3(device_id))
2687+
uuid_index = nvme_uuid_find(&uuid_list, WDC_UUID_SN640_3);
27002688

2689+
if (uuid_index > 0)
2690+
found = get_dev_mgmt_log_page_data(dev, data, uuid_index);
2691+
} else if (needs_c2_log_page_check(device_id)) {
2692+
/* In certain devices that don't support UUID lists, there are multiple
2693+
* definitions of the C2 logpage. In those cases, the code
2694+
* needs to try two UUID indexes and use an identification algorithm
2695+
* to determine which is returning the correct log page data.
2696+
*/
2697+
2698+
uuid_index = 1;
27012699
found = get_dev_mgmt_log_page_data(dev, data, uuid_index);
27022700

27032701
if (!found) {
@@ -2717,11 +2715,12 @@ static bool get_dev_mgment_cbs_data(nvme_root_t r, struct nvme_dev *dev,
27172715
__u8 log_id, void **cbs_data)
27182716
{
27192717
bool found = false;
2720-
__u8 uuid_ix = 0;
27212718
__u8 lid = 0;
2722-
*cbs_data = NULL;
27232719
__u32 device_id = 0, vendor_id = 0;
27242720
int uuid_index = 0;
2721+
struct nvme_id_uuid_list uuid_list;
2722+
2723+
*cbs_data = NULL;
27252724

27262725
/* The wdc_get_pci_ids function could fail when drives are connected
27272726
* 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,
27332732

27342733
lid = WDC_NVME_GET_DEV_MGMNT_LOG_PAGE_ID;
27352734

2736-
if ((wdc_FindUuidIndex(dev,
2737-
(struct nvme_id_uuid_list_entry *)WDC_UUID,
2738-
(int *)&uuid_index)) ||
2739-
(wdc_FindUuidIndex(dev,
2740-
(struct nvme_id_uuid_list_entry *)WDC_UUID_SN640_3,
2741-
(int *)&uuid_index) &&
2742-
wdc_is_sn640_3(device_id))) {
2743-
found = get_dev_mgmt_log_page_lid_data(dev, cbs_data, lid, log_id, uuid_index);
2735+
memset(&uuid_list, 0, sizeof(struct nvme_id_uuid_list));
2736+
if (wdc_CheckUuidListSupport(dev, &uuid_list)) {
2737+
uuid_index = nvme_uuid_find(&uuid_list, WDC_UUID);
2738+
if (uuid_index < 0 && wdc_is_sn640_3(device_id))
2739+
uuid_index = nvme_uuid_find(&uuid_list, WDC_UUID_SN640_3);
2740+
2741+
if (uuid_index < 0)
2742+
found = get_dev_mgmt_log_page_lid_data(dev, cbs_data, lid,
2743+
log_id, uuid_index);
2744+
27442745
} else if (wdc_is_zn350(device_id)) {
27452746
uuid_index = 0;
27462747
found = get_dev_mgmt_log_page_lid_data(dev, cbs_data, lid, log_id, uuid_index);
2747-
} else {
2748-
if (!uuid_index && needs_c2_log_page_check(device_id)) {
2749-
/* In certain devices that don't support UUID lists, there are multiple
2750-
* definitions of the C2 logpage. In those cases, the code
2751-
* needs to try two UUID indexes and use an identification algorithm
2752-
* to determine which is returning the correct log page data.
2753-
*/
2754-
uuid_ix = 1;
2755-
}
2756-
2757-
found = get_dev_mgmt_log_page_lid_data(dev, cbs_data, lid, log_id, uuid_ix);
2758-
2748+
} else if (needs_c2_log_page_check(device_id)) {
2749+
/* In certain devices that don't support UUID lists, there are multiple
2750+
* definitions of the C2 logpage. In those cases, the code
2751+
* needs to try two UUID indexes and use an identification algorithm
2752+
* to determine which is returning the correct log page data.
2753+
*/
2754+
uuid_index = 1;
2755+
found = get_dev_mgmt_log_page_lid_data(dev, cbs_data, lid, log_id, uuid_index);
27592756
if (!found) {
27602757
/* not found with uuid = 1 try with uuid = 0 */
2761-
uuid_ix = 0;
2758+
uuid_index = 0;
27622759
fprintf(stderr, "Not found, requesting log page with uuid_index %d\n",
27632760
uuid_index);
27642761

2765-
found = get_dev_mgmt_log_page_lid_data(dev, cbs_data, lid, log_id, uuid_ix);
2762+
found = get_dev_mgmt_log_page_lid_data(dev, cbs_data, lid, log_id,
2763+
uuid_index);
27662764
}
27672765
}
27682766

27692767
return found;
27702768
}
27712769

2772-
27732770
static int wdc_get_supported_log_pages(struct nvme_dev *dev,
27742771
struct nvme_supported_log_pages *supported,
27752772
int uuid_index)
@@ -9009,7 +9006,6 @@ static int wdc_drive_status(int argc, char **argv, struct command *command,
90099006
const char *desc = "Get Drive Status.";
90109007
struct nvme_dev *dev;
90119008
int ret = 0;
9012-
bool uuid_found = false;
90139009
int uuid_index;
90149010
nvme_root_t r;
90159011
void *dev_mng_log = NULL;
@@ -9020,6 +9016,7 @@ static int wdc_drive_status(int argc, char **argv, struct command *command,
90209016
__u32 assert_status = 0xFFFFFFFF;
90219017
__u32 thermal_status = 0xFFFFFFFF;
90229018
__u64 capabilities = 0;
9019+
struct nvme_id_uuid_list uuid_list;
90239020

90249021
OPT_ARGS(opts) = {
90259022
OPT_END()
@@ -9040,12 +9037,12 @@ static int wdc_drive_status(int argc, char **argv, struct command *command,
90409037
uuid_index = 0;
90419038

90429039
/* Find the WDC UUID index */
9043-
uuid_found = wdc_FindUuidIndex(dev,
9044-
(struct nvme_id_uuid_list_entry *)WDC_UUID,
9045-
(int *)&uuid_index);
9040+
memset(&uuid_list, 0, sizeof(struct nvme_id_uuid_list));
9041+
if (wdc_CheckUuidListSupport(dev, &uuid_list))
9042+
uuid_index = nvme_uuid_find(&uuid_list, WDC_UUID);
90469043

9047-
if (!uuid_found)
9048-
/* WD UUID not found, use default uuid index - 0 */
9044+
/* WD UUID not found, use default uuid index - 0 */
9045+
if (uuid_index < 0)
90499046
uuid_index = 0;
90509047

90519048
/* 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
1078110778
__u32 device_id, read_vendor_id;
1078210779
bool uuid_supported = false;
1078310780
struct nvme_id_uuid_list uuid_list;
10784-
bool uuid_found = false;
1078510781

1078610782
struct config {
1078710783
char *output_format;
@@ -10814,7 +10810,6 @@ static int wdc_log_page_directory(int argc, char **argv, struct command *command
1081410810
fprintf(stderr, "ERROR: WDC: unsupported device for this command\n");
1081510811
ret = -1;
1081610812
} else {
10817-
1081810813
memset(&uuid_list, 0, sizeof(struct nvme_id_uuid_list));
1081910814
if (wdc_CheckUuidListSupport(dev, &uuid_list))
1082010815
uuid_supported = true;
@@ -10831,20 +10826,16 @@ static int wdc_log_page_directory(int argc, char **argv, struct command *command
1083110826
WDC_NVME_GET_DEV_MGMNT_LOG_PAGE_ID;
1083210827

1083310828
if (!wdc_is_sn861(device_id)) {
10834-
/* Find the WDC UUID index */
10835-
uuid_found = wdc_FindUuidIndex(dev,
10836-
(struct nvme_id_uuid_list_entry *)WDC_UUID,
10837-
(int *)&uuid_index);
10829+
if (uuid_supported)
10830+
uuid_index = nvme_uuid_find(&uuid_list, WDC_UUID);
1083810831

10839-
if (!uuid_found)
10840-
/* WD UUID not found, use default uuid index - 0 */
10832+
/* WD UUID not found, use default uuid index - 0 */
10833+
if (uuid_index < 0)
1084110834
uuid_index = 0;
1084210835

1084310836
/* verify the 0xC2 Device Manageability log page is supported */
10844-
if (wdc_nvme_check_supported_log_page(r, dev,
10845-
log_id, uuid_index) == false) {
10846-
fprintf(stderr,
10847-
"%s: ERROR: WDC: 0x%x Log Page not supported\n",
10837+
if (!wdc_nvme_check_supported_log_page(r, dev, log_id, uuid_index)) {
10838+
fprintf(stderr, "%s: ERROR: WDC: 0x%x Log Page not supported\n",
1084810839
__func__, log_id);
1084910840
ret = -1;
1085010841
goto out;

‎plugins/wdc/wdc-utils.c‎

Lines changed: 0 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,6 @@
2929
#include "nvme-print.h"
3030
#include "wdc-utils.h"
3131

32-
/* UUID field with value of 0 indicates end of UUID List*/
33-
const uint8_t UUID_END[] = {
34-
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
35-
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
36-
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
37-
};
38-
3932
int wdc_UtilsSnprintf(char *buffer, unsigned int sizeOfBuffer, const char *format, ...)
4033
{
4134
int res = 0;
@@ -196,45 +189,3 @@ bool wdc_CheckUuidListSupport(struct nvme_dev *dev, struct nvme_id_uuid_list *uu
196189

197190
return false;
198191
}
199-
200-
bool wdc_UuidEqual(struct nvme_id_uuid_list_entry *entry1, struct nvme_id_uuid_list_entry *entry2)
201-
{
202-
return !memcmp(entry1->uuid, entry2->uuid, NVME_UUID_LEN);
203-
}
204-
205-
bool wdc_FindUuidIndex(struct nvme_dev *dev,
206-
struct nvme_id_uuid_list_entry *uuid_entry,
207-
int *uuid_index)
208-
{
209-
bool uuid_found = false;
210-
int index = 0;
211-
struct nvme_id_uuid_list uuid_list;
212-
213-
*uuid_index = 0;
214-
215-
memset(&uuid_list, 0, sizeof(struct nvme_id_uuid_list));
216-
if (wdc_CheckUuidListSupport(dev, &uuid_list)) {
217-
struct nvme_id_uuid_list_entry *uuid_list_entry_ptr =
218-
(struct nvme_id_uuid_list_entry *)&uuid_list.entry[0];
219-
220-
while (index <= NVME_ID_UUID_LIST_MAX &&
221-
!wdc_UuidEqual(uuid_list_entry_ptr,
222-
(struct nvme_id_uuid_list_entry *)UUID_END)) {
223-
224-
if (wdc_UuidEqual(uuid_list_entry_ptr, uuid_entry)) {
225-
uuid_found = true;
226-
break;
227-
}
228-
229-
index++;
230-
uuid_list_entry_ptr =
231-
(struct nvme_id_uuid_list_entry *)&uuid_list.entry[index];
232-
}
233-
234-
if (uuid_found)
235-
*uuid_index = index + 1;
236-
}
237-
238-
return uuid_found;
239-
}
240-

‎plugins/wdc/wdc-utils.h‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -77,10 +77,4 @@ int wdc_UtilsStrCompare(const char *pcSrc, const char *pcDst);
7777
int wdc_UtilsCreateDir(const char *path);
7878
int wdc_WriteToFile(const char *fileName, const char *buffer, unsigned int bufferLen);
7979
void wdc_StrFormat(char *formatter, size_t fmt_sz, char *tofmt, size_t tofmtsz);
80-
bool wdc_CheckUuidListSupport(struct nvme_dev *dev,
81-
struct nvme_id_uuid_list *uuid_list);
82-
bool wdc_UuidEqual(struct nvme_id_uuid_list_entry *entry1,
83-
struct nvme_id_uuid_list_entry *entry2);
84-
bool wdc_FindUuidIndex(struct nvme_dev *dev,
85-
struct nvme_id_uuid_list_entry *uuid_entry,
86-
int *uuid_index);
80+
bool wdc_CheckUuidListSupport(struct nvme_dev *dev, struct nvme_id_uuid_list *uuid_list);

0 commit comments

Comments
 (0)