)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"938477c993e2c4a96bdfc943e887332b1239a104","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"3a3b78e0_aaff47f8","updated":"2022-10-03 23:55:00.000000000","message":"Looks okay but unfortunately I don\u0027t quite understand how this works, so +0 from me for now.","commit_id":"6ba52f877e4dcd05f1e791dc54fc6204dfc6984e"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"4ec902e3b5cf593cba6cc9b769fc4ecbccd5713f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"eb4bc83c_e32bf114","updated":"2023-02-03 10:14:55.000000000","message":"recheck - Ceph job failure is unrelated","commit_id":"4aa62596dc500a2486ad72de3adb4241c833c2c7"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"2434b77b2444c9ad0902b610bfb0df79bbc0ad25","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"14e80156_2b2f6996","updated":"2023-02-17 17:14:22.000000000","message":"Code and tests look good; minor thing noted inline that could be addressed in a followup if it makes sense.","commit_id":"8e9a347c0e1d5821e43bacbd88ffd239c4274163"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"1ed82739ea77054866704df514d618585b3c029b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"101631c9_3dfeea34","updated":"2023-02-20 14:52:12.000000000","message":"Few questions inline but overall looks good.","commit_id":"8e9a347c0e1d5821e43bacbd88ffd239c4274163"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"8c4ce8ef6366015e951ed60524859c77951d2863","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"dd9928c8_d104f974","updated":"2023-02-17 18:16:20.000000000","message":"LGTM","commit_id":"8e9a347c0e1d5821e43bacbd88ffd239c4274163"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"33ff00e0963ac2dc57d4ab319ead9002cca2f1ed","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"1338b56f_31e07a33","updated":"2023-02-08 10:50:43.000000000","message":"recheck - Identity errors in cinder-tempest-plugin-lvm-lio-barbican","commit_id":"8e9a347c0e1d5821e43bacbd88ffd239c4274163"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"0c960ce1cdb066efd857761f37b4fbf98121b6c0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"c26c642c_28fb282e","updated":"2023-02-07 12:34:39.000000000","message":"recheck - failures unrelated to this patch","commit_id":"8e9a347c0e1d5821e43bacbd88ffd239c4274163"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"56988fa44b316515027905194031d6b8d76e3968","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"b470a757_4bdbe24c","updated":"2023-02-18 15:17:13.000000000","message":"recheck tempest failure that doesn\u0027t look like it\u0027s related.","commit_id":"8e9a347c0e1d5821e43bacbd88ffd239c4274163"}],"cinder/volume/driver.py":[{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"938477c993e2c4a96bdfc943e887332b1239a104","unresolved":true,"context_lines":[{"line_number":261,"context_line":"]"},{"line_number":262,"context_line":""},{"line_number":263,"context_line":"nvmeof_opts \u003d ["},{"line_number":264,"context_line":"    cfg.BoolOpt(\u0027nvmeof_new_conn_info\u0027,"},{"line_number":265,"context_line":"                default\u003dFalse,"},{"line_number":266,"context_line":"                help\u003d\u0027NVMe os-brick connector has 2 different connection info \u0027"},{"line_number":267,"context_line":"                     \u0027formats, this allows some NVMe-oF drivers that use the \u0027"}],"source_content_type":"text/x-python","patch_set":5,"id":"569b405d_129b472c","line":264,"updated":"2022-10-03 23:55:00.000000000","message":"Sorry for bikeshedding, but I am wary of using \"new\" or \"nextgen\" in this. What if we invent a yet another format for the properties? I would prefer this being called something like \"nvmeof_multi_conn_info\", or something to that effect.","commit_id":"6ba52f877e4dcd05f1e791dc54fc6204dfc6984e"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"78eb8f5e96dcf1e1de4b97b23c8e0f4a4610d0d8","unresolved":true,"context_lines":[{"line_number":261,"context_line":"]"},{"line_number":262,"context_line":""},{"line_number":263,"context_line":"nvmeof_opts \u003d ["},{"line_number":264,"context_line":"    cfg.BoolOpt(\u0027nvmeof_new_conn_info\u0027,"},{"line_number":265,"context_line":"                default\u003dFalse,"},{"line_number":266,"context_line":"                help\u003d\u0027NVMe os-brick connector has 2 different connection info \u0027"},{"line_number":267,"context_line":"                     \u0027formats, this allows some NVMe-oF drivers that use the \u0027"}],"source_content_type":"text/x-python","patch_set":5,"id":"f6ce2c05_4954c689","line":264,"in_reply_to":"569b405d_129b472c","updated":"2023-01-26 18:42:21.000000000","message":"I see your point, even though I don\u0027t think we\u0027ll have another BREAKING connection information format for nvmeof in the future I\u0027ll see if I can easily change this to \"nvmeof_conn_info_version\" and have it have numbers 1 for the old (default), and 2 for the new.","commit_id":"6ba52f877e4dcd05f1e791dc54fc6204dfc6984e"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"2434b77b2444c9ad0902b610bfb0df79bbc0ad25","unresolved":false,"context_lines":[{"line_number":261,"context_line":"]"},{"line_number":262,"context_line":""},{"line_number":263,"context_line":"nvmeof_opts \u003d ["},{"line_number":264,"context_line":"    cfg.BoolOpt(\u0027nvmeof_new_conn_info\u0027,"},{"line_number":265,"context_line":"                default\u003dFalse,"},{"line_number":266,"context_line":"                help\u003d\u0027NVMe os-brick connector has 2 different connection info \u0027"},{"line_number":267,"context_line":"                     \u0027formats, this allows some NVMe-oF drivers that use the \u0027"}],"source_content_type":"text/x-python","patch_set":5,"id":"af35e08e_a8b2130e","line":264,"in_reply_to":"f6ce2c05_4954c689","updated":"2023-02-17 17:14:22.000000000","message":"Done","commit_id":"6ba52f877e4dcd05f1e791dc54fc6204dfc6984e"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"1ed82739ea77054866704df514d618585b3c029b","unresolved":true,"context_lines":[{"line_number":264,"context_line":"]"},{"line_number":265,"context_line":""},{"line_number":266,"context_line":"nvmeof_opts \u003d ["},{"line_number":267,"context_line":"    cfg.IntOpt(\u0027nvmeof_conn_info_version\u0027,"},{"line_number":268,"context_line":"               default\u003d1,"},{"line_number":269,"context_line":"               min\u003d1, max\u003d2,"},{"line_number":270,"context_line":"               help\u003d\u0027NVMe os-brick connector has 2 different connection info \u0027"}],"source_content_type":"text/x-python","patch_set":10,"id":"6362ad14_8f6fbbe3","line":267,"range":{"start_line":267,"start_character":16,"end_line":267,"end_character":40},"updated":"2023-02-20 14:52:12.000000000","message":"nit: this is a good way to make these changes extendable and backward compatible at the same time (a new change in conn info format can introduce version 3).\n\nHowever, I feel using the integer values can be a bit confusing. I would\u0027ve preferred if we had used a boolean config option, like, enable_new_conn_info_format\u003dTrue for new conn info and default would be False for old conn info.\nIn any case, this approach works so I\u0027m good with it.","commit_id":"8e9a347c0e1d5821e43bacbd88ffd239c4274163"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"414384bd40618b3bc8770548260780404077ac9e","unresolved":true,"context_lines":[{"line_number":264,"context_line":"]"},{"line_number":265,"context_line":""},{"line_number":266,"context_line":"nvmeof_opts \u003d ["},{"line_number":267,"context_line":"    cfg.IntOpt(\u0027nvmeof_conn_info_version\u0027,"},{"line_number":268,"context_line":"               default\u003d1,"},{"line_number":269,"context_line":"               min\u003d1, max\u003d2,"},{"line_number":270,"context_line":"               help\u003d\u0027NVMe os-brick connector has 2 different connection info \u0027"}],"source_content_type":"text/x-python","patch_set":10,"id":"21bc10ce_1fd2b613","line":267,"range":{"start_line":267,"start_character":16,"end_line":267,"end_character":40},"in_reply_to":"3eb5b46b_728bf821","updated":"2023-02-21 07:44:32.000000000","message":"IMO these kind of changes are rare, I don\u0027t think we\u0027ve updated the connection information format of iSCSI or FC since a long time. Also changing the format multiple times is problematic for nvmeof drivers as they also have to update their code to send the information in new format.\nHaving said that, I don\u0027t have any issues with the current approach but i felt it might be confusing for operators who will need to configure this option but it\u0027s just my opinion.","commit_id":"8e9a347c0e1d5821e43bacbd88ffd239c4274163"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"36f939d98931d2cddcdb7bb3e154dfd2c4edb7bc","unresolved":true,"context_lines":[{"line_number":264,"context_line":"]"},{"line_number":265,"context_line":""},{"line_number":266,"context_line":"nvmeof_opts \u003d ["},{"line_number":267,"context_line":"    cfg.IntOpt(\u0027nvmeof_conn_info_version\u0027,"},{"line_number":268,"context_line":"               default\u003d1,"},{"line_number":269,"context_line":"               min\u003d1, max\u003d2,"},{"line_number":270,"context_line":"               help\u003d\u0027NVMe os-brick connector has 2 different connection info \u0027"}],"source_content_type":"text/x-python","patch_set":10,"id":"3eb5b46b_728bf821","line":267,"range":{"start_line":267,"start_character":16,"end_line":267,"end_character":40},"in_reply_to":"6362ad14_8f6fbbe3","updated":"2023-02-20 16:41:52.000000000","message":"On the other hand, if it were just a boolean then it limits it to just the old and the new versions. Having a number would allow it to be extended in the future without needing to introduce another config option change to handle multiple possible versions.","commit_id":"8e9a347c0e1d5821e43bacbd88ffd239c4274163"}],"cinder/volume/targets/nvmeof.py":[{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"1ed82739ea77054866704df514d618585b3c029b","unresolved":true,"context_lines":[{"line_number":108,"context_line":"          \u0027ns_id\u0027: namespace id associated with the subsystem, in case target"},{"line_number":109,"context_line":"                   doesn\u0027t provide the volume_uuid."},{"line_number":110,"context_line":"        }"},{"line_number":111,"context_line":"        Unlike the old format the transport_type can be one of RoCEv2 and tcp"},{"line_number":112,"context_line":""},{"line_number":113,"context_line":"        :return: dictionary with the connection properties using one of the 2"},{"line_number":114,"context_line":"                 existing formats depending on the nvmeof_conn_info_version"}],"source_content_type":"text/x-python","patch_set":10,"id":"adc3acce_a537ab60","line":111,"range":{"start_line":111,"start_character":34,"end_line":111,"end_character":48},"updated":"2023-02-20 14:52:12.000000000","message":"we didn\u0027t mention the transport type in the new format above?","commit_id":"8e9a347c0e1d5821e43bacbd88ffd239c4274163"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"2434b77b2444c9ad0902b610bfb0df79bbc0ad25","unresolved":true,"context_lines":[{"line_number":123,"context_line":"        if self.configuration.nvmeof_conn_info_version \u003d\u003d 2:"},{"line_number":124,"context_line":"            uuid \u003d self._get_nvme_uuid(volume)"},{"line_number":125,"context_line":"            if nvme_transport_type \u003d\u003d \u0027rdma\u0027:"},{"line_number":126,"context_line":"                nvme_transport_type \u003d \u0027RoCEv2\u0027"},{"line_number":127,"context_line":""},{"line_number":128,"context_line":"            return {"},{"line_number":129,"context_line":"                \u0027target_nqn\u0027: nqn,"}],"source_content_type":"text/x-python","patch_set":10,"id":"36986705_9d9e5ed8","line":126,"range":{"start_line":126,"start_character":39,"end_line":126,"end_character":45},"updated":"2023-02-17 17:14:22.000000000","message":"Should this be defined in cinder.common.constants so we don\u0027t run into case-sensitivity problems in the future?","commit_id":"8e9a347c0e1d5821e43bacbd88ffd239c4274163"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"1ed82739ea77054866704df514d618585b3c029b","unresolved":true,"context_lines":[{"line_number":123,"context_line":"        if self.configuration.nvmeof_conn_info_version \u003d\u003d 2:"},{"line_number":124,"context_line":"            uuid \u003d self._get_nvme_uuid(volume)"},{"line_number":125,"context_line":"            if nvme_transport_type \u003d\u003d \u0027rdma\u0027:"},{"line_number":126,"context_line":"                nvme_transport_type \u003d \u0027RoCEv2\u0027"},{"line_number":127,"context_line":""},{"line_number":128,"context_line":"            return {"},{"line_number":129,"context_line":"                \u0027target_nqn\u0027: nqn,"}],"source_content_type":"text/x-python","patch_set":10,"id":"2a9ab9b0_3cfb8fcc","line":126,"range":{"start_line":126,"start_character":39,"end_line":126,"end_character":45},"in_reply_to":"36986705_9d9e5ed8","updated":"2023-02-20 14:52:12.000000000","message":"+1","commit_id":"8e9a347c0e1d5821e43bacbd88ffd239c4274163"}]}
