)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"6ea09112ca32c9d69e1bf2deb93fae22316dd6e9","unresolved":true,"context_lines":[{"line_number":7,"context_line":"NVMeOF connector support multipath-enabled kernels"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Use the \u0027/sys/class/block/\u003cctrl\u003en*\u0027 form for finding nvme devices."},{"line_number":10,"context_line":"This way is more generic, and is mainly beneficial because the device name format does not change as it does in \u0027/sys/class/nvme-fabrics/ctl/\u0027 for multipath-enabled vs disabled kernels."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Closes-Bug: #1943615"},{"line_number":13,"context_line":"Change-Id: Ib8e5d79e7dbae0867c794f35ff3350419d2cdf09"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"0957e7c7_c3c6d8e2","line":10,"updated":"2021-12-02 12:31:25.000000000","message":"nit: Mention how the old and new devices look like.","commit_id":"96744203684a7eaa5ea87ea713096644c4233879"},{"author":{"_account_id":16721,"name":"Zohar Mamedov","email":"zohar.cloud@gmail.com","username":"zohar"},"change_message_id":"71856e13d5565ff54114564e5b0cb02c516b1643","unresolved":false,"context_lines":[{"line_number":7,"context_line":"NVMeOF connector support multipath-enabled kernels"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Use the \u0027/sys/class/block/\u003cctrl\u003en*\u0027 form for finding nvme devices."},{"line_number":10,"context_line":"This way is more generic, and is mainly beneficial because the device name format does not change as it does in \u0027/sys/class/nvme-fabrics/ctl/\u0027 for multipath-enabled vs disabled kernels."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Closes-Bug: #1943615"},{"line_number":13,"context_line":"Change-Id: Ib8e5d79e7dbae0867c794f35ff3350419d2cdf09"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"675c94db_cf6ee6e6","line":10,"in_reply_to":"0957e7c7_c3c6d8e2","updated":"2021-12-08 10:04:00.000000000","message":"Done","commit_id":"96744203684a7eaa5ea87ea713096644c4233879"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"6ea09112ca32c9d69e1bf2deb93fae22316dd6e9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"8b50e899_95af69fe","updated":"2021-12-02 12:31:25.000000000","message":"-1: Missing a release note with the bug it\u0027s fixing.","commit_id":"96744203684a7eaa5ea87ea713096644c4233879"},{"author":{"_account_id":16721,"name":"Zohar Mamedov","email":"zohar.cloud@gmail.com","username":"zohar"},"change_message_id":"91840551938237383477a40f52c3798a465b824a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"33b9836e_a47dc902","updated":"2021-11-12 09:02:58.000000000","message":"Requesting reviewers attention please! \u003c3","commit_id":"96744203684a7eaa5ea87ea713096644c4233879"},{"author":{"_account_id":16721,"name":"Zohar Mamedov","email":"zohar.cloud@gmail.com","username":"zohar"},"change_message_id":"9d135c0ea0c6bf82deea540a122c0f11c1f3ad62","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"362df2f5_fcaccf85","updated":"2021-10-13 11:51:36.000000000","message":"run-KIOXIA CI","commit_id":"96744203684a7eaa5ea87ea713096644c4233879"},{"author":{"_account_id":16721,"name":"Zohar Mamedov","email":"zohar.cloud@gmail.com","username":"zohar"},"change_message_id":"71856e13d5565ff54114564e5b0cb02c516b1643","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"83bb2236_8944cb41","in_reply_to":"8b50e899_95af69fe","updated":"2021-12-08 10:04:00.000000000","message":"Done","commit_id":"96744203684a7eaa5ea87ea713096644c4233879"},{"author":{"_account_id":16721,"name":"Zohar Mamedov","email":"zohar.cloud@gmail.com","username":"zohar"},"change_message_id":"71856e13d5565ff54114564e5b0cb02c516b1643","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"8ed5725c_e923cbde","updated":"2021-12-08 10:04:00.000000000","message":"Thank you Gorka for the insightful and very constructive review!\n\nI addressed everything except your \u0027fgrep\u0027 nit suggestion, please take a look at the latest implementation, and on the \u0027fgrep\u0027 comment response inline.","commit_id":"7eabc8f1e0f89d7dddd4ced2821c22eea90aae0a"},{"author":{"_account_id":30555,"name":"Fernando Ferraz","display_name":"Fernando Ferraz","email":"fesilva@redhat.com","username":"fernandoperches"},"change_message_id":"4a1d9fee99ddfcc6da1b41c762b2c922662e7571","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"22986b6b_f9868984","updated":"2022-01-19 14:10:23.000000000","message":"Change seems reasonable, code looks good to me.","commit_id":"3939b2173850e56007257457775718ac9db1effb"},{"author":{"_account_id":30615,"name":"Tushar Trambak Gite","email":"tushargite96@gmail.com","username":"tushargite96"},"change_message_id":"cd10617d17ed37e1da8770b5381a3b1bf2e7a37c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"f68e4f59_254dd419","updated":"2022-01-10 07:54:40.000000000","message":"LGTM","commit_id":"3939b2173850e56007257457775718ac9db1effb"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"4e1b7e8407e4f96bdc4d166715427679cb58365d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"52fca50f_6a2df262","updated":"2022-02-01 23:15:39.000000000","message":"Question and nit noted inline.","commit_id":"3939b2173850e56007257457775718ac9db1effb"},{"author":{"_account_id":16721,"name":"Zohar Mamedov","email":"zohar.cloud@gmail.com","username":"zohar"},"change_message_id":"4710e9a1ed1e76943e6aecea673a26a9bd46c43d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"c1c9bc5e_748a2b33","updated":"2021-12-14 06:16:01.000000000","message":"run-KIOXIA CI","commit_id":"3939b2173850e56007257457775718ac9db1effb"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"9eb7499ab42e078b457ae6bf2f241a1cd63e5c83","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"3acb4976_a8a84734","updated":"2022-02-09 03:57:32.000000000","message":"Dropping to +1.  Just realized that the Kioxia CI hasn\u0027t run on PS6.","commit_id":"5614196e53b2248fc32d7fa1ff09a1cfee10d5b3"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@purestorage.com","username":"sdodsley"},"change_message_id":"32c36babd366d93bbfbb12159dbca78bf2e1f24f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"815e79e7_14673247","updated":"2022-02-09 14:02:34.000000000","message":"I\u0027m good with this now.","commit_id":"5614196e53b2248fc32d7fa1ff09a1cfee10d5b3"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"3528b3f40e23a0bffe9365a707296943a377bbd4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"89884f7b_92bfead7","updated":"2022-02-09 21:26:59.000000000","message":"Kioxia CI has passed; Mellanox SPDK has not, but it looks like the failure happens before any tests are run.  Because of the release deadline, we need to move this along.","commit_id":"5614196e53b2248fc32d7fa1ff09a1cfee10d5b3"},{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"3ea48298ce114065d9ecdfffe0981ca35066d1d0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"9c36b22c_febcc90c","updated":"2022-02-14 15:35:32.000000000","message":"Looks like this has had good review and is now passing tests.","commit_id":"5614196e53b2248fc32d7fa1ff09a1cfee10d5b3"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"7b07c223b5914bed352a723bfda6470ad4d29641","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"653b1822_d37a9bc9","updated":"2022-02-09 03:49:48.000000000","message":"Thanks for the explanation, Lior.  Answer to your question inline.  Code LGTM.","commit_id":"5614196e53b2248fc32d7fa1ff09a1cfee10d5b3"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"b72342403541a9c5a57f80fa7728d6555d5c5b9a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"1541b073_524ffd5d","updated":"2022-02-09 03:58:52.000000000","message":"run-KIOXIA CI","commit_id":"5614196e53b2248fc32d7fa1ff09a1cfee10d5b3"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"691143609170a12e22813c6f0a6269f031432632","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"d32b2220_87c92ebb","updated":"2022-02-09 03:59:14.000000000","message":"run-Mellanox CI","commit_id":"5614196e53b2248fc32d7fa1ff09a1cfee10d5b3"},{"author":{"_account_id":34173,"name":"Lior Friedman","display_name":"Lior Friedman","email":"lior.friedman@kioxia.com","username":"liorf95"},"change_message_id":"f7e6400ad057c316ac6cb3534d6765037d75d87a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"41e9e37d_519c77ad","in_reply_to":"3acb4976_a8a84734","updated":"2022-02-09 13:37:07.000000000","message":"Thanks Brian,\nKioxia CI has just run on PS6.","commit_id":"5614196e53b2248fc32d7fa1ff09a1cfee10d5b3"}],"os_brick/initiator/connectors/nvmeof.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"6ea09112ca32c9d69e1bf2deb93fae22316dd6e9","unresolved":true,"context_lines":[{"line_number":595,"context_line":"    @utils.retry(exception.VolumeDeviceNotFound, retries\u003d5)"},{"line_number":596,"context_line":"    def get_nvme_device_path(executor, target_nqn, vol_uuid):"},{"line_number":597,"context_line":"        nvme_ctrl \u003d NVMeOFConnector._get_nvme_controller(executor, target_nqn)"},{"line_number":598,"context_line":"        blocks \u003d glob.glob(\u0027/sys/class/block/\u0027 + nvme_ctrl + \u0027n*\u0027)"},{"line_number":599,"context_line":"        for block in blocks:"},{"line_number":600,"context_line":"            uuid_path \u003d block + \u0027/uuid\u0027"},{"line_number":601,"context_line":"            try:"},{"line_number":602,"context_line":"                exists \u003d os.path.exists(uuid_path)"},{"line_number":603,"context_line":"                if exists:"},{"line_number":604,"context_line":"                    uuid_lines, _err \u003d priv_rootwrap.custom_execute("},{"line_number":605,"context_line":"                        \u0027cat\u0027, uuid_path)"},{"line_number":606,"context_line":"                    if uuid_lines.split(\u0027\\n\u0027)[0] \u003d\u003d vol_uuid:"}],"source_content_type":"text/x-python","patch_set":3,"id":"58fd5b74_6d208d2e","line":603,"range":{"start_line":598,"start_character":0,"end_line":603,"end_character":26},"updated":"2021-12-02 12:31:25.000000000","message":"nit: We can avoid calling `exists` with a small modification to the glob call\n\n    uuid_paths \u003d glob.glob(f\u0027/sys/class/block/{vme_ctrl}n*/uuid\u0027)\n    for uuid_path in uuid_paths:\n        try:\n\nWith this change we would also need the rfind on line 607","commit_id":"96744203684a7eaa5ea87ea713096644c4233879"},{"author":{"_account_id":16721,"name":"Zohar Mamedov","email":"zohar.cloud@gmail.com","username":"zohar"},"change_message_id":"71856e13d5565ff54114564e5b0cb02c516b1643","unresolved":true,"context_lines":[{"line_number":595,"context_line":"    @utils.retry(exception.VolumeDeviceNotFound, retries\u003d5)"},{"line_number":596,"context_line":"    def get_nvme_device_path(executor, target_nqn, vol_uuid):"},{"line_number":597,"context_line":"        nvme_ctrl \u003d NVMeOFConnector._get_nvme_controller(executor, target_nqn)"},{"line_number":598,"context_line":"        blocks \u003d glob.glob(\u0027/sys/class/block/\u0027 + nvme_ctrl + \u0027n*\u0027)"},{"line_number":599,"context_line":"        for block in blocks:"},{"line_number":600,"context_line":"            uuid_path \u003d block + \u0027/uuid\u0027"},{"line_number":601,"context_line":"            try:"},{"line_number":602,"context_line":"                exists \u003d os.path.exists(uuid_path)"},{"line_number":603,"context_line":"                if exists:"},{"line_number":604,"context_line":"                    uuid_lines, _err \u003d priv_rootwrap.custom_execute("},{"line_number":605,"context_line":"                        \u0027cat\u0027, uuid_path)"},{"line_number":606,"context_line":"                    if uuid_lines.split(\u0027\\n\u0027)[0] \u003d\u003d vol_uuid:"}],"source_content_type":"text/x-python","patch_set":3,"id":"d30ecf65_06e4ce10","line":603,"range":{"start_line":598,"start_character":0,"end_line":603,"end_character":26},"in_reply_to":"58fd5b74_6d208d2e","updated":"2021-12-08 10:04:00.000000000","message":"This is a great optimization, implemented with adjusting the rfind offset due to the added \u0027/uuid\u0027 string. Please take a look to see if this is satisfactory.","commit_id":"96744203684a7eaa5ea87ea713096644c4233879"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"6ea09112ca32c9d69e1bf2deb93fae22316dd6e9","unresolved":true,"context_lines":[{"line_number":601,"context_line":"            try:"},{"line_number":602,"context_line":"                exists \u003d os.path.exists(uuid_path)"},{"line_number":603,"context_line":"                if exists:"},{"line_number":604,"context_line":"                    uuid_lines, _err \u003d priv_rootwrap.custom_execute("},{"line_number":605,"context_line":"                        \u0027cat\u0027, uuid_path)"},{"line_number":606,"context_line":"                    if uuid_lines.split(\u0027\\n\u0027)[0] \u003d\u003d vol_uuid:"},{"line_number":607,"context_line":"                        return \u0027/dev/\u0027 + block[block.rfind(\u0027/\u0027) + 1:]"}],"source_content_type":"text/x-python","patch_set":3,"id":"b90764cd_57a3872b","line":604,"range":{"start_line":604,"start_character":39,"end_line":604,"end_character":67},"updated":"2021-12-02 12:31:25.000000000","message":"-1: I believe we should be using the \"executor\" instead of the custom_execute directly.","commit_id":"96744203684a7eaa5ea87ea713096644c4233879"},{"author":{"_account_id":16721,"name":"Zohar Mamedov","email":"zohar.cloud@gmail.com","username":"zohar"},"change_message_id":"71856e13d5565ff54114564e5b0cb02c516b1643","unresolved":true,"context_lines":[{"line_number":601,"context_line":"            try:"},{"line_number":602,"context_line":"                exists \u003d os.path.exists(uuid_path)"},{"line_number":603,"context_line":"                if exists:"},{"line_number":604,"context_line":"                    uuid_lines, _err \u003d priv_rootwrap.custom_execute("},{"line_number":605,"context_line":"                        \u0027cat\u0027, uuid_path)"},{"line_number":606,"context_line":"                    if uuid_lines.split(\u0027\\n\u0027)[0] \u003d\u003d vol_uuid:"},{"line_number":607,"context_line":"                        return \u0027/dev/\u0027 + block[block.rfind(\u0027/\u0027) + 1:]"}],"source_content_type":"text/x-python","patch_set":3,"id":"22488eaa_e0a672f0","line":604,"range":{"start_line":604,"start_character":39,"end_line":604,"end_character":67},"in_reply_to":"b90764cd_57a3872b","updated":"2021-12-08 10:04:00.000000000","message":"Done\n\nUsing custom_execute made it here because this patch was cherry picked from the \"xena nvmeof agent\" effort. I do agree that it is better to keep using \"executor\" to be consistent with the rest of the connector code.\n\nHowever, just for clarity, what is the difference? And why is using \"executor\" preferable?","commit_id":"96744203684a7eaa5ea87ea713096644c4233879"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"6ea09112ca32c9d69e1bf2deb93fae22316dd6e9","unresolved":true,"context_lines":[{"line_number":602,"context_line":"                exists \u003d os.path.exists(uuid_path)"},{"line_number":603,"context_line":"                if exists:"},{"line_number":604,"context_line":"                    uuid_lines, _err \u003d priv_rootwrap.custom_execute("},{"line_number":605,"context_line":"                        \u0027cat\u0027, uuid_path)"},{"line_number":606,"context_line":"                    if uuid_lines.split(\u0027\\n\u0027)[0] \u003d\u003d vol_uuid:"},{"line_number":607,"context_line":"                        return \u0027/dev/\u0027 + block[block.rfind(\u0027/\u0027) + 1:]"},{"line_number":608,"context_line":"            except putils.ProcessExecutionError as e:"}],"source_content_type":"text/x-python","patch_set":3,"id":"9706bcfc_bf44eb2b","line":605,"range":{"start_line":605,"start_character":0,"end_line":605,"end_character":41},"updated":"2021-12-02 12:31:25.000000000","message":"nit: Instead of doing the glob and then the cat for each one until we find it, can\u0027t we use grep instead?\n\n  f\u0027grep -l {vol_id} /sys/class/nvme-fabrics/ctl/nvme*\u0027","commit_id":"96744203684a7eaa5ea87ea713096644c4233879"},{"author":{"_account_id":30555,"name":"Fernando Ferraz","display_name":"Fernando Ferraz","email":"fesilva@redhat.com","username":"fernandoperches"},"change_message_id":"4a1d9fee99ddfcc6da1b41c762b2c922662e7571","unresolved":true,"context_lines":[{"line_number":602,"context_line":"                exists \u003d os.path.exists(uuid_path)"},{"line_number":603,"context_line":"                if exists:"},{"line_number":604,"context_line":"                    uuid_lines, _err \u003d priv_rootwrap.custom_execute("},{"line_number":605,"context_line":"                        \u0027cat\u0027, uuid_path)"},{"line_number":606,"context_line":"                    if uuid_lines.split(\u0027\\n\u0027)[0] \u003d\u003d vol_uuid:"},{"line_number":607,"context_line":"                        return \u0027/dev/\u0027 + block[block.rfind(\u0027/\u0027) + 1:]"},{"line_number":608,"context_line":"            except putils.ProcessExecutionError as e:"}],"source_content_type":"text/x-python","patch_set":3,"id":"3d793dbf_8e5f38dd","line":605,"range":{"start_line":605,"start_character":0,"end_line":605,"end_character":41},"in_reply_to":"22918027_6c91b3a5","updated":"2022-01-19 14:10:23.000000000","message":"I think Zohar\u0027s approach would be more optimized since there is no need to look for any files except from uuid. You could use head -1 instead and take just the line you need but that is not really necessary. :P","commit_id":"96744203684a7eaa5ea87ea713096644c4233879"},{"author":{"_account_id":16721,"name":"Zohar Mamedov","email":"zohar.cloud@gmail.com","username":"zohar"},"change_message_id":"71856e13d5565ff54114564e5b0cb02c516b1643","unresolved":true,"context_lines":[{"line_number":602,"context_line":"                exists \u003d os.path.exists(uuid_path)"},{"line_number":603,"context_line":"                if exists:"},{"line_number":604,"context_line":"                    uuid_lines, _err \u003d priv_rootwrap.custom_execute("},{"line_number":605,"context_line":"                        \u0027cat\u0027, uuid_path)"},{"line_number":606,"context_line":"                    if uuid_lines.split(\u0027\\n\u0027)[0] \u003d\u003d vol_uuid:"},{"line_number":607,"context_line":"                        return \u0027/dev/\u0027 + block[block.rfind(\u0027/\u0027) + 1:]"},{"line_number":608,"context_line":"            except putils.ProcessExecutionError as e:"}],"source_content_type":"text/x-python","patch_set":3,"id":"22918027_6c91b3a5","line":605,"range":{"start_line":605,"start_character":0,"end_line":605,"end_character":41},"in_reply_to":"9706bcfc_bf44eb2b","updated":"2021-12-08 10:04:00.000000000","message":"Yes, the only difference is that it would fgrep through the entire file instead of simply matching the first line to the uuid.\n\nIf this is the preferred way, would be nice to see how to properly implement this.","commit_id":"96744203684a7eaa5ea87ea713096644c4233879"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"4e1b7e8407e4f96bdc4d166715427679cb58365d","unresolved":true,"context_lines":[{"line_number":595,"context_line":"    @utils.retry(exception.VolumeDeviceNotFound, retries\u003d5)"},{"line_number":596,"context_line":"    def get_nvme_device_path(executor, target_nqn, vol_uuid):"},{"line_number":597,"context_line":"        nvme_ctrl \u003d NVMeOFConnector._get_nvme_controller(executor, target_nqn)"},{"line_number":598,"context_line":"        uuid_paths \u003d glob.glob(\u0027/sys/class/block/\u0027 + nvme_ctrl + \u0027n*/uuid\u0027)"},{"line_number":599,"context_line":"        for uuid_path in uuid_paths:"},{"line_number":600,"context_line":"            try:"},{"line_number":601,"context_line":"                uuid_lines, _err \u003d executor._execute("}],"source_content_type":"text/x-python","patch_set":5,"id":"9018e913_1de41de2","line":598,"range":{"start_line":598,"start_character":69,"end_line":598,"end_character":73},"updated":"2022-02-01 23:15:39.000000000","message":"Dumb question, but I don\u0027t see uuid in my /sys/class/block/nvme0n1 directory.  Is that because my transport type is pcie, or does it have to do with my kernel version?","commit_id":"3939b2173850e56007257457775718ac9db1effb"},{"author":{"_account_id":34173,"name":"Lior Friedman","display_name":"Lior Friedman","email":"lior.friedman@kioxia.com","username":"liorf95"},"change_message_id":"a4af4cf49cba9ceac55bb351a4937589e078054a","unresolved":true,"context_lines":[{"line_number":595,"context_line":"    @utils.retry(exception.VolumeDeviceNotFound, retries\u003d5)"},{"line_number":596,"context_line":"    def get_nvme_device_path(executor, target_nqn, vol_uuid):"},{"line_number":597,"context_line":"        nvme_ctrl \u003d NVMeOFConnector._get_nvme_controller(executor, target_nqn)"},{"line_number":598,"context_line":"        uuid_paths \u003d glob.glob(\u0027/sys/class/block/\u0027 + nvme_ctrl + \u0027n*/uuid\u0027)"},{"line_number":599,"context_line":"        for uuid_path in uuid_paths:"},{"line_number":600,"context_line":"            try:"},{"line_number":601,"context_line":"                uuid_lines, _err \u003d executor._execute("}],"source_content_type":"text/x-python","patch_set":5,"id":"d30ec049_d44e04be","line":598,"range":{"start_line":598,"start_character":69,"end_line":598,"end_character":73},"in_reply_to":"9018e913_1de41de2","updated":"2022-02-02 11:11:12.000000000","message":"Hi Brian,\nIt relates to the target driver itself which suppose to expose the namespace uuid on the initiator nvme controller block device path (/sys/class/block/nvme0n1/uuid) while sending this uuid as a connection property to the cinder driver to look for - otherwise the connector will not get to that code part at all.    \n\nOne dumb question from my side - what does it mean \u0027nit\u0027?","commit_id":"3939b2173850e56007257457775718ac9db1effb"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"7b07c223b5914bed352a723bfda6470ad4d29641","unresolved":true,"context_lines":[{"line_number":595,"context_line":"    @utils.retry(exception.VolumeDeviceNotFound, retries\u003d5)"},{"line_number":596,"context_line":"    def get_nvme_device_path(executor, target_nqn, vol_uuid):"},{"line_number":597,"context_line":"        nvme_ctrl \u003d NVMeOFConnector._get_nvme_controller(executor, target_nqn)"},{"line_number":598,"context_line":"        uuid_paths \u003d glob.glob(\u0027/sys/class/block/\u0027 + nvme_ctrl + \u0027n*/uuid\u0027)"},{"line_number":599,"context_line":"        for uuid_path in uuid_paths:"},{"line_number":600,"context_line":"            try:"},{"line_number":601,"context_line":"                uuid_lines, _err \u003d executor._execute("}],"source_content_type":"text/x-python","patch_set":5,"id":"8749d546_1cfde1d2","line":598,"range":{"start_line":598,"start_character":69,"end_line":598,"end_character":73},"in_reply_to":"d30ec049_d44e04be","updated":"2022-02-09 03:49:48.000000000","message":"A \u0027nit\u0027 is a comment about a very minor issue; it\u0027s usually something for which a reviewer isn\u0027t requiring a change, it\u0027s just something they noticed and wanted to mention.","commit_id":"3939b2173850e56007257457775718ac9db1effb"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"4e1b7e8407e4f96bdc4d166715427679cb58365d","unresolved":true,"context_lines":[{"line_number":603,"context_line":"                    root_helper\u003dexecutor._root_helper)"},{"line_number":604,"context_line":"                if uuid_lines.split(\u0027\\n\u0027)[0] \u003d\u003d vol_uuid:"},{"line_number":605,"context_line":"                    return \u0027/dev/\u0027 + uuid_path["},{"line_number":606,"context_line":"                        uuid_path.rfind(\u0027/\u0027, 0, -5) + 1: -5]"},{"line_number":607,"context_line":"            except putils.ProcessExecutionError as e:"},{"line_number":608,"context_line":"                LOG.exception(e)"},{"line_number":609,"context_line":"        raise exception.VolumeDeviceNotFound(device\u003dvol_uuid)"}],"source_content_type":"text/x-python","patch_set":5,"id":"4fe2b657_e0f2e758","line":606,"range":{"start_line":606,"start_character":49,"end_line":606,"end_character":50},"updated":"2022-02-01 23:15:39.000000000","message":"nit: you can take it or leave it ...\n\n                if uuid_lines.split(\u0027\\n\u0027)[0] \u003d\u003d vol_uuid:\n                    ignore \u003d len(\u0027/uuid\u0027)\n                    return \u0027/dev/\u0027 + uuid_path[\n                        uuid_path.rfind(\u0027/\u0027, 0, -ignore) + 1: -ignore]\n\nIt eliminates the \"magic\" number, but on the other hand, you still need to look up to line 598 to see where \u0027/uuid\u0027 is coming from, in which case where the -5 is coming from is pretty evident.  So it sort of makes the code self-documenting, though maybe not enough to be worth it.","commit_id":"3939b2173850e56007257457775718ac9db1effb"},{"author":{"_account_id":34173,"name":"Lior Friedman","display_name":"Lior Friedman","email":"lior.friedman@kioxia.com","username":"liorf95"},"change_message_id":"a4af4cf49cba9ceac55bb351a4937589e078054a","unresolved":false,"context_lines":[{"line_number":603,"context_line":"                    root_helper\u003dexecutor._root_helper)"},{"line_number":604,"context_line":"                if uuid_lines.split(\u0027\\n\u0027)[0] \u003d\u003d vol_uuid:"},{"line_number":605,"context_line":"                    return \u0027/dev/\u0027 + uuid_path["},{"line_number":606,"context_line":"                        uuid_path.rfind(\u0027/\u0027, 0, -5) + 1: -5]"},{"line_number":607,"context_line":"            except putils.ProcessExecutionError as e:"},{"line_number":608,"context_line":"                LOG.exception(e)"},{"line_number":609,"context_line":"        raise exception.VolumeDeviceNotFound(device\u003dvol_uuid)"}],"source_content_type":"text/x-python","patch_set":5,"id":"1603f5b9_b669a4b6","line":606,"range":{"start_line":606,"start_character":49,"end_line":606,"end_character":50},"in_reply_to":"4fe2b657_e0f2e758","updated":"2022-02-02 11:11:12.000000000","message":"Ack","commit_id":"3939b2173850e56007257457775718ac9db1effb"},{"author":{"_account_id":33612,"name":"yuval","email":"yuval@lightbitslabs.com","username":"yuval"},"change_message_id":"2483267738ff203628f082b86e16152d8ab8c256","unresolved":true,"context_lines":[{"line_number":596,"context_line":"    def get_nvme_device_path(executor, target_nqn, vol_uuid):"},{"line_number":597,"context_line":"        nvme_ctrl \u003d NVMeOFConnector._get_nvme_controller(executor, target_nqn)"},{"line_number":598,"context_line":"        uuid_paths \u003d glob.glob(\u0027/sys/class/block/\u0027 + nvme_ctrl + \u0027n*/uuid\u0027)"},{"line_number":599,"context_line":"        for uuid_path in uuid_paths:"},{"line_number":600,"context_line":"            try:"},{"line_number":601,"context_line":"                uuid_lines, _err \u003d executor._execute("},{"line_number":602,"context_line":"                    \u0027cat\u0027, uuid_path, run_as_root\u003dTrue,"}],"source_content_type":"text/x-python","patch_set":6,"id":"64b302b1_d8baeb54","line":599,"updated":"2022-02-09 16:13:33.000000000","message":"I think this function is already implemented in the lightos connector, consider moving it to a common place.\nhttps://opendev.org/openstack/os-brick/src/branch/master/os_brick/initiator/connectors/lightos.py#L203","commit_id":"5614196e53b2248fc32d7fa1ff09a1cfee10d5b3"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"55df327d1f1ff0bed6b0c452a9ae50f161f2e421","unresolved":true,"context_lines":[{"line_number":596,"context_line":"    def get_nvme_device_path(executor, target_nqn, vol_uuid):"},{"line_number":597,"context_line":"        nvme_ctrl \u003d NVMeOFConnector._get_nvme_controller(executor, target_nqn)"},{"line_number":598,"context_line":"        uuid_paths \u003d glob.glob(\u0027/sys/class/block/\u0027 + nvme_ctrl + \u0027n*/uuid\u0027)"},{"line_number":599,"context_line":"        for uuid_path in uuid_paths:"},{"line_number":600,"context_line":"            try:"},{"line_number":601,"context_line":"                uuid_lines, _err \u003d executor._execute("},{"line_number":602,"context_line":"                    \u0027cat\u0027, uuid_path, run_as_root\u003dTrue,"}],"source_content_type":"text/x-python","patch_set":6,"id":"dfa0b693_a1d5c225","line":599,"in_reply_to":"64b302b1_d8baeb54","updated":"2022-02-09 21:30:30.000000000","message":"It would be a good idea for the kioxia and lightos teams to look over each other\u0027s drivers and see what can be abstracted to a common location.","commit_id":"5614196e53b2248fc32d7fa1ff09a1cfee10d5b3"},{"author":{"_account_id":34550,"name":"Gili Buzaglo","email":"gili.buzaglo@kioxia.com","username":"gilib"},"change_message_id":"18b31d2dd087e5edc71840f0b235356aec1d9f64","unresolved":true,"context_lines":[{"line_number":596,"context_line":"    def get_nvme_device_path(executor, target_nqn, vol_uuid):"},{"line_number":597,"context_line":"        nvme_ctrl \u003d NVMeOFConnector._get_nvme_controller(executor, target_nqn)"},{"line_number":598,"context_line":"        uuid_paths \u003d glob.glob(\u0027/sys/class/block/\u0027 + nvme_ctrl + \u0027n*/uuid\u0027)"},{"line_number":599,"context_line":"        for uuid_path in uuid_paths:"},{"line_number":600,"context_line":"            try:"},{"line_number":601,"context_line":"                uuid_lines, _err \u003d executor._execute("},{"line_number":602,"context_line":"                    \u0027cat\u0027, uuid_path, run_as_root\u003dTrue,"}],"source_content_type":"text/x-python","patch_set":6,"id":"e8ed2ae5_48c7eb2c","line":599,"in_reply_to":"dfa0b693_a1d5c225","updated":"2022-02-10 09:48:44.000000000","message":"get_nvme_device_path first looks for the controle device and then scans only the block devices which are related to it.\nlightos method scans all block devices it\u0027ll take longer time in certain scenarios so we would like to avoid it. Anyway due to shortage of time I don\u0027t believe we can share the code now.","commit_id":"5614196e53b2248fc32d7fa1ff09a1cfee10d5b3"},{"author":{"_account_id":33612,"name":"yuval","email":"yuval@lightbitslabs.com","username":"yuval"},"change_message_id":"9301cca1a01427413a6308d0be2f53d867326445","unresolved":true,"context_lines":[{"line_number":596,"context_line":"    def get_nvme_device_path(executor, target_nqn, vol_uuid):"},{"line_number":597,"context_line":"        nvme_ctrl \u003d NVMeOFConnector._get_nvme_controller(executor, target_nqn)"},{"line_number":598,"context_line":"        uuid_paths \u003d glob.glob(\u0027/sys/class/block/\u0027 + nvme_ctrl + \u0027n*/uuid\u0027)"},{"line_number":599,"context_line":"        for uuid_path in uuid_paths:"},{"line_number":600,"context_line":"            try:"},{"line_number":601,"context_line":"                uuid_lines, _err \u003d executor._execute("},{"line_number":602,"context_line":"                    \u0027cat\u0027, uuid_path, run_as_root\u003dTrue,"}],"source_content_type":"text/x-python","patch_set":6,"id":"df727907_9ef3cc05","line":599,"in_reply_to":"e8ed2ae5_48c7eb2c","updated":"2022-02-10 13:09:19.000000000","message":"I am fine with that, since there is no time, I didnt -1","commit_id":"5614196e53b2248fc32d7fa1ff09a1cfee10d5b3"},{"author":{"_account_id":33612,"name":"yuval","email":"yuval@lightbitslabs.com","username":"yuval"},"change_message_id":"2483267738ff203628f082b86e16152d8ab8c256","unresolved":true,"context_lines":[{"line_number":599,"context_line":"        for uuid_path in uuid_paths:"},{"line_number":600,"context_line":"            try:"},{"line_number":601,"context_line":"                uuid_lines, _err \u003d executor._execute("},{"line_number":602,"context_line":"                    \u0027cat\u0027, uuid_path, run_as_root\u003dTrue,"},{"line_number":603,"context_line":"                    root_helper\u003dexecutor._root_helper)"},{"line_number":604,"context_line":"                if uuid_lines.split(\u0027\\n\u0027)[0] \u003d\u003d vol_uuid:"},{"line_number":605,"context_line":"                    ignore \u003d len(\u0027/uuid\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"07cd312e_81d7fc8b","line":602,"updated":"2022-02-09 16:13:33.000000000","message":"I believe you can use python \"open\" function here - instead of using the \"cat\"","commit_id":"5614196e53b2248fc32d7fa1ff09a1cfee10d5b3"},{"author":{"_account_id":34550,"name":"Gili Buzaglo","email":"gili.buzaglo@kioxia.com","username":"gilib"},"change_message_id":"18b31d2dd087e5edc71840f0b235356aec1d9f64","unresolved":true,"context_lines":[{"line_number":599,"context_line":"        for uuid_path in uuid_paths:"},{"line_number":600,"context_line":"            try:"},{"line_number":601,"context_line":"                uuid_lines, _err \u003d executor._execute("},{"line_number":602,"context_line":"                    \u0027cat\u0027, uuid_path, run_as_root\u003dTrue,"},{"line_number":603,"context_line":"                    root_helper\u003dexecutor._root_helper)"},{"line_number":604,"context_line":"                if uuid_lines.split(\u0027\\n\u0027)[0] \u003d\u003d vol_uuid:"},{"line_number":605,"context_line":"                    ignore \u003d len(\u0027/uuid\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"9b19aadf_14138879","line":602,"in_reply_to":"07cd312e_81d7fc8b","updated":"2022-02-10 09:48:44.000000000","message":"Good Idea , we\u0027ll go through usages of cat and consider this for next release","commit_id":"5614196e53b2248fc32d7fa1ff09a1cfee10d5b3"},{"author":{"_account_id":33612,"name":"yuval","email":"yuval@lightbitslabs.com","username":"yuval"},"change_message_id":"9301cca1a01427413a6308d0be2f53d867326445","unresolved":true,"context_lines":[{"line_number":599,"context_line":"        for uuid_path in uuid_paths:"},{"line_number":600,"context_line":"            try:"},{"line_number":601,"context_line":"                uuid_lines, _err \u003d executor._execute("},{"line_number":602,"context_line":"                    \u0027cat\u0027, uuid_path, run_as_root\u003dTrue,"},{"line_number":603,"context_line":"                    root_helper\u003dexecutor._root_helper)"},{"line_number":604,"context_line":"                if uuid_lines.split(\u0027\\n\u0027)[0] \u003d\u003d vol_uuid:"},{"line_number":605,"context_line":"                    ignore \u003d len(\u0027/uuid\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"9285d141_ee57db4c","line":602,"in_reply_to":"9b19aadf_14138879","updated":"2022-02-10 13:09:19.000000000","message":"I am fine with that, its just the same comments the lightos connector got.","commit_id":"5614196e53b2248fc32d7fa1ff09a1cfee10d5b3"}]}