)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":33612,"name":"yuval","email":"yuval@lightbitslabs.com","username":"yuval"},"change_message_id":"2541d88d30988bb13fec75a60d850018bf2734d3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"42c2d3dd_a03112c0","updated":"2022-01-09 07:43:42.000000000","message":"Lightbits LightOS is a clustered system that uses NVMe/TCP ANA support to dynamically deal with cluster changes, including LightOS cluster nodes going up and down and cluster expansion/shrink, i.e., the addition of new LightOS cluster nodes. Nova servers need to be connected (in the nvme connect sense) to all LightOS cluster nodes that have volumes accessible to those nova servers, *including new LightOS nodes that show up while the volume is already in use*. NVMe/TCP supports this through persistent discovery. Unfortunately there is incomplete/missing upstream support for NVMe/TCP discovery so we use a Lightbits provided discovery-client to handle it. The LightOS connector informs the discovery client which discovery services on the LightOS nodes to connect to and then the discovery-client takes care of nvme connecting to nodes as they show up. Long term we\u0027d love to switch to a common connector and whatever discovery solution finds its way to Linux upstream and makes our discovery-client unnecessary, but for the time being the LightOS-specific connector and the discovery-client are required for correct operation against clustered NVMe/TCP solutions.","commit_id":"81b81898b929d27a27656e661597afd0aa84d7d7"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@purestorage.com","username":"sdodsley"},"change_message_id":"ad931cf05d8ffad7ad2d081b2761ecf091b01506","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"7a479ce3_a614e97c","updated":"2022-01-05 14:35:11.000000000","message":"Not sure why you are using your own connector. You should try and use the NVMeoF connector as this has TCP support already in it","commit_id":"81b81898b929d27a27656e661597afd0aa84d7d7"},{"author":{"_account_id":33612,"name":"yuval","email":"yuval@lightbitslabs.com","username":"yuval"},"change_message_id":"2ed08f74db0a3e108d0d6832c1703673b6a7caba","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"2ccd2b2c_a481bd22","in_reply_to":"33d7ba0d_ca7a7ceb","updated":"2022-01-09 19:05:57.000000000","message":"it\u0027s not the connector that\u0027s missing it (or feature in it), it\u0027s the upstream kernel and nvme-cli package, and that we are working with the community to add it but it\u0027s not something that will happen in the near future.","commit_id":"81b81898b929d27a27656e661597afd0aa84d7d7"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@purestorage.com","username":"sdodsley"},"change_message_id":"ee6f5400b7c403a495f057406bd894ae6eaffe13","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"33d7ba0d_ca7a7ceb","in_reply_to":"46c075a5_435fe428","updated":"2022-01-09 14:01:27.000000000","message":"If there is missing support for persistent discovery in the current NVMe connectior, then it would make sense for you to contribute this code rather than adding a proprietary connector.\nI know Kioxia have done a lot of changes to the NVMe connector in both the Xena cycle and and the current Yoga cycle.\nMaybe some of these have moved the goalposts closer to your requirments.","commit_id":"81b81898b929d27a27656e661597afd0aa84d7d7"},{"author":{"_account_id":33612,"name":"yuval","email":"yuval@lightbitslabs.com","username":"yuval"},"change_message_id":"2541d88d30988bb13fec75a60d850018bf2734d3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"46c075a5_435fe428","in_reply_to":"7a479ce3_a614e97c","updated":"2022-01-09 07:43:42.000000000","message":"Lightbits LightOS is a clustered system that uses NVMe/TCP ANA support to dynamically deal with cluster changes, including LightOS cluster nodes going up and down and cluster expansion/shrink, i.e., the addition of new LightOS cluster nodes. Nova servers need to be connected (in the nvme connect sense) to all LightOS cluster nodes that have volumes accessible to those nova servers, *including new LightOS nodes that show up while the volume is already in use*. NVMe/TCP supports this through persistent discovery. Unfortunately there is incomplete/missing upstream support for NVMe/TCP discovery so we use a Lightbits provided discovery-client to handle it. The LightOS connector informs the discovery client which discovery services on the LightOS nodes to connect to and then the discovery-client takes care of nvme connecting to nodes as they show up. Long term we\u0027d love to switch to a common connector and whatever discovery solution finds its way to Linux upstream and makes our discovery-client unnecessary, but for the time being the LightOS-specific connector and the discovery-client are required for correct operation against clustered NVMe/TCP solutions.","commit_id":"81b81898b929d27a27656e661597afd0aa84d7d7"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"ebeed2259c751af92af4fce66c5f19d5b367c356","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"68c6968b_da6feb65","updated":"2022-01-13 17:17:53.000000000","message":"Ignore my previous comment, Simon already told me to look at the conversation from patchset #3","commit_id":"ab1b17b2019101e01892f5ff744388364b0d97b8"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"07d7e1b25b3218b3fd71f91a91f75e8b53909ec8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"cb91e148_a325043b","updated":"2022-01-13 16:33:32.000000000","message":"Isn\u0027t this just NVMe/TCP?\nBecause we already have that connector...","commit_id":"ab1b17b2019101e01892f5ff744388364b0d97b8"},{"author":{"_account_id":34459,"name":"Muli Ben-Yehuda","email":"muli@lightbitslabs.com","username":"muliby"},"change_message_id":"aee7d724521b4648e88ef40fc4452398e776d08a","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":6,"id":"427a06fe_8755fd7b","updated":"2022-01-14 09:38:49.000000000","message":"Thanks for the review, Gorka! We are on it.","commit_id":"ab1b17b2019101e01892f5ff744388364b0d97b8"},{"author":{"_account_id":33612,"name":"yuval","email":"yuval@lightbitslabs.com","username":"yuval"},"change_message_id":"9dbc0798aac0e74f0c9f16b34331633cb41d6bf9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"c9016c82_0ced7a2f","in_reply_to":"427a06fe_8755fd7b","updated":"2022-01-30 09:21:59.000000000","message":"Done","commit_id":"ab1b17b2019101e01892f5ff744388364b0d97b8"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@purestorage.com","username":"sdodsley"},"change_message_id":"ca4060f77d46d033033b9fc892870bf96e6657fb","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"92c4143f_a1feb441","updated":"2022-01-21 22:13:24.000000000","message":"All looks good to me, now that everyone\u0027s comments have been addressed","commit_id":"bb01f874f568efff44fcc896ff93b1a7811b214d"},{"author":{"_account_id":33612,"name":"yuval","email":"yuval@lightbitslabs.com","username":"yuval"},"change_message_id":"b10473b95a1fe5bb47e926704817bc8510c987c7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"079f086a_ecd3f5cd","updated":"2022-01-27 03:54:13.000000000","message":"upload a patch soon","commit_id":"bb01f874f568efff44fcc896ff93b1a7811b214d"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@purestorage.com","username":"sdodsley"},"change_message_id":"34146a45dd90c34824bfa82cefc017b782d5f0f0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"a6f5442a_f8feb171","updated":"2022-01-27 15:26:26.000000000","message":"Some inline comments, and some of Gorka\u0027s comments have been Ack\u0027ed but no changes to the code, plus still some unresolved comments from Gorka","commit_id":"85ac116d84a3a0e3538953c14058c29b4f15a1b0"},{"author":{"_account_id":34459,"name":"Muli Ben-Yehuda","email":"muli@lightbitslabs.com","username":"muliby"},"change_message_id":"3b307829ccd656456ca5c96f29cc4f6c97b6abd6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":14,"id":"ee74b1d2_b9b2cff8","updated":"2022-01-30 09:30:32.000000000","message":"Thank you everyone for the constructive feedback and fruitful discussions. I believe this code is now ready for merge.","commit_id":"83a7710f5e4b972e8f95ce39bb9c747d6514c367"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"5c14f7fb8014ae1ef9eb59e5547ddea8623713b2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":14,"id":"d8c88201_e1d978af","updated":"2022-02-01 11:52:18.000000000","message":"Thank you for all the changes. There are just 2 more things: a simple change in a LOG call and flushing data on the disconnect call.","commit_id":"83a7710f5e4b972e8f95ce39bb9c747d6514c367"},{"author":{"_account_id":33612,"name":"yuval","email":"yuval@lightbitslabs.com","username":"yuval"},"change_message_id":"9dbc0798aac0e74f0c9f16b34331633cb41d6bf9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":14,"id":"c6ebcb2f_67eee66b","updated":"2022-01-30 09:21:59.000000000","message":"resolved few comments to have clear sight on what still need to communicate","commit_id":"83a7710f5e4b972e8f95ce39bb9c747d6514c367"},{"author":{"_account_id":34470,"name":"Yan Tseitlin","email":"yan@lightbitslabs.com","username":"yants"},"change_message_id":"aa15f759b186763de20801767045757f1b3374f9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":14,"id":"88841363_f116a1d0","updated":"2022-02-01 14:55:17.000000000","message":"run-Lightbits CI","commit_id":"83a7710f5e4b972e8f95ce39bb9c747d6514c367"},{"author":{"_account_id":34470,"name":"Yan Tseitlin","email":"yan@lightbitslabs.com","username":"yants"},"change_message_id":"ff3588eb7fcab399a5498af0bb0bc9eedebcb759","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":14,"id":"b7720521_5bd58d4e","updated":"2022-02-01 14:54:55.000000000","message":"run-Lightbits CI","commit_id":"83a7710f5e4b972e8f95ce39bb9c747d6514c367"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"26a28b2710af91dd31bb07a6995647a150d8b83d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":15,"id":"8af2fd7c_722ecc75","updated":"2022-02-01 16:47:37.000000000","message":"LGTM","commit_id":"627da6a40b4679a41245502b12e06b8340f1ca05"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"fb4a86eb2d5fce9b24893622960c2bcb6790dc6e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":15,"id":"1872baf2_77df9804","updated":"2022-02-01 23:53:08.000000000","message":"Looks like the most pressing concerns have been met, and the third-party CI has reported success.\n\nMerging with the understanding that you\u0027ll be providing a follow-up patch that addresses at least the following points:\n\n1. add os_brick/privileged/lightos.py\n2. get_connector_properties: don\u0027t return hostnqn\n3. question on PS15 about force disconnect volume\n4. add a release note (for os-brick) announcing the new connector\n\nWe want the followup to be in the Yoga os-brick release, which means that it needs to merge by Thursday next week.  So congratulations on your new Cinder driver and os-brick connector, but don\u0027t celebrate too much until *after* the followup patch has been merged!","commit_id":"627da6a40b4679a41245502b12e06b8340f1ca05"}],"os_brick/initiator/connectors/lightos.py":[{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@purestorage.com","username":"sdodsley"},"change_message_id":"3456ca432e84418d6d927b74fe105c4debc5f3c4","unresolved":true,"context_lines":[{"line_number":69,"context_line":"        lightos_connector \u003d LightOSConnector(root_helper\u003droot_helper,"},{"line_number":70,"context_line":"                                             message_queue\u003dNone,"},{"line_number":71,"context_line":"                                             execute\u003dkwargs.get(\u0027execute\u0027))"},{"line_number":72,"context_line":"        hostnqn \u003d lightos_connector.get_hostnqn()"},{"line_number":73,"context_line":"        found_dsc \u003d lightos_connector.find_dsc()"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"        if not found_dsc:"}],"source_content_type":"text/x-python","patch_set":4,"id":"11a5713b_07257b54","line":72,"updated":"2022-01-11 13:35:48.000000000","message":"There is no module in the connector called this","commit_id":"bae6d11b42a95b7444e973674dad7656d6dcdac2"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"09f0d98f1a83007f83c26b913106bad2e1021409","unresolved":false,"context_lines":[{"line_number":69,"context_line":"        lightos_connector \u003d LightOSConnector(root_helper\u003droot_helper,"},{"line_number":70,"context_line":"                                             message_queue\u003dNone,"},{"line_number":71,"context_line":"                                             execute\u003dkwargs.get(\u0027execute\u0027))"},{"line_number":72,"context_line":"        hostnqn \u003d lightos_connector.get_hostnqn()"},{"line_number":73,"context_line":"        found_dsc \u003d lightos_connector.find_dsc()"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"        if not found_dsc:"}],"source_content_type":"text/x-python","patch_set":4,"id":"50dbdf4c_0314b234","line":72,"in_reply_to":"11a5713b_07257b54","updated":"2022-01-26 19:36:52.000000000","message":"Now there is on L319.","commit_id":"bae6d11b42a95b7444e973674dad7656d6dcdac2"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@purestorage.com","username":"sdodsley"},"change_message_id":"3456ca432e84418d6d927b74fe105c4debc5f3c4","unresolved":true,"context_lines":[{"line_number":109,"context_line":"    def dsc_do_connect_volume(self, connection_info):"},{"line_number":110,"context_line":"        subsysnqn \u003d connection_info[\u0027nqn\u0027]"},{"line_number":111,"context_line":"        uuid \u003d connection_info[\u0027uuid\u0027]"},{"line_number":112,"context_line":"        hostnqn \u003d self.get_hostnqn()"},{"line_number":113,"context_line":"        with tempfile.NamedTemporaryFile(mode\u003d\u0027w\u0027, delete\u003dFalse) as dscfile:"},{"line_number":114,"context_line":"            dscfile.write(\u0027# os_brick connector dsc file for LightOS\u0027"},{"line_number":115,"context_line":"                          \u0027 volume: {}\\n\u0027.format(uuid))"}],"source_content_type":"text/x-python","patch_set":4,"id":"406322c5_ece16042","line":112,"updated":"2022-01-11 13:35:48.000000000","message":"Do you mean self._get_host_nqn?","commit_id":"bae6d11b42a95b7444e973674dad7656d6dcdac2"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"09f0d98f1a83007f83c26b913106bad2e1021409","unresolved":true,"context_lines":[{"line_number":109,"context_line":"    def dsc_do_connect_volume(self, connection_info):"},{"line_number":110,"context_line":"        subsysnqn \u003d connection_info[\u0027nqn\u0027]"},{"line_number":111,"context_line":"        uuid \u003d connection_info[\u0027uuid\u0027]"},{"line_number":112,"context_line":"        hostnqn \u003d self.get_hostnqn()"},{"line_number":113,"context_line":"        with tempfile.NamedTemporaryFile(mode\u003d\u0027w\u0027, delete\u003dFalse) as dscfile:"},{"line_number":114,"context_line":"            dscfile.write(\u0027# os_brick connector dsc file for LightOS\u0027"},{"line_number":115,"context_line":"                          \u0027 volume: {}\\n\u0027.format(uuid))"}],"source_content_type":"text/x-python","patch_set":4,"id":"e8c4c76f_9bd4fd6b","line":112,"in_reply_to":"2e5181a4_c6ba0f52","updated":"2022-01-26 19:36:52.000000000","message":"I believe this is correct, since the method is get_hostnqn and not _get_hostnqn","commit_id":"bae6d11b42a95b7444e973674dad7656d6dcdac2"},{"author":{"_account_id":33612,"name":"yuval","email":"yuval@lightbitslabs.com","username":"yuval"},"change_message_id":"67780e1e489aa913ed23bb9d6c76d5fd525e830c","unresolved":true,"context_lines":[{"line_number":109,"context_line":"    def dsc_do_connect_volume(self, connection_info):"},{"line_number":110,"context_line":"        subsysnqn \u003d connection_info[\u0027nqn\u0027]"},{"line_number":111,"context_line":"        uuid \u003d connection_info[\u0027uuid\u0027]"},{"line_number":112,"context_line":"        hostnqn \u003d self.get_hostnqn()"},{"line_number":113,"context_line":"        with tempfile.NamedTemporaryFile(mode\u003d\u0027w\u0027, delete\u003dFalse) as dscfile:"},{"line_number":114,"context_line":"            dscfile.write(\u0027# os_brick connector dsc file for LightOS\u0027"},{"line_number":115,"context_line":"                          \u0027 volume: {}\\n\u0027.format(uuid))"}],"source_content_type":"text/x-python","patch_set":4,"id":"2e5181a4_c6ba0f52","line":112,"in_reply_to":"406322c5_ece16042","updated":"2022-01-11 20:45:55.000000000","message":"I have inserted a bug, while trying to fix an issue we had in the get_hostnqn function.\nThank you for noticing I will upload a fix asap.","commit_id":"bae6d11b42a95b7444e973674dad7656d6dcdac2"},{"author":{"_account_id":33612,"name":"yuval","email":"yuval@lightbitslabs.com","username":"yuval"},"change_message_id":"4a62188710facfce8bc6238e160b80b26ee25458","unresolved":false,"context_lines":[{"line_number":109,"context_line":"    def dsc_do_connect_volume(self, connection_info):"},{"line_number":110,"context_line":"        subsysnqn \u003d connection_info[\u0027nqn\u0027]"},{"line_number":111,"context_line":"        uuid \u003d connection_info[\u0027uuid\u0027]"},{"line_number":112,"context_line":"        hostnqn \u003d self.get_hostnqn()"},{"line_number":113,"context_line":"        with tempfile.NamedTemporaryFile(mode\u003d\u0027w\u0027, delete\u003dFalse) as dscfile:"},{"line_number":114,"context_line":"            dscfile.write(\u0027# os_brick connector dsc file for LightOS\u0027"},{"line_number":115,"context_line":"                          \u0027 volume: {}\\n\u0027.format(uuid))"}],"source_content_type":"text/x-python","patch_set":4,"id":"3bfd890f_55ffe004","line":112,"in_reply_to":"e8c4c76f_9bd4fd6b","updated":"2022-01-26 20:22:48.000000000","message":"Ack","commit_id":"bae6d11b42a95b7444e973674dad7656d6dcdac2"},{"author":{"_account_id":34459,"name":"Muli Ben-Yehuda","email":"muli@lightbitslabs.com","username":"muliby"},"change_message_id":"aee7d724521b4648e88ef40fc4452398e776d08a","unresolved":true,"context_lines":[{"line_number":1,"context_line":"# Copyright (C) 2016-2020 Lightbits Labs Ltd."},{"line_number":2,"context_line":"# Copyright (C) 2020 Intel Corporation"},{"line_number":3,"context_line":"# All Rights Reserved."},{"line_number":4,"context_line":"#"}],"source_content_type":"text/x-python","patch_set":6,"id":"1dff9ea6_991cbddc","line":1,"updated":"2022-01-14 09:38:49.000000000","message":"Yuval let\u0027s update this to 2022","commit_id":"ab1b17b2019101e01892f5ff744388364b0d97b8"},{"author":{"_account_id":33612,"name":"yuval","email":"yuval@lightbitslabs.com","username":"yuval"},"change_message_id":"9dbc0798aac0e74f0c9f16b34331633cb41d6bf9","unresolved":false,"context_lines":[{"line_number":1,"context_line":"# Copyright (C) 2016-2020 Lightbits Labs Ltd."},{"line_number":2,"context_line":"# Copyright (C) 2020 Intel Corporation"},{"line_number":3,"context_line":"# All Rights Reserved."},{"line_number":4,"context_line":"#"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa174e5c_d6ef7e5a","line":1,"in_reply_to":"1dff9ea6_991cbddc","updated":"2022-01-30 09:21:59.000000000","message":"Done","commit_id":"ab1b17b2019101e01892f5ff744388364b0d97b8"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"ebeed2259c751af92af4fce66c5f19d5b367c356","unresolved":true,"context_lines":[{"line_number":78,"context_line":"        if hostnqn:"},{"line_number":79,"context_line":"            LOG.info(\"LIGHTOS: finally hostnqn: %s dsc: %s\","},{"line_number":80,"context_line":"                     hostnqn, found_dsc)"},{"line_number":81,"context_line":"            props[\u0027hostnqn\u0027] \u003d hostnqn"},{"line_number":82,"context_line":"            props[\u0027found_dsc\u0027] \u003d found_dsc"},{"line_number":83,"context_line":"        else:"},{"line_number":84,"context_line":"            LOG.warning(\u0027LIGHTOS: no hostnqn found.\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"3fbec1a9_42f491d6","line":81,"range":{"start_line":81,"start_character":12,"end_line":81,"end_character":38},"updated":"2022-01-13 17:17:53.000000000","message":"The NVMe connector is already returning the NQN in the \u0027nqn\u0027 key running the same code, so we have code duplication as well as data duplication in the connector info.\n\nI don\u0027t think that\u0027s a good idea.","commit_id":"ab1b17b2019101e01892f5ff744388364b0d97b8"},{"author":{"_account_id":33612,"name":"yuval","email":"yuval@lightbitslabs.com","username":"yuval"},"change_message_id":"4a62188710facfce8bc6238e160b80b26ee25458","unresolved":true,"context_lines":[{"line_number":78,"context_line":"        if hostnqn:"},{"line_number":79,"context_line":"            LOG.info(\"LIGHTOS: finally hostnqn: %s dsc: %s\","},{"line_number":80,"context_line":"                     hostnqn, found_dsc)"},{"line_number":81,"context_line":"            props[\u0027hostnqn\u0027] \u003d hostnqn"},{"line_number":82,"context_line":"            props[\u0027found_dsc\u0027] \u003d found_dsc"},{"line_number":83,"context_line":"        else:"},{"line_number":84,"context_line":"            LOG.warning(\u0027LIGHTOS: no hostnqn found.\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"a689a1f9_4818fc8f","line":81,"range":{"start_line":81,"start_character":12,"end_line":81,"end_character":38},"in_reply_to":"152defba_1649b3b6","updated":"2022-01-26 20:22:48.000000000","message":"but why would the nvmeof connector will run (execute the call)? its using the lightos connector - or I am missing something?","commit_id":"ab1b17b2019101e01892f5ff744388364b0d97b8"},{"author":{"_account_id":34459,"name":"Muli Ben-Yehuda","email":"muli@lightbitslabs.com","username":"muliby"},"change_message_id":"1d217e50d3aa3530888ebc58287ad8d99967b599","unresolved":true,"context_lines":[{"line_number":78,"context_line":"        if hostnqn:"},{"line_number":79,"context_line":"            LOG.info(\"LIGHTOS: finally hostnqn: %s dsc: %s\","},{"line_number":80,"context_line":"                     hostnqn, found_dsc)"},{"line_number":81,"context_line":"            props[\u0027hostnqn\u0027] \u003d hostnqn"},{"line_number":82,"context_line":"            props[\u0027found_dsc\u0027] \u003d found_dsc"},{"line_number":83,"context_line":"        else:"},{"line_number":84,"context_line":"            LOG.warning(\u0027LIGHTOS: no hostnqn found.\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"4b8bda13_b397c387","line":81,"range":{"start_line":81,"start_character":12,"end_line":81,"end_character":38},"in_reply_to":"1fbe2199_098c6782","updated":"2022-01-14 18:11:28.000000000","message":"Agree it would be good to get Kumoscale feedback and sync going forward. I do think, however, that given the release timeline, it would be the right thing to merge the LightOS os_brick connector (tested and known to work) in addition to the existing NVMe connector, and then rework both if feasible so that the NVMe connector will support both use-cases going forward. Makes sense?\n\nAs to kernel and nvme-cli versions for persistent discovery support, kernels \u003e\u003d 5.6 support persistent discovery, still checking with regards to nvme-cli versions.","commit_id":"ab1b17b2019101e01892f5ff744388364b0d97b8"},{"author":{"_account_id":34459,"name":"Muli Ben-Yehuda","email":"muli@lightbitslabs.com","username":"muliby"},"change_message_id":"aee7d724521b4648e88ef40fc4452398e776d08a","unresolved":true,"context_lines":[{"line_number":78,"context_line":"        if hostnqn:"},{"line_number":79,"context_line":"            LOG.info(\"LIGHTOS: finally hostnqn: %s dsc: %s\","},{"line_number":80,"context_line":"                     hostnqn, found_dsc)"},{"line_number":81,"context_line":"            props[\u0027hostnqn\u0027] \u003d hostnqn"},{"line_number":82,"context_line":"            props[\u0027found_dsc\u0027] \u003d found_dsc"},{"line_number":83,"context_line":"        else:"},{"line_number":84,"context_line":"            LOG.warning(\u0027LIGHTOS: no hostnqn found.\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"c157debf_f9efa729","line":81,"range":{"start_line":81,"start_character":12,"end_line":81,"end_character":38},"in_reply_to":"3fbec1a9_42f491d6","updated":"2022-01-14 09:38:49.000000000","message":"You are aboslutely right in general but the NVMe/TCP connector is not suitable for LightOS at this point in time. LightOS is a *clustered* NVMe/TCP system that makes use of NVMe/TCP ANA and persistent discovery, two standard NVMe/TCP features. Unfortunately some versions of the Linux kernel and nvme-cli utils do not yet do a good job of dealing with persistent discovery, so the LightOS connector uses the Lightbits discovery-client for connecting to discovery services on the LightOS cluster and establishing NVMe/TCP paths to the LightOS cluster nodes as needed. Once the NVMe/TCP ecosystem, including the openstack NVMe/TCP connector, progresses to support ANA and persistent discovvery out of the box, we\u0027ll be able to retire the discovery-client and the LightOS connector and switch to using the standard NVMe/TCP connector, but we are not there yet.","commit_id":"ab1b17b2019101e01892f5ff744388364b0d97b8"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"09f0d98f1a83007f83c26b913106bad2e1021409","unresolved":true,"context_lines":[{"line_number":78,"context_line":"        if hostnqn:"},{"line_number":79,"context_line":"            LOG.info(\"LIGHTOS: finally hostnqn: %s dsc: %s\","},{"line_number":80,"context_line":"                     hostnqn, found_dsc)"},{"line_number":81,"context_line":"            props[\u0027hostnqn\u0027] \u003d hostnqn"},{"line_number":82,"context_line":"            props[\u0027found_dsc\u0027] \u003d found_dsc"},{"line_number":83,"context_line":"        else:"},{"line_number":84,"context_line":"            LOG.warning(\u0027LIGHTOS: no hostnqn found.\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"152defba_1649b3b6","line":81,"range":{"start_line":81,"start_character":12,"end_line":81,"end_character":38},"in_reply_to":"4b8bda13_b397c387","updated":"2022-01-26 19:36:52.000000000","message":"I think I wasn\u0027t clear in my explanation.\n\nThis connector sets the \"hostnqn\" value in the connector_info dictionary to the result of the \"get_hostnqn\" method, but that value is already being reported in key \"nqn\" by the NVMe-of connector.\n\nIn other words, when Nova wants to do an attachment is going to be returning the same data twice:\n\n {\n  \u0027nqn\u0027:  XYZ,    \u003d\u003d\u003e Set by the NVMe connector\n  \u0027hostnqn\u0027: XYZ, \u003d\u003d\u003e Set by this connector\n  . . .\n }\n\nBoth will have the same value, and I think we shouldn\u0027t be doing that and we should just have the \"nqn\" value.","commit_id":"ab1b17b2019101e01892f5ff744388364b0d97b8"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@purestorage.com","username":"sdodsley"},"change_message_id":"b06daaaf6de668369d9309a55ac70d0a1413e1fa","unresolved":true,"context_lines":[{"line_number":78,"context_line":"        if hostnqn:"},{"line_number":79,"context_line":"            LOG.info(\"LIGHTOS: finally hostnqn: %s dsc: %s\","},{"line_number":80,"context_line":"                     hostnqn, found_dsc)"},{"line_number":81,"context_line":"            props[\u0027hostnqn\u0027] \u003d hostnqn"},{"line_number":82,"context_line":"            props[\u0027found_dsc\u0027] \u003d found_dsc"},{"line_number":83,"context_line":"        else:"},{"line_number":84,"context_line":"            LOG.warning(\u0027LIGHTOS: no hostnqn found.\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"1fbe2199_098c6782","line":81,"range":{"start_line":81,"start_character":12,"end_line":81,"end_character":38},"in_reply_to":"64082214_6504b1d0","updated":"2022-01-14 15:30:02.000000000","message":"The NVMe connector, although originally based on mdraid has been modieifed by Kumoscale to be generic - we are expecting it to work with all NVMe ANA backends, \nThe recent changes they have made were very specifically to support ANA and dynamic discovery to my understanding.\nWe shoulkd try and get someone from Kumoscale to validate this.","commit_id":"ab1b17b2019101e01892f5ff744388364b0d97b8"},{"author":{"_account_id":34459,"name":"Muli Ben-Yehuda","email":"muli@lightbitslabs.com","username":"muliby"},"change_message_id":"bf76a41b4354d3e856da90effca915ef33174908","unresolved":true,"context_lines":[{"line_number":78,"context_line":"        if hostnqn:"},{"line_number":79,"context_line":"            LOG.info(\"LIGHTOS: finally hostnqn: %s dsc: %s\","},{"line_number":80,"context_line":"                     hostnqn, found_dsc)"},{"line_number":81,"context_line":"            props[\u0027hostnqn\u0027] \u003d hostnqn"},{"line_number":82,"context_line":"            props[\u0027found_dsc\u0027] \u003d found_dsc"},{"line_number":83,"context_line":"        else:"},{"line_number":84,"context_line":"            LOG.warning(\u0027LIGHTOS: no hostnqn found.\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"64082214_6504b1d0","line":81,"range":{"start_line":81,"start_character":12,"end_line":81,"end_character":38},"in_reply_to":"a4f41363_0ef95e3a","updated":"2022-01-14 14:53:58.000000000","message":"Thanks, Simon. I went over the outstanding os_brick patches and I don\u0027t see anything that is obviously relevant. I thought the NVMeOF connection agent may be relevant but it appears to be Kumoscale and mdraid specific, so not useful for us. We do not use mdraid on the client, as Kumoscale apparently does.\n\nMore importantly, from what I can see on the Kioxia Kumoscale website, Kumoscale clusters do not support dynamic storage cluster changes, which are were persistent discovery comes into play -- you want the compute hosts to be notified and connect to new storage cluster nodes when such nodes show up. This is something that happens without any openstack involvement, so either openstack polls for it (highly inefficient) or you have to rely on NVMe/TCP persistent discovery to get notifications, which is what we do through the discovery-client. If I\u0027m misunderstanding how Kumoscale works, happy to be corrected!\n\nWith regards to kernel versions and nvme-cli versions that do not support persistent discovery properly, I\u0027m checking with the team and will update.","commit_id":"ab1b17b2019101e01892f5ff744388364b0d97b8"},{"author":{"_account_id":34459,"name":"Muli Ben-Yehuda","email":"muli@lightbitslabs.com","username":"muliby"},"change_message_id":"3b307829ccd656456ca5c96f29cc4f6c97b6abd6","unresolved":false,"context_lines":[{"line_number":78,"context_line":"        if hostnqn:"},{"line_number":79,"context_line":"            LOG.info(\"LIGHTOS: finally hostnqn: %s dsc: %s\","},{"line_number":80,"context_line":"                     hostnqn, found_dsc)"},{"line_number":81,"context_line":"            props[\u0027hostnqn\u0027] \u003d hostnqn"},{"line_number":82,"context_line":"            props[\u0027found_dsc\u0027] \u003d found_dsc"},{"line_number":83,"context_line":"        else:"},{"line_number":84,"context_line":"            LOG.warning(\u0027LIGHTOS: no hostnqn found.\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"579bd97e_01e7264d","line":81,"range":{"start_line":81,"start_character":12,"end_line":81,"end_character":38},"in_reply_to":"a689a1f9_4818fc8f","updated":"2022-01-30 09:30:32.000000000","message":"Further to Yuval\u0027s comment, regardless of wheter the nvmeof and the lightos connectors both set the host nqn\u0027s, it seems wrong to me for the lightos code to rely on an unrelated connector until we go through it and merge the two connectors, once it becomes possible. Furthremore, there doesn\u0027t need to be just one hostnqn -- each connector could come up with/use/create a different hostnqn and it would still work. So I\u0027m resolving this for now and we\u0027ll revisit when we tackle the merge of the two connectors.","commit_id":"ab1b17b2019101e01892f5ff744388364b0d97b8"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@purestorage.com","username":"sdodsley"},"change_message_id":"58d67a0436edc197703965ce1a4a346dea9db319","unresolved":true,"context_lines":[{"line_number":78,"context_line":"        if hostnqn:"},{"line_number":79,"context_line":"            LOG.info(\"LIGHTOS: finally hostnqn: %s dsc: %s\","},{"line_number":80,"context_line":"                     hostnqn, found_dsc)"},{"line_number":81,"context_line":"            props[\u0027hostnqn\u0027] \u003d hostnqn"},{"line_number":82,"context_line":"            props[\u0027found_dsc\u0027] \u003d found_dsc"},{"line_number":83,"context_line":"        else:"},{"line_number":84,"context_line":"            LOG.warning(\u0027LIGHTOS: no hostnqn found.\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"a4f41363_0ef95e3a","line":81,"range":{"start_line":81,"start_character":12,"end_line":81,"end_character":38},"in_reply_to":"c157debf_f9efa729","updated":"2022-01-14 14:05:21.000000000","message":"I\u0027m still confused by this comment.\nKioxia use ANA based TCP and they have updated the NVMe connector to support this. Please check the outstanding NVMe connector patches they have open for os-brick as they may resolve your concerns on the current NVMe connector.\nWhat versions of Linux kernel and nvme-cli are you concerned about? Remeber that Yoga will only support a specific list of kernels and will provide a specific nvme-cli version.\nPlease check that these are OK for you as they seem to be for Kioxia.\nIf there is a problem with the minim,um supported Yoga versions then please raise this as well.","commit_id":"ab1b17b2019101e01892f5ff744388364b0d97b8"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"ebeed2259c751af92af4fce66c5f19d5b367c356","unresolved":true,"context_lines":[{"line_number":122,"context_line":"            dscfile.flush()"},{"line_number":123,"context_line":"            try:"},{"line_number":124,"context_line":"                dest_name \u003d self.dsc_file_name(uuid)"},{"line_number":125,"context_line":"                cmd \u003d shlex.split(\u0027mv {} {}\u0027.format(dscfile.name, dest_name))"},{"line_number":126,"context_line":"                (out, err) \u003d self._execute(*cmd, root_helper\u003dself._root_helper,"},{"line_number":127,"context_line":"                                           run_as_root\u003dTrue)"},{"line_number":128,"context_line":"            except putils.ProcessExecutionError as e:"}],"source_content_type":"text/x-python","patch_set":6,"id":"2cb09896_a080c5b2","line":125,"range":{"start_line":125,"start_character":16,"end_line":125,"end_character":77},"updated":"2022-01-13 17:17:53.000000000","message":"-1: Isn\u0027t this like doing:\n\n cmd \u003d (\u0027mv\u0027, dscfile.name, dest_name)","commit_id":"ab1b17b2019101e01892f5ff744388364b0d97b8"},{"author":{"_account_id":34459,"name":"Muli Ben-Yehuda","email":"muli@lightbitslabs.com","username":"muliby"},"change_message_id":"aee7d724521b4648e88ef40fc4452398e776d08a","unresolved":true,"context_lines":[{"line_number":122,"context_line":"            dscfile.flush()"},{"line_number":123,"context_line":"            try:"},{"line_number":124,"context_line":"                dest_name \u003d self.dsc_file_name(uuid)"},{"line_number":125,"context_line":"                cmd \u003d shlex.split(\u0027mv {} {}\u0027.format(dscfile.name, dest_name))"},{"line_number":126,"context_line":"                (out, err) \u003d self._execute(*cmd, root_helper\u003dself._root_helper,"},{"line_number":127,"context_line":"                                           run_as_root\u003dTrue)"},{"line_number":128,"context_line":"            except putils.ProcessExecutionError as e:"}],"source_content_type":"text/x-python","patch_set":6,"id":"eca4d3dd_88b96bd4","line":125,"range":{"start_line":125,"start_character":16,"end_line":125,"end_character":77},"in_reply_to":"2cb09896_a080c5b2","updated":"2022-01-14 09:38:49.000000000","message":"In theory, no, shlex will shell escape the names (think about the case where dest_name has spaces in it for example). In practice, they are the same, but the shlex method has better future proofing.","commit_id":"ab1b17b2019101e01892f5ff744388364b0d97b8"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"09f0d98f1a83007f83c26b913106bad2e1021409","unresolved":false,"context_lines":[{"line_number":122,"context_line":"            dscfile.flush()"},{"line_number":123,"context_line":"            try:"},{"line_number":124,"context_line":"                dest_name \u003d self.dsc_file_name(uuid)"},{"line_number":125,"context_line":"                cmd \u003d shlex.split(\u0027mv {} {}\u0027.format(dscfile.name, dest_name))"},{"line_number":126,"context_line":"                (out, err) \u003d self._execute(*cmd, root_helper\u003dself._root_helper,"},{"line_number":127,"context_line":"                                           run_as_root\u003dTrue)"},{"line_number":128,"context_line":"            except putils.ProcessExecutionError as e:"}],"source_content_type":"text/x-python","patch_set":6,"id":"65663084_ab1c17fe","line":125,"range":{"start_line":125,"start_character":16,"end_line":125,"end_character":77},"in_reply_to":"eca4d3dd_88b96bd4","updated":"2022-01-26 19:36:52.000000000","message":"Ack","commit_id":"ab1b17b2019101e01892f5ff744388364b0d97b8"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"ebeed2259c751af92af4fce66c5f19d5b367c356","unresolved":true,"context_lines":[{"line_number":123,"context_line":"            try:"},{"line_number":124,"context_line":"                dest_name \u003d self.dsc_file_name(uuid)"},{"line_number":125,"context_line":"                cmd \u003d shlex.split(\u0027mv {} {}\u0027.format(dscfile.name, dest_name))"},{"line_number":126,"context_line":"                (out, err) \u003d self._execute(*cmd, root_helper\u003dself._root_helper,"},{"line_number":127,"context_line":"                                           run_as_root\u003dTrue)"},{"line_number":128,"context_line":"            except putils.ProcessExecutionError as e:"},{"line_number":129,"context_line":"                LOG.warning(\"LIGHTOS: Failed to run command {}\".format(cmd))"},{"line_number":130,"context_line":"                raise e"}],"source_content_type":"text/x-python","patch_set":6,"id":"3aa754a0_f376eaa4","line":127,"range":{"start_line":126,"start_character":0,"end_line":127,"end_character":60},"updated":"2022-01-13 17:17:53.000000000","message":"nit: We should be using a privsep method instead, it will be faster and more pythonic.  Please look at the unlink_root method in os_brick/privileged/rootwrap.py.","commit_id":"ab1b17b2019101e01892f5ff744388364b0d97b8"},{"author":{"_account_id":34459,"name":"Muli Ben-Yehuda","email":"muli@lightbitslabs.com","username":"muliby"},"change_message_id":"aee7d724521b4648e88ef40fc4452398e776d08a","unresolved":true,"context_lines":[{"line_number":123,"context_line":"            try:"},{"line_number":124,"context_line":"                dest_name \u003d self.dsc_file_name(uuid)"},{"line_number":125,"context_line":"                cmd \u003d shlex.split(\u0027mv {} {}\u0027.format(dscfile.name, dest_name))"},{"line_number":126,"context_line":"                (out, err) \u003d self._execute(*cmd, root_helper\u003dself._root_helper,"},{"line_number":127,"context_line":"                                           run_as_root\u003dTrue)"},{"line_number":128,"context_line":"            except putils.ProcessExecutionError as e:"},{"line_number":129,"context_line":"                LOG.warning(\"LIGHTOS: Failed to run command {}\".format(cmd))"},{"line_number":130,"context_line":"                raise e"}],"source_content_type":"text/x-python","patch_set":6,"id":"53771531_77e73308","line":127,"range":{"start_line":126,"start_character":0,"end_line":127,"end_character":60},"in_reply_to":"3aa754a0_f376eaa4","updated":"2022-01-14 09:38:49.000000000","message":"OK, we\u0027ll look at it.","commit_id":"ab1b17b2019101e01892f5ff744388364b0d97b8"},{"author":{"_account_id":34459,"name":"Muli Ben-Yehuda","email":"muli@lightbitslabs.com","username":"muliby"},"change_message_id":"c078440d51afb0f9f8b7641469d1966e015baa54","unresolved":true,"context_lines":[{"line_number":123,"context_line":"            try:"},{"line_number":124,"context_line":"                dest_name \u003d self.dsc_file_name(uuid)"},{"line_number":125,"context_line":"                cmd \u003d shlex.split(\u0027mv {} {}\u0027.format(dscfile.name, dest_name))"},{"line_number":126,"context_line":"                (out, err) \u003d self._execute(*cmd, root_helper\u003dself._root_helper,"},{"line_number":127,"context_line":"                                           run_as_root\u003dTrue)"},{"line_number":128,"context_line":"            except putils.ProcessExecutionError as e:"},{"line_number":129,"context_line":"                LOG.warning(\"LIGHTOS: Failed to run command {}\".format(cmd))"},{"line_number":130,"context_line":"                raise e"}],"source_content_type":"text/x-python","patch_set":6,"id":"806db7b2_5015981b","line":127,"range":{"start_line":126,"start_character":0,"end_line":127,"end_character":60},"in_reply_to":"53771531_77e73308","updated":"2022-01-14 18:37:28.000000000","message":"Gorka, could you help us a bit here. We looked at unlink_root and it\u0027s not clear how it helps us, unless you meant we should add `@privileged.default.entrypoint def mv_root` to rootwrap.py and use that? We hesitate to add common code without a better understanding of how all users would want to use it and prefer to keep the execute call localized here.","commit_id":"ab1b17b2019101e01892f5ff744388364b0d97b8"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"09f0d98f1a83007f83c26b913106bad2e1021409","unresolved":true,"context_lines":[{"line_number":123,"context_line":"            try:"},{"line_number":124,"context_line":"                dest_name \u003d self.dsc_file_name(uuid)"},{"line_number":125,"context_line":"                cmd \u003d shlex.split(\u0027mv {} {}\u0027.format(dscfile.name, dest_name))"},{"line_number":126,"context_line":"                (out, err) \u003d self._execute(*cmd, root_helper\u003dself._root_helper,"},{"line_number":127,"context_line":"                                           run_as_root\u003dTrue)"},{"line_number":128,"context_line":"            except putils.ProcessExecutionError as e:"},{"line_number":129,"context_line":"                LOG.warning(\"LIGHTOS: Failed to run command {}\".format(cmd))"},{"line_number":130,"context_line":"                raise e"}],"source_content_type":"text/x-python","patch_set":6,"id":"f3aaaba2_8142cfbb","line":127,"range":{"start_line":126,"start_character":0,"end_line":127,"end_character":60},"in_reply_to":"806db7b2_5015981b","updated":"2022-01-26 19:36:52.000000000","message":"I mentioned that method as an example, but you would actually create your own file under the os_brick/privileged directory, like the rbd driver has (os_brick/privileged/rbd.py).\n\nIn that file you would be adding all the privileged Python code you wanted to run (except the command line execution which would still go with _execute).  Your code would be a lot faster.\n\nFor example the \"rm -f\" in line 137 would be a call to os.remove inside the decorate privsep method.","commit_id":"ab1b17b2019101e01892f5ff744388364b0d97b8"},{"author":{"_account_id":33612,"name":"yuval","email":"yuval@lightbitslabs.com","username":"yuval"},"change_message_id":"b10473b95a1fe5bb47e926704817bc8510c987c7","unresolved":false,"context_lines":[{"line_number":123,"context_line":"            try:"},{"line_number":124,"context_line":"                dest_name \u003d self.dsc_file_name(uuid)"},{"line_number":125,"context_line":"                cmd \u003d shlex.split(\u0027mv {} {}\u0027.format(dscfile.name, dest_name))"},{"line_number":126,"context_line":"                (out, err) \u003d self._execute(*cmd, root_helper\u003dself._root_helper,"},{"line_number":127,"context_line":"                                           run_as_root\u003dTrue)"},{"line_number":128,"context_line":"            except putils.ProcessExecutionError as e:"},{"line_number":129,"context_line":"                LOG.warning(\"LIGHTOS: Failed to run command {}\".format(cmd))"},{"line_number":130,"context_line":"                raise e"}],"source_content_type":"text/x-python","patch_set":6,"id":"a2ce86fb_2bb4718a","line":127,"range":{"start_line":126,"start_character":0,"end_line":127,"end_character":60},"in_reply_to":"f3aaaba2_8142cfbb","updated":"2022-01-27 03:54:13.000000000","message":"Ack","commit_id":"ab1b17b2019101e01892f5ff744388364b0d97b8"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"ebeed2259c751af92af4fce66c5f19d5b367c356","unresolved":true,"context_lines":[{"line_number":132,"context_line":"    def dsc_disconnect_volume(self, connection_info):"},{"line_number":133,"context_line":"        uuid \u003d connection_info[\u0027uuid\u0027]"},{"line_number":134,"context_line":"        try:"},{"line_number":135,"context_line":"            cmd \u003d shlex.split(\u0027mv -f {} /tmp\u0027.format(self.dsc_file_name(uuid)))"},{"line_number":136,"context_line":"            (out, err) \u003d self._execute(*cmd, root_helper\u003dself._root_helper,"},{"line_number":137,"context_line":"                                       run_as_root\u003dTrue)"},{"line_number":138,"context_line":"        except putils.ProcessExecutionError as e:"}],"source_content_type":"text/x-python","patch_set":6,"id":"c700453e_44584a2c","line":135,"range":{"start_line":135,"start_character":30,"end_line":135,"end_character":44},"updated":"2022-01-13 17:17:53.000000000","message":"-1: We should not be leaving temp files with connection information...","commit_id":"ab1b17b2019101e01892f5ff744388364b0d97b8"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"ebeed2259c751af92af4fce66c5f19d5b367c356","unresolved":true,"context_lines":[{"line_number":132,"context_line":"    def dsc_disconnect_volume(self, connection_info):"},{"line_number":133,"context_line":"        uuid \u003d connection_info[\u0027uuid\u0027]"},{"line_number":134,"context_line":"        try:"},{"line_number":135,"context_line":"            cmd \u003d shlex.split(\u0027mv -f {} /tmp\u0027.format(self.dsc_file_name(uuid)))"},{"line_number":136,"context_line":"            (out, err) \u003d self._execute(*cmd, root_helper\u003dself._root_helper,"},{"line_number":137,"context_line":"                                       run_as_root\u003dTrue)"},{"line_number":138,"context_line":"        except putils.ProcessExecutionError as e:"}],"source_content_type":"text/x-python","patch_set":6,"id":"f9a281aa_ae6f0851","line":135,"updated":"2022-01-13 17:17:53.000000000","message":"-1: similar to L125","commit_id":"ab1b17b2019101e01892f5ff744388364b0d97b8"},{"author":{"_account_id":34459,"name":"Muli Ben-Yehuda","email":"muli@lightbitslabs.com","username":"muliby"},"change_message_id":"aee7d724521b4648e88ef40fc4452398e776d08a","unresolved":true,"context_lines":[{"line_number":132,"context_line":"    def dsc_disconnect_volume(self, connection_info):"},{"line_number":133,"context_line":"        uuid \u003d connection_info[\u0027uuid\u0027]"},{"line_number":134,"context_line":"        try:"},{"line_number":135,"context_line":"            cmd \u003d shlex.split(\u0027mv -f {} /tmp\u0027.format(self.dsc_file_name(uuid)))"},{"line_number":136,"context_line":"            (out, err) \u003d self._execute(*cmd, root_helper\u003dself._root_helper,"},{"line_number":137,"context_line":"                                       run_as_root\u003dTrue)"},{"line_number":138,"context_line":"        except putils.ProcessExecutionError as e:"}],"source_content_type":"text/x-python","patch_set":6,"id":"fcd0859b_037e4106","line":135,"range":{"start_line":135,"start_character":30,"end_line":135,"end_character":44},"in_reply_to":"c700453e_44584a2c","updated":"2022-01-14 09:38:49.000000000","message":"You are right, this is debugging code that was left in by mistake. Yuval, let\u0027s remove it.","commit_id":"ab1b17b2019101e01892f5ff744388364b0d97b8"},{"author":{"_account_id":33612,"name":"yuval","email":"yuval@lightbitslabs.com","username":"yuval"},"change_message_id":"9dbc0798aac0e74f0c9f16b34331633cb41d6bf9","unresolved":false,"context_lines":[{"line_number":132,"context_line":"    def dsc_disconnect_volume(self, connection_info):"},{"line_number":133,"context_line":"        uuid \u003d connection_info[\u0027uuid\u0027]"},{"line_number":134,"context_line":"        try:"},{"line_number":135,"context_line":"            cmd \u003d shlex.split(\u0027mv -f {} /tmp\u0027.format(self.dsc_file_name(uuid)))"},{"line_number":136,"context_line":"            (out, err) \u003d self._execute(*cmd, root_helper\u003dself._root_helper,"},{"line_number":137,"context_line":"                                       run_as_root\u003dTrue)"},{"line_number":138,"context_line":"        except putils.ProcessExecutionError as e:"}],"source_content_type":"text/x-python","patch_set":6,"id":"7074871d_880e538c","line":135,"in_reply_to":"f9a281aa_ae6f0851","updated":"2022-01-30 09:21:59.000000000","message":"Done","commit_id":"ab1b17b2019101e01892f5ff744388364b0d97b8"},{"author":{"_account_id":33612,"name":"yuval","email":"yuval@lightbitslabs.com","username":"yuval"},"change_message_id":"cfb12f4f70449373ad3b9cf03b5fe93fdc43fb61","unresolved":false,"context_lines":[{"line_number":132,"context_line":"    def dsc_disconnect_volume(self, connection_info):"},{"line_number":133,"context_line":"        uuid \u003d connection_info[\u0027uuid\u0027]"},{"line_number":134,"context_line":"        try:"},{"line_number":135,"context_line":"            cmd \u003d shlex.split(\u0027mv -f {} /tmp\u0027.format(self.dsc_file_name(uuid)))"},{"line_number":136,"context_line":"            (out, err) \u003d self._execute(*cmd, root_helper\u003dself._root_helper,"},{"line_number":137,"context_line":"                                       run_as_root\u003dTrue)"},{"line_number":138,"context_line":"        except putils.ProcessExecutionError as e:"}],"source_content_type":"text/x-python","patch_set":6,"id":"201ac6ac_0858d4e7","line":135,"range":{"start_line":135,"start_character":30,"end_line":135,"end_character":44},"in_reply_to":"fcd0859b_037e4106","updated":"2022-01-18 19:50:15.000000000","message":"Ack","commit_id":"ab1b17b2019101e01892f5ff744388364b0d97b8"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"ebeed2259c751af92af4fce66c5f19d5b367c356","unresolved":true,"context_lines":[{"line_number":191,"context_line":"        file_path \u003d \"/sys/class/block/*/wwid\""},{"line_number":192,"context_line":"        wwid \u003d \"uuid.\" + uuid"},{"line_number":193,"context_line":"        start \u003d time.time()"},{"line_number":194,"context_line":"        while time.time() \u003c start + 60:"},{"line_number":195,"context_line":""},{"line_number":196,"context_line":"            try:"},{"line_number":197,"context_line":"                devname, err \u003d self._execute(\u0027readlink\u0027, \u0027-eq\u0027, lnk_path,"}],"source_content_type":"text/x-python","patch_set":6,"id":"742ec5c3_ec3f871c","line":194,"range":{"start_line":194,"start_character":36,"end_line":194,"end_character":38},"updated":"2022-01-13 17:17:53.000000000","message":"nit: We should define this as a class attribute so it\u0027s no longer a magic number","commit_id":"ab1b17b2019101e01892f5ff744388364b0d97b8"},{"author":{"_account_id":33612,"name":"yuval","email":"yuval@lightbitslabs.com","username":"yuval"},"change_message_id":"cfb12f4f70449373ad3b9cf03b5fe93fdc43fb61","unresolved":false,"context_lines":[{"line_number":191,"context_line":"        file_path \u003d \"/sys/class/block/*/wwid\""},{"line_number":192,"context_line":"        wwid \u003d \"uuid.\" + uuid"},{"line_number":193,"context_line":"        start \u003d time.time()"},{"line_number":194,"context_line":"        while time.time() \u003c start + 60:"},{"line_number":195,"context_line":""},{"line_number":196,"context_line":"            try:"},{"line_number":197,"context_line":"                devname, err \u003d self._execute(\u0027readlink\u0027, \u0027-eq\u0027, lnk_path,"}],"source_content_type":"text/x-python","patch_set":6,"id":"62b2124c_4d16c0da","line":194,"range":{"start_line":194,"start_character":36,"end_line":194,"end_character":38},"in_reply_to":"4c33f882_47069b95","updated":"2022-01-18 19:50:15.000000000","message":"Ack","commit_id":"ab1b17b2019101e01892f5ff744388364b0d97b8"},{"author":{"_account_id":34459,"name":"Muli Ben-Yehuda","email":"muli@lightbitslabs.com","username":"muliby"},"change_message_id":"aee7d724521b4648e88ef40fc4452398e776d08a","unresolved":true,"context_lines":[{"line_number":191,"context_line":"        file_path \u003d \"/sys/class/block/*/wwid\""},{"line_number":192,"context_line":"        wwid \u003d \"uuid.\" + uuid"},{"line_number":193,"context_line":"        start \u003d time.time()"},{"line_number":194,"context_line":"        while time.time() \u003c start + 60:"},{"line_number":195,"context_line":""},{"line_number":196,"context_line":"            try:"},{"line_number":197,"context_line":"                devname, err \u003d self._execute(\u0027readlink\u0027, \u0027-eq\u0027, lnk_path,"}],"source_content_type":"text/x-python","patch_set":6,"id":"4c33f882_47069b95","line":194,"range":{"start_line":194,"start_character":36,"end_line":194,"end_character":38},"in_reply_to":"742ec5c3_ec3f871c","updated":"2022-01-14 09:38:49.000000000","message":"ack","commit_id":"ab1b17b2019101e01892f5ff744388364b0d97b8"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"09f0d98f1a83007f83c26b913106bad2e1021409","unresolved":true,"context_lines":[{"line_number":46,"context_line":"    \"\"\"Connector class to attach/detach LightOS volumes using NVMe/TCP.\"\"\""},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"    WAIT_DEVICE_TIMEOUT \u003d 60"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"    def __init__(self,"},{"line_number":51,"context_line":"                 root_helper,"},{"line_number":52,"context_line":"                 message_queue\u003dNone,"},{"line_number":53,"context_line":"                 device_scan_attempts\u003dDEVICE_SCAN_ATTEMPTS_DEFAULT,"},{"line_number":54,"context_line":"                 driver\u003dNone,"},{"line_number":55,"context_line":"                 execute\u003dNone,"},{"line_number":56,"context_line":"                 *args,"},{"line_number":57,"context_line":"                 **kwargs):"},{"line_number":58,"context_line":"        super(LightOSConnector, self).__init__("}],"source_content_type":"text/x-python","patch_set":10,"id":"3013a584_a0dc3f86","line":55,"range":{"start_line":49,"start_character":0,"end_line":55,"end_character":30},"updated":"2022-01-26 19:36:52.000000000","message":"-1: Incorrect signature. It doesn\u0027t fail because the factory passes everything as keyword arguments, but still, the right signature is:\n\n    def __init__(self, root_helper, driver\u003dNone, execute\u003dNone,\n                 device_scan_attempts\u003dinitiator.DEVICE_SCAN_ATTEMPTS_DEFAULT,\n                 *args, **kwargs):\n\nAs defined in the `InitiatorConnector` class.\n\nIf you don\u0027t want to extract the message_queue parameter from kwargs, you can change it to:\n\n    def __init__(self, root_helper, driver\u003dNone, execute\u003dNone,\n                 device_scan_attempts\u003dinitiator.DEVICE_SCAN_ATTEMPTS_DEFAULT,\n                 message_queue\u003dNone, *args, **kwargs):","commit_id":"bb01f874f568efff44fcc896ff93b1a7811b214d"},{"author":{"_account_id":33612,"name":"yuval","email":"yuval@lightbitslabs.com","username":"yuval"},"change_message_id":"b10473b95a1fe5bb47e926704817bc8510c987c7","unresolved":false,"context_lines":[{"line_number":46,"context_line":"    \"\"\"Connector class to attach/detach LightOS volumes using NVMe/TCP.\"\"\""},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"    WAIT_DEVICE_TIMEOUT \u003d 60"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"    def __init__(self,"},{"line_number":51,"context_line":"                 root_helper,"},{"line_number":52,"context_line":"                 message_queue\u003dNone,"},{"line_number":53,"context_line":"                 device_scan_attempts\u003dDEVICE_SCAN_ATTEMPTS_DEFAULT,"},{"line_number":54,"context_line":"                 driver\u003dNone,"},{"line_number":55,"context_line":"                 execute\u003dNone,"},{"line_number":56,"context_line":"                 *args,"},{"line_number":57,"context_line":"                 **kwargs):"},{"line_number":58,"context_line":"        super(LightOSConnector, self).__init__("}],"source_content_type":"text/x-python","patch_set":10,"id":"48de5eee_695965a8","line":55,"range":{"start_line":49,"start_character":0,"end_line":55,"end_character":30},"in_reply_to":"3013a584_a0dc3f86","updated":"2022-01-27 03:54:13.000000000","message":"Ack","commit_id":"bb01f874f568efff44fcc896ff93b1a7811b214d"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"09f0d98f1a83007f83c26b913106bad2e1021409","unresolved":true,"context_lines":[{"line_number":78,"context_line":"            LOG.debug(\u0027LIGHTOS: did not find dsc, continuing anyway.\u0027)"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"        if hostnqn:"},{"line_number":81,"context_line":"            LOG.info(\"LIGHTOS: finally hostnqn: %s dsc: %s\","},{"line_number":82,"context_line":"                     hostnqn, found_dsc)"},{"line_number":83,"context_line":"            props[\u0027hostnqn\u0027] \u003d hostnqn"},{"line_number":84,"context_line":"            props[\u0027found_dsc\u0027] \u003d found_dsc"}],"source_content_type":"text/x-python","patch_set":10,"id":"bd6bd1f7_abfb1fde","line":81,"range":{"start_line":81,"start_character":16,"end_line":81,"end_character":20},"updated":"2022-01-26 19:36:52.000000000","message":"-1: Debug level, because otherwise this will appear on every connection even when lightos is not being used.","commit_id":"bb01f874f568efff44fcc896ff93b1a7811b214d"},{"author":{"_account_id":33612,"name":"yuval","email":"yuval@lightbitslabs.com","username":"yuval"},"change_message_id":"4a62188710facfce8bc6238e160b80b26ee25458","unresolved":false,"context_lines":[{"line_number":78,"context_line":"            LOG.debug(\u0027LIGHTOS: did not find dsc, continuing anyway.\u0027)"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"        if hostnqn:"},{"line_number":81,"context_line":"            LOG.info(\"LIGHTOS: finally hostnqn: %s dsc: %s\","},{"line_number":82,"context_line":"                     hostnqn, found_dsc)"},{"line_number":83,"context_line":"            props[\u0027hostnqn\u0027] \u003d hostnqn"},{"line_number":84,"context_line":"            props[\u0027found_dsc\u0027] \u003d found_dsc"}],"source_content_type":"text/x-python","patch_set":10,"id":"b1846fd2_174eee4d","line":81,"range":{"start_line":81,"start_character":16,"end_line":81,"end_character":20},"in_reply_to":"bd6bd1f7_abfb1fde","updated":"2022-01-26 20:22:48.000000000","message":"Ack","commit_id":"bb01f874f568efff44fcc896ff93b1a7811b214d"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"09f0d98f1a83007f83c26b913106bad2e1021409","unresolved":true,"context_lines":[{"line_number":83,"context_line":"            props[\u0027hostnqn\u0027] \u003d hostnqn"},{"line_number":84,"context_line":"            props[\u0027found_dsc\u0027] \u003d found_dsc"},{"line_number":85,"context_line":"        else:"},{"line_number":86,"context_line":"            LOG.warning(\u0027LIGHTOS: no hostnqn found.\u0027)"},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"        return props"},{"line_number":89,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"41fa78b4_08df7892","line":86,"updated":"2022-01-26 19:36:52.000000000","message":"-1:  Definitely not a warning. If we have a system that only uses FC storage and is missing nvme tools this log would be generated on every single connection.","commit_id":"bb01f874f568efff44fcc896ff93b1a7811b214d"},{"author":{"_account_id":33612,"name":"yuval","email":"yuval@lightbitslabs.com","username":"yuval"},"change_message_id":"4a62188710facfce8bc6238e160b80b26ee25458","unresolved":false,"context_lines":[{"line_number":83,"context_line":"            props[\u0027hostnqn\u0027] \u003d hostnqn"},{"line_number":84,"context_line":"            props[\u0027found_dsc\u0027] \u003d found_dsc"},{"line_number":85,"context_line":"        else:"},{"line_number":86,"context_line":"            LOG.warning(\u0027LIGHTOS: no hostnqn found.\u0027)"},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"        return props"},{"line_number":89,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"d86326f9_0c6d3044","line":86,"in_reply_to":"41fa78b4_08df7892","updated":"2022-01-26 20:22:48.000000000","message":"Ack","commit_id":"bb01f874f568efff44fcc896ff93b1a7811b214d"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"09f0d98f1a83007f83c26b913106bad2e1021409","unresolved":true,"context_lines":[{"line_number":97,"context_line":"            resp \u003d conn.getresponse()"},{"line_number":98,"context_line":"            return \u0027found\u0027 if resp.status \u003d\u003d http.client.OK else \u0027\u0027"},{"line_number":99,"context_line":"        except Exception as e:"},{"line_number":100,"context_line":"            LOG.debug(e)"},{"line_number":101,"context_line":"            out \u003d \u0027\u0027"},{"line_number":102,"context_line":"        return out"},{"line_number":103,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"85da4c9c_110fe60d","line":100,"updated":"2022-01-26 19:36:52.000000000","message":"nit:\n\n LOG.debug(f\u0027LIGHTOS: {e}\u0027)","commit_id":"bb01f874f568efff44fcc896ff93b1a7811b214d"},{"author":{"_account_id":33612,"name":"yuval","email":"yuval@lightbitslabs.com","username":"yuval"},"change_message_id":"4a62188710facfce8bc6238e160b80b26ee25458","unresolved":false,"context_lines":[{"line_number":97,"context_line":"            resp \u003d conn.getresponse()"},{"line_number":98,"context_line":"            return \u0027found\u0027 if resp.status \u003d\u003d http.client.OK else \u0027\u0027"},{"line_number":99,"context_line":"        except Exception as e:"},{"line_number":100,"context_line":"            LOG.debug(e)"},{"line_number":101,"context_line":"            out \u003d \u0027\u0027"},{"line_number":102,"context_line":"        return out"},{"line_number":103,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"57a8a4ba_6250ac11","line":100,"in_reply_to":"85da4c9c_110fe60d","updated":"2022-01-26 20:22:48.000000000","message":"Ack","commit_id":"bb01f874f568efff44fcc896ff93b1a7811b214d"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"09f0d98f1a83007f83c26b913106bad2e1021409","unresolved":true,"context_lines":[{"line_number":105,"context_line":"        return not os.path.isfile(self.dsc_file_name(connection_info[\u0027uuid\u0027]))"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"    def dsc_connect_volume(self, connection_info):"},{"line_number":108,"context_line":"        if self.dsc_need_connect(connection_info):"},{"line_number":109,"context_line":"            self.dsc_do_connect_volume(connection_info)"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"    def dsc_do_connect_volume(self, connection_info):"},{"line_number":112,"context_line":"        subsysnqn \u003d connection_info[\u0027nqn\u0027]"},{"line_number":113,"context_line":"        uuid \u003d connection_info[\u0027uuid\u0027]"},{"line_number":114,"context_line":"        hostnqn \u003d self.get_hostnqn()"}],"source_content_type":"text/x-python","patch_set":10,"id":"9ccfb052_b9bd13c2","line":111,"range":{"start_line":108,"start_character":0,"end_line":111,"end_character":53},"updated":"2022-01-26 19:36:52.000000000","message":"nit: No need for an additional method, we can replace L108-L111 with:\n\n        if not self.dsc_need_connect(connection_info):\n            return\n\nAnd the rest will remain the same.","commit_id":"bb01f874f568efff44fcc896ff93b1a7811b214d"},{"author":{"_account_id":33612,"name":"yuval","email":"yuval@lightbitslabs.com","username":"yuval"},"change_message_id":"b10473b95a1fe5bb47e926704817bc8510c987c7","unresolved":false,"context_lines":[{"line_number":105,"context_line":"        return not os.path.isfile(self.dsc_file_name(connection_info[\u0027uuid\u0027]))"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"    def dsc_connect_volume(self, connection_info):"},{"line_number":108,"context_line":"        if self.dsc_need_connect(connection_info):"},{"line_number":109,"context_line":"            self.dsc_do_connect_volume(connection_info)"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"    def dsc_do_connect_volume(self, connection_info):"},{"line_number":112,"context_line":"        subsysnqn \u003d connection_info[\u0027nqn\u0027]"},{"line_number":113,"context_line":"        uuid \u003d connection_info[\u0027uuid\u0027]"},{"line_number":114,"context_line":"        hostnqn \u003d self.get_hostnqn()"}],"source_content_type":"text/x-python","patch_set":10,"id":"08b4836a_6b74f33c","line":111,"range":{"start_line":108,"start_character":0,"end_line":111,"end_character":53},"in_reply_to":"9ccfb052_b9bd13c2","updated":"2022-01-27 03:54:13.000000000","message":"Ack","commit_id":"bb01f874f568efff44fcc896ff93b1a7811b214d"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"09f0d98f1a83007f83c26b913106bad2e1021409","unresolved":true,"context_lines":[{"line_number":134,"context_line":"    def dsc_disconnect_volume(self, connection_info):"},{"line_number":135,"context_line":"        uuid \u003d connection_info[\u0027uuid\u0027]"},{"line_number":136,"context_line":"        try:"},{"line_number":137,"context_line":"            cmd \u003d shlex.split(\u0027rm -f {}\u0027.format(self.dsc_file_name(uuid)))"},{"line_number":138,"context_line":"            (out, err) \u003d self._execute(*cmd, root_helper\u003dself._root_helper,"},{"line_number":139,"context_line":"                                       run_as_root\u003dTrue)"},{"line_number":140,"context_line":"        except putils.ProcessExecutionError as e:"},{"line_number":141,"context_line":"            LOG.warning(\"LIGHTOS: Failed to run command {}\".format(cmd))"},{"line_number":142,"context_line":"            raise e"}],"source_content_type":"text/x-python","patch_set":10,"id":"108ba6fa_289dcfdb","line":139,"range":{"start_line":137,"start_character":0,"end_line":139,"end_character":56},"updated":"2022-01-26 19:36:52.000000000","message":"nit: preferably a privsep method","commit_id":"bb01f874f568efff44fcc896ff93b1a7811b214d"},{"author":{"_account_id":33612,"name":"yuval","email":"yuval@lightbitslabs.com","username":"yuval"},"change_message_id":"b10473b95a1fe5bb47e926704817bc8510c987c7","unresolved":false,"context_lines":[{"line_number":134,"context_line":"    def dsc_disconnect_volume(self, connection_info):"},{"line_number":135,"context_line":"        uuid \u003d connection_info[\u0027uuid\u0027]"},{"line_number":136,"context_line":"        try:"},{"line_number":137,"context_line":"            cmd \u003d shlex.split(\u0027rm -f {}\u0027.format(self.dsc_file_name(uuid)))"},{"line_number":138,"context_line":"            (out, err) \u003d self._execute(*cmd, root_helper\u003dself._root_helper,"},{"line_number":139,"context_line":"                                       run_as_root\u003dTrue)"},{"line_number":140,"context_line":"        except putils.ProcessExecutionError as e:"},{"line_number":141,"context_line":"            LOG.warning(\"LIGHTOS: Failed to run command {}\".format(cmd))"},{"line_number":142,"context_line":"            raise e"}],"source_content_type":"text/x-python","patch_set":10,"id":"bec8d451_5abdfc19","line":139,"range":{"start_line":137,"start_character":0,"end_line":139,"end_character":56},"in_reply_to":"108ba6fa_289dcfdb","updated":"2022-01-27 03:54:13.000000000","message":"Ack","commit_id":"bb01f874f568efff44fcc896ff93b1a7811b214d"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"09f0d98f1a83007f83c26b913106bad2e1021409","unresolved":true,"context_lines":[{"line_number":141,"context_line":"            LOG.warning(\"LIGHTOS: Failed to run command {}\".format(cmd))"},{"line_number":142,"context_line":"            raise e"},{"line_number":143,"context_line":""},{"line_number":144,"context_line":"    def monitor_db(self, lightos_db):"},{"line_number":145,"context_line":"        for connection_info in lightos_db.values():"},{"line_number":146,"context_line":"            self.dsc_connect_volume(connection_info)"},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"    def monitor_message_queue(self, message_queue, lightos_db):"},{"line_number":149,"context_line":"        while not message_queue.empty():"},{"line_number":150,"context_line":"            msg \u003d message_queue.get()"},{"line_number":151,"context_line":"            op, connection \u003d msg"},{"line_number":152,"context_line":"            LOG.debug(\"LIGHTOS: queue got op: %s, connection: %s\","},{"line_number":153,"context_line":"                      op, connection)"},{"line_number":154,"context_line":"            if op \u003d\u003d \u0027delete\u0027:"},{"line_number":155,"context_line":"                LOG.info(\"LIGHTOS: Removing volume: %s from db\","},{"line_number":156,"context_line":"                         connection[\u0027uuid\u0027])"},{"line_number":157,"context_line":"                if connection[\u0027uuid\u0027] in lightos_db:"},{"line_number":158,"context_line":"                    del lightos_db[connection[\u0027uuid\u0027]]"},{"line_number":159,"context_line":"                else:"},{"line_number":160,"context_line":"                    LOG.warning(\"LIGHTOS: No volume: %s found in db\","},{"line_number":161,"context_line":"                                connection[\u0027uuid\u0027])"},{"line_number":162,"context_line":"            elif op \u003d\u003d \u0027add\u0027:"},{"line_number":163,"context_line":"                LOG.info(\"LIGHTOS: Adding volume: %s to db\","},{"line_number":164,"context_line":"                         connection[\u0027uuid\u0027])"},{"line_number":165,"context_line":"                lightos_db[connection[\u0027uuid\u0027]] \u003d connection"},{"line_number":166,"context_line":""},{"line_number":167,"context_line":"    def lightos_monitor(self, lightos_db, message_queue):"},{"line_number":168,"context_line":"        first_time \u003d True"},{"line_number":169,"context_line":"        while True:"},{"line_number":170,"context_line":"            self.monitor_db(lightos_db)"},{"line_number":171,"context_line":"            # give us some time before trying to access the MQ"},{"line_number":172,"context_line":"            # for the first time"},{"line_number":173,"context_line":"            if first_time:"},{"line_number":174,"context_line":"                time.sleep(5)"},{"line_number":175,"context_line":"                first_time \u003d False"},{"line_number":176,"context_line":"            else:"},{"line_number":177,"context_line":"                time.sleep(1)"},{"line_number":178,"context_line":""},{"line_number":179,"context_line":"            self.monitor_message_queue(message_queue, lightos_db)"},{"line_number":180,"context_line":""},{"line_number":181,"context_line":"    # This is part of our abstract interface"},{"line_number":182,"context_line":"    def get_search_path(self):"}],"source_content_type":"text/x-python","patch_set":10,"id":"0f7c53ab_3d5e5b8e","line":179,"range":{"start_line":144,"start_character":0,"end_line":179,"end_character":65},"updated":"2022-01-26 19:36:52.000000000","message":"-1: This is dead code, because it is not being used by the connector.","commit_id":"bb01f874f568efff44fcc896ff93b1a7811b214d"},{"author":{"_account_id":33612,"name":"yuval","email":"yuval@lightbitslabs.com","username":"yuval"},"change_message_id":"b10473b95a1fe5bb47e926704817bc8510c987c7","unresolved":true,"context_lines":[{"line_number":141,"context_line":"            LOG.warning(\"LIGHTOS: Failed to run command {}\".format(cmd))"},{"line_number":142,"context_line":"            raise e"},{"line_number":143,"context_line":""},{"line_number":144,"context_line":"    def monitor_db(self, lightos_db):"},{"line_number":145,"context_line":"        for connection_info in lightos_db.values():"},{"line_number":146,"context_line":"            self.dsc_connect_volume(connection_info)"},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"    def monitor_message_queue(self, message_queue, lightos_db):"},{"line_number":149,"context_line":"        while not message_queue.empty():"},{"line_number":150,"context_line":"            msg \u003d message_queue.get()"},{"line_number":151,"context_line":"            op, connection \u003d msg"},{"line_number":152,"context_line":"            LOG.debug(\"LIGHTOS: queue got op: %s, connection: %s\","},{"line_number":153,"context_line":"                      op, connection)"},{"line_number":154,"context_line":"            if op \u003d\u003d \u0027delete\u0027:"},{"line_number":155,"context_line":"                LOG.info(\"LIGHTOS: Removing volume: %s from db\","},{"line_number":156,"context_line":"                         connection[\u0027uuid\u0027])"},{"line_number":157,"context_line":"                if connection[\u0027uuid\u0027] in lightos_db:"},{"line_number":158,"context_line":"                    del lightos_db[connection[\u0027uuid\u0027]]"},{"line_number":159,"context_line":"                else:"},{"line_number":160,"context_line":"                    LOG.warning(\"LIGHTOS: No volume: %s found in db\","},{"line_number":161,"context_line":"                                connection[\u0027uuid\u0027])"},{"line_number":162,"context_line":"            elif op \u003d\u003d \u0027add\u0027:"},{"line_number":163,"context_line":"                LOG.info(\"LIGHTOS: Adding volume: %s to db\","},{"line_number":164,"context_line":"                         connection[\u0027uuid\u0027])"},{"line_number":165,"context_line":"                lightos_db[connection[\u0027uuid\u0027]] \u003d connection"},{"line_number":166,"context_line":""},{"line_number":167,"context_line":"    def lightos_monitor(self, lightos_db, message_queue):"},{"line_number":168,"context_line":"        first_time \u003d True"},{"line_number":169,"context_line":"        while True:"},{"line_number":170,"context_line":"            self.monitor_db(lightos_db)"},{"line_number":171,"context_line":"            # give us some time before trying to access the MQ"},{"line_number":172,"context_line":"            # for the first time"},{"line_number":173,"context_line":"            if first_time:"},{"line_number":174,"context_line":"                time.sleep(5)"},{"line_number":175,"context_line":"                first_time \u003d False"},{"line_number":176,"context_line":"            else:"},{"line_number":177,"context_line":"                time.sleep(1)"},{"line_number":178,"context_line":""},{"line_number":179,"context_line":"            self.monitor_message_queue(message_queue, lightos_db)"},{"line_number":180,"context_line":""},{"line_number":181,"context_line":"    # This is part of our abstract interface"},{"line_number":182,"context_line":"    def get_search_path(self):"}],"source_content_type":"text/x-python","patch_set":10,"id":"8e4af98f_b1362ff6","line":179,"range":{"start_line":144,"start_character":0,"end_line":179,"end_character":65},"in_reply_to":"0f7c53ab_3d5e5b8e","updated":"2022-01-27 03:54:13.000000000","message":"this code is been used in the nova driver:\nhttps://review.opendev.org/c/openstack/nova/+/821606/3/nova/virt/libvirt/volume/lightos.py#59","commit_id":"bb01f874f568efff44fcc896ff93b1a7811b214d"},{"author":{"_account_id":33612,"name":"yuval","email":"yuval@lightbitslabs.com","username":"yuval"},"change_message_id":"9dbc0798aac0e74f0c9f16b34331633cb41d6bf9","unresolved":false,"context_lines":[{"line_number":141,"context_line":"            LOG.warning(\"LIGHTOS: Failed to run command {}\".format(cmd))"},{"line_number":142,"context_line":"            raise e"},{"line_number":143,"context_line":""},{"line_number":144,"context_line":"    def monitor_db(self, lightos_db):"},{"line_number":145,"context_line":"        for connection_info in lightos_db.values():"},{"line_number":146,"context_line":"            self.dsc_connect_volume(connection_info)"},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"    def monitor_message_queue(self, message_queue, lightos_db):"},{"line_number":149,"context_line":"        while not message_queue.empty():"},{"line_number":150,"context_line":"            msg \u003d message_queue.get()"},{"line_number":151,"context_line":"            op, connection \u003d msg"},{"line_number":152,"context_line":"            LOG.debug(\"LIGHTOS: queue got op: %s, connection: %s\","},{"line_number":153,"context_line":"                      op, connection)"},{"line_number":154,"context_line":"            if op \u003d\u003d \u0027delete\u0027:"},{"line_number":155,"context_line":"                LOG.info(\"LIGHTOS: Removing volume: %s from db\","},{"line_number":156,"context_line":"                         connection[\u0027uuid\u0027])"},{"line_number":157,"context_line":"                if connection[\u0027uuid\u0027] in lightos_db:"},{"line_number":158,"context_line":"                    del lightos_db[connection[\u0027uuid\u0027]]"},{"line_number":159,"context_line":"                else:"},{"line_number":160,"context_line":"                    LOG.warning(\"LIGHTOS: No volume: %s found in db\","},{"line_number":161,"context_line":"                                connection[\u0027uuid\u0027])"},{"line_number":162,"context_line":"            elif op \u003d\u003d \u0027add\u0027:"},{"line_number":163,"context_line":"                LOG.info(\"LIGHTOS: Adding volume: %s to db\","},{"line_number":164,"context_line":"                         connection[\u0027uuid\u0027])"},{"line_number":165,"context_line":"                lightos_db[connection[\u0027uuid\u0027]] \u003d connection"},{"line_number":166,"context_line":""},{"line_number":167,"context_line":"    def lightos_monitor(self, lightos_db, message_queue):"},{"line_number":168,"context_line":"        first_time \u003d True"},{"line_number":169,"context_line":"        while True:"},{"line_number":170,"context_line":"            self.monitor_db(lightos_db)"},{"line_number":171,"context_line":"            # give us some time before trying to access the MQ"},{"line_number":172,"context_line":"            # for the first time"},{"line_number":173,"context_line":"            if first_time:"},{"line_number":174,"context_line":"                time.sleep(5)"},{"line_number":175,"context_line":"                first_time \u003d False"},{"line_number":176,"context_line":"            else:"},{"line_number":177,"context_line":"                time.sleep(1)"},{"line_number":178,"context_line":""},{"line_number":179,"context_line":"            self.monitor_message_queue(message_queue, lightos_db)"},{"line_number":180,"context_line":""},{"line_number":181,"context_line":"    # This is part of our abstract interface"},{"line_number":182,"context_line":"    def get_search_path(self):"}],"source_content_type":"text/x-python","patch_set":10,"id":"7bd7612e_dada9b98","line":179,"range":{"start_line":144,"start_character":0,"end_line":179,"end_character":65},"in_reply_to":"8e4af98f_b1362ff6","updated":"2022-01-30 09:21:59.000000000","message":"Done","commit_id":"bb01f874f568efff44fcc896ff93b1a7811b214d"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"09f0d98f1a83007f83c26b913106bad2e1021409","unresolved":true,"context_lines":[{"line_number":178,"context_line":""},{"line_number":179,"context_line":"            self.monitor_message_queue(message_queue, lightos_db)"},{"line_number":180,"context_line":""},{"line_number":181,"context_line":"    # This is part of our abstract interface"},{"line_number":182,"context_line":"    def get_search_path(self):"},{"line_number":183,"context_line":"        return \u0027/dev\u0027"},{"line_number":184,"context_line":""},{"line_number":185,"context_line":"    # This is part of our abstract interface"},{"line_number":186,"context_line":"    def get_volume_paths(self, connection_properties):"},{"line_number":187,"context_line":"        path \u003d connection_properties[\u0027device_path\u0027]"},{"line_number":188,"context_line":"        return [path]"},{"line_number":189,"context_line":""},{"line_number":190,"context_line":"    @utils.trace"},{"line_number":191,"context_line":"    def _get_device_by_uuid(self, uuid):"}],"source_content_type":"text/x-python","patch_set":10,"id":"5c3313f1_7efc0aac","line":188,"range":{"start_line":181,"start_character":0,"end_line":188,"end_character":21},"updated":"2022-01-26 19:36:52.000000000","message":"-1: What does that mean \"abstract interface\"?  Because in this connector this is currently dead code (from the OpenStack perspective)","commit_id":"bb01f874f568efff44fcc896ff93b1a7811b214d"},{"author":{"_account_id":33612,"name":"yuval","email":"yuval@lightbitslabs.com","username":"yuval"},"change_message_id":"95c95e50c2b664d0a27f0fa3fa94d768b4ef6d65","unresolved":false,"context_lines":[{"line_number":178,"context_line":""},{"line_number":179,"context_line":"            self.monitor_message_queue(message_queue, lightos_db)"},{"line_number":180,"context_line":""},{"line_number":181,"context_line":"    # This is part of our abstract interface"},{"line_number":182,"context_line":"    def get_search_path(self):"},{"line_number":183,"context_line":"        return \u0027/dev\u0027"},{"line_number":184,"context_line":""},{"line_number":185,"context_line":"    # This is part of our abstract interface"},{"line_number":186,"context_line":"    def get_volume_paths(self, connection_properties):"},{"line_number":187,"context_line":"        path \u003d connection_properties[\u0027device_path\u0027]"},{"line_number":188,"context_line":"        return [path]"},{"line_number":189,"context_line":""},{"line_number":190,"context_line":"    @utils.trace"},{"line_number":191,"context_line":"    def _get_device_by_uuid(self, uuid):"}],"source_content_type":"text/x-python","patch_set":10,"id":"6c212c8d_9dab556f","line":188,"range":{"start_line":181,"start_character":0,"end_line":188,"end_character":21},"in_reply_to":"1b88c0f1_c882d01b","updated":"2022-01-28 09:13:32.000000000","message":"I need to check, I am sure this code is been used in the UT. I will see how we can transfer it in a follow up patch","commit_id":"bb01f874f568efff44fcc896ff93b1a7811b214d"},{"author":{"_account_id":33612,"name":"yuval","email":"yuval@lightbitslabs.com","username":"yuval"},"change_message_id":"b10473b95a1fe5bb47e926704817bc8510c987c7","unresolved":false,"context_lines":[{"line_number":178,"context_line":""},{"line_number":179,"context_line":"            self.monitor_message_queue(message_queue, lightos_db)"},{"line_number":180,"context_line":""},{"line_number":181,"context_line":"    # This is part of our abstract interface"},{"line_number":182,"context_line":"    def get_search_path(self):"},{"line_number":183,"context_line":"        return \u0027/dev\u0027"},{"line_number":184,"context_line":""},{"line_number":185,"context_line":"    # This is part of our abstract interface"},{"line_number":186,"context_line":"    def get_volume_paths(self, connection_properties):"},{"line_number":187,"context_line":"        path \u003d connection_properties[\u0027device_path\u0027]"},{"line_number":188,"context_line":"        return [path]"},{"line_number":189,"context_line":""},{"line_number":190,"context_line":"    @utils.trace"},{"line_number":191,"context_line":"    def _get_device_by_uuid(self, uuid):"}],"source_content_type":"text/x-python","patch_set":10,"id":"1b88c0f1_c882d01b","line":188,"range":{"start_line":181,"start_character":0,"end_line":188,"end_character":21},"in_reply_to":"5c3313f1_7efc0aac","updated":"2022-01-27 03:54:13.000000000","message":"Ack","commit_id":"bb01f874f568efff44fcc896ff93b1a7811b214d"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"09f0d98f1a83007f83c26b913106bad2e1021409","unresolved":true,"context_lines":[{"line_number":189,"context_line":""},{"line_number":190,"context_line":"    @utils.trace"},{"line_number":191,"context_line":"    def _get_device_by_uuid(self, uuid):"},{"line_number":192,"context_line":"        lnk_path \u003d \"%s%s\" % (\"/dev/disk/by-id/nvme-uuid.\", uuid)"},{"line_number":193,"context_line":"        file_path \u003d \"/sys/class/block/*/wwid\""},{"line_number":194,"context_line":"        wwid \u003d \"uuid.\" + uuid"},{"line_number":195,"context_line":"        start \u003d time.time()"}],"source_content_type":"text/x-python","patch_set":10,"id":"3b10d062_a8d16965","line":192,"range":{"start_line":192,"start_character":0,"end_line":192,"end_character":64},"updated":"2022-01-26 19:36:52.000000000","message":"nit:\n\n        lnk_path \u003d f\"/dev/disk/by-id/nvme-uuid.{uuid}\"","commit_id":"bb01f874f568efff44fcc896ff93b1a7811b214d"},{"author":{"_account_id":33612,"name":"yuval","email":"yuval@lightbitslabs.com","username":"yuval"},"change_message_id":"b10473b95a1fe5bb47e926704817bc8510c987c7","unresolved":false,"context_lines":[{"line_number":189,"context_line":""},{"line_number":190,"context_line":"    @utils.trace"},{"line_number":191,"context_line":"    def _get_device_by_uuid(self, uuid):"},{"line_number":192,"context_line":"        lnk_path \u003d \"%s%s\" % (\"/dev/disk/by-id/nvme-uuid.\", uuid)"},{"line_number":193,"context_line":"        file_path \u003d \"/sys/class/block/*/wwid\""},{"line_number":194,"context_line":"        wwid \u003d \"uuid.\" + uuid"},{"line_number":195,"context_line":"        start \u003d time.time()"}],"source_content_type":"text/x-python","patch_set":10,"id":"e17a5a0d_8f7f6cea","line":192,"range":{"start_line":192,"start_character":0,"end_line":192,"end_character":64},"in_reply_to":"3b10d062_a8d16965","updated":"2022-01-27 03:54:13.000000000","message":"Ack","commit_id":"bb01f874f568efff44fcc896ff93b1a7811b214d"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"09f0d98f1a83007f83c26b913106bad2e1021409","unresolved":true,"context_lines":[{"line_number":190,"context_line":"    @utils.trace"},{"line_number":191,"context_line":"    def _get_device_by_uuid(self, uuid):"},{"line_number":192,"context_line":"        lnk_path \u003d \"%s%s\" % (\"/dev/disk/by-id/nvme-uuid.\", uuid)"},{"line_number":193,"context_line":"        file_path \u003d \"/sys/class/block/*/wwid\""},{"line_number":194,"context_line":"        wwid \u003d \"uuid.\" + uuid"},{"line_number":195,"context_line":"        start \u003d time.time()"},{"line_number":196,"context_line":"        while time.time() \u003c start + self.WAIT_DEVICE_TIMEOUT:"}],"source_content_type":"text/x-python","patch_set":10,"id":"3db3e182_6f44c838","line":193,"updated":"2022-01-26 19:36:52.000000000","message":"nit: move this declaration to right where it is used before L210","commit_id":"bb01f874f568efff44fcc896ff93b1a7811b214d"},{"author":{"_account_id":33612,"name":"yuval","email":"yuval@lightbitslabs.com","username":"yuval"},"change_message_id":"b10473b95a1fe5bb47e926704817bc8510c987c7","unresolved":false,"context_lines":[{"line_number":190,"context_line":"    @utils.trace"},{"line_number":191,"context_line":"    def _get_device_by_uuid(self, uuid):"},{"line_number":192,"context_line":"        lnk_path \u003d \"%s%s\" % (\"/dev/disk/by-id/nvme-uuid.\", uuid)"},{"line_number":193,"context_line":"        file_path \u003d \"/sys/class/block/*/wwid\""},{"line_number":194,"context_line":"        wwid \u003d \"uuid.\" + uuid"},{"line_number":195,"context_line":"        start \u003d time.time()"},{"line_number":196,"context_line":"        while time.time() \u003c start + self.WAIT_DEVICE_TIMEOUT:"}],"source_content_type":"text/x-python","patch_set":10,"id":"9dd73ace_b3f76fdc","line":193,"in_reply_to":"3db3e182_6f44c838","updated":"2022-01-27 03:54:13.000000000","message":"Ack","commit_id":"bb01f874f568efff44fcc896ff93b1a7811b214d"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"09f0d98f1a83007f83c26b913106bad2e1021409","unresolved":true,"context_lines":[{"line_number":192,"context_line":"        lnk_path \u003d \"%s%s\" % (\"/dev/disk/by-id/nvme-uuid.\", uuid)"},{"line_number":193,"context_line":"        file_path \u003d \"/sys/class/block/*/wwid\""},{"line_number":194,"context_line":"        wwid \u003d \"uuid.\" + uuid"},{"line_number":195,"context_line":"        start \u003d time.time()"},{"line_number":196,"context_line":"        while time.time() \u003c start + self.WAIT_DEVICE_TIMEOUT:"},{"line_number":197,"context_line":""},{"line_number":198,"context_line":"            try:"},{"line_number":199,"context_line":"                devname, err \u003d self._execute(\u0027readlink\u0027, \u0027-eq\u0027, lnk_path,"}],"source_content_type":"text/x-python","patch_set":10,"id":"9252a57e_ff043f54","line":196,"range":{"start_line":195,"start_character":0,"end_line":196,"end_character":61},"updated":"2022-01-26 19:36:52.000000000","message":"nit:\n\n        endtime \u003d time.time() + self.WAIT_DEVICE_TIMEOUT\n        while time.time() \u003c endtime:","commit_id":"bb01f874f568efff44fcc896ff93b1a7811b214d"},{"author":{"_account_id":33612,"name":"yuval","email":"yuval@lightbitslabs.com","username":"yuval"},"change_message_id":"4a62188710facfce8bc6238e160b80b26ee25458","unresolved":false,"context_lines":[{"line_number":192,"context_line":"        lnk_path \u003d \"%s%s\" % (\"/dev/disk/by-id/nvme-uuid.\", uuid)"},{"line_number":193,"context_line":"        file_path \u003d \"/sys/class/block/*/wwid\""},{"line_number":194,"context_line":"        wwid \u003d \"uuid.\" + uuid"},{"line_number":195,"context_line":"        start \u003d time.time()"},{"line_number":196,"context_line":"        while time.time() \u003c start + self.WAIT_DEVICE_TIMEOUT:"},{"line_number":197,"context_line":""},{"line_number":198,"context_line":"            try:"},{"line_number":199,"context_line":"                devname, err \u003d self._execute(\u0027readlink\u0027, \u0027-eq\u0027, lnk_path,"}],"source_content_type":"text/x-python","patch_set":10,"id":"2b2d115f_b4fda9e0","line":196,"range":{"start_line":195,"start_character":0,"end_line":196,"end_character":61},"in_reply_to":"9252a57e_ff043f54","updated":"2022-01-26 20:22:48.000000000","message":"Ack","commit_id":"bb01f874f568efff44fcc896ff93b1a7811b214d"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"09f0d98f1a83007f83c26b913106bad2e1021409","unresolved":true,"context_lines":[{"line_number":197,"context_line":""},{"line_number":198,"context_line":"            try:"},{"line_number":199,"context_line":"                devname, err \u003d self._execute(\u0027readlink\u0027, \u0027-eq\u0027, lnk_path,"},{"line_number":200,"context_line":"                                             run_as_root\u003dTrue,"},{"line_number":201,"context_line":"                                             root_helper\u003dself._root_helper)"},{"line_number":202,"context_line":"                if devname.startswith(\"/dev/nvme\"):"},{"line_number":203,"context_line":"                    devname \u003d devname.strip()"}],"source_content_type":"text/x-python","patch_set":10,"id":"fe6d0aae_37742ed3","line":200,"range":{"start_line":200,"start_character":45,"end_line":200,"end_character":62},"updated":"2022-01-26 19:36:52.000000000","message":"-1: As far as I know this doesn\u0027t need to be run as root.","commit_id":"bb01f874f568efff44fcc896ff93b1a7811b214d"},{"author":{"_account_id":33612,"name":"yuval","email":"yuval@lightbitslabs.com","username":"yuval"},"change_message_id":"b10473b95a1fe5bb47e926704817bc8510c987c7","unresolved":false,"context_lines":[{"line_number":197,"context_line":""},{"line_number":198,"context_line":"            try:"},{"line_number":199,"context_line":"                devname, err \u003d self._execute(\u0027readlink\u0027, \u0027-eq\u0027, lnk_path,"},{"line_number":200,"context_line":"                                             run_as_root\u003dTrue,"},{"line_number":201,"context_line":"                                             root_helper\u003dself._root_helper)"},{"line_number":202,"context_line":"                if devname.startswith(\"/dev/nvme\"):"},{"line_number":203,"context_line":"                    devname \u003d devname.strip()"}],"source_content_type":"text/x-python","patch_set":10,"id":"65127d76_78ff526f","line":200,"range":{"start_line":200,"start_character":45,"end_line":200,"end_character":62},"in_reply_to":"fe6d0aae_37742ed3","updated":"2022-01-27 03:54:13.000000000","message":"Ack","commit_id":"bb01f874f568efff44fcc896ff93b1a7811b214d"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"09f0d98f1a83007f83c26b913106bad2e1021409","unresolved":true,"context_lines":[{"line_number":209,"context_line":""},{"line_number":210,"context_line":"            for match_path in glob.glob(file_path):"},{"line_number":211,"context_line":"                try:"},{"line_number":212,"context_line":"                    match_wwid, err \u003d self._execute("},{"line_number":213,"context_line":"                        \u0027cat\u0027,"},{"line_number":214,"context_line":"                        match_path,"},{"line_number":215,"context_line":"                        run_as_root\u003dTrue,"},{"line_number":216,"context_line":"                        root_helper\u003dself._root_helper)"},{"line_number":217,"context_line":"                except putils.ProcessExecutionError:"},{"line_number":218,"context_line":"                    LOG.warning(\"LIGHTOS: Could not find the uuid at %s\","},{"line_number":219,"context_line":"                                match_path)"}],"source_content_type":"text/x-python","patch_set":10,"id":"5fe25ac4_d34160d3","line":216,"range":{"start_line":212,"start_character":0,"end_line":216,"end_character":54},"updated":"2022-01-26 19:36:52.000000000","message":"-1: This doesn\u0027t need to be run as root, the files can be opened with python directly.","commit_id":"bb01f874f568efff44fcc896ff93b1a7811b214d"},{"author":{"_account_id":33612,"name":"yuval","email":"yuval@lightbitslabs.com","username":"yuval"},"change_message_id":"b10473b95a1fe5bb47e926704817bc8510c987c7","unresolved":false,"context_lines":[{"line_number":209,"context_line":""},{"line_number":210,"context_line":"            for match_path in glob.glob(file_path):"},{"line_number":211,"context_line":"                try:"},{"line_number":212,"context_line":"                    match_wwid, err \u003d self._execute("},{"line_number":213,"context_line":"                        \u0027cat\u0027,"},{"line_number":214,"context_line":"                        match_path,"},{"line_number":215,"context_line":"                        run_as_root\u003dTrue,"},{"line_number":216,"context_line":"                        root_helper\u003dself._root_helper)"},{"line_number":217,"context_line":"                except putils.ProcessExecutionError:"},{"line_number":218,"context_line":"                    LOG.warning(\"LIGHTOS: Could not find the uuid at %s\","},{"line_number":219,"context_line":"                                match_path)"}],"source_content_type":"text/x-python","patch_set":10,"id":"22059e71_d2782271","line":216,"range":{"start_line":212,"start_character":0,"end_line":216,"end_character":54},"in_reply_to":"5fe25ac4_d34160d3","updated":"2022-01-27 03:54:13.000000000","message":"Ack","commit_id":"bb01f874f568efff44fcc896ff93b1a7811b214d"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"09f0d98f1a83007f83c26b913106bad2e1021409","unresolved":true,"context_lines":[{"line_number":215,"context_line":"                        run_as_root\u003dTrue,"},{"line_number":216,"context_line":"                        root_helper\u003dself._root_helper)"},{"line_number":217,"context_line":"                except putils.ProcessExecutionError:"},{"line_number":218,"context_line":"                    LOG.warning(\"LIGHTOS: Could not find the uuid at %s\","},{"line_number":219,"context_line":"                                match_path)"},{"line_number":220,"context_line":"                    continue"},{"line_number":221,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"6250517c_5a596f55","line":218,"updated":"2022-01-26 19:36:52.000000000","message":"?: Shouldn\u0027t this message be saying that it could not open/read the file instead?","commit_id":"bb01f874f568efff44fcc896ff93b1a7811b214d"},{"author":{"_account_id":33612,"name":"yuval","email":"yuval@lightbitslabs.com","username":"yuval"},"change_message_id":"b10473b95a1fe5bb47e926704817bc8510c987c7","unresolved":false,"context_lines":[{"line_number":215,"context_line":"                        run_as_root\u003dTrue,"},{"line_number":216,"context_line":"                        root_helper\u003dself._root_helper)"},{"line_number":217,"context_line":"                except putils.ProcessExecutionError:"},{"line_number":218,"context_line":"                    LOG.warning(\"LIGHTOS: Could not find the uuid at %s\","},{"line_number":219,"context_line":"                                match_path)"},{"line_number":220,"context_line":"                    continue"},{"line_number":221,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"a7b24a5b_17986d3e","line":218,"in_reply_to":"6250517c_5a596f55","updated":"2022-01-27 03:54:13.000000000","message":"Ack","commit_id":"bb01f874f568efff44fcc896ff93b1a7811b214d"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"09f0d98f1a83007f83c26b913106bad2e1021409","unresolved":true,"context_lines":[{"line_number":227,"context_line":"                    continue"},{"line_number":228,"context_line":""},{"line_number":229,"context_line":"                LOG.info(\"LIGHTOS: matching uuid %s was found\""},{"line_number":230,"context_line":"                         \" for device path %s\" % (uuid, match_path))"},{"line_number":231,"context_line":"                return os.path.join(\"/dev\", match_path.split(\"/\")[-2])"},{"line_number":232,"context_line":""},{"line_number":233,"context_line":"            time.sleep(1)"}],"source_content_type":"text/x-python","patch_set":10,"id":"73ef298e_45c513d9","line":230,"updated":"2022-01-26 19:36:52.000000000","message":"?: Do we have a different string than in L204 to differentiate which code path succeeded?","commit_id":"bb01f874f568efff44fcc896ff93b1a7811b214d"},{"author":{"_account_id":33612,"name":"yuval","email":"yuval@lightbitslabs.com","username":"yuval"},"change_message_id":"b10473b95a1fe5bb47e926704817bc8510c987c7","unresolved":false,"context_lines":[{"line_number":227,"context_line":"                    continue"},{"line_number":228,"context_line":""},{"line_number":229,"context_line":"                LOG.info(\"LIGHTOS: matching uuid %s was found\""},{"line_number":230,"context_line":"                         \" for device path %s\" % (uuid, match_path))"},{"line_number":231,"context_line":"                return os.path.join(\"/dev\", match_path.split(\"/\")[-2])"},{"line_number":232,"context_line":""},{"line_number":233,"context_line":"            time.sleep(1)"}],"source_content_type":"text/x-python","patch_set":10,"id":"30a8d561_384b7af0","line":230,"in_reply_to":"73ef298e_45c513d9","updated":"2022-01-27 03:54:13.000000000","message":"yes","commit_id":"bb01f874f568efff44fcc896ff93b1a7811b214d"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"09f0d98f1a83007f83c26b913106bad2e1021409","unresolved":true,"context_lines":[{"line_number":238,"context_line":"        devname \u003d devpath.split(\"/\")[-1]"},{"line_number":239,"context_line":"        try:"},{"line_number":240,"context_line":"            size_path_name \u003d os.path.join(\"/sys/class/block/\", devname, \"size\")"},{"line_number":241,"context_line":"            size_blks, err \u003d self._execute(\u0027cat\u0027, size_path_name,"},{"line_number":242,"context_line":"                                           run_as_root\u003dTrue,"},{"line_number":243,"context_line":"                                           root_helper\u003dself._root_helper)"},{"line_number":244,"context_line":"            bytesize \u003d int(size_blks) * 512"},{"line_number":245,"context_line":"            return bytesize"},{"line_number":246,"context_line":"        except putils.ProcessExecutionError:"},{"line_number":247,"context_line":"            LOG.warning(\"LIGHTOS: Could not find the size at for\""}],"source_content_type":"text/x-python","patch_set":10,"id":"63caa0a5_35abff8d","line":244,"range":{"start_line":241,"start_character":0,"end_line":244,"end_character":0},"updated":"2022-01-26 19:36:52.000000000","message":"-1: No need to run as root, this can be read directly from Python code.","commit_id":"bb01f874f568efff44fcc896ff93b1a7811b214d"},{"author":{"_account_id":33612,"name":"yuval","email":"yuval@lightbitslabs.com","username":"yuval"},"change_message_id":"b10473b95a1fe5bb47e926704817bc8510c987c7","unresolved":false,"context_lines":[{"line_number":238,"context_line":"        devname \u003d devpath.split(\"/\")[-1]"},{"line_number":239,"context_line":"        try:"},{"line_number":240,"context_line":"            size_path_name \u003d os.path.join(\"/sys/class/block/\", devname, \"size\")"},{"line_number":241,"context_line":"            size_blks, err \u003d self._execute(\u0027cat\u0027, size_path_name,"},{"line_number":242,"context_line":"                                           run_as_root\u003dTrue,"},{"line_number":243,"context_line":"                                           root_helper\u003dself._root_helper)"},{"line_number":244,"context_line":"            bytesize \u003d int(size_blks) * 512"},{"line_number":245,"context_line":"            return bytesize"},{"line_number":246,"context_line":"        except putils.ProcessExecutionError:"},{"line_number":247,"context_line":"            LOG.warning(\"LIGHTOS: Could not find the size at for\""}],"source_content_type":"text/x-python","patch_set":10,"id":"3b6d8e9d_30b9b31a","line":244,"range":{"start_line":241,"start_character":0,"end_line":244,"end_character":0},"in_reply_to":"63caa0a5_35abff8d","updated":"2022-01-27 03:54:13.000000000","message":"Ack","commit_id":"bb01f874f568efff44fcc896ff93b1a7811b214d"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"09f0d98f1a83007f83c26b913106bad2e1021409","unresolved":true,"context_lines":[{"line_number":269,"context_line":"                 \" properties: %s\","},{"line_number":270,"context_line":"                 uuid, connection_properties)"},{"line_number":271,"context_line":"        self.dsc_connect_volume(connection_properties)"},{"line_number":272,"context_line":""},{"line_number":273,"context_line":"        device_path \u003d self._get_device_by_uuid(uuid)"},{"line_number":274,"context_line":"        if not device_path:"},{"line_number":275,"context_line":"            msg \u003d _(\"Device with uuid %s did not show up\" % uuid)"},{"line_number":276,"context_line":"            raise exception.BrickException(message\u003dmsg)"},{"line_number":277,"context_line":""},{"line_number":278,"context_line":"        device_info[\u0027path\u0027] \u003d device_path"},{"line_number":279,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"35f3b386_aa248a11","line":276,"range":{"start_line":272,"start_character":0,"end_line":276,"end_character":55},"updated":"2022-01-26 19:36:52.000000000","message":"-1: Why aren\u0027t we removing the file from \"self.dsc_file_name(uuid)\" on failure?  \n-1: Shouldn\u0027t we be catching exceptions as well?","commit_id":"bb01f874f568efff44fcc896ff93b1a7811b214d"},{"author":{"_account_id":33612,"name":"yuval","email":"yuval@lightbitslabs.com","username":"yuval"},"change_message_id":"b10473b95a1fe5bb47e926704817bc8510c987c7","unresolved":false,"context_lines":[{"line_number":269,"context_line":"                 \" properties: %s\","},{"line_number":270,"context_line":"                 uuid, connection_properties)"},{"line_number":271,"context_line":"        self.dsc_connect_volume(connection_properties)"},{"line_number":272,"context_line":""},{"line_number":273,"context_line":"        device_path \u003d self._get_device_by_uuid(uuid)"},{"line_number":274,"context_line":"        if not device_path:"},{"line_number":275,"context_line":"            msg \u003d _(\"Device with uuid %s did not show up\" % uuid)"},{"line_number":276,"context_line":"            raise exception.BrickException(message\u003dmsg)"},{"line_number":277,"context_line":""},{"line_number":278,"context_line":"        device_info[\u0027path\u0027] \u003d device_path"},{"line_number":279,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"13da4e2b_623b183d","line":276,"range":{"start_line":272,"start_character":0,"end_line":276,"end_character":55},"in_reply_to":"35f3b386_aa248a11","updated":"2022-01-27 03:54:13.000000000","message":"Ack","commit_id":"bb01f874f568efff44fcc896ff93b1a7811b214d"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"09f0d98f1a83007f83c26b913106bad2e1021409","unresolved":true,"context_lines":[{"line_number":276,"context_line":"            raise exception.BrickException(message\u003dmsg)"},{"line_number":277,"context_line":""},{"line_number":278,"context_line":"        device_info[\u0027path\u0027] \u003d device_path"},{"line_number":279,"context_line":""},{"line_number":280,"context_line":"        if self.message_queue:"},{"line_number":281,"context_line":"            self.message_queue.put((\u0027add\u0027, connection_properties))"},{"line_number":282,"context_line":""},{"line_number":283,"context_line":"        return device_info"},{"line_number":284,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"72444675_d7bf6b7a","line":281,"range":{"start_line":279,"start_character":0,"end_line":281,"end_character":66},"updated":"2022-01-26 19:36:52.000000000","message":"?: Shouldn\u0027t we have some kind of comment on what this \"message_queue\" actually is in this context?","commit_id":"bb01f874f568efff44fcc896ff93b1a7811b214d"},{"author":{"_account_id":33612,"name":"yuval","email":"yuval@lightbitslabs.com","username":"yuval"},"change_message_id":"b10473b95a1fe5bb47e926704817bc8510c987c7","unresolved":false,"context_lines":[{"line_number":276,"context_line":"            raise exception.BrickException(message\u003dmsg)"},{"line_number":277,"context_line":""},{"line_number":278,"context_line":"        device_info[\u0027path\u0027] \u003d device_path"},{"line_number":279,"context_line":""},{"line_number":280,"context_line":"        if self.message_queue:"},{"line_number":281,"context_line":"            self.message_queue.put((\u0027add\u0027, connection_properties))"},{"line_number":282,"context_line":""},{"line_number":283,"context_line":"        return device_info"},{"line_number":284,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"70104174_62c98201","line":281,"range":{"start_line":279,"start_character":0,"end_line":281,"end_character":66},"in_reply_to":"72444675_d7bf6b7a","updated":"2022-01-27 03:54:13.000000000","message":"Ack","commit_id":"bb01f874f568efff44fcc896ff93b1a7811b214d"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"09f0d98f1a83007f83c26b913106bad2e1021409","unresolved":true,"context_lines":[{"line_number":304,"context_line":"        \"\"\""},{"line_number":305,"context_line":"        uuid \u003d connection_properties[\u0027uuid\u0027]"},{"line_number":306,"context_line":"        LOG.debug(\u0027LIGHTOS: disconnect_volume called for volume %s\u0027, uuid)"},{"line_number":307,"context_line":"        self.dsc_disconnect_volume(connection_properties)"},{"line_number":308,"context_line":""},{"line_number":309,"context_line":"        if self.message_queue:"},{"line_number":310,"context_line":"            self.message_queue.put((\u0027delete\u0027, connection_properties))"}],"source_content_type":"text/x-python","patch_set":10,"id":"18bfd7f7_7509e788","line":307,"updated":"2022-01-26 19:36:52.000000000","message":"-1: The call to dsc_disconnect_volume only requests the disconnect, but doesn\u0027t actually confirm that the data has been flushed and the volume  has been disconnected.\n\nOpenStack works under the premise that after disconnect_volume returns the volume is no longer connected to the host in any way.","commit_id":"bb01f874f568efff44fcc896ff93b1a7811b214d"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"5c14f7fb8014ae1ef9eb59e5547ddea8623713b2","unresolved":false,"context_lines":[{"line_number":304,"context_line":"        \"\"\""},{"line_number":305,"context_line":"        uuid \u003d connection_properties[\u0027uuid\u0027]"},{"line_number":306,"context_line":"        LOG.debug(\u0027LIGHTOS: disconnect_volume called for volume %s\u0027, uuid)"},{"line_number":307,"context_line":"        self.dsc_disconnect_volume(connection_properties)"},{"line_number":308,"context_line":""},{"line_number":309,"context_line":"        if self.message_queue:"},{"line_number":310,"context_line":"            self.message_queue.put((\u0027delete\u0027, connection_properties))"}],"source_content_type":"text/x-python","patch_set":10,"id":"4e46c7f7_0592e7be","line":307,"in_reply_to":"0d2f17d1_f3c64819","updated":"2022-02-01 11:52:18.000000000","message":"-1: We still need to flush the data before this method returns.","commit_id":"bb01f874f568efff44fcc896ff93b1a7811b214d"},{"author":{"_account_id":33612,"name":"yuval","email":"yuval@lightbitslabs.com","username":"yuval"},"change_message_id":"b10473b95a1fe5bb47e926704817bc8510c987c7","unresolved":false,"context_lines":[{"line_number":304,"context_line":"        \"\"\""},{"line_number":305,"context_line":"        uuid \u003d connection_properties[\u0027uuid\u0027]"},{"line_number":306,"context_line":"        LOG.debug(\u0027LIGHTOS: disconnect_volume called for volume %s\u0027, uuid)"},{"line_number":307,"context_line":"        self.dsc_disconnect_volume(connection_properties)"},{"line_number":308,"context_line":""},{"line_number":309,"context_line":"        if self.message_queue:"},{"line_number":310,"context_line":"            self.message_queue.put((\u0027delete\u0027, connection_properties))"}],"source_content_type":"text/x-python","patch_set":10,"id":"663e3c3e_20b8cdfb","line":307,"in_reply_to":"18bfd7f7_7509e788","updated":"2022-01-27 03:54:13.000000000","message":"Ack","commit_id":"bb01f874f568efff44fcc896ff93b1a7811b214d"},{"author":{"_account_id":34459,"name":"Muli Ben-Yehuda","email":"muli@lightbitslabs.com","username":"muliby"},"change_message_id":"b579e919f2d7d214241317017cd7730ba4498fb8","unresolved":false,"context_lines":[{"line_number":304,"context_line":"        \"\"\""},{"line_number":305,"context_line":"        uuid \u003d connection_properties[\u0027uuid\u0027]"},{"line_number":306,"context_line":"        LOG.debug(\u0027LIGHTOS: disconnect_volume called for volume %s\u0027, uuid)"},{"line_number":307,"context_line":"        self.dsc_disconnect_volume(connection_properties)"},{"line_number":308,"context_line":""},{"line_number":309,"context_line":"        if self.message_queue:"},{"line_number":310,"context_line":"            self.message_queue.put((\u0027delete\u0027, connection_properties))"}],"source_content_type":"text/x-python","patch_set":10,"id":"0d2f17d1_f3c64819","line":307,"in_reply_to":"663e3c3e_20b8cdfb","updated":"2022-01-28 08:19:15.000000000","message":"With NVMe/TCP the disconnect is inherently an async operations susceptible to network faults. NVMe/TCP connections are not per-volume but rather host to host. With LightOS and NMVe/TCP, the actual \"volume disconnection\" happens after the ACL is updated so that the initiator no longer has permission to access the volume. This happens in the Cinder driver *after* the disconnect operation in the brick happens. Furthermore, updating the ACL means that the volume is no longer accessible, but it does not mean that the host necessarily knows, so the device file may stick around until some event takes place that will cause the host to remove the device file as inaccessible. Bottom line: there is no way to verify that the os-brick disconnect actually disconnects the volume. After the os-brick disconnect succeeds, Cinder will upadate the ACL and cause the volume to be disconnected.","commit_id":"bb01f874f568efff44fcc896ff93b1a7811b214d"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@purestorage.com","username":"sdodsley"},"change_message_id":"34146a45dd90c34824bfa82cefc017b782d5f0f0","unresolved":true,"context_lines":[{"line_number":77,"context_line":"            LOG.debug(\u0027LIGHTOS: did not find dsc, continuing anyway.\u0027)"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"        if hostnqn:"},{"line_number":80,"context_line":"            LOG.info(\"LIGHTOS: finally hostnqn: %s dsc: %s\","},{"line_number":81,"context_line":"                     hostnqn, found_dsc)"},{"line_number":82,"context_line":"            props[\u0027hostnqn\u0027] \u003d hostnqn"},{"line_number":83,"context_line":"            props[\u0027found_dsc\u0027] \u003d found_dsc"}],"source_content_type":"text/x-python","patch_set":11,"id":"8d3ad018_0178a61f","line":80,"updated":"2022-01-27 15:26:26.000000000","message":"Should be debug as per previous comment","commit_id":"85ac116d84a3a0e3538953c14058c29b4f15a1b0"},{"author":{"_account_id":33612,"name":"yuval","email":"yuval@lightbitslabs.com","username":"yuval"},"change_message_id":"95c95e50c2b664d0a27f0fa3fa94d768b4ef6d65","unresolved":false,"context_lines":[{"line_number":77,"context_line":"            LOG.debug(\u0027LIGHTOS: did not find dsc, continuing anyway.\u0027)"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"        if hostnqn:"},{"line_number":80,"context_line":"            LOG.info(\"LIGHTOS: finally hostnqn: %s dsc: %s\","},{"line_number":81,"context_line":"                     hostnqn, found_dsc)"},{"line_number":82,"context_line":"            props[\u0027hostnqn\u0027] \u003d hostnqn"},{"line_number":83,"context_line":"            props[\u0027found_dsc\u0027] \u003d found_dsc"}],"source_content_type":"text/x-python","patch_set":11,"id":"95983709_55023752","line":80,"in_reply_to":"8d3ad018_0178a61f","updated":"2022-01-28 09:13:32.000000000","message":"Done","commit_id":"85ac116d84a3a0e3538953c14058c29b4f15a1b0"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@purestorage.com","username":"sdodsley"},"change_message_id":"34146a45dd90c34824bfa82cefc017b782d5f0f0","unresolved":true,"context_lines":[{"line_number":82,"context_line":"            props[\u0027hostnqn\u0027] \u003d hostnqn"},{"line_number":83,"context_line":"            props[\u0027found_dsc\u0027] \u003d found_dsc"},{"line_number":84,"context_line":"        else:"},{"line_number":85,"context_line":"            LOG.warning(\u0027LIGHTOS: no hostnqn found.\u0027)"},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"        return props"},{"line_number":88,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"274ab5bf_b7a13e1b","line":85,"updated":"2022-01-27 15:26:26.000000000","message":"Not a warning - debug","commit_id":"85ac116d84a3a0e3538953c14058c29b4f15a1b0"},{"author":{"_account_id":33612,"name":"yuval","email":"yuval@lightbitslabs.com","username":"yuval"},"change_message_id":"d2bbfb866cd93d5d1cee1d6f1e262b744f50d2a8","unresolved":true,"context_lines":[{"line_number":82,"context_line":"            props[\u0027hostnqn\u0027] \u003d hostnqn"},{"line_number":83,"context_line":"            props[\u0027found_dsc\u0027] \u003d found_dsc"},{"line_number":84,"context_line":"        else:"},{"line_number":85,"context_line":"            LOG.warning(\u0027LIGHTOS: no hostnqn found.\u0027)"},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"        return props"},{"line_number":88,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"75055d4c_a02e60ef","line":85,"in_reply_to":"274ab5bf_b7a13e1b","updated":"2022-01-27 15:42:11.000000000","message":"Thanks I missed it","commit_id":"85ac116d84a3a0e3538953c14058c29b4f15a1b0"},{"author":{"_account_id":33612,"name":"yuval","email":"yuval@lightbitslabs.com","username":"yuval"},"change_message_id":"9dbc0798aac0e74f0c9f16b34331633cb41d6bf9","unresolved":false,"context_lines":[{"line_number":82,"context_line":"            props[\u0027hostnqn\u0027] \u003d hostnqn"},{"line_number":83,"context_line":"            props[\u0027found_dsc\u0027] \u003d found_dsc"},{"line_number":84,"context_line":"        else:"},{"line_number":85,"context_line":"            LOG.warning(\u0027LIGHTOS: no hostnqn found.\u0027)"},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"        return props"},{"line_number":88,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"bd8840f5_69b49708","line":85,"in_reply_to":"75055d4c_a02e60ef","updated":"2022-01-30 09:21:59.000000000","message":"Done","commit_id":"85ac116d84a3a0e3538953c14058c29b4f15a1b0"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"5c14f7fb8014ae1ef9eb59e5547ddea8623713b2","unresolved":true,"context_lines":[{"line_number":127,"context_line":"                LOG.warning("},{"line_number":128,"context_line":"                    \"LIGHTOS: Failed to create dsc file for connection with\""},{"line_number":129,"context_line":"                    f\" uuid:{uuid}\")"},{"line_number":130,"context_line":"                raise e"},{"line_number":131,"context_line":""},{"line_number":132,"context_line":"    def dsc_disconnect_volume(self, connection_info):"},{"line_number":133,"context_line":"        uuid \u003d connection_info[\u0027uuid\u0027]"}],"source_content_type":"text/x-python","patch_set":14,"id":"b6b5f612_ff44bfc3","line":130,"range":{"start_line":130,"start_character":0,"end_line":130,"end_character":23},"updated":"2022-02-01 11:52:18.000000000","message":"nit: It\u0027s better not to raise from a variable, because then the point of exception changes (it appears as if the exception actualy happened here) and instead do:\n\n except Exception:\n     LOG.warning( . . . )\n     raise","commit_id":"83a7710f5e4b972e8f95ce39bb9c747d6514c367"},{"author":{"_account_id":33612,"name":"yuval","email":"yuval@lightbitslabs.com","username":"yuval"},"change_message_id":"7ff30f2ff95b01579cd5387663c44df89226cd66","unresolved":false,"context_lines":[{"line_number":127,"context_line":"                LOG.warning("},{"line_number":128,"context_line":"                    \"LIGHTOS: Failed to create dsc file for connection with\""},{"line_number":129,"context_line":"                    f\" uuid:{uuid}\")"},{"line_number":130,"context_line":"                raise e"},{"line_number":131,"context_line":""},{"line_number":132,"context_line":"    def dsc_disconnect_volume(self, connection_info):"},{"line_number":133,"context_line":"        uuid \u003d connection_info[\u0027uuid\u0027]"}],"source_content_type":"text/x-python","patch_set":14,"id":"1ebcaf67_e79a8cec","line":130,"range":{"start_line":130,"start_character":0,"end_line":130,"end_character":23},"in_reply_to":"b6b5f612_ff44bfc3","updated":"2022-02-01 16:52:41.000000000","message":"Ack","commit_id":"83a7710f5e4b972e8f95ce39bb9c747d6514c367"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"5c14f7fb8014ae1ef9eb59e5547ddea8623713b2","unresolved":true,"context_lines":[{"line_number":135,"context_line":"            priv_lightos.delete_dsc_file(self.dsc_file_name(uuid))"},{"line_number":136,"context_line":"        except Exception as e:"},{"line_number":137,"context_line":"            LOG.warning(\"LIGHTOS: Failed delete dsc file uuid:{}\".format(uuid))"},{"line_number":138,"context_line":"            raise e"},{"line_number":139,"context_line":""},{"line_number":140,"context_line":"    def monitor_db(self, lightos_db):"},{"line_number":141,"context_line":"        for connection_info in lightos_db.values():"}],"source_content_type":"text/x-python","patch_set":14,"id":"a33ff896_50f6416c","line":138,"updated":"2022-02-01 11:52:18.000000000","message":"nit: same here","commit_id":"83a7710f5e4b972e8f95ce39bb9c747d6514c367"},{"author":{"_account_id":33612,"name":"yuval","email":"yuval@lightbitslabs.com","username":"yuval"},"change_message_id":"7ff30f2ff95b01579cd5387663c44df89226cd66","unresolved":false,"context_lines":[{"line_number":135,"context_line":"            priv_lightos.delete_dsc_file(self.dsc_file_name(uuid))"},{"line_number":136,"context_line":"        except Exception as e:"},{"line_number":137,"context_line":"            LOG.warning(\"LIGHTOS: Failed delete dsc file uuid:{}\".format(uuid))"},{"line_number":138,"context_line":"            raise e"},{"line_number":139,"context_line":""},{"line_number":140,"context_line":"    def monitor_db(self, lightos_db):"},{"line_number":141,"context_line":"        for connection_info in lightos_db.values():"}],"source_content_type":"text/x-python","patch_set":14,"id":"d7bb28bb_51fe042a","line":138,"in_reply_to":"a33ff896_50f6416c","updated":"2022-02-01 16:52:41.000000000","message":"Ack","commit_id":"83a7710f5e4b972e8f95ce39bb9c747d6514c367"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"5c14f7fb8014ae1ef9eb59e5547ddea8623713b2","unresolved":true,"context_lines":[{"line_number":160,"context_line":"                         connection[\u0027uuid\u0027])"},{"line_number":161,"context_line":"                lightos_db[connection[\u0027uuid\u0027]] \u003d connection"},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"    def lightos_monitor(self, lightos_db, message_queue):"},{"line_number":164,"context_line":"        \u0027\u0027\u0027Bookkeeping lightos connections."},{"line_number":165,"context_line":""},{"line_number":166,"context_line":"        This is useful when the connector is comming up to a running node with"}],"source_content_type":"text/x-python","patch_set":14,"id":"0e000fd3_1d848436","line":163,"updated":"2022-02-01 11:52:18.000000000","message":"nit: It should be mentioned that this code is called from Nova. Otherwise it\u0027s hard to know since it\u0027s not part of the general connector interface.","commit_id":"83a7710f5e4b972e8f95ce39bb9c747d6514c367"},{"author":{"_account_id":33612,"name":"yuval","email":"yuval@lightbitslabs.com","username":"yuval"},"change_message_id":"7ff30f2ff95b01579cd5387663c44df89226cd66","unresolved":false,"context_lines":[{"line_number":160,"context_line":"                         connection[\u0027uuid\u0027])"},{"line_number":161,"context_line":"                lightos_db[connection[\u0027uuid\u0027]] \u003d connection"},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"    def lightos_monitor(self, lightos_db, message_queue):"},{"line_number":164,"context_line":"        \u0027\u0027\u0027Bookkeeping lightos connections."},{"line_number":165,"context_line":""},{"line_number":166,"context_line":"        This is useful when the connector is comming up to a running node with"}],"source_content_type":"text/x-python","patch_set":14,"id":"f96a1203_dfa23b1f","line":163,"in_reply_to":"0e000fd3_1d848436","updated":"2022-02-01 16:52:41.000000000","message":"Ack","commit_id":"83a7710f5e4b972e8f95ce39bb9c747d6514c367"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"5c14f7fb8014ae1ef9eb59e5547ddea8623713b2","unresolved":true,"context_lines":[{"line_number":193,"context_line":"        if os.path.exists(lnk_path):"},{"line_number":194,"context_line":"            devname \u003d os.path.realpath(lnk_path)"},{"line_number":195,"context_line":"            if devname.startswith(\"/dev/nvme\"):"},{"line_number":196,"context_line":"                devname \u003d devname.strip()"},{"line_number":197,"context_line":"                LOG.info(\"LIGHTOS: devpath %s detected for uuid %s\","},{"line_number":198,"context_line":"                         devname, uuid)"},{"line_number":199,"context_line":"                return devname"}],"source_content_type":"text/x-python","patch_set":14,"id":"6d550906_04472e34","line":196,"updated":"2022-02-01 11:52:18.000000000","message":"nit: Realpath doesn\u0027t return a string with leading or trailing spaces.","commit_id":"83a7710f5e4b972e8f95ce39bb9c747d6514c367"},{"author":{"_account_id":33612,"name":"yuval","email":"yuval@lightbitslabs.com","username":"yuval"},"change_message_id":"7ff30f2ff95b01579cd5387663c44df89226cd66","unresolved":false,"context_lines":[{"line_number":193,"context_line":"        if os.path.exists(lnk_path):"},{"line_number":194,"context_line":"            devname \u003d os.path.realpath(lnk_path)"},{"line_number":195,"context_line":"            if devname.startswith(\"/dev/nvme\"):"},{"line_number":196,"context_line":"                devname \u003d devname.strip()"},{"line_number":197,"context_line":"                LOG.info(\"LIGHTOS: devpath %s detected for uuid %s\","},{"line_number":198,"context_line":"                         devname, uuid)"},{"line_number":199,"context_line":"                return devname"}],"source_content_type":"text/x-python","patch_set":14,"id":"30dae57a_5d8a4eb4","line":196,"in_reply_to":"6d550906_04472e34","updated":"2022-02-01 16:52:41.000000000","message":"Ack","commit_id":"83a7710f5e4b972e8f95ce39bb9c747d6514c367"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"5c14f7fb8014ae1ef9eb59e5547ddea8623713b2","unresolved":true,"context_lines":[{"line_number":219,"context_line":"                continue"},{"line_number":220,"context_line":""},{"line_number":221,"context_line":"            LOG.info(\"LIGHTOS: matching uuid %s was found\""},{"line_number":222,"context_line":"                     \" for device path %s\" % (uuid, match_path))"},{"line_number":223,"context_line":"            return os.path.join(\"/dev\", match_path.split(\"/\")[-2])"},{"line_number":224,"context_line":"        return None"},{"line_number":225,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"f739f550_4c444c27","line":222,"updated":"2022-02-01 11:52:18.000000000","message":"-1: For LOG methods we should be using late binding for parameters as explained in the contributor documentation.\n\n            LOG.info(\"LIGHTOS: matching uuid %s was found\"\n                     \" for device path %s\", uuid, match_path)","commit_id":"83a7710f5e4b972e8f95ce39bb9c747d6514c367"},{"author":{"_account_id":33612,"name":"yuval","email":"yuval@lightbitslabs.com","username":"yuval"},"change_message_id":"7ff30f2ff95b01579cd5387663c44df89226cd66","unresolved":false,"context_lines":[{"line_number":219,"context_line":"                continue"},{"line_number":220,"context_line":""},{"line_number":221,"context_line":"            LOG.info(\"LIGHTOS: matching uuid %s was found\""},{"line_number":222,"context_line":"                     \" for device path %s\" % (uuid, match_path))"},{"line_number":223,"context_line":"            return os.path.join(\"/dev\", match_path.split(\"/\")[-2])"},{"line_number":224,"context_line":"        return None"},{"line_number":225,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"9042c4c3_71b7dc09","line":222,"in_reply_to":"f739f550_4c444c27","updated":"2022-02-01 16:52:41.000000000","message":"Ack","commit_id":"83a7710f5e4b972e8f95ce39bb9c747d6514c367"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"5c14f7fb8014ae1ef9eb59e5547ddea8623713b2","unresolved":true,"context_lines":[{"line_number":311,"context_line":"        \"\"\""},{"line_number":312,"context_line":"        uuid \u003d connection_properties[\u0027uuid\u0027]"},{"line_number":313,"context_line":"        LOG.debug(\u0027LIGHTOS: disconnect_volume called for volume %s\u0027, uuid)"},{"line_number":314,"context_line":"        self.dsc_disconnect_volume(connection_properties)"},{"line_number":315,"context_line":"        # bookkeeping lightos connections - delete connection"},{"line_number":316,"context_line":"        if self.message_queue:"},{"line_number":317,"context_line":"            self.message_queue.put((\u0027delete\u0027, connection_properties))"}],"source_content_type":"text/x-python","patch_set":14,"id":"80dadf8c_8196397f","line":314,"updated":"2022-02-01 11:52:18.000000000","message":"-1: This needs to flush I/O data to disk.","commit_id":"83a7710f5e4b972e8f95ce39bb9c747d6514c367"},{"author":{"_account_id":33612,"name":"yuval","email":"yuval@lightbitslabs.com","username":"yuval"},"change_message_id":"7ff30f2ff95b01579cd5387663c44df89226cd66","unresolved":false,"context_lines":[{"line_number":311,"context_line":"        \"\"\""},{"line_number":312,"context_line":"        uuid \u003d connection_properties[\u0027uuid\u0027]"},{"line_number":313,"context_line":"        LOG.debug(\u0027LIGHTOS: disconnect_volume called for volume %s\u0027, uuid)"},{"line_number":314,"context_line":"        self.dsc_disconnect_volume(connection_properties)"},{"line_number":315,"context_line":"        # bookkeeping lightos connections - delete connection"},{"line_number":316,"context_line":"        if self.message_queue:"},{"line_number":317,"context_line":"            self.message_queue.put((\u0027delete\u0027, connection_properties))"}],"source_content_type":"text/x-python","patch_set":14,"id":"e6e3f529_ef090cac","line":314,"in_reply_to":"80dadf8c_8196397f","updated":"2022-02-01 16:52:41.000000000","message":"Ack","commit_id":"83a7710f5e4b972e8f95ce39bb9c747d6514c367"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"26a28b2710af91dd31bb07a6995647a150d8b83d","unresolved":false,"context_lines":[{"line_number":311,"context_line":"        \"\"\""},{"line_number":312,"context_line":"        uuid \u003d connection_properties[\u0027uuid\u0027]"},{"line_number":313,"context_line":"        LOG.debug(\u0027LIGHTOS: disconnect_volume called for volume %s\u0027, uuid)"},{"line_number":314,"context_line":"        self.dsc_disconnect_volume(connection_properties)"},{"line_number":315,"context_line":"        # bookkeeping lightos connections - delete connection"},{"line_number":316,"context_line":"        if self.message_queue:"},{"line_number":317,"context_line":"            self.message_queue.put((\u0027delete\u0027, connection_properties))"}],"source_content_type":"text/x-python","patch_set":14,"id":"997c4e79_8c3da6f4","line":314,"in_reply_to":"80dadf8c_8196397f","updated":"2022-02-01 16:47:37.000000000","message":"Done","commit_id":"83a7710f5e4b972e8f95ce39bb9c747d6514c367"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"26a28b2710af91dd31bb07a6995647a150d8b83d","unresolved":true,"context_lines":[{"line_number":80,"context_line":"        if hostnqn:"},{"line_number":81,"context_line":"            LOG.debug(\"LIGHTOS: finally hostnqn: %s dsc: %s\","},{"line_number":82,"context_line":"                      hostnqn, found_dsc)"},{"line_number":83,"context_line":"            props[\u0027hostnqn\u0027] \u003d hostnqn"},{"line_number":84,"context_line":"            props[\u0027found_dsc\u0027] \u003d found_dsc"},{"line_number":85,"context_line":"        else:"},{"line_number":86,"context_line":"            LOG.debug(\u0027LIGHTOS: no hostnqn found.\u0027)"}],"source_content_type":"text/x-python","patch_set":15,"id":"19aebae1_f45e1da8","line":83,"updated":"2022-02-01 16:47:37.000000000","message":"Please, in a follow up, stop returning this hostnqn and use the currently returned nqn.\n\nAs discussed, we can move method `get_hostnqn` from here and NVMe to os_brick/utils.py and then fill the \"nqn\" key here [1], that way it\u0027s not the NVMe nor the Lightbits connector that returns it, but it\u0027s the common code.\n\n[1]: https://github.com/openstack/os-brick/blob/7a6a09fc84a779c3ee08d122664f941195eeab8f/os_brick/initiator/connector.py#L224","commit_id":"627da6a40b4679a41245502b12e06b8340f1ca05"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"26a28b2710af91dd31bb07a6995647a150d8b83d","unresolved":true,"context_lines":[{"line_number":312,"context_line":"        \"\"\""},{"line_number":313,"context_line":"        if force:"},{"line_number":314,"context_line":"            # if operator want to force detach - we just return and cinder"},{"line_number":315,"context_line":"            # driver will terminate the connection by updating the ACL"},{"line_number":316,"context_line":"            return"},{"line_number":317,"context_line":"        uuid \u003d connection_properties[\u0027uuid\u0027]"},{"line_number":318,"context_line":"        LOG.debug(\u0027LIGHTOS: disconnect_volume called for volume %s\u0027, uuid)"}],"source_content_type":"text/x-python","patch_set":15,"id":"61a26e67_8d230159","line":315,"updated":"2022-02-01 16:47:37.000000000","message":"?: Will that really remove the device from the host system?  I don\u0027t have a deployment at hand and for SCSI based protocols (iSCSI and FC) that is not the case even if the ACL is removed, the volume delete in the backend, etc.","commit_id":"627da6a40b4679a41245502b12e06b8340f1ca05"}]}