)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"4cadb8d56eeebb3148888a152428acad90a1abf4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"d9f460df_245694fc","updated":"2022-04-05 21:56:16.000000000","message":"Since we are changing code that affects extend and disconnect volume... is there a risk of anything happening when nova-compute is upgraded to use this version of os-brick, without detaching volumes that are currently in use?\n","commit_id":"c76d13ba7f7335277240e45e9a0007aad10dac03"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"a913482ac43ebf4306b22594711ecde0406c9f91","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"02e76b6d_5caf8408","in_reply_to":"d9f460df_245694fc","updated":"2022-04-08 11:36:15.000000000","message":"I took that into consideration when writing the code, so besides the risk that I inadvertently introduced a bug I don\u0027t think it\u0027d be a problem.\n\nThe reason is that the `connect_volume_undo_prepare_result` method uses the `_device_path_from_symlink` method to replace the symlink with the one that the connector originally returned, and that method explicitly looks for the custom symlink format (which is impossible to have been used by any of the existing connections) and if it doesn\u0027t match then it leaves the path as it was.\n\nThat way existing encrypted connections will not have their paths modified.  The only difference will be that they get a copy of the dictionary instead of the original one.\n\nIn any case, your concern is so important that I\u0027m going to harden it a bit more so that in those cases **nothing** changes.","commit_id":"c76d13ba7f7335277240e45e9a0007aad10dac03"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"64fd159a40f505241306e118159fdddc39d85351","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"d1b9832a_93f21dbf","updated":"2022-04-14 22:51:52.000000000","message":"I have a question. This does not mention dropping the udev rules. So the race is still there and I don\u0027t understand just how exactly this patch fixes bug 1967790.","commit_id":"dd9dc993e1088baf2d1efd88c21f1accd96adeae"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"d07648b7a9564fecfe78bf3caa9cda6294e386cf","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"d373cac0_5ef53d29","updated":"2022-04-12 09:49:51.000000000","message":"recheck os-brick-src-tempest-lvm-lio-barbican : Error is in self.create_test_server, which is before there is a Cinder attachment, so unrelated to this patch.","commit_id":"dd9dc993e1088baf2d1efd88c21f1accd96adeae"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"fa48492d28a3ae5eedfdeee5de4286c8850f16e1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"cf48fc6b_75f581f0","updated":"2022-04-20 16:30:46.000000000","message":"I hurts to downvote my own patch...","commit_id":"c84d82d26d23d6238f3fcaf265d302bb5faa7954"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"4bd5fbd93ac6ce9270d4c16e81f7b8075282d55a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"b07e08cc_a08567b5","updated":"2022-04-19 17:23:41.000000000","message":"Thanks for the reviews","commit_id":"c84d82d26d23d6238f3fcaf265d302bb5faa7954"},{"author":{"_account_id":9914,"name":"Ade Lee","email":"alee@redhat.com","username":"alee"},"change_message_id":"f812015be02af0ae66a17c14bb59271fa52e410b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"d9716184_b789e72f","updated":"2022-04-19 21:41:32.000000000","message":"This fix appears to fix issues found in the FIPS jobs","commit_id":"c84d82d26d23d6238f3fcaf265d302bb5faa7954"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"f3d804ec743e33b78e60261d081122183332cd66","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"9e5a9183_38a809c6","updated":"2022-04-20 18:19:01.000000000","message":"Looks like the only change was the added convert_str().","commit_id":"bd846b51a2c0cd8c082d9fc77b3283d1e1dd9ea3"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"e9f2fd653a6abe8fd2cb7011abca983b8a08407b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"b3bf5bca_de781de0","updated":"2022-04-27 13:09:26.000000000","message":"unit tests failing","commit_id":"f1b2159c763960ee336742ce245a614118a1f522"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"9a173cac67d43abe5facdf0515e03d48ee589463","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"f2f19633_7eb2c79d","updated":"2022-05-03 10:28:38.000000000","message":"Lightbits failure is due to an unrelated error: \u0027No valid host was found. There are not enough hosts available.\u0027","commit_id":"90f46447cad41fcc1f6a53596a077acbe0203127"},{"author":{"_account_id":30615,"name":"Tushar Trambak Gite","email":"tushargite96@gmail.com","username":"tushargite96"},"change_message_id":"8a4d474ff76752e758d4b8b5ba3368f30d19f319","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"f6ea99c0_902bd24e","updated":"2022-05-17 16:51:06.000000000","message":"Looks good ","commit_id":"2fca258cb9e6296f18e31e4e8c4c863605b80456"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"db6d93dc1df06e445e86204955894a423e368e19","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"3352e04d_712a0a59","updated":"2022-05-12 04:03:14.000000000","message":"Sorry for being dumb, I finally figured it out with the extend_volume. Of course it can be called several times, but it\u0027s completely okay, because we copy the dictionaries that are passed as call arguments, before we modify them in the wrapper installed by the connect_volume_undo_prepare_result decorator.","commit_id":"2fca258cb9e6296f18e31e4e8c4c863605b80456"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"775c19a0eb90e27d04b4c0b3fed7696923d564dd","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"c182a6ab_3a2a3459","updated":"2022-05-11 17:00:25.000000000","message":"Thanks, the latest update addresses all my previous doubts and concerns. And the additional comments regarding use of the unprepare decorator for extend_volume() really helped.","commit_id":"2fca258cb9e6296f18e31e4e8c4c863605b80456"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"4891672ac871fa43b2ee70f9dda441598db02b2d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"8b21ce4a_6953aa6d","updated":"2022-05-26 15:13:37.000000000","message":"Thanks for working on this. LGTM and Zuul passed","commit_id":"1583a5038d34fd560b554e0eee5c1c5a7612722f"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"b76f0f7088846d954988c33fbdb7ac081326c355","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"62e88fce_f48c2c3a","updated":"2022-05-25 14:39:34.000000000","message":"recheck os-brick-src-tempest-lvm-lio-barbican : unrelated incremental backup failure","commit_id":"1583a5038d34fd560b554e0eee5c1c5a7612722f"}],"os_brick/initiator/initiator_connector.py":[{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"a4497add1be694a8456303110101442f33f51735","unresolved":true,"context_lines":[{"line_number":192,"context_line":"        pass"},{"line_number":193,"context_line":""},{"line_number":194,"context_line":"    @abc.abstractmethod"},{"line_number":195,"context_line":"    def extend_volume(self, connection_properties):"},{"line_number":196,"context_line":"        \"\"\"Update the attached volume\u0027s size."},{"line_number":197,"context_line":""},{"line_number":198,"context_line":"        This method will attempt to update the local hosts\u0027s"}],"source_content_type":"text/x-python","patch_set":8,"id":"dc7d8879_4d617e04","line":195,"updated":"2022-05-06 20:55:24.000000000","message":"It seems this method is often (always?) wrapped by the connect_volume_undo_prepare_result decorator. That should be documented here, if for no other reason than I haven\u0027t yet figured out why this is done!","commit_id":"90f46447cad41fcc1f6a53596a077acbe0203127"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"502f31076e8155f8b9eb6eac03fef6daad565db1","unresolved":false,"context_lines":[{"line_number":192,"context_line":"        pass"},{"line_number":193,"context_line":""},{"line_number":194,"context_line":"    @abc.abstractmethod"},{"line_number":195,"context_line":"    def extend_volume(self, connection_properties):"},{"line_number":196,"context_line":"        \"\"\"Update the attached volume\u0027s size."},{"line_number":197,"context_line":""},{"line_number":198,"context_line":"        This method will attempt to update the local hosts\u0027s"}],"source_content_type":"text/x-python","patch_set":8,"id":"c7e770b4_daa4e922","line":195,"in_reply_to":"dc7d8879_4d617e04","updated":"2022-05-11 12:03:51.000000000","message":"It\u0027s for the same reasons as the disconnect, if the extend_volume uses the path that was returned to know what device is being extended.\n\nAdded a comment.","commit_id":"90f46447cad41fcc1f6a53596a077acbe0203127"}],"os_brick/privileged/rootwrap.py":[{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"a4497add1be694a8456303110101442f33f51735","unresolved":true,"context_lines":[{"line_number":230,"context_line":"    LOG.debug(\u0027Linking %s -\u003e %s\u0027, link_name, target)"},{"line_number":231,"context_line":"    if force:"},{"line_number":232,"context_line":"        try:"},{"line_number":233,"context_line":"            os.remove(link_name)"},{"line_number":234,"context_line":"        except FileNotFoundError:"},{"line_number":235,"context_line":"            pass"},{"line_number":236,"context_line":"    os.symlink(target, link_name)"}],"source_content_type":"text/x-python","patch_set":8,"id":"9857ee9a_a5360ae1","line":233,"updated":"2022-05-06 20:55:24.000000000","message":"If link_name exists, should the code check whether it\u0027s actually a symlink (as opposed to a regular file)? I don\u0027t know if there\u0027s any risk of this being called that way. Conversely, maybe that\u0027s a valid use case? (delete a file and replace it with a symlink)","commit_id":"90f46447cad41fcc1f6a53596a077acbe0203127"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"502f31076e8155f8b9eb6eac03fef6daad565db1","unresolved":false,"context_lines":[{"line_number":230,"context_line":"    LOG.debug(\u0027Linking %s -\u003e %s\u0027, link_name, target)"},{"line_number":231,"context_line":"    if force:"},{"line_number":232,"context_line":"        try:"},{"line_number":233,"context_line":"            os.remove(link_name)"},{"line_number":234,"context_line":"        except FileNotFoundError:"},{"line_number":235,"context_line":"            pass"},{"line_number":236,"context_line":"    os.symlink(target, link_name)"}],"source_content_type":"text/x-python","patch_set":8,"id":"e764745d_1016cbdc","line":233,"in_reply_to":"9857ee9a_a5360ae1","updated":"2022-05-11 12:03:51.000000000","message":"My intention was to make the link_root method behave like the \"ln -s\" command, and the force parameter would be the equivalent of the \"--force\" there.  Which replaces the target file even if it\u0027s not a link.\n\nI\u0027ll add a comment to the docstring to clarify it.","commit_id":"90f46447cad41fcc1f6a53596a077acbe0203127"}],"os_brick/utils.py":[{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"3159fb2c6d9a7da93793332a96da872810fd9509","unresolved":true,"context_lines":[{"line_number":263,"context_line":"    return symlink"},{"line_number":264,"context_line":""},{"line_number":265,"context_line":""},{"line_number":266,"context_line":"def connect_volume_prepare_result(func):"},{"line_number":267,"context_line":"    \"\"\"Decorator to prepare the result of connect_volume for encrypted volumes."},{"line_number":268,"context_line":""},{"line_number":269,"context_line":"    WARNING: This decorator must be **before** any connect_volume locking"}],"source_content_type":"text/x-python","patch_set":1,"id":"e552f98e_683163b4","line":266,"updated":"2022-04-05 13:29:52.000000000","message":"There is probably some value in adding type annotations to these to prevent misuse -- i.e.\n    def connect_volume_prepare_result(\n        func: Callable[[Any, dict], dict]) -\u003e Callable[[Any, dict], dict]:\n\nor so.  (We may eventually have a stronger type for connection_properties than just \"dict\", but this is a good start.)","commit_id":"c76d13ba7f7335277240e45e9a0007aad10dac03"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"a913482ac43ebf4306b22594711ecde0406c9f91","unresolved":false,"context_lines":[{"line_number":263,"context_line":"    return symlink"},{"line_number":264,"context_line":""},{"line_number":265,"context_line":""},{"line_number":266,"context_line":"def connect_volume_prepare_result(func):"},{"line_number":267,"context_line":"    \"\"\"Decorator to prepare the result of connect_volume for encrypted volumes."},{"line_number":268,"context_line":""},{"line_number":269,"context_line":"    WARNING: This decorator must be **before** any connect_volume locking"}],"source_content_type":"text/x-python","patch_set":1,"id":"df4dce25_afe73965","line":266,"in_reply_to":"e552f98e_683163b4","updated":"2022-04-08 11:36:15.000000000","message":"Definitely a good idea.","commit_id":"c76d13ba7f7335277240e45e9a0007aad10dac03"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"4cadb8d56eeebb3148888a152428acad90a1abf4","unresolved":true,"context_lines":[{"line_number":336,"context_line":"    Examples of connector methods that may want to use this are"},{"line_number":337,"context_line":"    disconnect_volume and extend_volume."},{"line_number":338,"context_line":"    \"\"\""},{"line_number":339,"context_line":"    # Don\u0027t use functools.wraps because it breaks the introspection"},{"line_number":340,"context_line":"    @functools.wraps(func)"},{"line_number":341,"context_line":"    def change_encrypted(*args, **kwargs):"},{"line_number":342,"context_line":"        # We may receive only connection_properties or also device_info params"}],"source_content_type":"text/x-python","patch_set":1,"id":"21b77711_c7faae80","line":339,"range":{"start_line":339,"start_character":4,"end_line":339,"end_character":31},"updated":"2022-04-05 21:56:16.000000000","message":"Not sure what this comment means -- it\u0027s used at line 340?","commit_id":"c76d13ba7f7335277240e45e9a0007aad10dac03"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"a913482ac43ebf4306b22594711ecde0406c9f91","unresolved":false,"context_lines":[{"line_number":336,"context_line":"    Examples of connector methods that may want to use this are"},{"line_number":337,"context_line":"    disconnect_volume and extend_volume."},{"line_number":338,"context_line":"    \"\"\""},{"line_number":339,"context_line":"    # Don\u0027t use functools.wraps because it breaks the introspection"},{"line_number":340,"context_line":"    @functools.wraps(func)"},{"line_number":341,"context_line":"    def change_encrypted(*args, **kwargs):"},{"line_number":342,"context_line":"        # We may receive only connection_properties or also device_info params"}],"source_content_type":"text/x-python","patch_set":1,"id":"6a20f0c6_c6ed566b","line":339,"range":{"start_line":339,"start_character":4,"end_line":339,"end_character":31},"in_reply_to":"21b77711_c7faae80","updated":"2022-04-08 11:36:15.000000000","message":"Thanks for catching this one.\n\nIt\u0027s a leftover comment from when I thought this was breaking the introspection, but it turns out that it wasn\u0027t this, but the synchronize partial function, so I added warning from L327-328.\n\nWill remove it.","commit_id":"c76d13ba7f7335277240e45e9a0007aad10dac03"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"64fd159a40f505241306e118159fdddc39d85351","unresolved":true,"context_lines":[{"line_number":243,"context_line":""},{"line_number":244,"context_line":"    The symlink will be created under the /dev/disk/by-id directory and will"},{"line_number":245,"context_line":"    prefix the name with os-brick- and then continue with the full device path"},{"line_number":246,"context_line":"    that was passed (replacing / with _"},{"line_number":247,"context_line":"    \"\"\""},{"line_number":248,"context_line":"    # Convert / into _ that is unlikely used by devices (tried usin · but"},{"line_number":249,"context_line":"    # cryptsetup is not happy with it)"}],"source_content_type":"text/x-python","patch_set":2,"id":"e192803b_6f70aa0f","line":246,"updated":"2022-04-14 22:51:52.000000000","message":"kinda bugs me that closing parenthesis is missing :-)","commit_id":"dd9dc993e1088baf2d1efd88c21f1accd96adeae"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"4bd5fbd93ac6ce9270d4c16e81f7b8075282d55a","unresolved":false,"context_lines":[{"line_number":243,"context_line":""},{"line_number":244,"context_line":"    The symlink will be created under the /dev/disk/by-id directory and will"},{"line_number":245,"context_line":"    prefix the name with os-brick- and then continue with the full device path"},{"line_number":246,"context_line":"    that was passed (replacing / with _"},{"line_number":247,"context_line":"    \"\"\""},{"line_number":248,"context_line":"    # Convert / into _ that is unlikely used by devices (tried usin · but"},{"line_number":249,"context_line":"    # cryptsetup is not happy with it)"}],"source_content_type":"text/x-python","patch_set":2,"id":"a9ab3ff3_71481b28","line":246,"in_reply_to":"e192803b_6f70aa0f","updated":"2022-04-19 17:23:41.000000000","message":"Done","commit_id":"dd9dc993e1088baf2d1efd88c21f1accd96adeae"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"64fd159a40f505241306e118159fdddc39d85351","unresolved":true,"context_lines":[{"line_number":254,"context_line":"def _device_path_from_symlink(symlink):"},{"line_number":255,"context_line":"    \"\"\"Get the original encrypted device path from the device symlink."},{"line_number":256,"context_line":""},{"line_number":257,"context_line":"    This is the reverse operation of what the one performed by the"},{"line_number":258,"context_line":"    _symlink_name_from_device_path method."},{"line_number":259,"context_line":"    \"\"\""},{"line_number":260,"context_line":"    if symlink and symlink.startswith(CUSTOM_LINK_PREFIX):"}],"source_content_type":"text/x-python","patch_set":2,"id":"8bfe0714_d586a3c4","line":257,"updated":"2022-04-14 22:51:52.000000000","message":"drop \"what\"","commit_id":"dd9dc993e1088baf2d1efd88c21f1accd96adeae"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"4bd5fbd93ac6ce9270d4c16e81f7b8075282d55a","unresolved":false,"context_lines":[{"line_number":254,"context_line":"def _device_path_from_symlink(symlink):"},{"line_number":255,"context_line":"    \"\"\"Get the original encrypted device path from the device symlink."},{"line_number":256,"context_line":""},{"line_number":257,"context_line":"    This is the reverse operation of what the one performed by the"},{"line_number":258,"context_line":"    _symlink_name_from_device_path method."},{"line_number":259,"context_line":"    \"\"\""},{"line_number":260,"context_line":"    if symlink and symlink.startswith(CUSTOM_LINK_PREFIX):"}],"source_content_type":"text/x-python","patch_set":2,"id":"0b7d7a4f_353f2886","line":257,"in_reply_to":"8bfe0714_d586a3c4","updated":"2022-04-19 17:23:41.000000000","message":"Done","commit_id":"dd9dc993e1088baf2d1efd88c21f1accd96adeae"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"64fd159a40f505241306e118159fdddc39d85351","unresolved":true,"context_lines":[{"line_number":259,"context_line":"    \"\"\""},{"line_number":260,"context_line":"    if symlink and symlink.startswith(CUSTOM_LINK_PREFIX):"},{"line_number":261,"context_line":"        ending \u003d symlink[len(CUSTOM_LINK_PREFIX):]"},{"line_number":262,"context_line":"        return ending.replace(\u0027_\u0027, \u0027/\u0027)"},{"line_number":263,"context_line":"    return symlink"},{"line_number":264,"context_line":""},{"line_number":265,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"b7a3d1ef_8dfda86c","line":262,"updated":"2022-04-14 22:51:52.000000000","message":"Wait, this is not undoing what _symlink_name_from_device did. It could contain underscores in addition to slashes. Now they all went back to slashes.","commit_id":"dd9dc993e1088baf2d1efd88c21f1accd96adeae"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"fc2cfb44a087a7af5844430808163cadcf3c234a","unresolved":false,"context_lines":[{"line_number":259,"context_line":"    \"\"\""},{"line_number":260,"context_line":"    if symlink and symlink.startswith(CUSTOM_LINK_PREFIX):"},{"line_number":261,"context_line":"        ending \u003d symlink[len(CUSTOM_LINK_PREFIX):]"},{"line_number":262,"context_line":"        return ending.replace(\u0027_\u0027, \u0027/\u0027)"},{"line_number":263,"context_line":"    return symlink"},{"line_number":264,"context_line":""},{"line_number":265,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"c1a20171_e64f9f9c","line":262,"in_reply_to":"6b6ac942_dd058439","updated":"2022-05-12 16:17:08.000000000","message":"Done","commit_id":"dd9dc993e1088baf2d1efd88c21f1accd96adeae"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"4bd5fbd93ac6ce9270d4c16e81f7b8075282d55a","unresolved":true,"context_lines":[{"line_number":259,"context_line":"    \"\"\""},{"line_number":260,"context_line":"    if symlink and symlink.startswith(CUSTOM_LINK_PREFIX):"},{"line_number":261,"context_line":"        ending \u003d symlink[len(CUSTOM_LINK_PREFIX):]"},{"line_number":262,"context_line":"        return ending.replace(\u0027_\u0027, \u0027/\u0027)"},{"line_number":263,"context_line":"    return symlink"},{"line_number":264,"context_line":""},{"line_number":265,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"bc3f9012_224cb19f","line":262,"in_reply_to":"b7a3d1ef_8dfda86c","updated":"2022-04-19 17:23:41.000000000","message":"As mentioned on L248 afaik the _ is not used by devices. To the best of my knowledge not on NVMe-oF, iSCSI, or FC.\n\nI tried using the \u0027·\u0027 character, but cryptsetup didn\u0027t like it.  I\u0027m open to suggestions on alternative mechanisms to encode the original device path in the symlink name.","commit_id":"dd9dc993e1088baf2d1efd88c21f1accd96adeae"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"afd2be4df8582814432a0151cc8c98db256630c9","unresolved":true,"context_lines":[{"line_number":259,"context_line":"    \"\"\""},{"line_number":260,"context_line":"    if symlink and symlink.startswith(CUSTOM_LINK_PREFIX):"},{"line_number":261,"context_line":"        ending \u003d symlink[len(CUSTOM_LINK_PREFIX):]"},{"line_number":262,"context_line":"        return ending.replace(\u0027_\u0027, \u0027/\u0027)"},{"line_number":263,"context_line":"    return symlink"},{"line_number":264,"context_line":""},{"line_number":265,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"6b6ac942_dd058439","line":262,"in_reply_to":"ba70638a_8fc2e3a2","updated":"2022-04-25 13:18:05.000000000","message":"I\u0027ll replace it with \u0027+\u0027 instead, because I found some udev rules that actually use \u0027_\u0027 in the symlinks\n\n/usr/lib/udev/rules.d/63-scsi-sg3_symlink.rules:12:ENV{SCSI_IDENT_SERIAL}\u003d\u003d\"?*\", ENV{DEVTYPE}\u003d\u003d\"disk\", SYMLINK+\u003d\"disk/by-id/scsi-S$env{SCSI_VENDOR}_$env{SCSI_MODEL}_$env{SCSI_IDENT_SERIAL}\"","commit_id":"dd9dc993e1088baf2d1efd88c21f1accd96adeae"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"b9d1bf105273c8559dd2e0db9124d4f064236b0c","unresolved":true,"context_lines":[{"line_number":259,"context_line":"    \"\"\""},{"line_number":260,"context_line":"    if symlink and symlink.startswith(CUSTOM_LINK_PREFIX):"},{"line_number":261,"context_line":"        ending \u003d symlink[len(CUSTOM_LINK_PREFIX):]"},{"line_number":262,"context_line":"        return ending.replace(\u0027_\u0027, \u0027/\u0027)"},{"line_number":263,"context_line":"    return symlink"},{"line_number":264,"context_line":""},{"line_number":265,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"ba70638a_8fc2e3a2","line":262,"in_reply_to":"bc3f9012_224cb19f","updated":"2022-04-19 17:39:04.000000000","message":"I\u0027m still suspicious. The underscore seems like the first thing I\u0027d reach for if I were to name a device. But apparently we\u0027re lucky in storage. I only see things like\n\n/sys/devices/pci0000:00/0000:00:15.1/i2c_designware.1/i2c-1/i2c-DELL0A86:00/firmware_node\n\nIt is indeed uncommon (not considering leafs).","commit_id":"dd9dc993e1088baf2d1efd88c21f1accd96adeae"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"d3ab6f43dfd8ade319f30df8a304d5a6f2ed179f","unresolved":true,"context_lines":[{"line_number":270,"context_line":"    WARNING: This decorator must be **before** any connect_volume locking"},{"line_number":271,"context_line":"             because it may call disconnect_volume."},{"line_number":272,"context_line":""},{"line_number":273,"context_line":"    Encryptor drivers expect a symlink that they \"own\", so that theey can"},{"line_number":274,"context_line":"    modify it as they want."},{"line_number":275,"context_line":""},{"line_number":276,"context_line":"    The current flow is like this:"}],"source_content_type":"text/x-python","patch_set":2,"id":"130ea6d1_510c3cc8","line":273,"range":{"start_line":273,"start_character":64,"end_line":273,"end_character":69},"updated":"2022-04-14 15:24:52.000000000","message":"\"they\" typo","commit_id":"dd9dc993e1088baf2d1efd88c21f1accd96adeae"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"4bd5fbd93ac6ce9270d4c16e81f7b8075282d55a","unresolved":false,"context_lines":[{"line_number":270,"context_line":"    WARNING: This decorator must be **before** any connect_volume locking"},{"line_number":271,"context_line":"             because it may call disconnect_volume."},{"line_number":272,"context_line":""},{"line_number":273,"context_line":"    Encryptor drivers expect a symlink that they \"own\", so that theey can"},{"line_number":274,"context_line":"    modify it as they want."},{"line_number":275,"context_line":""},{"line_number":276,"context_line":"    The current flow is like this:"}],"source_content_type":"text/x-python","patch_set":2,"id":"d9268b35_d319fdf2","line":273,"range":{"start_line":273,"start_character":64,"end_line":273,"end_character":69},"in_reply_to":"130ea6d1_510c3cc8","updated":"2022-04-19 17:23:41.000000000","message":"Done","commit_id":"dd9dc993e1088baf2d1efd88c21f1accd96adeae"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"64fd159a40f505241306e118159fdddc39d85351","unresolved":true,"context_lines":[{"line_number":298,"context_line":"        device_path \u003d res[\u0027path\u0027]"},{"line_number":299,"context_line":"        # There are connectors that sometimes return file descriptors (rbd)"},{"line_number":300,"context_line":"        if (connection_properties.get(\u0027encrypted\u0027) and"},{"line_number":301,"context_line":"                isinstance(device_path, str)):"},{"line_number":302,"context_line":"            symlink \u003d _symlink_name_from_device_path(device_path)"},{"line_number":303,"context_line":"            try:"},{"line_number":304,"context_line":"                priv_rootwrap.link_root(os.path.realpath(device_path),"}],"source_content_type":"text/x-python","patch_set":2,"id":"8e9a4d49_20f21d1b","line":301,"updated":"2022-04-14 22:51:52.000000000","message":"I have a bad feeling that a connector somewhere will use a bytes string, because os.path and open() on py3 takes both bytes and str.","commit_id":"dd9dc993e1088baf2d1efd88c21f1accd96adeae"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"4bd5fbd93ac6ce9270d4c16e81f7b8075282d55a","unresolved":false,"context_lines":[{"line_number":298,"context_line":"        device_path \u003d res[\u0027path\u0027]"},{"line_number":299,"context_line":"        # There are connectors that sometimes return file descriptors (rbd)"},{"line_number":300,"context_line":"        if (connection_properties.get(\u0027encrypted\u0027) and"},{"line_number":301,"context_line":"                isinstance(device_path, str)):"},{"line_number":302,"context_line":"            symlink \u003d _symlink_name_from_device_path(device_path)"},{"line_number":303,"context_line":"            try:"},{"line_number":304,"context_line":"                priv_rootwrap.link_root(os.path.realpath(device_path),"}],"source_content_type":"text/x-python","patch_set":2,"id":"9b28ee04_670cadf3","line":301,"in_reply_to":"8e9a4d49_20f21d1b","updated":"2022-04-19 17:23:41.000000000","message":"The only place in os-brick where we do a dance with bytes is in the rbd driver (calling convert_str method), and that was done to pass parameters to linuxrbd, not on the device path that is returned.\n\nBut it costs nothing to support both.","commit_id":"dd9dc993e1088baf2d1efd88c21f1accd96adeae"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"fa48492d28a3ae5eedfdeee5de4286c8850f16e1","unresolved":true,"context_lines":[{"line_number":257,"context_line":"    This is the reverse operation of the one performed by the"},{"line_number":258,"context_line":"    _symlink_name_from_device_path method."},{"line_number":259,"context_line":"    \"\"\""},{"line_number":260,"context_line":"    if symlink and symlink.startswith(CUSTOM_LINK_PREFIX):"},{"line_number":261,"context_line":"        ending \u003d symlink[len(CUSTOM_LINK_PREFIX):]"},{"line_number":262,"context_line":"        return ending.replace(\u0027_\u0027, \u0027/\u0027)"},{"line_number":263,"context_line":"    return symlink"}],"source_content_type":"text/x-python","patch_set":3,"id":"2c3e4d9e_d1ccda8e","line":260,"range":{"start_line":260,"start_character":27,"end_line":260,"end_character":37},"updated":"2022-04-20 16:30:46.000000000","message":"-1: We accepted the possibility that a path could be bytes and not str (on line 299), and startswith will fail if symlink is bytes.","commit_id":"c84d82d26d23d6238f3fcaf265d302bb5faa7954"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"502f31076e8155f8b9eb6eac03fef6daad565db1","unresolved":false,"context_lines":[{"line_number":257,"context_line":"    This is the reverse operation of the one performed by the"},{"line_number":258,"context_line":"    _symlink_name_from_device_path method."},{"line_number":259,"context_line":"    \"\"\""},{"line_number":260,"context_line":"    if symlink and symlink.startswith(CUSTOM_LINK_PREFIX):"},{"line_number":261,"context_line":"        ending \u003d symlink[len(CUSTOM_LINK_PREFIX):]"},{"line_number":262,"context_line":"        return ending.replace(\u0027_\u0027, \u0027/\u0027)"},{"line_number":263,"context_line":"    return symlink"}],"source_content_type":"text/x-python","patch_set":3,"id":"38657f8a_0aa06437","line":260,"range":{"start_line":260,"start_character":27,"end_line":260,"end_character":37},"in_reply_to":"2813fa18_13c26eea","updated":"2022-05-11 12:03:51.000000000","message":"This was a self note on a different patchset of an incongruity I had detected. It\u0027s fixed now but I forgot to \"resolve\" the comment.","commit_id":"c84d82d26d23d6238f3fcaf265d302bb5faa7954"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"a4497add1be694a8456303110101442f33f51735","unresolved":true,"context_lines":[{"line_number":257,"context_line":"    This is the reverse operation of the one performed by the"},{"line_number":258,"context_line":"    _symlink_name_from_device_path method."},{"line_number":259,"context_line":"    \"\"\""},{"line_number":260,"context_line":"    if symlink and symlink.startswith(CUSTOM_LINK_PREFIX):"},{"line_number":261,"context_line":"        ending \u003d symlink[len(CUSTOM_LINK_PREFIX):]"},{"line_number":262,"context_line":"        return ending.replace(\u0027_\u0027, \u0027/\u0027)"},{"line_number":263,"context_line":"    return symlink"}],"source_content_type":"text/x-python","patch_set":3,"id":"2813fa18_13c26eea","line":260,"range":{"start_line":260,"start_character":27,"end_line":260,"end_character":37},"in_reply_to":"2c3e4d9e_d1ccda8e","updated":"2022-05-06 20:55:24.000000000","message":"Not sure where/when we \"accepted the possibility that a path could be bytes,\" but isn\u0027t this being handled by judicious use of convert_str()? The comment was made on an earlier patchset, so maybe it\u0027s no longer an issue? Or could you toss another convert_str() at L250 or L251?","commit_id":"c84d82d26d23d6238f3fcaf265d302bb5faa7954"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"093e69b30bdfeff083524dfe3dee44de91dee145","unresolved":true,"context_lines":[{"line_number":367,"context_line":""},{"line_number":368,"context_line":"        res \u003d func(**call_args)"},{"line_number":369,"context_line":"        # Try to clean the symlink, it\u0027s not an error if the link stays"},{"line_number":370,"context_line":"        if custom_symlink:"},{"line_number":371,"context_line":"            try:"},{"line_number":372,"context_line":"                priv_rootwrap.unlink_root(symlink)"},{"line_number":373,"context_line":"            except Exception:"},{"line_number":374,"context_line":"                LOG.warning(\u0027Failed to remove encrypted custom symlink %s\u0027,"},{"line_number":375,"context_line":"                            symlink)"}],"source_content_type":"text/x-python","patch_set":6,"id":"e8545717_0ed7a342","line":372,"range":{"start_line":370,"start_character":0,"end_line":372,"end_character":50},"updated":"2022-04-29 19:03:25.000000000","message":"-1: This will also remove the symlink on extend!!","commit_id":"fd1eecbbbca5a9872654676c65398e71c68cf9e4"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"502f31076e8155f8b9eb6eac03fef6daad565db1","unresolved":false,"context_lines":[{"line_number":367,"context_line":""},{"line_number":368,"context_line":"        res \u003d func(**call_args)"},{"line_number":369,"context_line":"        # Try to clean the symlink, it\u0027s not an error if the link stays"},{"line_number":370,"context_line":"        if custom_symlink:"},{"line_number":371,"context_line":"            try:"},{"line_number":372,"context_line":"                priv_rootwrap.unlink_root(symlink)"},{"line_number":373,"context_line":"            except Exception:"},{"line_number":374,"context_line":"                LOG.warning(\u0027Failed to remove encrypted custom symlink %s\u0027,"},{"line_number":375,"context_line":"                            symlink)"}],"source_content_type":"text/x-python","patch_set":6,"id":"b4ddb816_8e4ba612","line":372,"range":{"start_line":370,"start_character":0,"end_line":372,"end_character":50},"in_reply_to":"e8545717_0ed7a342","updated":"2022-05-11 12:03:51.000000000","message":"Fixed in patchset #8","commit_id":"fd1eecbbbca5a9872654676c65398e71c68cf9e4"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"a4497add1be694a8456303110101442f33f51735","unresolved":true,"context_lines":[{"line_number":367,"context_line":""},{"line_number":368,"context_line":"        res \u003d func(**call_args)"},{"line_number":369,"context_line":"        # Clean symlink only on disconnect, it\u0027s not an error if the link stays"},{"line_number":370,"context_line":"        if custom_symlink and func.__name__ \u003d\u003d \u0027disconnect_volume\u0027:"},{"line_number":371,"context_line":"            try:"},{"line_number":372,"context_line":"                priv_rootwrap.unlink_root(symlink)"},{"line_number":373,"context_line":"            except Exception:"}],"source_content_type":"text/x-python","patch_set":8,"id":"d1ac65ff_9161c76e","line":370,"updated":"2022-05-06 20:55:24.000000000","message":"Ooh! I can\u0027t decide if this is clever or sneaky :-)\n\nI was wonder how extend_volume() could use this decorator. I thought it would prematurely delete the symlink, but I guess this is how you avoid deleting it.\n\nIs there anyway this could be made more explicit? Maybe add a parameter so the caller specifies the behavior?","commit_id":"90f46447cad41fcc1f6a53596a077acbe0203127"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"502f31076e8155f8b9eb6eac03fef6daad565db1","unresolved":false,"context_lines":[{"line_number":367,"context_line":""},{"line_number":368,"context_line":"        res \u003d func(**call_args)"},{"line_number":369,"context_line":"        # Clean symlink only on disconnect, it\u0027s not an error if the link stays"},{"line_number":370,"context_line":"        if custom_symlink and func.__name__ \u003d\u003d \u0027disconnect_volume\u0027:"},{"line_number":371,"context_line":"            try:"},{"line_number":372,"context_line":"                priv_rootwrap.unlink_root(symlink)"},{"line_number":373,"context_line":"            except Exception:"}],"source_content_type":"text/x-python","patch_set":8,"id":"2880229d_8ec3e4dd","line":370,"in_reply_to":"d1ac65ff_9161c76e","updated":"2022-05-11 12:03:51.000000000","message":"Well, in a previous patch I was deleting the symlink  :-(\n\nIt can be done, making the decorator accept parameters, although it will complicate it a bit.  Though given the current complexity I don\u0027t think it will get much worse.","commit_id":"90f46447cad41fcc1f6a53596a077acbe0203127"}],"releasenotes/notes/encryption-a642889a82ff9207.yaml":[{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"855f1bc530ea92eefba0c5c60d86e52236035d21","unresolved":true,"context_lines":[{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    NVMe-oF connector `bug #1964379"},{"line_number":5,"context_line":"    \u003chttps://bugs.launchpad.net/os-brick/+bug/1964379\u003e`_: Fixed using non LUKS"},{"line_number":6,"context_line":"    v1 encrypted volumes, as once of such volumes is disconnected from a host"},{"line_number":7,"context_line":"    all successive NVMe-oF attachments would fail."},{"line_number":8,"context_line":"  - |"},{"line_number":9,"context_line":"    `Bug #1967790"}],"source_content_type":"text/x-yaml","patch_set":9,"id":"6fdebd07_5c3b425a","line":6,"range":{"start_line":6,"start_character":29,"end_line":6,"end_character":36},"updated":"2022-05-23 17:34:04.000000000","message":"\"once one of\"?","commit_id":"2fca258cb9e6296f18e31e4e8c4c863605b80456"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"6dbb6548bcb9b4aeebbb138a341eddfdfbf9737c","unresolved":false,"context_lines":[{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    NVMe-oF connector `bug #1964379"},{"line_number":5,"context_line":"    \u003chttps://bugs.launchpad.net/os-brick/+bug/1964379\u003e`_: Fixed using non LUKS"},{"line_number":6,"context_line":"    v1 encrypted volumes, as once of such volumes is disconnected from a host"},{"line_number":7,"context_line":"    all successive NVMe-oF attachments would fail."},{"line_number":8,"context_line":"  - |"},{"line_number":9,"context_line":"    `Bug #1967790"}],"source_content_type":"text/x-yaml","patch_set":9,"id":"d9ebc2c9_1cb196a5","line":6,"range":{"start_line":6,"start_character":29,"end_line":6,"end_character":36},"in_reply_to":"6fdebd07_5c3b425a","updated":"2022-05-25 08:36:47.000000000","message":"Done","commit_id":"2fca258cb9e6296f18e31e4e8c4c863605b80456"}]}