)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"26ba0bc6f3e86c06165d312d9ce72e74036c5eb8","unresolved":false,"context_lines":[{"line_number":15,"context_line":"refresh_conn_info\u003dTrue, resulting in refreshed connection_info."},{"line_number":16,"context_line":"Calls are updated in _power_on() and reboot_instance()."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"Note: Only hard reboots will currently update connection_info."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"Change-Id: I91d391ba997312aa9ecbad4de65c42fa4c1ced08"},{"line_number":21,"context_line":"Partial-Bug: #1452641"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"3f79a3b5_fbf3a307","line":18,"range":{"start_line":18,"start_character":0,"end_line":18,"end_character":62},"updated":"2018-08-24 16:29:52.000000000","message":"This is misleading - if you mean we only call attachment_update from refresh_connection_info for reboots, that\u0027s not true b/c you\u0027ve changed the behavior of refresh_connection_info to be a PUT rather than GET of the attachment, and there are other flows that call refresh_connection_info (like resize).","commit_id":"0edb2a8ffcfa394afca08d3f4eec37aeff3977ad"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"ad6d039c0f7d60bf8d2ec5f8c8737422e95ddc15","unresolved":false,"context_lines":[{"line_number":15,"context_line":"refresh_conn_info\u003dTrue, resulting in refreshed connection_info."},{"line_number":16,"context_line":"Calls are updated in _power_on() and reboot_instance()."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"Note: Only hard reboots will currently update connection_info."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"Change-Id: I91d391ba997312aa9ecbad4de65c42fa4c1ced08"},{"line_number":21,"context_line":"Partial-Bug: #1452641"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"3f79a3b5_1d727019","line":18,"updated":"2018-11-20 10:26:50.000000000","message":"The code sets the flag for both power_on and reboot so I guess both hard reboot and stop / start will refresh the connection info.","commit_id":"6659d9c15fd4c8d34f68ce26e3e66e174d36d309"}],"nova/compute/manager.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1567c9cacfea8dac90c8420a884c8f48d57e162f","unresolved":false,"context_lines":[{"line_number":3178,"context_line":""},{"line_number":3179,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":3180,"context_line":"            context, instance.uuid)"},{"line_number":3181,"context_line":"        connector \u003d self.driver.get_volume_connector(instance)"},{"line_number":3182,"context_line":"        for bdm in bdms:"},{"line_number":3183,"context_line":"            if bdm.is_volume and bdm.attachment_id is not None:"},{"line_number":3184,"context_line":"            attach_ref \u003d self.volume_api.attachment_update("},{"line_number":3185,"context_line":"                context, bdm.attachment_id, connector\u003dconnector,"},{"line_number":3186,"context_line":"                mountpoint\u003dbdm.device_name)"},{"line_number":3187,"context_line":"            bdm.connection_info \u003d jsonutils.dumps(attach_ref[\u0027connection_info\u0027])"},{"line_number":3188,"context_line":"        block_device_info \u003d self._get_instance_block_device_info("},{"line_number":3189,"context_line":"            context, instance, bdms\u003dbdms)"},{"line_number":3190,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_85aae917","line":3187,"range":{"start_line":3181,"start_character":8,"end_line":3187,"end_character":80},"updated":"2018-06-28 21:08:23.000000000","message":"This should be down in the bowels of refresh_connection_info:\n\nhttps://github.com/openstack/nova/blob/f5d0d85285c894b48c463e3b72646537db90514d/nova/virt/block_device.py#L641\n\nThen you can simply pass the refresh flag to _get_instance_block_device_info and that will do the thing for the new-style attachments (bdms that have attachment_id set). And it would do the proper refresh for old-style attachments (initialize connection calls to the storage backend to get a new connection_info dict).\n\nAlso, what I\u0027m proposing will save the updated bdm.connection_info into the database, something you\u0027re not doing here.\n\nNote that currently today refresh_connection_info doesn\u0027t update the attachment record, it just gets the attachment record and pulls the connection_info off it, which is stored in the cinder database. So it\u0027s not really doing a refresh.\n\nhttps://github.com/openstack/nova/blob/f5d0d85285c894b48c463e3b72646537db90514d/nova/virt/block_device.py#L653","commit_id":"70082a940ee00a674c4ad73beadf6c250ddb2997"}],"nova/tests/fixtures.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"7e333952801a97a592f5077c4a68b15521d39aed","unresolved":false,"context_lines":[{"line_number":1768,"context_line":"                \u0027name\u0027: \u0027lvm-1\u0027"},{"line_number":1769,"context_line":"            }]"},{"line_number":1770,"context_line":""},{"line_number":1771,"context_line":"        def fake_initialize_connection(_self, context, volume_id, connector):"},{"line_number":1772,"context_line":"            return {\u0027data\u0027:"},{"line_number":1773,"context_line":"                    {\u0027foo\u0027: \u0027bar\u0027}}"},{"line_number":1774,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_326959be","line":1771,"range":{"start_line":1771,"start_character":66,"end_line":1771,"end_character":75},"updated":"2018-12-06 22:39:19.000000000","message":"Might not be a bad idea to future proof this with:\n\n*args, **kwargs\n\nFor everything after volume_id.","commit_id":"6659d9c15fd4c8d34f68ce26e3e66e174d36d309"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"7e333952801a97a592f5077c4a68b15521d39aed","unresolved":false,"context_lines":[{"line_number":1768,"context_line":"                \u0027name\u0027: \u0027lvm-1\u0027"},{"line_number":1769,"context_line":"            }]"},{"line_number":1770,"context_line":""},{"line_number":1771,"context_line":"        def fake_initialize_connection(_self, context, volume_id, connector):"},{"line_number":1772,"context_line":"            return {\u0027data\u0027:"},{"line_number":1773,"context_line":"                    {\u0027foo\u0027: \u0027bar\u0027}}"},{"line_number":1774,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_12645de7","line":1771,"range":{"start_line":1771,"start_character":46,"end_line":1771,"end_character":53},"updated":"2018-12-06 22:39:19.000000000","message":"nit: this masks \u0027context\u0027 from imports.","commit_id":"6659d9c15fd4c8d34f68ce26e3e66e174d36d309"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"7e333952801a97a592f5077c4a68b15521d39aed","unresolved":false,"context_lines":[{"line_number":1769,"context_line":"            }]"},{"line_number":1770,"context_line":""},{"line_number":1771,"context_line":"        def fake_initialize_connection(_self, context, volume_id, connector):"},{"line_number":1772,"context_line":"            return {\u0027data\u0027:"},{"line_number":1773,"context_line":"                    {\u0027foo\u0027: \u0027bar\u0027}}"},{"line_number":1774,"context_line":""},{"line_number":1775,"context_line":"        def fake_terminate_connection(_self, context, volume_id, connector):"},{"line_number":1776,"context_line":"            return True"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_526e55cb","line":1773,"range":{"start_line":1772,"start_character":19,"end_line":1773,"end_character":35},"updated":"2018-12-06 22:39:19.000000000","message":"nit: could be on a single line, and I\u0027d make this a tad bit more realistic with:\n\nreturn {\u0027data\u0027: {\u0027serial\u0027: volume_id}}\n\nSee nova.virt.block_device.get_volume_id for why.","commit_id":"6659d9c15fd4c8d34f68ce26e3e66e174d36d309"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"7e333952801a97a592f5077c4a68b15521d39aed","unresolved":false,"context_lines":[{"line_number":1789,"context_line":"                           lambda *args, **kwargs: None)"},{"line_number":1790,"context_line":"        self.test.stub_out(\u0027nova.volume.cinder.API.get\u0027,"},{"line_number":1791,"context_line":"                           fake_get)"},{"line_number":1792,"context_line":"        self.test.stub_out(\u0027nova.volume.cinder.API.initialize_connection\u0027,"},{"line_number":1793,"context_line":"                           fake_initialize_connection)"},{"line_number":1794,"context_line":"        self.test.stub_out(\u0027nova.volume.cinder.API.terminate_connection\u0027,"},{"line_number":1795,"context_line":"                           fake_terminate_connection)"},{"line_number":1796,"context_line":"        self.test.stub_out("},{"line_number":1797,"context_line":"            \u0027nova.volume.cinder.API.migrate_volume_completion\u0027,"},{"line_number":1798,"context_line":"            fake_migrate_volume_completion)"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_5293b5b0","line":1795,"range":{"start_line":1792,"start_character":8,"end_line":1795,"end_character":53},"updated":"2018-12-06 22:39:19.000000000","message":"Hmm, actually, why are these even needed in the *New fixture? The whole point of the bdm.attachment_id (new flow) conditionals is that we don\u0027t need to call these older API methods (we use attachment_create/update and delete instead).","commit_id":"6659d9c15fd4c8d34f68ce26e3e66e174d36d309"}],"nova/virt/block_device.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ef53c9d47dfe463c00fa918cb7b38e1a7abda36a","unresolved":false,"context_lines":[{"line_number":650,"context_line":"                                                               self.volume_id,"},{"line_number":651,"context_line":"                                                               connector)"},{"line_number":652,"context_line":"        else:"},{"line_number":653,"context_line":"            attachment_ref \u003d volume_api.attachment_update("},{"line_number":654,"context_line":"                context, self[\u0027attachment_id\u0027], connector\u003dconnector)"},{"line_number":655,"context_line":"            # The _volume_attach method stashes a \u0027multiattach\u0027 flag in the"},{"line_number":656,"context_line":"            # BlockDeviceMapping.connection_info which is not persisted back"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_fbb643f9","line":653,"range":{"start_line":653,"start_character":40,"end_line":653,"end_character":57},"updated":"2018-08-24 16:37:56.000000000","message":"So we\u0027re getting a lot of test failures, but nothing really obviously failing in the cinder API. However, I want to say I don\u0027t think this is how this API is meant to be used, but would need jgriffith properly to help us. My understanding is rather than update an existing attachment with a new connector (which is how we would have done things in the past when we just had a single volume and no cardinality on the attachments themselves), we should be creating a new attachment with the new host connector and then deleting the old attachment. That would also result in a new attachment_id which we\u0027d need to storage on self so it gets updated in the backing BlockDeviceMapping (see the @update_db decorator).","commit_id":"0edb2a8ffcfa394afca08d3f4eec37aeff3977ad"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"f9fa79ee5ff51411f694f85a39f9628b7c7d1f90","unresolved":false,"context_lines":[{"line_number":650,"context_line":"                                                               self.volume_id,"},{"line_number":651,"context_line":"                                                               connector)"},{"line_number":652,"context_line":"        else:"},{"line_number":653,"context_line":"            attachment_ref \u003d volume_api.attachment_update("},{"line_number":654,"context_line":"                context, self[\u0027attachment_id\u0027], connector\u003dconnector)"},{"line_number":655,"context_line":"            # The _volume_attach method stashes a \u0027multiattach\u0027 flag in the"},{"line_number":656,"context_line":"            # BlockDeviceMapping.connection_info which is not persisted back"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_3b191bd6","line":653,"range":{"start_line":653,"start_character":40,"end_line":653,"end_character":57},"in_reply_to":"3f79a3b5_fbb643f9","updated":"2018-08-24 16:47:41.000000000","message":"Per http://lists.openstack.org/pipermail/openstack-dev/2017-September/122170.html we said we wouldn\u0027t fail the entire operation if this fails:\n\n\"We also said we wouldn\u0027t fail the operation if the refresh fails for whatever reason, like if Cinder fails.\"\n\nIn that case I think it just means if we get a Cinder error of some kind we log it and return so we\u0027re left with the BDM.connection_info we already have and hope it\u0027s correct.","commit_id":"0edb2a8ffcfa394afca08d3f4eec37aeff3977ad"},{"author":{"_account_id":2243,"name":"John Griffith","email":"john.griffith8@gmail.com","username":"john-griffith"},"change_message_id":"fd69392164080f6bbcac8cd61d7c329c8c9fff2a","unresolved":false,"context_lines":[{"line_number":650,"context_line":"                                                               self.volume_id,"},{"line_number":651,"context_line":"                                                               connector)"},{"line_number":652,"context_line":"        else:"},{"line_number":653,"context_line":"            attachment_ref \u003d volume_api.attachment_update("},{"line_number":654,"context_line":"                context, self[\u0027attachment_id\u0027], connector\u003dconnector)"},{"line_number":655,"context_line":"            # The _volume_attach method stashes a \u0027multiattach\u0027 flag in the"},{"line_number":656,"context_line":"            # BlockDeviceMapping.connection_info which is not persisted back"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_fbeac371","line":653,"range":{"start_line":653,"start_character":40,"end_line":653,"end_character":57},"in_reply_to":"3f79a3b5_fbb643f9","updated":"2018-08-24 16:59:05.000000000","message":"You\u0027re 100% correct Matt, the design was in fact to instead make connections sort of \"ephemeral\" and just create a new one.  Update is there as a special case to handle a way of creating an attachment before you\u0027re scheduled to a compute node (so you can pass the connector to it when you actually have the Server scheduled).\n\nThe naming is somewhat unfortunate, \"finalize\" is probably a better name for this method as that\u0027s the primary intent.","commit_id":"0edb2a8ffcfa394afca08d3f4eec37aeff3977ad"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"7e333952801a97a592f5077c4a68b15521d39aed","unresolved":false,"context_lines":[{"line_number":655,"context_line":"                                                               self.volume_id,"},{"line_number":656,"context_line":"                                                               connector)"},{"line_number":657,"context_line":"        else:"},{"line_number":658,"context_line":"            volume_api.terminate_connection(context,"},{"line_number":659,"context_line":"                                            self.volume_id,"},{"line_number":660,"context_line":"                                            connector)"},{"line_number":661,"context_line":"            connection_info \u003d volume_api.initialize_connection(context,"},{"line_number":662,"context_line":"                                                               self.volume_id,"},{"line_number":663,"context_line":"                                                               connector)"},{"line_number":664,"context_line":"            attachment_ref \u003d volume_api.attachment_get(context,"},{"line_number":665,"context_line":"                                                       self[\u0027attachment_id\u0027])"},{"line_number":666,"context_line":"            # The _volume_attach method stashes a \u0027multiattach\u0027 flag in the"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_527c1573","line":663,"range":{"start_line":658,"start_character":12,"end_line":663,"end_character":73},"updated":"2018-12-06 22:39:19.000000000","message":"No this is wrong, we shouldn\u0027t be using these old methods for dealing with attachments when we have an attachment_id. The bdm.attachment_id means we\u0027re doing the new style attachment stuff, so we create/update/delete attachments rather than init/terminate connections.\n\nAs noted earlier:\n\nhttps://review.openstack.org/#/c/579004/2/nova/virt/block_device.py@653\n\nSo I think what you want is something like this:\n\n# Keep track of the existing attachment_id so we can delete it.\nold_attachment_id \u003d self[\u0027attachment_id\u0027]\n# Create a new attachment with the current host connector.\n# This will give us an attachment with fresh connection_info.\nattachment_ref \u003d volume_api.attachment_create(\n    context, self.volume_id, instance.uuid,\n    connector\u003dconnector)\n# Save the attachment_id on this bdm (see @update_db).\nself[\u0027attachment_id\u0027] \u003d attachment_ref[\u0027id\u0027]\n# Now that we\u0027ve got a new attachment, delete the old one.\nvolume_api.attachment_delete(context, old_attachment_id)\n\nAnd I think you might need to also complete the attachment to make sure the volume status is in-use at the end:\n\n  volume_api.attachment_complete(context, attachment_ref[\u0027id\u0027])\n\nIf you\u0027re testing this in devstack, you\u0027ll know attachment_complete is needed if you omit it and the volume status is \u0027reserved\u0027 when you\u0027re done with the operation (reboot would be a good test).","commit_id":"6659d9c15fd4c8d34f68ce26e3e66e174d36d309"},{"author":{"_account_id":2243,"name":"John Griffith","email":"john.griffith8@gmail.com","username":"john-griffith"},"change_message_id":"7ccaa44decece55a31ea6d383231f445ef0dc436","unresolved":false,"context_lines":[{"line_number":655,"context_line":"                                                               self.volume_id,"},{"line_number":656,"context_line":"                                                               connector)"},{"line_number":657,"context_line":"        else:"},{"line_number":658,"context_line":"            volume_api.terminate_connection(context,"},{"line_number":659,"context_line":"                                            self.volume_id,"},{"line_number":660,"context_line":"                                            connector)"},{"line_number":661,"context_line":"            connection_info \u003d volume_api.initialize_connection(context,"},{"line_number":662,"context_line":"                                                               self.volume_id,"},{"line_number":663,"context_line":"                                                               connector)"},{"line_number":664,"context_line":"            attachment_ref \u003d volume_api.attachment_get(context,"},{"line_number":665,"context_line":"                                                       self[\u0027attachment_id\u0027])"},{"line_number":666,"context_line":"            # The _volume_attach method stashes a \u0027multiattach\u0027 flag in the"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_9292ad55","line":663,"range":{"start_line":658,"start_character":12,"end_line":663,"end_character":73},"in_reply_to":"3f79a3b5_527c1573","updated":"2018-12-06 23:05:02.000000000","message":"Yes!  +1 to the notes from Matt.  The whole goal was to avoid re-initializing attachments and instead just create a new one and delete the old one.  This change is perfect for putting the old style attachment code back in, but that would make me very sad.","commit_id":"6659d9c15fd4c8d34f68ce26e3e66e174d36d309"},{"author":{"_account_id":5112,"name":"Seyeong Kim","email":"seyeong.kim@canonical.com","username":"xtrusia"},"change_message_id":"5b07db6abec0bf4574cbafcd999a86fecdfd4546","unresolved":false,"context_lines":[{"line_number":416,"context_line":"            volume_api.detach(context.elevated(), volume_id, instance.uuid,"},{"line_number":417,"context_line":"                              attachment_id)"},{"line_number":418,"context_line":"        else:"},{"line_number":419,"context_line":"            volume_api.attachment_delete(context, attachment_id)"},{"line_number":420,"context_line":""},{"line_number":421,"context_line":"    def detach(self, context, instance, volume_api, virt_driver,"},{"line_number":422,"context_line":"               attachment_id\u003dNone, destroy_bdm\u003dFalse):"}],"source_content_type":"text/x-python","patch_set":9,"id":"ffb9cba7_1e7dfd6d","line":419,"range":{"start_line":419,"start_character":63,"end_line":419,"end_character":64},"updated":"2019-04-21 03:38:57.000000000","message":"Here, error on tempest, because self[\u0027attachment_id\u0027] supposed to be changed after rebooting but it didn\u0027t. \nIn this function, attachment_id is passed as a parameter, but why self[\u0027attachment_id\u0027] is used?","commit_id":"61648ad70ee9ec8dcaa4fb708def2b5e1c863140"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"3c77446b08c8f62e8e4320a6af1d4802ea1ea8ed","unresolved":false,"context_lines":[{"line_number":695,"context_line":"            # Finally complete the attachment to ensure the volume is \u0027in-use\u0027."},{"line_number":696,"context_line":"            volume_api.attachment_complete(context, self[\u0027attachment_id\u0027])"},{"line_number":697,"context_line":""},{"line_number":698,"context_line":"            if bdms:"},{"line_number":699,"context_line":"                for bdm in bdms:"},{"line_number":700,"context_line":"                    if bdm.attachment_id \u003d\u003d old_attachment_id:"},{"line_number":701,"context_line":"                        bdm.attachment_id \u003d self[\u0027attachment_id\u0027]"},{"line_number":702,"context_line":"                        bdm.save()"},{"line_number":703,"context_line":""},{"line_number":704,"context_line":"        else:"},{"line_number":705,"context_line":"            # FIXME(lyarwood): At present in the compute manager"}],"source_content_type":"text/x-python","patch_set":12,"id":"ffb9cba7_dd77c8d3","line":702,"range":{"start_line":698,"start_character":0,"end_line":702,"end_character":34},"updated":"2019-05-01 11:30:25.000000000","message":"We don\u0027t actually need this given the @update_db decorator. I also don\u0027t understand the reason for the conditional here.","commit_id":"8b0f4944d9aaf88f5b3b27ca039fd7687cfb37ec"},{"author":{"_account_id":5112,"name":"Seyeong Kim","email":"seyeong.kim@canonical.com","username":"xtrusia"},"change_message_id":"3ffefc432bed68f2d2e7ade448b8f0a29009560a","unresolved":false,"context_lines":[{"line_number":695,"context_line":"            # Finally complete the attachment to ensure the volume is \u0027in-use\u0027."},{"line_number":696,"context_line":"            volume_api.attachment_complete(context, self[\u0027attachment_id\u0027])"},{"line_number":697,"context_line":""},{"line_number":698,"context_line":"            if bdms:"},{"line_number":699,"context_line":"                for bdm in bdms:"},{"line_number":700,"context_line":"                    if bdm.attachment_id \u003d\u003d old_attachment_id:"},{"line_number":701,"context_line":"                        bdm.attachment_id \u003d self[\u0027attachment_id\u0027]"},{"line_number":702,"context_line":"                        bdm.save()"},{"line_number":703,"context_line":""},{"line_number":704,"context_line":"        else:"},{"line_number":705,"context_line":"            # FIXME(lyarwood): At present in the compute manager"}],"source_content_type":"text/x-python","patch_set":12,"id":"dfbec78f_f9010cb1","line":702,"range":{"start_line":698,"start_character":0,"end_line":702,"end_character":34},"in_reply_to":"ffb9cba7_dd77c8d3","updated":"2019-05-04 03:32:22.000000000","message":"You should be correct but I tested a lot of code and I finally figured out It has old attachment_id in this method. It caused detaching volume fail. Please take a look at this and advice me what to do or something. Thanks.","commit_id":"8b0f4944d9aaf88f5b3b27ca039fd7687cfb37ec"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"3c77446b08c8f62e8e4320a6af1d4802ea1ea8ed","unresolved":false,"context_lines":[{"line_number":707,"context_line":"            # call refresh_connection_info with recreate_attachments\u003dFalse as"},{"line_number":708,"context_line":"            # these flows already handle when and how attachments are"},{"line_number":709,"context_line":"            # recreated. As such for now we grab the existing attachment ref"},{"line_number":710,"context_line":"            # and udpate the connection_info for the BDM. In future all of"},{"line_number":711,"context_line":"            # these flows should have their attachments recreated here."},{"line_number":712,"context_line":"            attachment_ref \u003d volume_api.attachment_get(context, attachment_id)"},{"line_number":713,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"ffb9cba7_5d643822","line":710,"range":{"start_line":710,"start_character":18,"end_line":710,"end_character":24},"updated":"2019-05-01 11:30:25.000000000","message":"update","commit_id":"8b0f4944d9aaf88f5b3b27ca039fd7687cfb37ec"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c7ef78df81a06db28d42959894f9563f8c479f50","unresolved":false,"context_lines":[{"line_number":666,"context_line":"                            virt_driver, do_driver_attach)"},{"line_number":667,"context_line":""},{"line_number":668,"context_line":"    @update_db"},{"line_number":669,"context_line":"    def refresh_connection_info(self, context, instance, volume_api,"},{"line_number":670,"context_line":"                                virt_driver, recreate_attachments\u003dTrue):"},{"line_number":671,"context_line":"        # NOTE (ndipanov): A no-op if there is no connection info already"},{"line_number":672,"context_line":"        if not self[\u0027connection_info\u0027]:"}],"source_content_type":"text/x-python","patch_set":20,"id":"3fa7e38b_b4367e01","line":669,"updated":"2019-11-21 21:13:40.000000000","message":"Gorka said in the ML that the changes in here don\u0027t look correct/safe:\n\nhttp://lists.openstack.org/pipermail/openstack-discuss/2019-October/010185.html\n\n\u003e\n\u003e Also if you can\u0027t refresh the connection info on the same host then a change\n\u003e like this:\n\u003e\n\u003e https://review.opendev.org/#/c/579004/\n\u003e\n\u003e Which does just that - refreshes the connection info doing reboot and start\n\u003e instance operations - would break on those volume drivers if I\u0027m following\n\u003e you.\n\u003e\n\n\"The part related to the new API looks fine, but doing that for the old\ninitialize_connection doesn\u0027t look right to me.\"","commit_id":"3ce560ef89d236419e6340bb254845a0e4c2f32d"},{"author":{"_account_id":11805,"name":"Corey Bryant","email":"corey.bryant@canonical.com","username":"coreycb"},"change_message_id":"74af8b0392e0f2914412c0133e7468d9b0071127","unresolved":false,"context_lines":[{"line_number":666,"context_line":"                            virt_driver, do_driver_attach)"},{"line_number":667,"context_line":""},{"line_number":668,"context_line":"    @update_db"},{"line_number":669,"context_line":"    def refresh_connection_info(self, context, instance, volume_api,"},{"line_number":670,"context_line":"                                virt_driver, recreate_attachments\u003dTrue):"},{"line_number":671,"context_line":"        # NOTE (ndipanov): A no-op if there is no connection info already"},{"line_number":672,"context_line":"        if not self[\u0027connection_info\u0027]:"}],"source_content_type":"text/x-python","patch_set":20,"id":"1f493fa4_8fd74e24","line":669,"in_reply_to":"1f493fa4_0c24ec0c","updated":"2020-04-21 17:59:23.000000000","message":"Got it, thanks for the update Lee. Will keep an eye on the ML.","commit_id":"3ce560ef89d236419e6340bb254845a0e4c2f32d"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"515f00362bc6a9822aaeb7e5227dbf4398b5bdaa","unresolved":false,"context_lines":[{"line_number":666,"context_line":"                            virt_driver, do_driver_attach)"},{"line_number":667,"context_line":""},{"line_number":668,"context_line":"    @update_db"},{"line_number":669,"context_line":"    def refresh_connection_info(self, context, instance, volume_api,"},{"line_number":670,"context_line":"                                virt_driver, recreate_attachments\u003dTrue):"},{"line_number":671,"context_line":"        # NOTE (ndipanov): A no-op if there is no connection info already"},{"line_number":672,"context_line":"        if not self[\u0027connection_info\u0027]:"}],"source_content_type":"text/x-python","patch_set":20,"id":"1f493fa4_0c24ec0c","line":669,"in_reply_to":"1f493fa4_4461c439","updated":"2020-04-21 17:33:20.000000000","message":"Timing, I\u0027ve been approaching this from a different angle for a NFS snapshot issue in Ibbd17dce9763511361cc4236661f834864e13d53 and ran up agains this.\n\nI don\u0027t think we can do this without a new cinder API tbh, even the v3 attachment_update API eventually calls down into the intilaize_connection code path on the c-vol side \n [1][2] hitting the same issues as the pure v2 initialize_connection calls that were raised before.\n\nI\u0027m going to poke the thread to see if any folks want to work on this in V now U is over.\n\n[1] https://github.com/openstack/cinder/blob/99c09bddceca754b550955db7e6786686316d525/cinder/volume/manager.py#L4551-L4554\n[2] https://github.com/openstack/cinder/blob/99c09bddceca754b550955db7e6786686316d525/cinder/volume/manager.py#L4496-L4504","commit_id":"3ce560ef89d236419e6340bb254845a0e4c2f32d"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"f7723f6be4890b4f2a335f28efea908c69b56644","unresolved":false,"context_lines":[{"line_number":666,"context_line":"                            virt_driver, do_driver_attach)"},{"line_number":667,"context_line":""},{"line_number":668,"context_line":"    @update_db"},{"line_number":669,"context_line":"    def refresh_connection_info(self, context, instance, volume_api,"},{"line_number":670,"context_line":"                                virt_driver, recreate_attachments\u003dTrue):"},{"line_number":671,"context_line":"        # NOTE (ndipanov): A no-op if there is no connection info already"},{"line_number":672,"context_line":"        if not self[\u0027connection_info\u0027]:"}],"source_content_type":"text/x-python","patch_set":20,"id":"1f493fa4_74944a45","line":669,"in_reply_to":"1f493fa4_8fd74e24","updated":"2020-04-22 09:21:35.000000000","message":"I\u0027ve had no replies overnight on the ML so I\u0027ll dump my idea here and on the thread shortly.\n\n- Have the c-vol drivers return a flag in their connection_info to indicate when calls to intialize_connection/attachment_update are idempotent. This will allow n-cpu to know here when it\u0027s safe to call intialize_connection/attachment_update to fully refresh the connection_info. Both changes should be backportable.\n\n- For the specific ceph MON update issue here, introduce a backportable workaround configurable that forces a refresh of the connection_info for running instances somehow, either through a periodic job, start up of n-cpu or some other trigger.\n\n- Introduce a new c-api v3 API to refresh an attachments connection_info, essentially do all of the above on the Cinder side and have n-cpu call that instead of checking connection_info for the idempotent flag. This wouldn\u0027t be backportable.\n\n- Introduce a new n-api API to trigger the refresh of a specific attachments connection_info via the above c-api API. This wouldn\u0027t be backportable.\n\n- Not related to this issue but we could then also introduce a new c-api v3 API to migrate from v2 exports to v3 attachments using the above refresh logic, again on the Cinder side.\n\n- Finally remove the idempotent connection_info flag and workaround option.\n\nThoughts?","commit_id":"3ce560ef89d236419e6340bb254845a0e4c2f32d"},{"author":{"_account_id":11805,"name":"Corey Bryant","email":"corey.bryant@canonical.com","username":"coreycb"},"change_message_id":"4b349569f4e249c9b624167fcca51cabd19e5c83","unresolved":false,"context_lines":[{"line_number":666,"context_line":"                            virt_driver, do_driver_attach)"},{"line_number":667,"context_line":""},{"line_number":668,"context_line":"    @update_db"},{"line_number":669,"context_line":"    def refresh_connection_info(self, context, instance, volume_api,"},{"line_number":670,"context_line":"                                virt_driver, recreate_attachments\u003dTrue):"},{"line_number":671,"context_line":"        # NOTE (ndipanov): A no-op if there is no connection info already"},{"line_number":672,"context_line":"        if not self[\u0027connection_info\u0027]:"}],"source_content_type":"text/x-python","patch_set":20,"id":"1f493fa4_4461c439","line":669,"in_reply_to":"3fa7e38b_b4367e01","updated":"2020-04-20 18:26:45.000000000","message":"Any thoughts on fixing this for the new API and leaving the old API path unpatched? Would that be a viable step forward? It seems as if that could at least fix new deployments going forward.","commit_id":"3ce560ef89d236419e6340bb254845a0e4c2f32d"}]}