From: Martin Belanger Date: Tue, 18 Apr 2023 13:19:32 +0000 (-0400) Subject: Python: make NBFT data more pythonic X-Git-Tag: v1.5~39 X-Git-Url: https://www.infradead.org/git/?a=commitdiff_plain;h=1baa0757567d8951e483ebedf2c7d81882ad829f;p=users%2Fsagi%2Flibnvme.git Python: make NBFT data more pythonic I made the nfollowing changes so that the data is more Pythonic. 1) For boolean values, set them to True/False instead of 1/0. 2) NBFT data contains ordered lists. In the raw NBFT data the position of each element in the list is indicated by a 1-based index. When converting to Python lists, make sure that each element is inserted in the list at the right position. This is done by converting the 1-based index to a 0-based index. 3) For objects that contain index variables that refer to items in a list, make sure to convert the 1-based index to 0-based so that it can be used directly to access the python lists (e.g. list[index]). 4) Since Python lists are ordered (per 2 above), there is no need to keep an explicit 1-based index in each of the list items. Therefore those 1-based indexes were removed. 5) No need to keep explicit variables representing the length of a list. In Python one need only use len(list) to get the length. Signed-off-by: Martin Belanger --- diff --git a/libnvme/nvme.i b/libnvme/nvme.i index 0f2a7cdb..d32fba5a 100644 --- a/libnvme/nvme.i +++ b/libnvme/nvme.i @@ -37,10 +37,6 @@ static int connect_err = 0; static int discover_err = 0; - static void PyList_AppendDecRef(PyObject *p, PyObject *val) { - PyList_Append(p, val); /* Does NOT steal reference to val .. */ - Py_XDECREF(val); /* .. therefore decrement ref. count. */ - } static void PyDict_SetItemStringDecRef(PyObject * p, const char *key, PyObject *val) { PyDict_SetItemString(p, key, val); /* Does NOT steal reference to val .. */ Py_XDECREF(val); /* .. therefore decrement ref. count. */ @@ -319,7 +315,7 @@ PyObject *hostid_from_file(); PyDict_SetItemStringDecRef(entry, "asqsz", val); val = PyLong_FromLong(e->eflags); PyDict_SetItemStringDecRef(entry, "eflags", val); - PyList_SetItem(obj, i, entry); /* steals ref. to entry */ + PyList_SetItem(obj, i, entry); /* steals ref. to object - no need to decref */ } $result = obj; }; @@ -738,7 +734,7 @@ struct nvme_ns { if (!obj) Py_RETURN_NONE; for (int i = 0; i < NVME_LOG_SUPPORTED_LOG_PAGES_MAX; i++) - PyList_SetItem(obj, i, PyLong_FromLong(le32_to_cpu(log.lid_support[i]))); /* steals ref. */ + PyList_SetItem(obj, i, PyLong_FromLong(le32_to_cpu(log.lid_support[i]))); /* steals ref. to object - no need to decref */ return obj; } @@ -827,13 +823,10 @@ struct nvme_ns { PyObject *output = PyDict_New(); PyObject *hfis = PyList_New(ss->num_hfis); - PyDict_SetItemStringDecRef(output, "index", PyLong_FromLong(ss->index)); - - PyDict_SetItemStringDecRef(output, "num_hfis", PyLong_FromLong(ss->num_hfis)); for (i = 0; i < ss->num_hfis; i++) - PyList_SetItem(hfis, i, PyLong_FromLong(ss->hfis[i]->index)); /* steals ref. to object - no need to decref */ + PyList_SetItem(hfis, i, PyLong_FromLong(ss->hfis[i]->index - 1)); /* steals ref. to object - no need to decref */ - PyDict_SetItemStringDecRef(output, "hfis", hfis); + PyDict_SetItemStringDecRef(output, "hfi_indexes", hfis); PyDict_SetItemStringDecRef(output, "trtype", PyUnicode_FromString(ss->transport)); PyDict_SetItemStringDecRef(output, "traddr", PyUnicode_FromString(ss->traddr)); @@ -886,8 +879,8 @@ struct nvme_ns { if (ss->dhcp_root_path_string) PyDict_SetItemStringDecRef(output, "dhcp_root_path_string", PyUnicode_FromString(ss->dhcp_root_path_string)); - PyDict_SetItemStringDecRef(output, "pdu_header_digest_required", PyLong_FromLong(ss->pdu_header_digest_required)); - PyDict_SetItemStringDecRef(output, "data_digest_required", PyLong_FromLong(ss->data_digest_required)); + PyDict_SetItemStringDecRef(output, "pdu_header_digest_required", PyBool_FromLong(ss->pdu_header_digest_required)); + PyDict_SetItemStringDecRef(output, "data_digest_required", PyBool_FromLong(ss->data_digest_required)); return output; } @@ -896,7 +889,6 @@ struct nvme_ns { { PyObject *output = PyDict_New(); - PyDict_SetItemStringDecRef(output, "index", PyLong_FromLong(hfi->index)); PyDict_SetItemStringDecRef(output, "trtype", PyUnicode_FromString(hfi->transport)); if (!strcmp(hfi->transport, "tcp")) { @@ -929,8 +921,8 @@ struct nvme_ns { if (hfi->tcp_info.host_name) PyDict_SetItemStringDecRef(output, "host_name", PyUnicode_FromString(hfi->tcp_info.host_name)); - PyDict_SetItemStringDecRef(output, "this_hfi_is_default_route", PyLong_FromLong(hfi->tcp_info.this_hfi_is_default_route)); - PyDict_SetItemStringDecRef(output, "dhcp_override", PyLong_FromLong(hfi->tcp_info.dhcp_override)); + PyDict_SetItemStringDecRef(output, "this_hfi_is_default_route", PyBool_FromLong(hfi->tcp_info.this_hfi_is_default_route)); + PyDict_SetItemStringDecRef(output, "dhcp_override", PyBool_FromLong(hfi->tcp_info.dhcp_override)); } return output; @@ -940,12 +932,10 @@ struct nvme_ns { { PyObject *output = PyDict_New(); - PyDict_SetItemStringDecRef(output, "index", PyLong_FromLong(disc->index)); - if (disc->security) - PyDict_SetItemStringDecRef(output, "security", PyLong_FromLong(disc->security->index)); + PyDict_SetItemStringDecRef(output, "security_index", PyLong_FromLong(disc->security->index)); if (disc->hfi) - PyDict_SetItemStringDecRef(output, "hfi", PyLong_FromLong(disc->hfi->index)); + PyDict_SetItemStringDecRef(output, "hfi_index", PyLong_FromLong(disc->hfi->index - 1)); if (disc->uri) PyDict_SetItemStringDecRef(output, "uri", PyUnicode_FromString(disc->uri)); if (disc->nqn) @@ -982,35 +972,61 @@ struct nvme_ns { } { + size_t ss_num = 0; struct nbft_info_subsystem_ns **ss; - PyObject *subsystem = PyList_New(0); + PyObject *subsystem; + + /* First, let's find how many entries there are */ + for (ss = nbft->subsystem_ns_list; ss && *ss; ss++) + ss_num++; + /* Now, let's fill the list using "(*ss)->index - 1" + as the index for writing to the list */ + subsystem = PyList_New(ss_num); for (ss = nbft->subsystem_ns_list; ss && *ss; ss++) - PyList_AppendDecRef(subsystem, ssns_to_dict(*ss)); + PyList_SetItem(subsystem, (*ss)->index - 1, ssns_to_dict(*ss)); /* steals ref. to object - no need to decref */ PyDict_SetItemStringDecRef(output, "subsystem", subsystem); } { + size_t hfi_num = 0; struct nbft_info_hfi **hfi; - PyObject *hfis = PyList_New(0); + PyObject *hfis; + /* First, let's find how many entries there are */ for (hfi = nbft->hfi_list; hfi && *hfi; hfi++) - PyList_AppendDecRef(hfis, hfi_to_dict(*hfi)); + hfi_num++; + + /* Now, let's fill the list using "(*hfi)->index - 1" + as the index for writing to the list */ + hfis = PyList_New(hfi_num); + for (hfi = nbft->hfi_list; hfi && *hfi; hfi++) + PyList_SetItem(hfis, (*hfi)->index-1, hfi_to_dict(*hfi)); /* steals ref. to object - no need to decref */ PyDict_SetItemStringDecRef(output, "hfi", hfis); } { + size_t disc_num = 0; struct nbft_info_discovery **disc; - PyObject *discovery = PyList_New(0); + PyObject *discovery; + /* First, let's find how many entries there are */ for (disc = nbft->discovery_list; disc && *disc; disc++) - PyList_AppendDecRef(discovery, discovery_to_dict(*disc)); + disc_num++; + + /* Now, let's fill the list using "(*disc)->index - 1" + as the index for writing to the list */ + discovery = PyList_New(disc_num); + for (disc = nbft->discovery_list; disc && *disc; disc++) + PyList_SetItem(discovery, (*disc)->index - 1, discovery_to_dict(*disc)); /* steals ref. to object - no need to decref */ PyDict_SetItemStringDecRef(output, "discovery", discovery); } + /* Security profiles are currently not implemented. */ + return output; } diff --git a/libnvme/tests/test-nbft.py b/libnvme/tests/test-nbft.py index bba7a123..3aeeba45 100755 --- a/libnvme/tests/test-nbft.py +++ b/libnvme/tests/test-nbft.py @@ -11,18 +11,16 @@ class Testclass(unittest.TestCase): self.expected_nbft = { "discovery": [ { - "hfi": 1, - "index": 1, + "hfi_index": 0, "nqn": "nqn.2014-08.org.nvmexpress.discovery", "uri": "nvme+tcp://100.71.103.50:8009/", } ], "hfi": [ { - "dhcp_override": 1, + "dhcp_override": True, "dhcp_server_ipaddr": "100.71.245.254", "gateway_ipaddr": "100.71.245.254", - "index": 1, "ip_origin": 82, "ipaddr": "100.71.245.232", "mac_addr": "b0:26:28:e8:7c:0e", @@ -47,14 +45,12 @@ class Testclass(unittest.TestCase): { "asqsz": 0, "controller_id": 5, - "data_digest_required": 0, - "hfis": [1], - "index": 1, + "data_digest_required": False, + "hfi_indexes": [0], "nid": "c82404ed9c15f53b8ccf0968002e0fca", "nid_type": "nguid", "nsid": 148, - "num_hfis": 1, - "pdu_header_digest_required": 0, + "pdu_header_digest_required": False, "subsys_nqn": "nqn.1988-11.com.dell:powerstore:00:2a64abf1c5b81F6C4549", "subsys_port_id": 0, "traddr": "100.71.103.48", @@ -64,14 +60,12 @@ class Testclass(unittest.TestCase): { "asqsz": 0, "controller_id": 4166, - "data_digest_required": 0, - "hfis": [1], - "index": 2, + "data_digest_required": False, + "hfi_indexes": [0], "nid": "c82404ed9c15f53b8ccf0968002e0fca", "nid_type": "nguid", "nsid": 148, - "num_hfis": 1, - "pdu_header_digest_required": 0, + "pdu_header_digest_required": False, "subsys_nqn": "nqn.1988-11.com.dell:powerstore:00:2a64abf1c5b81F6C4549", "subsys_port_id": 0, "traddr": "100.71.103.49", @@ -83,7 +77,8 @@ class Testclass(unittest.TestCase): def test_read_nbft_file(self): """Make sure we get expected data when reading from binary NBFT file""" - self.assertEqual(nvme.nbft_get(args.filename), self.expected_nbft) + actual_nbft = nvme.nbft_get(args.filename) + self.assertEqual(actual_nbft, self.expected_nbft) if __name__ == "__main__":