)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"768958900377a6888e7e62b434d365c2598826e4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":14,"id":"1f1a1f67_f4e516dd","updated":"2017-07-18 16:19:27.000000000","message":"Great commit message. Gives all the context + explanations for the the other side effects of the change","commit_id":"3237b6c593d1b15ae3a2976b9e7c6aee3b5b9d14"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"54a5a32260552d80102acebfb8d180921119a0c6","unresolved":false,"context_lines":[{"line_number":23,"context_line":"updated to avoid an exception thrown by the new code. A fake"},{"line_number":24,"context_line":"instance is now used instead of a simple dict."},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"6 new unit tests are added."},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"Partially Implements: blueprint cinder-new-attach-apis"},{"line_number":29,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":18,"id":"9f5c4f37_3600ef34","line":26,"range":{"start_line":26,"start_character":0,"end_line":26,"end_character":27},"updated":"2017-09-21 19:20:31.000000000","message":"nit: I\u0027d remove this so don\u0027t need to keep updating it as the patch changes.","commit_id":"f576578b2e1050def3ec2995fb4438b27a66aab4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"54a5a32260552d80102acebfb8d180921119a0c6","unresolved":false,"context_lines":[{"line_number":27,"context_line":""},{"line_number":28,"context_line":"Partially Implements: blueprint cinder-new-attach-apis"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"Depends-On: Ia2102f1d47a764819c86972866efcfd1c2be7d0d"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"Change-Id: I0bfb11296430dfffe9b091ae7c3a793617bd9d0d"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":18,"id":"9f5c4f37_16e4cbab","line":30,"updated":"2017-09-21 19:20:31.000000000","message":"This could be removed now.","commit_id":"f576578b2e1050def3ec2995fb4438b27a66aab4"}],"nova/compute/manager.py":[{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"97416813b08486fed0135867a4448bb2c20c7224","unresolved":false,"context_lines":[{"line_number":5529,"context_line":"                else:"},{"line_number":5530,"context_line":"                    # New cinder v3 api flow. Detach all the instance\u0027s"},{"line_number":5531,"context_line":"                    # volumes on the source host."},{"line_number":5532,"context_line":"                    connector \u003d bdm.connection_info.get(\u0027connector\u0027, {})"},{"line_number":5533,"context_line":"                    bdm_host \u003d connector.get(\u0027host\u0027, None)"},{"line_number":5534,"context_line":"                    if bdm_host \u003d\u003d self.host:"},{"line_number":5535,"context_line":"                        self._detach_volume("}],"source_content_type":"text/x-python","patch_set":1,"id":"1f013ff3_746d2a97","line":5532,"updated":"2017-05-15 17:40:18.000000000","message":"bdm.connection_info returns a string, not a dict. I need to convert this to a dict.","commit_id":"6b3cd28633048c934ecf55d11de0244dee558f0a"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"97416813b08486fed0135867a4448bb2c20c7224","unresolved":false,"context_lines":[{"line_number":5796,"context_line":"                if bdm.attachment_id is not None:"},{"line_number":5797,"context_line":"                    # Detach volumes that were attached on destination"},{"line_number":5798,"context_line":"                    # host during pre_live_migration"},{"line_number":5799,"context_line":"                    connector \u003d bdm.connection_info.get(\u0027connector\u0027, {})"},{"line_number":5800,"context_line":"                    bdm_host \u003d connector.get(\u0027host\u0027, None)"},{"line_number":5801,"context_line":"                    if bdm_host \u003d\u003d self.host:"},{"line_number":5802,"context_line":"                        self._detach_volume("}],"source_content_type":"text/x-python","patch_set":1,"id":"1f013ff3_d413de0c","line":5799,"updated":"2017-05-15 17:40:18.000000000","message":"also here.","commit_id":"6b3cd28633048c934ecf55d11de0244dee558f0a"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"f89b8e9b10e0e13272fb5e6f2b0857d553402780","unresolved":false,"context_lines":[{"line_number":4815,"context_line":"            phase\u003dfields.NotificationPhase.START,"},{"line_number":4816,"context_line":"            volume_id\u003dbdm.volume_id)"},{"line_number":4817,"context_line":"        try:"},{"line_number":4818,"context_line":"            bdm.attach(context, instance, self.volume_api, self.driver,"},{"line_number":4819,"context_line":"                       do_driver_attach\u003dTrue)"},{"line_number":4820,"context_line":"        except Exception as e:"},{"line_number":4821,"context_line":"            with excutils.save_and_reraise_exception():"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff0f0b1f_6d142fa1","line":4818,"updated":"2017-05-18 16:31:16.000000000","message":"hmm, isn\u0027t the attachment usually created already at this point in the API, so we should create the attachment in the live-migration code first?","commit_id":"3065d5b4d4d647c9c6d92712265e0e0c0457c7df"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"f1b1e9b2e0f43fea041e7fe529d5ea728c46a0a9","unresolved":false,"context_lines":[{"line_number":4815,"context_line":"            phase\u003dfields.NotificationPhase.START,"},{"line_number":4816,"context_line":"            volume_id\u003dbdm.volume_id)"},{"line_number":4817,"context_line":"        try:"},{"line_number":4818,"context_line":"            bdm.attach(context, instance, self.volume_api, self.driver,"},{"line_number":4819,"context_line":"                       do_driver_attach\u003dTrue)"},{"line_number":4820,"context_line":"        except Exception as e:"},{"line_number":4821,"context_line":"            with excutils.save_and_reraise_exception():"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff0f0b1f_dea155fa","line":4818,"in_reply_to":"ff0f0b1f_6d142fa1","updated":"2017-05-22 12:58:59.000000000","message":"When this is called (on the destination node during pre_live_migrate), there is an attachment on the source host, but not on the dest host. This is what creates the new attachment on the dest host.","commit_id":"3065d5b4d4d647c9c6d92712265e0e0c0457c7df"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"7ba3f53f79aea5c8145aeb6f8d3ad42f5afc410c","unresolved":false,"context_lines":[{"line_number":5292,"context_line":"                new_bdm \u003d self.reserve_block_device_name("},{"line_number":5293,"context_line":"                    context, instance, bdm.device_name, bdm.volume_id,"},{"line_number":5294,"context_line":"                    bdm.disk_bus, bdm.device_type)"},{"line_number":5295,"context_line":"                self.attach_volume(context, instance, new_bdm)"},{"line_number":5296,"context_line":""},{"line_number":5297,"context_line":"        block_device_info \u003d self._get_instance_block_device_info("},{"line_number":5298,"context_line":"                            context, instance, refresh_conn_info\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff0f0b1f_0dbb73e2","line":5295,"updated":"2017-05-18 16:30:05.000000000","message":"this looks good, we have a new and old attachment id here, lets make it explicit.\n\nWere new is for the destination, host. Maybe bdm.attachment_id_destination to save the new attachment generated in this attach_volume code? I guess thats where the new attachment is created?","commit_id":"3065d5b4d4d647c9c6d92712265e0e0c0457c7df"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"ac0a595b40f76e9863bf9b1b0f0314a3a922f9e0","unresolved":false,"context_lines":[{"line_number":5292,"context_line":"                new_bdm \u003d self.reserve_block_device_name("},{"line_number":5293,"context_line":"                    context, instance, bdm.device_name, bdm.volume_id,"},{"line_number":5294,"context_line":"                    bdm.disk_bus, bdm.device_type)"},{"line_number":5295,"context_line":"                self.attach_volume(context, instance, new_bdm)"},{"line_number":5296,"context_line":""},{"line_number":5297,"context_line":"        block_device_info \u003d self._get_instance_block_device_info("},{"line_number":5298,"context_line":"                            context, instance, refresh_conn_info\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff0f0b1f_619b1372","line":5295,"in_reply_to":"ff0f0b1f_0dbb73e2","updated":"2017-05-18 20:18:03.000000000","message":"Hi John, thanks for the comments. The way I interpreted the new v3 flow from the spec, is that during the migration, there will now be 2 bdms for each volume, one for the source and one for the destination. When the migration succeeds, the old source bdms will be destroyed during detach. That way all the source and dest attachment info is available in the bdms during the migration and a new dest attachment id field doesn\u0027t need to exist (which is desirable because it avoids adding new fields to a data structure).\n\nWere you thinking that we would do the new attachment using the original bdm?","commit_id":"3065d5b4d4d647c9c6d92712265e0e0c0457c7df"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"ef9d7005c7928fc5b0a3555898c803efdadb74db","unresolved":false,"context_lines":[{"line_number":5302,"context_line":"                     context, instance, \"live_migration.pre.start\","},{"line_number":5303,"context_line":"                     network_info\u003dnetwork_info)"},{"line_number":5304,"context_line":""},{"line_number":5305,"context_line":"        migrate_data \u003d self.driver.pre_live_migration(context,"},{"line_number":5306,"context_line":"                                       instance,"},{"line_number":5307,"context_line":"                                       block_device_info,"},{"line_number":5308,"context_line":"                                       network_info,"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff0f0b1f_ade487ad","line":5305,"updated":"2017-05-18 16:34:54.000000000","message":"oh, wait, does the driver attach currently happen inside this code, not sure if self.attach_volume already does that... attach_volume is probably the wrong thing to call on line 5295, maybe we only need to create the attachment there?","commit_id":"3065d5b4d4d647c9c6d92712265e0e0c0457c7df"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"7ba3f53f79aea5c8145aeb6f8d3ad42f5afc410c","unresolved":false,"context_lines":[{"line_number":5559,"context_line":"                    if bdm_host \u003d\u003d self.host:"},{"line_number":5560,"context_line":"                        self._detach_volume("},{"line_number":5561,"context_line":"                            ctxt, bdm.volume_id, instance, destroy_bdm\u003dTrue,"},{"line_number":5562,"context_line":"                            attachment_id\u003dbdm.attachment_id)"},{"line_number":5563,"context_line":""},{"line_number":5564,"context_line":"        # Releasing vlan."},{"line_number":5565,"context_line":"        # (not necessary in current implementation?)"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff0f0b1f_2db2b7e7","line":5562,"updated":"2017-05-18 16:30:05.000000000","message":"So this is the happy path, i.e. detach on the source, so this is the old_attachment_id we should use here.\n\nMaybe this is where we update the bdm.attachment_id \u003d bdm.attachment_id_destionation and bdm.attachment_id_destionation \u003d None?","commit_id":"3065d5b4d4d647c9c6d92712265e0e0c0457c7df"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"7ba3f53f79aea5c8145aeb6f8d3ad42f5afc410c","unresolved":false,"context_lines":[{"line_number":5830,"context_line":"                    if bdm_host \u003d\u003d self.host:"},{"line_number":5831,"context_line":"                        self._detach_volume("},{"line_number":5832,"context_line":"                            context, bdm.volume_id, instance, destroy_bdm\u003dTrue,"},{"line_number":5833,"context_line":"                            attachment_id\u003dbdm.attachment_id)"},{"line_number":5834,"context_line":""},{"line_number":5835,"context_line":"        self._notify_about_instance_usage("},{"line_number":5836,"context_line":"                        context, instance, \"live_migration.rollback.dest.end\","}],"source_content_type":"text/x-python","patch_set":2,"id":"ff0f0b1f_0d2db342","line":5833,"updated":"2017-05-18 16:30:05.000000000","message":"So here we are detaching the volume on the destination host, so it needs to be the \"new\" attachment_id, maybe bdm.attachment_id_destination?","commit_id":"3065d5b4d4d647c9c6d92712265e0e0c0457c7df"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"ac0a595b40f76e9863bf9b1b0f0314a3a922f9e0","unresolved":false,"context_lines":[{"line_number":5830,"context_line":"                    if bdm_host \u003d\u003d self.host:"},{"line_number":5831,"context_line":"                        self._detach_volume("},{"line_number":5832,"context_line":"                            context, bdm.volume_id, instance, destroy_bdm\u003dTrue,"},{"line_number":5833,"context_line":"                            attachment_id\u003dbdm.attachment_id)"},{"line_number":5834,"context_line":""},{"line_number":5835,"context_line":"        self._notify_about_instance_usage("},{"line_number":5836,"context_line":"                        context, instance, \"live_migration.rollback.dest.end\","}],"source_content_type":"text/x-python","patch_set":2,"id":"ff0f0b1f_fed0e752","line":5833,"in_reply_to":"ff0f0b1f_0d2db342","updated":"2017-05-18 20:18:03.000000000","message":"this bdm is specific to the destination host (bdm_host \u003d\u003d self.host \u003d\u003d destination host), and since it is on the destination host it already contains the new attachment id. that\u0027s why i don\u0027t think we need to save the destination attach id anywhere, since it\u0027s already in the bdm. so i think it\u0027s ok logically as it is.\n\nof course this is new code for me, so i could be missing something.","commit_id":"3065d5b4d4d647c9c6d92712265e0e0c0457c7df"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"811b213d28b1b7f204669c696da19cf634241b64","unresolved":false,"context_lines":[{"line_number":5754,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":5755,"context_line":"                context, instance.uuid)"},{"line_number":5756,"context_line":"        for bdm in bdms:"},{"line_number":5757,"context_line":"            if bdm.is_volume:"},{"line_number":5758,"context_line":"                self.compute_rpcapi.remove_volume_connection("},{"line_number":5759,"context_line":"                        context, instance, bdm.volume_id, dest)"},{"line_number":5760,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"ff0f0b1f_48f50e56","line":5757,"updated":"2017-05-25 14:56:08.000000000","message":"this needs to be updated for the cinder v3 flow:\n\nif bdm.is_volume and bdm.attachment_is is None:","commit_id":"a7d115d99dbf24ed68c0a390fd9bf56e88a8d2b8"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"2272d2f1ebe8af3bf929efdb8317f85a2fd36c90","unresolved":false,"context_lines":[{"line_number":5754,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":5755,"context_line":"                context, instance.uuid)"},{"line_number":5756,"context_line":"        for bdm in bdms:"},{"line_number":5757,"context_line":"            if bdm.is_volume:"},{"line_number":5758,"context_line":"                self.compute_rpcapi.remove_volume_connection("},{"line_number":5759,"context_line":"                        context, instance, bdm.volume_id, dest)"},{"line_number":5760,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"ff0f0b1f_974b3b5e","line":5757,"in_reply_to":"ff0f0b1f_48f50e56","updated":"2017-05-25 20:18:30.000000000","message":"Done","commit_id":"a7d115d99dbf24ed68c0a390fd9bf56e88a8d2b8"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"811b213d28b1b7f204669c696da19cf634241b64","unresolved":false,"context_lines":[{"line_number":5755,"context_line":"                context, instance.uuid)"},{"line_number":5756,"context_line":"        for bdm in bdms:"},{"line_number":5757,"context_line":"            if bdm.is_volume:"},{"line_number":5758,"context_line":"                self.compute_rpcapi.remove_volume_connection("},{"line_number":5759,"context_line":"                        context, instance, bdm.volume_id, dest)"},{"line_number":5760,"context_line":""},{"line_number":5761,"context_line":"        self._notify_about_instance_usage(context, instance,"}],"source_content_type":"text/x-python","patch_set":3,"id":"ff0f0b1f_08a03641","line":5758,"updated":"2017-05-25 14:56:08.000000000","message":"this call is broken. this rpcapi method is:\n\ndef remove_volume_connection(self, ctxt, instance, volume_id, host):\n\nThough, it may be better that it\u0027s broken, why would you want to remove a volume connection on the source host here?","commit_id":"a7d115d99dbf24ed68c0a390fd9bf56e88a8d2b8"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"92931102f2336b7cb671908f1e6f42093ece717c","unresolved":false,"context_lines":[{"line_number":5755,"context_line":"                context, instance.uuid)"},{"line_number":5756,"context_line":"        for bdm in bdms:"},{"line_number":5757,"context_line":"            if bdm.is_volume:"},{"line_number":5758,"context_line":"                self.compute_rpcapi.remove_volume_connection("},{"line_number":5759,"context_line":"                        context, instance, bdm.volume_id, dest)"},{"line_number":5760,"context_line":""},{"line_number":5761,"context_line":"        self._notify_about_instance_usage(context, instance,"}],"source_content_type":"text/x-python","patch_set":3,"id":"ff0f0b1f_c361fcd5","line":5758,"in_reply_to":"ff0f0b1f_08a03641","updated":"2017-05-25 16:30:32.000000000","message":"I don\u0027t understand your point. The args aren\u0027t wrong. This is the source compute making an RPC call to the dest compute and telling it to remove it\u0027s volume connections (from the dest host) as part of the rollback. It\u0027s cleaning up the dest host in other words.","commit_id":"a7d115d99dbf24ed68c0a390fd9bf56e88a8d2b8"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"6d6bc81f20209317f4d77bdaa9958cf8149f02ab","unresolved":false,"context_lines":[{"line_number":5755,"context_line":"                context, instance.uuid)"},{"line_number":5756,"context_line":"        for bdm in bdms:"},{"line_number":5757,"context_line":"            if bdm.is_volume:"},{"line_number":5758,"context_line":"                self.compute_rpcapi.remove_volume_connection("},{"line_number":5759,"context_line":"                        context, instance, bdm.volume_id, dest)"},{"line_number":5760,"context_line":""},{"line_number":5761,"context_line":"        self._notify_about_instance_usage(context, instance,"}],"source_content_type":"text/x-python","patch_set":3,"id":"df140735_b60def3f","line":5758,"in_reply_to":"ff0f0b1f_c361fcd5","updated":"2017-05-30 21:54:48.000000000","message":"Matt, I finally got back to this. I had flagged the wrong method here. The problem is actually in remove_volume_connection calling detach_volume.\n\nI created a bug here - https://bugs.launchpad.net/nova/+bug/1694535","commit_id":"a7d115d99dbf24ed68c0a390fd9bf56e88a8d2b8"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"221072401f8b0b0468e4fe63bc2373ffc2199497","unresolved":false,"context_lines":[{"line_number":5755,"context_line":"                context, instance.uuid)"},{"line_number":5756,"context_line":"        for bdm in bdms:"},{"line_number":5757,"context_line":"            if bdm.is_volume:"},{"line_number":5758,"context_line":"                self.compute_rpcapi.remove_volume_connection("},{"line_number":5759,"context_line":"                        context, instance, bdm.volume_id, dest)"},{"line_number":5760,"context_line":""},{"line_number":5761,"context_line":"        self._notify_about_instance_usage(context, instance,"}],"source_content_type":"text/x-python","patch_set":3,"id":"ff0f0b1f_8f74459e","line":5758,"in_reply_to":"ff0f0b1f_c361fcd5","updated":"2017-05-25 18:38:30.000000000","message":"not sure what happened here. I agree this is fine. somehow, other args crept into my migrate source tree. too many patches...","commit_id":"a7d115d99dbf24ed68c0a390fd9bf56e88a8d2b8"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1889f63d0dd9a874fecbe18b12ad90ee9ff9cd27","unresolved":false,"context_lines":[{"line_number":4961,"context_line":"            action\u003dfields.NotificationAction.VOLUME_DETACH,"},{"line_number":4962,"context_line":"            phase\u003dfields.NotificationPhase.START,"},{"line_number":4963,"context_line":"            volume_id\u003dvolume_id)"},{"line_number":4964,"context_line":"        bdm \u003d objects.BlockDeviceMapping.get_by_volume_and_instance("},{"line_number":4965,"context_line":"                context, volume_id, instance.uuid)"},{"line_number":4966,"context_line":""},{"line_number":4967,"context_line":"        self._notify_volume_usage_detach(context, instance, bdm)"},{"line_number":4968,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"5f201791_2a41790c","line":4965,"range":{"start_line":4964,"start_character":8,"end_line":4965,"end_character":50},"updated":"2017-06-21 19:02:49.000000000","message":"It sucks that we look this up again even when we had the bdm in scope already from the calling code. I think I\u0027m going to push a separate change to clean this up.","commit_id":"373b01d5d432bbb0c2ddfb71a79d5129c1775b96"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"24502abfbd93e958bf4d2603774f36d8d0f8a32b","unresolved":false,"context_lines":[{"line_number":4961,"context_line":"            action\u003dfields.NotificationAction.VOLUME_DETACH,"},{"line_number":4962,"context_line":"            phase\u003dfields.NotificationPhase.START,"},{"line_number":4963,"context_line":"            volume_id\u003dvolume_id)"},{"line_number":4964,"context_line":"        bdm \u003d objects.BlockDeviceMapping.get_by_volume_and_instance("},{"line_number":4965,"context_line":"                context, volume_id, instance.uuid)"},{"line_number":4966,"context_line":""},{"line_number":4967,"context_line":"        self._notify_volume_usage_detach(context, instance, bdm)"},{"line_number":4968,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"5f201791_8df0a792","line":4965,"range":{"start_line":4964,"start_character":8,"end_line":4965,"end_character":50},"in_reply_to":"5f201791_2a41790c","updated":"2017-06-21 19:56:17.000000000","message":"https://review.openstack.org/476258","commit_id":"373b01d5d432bbb0c2ddfb71a79d5129c1775b96"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1889f63d0dd9a874fecbe18b12ad90ee9ff9cd27","unresolved":false,"context_lines":[{"line_number":5358,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":5359,"context_line":"            context, instance.uuid)"},{"line_number":5360,"context_line":"        for bdm in bdms:"},{"line_number":5361,"context_line":"            if bdm.is_volume and bdm.attachment_id is not None:"},{"line_number":5362,"context_line":"                # This bdm uses the new cinder v3 APIs."},{"line_number":5363,"context_line":"                # We will create a new attachment for this"},{"line_number":5364,"context_line":"                # volume on this migration destination host. The old"}],"source_content_type":"text/x-python","patch_set":8,"id":"5f201791_a791449c","line":5361,"range":{"start_line":5361,"start_character":15,"end_line":5361,"end_character":28},"updated":"2017-06-21 19:02:49.000000000","message":"Can we have BDMs that aren\u0027t volumes but that have an attachment_id? I would think that checking for attachment_id is enough. The only alternative is the bdm\u0027s destination_type is \u0027local\u0027 and for those types of BDMs I\u0027m pretty sure we don\u0027t have an attachment with Cinder as they are local swap or ephemeral block devices.\n\nHowever, it\u0027s probably fine to leave this. It shouldn\u0027t hurt anything and it\u0027s a safeguard in case dest_type\u003d\u0027local\u0027 has some case I\u0027m not aware of.","commit_id":"373b01d5d432bbb0c2ddfb71a79d5129c1775b96"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"3f4ba5fa25a33b8f119e1b43fb92f46c6a614913","unresolved":false,"context_lines":[{"line_number":5358,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":5359,"context_line":"            context, instance.uuid)"},{"line_number":5360,"context_line":"        for bdm in bdms:"},{"line_number":5361,"context_line":"            if bdm.is_volume and bdm.attachment_id is not None:"},{"line_number":5362,"context_line":"                # This bdm uses the new cinder v3 APIs."},{"line_number":5363,"context_line":"                # We will create a new attachment for this"},{"line_number":5364,"context_line":"                # volume on this migration destination host. The old"}],"source_content_type":"text/x-python","patch_set":8,"id":"5f201791_5b2ec943","line":5361,"range":{"start_line":5361,"start_character":15,"end_line":5361,"end_character":28},"in_reply_to":"5f201791_a791449c","updated":"2017-06-22 21:49:26.000000000","message":"ok, I\u0027ll leave this as is.","commit_id":"373b01d5d432bbb0c2ddfb71a79d5129c1775b96"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1889f63d0dd9a874fecbe18b12ad90ee9ff9cd27","unresolved":false,"context_lines":[{"line_number":5359,"context_line":"            context, instance.uuid)"},{"line_number":5360,"context_line":"        for bdm in bdms:"},{"line_number":5361,"context_line":"            if bdm.is_volume and bdm.attachment_id is not None:"},{"line_number":5362,"context_line":"                # This bdm uses the new cinder v3 APIs."},{"line_number":5363,"context_line":"                # We will create a new attachment for this"},{"line_number":5364,"context_line":"                # volume on this migration destination host. The old"},{"line_number":5365,"context_line":"                # attachment will be deleted on the source host"}],"source_content_type":"text/x-python","patch_set":8,"id":"5f201791_6737acb2","line":5362,"range":{"start_line":5362,"start_character":47,"end_line":5362,"end_character":49},"updated":"2017-06-21 19:02:49.000000000","message":"Let\u0027s say 3.27 to be more specific. The v3.0 API is backward compatible with the v2 API and won\u0027t have the attachment_id.","commit_id":"373b01d5d432bbb0c2ddfb71a79d5129c1775b96"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"3f4ba5fa25a33b8f119e1b43fb92f46c6a614913","unresolved":false,"context_lines":[{"line_number":5359,"context_line":"            context, instance.uuid)"},{"line_number":5360,"context_line":"        for bdm in bdms:"},{"line_number":5361,"context_line":"            if bdm.is_volume and bdm.attachment_id is not None:"},{"line_number":5362,"context_line":"                # This bdm uses the new cinder v3 APIs."},{"line_number":5363,"context_line":"                # We will create a new attachment for this"},{"line_number":5364,"context_line":"                # volume on this migration destination host. The old"},{"line_number":5365,"context_line":"                # attachment will be deleted on the source host"}],"source_content_type":"text/x-python","patch_set":8,"id":"5f201791_4d516f36","line":5362,"range":{"start_line":5362,"start_character":47,"end_line":5362,"end_character":49},"in_reply_to":"5f201791_6737acb2","updated":"2017-06-22 21:49:26.000000000","message":"Done","commit_id":"373b01d5d432bbb0c2ddfb71a79d5129c1775b96"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1889f63d0dd9a874fecbe18b12ad90ee9ff9cd27","unresolved":false,"context_lines":[{"line_number":5364,"context_line":"                # volume on this migration destination host. The old"},{"line_number":5365,"context_line":"                # attachment will be deleted on the source host"},{"line_number":5366,"context_line":"                # when the migration succeeds."},{"line_number":5367,"context_line":"                attach_ref \u003d self.volume_api.attachment_create("},{"line_number":5368,"context_line":"                    context, bdm.volume_id, bdm.instance_uuid)"},{"line_number":5369,"context_line":""},{"line_number":5370,"context_line":"                # save current attachments, in case they\u0027re needed later"}],"source_content_type":"text/x-python","patch_set":8,"id":"5f201791_4767104f","line":5367,"range":{"start_line":5367,"start_character":16,"end_line":5367,"end_character":26},"updated":"2017-06-21 19:02:49.000000000","message":"Where do we eventually update this with the new connector on the dest host?","commit_id":"373b01d5d432bbb0c2ddfb71a79d5129c1775b96"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"3f4ba5fa25a33b8f119e1b43fb92f46c6a614913","unresolved":false,"context_lines":[{"line_number":5364,"context_line":"                # volume on this migration destination host. The old"},{"line_number":5365,"context_line":"                # attachment will be deleted on the source host"},{"line_number":5366,"context_line":"                # when the migration succeeds."},{"line_number":5367,"context_line":"                attach_ref \u003d self.volume_api.attachment_create("},{"line_number":5368,"context_line":"                    context, bdm.volume_id, bdm.instance_uuid)"},{"line_number":5369,"context_line":""},{"line_number":5370,"context_line":"                # save current attachments, in case they\u0027re needed later"}],"source_content_type":"text/x-python","patch_set":8,"id":"5f201791_db6c1938","line":5367,"range":{"start_line":5367,"start_character":16,"end_line":5367,"end_character":26},"in_reply_to":"5f201791_4767104f","updated":"2017-06-22 21:49:26.000000000","message":"It happens down in block_device.py. Cinder is updated via attachment_update (this is in JGriffith\u0027s POC changes):\n\n  /mnt/share/stack/nova/nova/compute/manager.py(5356)pre_live_migration()\n-\u003e context, instance, refresh_conn_info\u003dTrue)\n  /mnt/share/stack/nova/nova/compute/manager.py(1685)_get_instance_block_device_info()\n-\u003e context, instance, self.volume_api, self.driver)\n  /mnt/share/stack/nova/nova/virt/block_device.py(740)refresh_conn_infos()\n-\u003e device.refresh_connection_info(*refresh_args, **refresh_kwargs)\n  /mnt/share/stack/nova/nova/virt/block_device.py(45)wrapped()\n-\u003e ret_val \u003d method(obj, context, *args, **kwargs)\n  /mnt/share/stack/nova/nova/virt/block_device.py(551)refresh_connection_info()\n-\u003e context, connector, volume_api, instance, virt_driver)\n\u003e /mnt/share/stack/nova/nova/virt/block_device.py(533)_get_connection_updates()\n-\u003e return volume_api.attachment_update(context, self[\u0027attachment_id\u0027], connector)","commit_id":"373b01d5d432bbb0c2ddfb71a79d5129c1775b96"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1889f63d0dd9a874fecbe18b12ad90ee9ff9cd27","unresolved":false,"context_lines":[{"line_number":5369,"context_line":""},{"line_number":5370,"context_line":"                # save current attachments, in case they\u0027re needed later"},{"line_number":5371,"context_line":"                # on a rollback."},{"line_number":5372,"context_line":"                old_attachments[str(bdm.id)] \u003d bdm.attachment_id"},{"line_number":5373,"context_line":""},{"line_number":5374,"context_line":"                # update the bdm (in 2 places) with the new attachment_id"},{"line_number":5375,"context_line":"                bdm.attachment_id \u003d attach_ref[\u0027id\u0027]"}],"source_content_type":"text/x-python","patch_set":8,"id":"5f201791_275ad4f2","line":5372,"range":{"start_line":5372,"start_character":32,"end_line":5372,"end_character":43},"updated":"2017-06-21 19:02:49.000000000","message":"I don\u0027t think we want to key off the bdm.id, which is the internal primary key for the block_device_mapping table record, and is not unique across cells. If you really need to map with something, use the bdm.volume_id? But since the migration is scoped to a single instance and the bdms in the migrate data are going to be for the volumes attached to that instance, can\u0027t we just make old_attachments a list of attachment uuids?","commit_id":"373b01d5d432bbb0c2ddfb71a79d5129c1775b96"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"3f4ba5fa25a33b8f119e1b43fb92f46c6a614913","unresolved":false,"context_lines":[{"line_number":5369,"context_line":""},{"line_number":5370,"context_line":"                # save current attachments, in case they\u0027re needed later"},{"line_number":5371,"context_line":"                # on a rollback."},{"line_number":5372,"context_line":"                old_attachments[str(bdm.id)] \u003d bdm.attachment_id"},{"line_number":5373,"context_line":""},{"line_number":5374,"context_line":"                # update the bdm (in 2 places) with the new attachment_id"},{"line_number":5375,"context_line":"                bdm.attachment_id \u003d attach_ref[\u0027id\u0027]"}],"source_content_type":"text/x-python","patch_set":8,"id":"5f201791_b0c97c79","line":5372,"range":{"start_line":5372,"start_character":32,"end_line":5372,"end_character":43},"in_reply_to":"5f201791_275ad4f2","updated":"2017-06-22 21:49:26.000000000","message":"A list by itself is not enough, because in rollback you need to know which bdm is to get which old attachment_id. I originally used the volume_id here as the key, but saw that nova doesn\u0027t prevent you from attaching the same volume multiple times to an instance. Thus, on rollback, using volume_id as a key, attachment_ids could get switched between bdms. I don\u0027t know if that would cause a problem. (btw, are multiple attachments of a volume to the same instance a bug? or is there some reason to allow that?)\n\nI\u0027ll change the dict to use the device name. That should be unique for each attachment and would cover the case of the same volume being attached multiple times to the instance","commit_id":"373b01d5d432bbb0c2ddfb71a79d5129c1775b96"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"41f620f771552715ace75d58e11fd6b4ddc79092","unresolved":false,"context_lines":[{"line_number":5369,"context_line":""},{"line_number":5370,"context_line":"                # save current attachments, in case they\u0027re needed later"},{"line_number":5371,"context_line":"                # on a rollback."},{"line_number":5372,"context_line":"                old_attachments[str(bdm.id)] \u003d bdm.attachment_id"},{"line_number":5373,"context_line":""},{"line_number":5374,"context_line":"                # update the bdm (in 2 places) with the new attachment_id"},{"line_number":5375,"context_line":"                bdm.attachment_id \u003d attach_ref[\u0027id\u0027]"}],"source_content_type":"text/x-python","patch_set":8,"id":"5f201791_ea91189b","line":5372,"range":{"start_line":5372,"start_character":32,"end_line":5372,"end_character":43},"in_reply_to":"5f201791_b0c97c79","updated":"2017-06-22 21:51:05.000000000","message":"that was an early comment. The new change will use volume_id as the key.","commit_id":"373b01d5d432bbb0c2ddfb71a79d5129c1775b96"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1889f63d0dd9a874fecbe18b12ad90ee9ff9cd27","unresolved":false,"context_lines":[{"line_number":5373,"context_line":""},{"line_number":5374,"context_line":"                # update the bdm (in 2 places) with the new attachment_id"},{"line_number":5375,"context_line":"                bdm.attachment_id \u003d attach_ref[\u0027id\u0027]"},{"line_number":5376,"context_line":"                connection_info \u003d jsonutils.loads(bdm.connection_info)"},{"line_number":5377,"context_line":"                connection_info[\u0027attachment_id\u0027] \u003d bdm.attachment_id"},{"line_number":5378,"context_line":"                bdm.connection_info \u003d jsonutils.dumps(connection_info)"},{"line_number":5379,"context_line":"                bdm.save()"},{"line_number":5380,"context_line":""},{"line_number":5381,"context_line":"        migrate_data.old_attachments \u003d old_attachments"}],"source_content_type":"text/x-python","patch_set":8,"id":"5f201791_e73e1c28","line":5378,"range":{"start_line":5376,"start_character":16,"end_line":5378,"end_character":70},"updated":"2017-06-21 19:02:49.000000000","message":"Why do we need this if we can just check the bdm.attachment_id?","commit_id":"373b01d5d432bbb0c2ddfb71a79d5129c1775b96"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"3f4ba5fa25a33b8f119e1b43fb92f46c6a614913","unresolved":false,"context_lines":[{"line_number":5373,"context_line":""},{"line_number":5374,"context_line":"                # update the bdm (in 2 places) with the new attachment_id"},{"line_number":5375,"context_line":"                bdm.attachment_id \u003d attach_ref[\u0027id\u0027]"},{"line_number":5376,"context_line":"                connection_info \u003d jsonutils.loads(bdm.connection_info)"},{"line_number":5377,"context_line":"                connection_info[\u0027attachment_id\u0027] \u003d bdm.attachment_id"},{"line_number":5378,"context_line":"                bdm.connection_info \u003d jsonutils.dumps(connection_info)"},{"line_number":5379,"context_line":"                bdm.save()"},{"line_number":5380,"context_line":""},{"line_number":5381,"context_line":"        migrate_data.old_attachments \u003d old_attachments"}],"source_content_type":"text/x-python","patch_set":8,"id":"5f201791_fff8ca49","line":5378,"range":{"start_line":5376,"start_character":16,"end_line":5378,"end_character":70},"in_reply_to":"5f201791_e73e1c28","updated":"2017-06-22 21:49:26.000000000","message":"OK, after reading all your comments, I now understand that you want nova to ignore the attachment_id in connection_info. I\u0027ll add a comment about this.","commit_id":"373b01d5d432bbb0c2ddfb71a79d5129c1775b96"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1889f63d0dd9a874fecbe18b12ad90ee9ff9cd27","unresolved":false,"context_lines":[{"line_number":5626,"context_line":"        for bdm in bdms:"},{"line_number":5627,"context_line":"            if bdm.is_volume:"},{"line_number":5628,"context_line":"                if bdm.attachment_id is None:"},{"line_number":5629,"context_line":"                    # NOTE(vish): Prior to cinder v3,"},{"line_number":5630,"context_line":"                    #             we don\u0027t want to actually mark the volume"},{"line_number":5631,"context_line":"                    #             detached, or delete the bdm, just remove the"},{"line_number":5632,"context_line":"                    #             connection from this host."}],"source_content_type":"text/x-python","patch_set":8,"id":"5f201791_07bd9864","line":5629,"range":{"start_line":5629,"start_character":50,"end_line":5629,"end_character":52},"updated":"2017-06-21 19:02:49.000000000","message":"3.27","commit_id":"373b01d5d432bbb0c2ddfb71a79d5129c1775b96"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1889f63d0dd9a874fecbe18b12ad90ee9ff9cd27","unresolved":false,"context_lines":[{"line_number":5626,"context_line":"        for bdm in bdms:"},{"line_number":5627,"context_line":"            if bdm.is_volume:"},{"line_number":5628,"context_line":"                if bdm.attachment_id is None:"},{"line_number":5629,"context_line":"                    # NOTE(vish): Prior to cinder v3,"},{"line_number":5630,"context_line":"                    #             we don\u0027t want to actually mark the volume"},{"line_number":5631,"context_line":"                    #             detached, or delete the bdm, just remove the"},{"line_number":5632,"context_line":"                    #             connection from this host."}],"source_content_type":"text/x-python","patch_set":8,"id":"5f201791_67be8c66","line":5629,"range":{"start_line":5629,"start_character":22,"end_line":5629,"end_character":32},"updated":"2017-06-21 19:02:49.000000000","message":"I\u0027d remove this old NOTE marker from vish so someone doesn\u0027t think vishy is still working on this.","commit_id":"373b01d5d432bbb0c2ddfb71a79d5129c1775b96"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"3f4ba5fa25a33b8f119e1b43fb92f46c6a614913","unresolved":false,"context_lines":[{"line_number":5626,"context_line":"        for bdm in bdms:"},{"line_number":5627,"context_line":"            if bdm.is_volume:"},{"line_number":5628,"context_line":"                if bdm.attachment_id is None:"},{"line_number":5629,"context_line":"                    # NOTE(vish): Prior to cinder v3,"},{"line_number":5630,"context_line":"                    #             we don\u0027t want to actually mark the volume"},{"line_number":5631,"context_line":"                    #             detached, or delete the bdm, just remove the"},{"line_number":5632,"context_line":"                    #             connection from this host."}],"source_content_type":"text/x-python","patch_set":8,"id":"5f201791_90744037","line":5629,"range":{"start_line":5629,"start_character":50,"end_line":5629,"end_character":52},"in_reply_to":"5f201791_07bd9864","updated":"2017-06-22 21:49:26.000000000","message":"Done","commit_id":"373b01d5d432bbb0c2ddfb71a79d5129c1775b96"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"3f4ba5fa25a33b8f119e1b43fb92f46c6a614913","unresolved":false,"context_lines":[{"line_number":5626,"context_line":"        for bdm in bdms:"},{"line_number":5627,"context_line":"            if bdm.is_volume:"},{"line_number":5628,"context_line":"                if bdm.attachment_id is None:"},{"line_number":5629,"context_line":"                    # NOTE(vish): Prior to cinder v3,"},{"line_number":5630,"context_line":"                    #             we don\u0027t want to actually mark the volume"},{"line_number":5631,"context_line":"                    #             detached, or delete the bdm, just remove the"},{"line_number":5632,"context_line":"                    #             connection from this host."}],"source_content_type":"text/x-python","patch_set":8,"id":"5f201791_b0773c3b","line":5629,"range":{"start_line":5629,"start_character":22,"end_line":5629,"end_character":32},"in_reply_to":"5f201791_67be8c66","updated":"2017-06-22 21:49:26.000000000","message":"Done","commit_id":"373b01d5d432bbb0c2ddfb71a79d5129c1775b96"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1889f63d0dd9a874fecbe18b12ad90ee9ff9cd27","unresolved":false,"context_lines":[{"line_number":5637,"context_line":"                    self.volume_api.terminate_connection(ctxt, bdm.volume_id,"},{"line_number":5638,"context_line":"                                                         connector)"},{"line_number":5639,"context_line":"                else:"},{"line_number":5640,"context_line":"                    # cinder v3 api flow - delete the old attachment"},{"line_number":5641,"context_line":"                    # for the source host"},{"line_number":5642,"context_line":"                    old_attachment_id \u003d \\"},{"line_number":5643,"context_line":"                        migrate_data.old_attachments[str(bdm.id)]"}],"source_content_type":"text/x-python","patch_set":8,"id":"5f201791_e7b17c91","line":5640,"range":{"start_line":5640,"start_character":29,"end_line":5640,"end_character":31},"updated":"2017-06-21 19:02:49.000000000","message":"3.27","commit_id":"373b01d5d432bbb0c2ddfb71a79d5129c1775b96"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"3f4ba5fa25a33b8f119e1b43fb92f46c6a614913","unresolved":false,"context_lines":[{"line_number":5637,"context_line":"                    self.volume_api.terminate_connection(ctxt, bdm.volume_id,"},{"line_number":5638,"context_line":"                                                         connector)"},{"line_number":5639,"context_line":"                else:"},{"line_number":5640,"context_line":"                    # cinder v3 api flow - delete the old attachment"},{"line_number":5641,"context_line":"                    # for the source host"},{"line_number":5642,"context_line":"                    old_attachment_id \u003d \\"},{"line_number":5643,"context_line":"                        migrate_data.old_attachments[str(bdm.id)]"}],"source_content_type":"text/x-python","patch_set":8,"id":"5f201791_d07ab822","line":5640,"range":{"start_line":5640,"start_character":29,"end_line":5640,"end_character":31},"in_reply_to":"5f201791_e7b17c91","updated":"2017-06-22 21:49:26.000000000","message":"Done","commit_id":"373b01d5d432bbb0c2ddfb71a79d5129c1775b96"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1889f63d0dd9a874fecbe18b12ad90ee9ff9cd27","unresolved":false,"context_lines":[{"line_number":5640,"context_line":"                    # cinder v3 api flow - delete the old attachment"},{"line_number":5641,"context_line":"                    # for the source host"},{"line_number":5642,"context_line":"                    old_attachment_id \u003d \\"},{"line_number":5643,"context_line":"                        migrate_data.old_attachments[str(bdm.id)]"},{"line_number":5644,"context_line":"                    self.volume_api.attachment_delete(ctxt, old_attachment_id)"},{"line_number":5645,"context_line":""},{"line_number":5646,"context_line":"        # Releasing vlan."}],"source_content_type":"text/x-python","patch_set":8,"id":"5f201791_077238e6","line":5643,"range":{"start_line":5643,"start_character":53,"end_line":5643,"end_character":64},"updated":"2017-06-21 19:02:49.000000000","message":"This is where I\u0027d use bdm.volume_id, or just don\u0027t use a dict, use a list of attachment uuids and then you don\u0027t have to care about mapping things. But I guess you\u0027re already looping through a list of bdms so maybe that\u0027s why you used a dict of strings.","commit_id":"373b01d5d432bbb0c2ddfb71a79d5129c1775b96"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"3f4ba5fa25a33b8f119e1b43fb92f46c6a614913","unresolved":false,"context_lines":[{"line_number":5640,"context_line":"                    # cinder v3 api flow - delete the old attachment"},{"line_number":5641,"context_line":"                    # for the source host"},{"line_number":5642,"context_line":"                    old_attachment_id \u003d \\"},{"line_number":5643,"context_line":"                        migrate_data.old_attachments[str(bdm.id)]"},{"line_number":5644,"context_line":"                    self.volume_api.attachment_delete(ctxt, old_attachment_id)"},{"line_number":5645,"context_line":""},{"line_number":5646,"context_line":"        # Releasing vlan."}],"source_content_type":"text/x-python","patch_set":8,"id":"5f201791_5b05a9c1","line":5643,"range":{"start_line":5643,"start_character":53,"end_line":5643,"end_character":64},"in_reply_to":"5f201791_077238e6","updated":"2017-06-22 21:49:26.000000000","message":"changing this to use bdm.volume_id","commit_id":"373b01d5d432bbb0c2ddfb71a79d5129c1775b96"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1889f63d0dd9a874fecbe18b12ad90ee9ff9cd27","unresolved":false,"context_lines":[{"line_number":5835,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":5836,"context_line":"                context, instance.uuid)"},{"line_number":5837,"context_line":"        for bdm in bdms:"},{"line_number":5838,"context_line":"            # if cinder v3 migrate flow (attachment_id is not None)"},{"line_number":5839,"context_line":"            # the volume is detached on the destination during"},{"line_number":5840,"context_line":"            # rollback_live_migration_at_destination"},{"line_number":5841,"context_line":"            if bdm.is_volume and bdm.attachment_id is None:"}],"source_content_type":"text/x-python","patch_set":8,"id":"5f201791_2714144e","line":5838,"range":{"start_line":5838,"start_character":24,"end_line":5838,"end_character":26},"updated":"2017-06-21 19:02:49.000000000","message":"3.27","commit_id":"373b01d5d432bbb0c2ddfb71a79d5129c1775b96"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"3f4ba5fa25a33b8f119e1b43fb92f46c6a614913","unresolved":false,"context_lines":[{"line_number":5835,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":5836,"context_line":"                context, instance.uuid)"},{"line_number":5837,"context_line":"        for bdm in bdms:"},{"line_number":5838,"context_line":"            # if cinder v3 migrate flow (attachment_id is not None)"},{"line_number":5839,"context_line":"            # the volume is detached on the destination during"},{"line_number":5840,"context_line":"            # rollback_live_migration_at_destination"},{"line_number":5841,"context_line":"            if bdm.is_volume and bdm.attachment_id is None:"}],"source_content_type":"text/x-python","patch_set":8,"id":"5f201791_109ef055","line":5838,"range":{"start_line":5838,"start_character":24,"end_line":5838,"end_character":26},"in_reply_to":"5f201791_2714144e","updated":"2017-06-22 21:49:26.000000000","message":"Done","commit_id":"373b01d5d432bbb0c2ddfb71a79d5129c1775b96"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1889f63d0dd9a874fecbe18b12ad90ee9ff9cd27","unresolved":false,"context_lines":[{"line_number":5903,"context_line":"                context, instance, network_info, block_device_info,"},{"line_number":5904,"context_line":"                destroy_disks\u003ddestroy_disks, migrate_data\u003dmigrate_data)"},{"line_number":5905,"context_line":""},{"line_number":5906,"context_line":"            bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":5907,"context_line":"                context, instance.uuid)"},{"line_number":5908,"context_line":"            for bdm in bdms:"},{"line_number":5909,"context_line":"                if bdm.is_volume and bdm.attachment_id is not None:"},{"line_number":5910,"context_line":"                    # New cinder v3 api flow. Detach volumes"}],"source_content_type":"text/x-python","patch_set":8,"id":"5f201791_4a527515","line":5907,"range":{"start_line":5906,"start_character":12,"end_line":5907,"end_character":39},"updated":"2017-06-21 19:02:49.000000000","message":"block_device_info[\u0027block_device_mapping\u0027] already has the bdms retrieved above, so you can avoid another call to the DB here.","commit_id":"373b01d5d432bbb0c2ddfb71a79d5129c1775b96"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"3f4ba5fa25a33b8f119e1b43fb92f46c6a614913","unresolved":false,"context_lines":[{"line_number":5903,"context_line":"                context, instance, network_info, block_device_info,"},{"line_number":5904,"context_line":"                destroy_disks\u003ddestroy_disks, migrate_data\u003dmigrate_data)"},{"line_number":5905,"context_line":""},{"line_number":5906,"context_line":"            bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":5907,"context_line":"                context, instance.uuid)"},{"line_number":5908,"context_line":"            for bdm in bdms:"},{"line_number":5909,"context_line":"                if bdm.is_volume and bdm.attachment_id is not None:"},{"line_number":5910,"context_line":"                    # New cinder v3 api flow. Detach volumes"}],"source_content_type":"text/x-python","patch_set":8,"id":"5f201791_3b89cd74","line":5907,"range":{"start_line":5906,"start_character":12,"end_line":5907,"end_character":39},"in_reply_to":"5f201791_4a527515","updated":"2017-06-22 21:49:26.000000000","message":"block_device_info does have all the bdm info, but it\u0027s a dict and not a bdm object. The bdm object is needed so it can be updated and saved below.","commit_id":"373b01d5d432bbb0c2ddfb71a79d5129c1775b96"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1889f63d0dd9a874fecbe18b12ad90ee9ff9cd27","unresolved":false,"context_lines":[{"line_number":5907,"context_line":"                context, instance.uuid)"},{"line_number":5908,"context_line":"            for bdm in bdms:"},{"line_number":5909,"context_line":"                if bdm.is_volume and bdm.attachment_id is not None:"},{"line_number":5910,"context_line":"                    # New cinder v3 api flow. Detach volumes"},{"line_number":5911,"context_line":"                    # that were attached on destination host during"},{"line_number":5912,"context_line":"                    # pre_live_migration and delete the attachment."},{"line_number":5913,"context_line":"                    new_attachment_id \u003d bdm.attachment_id"}],"source_content_type":"text/x-python","patch_set":8,"id":"5f201791_ea402138","line":5910,"range":{"start_line":5910,"start_character":33,"end_line":5910,"end_character":35},"updated":"2017-06-21 19:02:49.000000000","message":"3.27","commit_id":"373b01d5d432bbb0c2ddfb71a79d5129c1775b96"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"3f4ba5fa25a33b8f119e1b43fb92f46c6a614913","unresolved":false,"context_lines":[{"line_number":5907,"context_line":"                context, instance.uuid)"},{"line_number":5908,"context_line":"            for bdm in bdms:"},{"line_number":5909,"context_line":"                if bdm.is_volume and bdm.attachment_id is not None:"},{"line_number":5910,"context_line":"                    # New cinder v3 api flow. Detach volumes"},{"line_number":5911,"context_line":"                    # that were attached on destination host during"},{"line_number":5912,"context_line":"                    # pre_live_migration and delete the attachment."},{"line_number":5913,"context_line":"                    new_attachment_id \u003d bdm.attachment_id"}],"source_content_type":"text/x-python","patch_set":8,"id":"5f201791_5b84895e","line":5910,"range":{"start_line":5910,"start_character":33,"end_line":5910,"end_character":35},"in_reply_to":"5f201791_ea402138","updated":"2017-06-22 21:49:26.000000000","message":"Done","commit_id":"373b01d5d432bbb0c2ddfb71a79d5129c1775b96"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1889f63d0dd9a874fecbe18b12ad90ee9ff9cd27","unresolved":false,"context_lines":[{"line_number":5916,"context_line":"                    # source host (2 places in the bdm). If"},{"line_number":5917,"context_line":"                    # old_attachments is not there, then there was an"},{"line_number":5918,"context_line":"                    # error before the new attachment was made."},{"line_number":5919,"context_line":"                    old_attachments \u003d getattr(migrate_data, \u0027old_attachments\u0027,"},{"line_number":5920,"context_line":"                                              None)"},{"line_number":5921,"context_line":"                    if old_attachments:"},{"line_number":5922,"context_line":"                        bdm.attachment_id \u003d \\"},{"line_number":5923,"context_line":"                            migrate_data.old_attachments[str(bdm.id)]"}],"source_content_type":"text/x-python","patch_set":8,"id":"5f201791_2a7e196b","line":5920,"range":{"start_line":5919,"start_character":38,"end_line":5920,"end_character":51},"updated":"2017-06-21 19:02:49.000000000","message":"This is going to blow up with a NotImplementedError if migrate_data.old_attachments was not set:\n\nhttps://github.com/openstack/oslo.versionedobjects/blob/1.24.0/oslo_versionedobjects/base.py#L596\n\nThe way to do this is:\n\nold_attachments \u003d migrate_data.old_attachments if \u0027old_attachments\u0027 in migrate_data else None","commit_id":"373b01d5d432bbb0c2ddfb71a79d5129c1775b96"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"3f4ba5fa25a33b8f119e1b43fb92f46c6a614913","unresolved":false,"context_lines":[{"line_number":5916,"context_line":"                    # source host (2 places in the bdm). If"},{"line_number":5917,"context_line":"                    # old_attachments is not there, then there was an"},{"line_number":5918,"context_line":"                    # error before the new attachment was made."},{"line_number":5919,"context_line":"                    old_attachments \u003d getattr(migrate_data, \u0027old_attachments\u0027,"},{"line_number":5920,"context_line":"                                              None)"},{"line_number":5921,"context_line":"                    if old_attachments:"},{"line_number":5922,"context_line":"                        bdm.attachment_id \u003d \\"},{"line_number":5923,"context_line":"                            migrate_data.old_attachments[str(bdm.id)]"}],"source_content_type":"text/x-python","patch_set":8,"id":"5f201791_dc799cfc","line":5920,"range":{"start_line":5919,"start_character":38,"end_line":5920,"end_character":51},"in_reply_to":"5f201791_2a7e196b","updated":"2017-06-22 21:49:26.000000000","message":"This actually works OK as is:\n\n\u003e /mnt/share/stack/nova/nova/compute/manager.py(5894)rollback_live_migration_at_destination()\n(Pdb) getattr(migrate_data, \u0027QQQQ\u0027)\n*** AttributeError: \u0027LibvirtLiveMigrateData\u0027 object has no attribute \u0027QQQQ\u0027\n(Pdb) getattr(migrate_data, \u0027QQQQ\u0027, None)\n(Pdb)","commit_id":"373b01d5d432bbb0c2ddfb71a79d5129c1775b96"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1889f63d0dd9a874fecbe18b12ad90ee9ff9cd27","unresolved":false,"context_lines":[{"line_number":5920,"context_line":"                                              None)"},{"line_number":5921,"context_line":"                    if old_attachments:"},{"line_number":5922,"context_line":"                        bdm.attachment_id \u003d \\"},{"line_number":5923,"context_line":"                            migrate_data.old_attachments[str(bdm.id)]"},{"line_number":5924,"context_line":"                        connection_info \u003d jsonutils.loads(bdm.connection_info)"},{"line_number":5925,"context_line":"                        connection_info[\u0027attachment_id\u0027] \u003d bdm.attachment_id"},{"line_number":5926,"context_line":"                        bdm.connection_info \u003d jsonutils.dumps(connection_info)"}],"source_content_type":"text/x-python","patch_set":8,"id":"5f201791_aae6a943","line":5923,"range":{"start_line":5923,"start_character":57,"end_line":5923,"end_character":68},"updated":"2017-06-21 19:02:49.000000000","message":"bdm.volume_id","commit_id":"373b01d5d432bbb0c2ddfb71a79d5129c1775b96"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"3f4ba5fa25a33b8f119e1b43fb92f46c6a614913","unresolved":false,"context_lines":[{"line_number":5920,"context_line":"                                              None)"},{"line_number":5921,"context_line":"                    if old_attachments:"},{"line_number":5922,"context_line":"                        bdm.attachment_id \u003d \\"},{"line_number":5923,"context_line":"                            migrate_data.old_attachments[str(bdm.id)]"},{"line_number":5924,"context_line":"                        connection_info \u003d jsonutils.loads(bdm.connection_info)"},{"line_number":5925,"context_line":"                        connection_info[\u0027attachment_id\u0027] \u003d bdm.attachment_id"},{"line_number":5926,"context_line":"                        bdm.connection_info \u003d jsonutils.dumps(connection_info)"}],"source_content_type":"text/x-python","patch_set":8,"id":"5f201791_1fadc687","line":5923,"range":{"start_line":5923,"start_character":57,"end_line":5923,"end_character":68},"in_reply_to":"5f201791_aae6a943","updated":"2017-06-22 21:49:26.000000000","message":"Done","commit_id":"373b01d5d432bbb0c2ddfb71a79d5129c1775b96"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1889f63d0dd9a874fecbe18b12ad90ee9ff9cd27","unresolved":false,"context_lines":[{"line_number":5921,"context_line":"                    if old_attachments:"},{"line_number":5922,"context_line":"                        bdm.attachment_id \u003d \\"},{"line_number":5923,"context_line":"                            migrate_data.old_attachments[str(bdm.id)]"},{"line_number":5924,"context_line":"                        connection_info \u003d jsonutils.loads(bdm.connection_info)"},{"line_number":5925,"context_line":"                        connection_info[\u0027attachment_id\u0027] \u003d bdm.attachment_id"},{"line_number":5926,"context_line":"                        bdm.connection_info \u003d jsonutils.dumps(connection_info)"},{"line_number":5927,"context_line":"                        bdm.save()"},{"line_number":5928,"context_line":""},{"line_number":5929,"context_line":"                        self._detach_volume("}],"source_content_type":"text/x-python","patch_set":8,"id":"5f201791_4ad7154d","line":5926,"range":{"start_line":5924,"start_character":24,"end_line":5926,"end_character":78},"updated":"2017-06-21 19:02:49.000000000","message":"Again I think we want to avoid messing with connection_info.","commit_id":"373b01d5d432bbb0c2ddfb71a79d5129c1775b96"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"3f4ba5fa25a33b8f119e1b43fb92f46c6a614913","unresolved":false,"context_lines":[{"line_number":5921,"context_line":"                    if old_attachments:"},{"line_number":5922,"context_line":"                        bdm.attachment_id \u003d \\"},{"line_number":5923,"context_line":"                            migrate_data.old_attachments[str(bdm.id)]"},{"line_number":5924,"context_line":"                        connection_info \u003d jsonutils.loads(bdm.connection_info)"},{"line_number":5925,"context_line":"                        connection_info[\u0027attachment_id\u0027] \u003d bdm.attachment_id"},{"line_number":5926,"context_line":"                        bdm.connection_info \u003d jsonutils.dumps(connection_info)"},{"line_number":5927,"context_line":"                        bdm.save()"},{"line_number":5928,"context_line":""},{"line_number":5929,"context_line":"                        self._detach_volume("}],"source_content_type":"text/x-python","patch_set":8,"id":"5f201791_ff71ea0d","line":5926,"range":{"start_line":5924,"start_character":24,"end_line":5926,"end_character":78},"in_reply_to":"5f201791_4ad7154d","updated":"2017-06-22 21:49:26.000000000","message":"Done","commit_id":"373b01d5d432bbb0c2ddfb71a79d5129c1775b96"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"81a918526ab837de2d9cc509be5da42d68e2710a","unresolved":false,"context_lines":[{"line_number":4957,"context_line":"                 {\u0027volume_id\u0027: volume_id}, instance\u003dinstance)"},{"line_number":4958,"context_line":""},{"line_number":4959,"context_line":"        driver_bdm \u003d driver_block_device.convert_volume(bdm)"},{"line_number":4960,"context_line":"        driver_bdm.detach(context, instance, self.volume_api, self.driver,"},{"line_number":4961,"context_line":"                          attachment_id\u003dattachment_id, destroy_bdm\u003ddestroy_bdm)"},{"line_number":4962,"context_line":""},{"line_number":4963,"context_line":"        info \u003d dict(volume_id\u003dvolume_id)"}],"source_content_type":"text/x-python","patch_set":13,"id":"1f1a1f67_eb713ee2","line":4960,"range":{"start_line":4960,"start_character":8,"end_line":4960,"end_character":25},"updated":"2017-07-13 15:00:59.000000000","message":"So on rollback_live_migration_at_destination, this method is what actually deletes the new attachment:\n\nhttps://github.com/openstack/nova/blob/6205a3f8c4775f1e76277e152a379f237fd0ab7b/nova/virt/block_device.py#L359\n\nHowever, are we sure that driver_bdm object will actually have the attachment_id attribute set? Because it\u0027s ignoring the attachment_id passed in. I think it will because wen it transforms the BlockDeviceMapping object to the DriverVolumeBlockDevice object, it will update the driver bdm with the attachment_id here:\n\nhttps://github.com/openstack/nova/blob/6205a3f8c4775f1e76277e152a379f237fd0ab7b/nova/virt/block_device.py#L244\n\nAnd attachment_id is in _new_fields:\n\nhttps://github.com/openstack/nova/blob/6205a3f8c4775f1e76277e152a379f237fd0ab7b/nova/virt/block_device.py#L223","commit_id":"6298563672edfa6b702ac650ddb25afc18df370b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1c812e7266ded6815dfb54bca34d7215158f51f9","unresolved":false,"context_lines":[{"line_number":5160,"context_line":""},{"line_number":5161,"context_line":"    @wrap_exception()"},{"line_number":5162,"context_line":"    def remove_volume_connection(self, context, volume_id, instance):"},{"line_number":5163,"context_line":"        \"\"\"Remove a volume connection using the volume api.\"\"\""},{"line_number":5164,"context_line":"        # NOTE(vish): We don\u0027t want to actually mark the volume"},{"line_number":5165,"context_line":"        #             detached, or delete the bdm, just remove the"},{"line_number":5166,"context_line":"        #             connection from this host."}],"source_content_type":"text/x-python","patch_set":13,"id":"1f1a1f67_120f0fe2","line":5163,"updated":"2017-07-13 14:00:08.000000000","message":"I think it would be helpful if we expanded this docstring to explain that this method is running on the destination host when rolling back a failed live migration, and the BDM here is only for an old style (pre-Cinder 3.27) volume attachment.","commit_id":"6298563672edfa6b702ac650ddb25afc18df370b"},{"author":{"_account_id":21813,"name":"Andrey Volkov","email":"m@amadev.ru","username":"avolkov"},"change_message_id":"c276a3113ee59d5a5c6992fd48f9b54c9e8b2cc8","unresolved":false,"context_lines":[{"line_number":5661,"context_line":"                    # for the source host"},{"line_number":5662,"context_line":"                    old_attachment_id \u003d \\"},{"line_number":5663,"context_line":"                        migrate_data.old_vol_attachment_ids[bdm.volume_id]"},{"line_number":5664,"context_line":"                    self.volume_api.attachment_delete(ctxt, old_attachment_id)"},{"line_number":5665,"context_line":""},{"line_number":5666,"context_line":"        # Releasing vlan."},{"line_number":5667,"context_line":"        # (not necessary in current implementation?)"}],"source_content_type":"text/x-python","patch_set":13,"id":"1f1a1f67_747abd24","line":5664,"updated":"2017-07-13 08:57:42.000000000","message":"Shouldn\u0027t we process the case when the old_vol_attachement_ids attribute is not available, e.g. source node has the old migrate_data object?","commit_id":"6298563672edfa6b702ac650ddb25afc18df370b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1c812e7266ded6815dfb54bca34d7215158f51f9","unresolved":false,"context_lines":[{"line_number":5661,"context_line":"                    # for the source host"},{"line_number":5662,"context_line":"                    old_attachment_id \u003d \\"},{"line_number":5663,"context_line":"                        migrate_data.old_vol_attachment_ids[bdm.volume_id]"},{"line_number":5664,"context_line":"                    self.volume_api.attachment_delete(ctxt, old_attachment_id)"},{"line_number":5665,"context_line":""},{"line_number":5666,"context_line":"        # Releasing vlan."},{"line_number":5667,"context_line":"        # (not necessary in current implementation?)"}],"source_content_type":"text/x-python","patch_set":13,"id":"1f1a1f67_fe18df81","line":5664,"in_reply_to":"1f1a1f67_2cdd1942","updated":"2017-07-13 14:00:08.000000000","message":"Agree with Ildiko. None of the new attachment flows (bdm.attachment_id is not None) happens until (1) Cinder is new enough to provide the 3.27 API and (2) all computes are upgraded so the control plane code knows the computes can handle new-style attachments.","commit_id":"6298563672edfa6b702ac650ddb25afc18df370b"},{"author":{"_account_id":21813,"name":"Andrey Volkov","email":"m@amadev.ru","username":"avolkov"},"change_message_id":"8098f5601d9bb95e57e5d4cea7026efd18500bfe","unresolved":false,"context_lines":[{"line_number":5661,"context_line":"                    # for the source host"},{"line_number":5662,"context_line":"                    old_attachment_id \u003d \\"},{"line_number":5663,"context_line":"                        migrate_data.old_vol_attachment_ids[bdm.volume_id]"},{"line_number":5664,"context_line":"                    self.volume_api.attachment_delete(ctxt, old_attachment_id)"},{"line_number":5665,"context_line":""},{"line_number":5666,"context_line":"        # Releasing vlan."},{"line_number":5667,"context_line":"        # (not necessary in current implementation?)"}],"source_content_type":"text/x-python","patch_set":13,"id":"1f1a1f67_520587c1","line":5664,"in_reply_to":"1f1a1f67_2cdd1942","updated":"2017-07-13 13:41:40.000000000","message":"Please correct me if I\u0027m wrong but if only one compute is updated than\nbdm can have attachement_id. As DB is shared among all computes so\nit\u0027s possible that bdm has attachement_id but on destination node we\nhave old migrate_data. Just want to find out the truth )","commit_id":"6298563672edfa6b702ac650ddb25afc18df370b"},{"author":{"_account_id":9562,"name":"Ildiko Vancsa","email":"ildiko.vancsa@gmail.com","username":"ildikov"},"change_message_id":"b0d126be112fdb07804a2c6b9eb20c46993d3577","unresolved":false,"context_lines":[{"line_number":5661,"context_line":"                    # for the source host"},{"line_number":5662,"context_line":"                    old_attachment_id \u003d \\"},{"line_number":5663,"context_line":"                        migrate_data.old_vol_attachment_ids[bdm.volume_id]"},{"line_number":5664,"context_line":"                    self.volume_api.attachment_delete(ctxt, old_attachment_id)"},{"line_number":5665,"context_line":""},{"line_number":5666,"context_line":"        # Releasing vlan."},{"line_number":5667,"context_line":"        # (not necessary in current implementation?)"}],"source_content_type":"text/x-python","patch_set":13,"id":"1f1a1f67_329a0b14","line":5664,"in_reply_to":"1f1a1f67_520587c1","updated":"2017-07-13 13:52:29.000000000","message":"In line #3761 here: https://review.openstack.org/#/c/330285/91/nova/compute/api.py we check the minimum compute version. If it\u0027s not high enough we don\u0027t allow the volume to be attached with the new flow and fall back to the old flow, which will not add the attachment_id to the BDM.\n\nOnce the minimum compute version is high enough that means that ALL the compute nodes in the deployment have been updated to (in that patch) version 22, which has the support for the new flow implemented for all the use cases.\n\nThat attach patch is the last to get merged in the chain, therefore the object change for live_migration here is covered by it.\n\nDoes this answer your question/concern?","commit_id":"6298563672edfa6b702ac650ddb25afc18df370b"},{"author":{"_account_id":9562,"name":"Ildiko Vancsa","email":"ildiko.vancsa@gmail.com","username":"ildikov"},"change_message_id":"c937b3129a19453993e08fd0da18afe3e195db4d","unresolved":false,"context_lines":[{"line_number":5661,"context_line":"                    # for the source host"},{"line_number":5662,"context_line":"                    old_attachment_id \u003d \\"},{"line_number":5663,"context_line":"                        migrate_data.old_vol_attachment_ids[bdm.volume_id]"},{"line_number":5664,"context_line":"                    self.volume_api.attachment_delete(ctxt, old_attachment_id)"},{"line_number":5665,"context_line":""},{"line_number":5666,"context_line":"        # Releasing vlan."},{"line_number":5667,"context_line":"        # (not necessary in current implementation?)"}],"source_content_type":"text/x-python","patch_set":13,"id":"1f1a1f67_2cdd1942","line":5664,"in_reply_to":"1f1a1f67_747abd24","updated":"2017-07-13 10:36:37.000000000","message":"You can see in this patch that we check everywhere whether or not the attachment_id in the BDM is available. It is available only in case of the volume was attached with the new Cinder attach API (available from the microversion 3.27), which will be enabled if this patch gets merged: https://review.openstack.org/#/c/330285/\n\nIn the enabler patch there is a compute version bump and check. It ensures that new flow gets used only in case all the compute nodes are upgraded, which ensures that the case you\u0027re referring to here cannot happen.","commit_id":"6298563672edfa6b702ac650ddb25afc18df370b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1c812e7266ded6815dfb54bca34d7215158f51f9","unresolved":false,"context_lines":[{"line_number":5708,"context_line":"            LOG.exception(\"Post live migration at destination %s failed\","},{"line_number":5709,"context_line":"                          dest, instance\u003dinstance, error\u003derror)"},{"line_number":5710,"context_line":""},{"line_number":5711,"context_line":"        do_cleanup, destroy_disks \u003d self._live_migration_cleanup_flags("},{"line_number":5712,"context_line":"                migrate_data)"},{"line_number":5713,"context_line":""},{"line_number":5714,"context_line":"        if do_cleanup:"}],"source_content_type":"text/x-python","patch_set":13,"id":"1f1a1f67_32c32bf0","line":5711,"range":{"start_line":5711,"start_character":8,"end_line":5711,"end_character":18},"updated":"2017-07-13 14:00:08.000000000","message":"I think this will be false if we\u0027re using shared storage for the local disk, which would mean we wouldn\u0027t end up deleting the dest host volume attachments on rollback.","commit_id":"6298563672edfa6b702ac650ddb25afc18df370b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ec29df3216c6149c96bd739164aa07643c86cc79","unresolved":false,"context_lines":[{"line_number":5708,"context_line":"            LOG.exception(\"Post live migration at destination %s failed\","},{"line_number":5709,"context_line":"                          dest, instance\u003dinstance, error\u003derror)"},{"line_number":5710,"context_line":""},{"line_number":5711,"context_line":"        do_cleanup, destroy_disks \u003d self._live_migration_cleanup_flags("},{"line_number":5712,"context_line":"                migrate_data)"},{"line_number":5713,"context_line":""},{"line_number":5714,"context_line":"        if do_cleanup:"}],"source_content_type":"text/x-python","patch_set":13,"id":"1f1a1f67_6660c5cf","line":5711,"range":{"start_line":5711,"start_character":8,"end_line":5711,"end_character":18},"in_reply_to":"1f1a1f67_32c32bf0","updated":"2017-07-13 15:05:01.000000000","message":"Sorry I meant to post this below.","commit_id":"6298563672edfa6b702ac650ddb25afc18df370b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1c812e7266ded6815dfb54bca34d7215158f51f9","unresolved":false,"context_lines":[{"line_number":5854,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":5855,"context_line":"                context, instance.uuid)"},{"line_number":5856,"context_line":"        for bdm in bdms:"},{"line_number":5857,"context_line":"            # if cinder v3.27 migrate flow (attachment_id is not None),"},{"line_number":5858,"context_line":"            # the volume is detached on the destination during"},{"line_number":5859,"context_line":"            # rollback_live_migration_at_destination"},{"line_number":5860,"context_line":"            if bdm.is_volume and bdm.attachment_id is None:"},{"line_number":5861,"context_line":"                self.compute_rpcapi.remove_volume_connection("},{"line_number":5862,"context_line":"                        context, instance, bdm.volume_id, dest)"}],"source_content_type":"text/x-python","patch_set":13,"id":"1f1a1f67_92f0bfcb","line":5859,"range":{"start_line":5857,"start_character":12,"end_line":5859,"end_character":52},"updated":"2017-07-13 14:00:08.000000000","message":"Why wouldn\u0027t we just also delete the attachment for the target host in remove_volume_connection like we do for old style attachments? Then the logic is contained within that single method? Is there some reason we need to be different for the new style attachments and delete those in rollback_live_migration_at_destination?\n\nThe other thing, is that rollback_live_migration_at_destination is only conditionally called based on whether or not there is some expected file cleanup on the dest host, I believe - which might mean that we may not end up deleting the target host attachments. It seems that remove_volume_connection is really meant for cleaning up volume attachments on the target host so that\u0027s where we should also delete the new style attachment for the target host.","commit_id":"6298563672edfa6b702ac650ddb25afc18df370b"},{"author":{"_account_id":9562,"name":"Ildiko Vancsa","email":"ildiko.vancsa@gmail.com","username":"ildikov"},"change_message_id":"ddc71967e678a8b370d805dddacdd58bb87bc422","unresolved":false,"context_lines":[{"line_number":5854,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":5855,"context_line":"                context, instance.uuid)"},{"line_number":5856,"context_line":"        for bdm in bdms:"},{"line_number":5857,"context_line":"            # if cinder v3.27 migrate flow (attachment_id is not None),"},{"line_number":5858,"context_line":"            # the volume is detached on the destination during"},{"line_number":5859,"context_line":"            # rollback_live_migration_at_destination"},{"line_number":5860,"context_line":"            if bdm.is_volume and bdm.attachment_id is None:"},{"line_number":5861,"context_line":"                self.compute_rpcapi.remove_volume_connection("},{"line_number":5862,"context_line":"                        context, instance, bdm.volume_id, dest)"}],"source_content_type":"text/x-python","patch_set":13,"id":"1f1a1f67_bc970a91","line":5859,"range":{"start_line":5857,"start_character":12,"end_line":5859,"end_character":52},"in_reply_to":"1f1a1f67_66044518","updated":"2017-07-13 16:00:39.000000000","message":"Is it slightly weir only to me that remove_volume_connection expects instance to be passed to it and it gets the destination host instead?","commit_id":"6298563672edfa6b702ac650ddb25afc18df370b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4e2a1cc8466f162072379558b39b6fc127709695","unresolved":false,"context_lines":[{"line_number":5854,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":5855,"context_line":"                context, instance.uuid)"},{"line_number":5856,"context_line":"        for bdm in bdms:"},{"line_number":5857,"context_line":"            # if cinder v3.27 migrate flow (attachment_id is not None),"},{"line_number":5858,"context_line":"            # the volume is detached on the destination during"},{"line_number":5859,"context_line":"            # rollback_live_migration_at_destination"},{"line_number":5860,"context_line":"            if bdm.is_volume and bdm.attachment_id is None:"},{"line_number":5861,"context_line":"                self.compute_rpcapi.remove_volume_connection("},{"line_number":5862,"context_line":"                        context, instance, bdm.volume_id, dest)"}],"source_content_type":"text/x-python","patch_set":13,"id":"1f1a1f67_013b3beb","line":5859,"range":{"start_line":5857,"start_character":12,"end_line":5859,"end_character":52},"in_reply_to":"1f1a1f67_66044518","updated":"2017-07-13 15:47:26.000000000","message":"Note my concern about not having migrate_data in the remove_volume_connection method - does that mean we won\u0027t be able to set the bdm.attachment_id to the old attachment_id? We can\u0027t do that in remove_volume_connection since it doesn\u0027t have migrate_data, so we\u0027d have to do that elsewhere, maybe in this method where we do have the bdm and migrate_data.","commit_id":"6298563672edfa6b702ac650ddb25afc18df370b"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"74073e132608eb8aa595bacdf96a82f9b12df7fa","unresolved":false,"context_lines":[{"line_number":5854,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":5855,"context_line":"                context, instance.uuid)"},{"line_number":5856,"context_line":"        for bdm in bdms:"},{"line_number":5857,"context_line":"            # if cinder v3.27 migrate flow (attachment_id is not None),"},{"line_number":5858,"context_line":"            # the volume is detached on the destination during"},{"line_number":5859,"context_line":"            # rollback_live_migration_at_destination"},{"line_number":5860,"context_line":"            if bdm.is_volume and bdm.attachment_id is None:"},{"line_number":5861,"context_line":"                self.compute_rpcapi.remove_volume_connection("},{"line_number":5862,"context_line":"                        context, instance, bdm.volume_id, dest)"}],"source_content_type":"text/x-python","patch_set":13,"id":"1f1a1f67_66044518","line":5859,"range":{"start_line":5857,"start_character":12,"end_line":5859,"end_character":52},"in_reply_to":"1f1a1f67_861c016b","updated":"2017-07-13 15:23:37.000000000","message":"I think the reason I didn\u0027t change remove_volume_connection is that it seemed to be all about the old style of the v2 api. But I see your point on the conditional case for rollback_at_destination. I\u0027ll make the change.","commit_id":"6298563672edfa6b702ac650ddb25afc18df370b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"50ae51e8a3c8629946e1d76a2a964bfdd8df061a","unresolved":false,"context_lines":[{"line_number":5854,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":5855,"context_line":"                context, instance.uuid)"},{"line_number":5856,"context_line":"        for bdm in bdms:"},{"line_number":5857,"context_line":"            # if cinder v3.27 migrate flow (attachment_id is not None),"},{"line_number":5858,"context_line":"            # the volume is detached on the destination during"},{"line_number":5859,"context_line":"            # rollback_live_migration_at_destination"},{"line_number":5860,"context_line":"            if bdm.is_volume and bdm.attachment_id is None:"},{"line_number":5861,"context_line":"                self.compute_rpcapi.remove_volume_connection("},{"line_number":5862,"context_line":"                        context, instance, bdm.volume_id, dest)"}],"source_content_type":"text/x-python","patch_set":13,"id":"1f1a1f67_861c016b","line":5859,"range":{"start_line":5857,"start_character":12,"end_line":5859,"end_character":52},"in_reply_to":"1f1a1f67_92f0bfcb","updated":"2017-07-13 15:01:43.000000000","message":"Is the problem that remove_volume_connection doesn\u0027t have the migrate_data so we don\u0027t have the old bdm attachment_id to set back on the bdm?","commit_id":"6298563672edfa6b702ac650ddb25afc18df370b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"9e8ec4911dd90ba97f5eb2f309b97e3a711d50ec","unresolved":false,"context_lines":[{"line_number":5854,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":5855,"context_line":"                context, instance.uuid)"},{"line_number":5856,"context_line":"        for bdm in bdms:"},{"line_number":5857,"context_line":"            # if cinder v3.27 migrate flow (attachment_id is not None),"},{"line_number":5858,"context_line":"            # the volume is detached on the destination during"},{"line_number":5859,"context_line":"            # rollback_live_migration_at_destination"},{"line_number":5860,"context_line":"            if bdm.is_volume and bdm.attachment_id is None:"},{"line_number":5861,"context_line":"                self.compute_rpcapi.remove_volume_connection("},{"line_number":5862,"context_line":"                        context, instance, bdm.volume_id, dest)"}],"source_content_type":"text/x-python","patch_set":13,"id":"1f1a1f67_c5cf58ae","line":5859,"range":{"start_line":5857,"start_character":12,"end_line":5859,"end_character":52},"in_reply_to":"1f1a1f67_bc970a91","updated":"2017-07-15 01:18:16.000000000","message":"Yeah the function signatures don\u0027t match, but the rpcapi sorts it out with kwargs:\n\nhttps://github.com/openstack/nova/blob/master/nova/compute/rpcapi.py#L839","commit_id":"6298563672edfa6b702ac650ddb25afc18df370b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ec29df3216c6149c96bd739164aa07643c86cc79","unresolved":false,"context_lines":[{"line_number":5868,"context_line":"                action\u003dfields.NotificationAction.LIVE_MIGRATION_ROLLBACK,"},{"line_number":5869,"context_line":"                phase\u003dfields.NotificationPhase.START)"},{"line_number":5870,"context_line":""},{"line_number":5871,"context_line":"        do_cleanup, destroy_disks \u003d self._live_migration_cleanup_flags("},{"line_number":5872,"context_line":"                migrate_data)"},{"line_number":5873,"context_line":""},{"line_number":5874,"context_line":"        if do_cleanup:"},{"line_number":5875,"context_line":"            self.compute_rpcapi.rollback_live_migration_at_destination("}],"source_content_type":"text/x-python","patch_set":13,"id":"1f1a1f67_26a08df4","line":5872,"range":{"start_line":5871,"start_character":8,"end_line":5872,"end_character":29},"updated":"2017-07-13 15:05:01.000000000","message":"I think this will be false if we\u0027re using shared storage for the local disk, which would mean we wouldn\u0027t end up deleting the dest host volume attachments on rollback.","commit_id":"6298563672edfa6b702ac650ddb25afc18df370b"},{"author":{"_account_id":21813,"name":"Andrey Volkov","email":"m@amadev.ru","username":"avolkov"},"change_message_id":"c276a3113ee59d5a5c6992fd48f9b54c9e8b2cc8","unresolved":false,"context_lines":[{"line_number":5940,"context_line":""},{"line_number":5941,"context_line":"                    # restore attachment_id to old attachment of the"},{"line_number":5942,"context_line":"                    # source host (2 places in the bdm). If"},{"line_number":5943,"context_line":"                    # old_attachments is not there, then there was an"},{"line_number":5944,"context_line":"                    # error before the new attachment was made."},{"line_number":5945,"context_line":"                    old_attachments \u003d getattr(migrate_data,"},{"line_number":5946,"context_line":"                                              \u0027old_vol_attachment_ids\u0027, None)"},{"line_number":5947,"context_line":"                    if old_attachments:"}],"source_content_type":"text/x-python","patch_set":13,"id":"1f1a1f67_54ab411c","line":5944,"range":{"start_line":5943,"start_character":52,"end_line":5944,"end_character":63},"updated":"2017-07-13 08:57:42.000000000","message":"or it\u0027s rolling upgrade and versioning stuff?","commit_id":"6298563672edfa6b702ac650ddb25afc18df370b"},{"author":{"_account_id":9562,"name":"Ildiko Vancsa","email":"ildiko.vancsa@gmail.com","username":"ildikov"},"change_message_id":"c937b3129a19453993e08fd0da18afe3e195db4d","unresolved":false,"context_lines":[{"line_number":5940,"context_line":""},{"line_number":5941,"context_line":"                    # restore attachment_id to old attachment of the"},{"line_number":5942,"context_line":"                    # source host (2 places in the bdm). If"},{"line_number":5943,"context_line":"                    # old_attachments is not there, then there was an"},{"line_number":5944,"context_line":"                    # error before the new attachment was made."},{"line_number":5945,"context_line":"                    old_attachments \u003d getattr(migrate_data,"},{"line_number":5946,"context_line":"                                              \u0027old_vol_attachment_ids\u0027, None)"},{"line_number":5947,"context_line":"                    if old_attachments:"}],"source_content_type":"text/x-python","patch_set":13,"id":"1f1a1f67_4c33b5ac","line":5944,"range":{"start_line":5943,"start_character":52,"end_line":5944,"end_character":63},"in_reply_to":"1f1a1f67_54ab411c","updated":"2017-07-13 10:36:37.000000000","message":"Same as above.\n\nThe note here refers to the case when there was an error in the live migration process earlier than this step.","commit_id":"6298563672edfa6b702ac650ddb25afc18df370b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"81a918526ab837de2d9cc509be5da42d68e2710a","unresolved":false,"context_lines":[{"line_number":5942,"context_line":"                    # source host (2 places in the bdm). If"},{"line_number":5943,"context_line":"                    # old_attachments is not there, then there was an"},{"line_number":5944,"context_line":"                    # error before the new attachment was made."},{"line_number":5945,"context_line":"                    old_attachments \u003d getattr(migrate_data,"},{"line_number":5946,"context_line":"                                              \u0027old_vol_attachment_ids\u0027, None)"},{"line_number":5947,"context_line":"                    if old_attachments:"},{"line_number":5948,"context_line":"                        bdm.attachment_id \u003d old_attachments[bdm.volume_id]"},{"line_number":5949,"context_line":"                        bdm.save()"}],"source_content_type":"text/x-python","patch_set":13,"id":"1f1a1f67_4b7952fa","line":5946,"range":{"start_line":5945,"start_character":38,"end_line":5946,"end_character":77},"updated":"2017-07-13 15:00:59.000000000","message":"With the way versioned objects works, I think you probably want to do this as:\n\nold_attachments \u003d migrate_data.old_vol_attachment_ids if \u0027old_vol_attachment_ids\u0027 in migrate_data else None\n\nBecause doing a getattr will try to load it and could result in a failure if it\u0027s not there, these things aren\u0027t dicts.","commit_id":"6298563672edfa6b702ac650ddb25afc18df370b"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"d7e7a81688e9443e49daa6b7fc2126e202cee00b","unresolved":false,"context_lines":[{"line_number":5942,"context_line":"                    # source host (2 places in the bdm). If"},{"line_number":5943,"context_line":"                    # old_attachments is not there, then there was an"},{"line_number":5944,"context_line":"                    # error before the new attachment was made."},{"line_number":5945,"context_line":"                    old_attachments \u003d getattr(migrate_data,"},{"line_number":5946,"context_line":"                                              \u0027old_vol_attachment_ids\u0027, None)"},{"line_number":5947,"context_line":"                    if old_attachments:"},{"line_number":5948,"context_line":"                        bdm.attachment_id \u003d old_attachments[bdm.volume_id]"},{"line_number":5949,"context_line":"                        bdm.save()"}],"source_content_type":"text/x-python","patch_set":13,"id":"1f1a1f67_721dedbb","line":5946,"range":{"start_line":5945,"start_character":38,"end_line":5946,"end_character":77},"in_reply_to":"1f1a1f67_4b7952fa","updated":"2017-07-17 15:03:08.000000000","message":"Done","commit_id":"6298563672edfa6b702ac650ddb25afc18df370b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"81a918526ab837de2d9cc509be5da42d68e2710a","unresolved":false,"context_lines":[{"line_number":5949,"context_line":"                        bdm.save()"},{"line_number":5950,"context_line":""},{"line_number":5951,"context_line":"                        self._detach_volume("},{"line_number":5952,"context_line":"                            context, bdm.volume_id, instance,"},{"line_number":5953,"context_line":"                            destroy_bdm\u003dFalse,"},{"line_number":5954,"context_line":"                            attachment_id\u003dnew_attachment_id)"},{"line_number":5955,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"1f1a1f67_ab00e635","line":5952,"range":{"start_line":5952,"start_character":37,"end_line":5952,"end_character":50},"updated":"2017-07-13 15:00:59.000000000","message":"This is broken, and probably missed in the rebase, and your tests are probably mocking this _detach_volume method out which is why it wasn\u0027t caught in test, but I changed that method to take a bdm object so we wouldn\u0027t have to go back to the database to look it up when we already have the bdm in scope - so you need to pass the bdm object here, not the volume id.","commit_id":"6298563672edfa6b702ac650ddb25afc18df370b"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"d7e7a81688e9443e49daa6b7fc2126e202cee00b","unresolved":false,"context_lines":[{"line_number":5949,"context_line":"                        bdm.save()"},{"line_number":5950,"context_line":""},{"line_number":5951,"context_line":"                        self._detach_volume("},{"line_number":5952,"context_line":"                            context, bdm.volume_id, instance,"},{"line_number":5953,"context_line":"                            destroy_bdm\u003dFalse,"},{"line_number":5954,"context_line":"                            attachment_id\u003dnew_attachment_id)"},{"line_number":5955,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"1f1a1f67_a1f5a7cd","line":5952,"range":{"start_line":5952,"start_character":37,"end_line":5952,"end_character":50},"in_reply_to":"1f1a1f67_ab00e635","updated":"2017-07-17 15:03:08.000000000","message":"Done","commit_id":"6298563672edfa6b702ac650ddb25afc18df370b"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"768958900377a6888e7e62b434d365c2598826e4","unresolved":false,"context_lines":[{"line_number":5161,"context_line":""},{"line_number":5162,"context_line":"    @wrap_exception()"},{"line_number":5163,"context_line":"    def remove_volume_connection(self, context, volume_id, instance):"},{"line_number":5164,"context_line":"        \"\"\"Remove the volume connection on this host"},{"line_number":5165,"context_line":""},{"line_number":5166,"context_line":"        Detach the volume from this instance on this host, and if this is"},{"line_number":5167,"context_line":"        the cinder v2 flow, call cinder to terminate the connection."}],"source_content_type":"text/x-python","patch_set":14,"id":"1f1a1f67_f433d660","line":5164,"updated":"2017-07-18 16:19:27.000000000","message":"nitiest of nits: missing full stop","commit_id":"3237b6c593d1b15ae3a2976b9e7c6aee3b5b9d14"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"768958900377a6888e7e62b434d365c2598826e4","unresolved":false,"context_lines":[{"line_number":5172,"context_line":"            driver_bdm \u003d driver_block_device.convert_volume(bdm)"},{"line_number":5173,"context_line":"            driver_bdm.driver_detach(context, instance,"},{"line_number":5174,"context_line":"                                     self.volume_api, self.driver)"},{"line_number":5175,"context_line":"            if bdm.attachment_id is None:"},{"line_number":5176,"context_line":"                # cinder v2 api flow"},{"line_number":5177,"context_line":"                connector \u003d self.driver.get_volume_connector(instance)"},{"line_number":5178,"context_line":"                self.volume_api.terminate_connection(context, volume_id,"}],"source_content_type":"text/x-python","patch_set":14,"id":"1f1a1f67_b42afed7","line":5175,"range":{"start_line":5175,"start_character":12,"end_line":5175,"end_character":41},"updated":"2017-07-18 16:19:27.000000000","message":"So previously we always ran the two commands below. Now we only do this when \u0027attachment_id\u0027 is set on the bdm. What\u0027s the distinction between attachment_id being set vs. unset, and why don\u0027t we need to provide a legacy path?","commit_id":"3237b6c593d1b15ae3a2976b9e7c6aee3b5b9d14"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"a1247190eb4d261ad6f36b69202396083097181e","unresolved":false,"context_lines":[{"line_number":5172,"context_line":"            driver_bdm \u003d driver_block_device.convert_volume(bdm)"},{"line_number":5173,"context_line":"            driver_bdm.driver_detach(context, instance,"},{"line_number":5174,"context_line":"                                     self.volume_api, self.driver)"},{"line_number":5175,"context_line":"            if bdm.attachment_id is None:"},{"line_number":5176,"context_line":"                # cinder v2 api flow"},{"line_number":5177,"context_line":"                connector \u003d self.driver.get_volume_connector(instance)"},{"line_number":5178,"context_line":"                self.volume_api.terminate_connection(context, volume_id,"}],"source_content_type":"text/x-python","patch_set":14,"id":"1f1a1f67_7048d72b","line":5175,"range":{"start_line":5175,"start_character":12,"end_line":5175,"end_character":41},"in_reply_to":"1f1a1f67_a0acf8ec","updated":"2017-07-19 08:35:59.000000000","message":"Aha, I misunderstood the logic here. We\u0027re checking if \u0027attachment_id\u0027 is *unset*, which happens when using the legacy v2 API. Cool cool","commit_id":"3237b6c593d1b15ae3a2976b9e7c6aee3b5b9d14"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"ca703ed0a20fb5a3cc72759eed74f2242fa96d46","unresolved":false,"context_lines":[{"line_number":5172,"context_line":"            driver_bdm \u003d driver_block_device.convert_volume(bdm)"},{"line_number":5173,"context_line":"            driver_bdm.driver_detach(context, instance,"},{"line_number":5174,"context_line":"                                     self.volume_api, self.driver)"},{"line_number":5175,"context_line":"            if bdm.attachment_id is None:"},{"line_number":5176,"context_line":"                # cinder v2 api flow"},{"line_number":5177,"context_line":"                connector \u003d self.driver.get_volume_connector(instance)"},{"line_number":5178,"context_line":"                self.volume_api.terminate_connection(context, volume_id,"}],"source_content_type":"text/x-python","patch_set":14,"id":"1f1a1f67_a0acf8ec","line":5175,"range":{"start_line":5175,"start_character":12,"end_line":5175,"end_character":41},"in_reply_to":"1f1a1f67_b42afed7","updated":"2017-07-18 17:25:53.000000000","message":"When the bdm has an attachment_id, it means that the volume was attached using the new v3 cinder api. Volumes that were attached pre-pike will not have that set and we need to preserve the legacy behavior. Spec is here: http://specs.openstack.org/openstack/cinder-specs/specs/ocata/add-new-attach-apis.html","commit_id":"3237b6c593d1b15ae3a2976b9e7c6aee3b5b9d14"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"768958900377a6888e7e62b434d365c2598826e4","unresolved":false,"context_lines":[{"line_number":5380,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":5381,"context_line":"            context, instance.uuid)"},{"line_number":5382,"context_line":"        for bdm in bdms:"},{"line_number":5383,"context_line":"            if bdm.is_volume and bdm.attachment_id is not None:"},{"line_number":5384,"context_line":"                # This bdm uses the new cinder v3.27 API."},{"line_number":5385,"context_line":"                # We will create a new attachment for this"},{"line_number":5386,"context_line":"                # volume on this migration destination host. The old"}],"source_content_type":"text/x-python","patch_set":14,"id":"1f1a1f67_5fe06326","line":5383,"range":{"start_line":5383,"start_character":33,"end_line":5383,"end_character":62},"updated":"2017-07-18 16:19:27.000000000","message":"Is there a migration path for bdms that don\u0027t have this attribute set? Will we eventually get to state where attachment_id will always be set?","commit_id":"3237b6c593d1b15ae3a2976b9e7c6aee3b5b9d14"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"ca703ed0a20fb5a3cc72759eed74f2242fa96d46","unresolved":false,"context_lines":[{"line_number":5380,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":5381,"context_line":"            context, instance.uuid)"},{"line_number":5382,"context_line":"        for bdm in bdms:"},{"line_number":5383,"context_line":"            if bdm.is_volume and bdm.attachment_id is not None:"},{"line_number":5384,"context_line":"                # This bdm uses the new cinder v3.27 API."},{"line_number":5385,"context_line":"                # We will create a new attachment for this"},{"line_number":5386,"context_line":"                # volume on this migration destination host. The old"}],"source_content_type":"text/x-python","patch_set":14,"id":"1f1a1f67_00404c06","line":5383,"range":{"start_line":5383,"start_character":33,"end_line":5383,"end_character":62},"in_reply_to":"1f1a1f67_5fe06326","updated":"2017-07-18 17:25:53.000000000","message":"all new attachments in pike will have it set, all attachments pre-pike will have it blank. I believe the migration strategy is attrition.","commit_id":"3237b6c593d1b15ae3a2976b9e7c6aee3b5b9d14"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"768958900377a6888e7e62b434d365c2598826e4","unresolved":false,"context_lines":[{"line_number":5413,"context_line":"                     context, instance, \"live_migration.pre.start\","},{"line_number":5414,"context_line":"                     network_info\u003dnetwork_info)"},{"line_number":5415,"context_line":""},{"line_number":5416,"context_line":"        migrate_data \u003d self.driver.pre_live_migration(context,"},{"line_number":5417,"context_line":"                                       instance,"},{"line_number":5418,"context_line":"                                       block_device_info,"},{"line_number":5419,"context_line":"                                       network_info,"}],"source_content_type":"text/x-python","patch_set":14,"id":"1f1a1f67_ff204f9d","line":5416,"range":{"start_line":5416,"start_character":8,"end_line":5416,"end_character":20},"updated":"2017-07-18 16:19:27.000000000","message":"I was going to note that we\u0027re overriding the \u0027old_vol_attachement_ids\u0027 value that we set above, but we\u0027re simply reusing a variable name here.","commit_id":"3237b6c593d1b15ae3a2976b9e7c6aee3b5b9d14"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"9271f71a3f10b6d2ddfff1507be131833a2fa5fe","unresolved":false,"context_lines":[{"line_number":5413,"context_line":"                     context, instance, \"live_migration.pre.start\","},{"line_number":5414,"context_line":"                     network_info\u003dnetwork_info)"},{"line_number":5415,"context_line":""},{"line_number":5416,"context_line":"        migrate_data \u003d self.driver.pre_live_migration(context,"},{"line_number":5417,"context_line":"                                       instance,"},{"line_number":5418,"context_line":"                                       block_device_info,"},{"line_number":5419,"context_line":"                                       network_info,"}],"source_content_type":"text/x-python","patch_set":14,"id":"1f1a1f67_84a686c0","line":5416,"range":{"start_line":5416,"start_character":8,"end_line":5416,"end_character":20},"in_reply_to":"1f1a1f67_d1045a17","updated":"2017-07-19 17:18:00.000000000","message":"You are correct that if the driver over-writes migrate_data.old_attachments this flow will fail. Checking through the various drivers, this is not currently an issue. \n\nBut this does seem to be the first time that the manager is using something in migrate_data for its own purposes. Do you have a suggestion of a way to doc this? or maybe I could make the code withstand any changes to this by the drivers, but that seems like a bit of overkill. comments?","commit_id":"3237b6c593d1b15ae3a2976b9e7c6aee3b5b9d14"},{"author":{"_account_id":17922,"name":"Huan Xie","email":"huan.xie@citrix.com","username":"huan"},"change_message_id":"6ff5ea7abb2de5e989eda9cdcab15ee5a576be43","unresolved":false,"context_lines":[{"line_number":5413,"context_line":"                     context, instance, \"live_migration.pre.start\","},{"line_number":5414,"context_line":"                     network_info\u003dnetwork_info)"},{"line_number":5415,"context_line":""},{"line_number":5416,"context_line":"        migrate_data \u003d self.driver.pre_live_migration(context,"},{"line_number":5417,"context_line":"                                       instance,"},{"line_number":5418,"context_line":"                                       block_device_info,"},{"line_number":5419,"context_line":"                                       network_info,"}],"source_content_type":"text/x-python","patch_set":14,"id":"1f1a1f67_d1045a17","line":5416,"range":{"start_line":5416,"start_character":8,"end_line":5416,"end_character":20},"in_reply_to":"1f1a1f67_ff204f9d","updated":"2017-07-19 01:11:14.000000000","message":"You mean this migrate_data will override the above migrate_data? It seems the driver won\u0027t override migrate_data in their pre_live_migration() function, just add some new values and return migrate_data again. But yes, it maybe have potential issue if any driver changes the logic\nXenAPI: https://github.com/openstack/nova/blob/b79f57a31d9321be471ca9add5a63976f2320830/nova/virt/xenapi/vmops.py#L2471\nlibvirt:\nhttps://github.com/openstack/nova/blob/b79f57a31d9321be471ca9add5a63976f2320830/nova/virt/libvirt/driver.py#L6741","commit_id":"3237b6c593d1b15ae3a2976b9e7c6aee3b5b9d14"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"ca703ed0a20fb5a3cc72759eed74f2242fa96d46","unresolved":false,"context_lines":[{"line_number":5413,"context_line":"                     context, instance, \"live_migration.pre.start\","},{"line_number":5414,"context_line":"                     network_info\u003dnetwork_info)"},{"line_number":5415,"context_line":""},{"line_number":5416,"context_line":"        migrate_data \u003d self.driver.pre_live_migration(context,"},{"line_number":5417,"context_line":"                                       instance,"},{"line_number":5418,"context_line":"                                       block_device_info,"},{"line_number":5419,"context_line":"                                       network_info,"}],"source_content_type":"text/x-python","patch_set":14,"id":"1f1a1f67_203d888e","line":5416,"range":{"start_line":5416,"start_character":8,"end_line":5416,"end_character":20},"in_reply_to":"1f1a1f67_ff204f9d","updated":"2017-07-18 17:25:53.000000000","message":"the old_attachment_ids survive the pre_live_migration call.","commit_id":"3237b6c593d1b15ae3a2976b9e7c6aee3b5b9d14"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"768958900377a6888e7e62b434d365c2598826e4","unresolved":false,"context_lines":[{"line_number":5868,"context_line":"                    # 3.27 cinder api flow. Set the bdm\u0027s"},{"line_number":5869,"context_line":"                    # attachment_id to the old attachment of the source"},{"line_number":5870,"context_line":"                    # host. If old_attachments is not there, then"},{"line_number":5871,"context_line":"                    # there was an error before the new attachment was made."},{"line_number":5872,"context_line":"                    old_attachments \u003d migrate_data.old_vol_attachment_ids \\"},{"line_number":5873,"context_line":"                        if \u0027old_vol_attachment_ids\u0027 in migrate_data else None"},{"line_number":5874,"context_line":"                    if old_attachments:"}],"source_content_type":"text/x-python","patch_set":14,"id":"1f1a1f67_bffbf7be","line":5871,"range":{"start_line":5871,"start_character":0,"end_line":5871,"end_character":76},"updated":"2017-07-18 16:19:27.000000000","message":"It\u0027s probably not an exception, but should we log about this?","commit_id":"3237b6c593d1b15ae3a2976b9e7c6aee3b5b9d14"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"ca703ed0a20fb5a3cc72759eed74f2242fa96d46","unresolved":false,"context_lines":[{"line_number":5868,"context_line":"                    # 3.27 cinder api flow. Set the bdm\u0027s"},{"line_number":5869,"context_line":"                    # attachment_id to the old attachment of the source"},{"line_number":5870,"context_line":"                    # host. If old_attachments is not there, then"},{"line_number":5871,"context_line":"                    # there was an error before the new attachment was made."},{"line_number":5872,"context_line":"                    old_attachments \u003d migrate_data.old_vol_attachment_ids \\"},{"line_number":5873,"context_line":"                        if \u0027old_vol_attachment_ids\u0027 in migrate_data else None"},{"line_number":5874,"context_line":"                    if old_attachments:"}],"source_content_type":"text/x-python","patch_set":14,"id":"1f1a1f67_200b48d6","line":5871,"range":{"start_line":5871,"start_character":0,"end_line":5871,"end_character":76},"in_reply_to":"1f1a1f67_bffbf7be","updated":"2017-07-18 17:25:53.000000000","message":"If an exception occurred, it would have been previously logged so I didn\u0027t see a need to log anything further.","commit_id":"3237b6c593d1b15ae3a2976b9e7c6aee3b5b9d14"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"f324aec89c0019f79cad611c20f70285fc2ab943","unresolved":false,"context_lines":[{"line_number":5337,"context_line":"                migrate_data_obj.LiveMigrateData.detect_implementation("},{"line_number":5338,"context_line":"                    migrate_data)"},{"line_number":5339,"context_line":""},{"line_number":5340,"context_line":"        migrate_data.old_vol_attachment_ids \u003d {}"},{"line_number":5341,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":5342,"context_line":"            context, instance.uuid)"},{"line_number":5343,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":16,"id":"1f1a1f67_bcf82a7c","line":5340,"range":{"start_line":5340,"start_character":8,"end_line":5340,"end_character":48},"updated":"2017-07-21 14:25:33.000000000","message":"Why did this change?","commit_id":"98f98b9c86e2cf385444961f48fb4df0c7a8d34f"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"56478947be61150a76c5678ba2f80c20059af7bf","unresolved":false,"context_lines":[{"line_number":5337,"context_line":"                migrate_data_obj.LiveMigrateData.detect_implementation("},{"line_number":5338,"context_line":"                    migrate_data)"},{"line_number":5339,"context_line":""},{"line_number":5340,"context_line":"        migrate_data.old_vol_attachment_ids \u003d {}"},{"line_number":5341,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":5342,"context_line":"            context, instance.uuid)"},{"line_number":5343,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":16,"id":"1f1a1f67_1b3603e9","line":5340,"range":{"start_line":5340,"start_character":8,"end_line":5340,"end_character":48},"in_reply_to":"1f1a1f67_bcf82a7c","updated":"2017-07-21 15:10:07.000000000","message":"I found a problem where if we threw an exception (maybe talking to cinder for eg) we\u0027d leave the old_attachments empty. So this now updates each time a volume is attached. Makes it possible (tho not 100% certain, see below) to rollback a partial set of attachments).","commit_id":"98f98b9c86e2cf385444961f48fb4df0c7a8d34f"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"f324aec89c0019f79cad611c20f70285fc2ab943","unresolved":false,"context_lines":[{"line_number":5366,"context_line":"                    # nova."},{"line_number":5367,"context_line":"                    bdm.attachment_id \u003d attach_ref[\u0027id\u0027]"},{"line_number":5368,"context_line":"                    bdm.save()"},{"line_number":5369,"context_line":"        except Exception as e:"},{"line_number":5370,"context_line":"            # If we raise, migrate_data with the updated attachment ids"},{"line_number":5371,"context_line":"            # will not be returned to the source host for rollback."},{"line_number":5372,"context_line":"            # So we need to rollback new attachments here."}],"source_content_type":"text/x-python","patch_set":16,"id":"1f1a1f67_fc334261","line":5369,"range":{"start_line":5369,"start_character":15,"end_line":5369,"end_character":24},"updated":"2017-07-21 14:25:33.000000000","message":"Can we be more specific? What exceptions are we hoping to catch here?","commit_id":"98f98b9c86e2cf385444961f48fb4df0c7a8d34f"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"56478947be61150a76c5678ba2f80c20059af7bf","unresolved":false,"context_lines":[{"line_number":5366,"context_line":"                    # nova."},{"line_number":5367,"context_line":"                    bdm.attachment_id \u003d attach_ref[\u0027id\u0027]"},{"line_number":5368,"context_line":"                    bdm.save()"},{"line_number":5369,"context_line":"        except Exception as e:"},{"line_number":5370,"context_line":"            # If we raise, migrate_data with the updated attachment ids"},{"line_number":5371,"context_line":"            # will not be returned to the source host for rollback."},{"line_number":5372,"context_line":"            # So we need to rollback new attachments here."}],"source_content_type":"text/x-python","patch_set":16,"id":"1f1a1f67_db440b4a","line":5369,"range":{"start_line":5369,"start_character":15,"end_line":5369,"end_character":24},"in_reply_to":"1f1a1f67_fc334261","updated":"2017-07-21 15:10:07.000000000","message":"Really any exception that interrupts this loop we need to catch and then do the rollback. Most likely would be a network issue to cinder, or some bad param issue to cinder. Currently we throw out on any exception and this doesn\u0027t change that.","commit_id":"98f98b9c86e2cf385444961f48fb4df0c7a8d34f"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"f324aec89c0019f79cad611c20f70285fc2ab943","unresolved":false,"context_lines":[{"line_number":5374,"context_line":"            for bdm in bdms:"},{"line_number":5375,"context_line":"                if (bdm.is_volume and bdm.attachment_id is not None and"},{"line_number":5376,"context_line":"                            bdm.volume_id in old_attachments):"},{"line_number":5377,"context_line":"                        self.volume_api.attachment_delete(context,"},{"line_number":5378,"context_line":"                                                          bdm.attachment_id)"},{"line_number":5379,"context_line":"                        bdm.attachment_id \u003d old_attachments[bdm.volume_id]"},{"line_number":5380,"context_line":"                        bdm.save()"},{"line_number":5381,"context_line":"            raise e"},{"line_number":5382,"context_line":""},{"line_number":5383,"context_line":"        block_device_info \u003d self._get_instance_block_device_info("}],"source_content_type":"text/x-python","patch_set":16,"id":"1f1a1f67_5c4476b7","line":5380,"range":{"start_line":5377,"start_character":0,"end_line":5380,"end_character":34},"updated":"2017-07-21 14:25:33.000000000","message":"I assume this can\u0027t raise itself?","commit_id":"98f98b9c86e2cf385444961f48fb4df0c7a8d34f"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"56478947be61150a76c5678ba2f80c20059af7bf","unresolved":false,"context_lines":[{"line_number":5374,"context_line":"            for bdm in bdms:"},{"line_number":5375,"context_line":"                if (bdm.is_volume and bdm.attachment_id is not None and"},{"line_number":5376,"context_line":"                            bdm.volume_id in old_attachments):"},{"line_number":5377,"context_line":"                        self.volume_api.attachment_delete(context,"},{"line_number":5378,"context_line":"                                                          bdm.attachment_id)"},{"line_number":5379,"context_line":"                        bdm.attachment_id \u003d old_attachments[bdm.volume_id]"},{"line_number":5380,"context_line":"                        bdm.save()"},{"line_number":5381,"context_line":"            raise e"},{"line_number":5382,"context_line":""},{"line_number":5383,"context_line":"        block_device_info \u003d self._get_instance_block_device_info("}],"source_content_type":"text/x-python","patch_set":16,"id":"1f1a1f67_3bae5fe7","line":5380,"range":{"start_line":5377,"start_character":0,"end_line":5380,"end_character":34},"in_reply_to":"1f1a1f67_5c4476b7","updated":"2017-07-21 15:10:07.000000000","message":"It could; for example, if we lost connectivity with cinder, this would throw too. But at that point there\u0027s not much we can do about rolling back and there will have to be some manual intervention when cinder comes back on-line to clean things up.","commit_id":"98f98b9c86e2cf385444961f48fb4df0c7a8d34f"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"54a5a32260552d80102acebfb8d180921119a0c6","unresolved":false,"context_lines":[{"line_number":5388,"context_line":"        try:"},{"line_number":5389,"context_line":"            for bdm in bdms:"},{"line_number":5390,"context_line":"                if bdm.is_volume and bdm.attachment_id is not None:"},{"line_number":5391,"context_line":"                    # This bdm uses the new cinder v3.44 API."},{"line_number":5392,"context_line":"                    # We will create a new attachment for this"},{"line_number":5393,"context_line":"                    # volume on this migration destination host. The old"},{"line_number":5394,"context_line":"                    # attachment will be deleted on the source host"}],"source_content_type":"text/x-python","patch_set":18,"id":"9f5c4f37_a326deb6","line":5391,"updated":"2017-09-21 19:20:31.000000000","message":"Thanks for the nice documentation here.","commit_id":"f576578b2e1050def3ec2995fb4438b27a66aab4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"54a5a32260552d80102acebfb8d180921119a0c6","unresolved":false,"context_lines":[{"line_number":5403,"context_line":"                    migrate_data.old_vol_attachment_ids[bdm.volume_id] \u003d \\"},{"line_number":5404,"context_line":"                        bdm.attachment_id"},{"line_number":5405,"context_line":""},{"line_number":5406,"context_line":"                    # update the bdm with the new attachment_id. Note that"},{"line_number":5407,"context_line":"                    # connection_info that comes from cinder also contains"},{"line_number":5408,"context_line":"                    # an attachment_id. Even though we are changing the"},{"line_number":5409,"context_line":"                    # attachment_id on the bdm, the connection_info"},{"line_number":5410,"context_line":"                    # attachment_id is left alone as it should be ignored by"},{"line_number":5411,"context_line":"                    # nova."},{"line_number":5412,"context_line":"                    bdm.attachment_id \u003d attach_ref[\u0027id\u0027]"},{"line_number":5413,"context_line":"                    bdm.save()"},{"line_number":5414,"context_line":"        except Exception as e:"}],"source_content_type":"text/x-python","patch_set":18,"id":"9f5c4f37_e32cd69a","line":5411,"range":{"start_line":5406,"start_character":65,"end_line":5411,"end_character":27},"updated":"2017-09-21 19:20:31.000000000","message":"We shouldn\u0027t actually get a connection_info when creating a new attachment if we didn\u0027t provide a host connector, so this is just kind of confusing for me.","commit_id":"f576578b2e1050def3ec2995fb4438b27a66aab4"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"7b29f9a04263a131d2699345327692fe81fdc5c8","unresolved":false,"context_lines":[{"line_number":5403,"context_line":"                    migrate_data.old_vol_attachment_ids[bdm.volume_id] \u003d \\"},{"line_number":5404,"context_line":"                        bdm.attachment_id"},{"line_number":5405,"context_line":""},{"line_number":5406,"context_line":"                    # update the bdm with the new attachment_id. Note that"},{"line_number":5407,"context_line":"                    # connection_info that comes from cinder also contains"},{"line_number":5408,"context_line":"                    # an attachment_id. Even though we are changing the"},{"line_number":5409,"context_line":"                    # attachment_id on the bdm, the connection_info"},{"line_number":5410,"context_line":"                    # attachment_id is left alone as it should be ignored by"},{"line_number":5411,"context_line":"                    # nova."},{"line_number":5412,"context_line":"                    bdm.attachment_id \u003d attach_ref[\u0027id\u0027]"},{"line_number":5413,"context_line":"                    bdm.save()"},{"line_number":5414,"context_line":"        except Exception as e:"}],"source_content_type":"text/x-python","patch_set":18,"id":"7f515b1d_cdb101e4","line":5411,"range":{"start_line":5406,"start_character":65,"end_line":5411,"end_character":27},"in_reply_to":"9f5c4f37_e32cd69a","updated":"2017-09-25 20:57:58.000000000","message":"Done","commit_id":"f576578b2e1050def3ec2995fb4438b27a66aab4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"54a5a32260552d80102acebfb8d180921119a0c6","unresolved":false,"context_lines":[{"line_number":5412,"context_line":"                    bdm.attachment_id \u003d attach_ref[\u0027id\u0027]"},{"line_number":5413,"context_line":"                    bdm.save()"},{"line_number":5414,"context_line":"        except Exception as e:"},{"line_number":5415,"context_line":"            # If we raise, migrate_data with the updated attachment ids"},{"line_number":5416,"context_line":"            # will not be returned to the source host for rollback."},{"line_number":5417,"context_line":"            # So we need to rollback new attachments here."},{"line_number":5418,"context_line":"            old_attachments \u003d migrate_data.old_vol_attachment_ids"},{"line_number":5419,"context_line":"            for bdm in bdms:"},{"line_number":5420,"context_line":"                if (bdm.is_volume and bdm.attachment_id is not None and"},{"line_number":5421,"context_line":"                            bdm.volume_id in old_attachments):"},{"line_number":5422,"context_line":"                        self.volume_api.attachment_delete(context,"},{"line_number":5423,"context_line":"                                                          bdm.attachment_id)"}],"source_content_type":"text/x-python","patch_set":18,"id":"9f5c4f37_e39f36b2","line":5420,"range":{"start_line":5415,"start_character":12,"end_line":5420,"end_character":71},"updated":"2017-09-21 19:20:31.000000000","message":"We should do this in an excutils.save_and_reraise_exception context. Otherwise if attachment_delete fails, we lose the original exception.","commit_id":"f576578b2e1050def3ec2995fb4438b27a66aab4"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"7b29f9a04263a131d2699345327692fe81fdc5c8","unresolved":false,"context_lines":[{"line_number":5412,"context_line":"                    bdm.attachment_id \u003d attach_ref[\u0027id\u0027]"},{"line_number":5413,"context_line":"                    bdm.save()"},{"line_number":5414,"context_line":"        except Exception as e:"},{"line_number":5415,"context_line":"            # If we raise, migrate_data with the updated attachment ids"},{"line_number":5416,"context_line":"            # will not be returned to the source host for rollback."},{"line_number":5417,"context_line":"            # So we need to rollback new attachments here."},{"line_number":5418,"context_line":"            old_attachments \u003d migrate_data.old_vol_attachment_ids"},{"line_number":5419,"context_line":"            for bdm in bdms:"},{"line_number":5420,"context_line":"                if (bdm.is_volume and bdm.attachment_id is not None and"},{"line_number":5421,"context_line":"                            bdm.volume_id in old_attachments):"},{"line_number":5422,"context_line":"                        self.volume_api.attachment_delete(context,"},{"line_number":5423,"context_line":"                                                          bdm.attachment_id)"}],"source_content_type":"text/x-python","patch_set":18,"id":"7f515b1d_ad9da55a","line":5420,"range":{"start_line":5415,"start_character":12,"end_line":5420,"end_character":71},"in_reply_to":"9f5c4f37_e39f36b2","updated":"2017-09-25 20:57:58.000000000","message":"Done","commit_id":"f576578b2e1050def3ec2995fb4438b27a66aab4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"54a5a32260552d80102acebfb8d180921119a0c6","unresolved":false,"context_lines":[{"line_number":5418,"context_line":"            old_attachments \u003d migrate_data.old_vol_attachment_ids"},{"line_number":5419,"context_line":"            for bdm in bdms:"},{"line_number":5420,"context_line":"                if (bdm.is_volume and bdm.attachment_id is not None and"},{"line_number":5421,"context_line":"                            bdm.volume_id in old_attachments):"},{"line_number":5422,"context_line":"                        self.volume_api.attachment_delete(context,"},{"line_number":5423,"context_line":"                                                          bdm.attachment_id)"},{"line_number":5424,"context_line":"                        bdm.attachment_id \u003d old_attachments[bdm.volume_id]"},{"line_number":5425,"context_line":"                        bdm.save()"},{"line_number":5426,"context_line":"            raise e"},{"line_number":5427,"context_line":""},{"line_number":5428,"context_line":"        # _get_instance_block_device_info calls refresh_conn_info,"}],"source_content_type":"text/x-python","patch_set":18,"id":"9f5c4f37_c36f92c3","line":5425,"range":{"start_line":5421,"start_character":0,"end_line":5425,"end_character":34},"updated":"2017-09-21 19:20:31.000000000","message":"Drop the intend one tab?","commit_id":"f576578b2e1050def3ec2995fb4438b27a66aab4"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"7b29f9a04263a131d2699345327692fe81fdc5c8","unresolved":false,"context_lines":[{"line_number":5418,"context_line":"            old_attachments \u003d migrate_data.old_vol_attachment_ids"},{"line_number":5419,"context_line":"            for bdm in bdms:"},{"line_number":5420,"context_line":"                if (bdm.is_volume and bdm.attachment_id is not None and"},{"line_number":5421,"context_line":"                            bdm.volume_id in old_attachments):"},{"line_number":5422,"context_line":"                        self.volume_api.attachment_delete(context,"},{"line_number":5423,"context_line":"                                                          bdm.attachment_id)"},{"line_number":5424,"context_line":"                        bdm.attachment_id \u003d old_attachments[bdm.volume_id]"},{"line_number":5425,"context_line":"                        bdm.save()"},{"line_number":5426,"context_line":"            raise e"},{"line_number":5427,"context_line":""},{"line_number":5428,"context_line":"        # _get_instance_block_device_info calls refresh_conn_info,"}],"source_content_type":"text/x-python","patch_set":18,"id":"7f515b1d_adb6c5d8","line":5425,"range":{"start_line":5421,"start_character":0,"end_line":5425,"end_character":34},"in_reply_to":"9f5c4f37_c36f92c3","updated":"2017-09-25 20:57:58.000000000","message":"Done","commit_id":"f576578b2e1050def3ec2995fb4438b27a66aab4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"54a5a32260552d80102acebfb8d180921119a0c6","unresolved":false,"context_lines":[{"line_number":5425,"context_line":"                        bdm.save()"},{"line_number":5426,"context_line":"            raise e"},{"line_number":5427,"context_line":""},{"line_number":5428,"context_line":"        # _get_instance_block_device_info calls refresh_conn_info,"},{"line_number":5429,"context_line":"        # which calls cinder attachment_update (if new cinder attachment"},{"line_number":5430,"context_line":"        # flow)"},{"line_number":5431,"context_line":"        block_device_info \u003d self._get_instance_block_device_info("},{"line_number":5432,"context_line":"                            context, instance, refresh_conn_info\u003dTrue)"},{"line_number":5433,"context_line":""}],"source_content_type":"text/x-python","patch_set":18,"id":"9f5c4f37_a3d01e52","line":5430,"range":{"start_line":5428,"start_character":8,"end_line":5430,"end_character":15},"updated":"2017-09-21 19:20:31.000000000","message":"It does? It doesn\u0027t happen in this change. It happens in this change:\n\nhttps://review.openstack.org/#/c/330285/141/nova/virt/block_device.py@527\n\nAnd that change is dependent on this change, so we\u0027re in a weird chicken-and-egg situation. This is working for you since you\u0027re testing the changes together.\n\nMaybe it\u0027s OK to leave this though since we won\u0027t call attachment_complete below until we start allowing the new flow, which is https://review.openstack.org/#/c/330285.","commit_id":"f576578b2e1050def3ec2995fb4438b27a66aab4"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"7b29f9a04263a131d2699345327692fe81fdc5c8","unresolved":false,"context_lines":[{"line_number":5425,"context_line":"                        bdm.save()"},{"line_number":5426,"context_line":"            raise e"},{"line_number":5427,"context_line":""},{"line_number":5428,"context_line":"        # _get_instance_block_device_info calls refresh_conn_info,"},{"line_number":5429,"context_line":"        # which calls cinder attachment_update (if new cinder attachment"},{"line_number":5430,"context_line":"        # flow)"},{"line_number":5431,"context_line":"        block_device_info \u003d self._get_instance_block_device_info("},{"line_number":5432,"context_line":"                            context, instance, refresh_conn_info\u003dTrue)"},{"line_number":5433,"context_line":""}],"source_content_type":"text/x-python","patch_set":18,"id":"7f515b1d_0d2b99eb","line":5430,"range":{"start_line":5428,"start_character":8,"end_line":5430,"end_character":15},"in_reply_to":"9f5c4f37_a3d01e52","updated":"2017-09-25 20:57:58.000000000","message":"This should be OK. The comment does mention it only happens in the new flow. And, yes, attachment_complete won\u0027t be called until the new flow is in place.\n\nbtw, The goal of this migrate change was to have migrate work with or without the new attach change.","commit_id":"f576578b2e1050def3ec2995fb4438b27a66aab4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"54a5a32260552d80102acebfb8d180921119a0c6","unresolved":false,"context_lines":[{"line_number":5428,"context_line":"        # _get_instance_block_device_info calls refresh_conn_info,"},{"line_number":5429,"context_line":"        # which calls cinder attachment_update (if new cinder attachment"},{"line_number":5430,"context_line":"        # flow)"},{"line_number":5431,"context_line":"        block_device_info \u003d self._get_instance_block_device_info("},{"line_number":5432,"context_line":"                            context, instance, refresh_conn_info\u003dTrue)"},{"line_number":5433,"context_line":""},{"line_number":5434,"context_line":"        network_info \u003d self.network_api.get_instance_nw_info(context, instance)"}],"source_content_type":"text/x-python","patch_set":18,"id":"9f5c4f37_0707f71e","line":5431,"range":{"start_line":5431,"start_character":33,"end_line":5431,"end_character":64},"updated":"2017-09-21 19:20:31.000000000","message":"Pass the bdms in here otherwise we\u0027re pulling them all over again from the database.","commit_id":"f576578b2e1050def3ec2995fb4438b27a66aab4"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"7b29f9a04263a131d2699345327692fe81fdc5c8","unresolved":false,"context_lines":[{"line_number":5428,"context_line":"        # _get_instance_block_device_info calls refresh_conn_info,"},{"line_number":5429,"context_line":"        # which calls cinder attachment_update (if new cinder attachment"},{"line_number":5430,"context_line":"        # flow)"},{"line_number":5431,"context_line":"        block_device_info \u003d self._get_instance_block_device_info("},{"line_number":5432,"context_line":"                            context, instance, refresh_conn_info\u003dTrue)"},{"line_number":5433,"context_line":""},{"line_number":5434,"context_line":"        network_info \u003d self.network_api.get_instance_nw_info(context, instance)"}],"source_content_type":"text/x-python","patch_set":18,"id":"7f515b1d_9040c037","line":5431,"range":{"start_line":5431,"start_character":33,"end_line":5431,"end_character":64},"in_reply_to":"9f5c4f37_0707f71e","updated":"2017-09-25 20:57:58.000000000","message":"Done","commit_id":"f576578b2e1050def3ec2995fb4438b27a66aab4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"54a5a32260552d80102acebfb8d180921119a0c6","unresolved":false,"context_lines":[{"line_number":5444,"context_line":"                                       migrate_data)"},{"line_number":5445,"context_line":"        LOG.debug(\u0027driver pre_live_migration data is %s\u0027, migrate_data)"},{"line_number":5446,"context_line":""},{"line_number":5447,"context_line":"        # Volume connections are complete, tell cinder that all the"},{"line_number":5448,"context_line":"        # attachments have completed."},{"line_number":5449,"context_line":"        for bdm in bdms:"},{"line_number":5450,"context_line":"            if bdm.is_volume and bdm.attachment_id is not None:"}],"source_content_type":"text/x-python","patch_set":18,"id":"9f5c4f37_e364d6bb","line":5447,"range":{"start_line":5447,"start_character":10,"end_line":5447,"end_character":41},"updated":"2017-09-21 19:20:31.000000000","message":"The bdms are connected to the dest host in driver.pre_live_migration, so yeah.","commit_id":"f576578b2e1050def3ec2995fb4438b27a66aab4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"54a5a32260552d80102acebfb8d180921119a0c6","unresolved":false,"context_lines":[{"line_number":5695,"context_line":"                    # cinder v3.44 api flow - delete the old attachment"},{"line_number":5696,"context_line":"                    # for the source host"},{"line_number":5697,"context_line":"                    old_attachment_id \u003d \\"},{"line_number":5698,"context_line":"                        migrate_data.old_vol_attachment_ids[bdm.volume_id]"},{"line_number":5699,"context_line":"                    self.volume_api.attachment_delete(ctxt, old_attachment_id)"},{"line_number":5700,"context_line":""},{"line_number":5701,"context_line":"        # Releasing vlan."}],"source_content_type":"text/x-python","patch_set":18,"id":"7f515b1d_c646a222","line":5698,"range":{"start_line":5698,"start_character":24,"end_line":5698,"end_character":74},"updated":"2017-09-21 19:20:31.000000000","message":"So I guess we don\u0027t need to worry about a KeyError here because if bdm.attachment_id is not None, all computes should be upgraded to Queens with the new flow support, and the migrate_data object should have been populated properly.","commit_id":"f576578b2e1050def3ec2995fb4438b27a66aab4"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"7b29f9a04263a131d2699345327692fe81fdc5c8","unresolved":false,"context_lines":[{"line_number":5695,"context_line":"                    # cinder v3.44 api flow - delete the old attachment"},{"line_number":5696,"context_line":"                    # for the source host"},{"line_number":5697,"context_line":"                    old_attachment_id \u003d \\"},{"line_number":5698,"context_line":"                        migrate_data.old_vol_attachment_ids[bdm.volume_id]"},{"line_number":5699,"context_line":"                    self.volume_api.attachment_delete(ctxt, old_attachment_id)"},{"line_number":5700,"context_line":""},{"line_number":5701,"context_line":"        # Releasing vlan."}],"source_content_type":"text/x-python","patch_set":18,"id":"7f515b1d_4d763109","line":5698,"range":{"start_line":5698,"start_character":24,"end_line":5698,"end_character":74},"in_reply_to":"7f515b1d_c646a222","updated":"2017-09-25 20:57:58.000000000","message":"I usually like to gracefully handle nulls, but I thought it better to not do that here because if this throws that means there\u0027s a bad code bug here and I\u0027d want to catch it before it gets merged.","commit_id":"f576578b2e1050def3ec2995fb4438b27a66aab4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"54a5a32260552d80102acebfb8d180921119a0c6","unresolved":false,"context_lines":[{"line_number":5902,"context_line":"                self.compute_rpcapi.remove_volume_connection("},{"line_number":5903,"context_line":"                        context, instance, bdm.volume_id, dest)"},{"line_number":5904,"context_line":""},{"line_number":5905,"context_line":"                if bdm.attachment_id:"},{"line_number":5906,"context_line":"                    # 3.44 cinder api flow. Set the bdm\u0027s"},{"line_number":5907,"context_line":"                    # attachment_id to the old attachment of the source"},{"line_number":5908,"context_line":"                    # host. If old_attachments is not there, then"},{"line_number":5909,"context_line":"                    # there was an error before the new attachment was made."},{"line_number":5910,"context_line":"                    old_attachments \u003d migrate_data.old_vol_attachment_ids \\"},{"line_number":5911,"context_line":"                        if \u0027old_vol_attachment_ids\u0027 in migrate_data else None"},{"line_number":5912,"context_line":"                    if old_attachments and bdm.volume_id in old_attachments:"},{"line_number":5913,"context_line":"                        self.volume_api.attachment_delete(context,"},{"line_number":5914,"context_line":"                                                          bdm.attachment_id)"},{"line_number":5915,"context_line":"                        bdm.attachment_id \u003d old_attachments[bdm.volume_id]"},{"line_number":5916,"context_line":"                        bdm.save()"},{"line_number":5917,"context_line":""},{"line_number":5918,"context_line":"        self._notify_about_instance_usage(context, instance,"},{"line_number":5919,"context_line":"                                          \"live_migration._rollback.start\")"}],"source_content_type":"text/x-python","patch_set":18,"id":"9f5c4f37_ec2a888e","line":5916,"range":{"start_line":5905,"start_character":16,"end_line":5916,"end_character":34},"updated":"2017-09-21 19:20:31.000000000","message":"Why aren\u0027t we doing this in remove_volume_connection? I suppose because we don\u0027t have the migrate_data there.\n\nMaybe we should add a TODO to make remove_volume_connection take the migrate_data parameter. It\u0027s an RPC call so we\u0027d have to bump the version and handle older computes still, but it would serve as a reminder that we could consolidate this code in one place.","commit_id":"f576578b2e1050def3ec2995fb4438b27a66aab4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0084b9a0d261fbe810036ea28d162a3853348f19","unresolved":false,"context_lines":[{"line_number":5902,"context_line":"                self.compute_rpcapi.remove_volume_connection("},{"line_number":5903,"context_line":"                        context, instance, bdm.volume_id, dest)"},{"line_number":5904,"context_line":""},{"line_number":5905,"context_line":"                if bdm.attachment_id:"},{"line_number":5906,"context_line":"                    # 3.44 cinder api flow. Set the bdm\u0027s"},{"line_number":5907,"context_line":"                    # attachment_id to the old attachment of the source"},{"line_number":5908,"context_line":"                    # host. If old_attachments is not there, then"},{"line_number":5909,"context_line":"                    # there was an error before the new attachment was made."},{"line_number":5910,"context_line":"                    old_attachments \u003d migrate_data.old_vol_attachment_ids \\"},{"line_number":5911,"context_line":"                        if \u0027old_vol_attachment_ids\u0027 in migrate_data else None"},{"line_number":5912,"context_line":"                    if old_attachments and bdm.volume_id in old_attachments:"},{"line_number":5913,"context_line":"                        self.volume_api.attachment_delete(context,"},{"line_number":5914,"context_line":"                                                          bdm.attachment_id)"},{"line_number":5915,"context_line":"                        bdm.attachment_id \u003d old_attachments[bdm.volume_id]"},{"line_number":5916,"context_line":"                        bdm.save()"},{"line_number":5917,"context_line":""},{"line_number":5918,"context_line":"        self._notify_about_instance_usage(context, instance,"},{"line_number":5919,"context_line":"                                          \"live_migration._rollback.start\")"}],"source_content_type":"text/x-python","patch_set":18,"id":"7f515b1d_fd7a99b7","line":5916,"range":{"start_line":5905,"start_character":16,"end_line":5916,"end_character":34},"in_reply_to":"7f515b1d_d02198a1","updated":"2017-09-28 16:13:34.000000000","message":"I just don\u0027t like dealing with old and new flows in multiple places. We could always get the BDM from the database in remove_volume_connection (or pass it over RPC). Anyway, was just asking for a TODO to consider consolidating that at some point, but then again by the time we\u0027d get around to doing that, we\u0027d probably be talking about dropping the old flow anyway, so meh. This will be our little secret.","commit_id":"f576578b2e1050def3ec2995fb4438b27a66aab4"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"2303fd1084aede17c3f4c0f2a78ac43a483773c4","unresolved":false,"context_lines":[{"line_number":5902,"context_line":"                self.compute_rpcapi.remove_volume_connection("},{"line_number":5903,"context_line":"                        context, instance, bdm.volume_id, dest)"},{"line_number":5904,"context_line":""},{"line_number":5905,"context_line":"                if bdm.attachment_id:"},{"line_number":5906,"context_line":"                    # 3.44 cinder api flow. Set the bdm\u0027s"},{"line_number":5907,"context_line":"                    # attachment_id to the old attachment of the source"},{"line_number":5908,"context_line":"                    # host. If old_attachments is not there, then"},{"line_number":5909,"context_line":"                    # there was an error before the new attachment was made."},{"line_number":5910,"context_line":"                    old_attachments \u003d migrate_data.old_vol_attachment_ids \\"},{"line_number":5911,"context_line":"                        if \u0027old_vol_attachment_ids\u0027 in migrate_data else None"},{"line_number":5912,"context_line":"                    if old_attachments and bdm.volume_id in old_attachments:"},{"line_number":5913,"context_line":"                        self.volume_api.attachment_delete(context,"},{"line_number":5914,"context_line":"                                                          bdm.attachment_id)"},{"line_number":5915,"context_line":"                        bdm.attachment_id \u003d old_attachments[bdm.volume_id]"},{"line_number":5916,"context_line":"                        bdm.save()"},{"line_number":5917,"context_line":""},{"line_number":5918,"context_line":"        self._notify_about_instance_usage(context, instance,"},{"line_number":5919,"context_line":"                                          \"live_migration._rollback.start\")"}],"source_content_type":"text/x-python","patch_set":18,"id":"7f515b1d_d2c8e722","line":5916,"range":{"start_line":5905,"start_character":16,"end_line":5916,"end_character":34},"in_reply_to":"7f515b1d_fd7a99b7","updated":"2017-09-28 17:31:26.000000000","message":"ok, mum\u0027s the word.","commit_id":"f576578b2e1050def3ec2995fb4438b27a66aab4"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"7b29f9a04263a131d2699345327692fe81fdc5c8","unresolved":false,"context_lines":[{"line_number":5902,"context_line":"                self.compute_rpcapi.remove_volume_connection("},{"line_number":5903,"context_line":"                        context, instance, bdm.volume_id, dest)"},{"line_number":5904,"context_line":""},{"line_number":5905,"context_line":"                if bdm.attachment_id:"},{"line_number":5906,"context_line":"                    # 3.44 cinder api flow. Set the bdm\u0027s"},{"line_number":5907,"context_line":"                    # attachment_id to the old attachment of the source"},{"line_number":5908,"context_line":"                    # host. If old_attachments is not there, then"},{"line_number":5909,"context_line":"                    # there was an error before the new attachment was made."},{"line_number":5910,"context_line":"                    old_attachments \u003d migrate_data.old_vol_attachment_ids \\"},{"line_number":5911,"context_line":"                        if \u0027old_vol_attachment_ids\u0027 in migrate_data else None"},{"line_number":5912,"context_line":"                    if old_attachments and bdm.volume_id in old_attachments:"},{"line_number":5913,"context_line":"                        self.volume_api.attachment_delete(context,"},{"line_number":5914,"context_line":"                                                          bdm.attachment_id)"},{"line_number":5915,"context_line":"                        bdm.attachment_id \u003d old_attachments[bdm.volume_id]"},{"line_number":5916,"context_line":"                        bdm.save()"},{"line_number":5917,"context_line":""},{"line_number":5918,"context_line":"        self._notify_about_instance_usage(context, instance,"},{"line_number":5919,"context_line":"                                          \"live_migration._rollback.start\")"}],"source_content_type":"text/x-python","patch_set":18,"id":"7f515b1d_d02198a1","line":5916,"range":{"start_line":5905,"start_character":16,"end_line":5916,"end_character":34},"in_reply_to":"9f5c4f37_ec2a888e","updated":"2017-09-25 20:57:58.000000000","message":"remove_volume_connection would need both migrate_data and the whole bdm (for save). Seems simpler just to leave it as is.","commit_id":"f576578b2e1050def3ec2995fb4438b27a66aab4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0084b9a0d261fbe810036ea28d162a3853348f19","unresolved":false,"context_lines":[{"line_number":5075,"context_line":"                          \"for volume %(volume)s\", {\u0027volume\u0027: conn_volume},"},{"line_number":5076,"context_line":"                          instance\u003dinstance)"},{"line_number":5077,"context_line":"                if bdm.attachment_id is None:"},{"line_number":5078,"context_line":"                    # This is the pre-3.44 flow for new-style volume"},{"line_number":5079,"context_line":"                    # attachments so just terminate the connection."},{"line_number":5080,"context_line":"                    self.volume_api.terminate_connection(context,"},{"line_number":5081,"context_line":"                                                         conn_volume,"}],"source_content_type":"text/x-python","patch_set":20,"id":"7f515b1d_9d9cc50e","line":5078,"updated":"2017-09-28 16:13:34.000000000","message":"The version changes in the comments for the swap volume flow could happen in a separate change so they don\u0027t muddy up the live migration patch. Or they could be part of the final change which adds the API support.","commit_id":"f261ce45d44e160f6988ee87981a2342fd018b2c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"33074063f30d9c204f42287561ec4183d08d21bc","unresolved":false,"context_lines":[{"line_number":5075,"context_line":"                          \"for volume %(volume)s\", {\u0027volume\u0027: conn_volume},"},{"line_number":5076,"context_line":"                          instance\u003dinstance)"},{"line_number":5077,"context_line":"                if bdm.attachment_id is None:"},{"line_number":5078,"context_line":"                    # This is the pre-3.44 flow for new-style volume"},{"line_number":5079,"context_line":"                    # attachments so just terminate the connection."},{"line_number":5080,"context_line":"                    self.volume_api.terminate_connection(context,"},{"line_number":5081,"context_line":"                                                         conn_volume,"}],"source_content_type":"text/x-python","patch_set":20,"id":"7f515b1d_afd98457","line":5078,"in_reply_to":"7f515b1d_9d9cc50e","updated":"2017-09-28 16:23:17.000000000","message":"Nevermind, these must have happened separately, I was looking at a diff and these showed up, so ignore.","commit_id":"f261ce45d44e160f6988ee87981a2342fd018b2c"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"9f38155c37302b1c28b0ef8046f862c21d02ff0d","unresolved":false,"context_lines":[{"line_number":5311,"context_line":"                    context, volume_id, instance.uuid)"},{"line_number":5312,"context_line":"            driver_bdm \u003d driver_block_device.convert_volume(bdm)"},{"line_number":5313,"context_line":"            driver_bdm.driver_detach(context, instance,"},{"line_number":5314,"context_line":"                                     self.volume_api, self.driver)"},{"line_number":5315,"context_line":"            if bdm.attachment_id is None:"},{"line_number":5316,"context_line":"                # cinder v2 api flow"},{"line_number":5317,"context_line":"                connector \u003d self.driver.get_volume_connector(instance)"}],"source_content_type":"text/x-python","patch_set":24,"id":"5f4e5783_24028c0c","line":5314,"updated":"2017-10-13 16:50:06.000000000","message":"On line 290 in virt/block_device.py I see we call volume_api.roll_detaching(...) in certain error cases. That seems odd in this context, I guess it would just fail silently in this case because we don\u0027t start of the detaching, because the volume overall is staying attached?\n\nIf that is a problem, lets fix that in a follow up patch.","commit_id":"c3b5fdbb044d228fc4fc15bd7e9fc850363a75e7"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"2e05e83673ef55ea669237d6e3e379d6c7a33da6","unresolved":false,"context_lines":[{"line_number":5311,"context_line":"                    context, volume_id, instance.uuid)"},{"line_number":5312,"context_line":"            driver_bdm \u003d driver_block_device.convert_volume(bdm)"},{"line_number":5313,"context_line":"            driver_bdm.driver_detach(context, instance,"},{"line_number":5314,"context_line":"                                     self.volume_api, self.driver)"},{"line_number":5315,"context_line":"            if bdm.attachment_id is None:"},{"line_number":5316,"context_line":"                # cinder v2 api flow"},{"line_number":5317,"context_line":"                connector \u003d self.driver.get_volume_connector(instance)"}],"source_content_type":"text/x-python","patch_set":24,"id":"3f4b6375_74c5298e","line":5314,"in_reply_to":"3f4b6375_919e8905","updated":"2017-10-19 14:35:26.000000000","message":"Ah, good catch. We can only blame ourselves for that one I guess, lets fix that separately.","commit_id":"c3b5fdbb044d228fc4fc15bd7e9fc850363a75e7"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ce50f93da2b5c7855a4d5519642e9c9469694567","unresolved":false,"context_lines":[{"line_number":5311,"context_line":"                    context, volume_id, instance.uuid)"},{"line_number":5312,"context_line":"            driver_bdm \u003d driver_block_device.convert_volume(bdm)"},{"line_number":5313,"context_line":"            driver_bdm.driver_detach(context, instance,"},{"line_number":5314,"context_line":"                                     self.volume_api, self.driver)"},{"line_number":5315,"context_line":"            if bdm.attachment_id is None:"},{"line_number":5316,"context_line":"                # cinder v2 api flow"},{"line_number":5317,"context_line":"                connector \u003d self.driver.get_volume_connector(instance)"}],"source_content_type":"text/x-python","patch_set":24,"id":"3f4b6375_919e8905","line":5314,"in_reply_to":"3f4b6375_f1f93d4d","updated":"2017-10-19 00:58:28.000000000","message":"Yup, added as a result of this change:\n\nI7a53e08f3fad6abb27a1d8ad425b4f916341cab3\n\nHowever, remove_volume_connection was calling the old _driver_volume_detach which call roll_detaching if the detach failed anyway, so it\u0027s latent.","commit_id":"c3b5fdbb044d228fc4fc15bd7e9fc850363a75e7"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b025ec695a40f1908a3bd6f9d178f5d0f4db9b41","unresolved":false,"context_lines":[{"line_number":5311,"context_line":"                    context, volume_id, instance.uuid)"},{"line_number":5312,"context_line":"            driver_bdm \u003d driver_block_device.convert_volume(bdm)"},{"line_number":5313,"context_line":"            driver_bdm.driver_detach(context, instance,"},{"line_number":5314,"context_line":"                                     self.volume_api, self.driver)"},{"line_number":5315,"context_line":"            if bdm.attachment_id is None:"},{"line_number":5316,"context_line":"                # cinder v2 api flow"},{"line_number":5317,"context_line":"                connector \u003d self.driver.get_volume_connector(instance)"}],"source_content_type":"text/x-python","patch_set":24,"id":"3f4b6375_f1f93d4d","line":5314,"in_reply_to":"5f4e5783_09b884e2","updated":"2017-10-19 00:54:01.000000000","message":"Was this maybe regressed when Lee refactored a bunch of the code in Pike and moved a lot of the volume detach code from the compute manager into the block device module? Maybe moved too much stuff?","commit_id":"c3b5fdbb044d228fc4fc15bd7e9fc850363a75e7"},{"author":{"_account_id":9562,"name":"Ildiko Vancsa","email":"ildiko.vancsa@gmail.com","username":"ildikov"},"change_message_id":"6b265cf2fd33fd623063ac9aa0f1e5301245f6a0","unresolved":false,"context_lines":[{"line_number":5311,"context_line":"                    context, volume_id, instance.uuid)"},{"line_number":5312,"context_line":"            driver_bdm \u003d driver_block_device.convert_volume(bdm)"},{"line_number":5313,"context_line":"            driver_bdm.driver_detach(context, instance,"},{"line_number":5314,"context_line":"                                     self.volume_api, self.driver)"},{"line_number":5315,"context_line":"            if bdm.attachment_id is None:"},{"line_number":5316,"context_line":"                # cinder v2 api flow"},{"line_number":5317,"context_line":"                connector \u003d self.driver.get_volume_connector(instance)"}],"source_content_type":"text/x-python","patch_set":24,"id":"5f4e5783_97116655","line":5314,"in_reply_to":"5f4e5783_24028c0c","updated":"2017-10-16 14:21:21.000000000","message":"As this bit of the code hasn\u0027t changed that would be a problem in the old flow too. Or am I missing something?","commit_id":"c3b5fdbb044d228fc4fc15bd7e9fc850363a75e7"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"3397e0fda00d1b1a1914789ed1d06202889ae65a","unresolved":false,"context_lines":[{"line_number":5311,"context_line":"                    context, volume_id, instance.uuid)"},{"line_number":5312,"context_line":"            driver_bdm \u003d driver_block_device.convert_volume(bdm)"},{"line_number":5313,"context_line":"            driver_bdm.driver_detach(context, instance,"},{"line_number":5314,"context_line":"                                     self.volume_api, self.driver)"},{"line_number":5315,"context_line":"            if bdm.attachment_id is None:"},{"line_number":5316,"context_line":"                # cinder v2 api flow"},{"line_number":5317,"context_line":"                connector \u003d self.driver.get_volume_connector(instance)"}],"source_content_type":"text/x-python","patch_set":24,"id":"5f4e5783_09b884e2","line":5314,"in_reply_to":"5f4e5783_97116655","updated":"2017-10-17 08:26:04.000000000","message":"Good question, it probably is an existing problem. Either way I think we could raise a bug for this bit, and worry about it in a follow on patch.","commit_id":"c3b5fdbb044d228fc4fc15bd7e9fc850363a75e7"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"9f38155c37302b1c28b0ef8046f862c21d02ff0d","unresolved":false,"context_lines":[{"line_number":5316,"context_line":"                # cinder v2 api flow"},{"line_number":5317,"context_line":"                connector \u003d self.driver.get_volume_connector(instance)"},{"line_number":5318,"context_line":"                self.volume_api.terminate_connection(context, volume_id,"},{"line_number":5319,"context_line":"                                                     connector)"},{"line_number":5320,"context_line":"        except exception.NotFound:"},{"line_number":5321,"context_line":"            pass"},{"line_number":5322,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"5f4e5783_847f0014","line":5319,"updated":"2017-10-13 16:50:06.000000000","message":"Nit: I think I would prefer that lines 6069-6080 lived here, as its really the other half of this if statement, and would fail together or work together if we loose rabbit half way through this. But I can see arguments for keeping the other piece at the other side too.","commit_id":"c3b5fdbb044d228fc4fc15bd7e9fc850363a75e7"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"15d6e1063e221fca3a6bf52de46b7a0a9e151efe","unresolved":false,"context_lines":[{"line_number":5316,"context_line":"                # cinder v2 api flow"},{"line_number":5317,"context_line":"                connector \u003d self.driver.get_volume_connector(instance)"},{"line_number":5318,"context_line":"                self.volume_api.terminate_connection(context, volume_id,"},{"line_number":5319,"context_line":"                                                     connector)"},{"line_number":5320,"context_line":"        except exception.NotFound:"},{"line_number":5321,"context_line":"            pass"},{"line_number":5322,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"5f4e5783_a1c78996","line":5319,"in_reply_to":"5f4e5783_5e7e74af","updated":"2017-10-13 17:53:38.000000000","message":"ah, got it.","commit_id":"c3b5fdbb044d228fc4fc15bd7e9fc850363a75e7"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"07b3bc343bf3885e600228a128edcfee6c5bdc00","unresolved":false,"context_lines":[{"line_number":5316,"context_line":"                # cinder v2 api flow"},{"line_number":5317,"context_line":"                connector \u003d self.driver.get_volume_connector(instance)"},{"line_number":5318,"context_line":"                self.volume_api.terminate_connection(context, volume_id,"},{"line_number":5319,"context_line":"                                                     connector)"},{"line_number":5320,"context_line":"        except exception.NotFound:"},{"line_number":5321,"context_line":"            pass"},{"line_number":5322,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"5f4e5783_5e7e74af","line":5319,"in_reply_to":"5f4e5783_847f0014","updated":"2017-10-13 17:48:25.000000000","message":"I brought this up earlier too, the problem is we don\u0027t have migrate_data here. We could pass it, but that wouldn\u0027t work if the dest is a Pike compute node that doesn\u0027t understand getting a migrate_data parameter (and the RPC API code would have to pop it out). It\u0027s a future cleanup for sure.\n\nhttps://review.openstack.org/#/c/463987/18/nova/compute/manager.py@5916","commit_id":"c3b5fdbb044d228fc4fc15bd7e9fc850363a75e7"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"9f38155c37302b1c28b0ef8046f862c21d02ff0d","unresolved":false,"context_lines":[{"line_number":5537,"context_line":"                migrate_data_obj.LiveMigrateData.detect_implementation("},{"line_number":5538,"context_line":"                    migrate_data)"},{"line_number":5539,"context_line":""},{"line_number":5540,"context_line":"        migrate_data.old_vol_attachment_ids \u003d {}"},{"line_number":5541,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":5542,"context_line":"            context, instance.uuid)"},{"line_number":5543,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":24,"id":"5f4e5783_8fc71997","line":5540,"updated":"2017-10-13 16:50:06.000000000","message":"I think I was expecting to store this in the BDM, but this is probably better.","commit_id":"c3b5fdbb044d228fc4fc15bd7e9fc850363a75e7"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"07b3bc343bf3885e600228a128edcfee6c5bdc00","unresolved":false,"context_lines":[{"line_number":5537,"context_line":"                migrate_data_obj.LiveMigrateData.detect_implementation("},{"line_number":5538,"context_line":"                    migrate_data)"},{"line_number":5539,"context_line":""},{"line_number":5540,"context_line":"        migrate_data.old_vol_attachment_ids \u003d {}"},{"line_number":5541,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":5542,"context_line":"            context, instance.uuid)"},{"line_number":5543,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":24,"id":"5f4e5783_de43846b","line":5540,"in_reply_to":"5f4e5783_8fc71997","updated":"2017-10-13 17:48:25.000000000","message":"Yeah we want this stuff in the un-persisted short-lived migrate data object.","commit_id":"c3b5fdbb044d228fc4fc15bd7e9fc850363a75e7"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"15d6e1063e221fca3a6bf52de46b7a0a9e151efe","unresolved":false,"context_lines":[{"line_number":5537,"context_line":"                migrate_data_obj.LiveMigrateData.detect_implementation("},{"line_number":5538,"context_line":"                    migrate_data)"},{"line_number":5539,"context_line":""},{"line_number":5540,"context_line":"        migrate_data.old_vol_attachment_ids \u003d {}"},{"line_number":5541,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":5542,"context_line":"            context, instance.uuid)"},{"line_number":5543,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":24,"id":"5f4e5783_013e5d8b","line":5540,"in_reply_to":"5f4e5783_de43846b","updated":"2017-10-13 17:53:38.000000000","message":"I guess I am worried about when we drop that info on the floor across a nova-compute restart, but that is for another day.","commit_id":"c3b5fdbb044d228fc4fc15bd7e9fc850363a75e7"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"9f38155c37302b1c28b0ef8046f862c21d02ff0d","unresolved":false,"context_lines":[{"line_number":5578,"context_line":"        # _get_instance_block_device_info calls refresh_conn_info,"},{"line_number":5579,"context_line":"        # which calls cinder attachment_update (if new cinder attachment"},{"line_number":5580,"context_line":"        # flow)"},{"line_number":5581,"context_line":"        block_device_info \u003d self._get_instance_block_device_info("},{"line_number":5582,"context_line":"                            context, instance, refresh_conn_info\u003dTrue,"},{"line_number":5583,"context_line":"                            bdms\u003dbdms)"},{"line_number":5584,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"5f4e5783_aa11a306","line":5581,"updated":"2017-10-13 16:50:06.000000000","message":"Hmm, so in here it seems we call into volume_api.initialize_connection, should we not be calling down into update_attachment in this case? Or did I get that all mixed up?\n\nI am thinking we actually call down into here:\nhttps://github.com/openstack/nova/blob/d9212edb8fe8376c218cc40e0bee1540083b25e6/nova/virt/block_device.py#L445\n\nBut I was expecting to something more like here:\nhttps://github.com/openstack/nova/blob/d9212edb8fe8376c218cc40e0bee1540083b25e6/nova/compute/manager.py#L5117","commit_id":"c3b5fdbb044d228fc4fc15bd7e9fc850363a75e7"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"3397e0fda00d1b1a1914789ed1d06202889ae65a","unresolved":false,"context_lines":[{"line_number":5578,"context_line":"        # _get_instance_block_device_info calls refresh_conn_info,"},{"line_number":5579,"context_line":"        # which calls cinder attachment_update (if new cinder attachment"},{"line_number":5580,"context_line":"        # flow)"},{"line_number":5581,"context_line":"        block_device_info \u003d self._get_instance_block_device_info("},{"line_number":5582,"context_line":"                            context, instance, refresh_conn_info\u003dTrue,"},{"line_number":5583,"context_line":"                            bdms\u003dbdms)"},{"line_number":5584,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"5f4e5783_a99df066","line":5581,"in_reply_to":"5f4e5783_304dfcc3","updated":"2017-10-17 08:26:04.000000000","message":"I just worry we forget about this flow when reviewing the next patch, I sure did last time I reviewed that patch :(","commit_id":"c3b5fdbb044d228fc4fc15bd7e9fc850363a75e7"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"07b3bc343bf3885e600228a128edcfee6c5bdc00","unresolved":false,"context_lines":[{"line_number":5578,"context_line":"        # _get_instance_block_device_info calls refresh_conn_info,"},{"line_number":5579,"context_line":"        # which calls cinder attachment_update (if new cinder attachment"},{"line_number":5580,"context_line":"        # flow)"},{"line_number":5581,"context_line":"        block_device_info \u003d self._get_instance_block_device_info("},{"line_number":5582,"context_line":"                            context, instance, refresh_conn_info\u003dTrue,"},{"line_number":5583,"context_line":"                            bdms\u003dbdms)"},{"line_number":5584,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"5f4e5783_c1596535","line":5581,"in_reply_to":"5f4e5783_9ee86ce2","updated":"2017-10-13 17:48:25.000000000","message":"I brought this up before also:\n\nhttps://review.openstack.org/#/c/463987/18/nova/compute/manager.py@5430\n\nSo this doesn\u0027t do attachment_update until the new patch turns everything on for the new flow, which depends on this change.\n\nThe new issue here, however, is that if we change refresh_connection_info for the new flow to not actually do the attachment_update, because it screws up other flows where it\u0027s not expected to actually be attaching a volume again, we were going to change the new flow to just call attachment_get in refresh_connection_info:\n\nhttps://review.openstack.org/#/c/330285/144/nova/virt/block_device.py@523\n\nThat would break the assertion here that refresh_connection_info is doing the attachment_update for us.\n\nSo yeah that probably needs to be dealt with separately - so if refresh_connection_info doesn\u0027t call attachment_update, we\u0027re going to have to do it here explicitly for the new flow.","commit_id":"c3b5fdbb044d228fc4fc15bd7e9fc850363a75e7"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"a5a522115ea99321966bcf9ce864cbbe6228cd50","unresolved":false,"context_lines":[{"line_number":5578,"context_line":"        # _get_instance_block_device_info calls refresh_conn_info,"},{"line_number":5579,"context_line":"        # which calls cinder attachment_update (if new cinder attachment"},{"line_number":5580,"context_line":"        # flow)"},{"line_number":5581,"context_line":"        block_device_info \u003d self._get_instance_block_device_info("},{"line_number":5582,"context_line":"                            context, instance, refresh_conn_info\u003dTrue,"},{"line_number":5583,"context_line":"                            bdms\u003dbdms)"},{"line_number":5584,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"5f4e5783_9ee86ce2","line":5581,"in_reply_to":"5f4e5783_aa11a306","updated":"2017-10-13 17:16:25.000000000","message":"So I think the best fix may be to call init/update explicitly, update connection info in the bdm, then have refresh_conn_info\u003dFalse? I am not sure really.","commit_id":"c3b5fdbb044d228fc4fc15bd7e9fc850363a75e7"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"15d6e1063e221fca3a6bf52de46b7a0a9e151efe","unresolved":false,"context_lines":[{"line_number":5578,"context_line":"        # _get_instance_block_device_info calls refresh_conn_info,"},{"line_number":5579,"context_line":"        # which calls cinder attachment_update (if new cinder attachment"},{"line_number":5580,"context_line":"        # flow)"},{"line_number":5581,"context_line":"        block_device_info \u003d self._get_instance_block_device_info("},{"line_number":5582,"context_line":"                            context, instance, refresh_conn_info\u003dTrue,"},{"line_number":5583,"context_line":"                            bdms\u003dbdms)"},{"line_number":5584,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"5f4e5783_e13ea189","line":5581,"in_reply_to":"5f4e5783_c1596535","updated":"2017-10-13 17:53:38.000000000","message":"++","commit_id":"c3b5fdbb044d228fc4fc15bd7e9fc850363a75e7"},{"author":{"_account_id":9562,"name":"Ildiko Vancsa","email":"ildiko.vancsa@gmail.com","username":"ildikov"},"change_message_id":"6b265cf2fd33fd623063ac9aa0f1e5301245f6a0","unresolved":false,"context_lines":[{"line_number":5578,"context_line":"        # _get_instance_block_device_info calls refresh_conn_info,"},{"line_number":5579,"context_line":"        # which calls cinder attachment_update (if new cinder attachment"},{"line_number":5580,"context_line":"        # flow)"},{"line_number":5581,"context_line":"        block_device_info \u003d self._get_instance_block_device_info("},{"line_number":5582,"context_line":"                            context, instance, refresh_conn_info\u003dTrue,"},{"line_number":5583,"context_line":"                            bdms\u003dbdms)"},{"line_number":5584,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"5f4e5783_304dfcc3","line":5581,"in_reply_to":"5f4e5783_e13ea189","updated":"2017-10-16 14:21:21.000000000","message":"Would that be possible to merge this patch and do the fix in the new attach patch?\n\nIt is easier if we are not cross depending between these two and the new flow is not enabled until we have the new attach patch merged anyway, so we cannot really mess anything up until that point.","commit_id":"c3b5fdbb044d228fc4fc15bd7e9fc850363a75e7"}],"nova/objects/migrate_data.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1889f63d0dd9a874fecbe18b12ad90ee9ff9cd27","unresolved":false,"context_lines":[{"line_number":63,"context_line":""},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"@obj_base.NovaObjectRegistry.register"},{"line_number":66,"context_line":"class LibvirtLiveMigrateBDMInfo(obj_base.NovaObject):"},{"line_number":67,"context_line":"    VERSION \u003d \u00271.0\u0027"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"    fields \u003d {"}],"source_content_type":"text/x-python","patch_set":8,"id":"5f201791_477b908b","line":66,"updated":"2017-06-21 19:02:49.000000000","message":"Maybe we should have the attachment_id in here? This is the type for the LibvirtLiveMigrateData.bdms field. Although that doesn\u0027t help the other migrate data types for the other virt drivers.","commit_id":"373b01d5d432bbb0c2ddfb71a79d5129c1775b96"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"3f4ba5fa25a33b8f119e1b43fb92f46c6a614913","unresolved":false,"context_lines":[{"line_number":63,"context_line":""},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"@obj_base.NovaObjectRegistry.register"},{"line_number":66,"context_line":"class LibvirtLiveMigrateBDMInfo(obj_base.NovaObject):"},{"line_number":67,"context_line":"    VERSION \u003d \u00271.0\u0027"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"    fields \u003d {"}],"source_content_type":"text/x-python","patch_set":8,"id":"5f201791_0aaa14d1","line":66,"in_reply_to":"5f201791_477b908b","updated":"2017-06-22 21:49:26.000000000","message":"it\u0027s now up in the base class...","commit_id":"373b01d5d432bbb0c2ddfb71a79d5129c1775b96"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1889f63d0dd9a874fecbe18b12ad90ee9ff9cd27","unresolved":false,"context_lines":[{"line_number":110,"context_line":"    # Version 1.2: Added \u0027serial_listen_ports\u0027 to allow live migration with"},{"line_number":111,"context_line":"    #              serial console."},{"line_number":112,"context_line":"    # Version 1.3: Added \u0027supported_perf_events\u0027"},{"line_number":113,"context_line":"    # Version 1.4: Added old_attachments"},{"line_number":114,"context_line":"    VERSION \u003d \u00271.4\u0027"},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"    fields \u003d {"}],"source_content_type":"text/x-python","patch_set":8,"id":"5f201791_67c8cd54","line":113,"range":{"start_line":113,"start_character":25,"end_line":113,"end_character":40},"updated":"2017-06-21 19:02:49.000000000","message":"This isn\u0027t a problem that\u0027s specific to libvirt, right? I\u0027d expect this to be set in the base LiveMigrateData class since it\u0027s something we\u0027ll need to handle for all virt drivers that support live migration with attached volumes, like xen and hyperv too.","commit_id":"373b01d5d432bbb0c2ddfb71a79d5129c1775b96"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"3f4ba5fa25a33b8f119e1b43fb92f46c6a614913","unresolved":false,"context_lines":[{"line_number":110,"context_line":"    # Version 1.2: Added \u0027serial_listen_ports\u0027 to allow live migration with"},{"line_number":111,"context_line":"    #              serial console."},{"line_number":112,"context_line":"    # Version 1.3: Added \u0027supported_perf_events\u0027"},{"line_number":113,"context_line":"    # Version 1.4: Added old_attachments"},{"line_number":114,"context_line":"    VERSION \u003d \u00271.4\u0027"},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"    fields \u003d {"}],"source_content_type":"text/x-python","patch_set":8,"id":"5f201791_f037d9af","line":113,"range":{"start_line":113,"start_character":25,"end_line":113,"end_character":40},"in_reply_to":"5f201791_67c8cd54","updated":"2017-06-22 21:49:26.000000000","message":"I changed the name and added it to the base class.","commit_id":"373b01d5d432bbb0c2ddfb71a79d5129c1775b96"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1889f63d0dd9a874fecbe18b12ad90ee9ff9cd27","unresolved":false,"context_lines":[{"line_number":130,"context_line":"        \u0027bdms\u0027: fields.ListOfObjectsField(\u0027LibvirtLiveMigrateBDMInfo\u0027),"},{"line_number":131,"context_line":"        \u0027target_connect_addr\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":132,"context_line":"        \u0027supported_perf_events\u0027: fields.ListOfStringsField(),"},{"line_number":133,"context_line":"        \u0027old_attachments\u0027: fields.DictOfStringsField(),"},{"line_number":134,"context_line":"    }"},{"line_number":135,"context_line":""},{"line_number":136,"context_line":"    def obj_make_compatible(self, primitive, target_version):"}],"source_content_type":"text/x-python","patch_set":8,"id":"5f201791_47f3919f","line":133,"range":{"start_line":133,"start_character":34,"end_line":133,"end_character":52},"updated":"2017-06-21 19:02:49.000000000","message":"I\u0027m not sure why this is a dict instead of a list. I guess I\u0027ll find out when I see how it\u0027s used.","commit_id":"373b01d5d432bbb0c2ddfb71a79d5129c1775b96"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"7a5025ab4f6a4451ecab17da93d15b887154892e","unresolved":false,"context_lines":[{"line_number":130,"context_line":"        \u0027bdms\u0027: fields.ListOfObjectsField(\u0027LibvirtLiveMigrateBDMInfo\u0027),"},{"line_number":131,"context_line":"        \u0027target_connect_addr\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":132,"context_line":"        \u0027supported_perf_events\u0027: fields.ListOfStringsField(),"},{"line_number":133,"context_line":"        \u0027old_attachments\u0027: fields.DictOfStringsField(),"},{"line_number":134,"context_line":"    }"},{"line_number":135,"context_line":""},{"line_number":136,"context_line":"    def obj_make_compatible(self, primitive, target_version):"}],"source_content_type":"text/x-python","patch_set":8,"id":"5f201791_2a005967","line":133,"range":{"start_line":133,"start_character":9,"end_line":133,"end_character":24},"updated":"2017-06-21 19:12:37.000000000","message":"two things:\n\n1. maybe we should call this \"old_vol_attachment_ids\" or \"source_vol_attachment_ids\" so it\u0027s clear they are volume attachments and not something like network interface attachments (VIFs/ports).\n\n2. A comment about what these are would be useful, i.e. a mapping of BDM volume_id to volume attachment ID for volume attachments on the source host","commit_id":"373b01d5d432bbb0c2ddfb71a79d5129c1775b96"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"3f4ba5fa25a33b8f119e1b43fb92f46c6a614913","unresolved":false,"context_lines":[{"line_number":130,"context_line":"        \u0027bdms\u0027: fields.ListOfObjectsField(\u0027LibvirtLiveMigrateBDMInfo\u0027),"},{"line_number":131,"context_line":"        \u0027target_connect_addr\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":132,"context_line":"        \u0027supported_perf_events\u0027: fields.ListOfStringsField(),"},{"line_number":133,"context_line":"        \u0027old_attachments\u0027: fields.DictOfStringsField(),"},{"line_number":134,"context_line":"    }"},{"line_number":135,"context_line":""},{"line_number":136,"context_line":"    def obj_make_compatible(self, primitive, target_version):"}],"source_content_type":"text/x-python","patch_set":8,"id":"5f201791_f02eb941","line":133,"range":{"start_line":133,"start_character":9,"end_line":133,"end_character":24},"in_reply_to":"5f201791_2a005967","updated":"2017-06-22 21:49:26.000000000","message":"Done","commit_id":"373b01d5d432bbb0c2ddfb71a79d5129c1775b96"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1889f63d0dd9a874fecbe18b12ad90ee9ff9cd27","unresolved":false,"context_lines":[{"line_number":137,"context_line":"        super(LibvirtLiveMigrateData, self).obj_make_compatible("},{"line_number":138,"context_line":"            primitive, target_version)"},{"line_number":139,"context_line":"        target_version \u003d versionutils.convert_version_to_tuple(target_version)"},{"line_number":140,"context_line":"        if target_version \u003c (1, 4):"},{"line_number":141,"context_line":"            if \u0027old_attachments\u0027 in primitive:"},{"line_number":142,"context_line":"                del primitive[\u0027old_attachments\u0027]"},{"line_number":143,"context_line":"        if target_version \u003c (1, 3):"}],"source_content_type":"text/x-python","patch_set":8,"id":"5f201791_c734414d","line":140,"updated":"2017-06-21 19:02:49.000000000","message":"We should probably have a unit test to make sure we pop old_attachments when converting to 1.3 or something.","commit_id":"373b01d5d432bbb0c2ddfb71a79d5129c1775b96"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"3f4ba5fa25a33b8f119e1b43fb92f46c6a614913","unresolved":false,"context_lines":[{"line_number":137,"context_line":"        super(LibvirtLiveMigrateData, self).obj_make_compatible("},{"line_number":138,"context_line":"            primitive, target_version)"},{"line_number":139,"context_line":"        target_version \u003d versionutils.convert_version_to_tuple(target_version)"},{"line_number":140,"context_line":"        if target_version \u003c (1, 4):"},{"line_number":141,"context_line":"            if \u0027old_attachments\u0027 in primitive:"},{"line_number":142,"context_line":"                del primitive[\u0027old_attachments\u0027]"},{"line_number":143,"context_line":"        if target_version \u003c (1, 3):"}],"source_content_type":"text/x-python","patch_set":8,"id":"5f201791_ca5e5c11","line":140,"in_reply_to":"5f201791_c734414d","updated":"2017-06-22 21:49:26.000000000","message":"Done","commit_id":"373b01d5d432bbb0c2ddfb71a79d5129c1775b96"},{"author":{"_account_id":21813,"name":"Andrey Volkov","email":"m@amadev.ru","username":"avolkov"},"change_message_id":"c276a3113ee59d5a5c6992fd48f9b54c9e8b2cc8","unresolved":false,"context_lines":[{"line_number":141,"context_line":"            primitive, target_version)"},{"line_number":142,"context_line":"        target_version \u003d versionutils.convert_version_to_tuple(target_version)"},{"line_number":143,"context_line":"        if target_version \u003c (1, 4):"},{"line_number":144,"context_line":"            if \u0027old_vol_attachment_ids\u0027 in primitive:"},{"line_number":145,"context_line":"                del primitive[\u0027old_vol_attachment_ids\u0027]"},{"line_number":146,"context_line":"        if target_version \u003c (1, 3):"},{"line_number":147,"context_line":"            if \u0027supported_perf_events\u0027 in primitive:"},{"line_number":148,"context_line":"                del primitive[\u0027supported_perf_events\u0027]"}],"source_content_type":"text/x-python","patch_set":13,"id":"1f1a1f67_af0da038","line":145,"range":{"start_line":144,"start_character":0,"end_line":145,"end_character":55},"updated":"2017-07-13 08:57:42.000000000","message":"nit: replace duplication with a method in the base class, maybe?","commit_id":"6298563672edfa6b702ac650ddb25afc18df370b"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"768958900377a6888e7e62b434d365c2598826e4","unresolved":false,"context_lines":[{"line_number":114,"context_line":"    # Version 1.2: Added \u0027serial_listen_ports\u0027 to allow live migration with"},{"line_number":115,"context_line":"    #              serial console."},{"line_number":116,"context_line":"    # Version 1.3: Added \u0027supported_perf_events\u0027"},{"line_number":117,"context_line":"    # Version 1.4: Added old_vol_attachment_ids"},{"line_number":118,"context_line":"    VERSION \u003d \u00271.4\u0027"},{"line_number":119,"context_line":""},{"line_number":120,"context_line":"    fields \u003d {"}],"source_content_type":"text/x-python","patch_set":14,"id":"1f1a1f67_ffb4cff3","line":117,"range":{"start_line":117,"start_character":4,"end_line":117,"end_character":47},"updated":"2017-07-18 16:19:27.000000000","message":"Ah, because we\u0027re subclassing LiveMigrateData above","commit_id":"3237b6c593d1b15ae3a2976b9e7c6aee3b5b9d14"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"ca703ed0a20fb5a3cc72759eed74f2242fa96d46","unresolved":false,"context_lines":[{"line_number":114,"context_line":"    # Version 1.2: Added \u0027serial_listen_ports\u0027 to allow live migration with"},{"line_number":115,"context_line":"    #              serial console."},{"line_number":116,"context_line":"    # Version 1.3: Added \u0027supported_perf_events\u0027"},{"line_number":117,"context_line":"    # Version 1.4: Added old_vol_attachment_ids"},{"line_number":118,"context_line":"    VERSION \u003d \u00271.4\u0027"},{"line_number":119,"context_line":""},{"line_number":120,"context_line":"    fields \u003d {"}],"source_content_type":"text/x-python","patch_set":14,"id":"1f1a1f67_60fca0b4","line":117,"range":{"start_line":117,"start_character":4,"end_line":117,"end_character":47},"in_reply_to":"1f1a1f67_ffb4cff3","updated":"2017-07-18 17:25:53.000000000","message":"yes, we have to add new versions to all the subclasses.","commit_id":"3237b6c593d1b15ae3a2976b9e7c6aee3b5b9d14"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"54a5a32260552d80102acebfb8d180921119a0c6","unresolved":false,"context_lines":[{"line_number":31,"context_line":"        \u0027migration\u0027: fields.ObjectField(\u0027Migration\u0027),"},{"line_number":32,"context_line":"        # old_vol_attachment_ids is a dict used to store the old attachment_ids"},{"line_number":33,"context_line":"        # for each volume so they can be restored on a migration rollback. The"},{"line_number":34,"context_line":"        # key is the volume_id, and the value is the attachment_id."},{"line_number":35,"context_line":"        \u0027old_vol_attachment_ids\u0027: fields.DictOfStringsField(),"},{"line_number":36,"context_line":"    }"},{"line_number":37,"context_line":""}],"source_content_type":"text/x-python","patch_set":18,"id":"9f5c4f37_96343b4c","line":34,"range":{"start_line":34,"start_character":32,"end_line":34,"end_character":67},"updated":"2017-09-21 19:20:31.000000000","message":"Should the value be a list of attachment IDs? I guess right now, without multi-attach support, we should just have 1:1 between volume ID and attachment ID for a given instance, right? Although this is scoped to a single instance being migrated, and isn\u0027t persisted, and we wouldn\u0027t have the same volume attached multiple times to an instance unless we are in the process of migrating it, which is what this is for. I\u0027m just overthinking this I think. This is for accounting on how to update the BDM if the migration succeeds or fails and we have to rollback.","commit_id":"f576578b2e1050def3ec2995fb4438b27a66aab4"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"7b29f9a04263a131d2699345327692fe81fdc5c8","unresolved":false,"context_lines":[{"line_number":31,"context_line":"        \u0027migration\u0027: fields.ObjectField(\u0027Migration\u0027),"},{"line_number":32,"context_line":"        # old_vol_attachment_ids is a dict used to store the old attachment_ids"},{"line_number":33,"context_line":"        # for each volume so they can be restored on a migration rollback. The"},{"line_number":34,"context_line":"        # key is the volume_id, and the value is the attachment_id."},{"line_number":35,"context_line":"        \u0027old_vol_attachment_ids\u0027: fields.DictOfStringsField(),"},{"line_number":36,"context_line":"    }"},{"line_number":37,"context_line":""}],"source_content_type":"text/x-python","patch_set":18,"id":"7f515b1d_30b58c32","line":34,"range":{"start_line":34,"start_character":32,"end_line":34,"end_character":67},"in_reply_to":"9f5c4f37_96343b4c","updated":"2017-09-25 20:57:58.000000000","message":"This will only ever be populated with new attachments. It\u0027s not used for the old attachments.\n\nI think it\u0027s currently allowed to attach a volume multiple times to the same vm... though that doesn\u0027t make much sense and probably should be forbidden at the API level (where migrate wouldn\u0027t hit it)","commit_id":"f576578b2e1050def3ec2995fb4438b27a66aab4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"54a5a32260552d80102acebfb8d180921119a0c6","unresolved":false,"context_lines":[{"line_number":67,"context_line":""},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"@obj_base.NovaObjectRegistry.register"},{"line_number":70,"context_line":"class LibvirtLiveMigrateBDMInfo(obj_base.NovaObject):"},{"line_number":71,"context_line":"    VERSION \u003d \u00271.0\u0027"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"    fields \u003d {"}],"source_content_type":"text/x-python","patch_set":18,"id":"9f5c4f37_36690f25","line":70,"range":{"start_line":70,"start_character":6,"end_line":70,"end_character":31},"updated":"2017-09-21 19:20:31.000000000","message":"It seems odd that we don\u0027t have an attachment_id field in here.","commit_id":"f576578b2e1050def3ec2995fb4438b27a66aab4"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"7b29f9a04263a131d2699345327692fe81fdc5c8","unresolved":false,"context_lines":[{"line_number":67,"context_line":""},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"@obj_base.NovaObjectRegistry.register"},{"line_number":70,"context_line":"class LibvirtLiveMigrateBDMInfo(obj_base.NovaObject):"},{"line_number":71,"context_line":"    VERSION \u003d \u00271.0\u0027"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"    fields \u003d {"}],"source_content_type":"text/x-python","patch_set":18,"id":"7f515b1d_10e97019","line":70,"range":{"start_line":70,"start_character":6,"end_line":70,"end_character":31},"in_reply_to":"9f5c4f37_36690f25","updated":"2017-09-25 20:57:58.000000000","message":"Not sure, but I don\u0027t think libvirt, xen, etc should care what the attachment_id is.","commit_id":"f576578b2e1050def3ec2995fb4438b27a66aab4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"54a5a32260552d80102acebfb8d180921119a0c6","unresolved":false,"context_lines":[{"line_number":152,"context_line":"        if target_version \u003c (1, 1) and \u0027target_connect_addr\u0027 in primitive:"},{"line_number":153,"context_line":"            del primitive[\u0027target_connect_addr\u0027]"},{"line_number":154,"context_line":""},{"line_number":155,"context_line":"    def _bdms_to_legacy(self, legacy):"},{"line_number":156,"context_line":"        if not self.obj_attr_is_set(\u0027bdms\u0027):"},{"line_number":157,"context_line":"            return"},{"line_number":158,"context_line":"        legacy[\u0027volume\u0027] \u003d {}"},{"line_number":159,"context_line":"        for bdmi in self.bdms:"},{"line_number":160,"context_line":"            legacy[\u0027volume\u0027][bdmi.serial] \u003d {"},{"line_number":161,"context_line":"                \u0027disk_info\u0027: bdmi.as_disk_info(),"},{"line_number":162,"context_line":"                \u0027connection_info\u0027: bdmi.connection_info}"},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"    def _bdms_from_legacy(self, legacy_pre_result):"},{"line_number":165,"context_line":"        self.bdms \u003d []"},{"line_number":166,"context_line":"        volume \u003d legacy_pre_result.get(\u0027volume\u0027, {})"},{"line_number":167,"context_line":"        for serial in volume:"},{"line_number":168,"context_line":"            vol \u003d volume[serial]"},{"line_number":169,"context_line":"            bdmi \u003d objects.LibvirtLiveMigrateBDMInfo(serial\u003dserial)"},{"line_number":170,"context_line":"            bdmi.connection_info \u003d vol[\u0027connection_info\u0027]"},{"line_number":171,"context_line":"            bdmi.bus \u003d vol[\u0027disk_info\u0027][\u0027bus\u0027]"},{"line_number":172,"context_line":"            bdmi.dev \u003d vol[\u0027disk_info\u0027][\u0027dev\u0027]"},{"line_number":173,"context_line":"            bdmi.type \u003d vol[\u0027disk_info\u0027][\u0027type\u0027]"},{"line_number":174,"context_line":"            if \u0027format\u0027 in vol:"},{"line_number":175,"context_line":"                bdmi.format \u003d vol[\u0027disk_info\u0027][\u0027format\u0027]"},{"line_number":176,"context_line":"            if \u0027boot_index\u0027 in vol:"},{"line_number":177,"context_line":"                bdmi.boot_index \u003d int(vol[\u0027disk_info\u0027][\u0027boot_index\u0027])"},{"line_number":178,"context_line":"            self.bdms.append(bdmi)"},{"line_number":179,"context_line":""},{"line_number":180,"context_line":"    def to_legacy_dict(self, pre_migration_result\u003dFalse):"},{"line_number":181,"context_line":"        LOG.debug(\u0027Converting to legacy: %s\u0027, self)"}],"source_content_type":"text/x-python","patch_set":18,"id":"9f5c4f37_36522f69","line":178,"range":{"start_line":155,"start_character":4,"end_line":178,"end_character":34},"updated":"2017-09-21 19:20:31.000000000","message":"Seems sort of weird that we don\u0027t deal with an attachment_id in here, but maybe that\u0027s just handled via old_vol_attachment_ids?\n\nLegacy BDMs won\u0027t have an attachment ID, but new ones could.\n\nThe to_legacy_dict and from_legacy_dict stuff is all about mixed version computes where we\u0027re sending information between pike and queens compute nodes, for example.","commit_id":"f576578b2e1050def3ec2995fb4438b27a66aab4"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"7b29f9a04263a131d2699345327692fe81fdc5c8","unresolved":false,"context_lines":[{"line_number":152,"context_line":"        if target_version \u003c (1, 1) and \u0027target_connect_addr\u0027 in primitive:"},{"line_number":153,"context_line":"            del primitive[\u0027target_connect_addr\u0027]"},{"line_number":154,"context_line":""},{"line_number":155,"context_line":"    def _bdms_to_legacy(self, legacy):"},{"line_number":156,"context_line":"        if not self.obj_attr_is_set(\u0027bdms\u0027):"},{"line_number":157,"context_line":"            return"},{"line_number":158,"context_line":"        legacy[\u0027volume\u0027] \u003d {}"},{"line_number":159,"context_line":"        for bdmi in self.bdms:"},{"line_number":160,"context_line":"            legacy[\u0027volume\u0027][bdmi.serial] \u003d {"},{"line_number":161,"context_line":"                \u0027disk_info\u0027: bdmi.as_disk_info(),"},{"line_number":162,"context_line":"                \u0027connection_info\u0027: bdmi.connection_info}"},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"    def _bdms_from_legacy(self, legacy_pre_result):"},{"line_number":165,"context_line":"        self.bdms \u003d []"},{"line_number":166,"context_line":"        volume \u003d legacy_pre_result.get(\u0027volume\u0027, {})"},{"line_number":167,"context_line":"        for serial in volume:"},{"line_number":168,"context_line":"            vol \u003d volume[serial]"},{"line_number":169,"context_line":"            bdmi \u003d objects.LibvirtLiveMigrateBDMInfo(serial\u003dserial)"},{"line_number":170,"context_line":"            bdmi.connection_info \u003d vol[\u0027connection_info\u0027]"},{"line_number":171,"context_line":"            bdmi.bus \u003d vol[\u0027disk_info\u0027][\u0027bus\u0027]"},{"line_number":172,"context_line":"            bdmi.dev \u003d vol[\u0027disk_info\u0027][\u0027dev\u0027]"},{"line_number":173,"context_line":"            bdmi.type \u003d vol[\u0027disk_info\u0027][\u0027type\u0027]"},{"line_number":174,"context_line":"            if \u0027format\u0027 in vol:"},{"line_number":175,"context_line":"                bdmi.format \u003d vol[\u0027disk_info\u0027][\u0027format\u0027]"},{"line_number":176,"context_line":"            if \u0027boot_index\u0027 in vol:"},{"line_number":177,"context_line":"                bdmi.boot_index \u003d int(vol[\u0027disk_info\u0027][\u0027boot_index\u0027])"},{"line_number":178,"context_line":"            self.bdms.append(bdmi)"},{"line_number":179,"context_line":""},{"line_number":180,"context_line":"    def to_legacy_dict(self, pre_migration_result\u003dFalse):"},{"line_number":181,"context_line":"        LOG.debug(\u0027Converting to legacy: %s\u0027, self)"}],"source_content_type":"text/x-python","patch_set":18,"id":"7f515b1d_10a170aa","line":178,"range":{"start_line":155,"start_character":4,"end_line":178,"end_character":34},"in_reply_to":"9f5c4f37_36522f69","updated":"2017-09-25 20:57:58.000000000","message":"I have to say that I don\u0027t understand how all this works. It does seem odd that attachment_id is not at least added as a field here, although pike bdms do have an attachment_id field so I don\u0027t think it\u0027s a queens problem (maybe a pike problem?). Is it possible to get a pre-pike bdm to show up in a queens environment?","commit_id":"f576578b2e1050def3ec2995fb4438b27a66aab4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c801feed3897d5fbd8e6dfc56b2a3daf08d86fcc","unresolved":false,"context_lines":[{"line_number":32,"context_line":"        # old_vol_attachment_ids is a dict used to store the old attachment_ids"},{"line_number":33,"context_line":"        # for each volume so they can be restored on a migration rollback. The"},{"line_number":34,"context_line":"        # key is the volume_id, and the value is the attachment_id."},{"line_number":35,"context_line":"        \u0027old_vol_attachment_ids\u0027: fields.DictOfStringsField(),"},{"line_number":36,"context_line":"    }"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"    def to_legacy_dict(self, pre_migration_result\u003dFalse):"}],"source_content_type":"text/x-python","patch_set":20,"id":"7f515b1d_729b7365","line":35,"range":{"start_line":35,"start_character":9,"end_line":35,"end_character":31},"updated":"2017-09-28 17:16:07.000000000","message":"As noted elsewhere, this is a string-\u003estring mapping, not a string-\u003elist(string) mapping, so the pluralization of the name makes this confusing.\n\nOr is this plural in case you have multiple volumes attached to the same instance?","commit_id":"f261ce45d44e160f6988ee87981a2342fd018b2c"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"2303fd1084aede17c3f4c0f2a78ac43a483773c4","unresolved":false,"context_lines":[{"line_number":32,"context_line":"        # old_vol_attachment_ids is a dict used to store the old attachment_ids"},{"line_number":33,"context_line":"        # for each volume so they can be restored on a migration rollback. The"},{"line_number":34,"context_line":"        # key is the volume_id, and the value is the attachment_id."},{"line_number":35,"context_line":"        \u0027old_vol_attachment_ids\u0027: fields.DictOfStringsField(),"},{"line_number":36,"context_line":"    }"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"    def to_legacy_dict(self, pre_migration_result\u003dFalse):"}],"source_content_type":"text/x-python","patch_set":20,"id":"7f515b1d_32261b17","line":35,"range":{"start_line":35,"start_character":9,"end_line":35,"end_character":31},"in_reply_to":"7f515b1d_729b7365","updated":"2017-09-28 17:31:26.000000000","message":"Yes, it\u0027s plural as there will be multiple attachment_ids (values) in the dict, one for each volume on an instance (the key is the volume_id).","commit_id":"f261ce45d44e160f6988ee87981a2342fd018b2c"}],"nova/tests/unit/compute/test_compute_mgr.py":[{"author":{"_account_id":24147,"name":"Mark Giles","email":"mark.giles@oracle.com","username":"mark-giles"},"change_message_id":"a7c1a54076a92a11bbea7eda225e136b87208de6","unresolved":false,"context_lines":[{"line_number":5592,"context_line":"        image_bdm.attachment_id \u003d uuids.attachment3"},{"line_number":5593,"context_line":""},{"line_number":5594,"context_line":"        migrate_data \u003d migrate_data_obj.LiveMigrateData()"},{"line_number":5595,"context_line":"        migrate_data.old_vol_attachment_ids \u003d {volume_id: orig_attachment_id}"},{"line_number":5596,"context_line":""},{"line_number":5597,"context_line":"        @mock.patch.object(vol_bdm, \u0027save\u0027)"},{"line_number":5598,"context_line":"        @mock.patch.object(compute, \u0027update_available_resource\u0027)"}],"source_content_type":"text/x-python","patch_set":10,"id":"5f201791_dc3d6abb","line":5595,"updated":"2017-06-28 19:18:11.000000000","message":"This appears to be duplicate code (of lines 5590-1)","commit_id":"65db494a6de19aa6d3afac59522246269992347c"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"66a4f2c0430ab4dc1ae5508aebd7b86892c1c536","unresolved":false,"context_lines":[{"line_number":5592,"context_line":"        image_bdm.attachment_id \u003d uuids.attachment3"},{"line_number":5593,"context_line":""},{"line_number":5594,"context_line":"        migrate_data \u003d migrate_data_obj.LiveMigrateData()"},{"line_number":5595,"context_line":"        migrate_data.old_vol_attachment_ids \u003d {volume_id: orig_attachment_id}"},{"line_number":5596,"context_line":""},{"line_number":5597,"context_line":"        @mock.patch.object(vol_bdm, \u0027save\u0027)"},{"line_number":5598,"context_line":"        @mock.patch.object(compute, \u0027update_available_resource\u0027)"}],"source_content_type":"text/x-python","patch_set":10,"id":"5f201791_f70f2134","line":5595,"in_reply_to":"5f201791_dc3d6abb","updated":"2017-06-28 20:37:40.000000000","message":"Done","commit_id":"65db494a6de19aa6d3afac59522246269992347c"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"768958900377a6888e7e62b434d365c2598826e4","unresolved":false,"context_lines":[{"line_number":2594,"context_line":"            mock_volume_api.terminate_connection.assert_called_once_with("},{"line_number":2595,"context_line":"                    self.context, uuids.volume_id, connector)"},{"line_number":2596,"context_line":""},{"line_number":2597,"context_line":"            # Now try it with an attachment id. Terminate connection should"},{"line_number":2598,"context_line":"            # not be called because this is the cinder v3.27 flow."},{"line_number":2599,"context_line":"            bdm.attachment_id \u003d uuids.attachment"},{"line_number":2600,"context_line":"            mock_driver_detach.reset_mock()"},{"line_number":2601,"context_line":"            mock_volume_api.reset_mock()"},{"line_number":2602,"context_line":""},{"line_number":2603,"context_line":"            self.compute.remove_volume_connection(self.context,"},{"line_number":2604,"context_line":"                                                  uuids.volume_id, inst)"},{"line_number":2605,"context_line":""},{"line_number":2606,"context_line":"            mock_driver_detach.assert_called_once_with(self.context, inst,"},{"line_number":2607,"context_line":"                                                       mock_volume_api,"},{"line_number":2608,"context_line":"                                                       mock_virt_driver)"},{"line_number":2609,"context_line":""},{"line_number":2610,"context_line":"            mock_volume_api.terminate_connection.assert_not_called()"},{"line_number":2611,"context_line":""},{"line_number":2612,"context_line":"    def test_delete_disk_metadata(self):"},{"line_number":2613,"context_line":"        bdm \u003d objects.BlockDeviceMapping(volume_id\u003duuids.volume_id, tag\u003d\u0027foo\u0027)"}],"source_content_type":"text/x-python","patch_set":14,"id":"1f1a1f67_bfa537a4","line":2610,"range":{"start_line":2597,"start_character":0,"end_line":2610,"end_character":68},"updated":"2017-07-18 16:19:27.000000000","message":"This is really a separate test. I\u0027d prefer to see it split it out (perhaps with a \u0027_test_remove_volume_connection\u0027 \"helper\" test), but I won\u0027t block on this","commit_id":"3237b6c593d1b15ae3a2976b9e7c6aee3b5b9d14"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"ef6934127302ab553b683b7444f755cda5affcfe","unresolved":false,"context_lines":[{"line_number":2594,"context_line":"            mock_volume_api.terminate_connection.assert_called_once_with("},{"line_number":2595,"context_line":"                    self.context, uuids.volume_id, connector)"},{"line_number":2596,"context_line":""},{"line_number":2597,"context_line":"            # Now try it with an attachment id. Terminate connection should"},{"line_number":2598,"context_line":"            # not be called because this is the cinder v3.27 flow."},{"line_number":2599,"context_line":"            bdm.attachment_id \u003d uuids.attachment"},{"line_number":2600,"context_line":"            mock_driver_detach.reset_mock()"},{"line_number":2601,"context_line":"            mock_volume_api.reset_mock()"},{"line_number":2602,"context_line":""},{"line_number":2603,"context_line":"            self.compute.remove_volume_connection(self.context,"},{"line_number":2604,"context_line":"                                                  uuids.volume_id, inst)"},{"line_number":2605,"context_line":""},{"line_number":2606,"context_line":"            mock_driver_detach.assert_called_once_with(self.context, inst,"},{"line_number":2607,"context_line":"                                                       mock_volume_api,"},{"line_number":2608,"context_line":"                                                       mock_virt_driver)"},{"line_number":2609,"context_line":""},{"line_number":2610,"context_line":"            mock_volume_api.terminate_connection.assert_not_called()"},{"line_number":2611,"context_line":""},{"line_number":2612,"context_line":"    def test_delete_disk_metadata(self):"},{"line_number":2613,"context_line":"        bdm \u003d objects.BlockDeviceMapping(volume_id\u003duuids.volume_id, tag\u003d\u0027foo\u0027)"}],"source_content_type":"text/x-python","patch_set":14,"id":"1f1a1f67_31efbc88","line":2610,"range":{"start_line":2597,"start_character":0,"end_line":2610,"end_character":68},"in_reply_to":"1f1a1f67_00dccc43","updated":"2017-07-21 00:27:07.000000000","message":"After the latest set of changes, I broke it out into a new test.","commit_id":"3237b6c593d1b15ae3a2976b9e7c6aee3b5b9d14"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"ca703ed0a20fb5a3cc72759eed74f2242fa96d46","unresolved":false,"context_lines":[{"line_number":2594,"context_line":"            mock_volume_api.terminate_connection.assert_called_once_with("},{"line_number":2595,"context_line":"                    self.context, uuids.volume_id, connector)"},{"line_number":2596,"context_line":""},{"line_number":2597,"context_line":"            # Now try it with an attachment id. Terminate connection should"},{"line_number":2598,"context_line":"            # not be called because this is the cinder v3.27 flow."},{"line_number":2599,"context_line":"            bdm.attachment_id \u003d uuids.attachment"},{"line_number":2600,"context_line":"            mock_driver_detach.reset_mock()"},{"line_number":2601,"context_line":"            mock_volume_api.reset_mock()"},{"line_number":2602,"context_line":""},{"line_number":2603,"context_line":"            self.compute.remove_volume_connection(self.context,"},{"line_number":2604,"context_line":"                                                  uuids.volume_id, inst)"},{"line_number":2605,"context_line":""},{"line_number":2606,"context_line":"            mock_driver_detach.assert_called_once_with(self.context, inst,"},{"line_number":2607,"context_line":"                                                       mock_volume_api,"},{"line_number":2608,"context_line":"                                                       mock_virt_driver)"},{"line_number":2609,"context_line":""},{"line_number":2610,"context_line":"            mock_volume_api.terminate_connection.assert_not_called()"},{"line_number":2611,"context_line":""},{"line_number":2612,"context_line":"    def test_delete_disk_metadata(self):"},{"line_number":2613,"context_line":"        bdm \u003d objects.BlockDeviceMapping(volume_id\u003duuids.volume_id, tag\u003d\u0027foo\u0027)"}],"source_content_type":"text/x-python","patch_set":14,"id":"1f1a1f67_00dccc43","line":2610,"range":{"start_line":2597,"start_character":0,"end_line":2610,"end_character":68},"in_reply_to":"1f1a1f67_bfa537a4","updated":"2017-07-18 17:25:53.000000000","message":"I had originally thought of doing it as a separate test, but thought there was so much shared code, it would be simplest to tack this on at the end.","commit_id":"3237b6c593d1b15ae3a2976b9e7c6aee3b5b9d14"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"768958900377a6888e7e62b434d365c2598826e4","unresolved":false,"context_lines":[{"line_number":5496,"context_line":"                           \u0027get_by_instance_uuid\u0027)"},{"line_number":5497,"context_line":"        def _test(mock_get_bdms, mock_ivbi, mock_gibdi, mock_plm, mock_nwapi,"},{"line_number":5498,"context_line":"                  mock_notify):"},{"line_number":5499,"context_line":"            mock_get_bdms.return_value \u003d []"},{"line_number":5500,"context_line":"            instance \u003d fake_instance.fake_instance_obj(self.context,"},{"line_number":5501,"context_line":"                                                       uuid\u003duuids.instance)"},{"line_number":5502,"context_line":"            migrate_data \u003d migrate_data_obj.LiveMigrateData()"},{"line_number":5503,"context_line":"            mock_plm.return_value \u003d migrate_data"},{"line_number":5504,"context_line":"            r \u003d compute.pre_live_migration(self.context, instance,"}],"source_content_type":"text/x-python","patch_set":14,"id":"1f1a1f67_5fa623ad","line":5501,"range":{"start_line":5499,"start_character":0,"end_line":5501,"end_character":75},"updated":"2017-07-18 16:19:27.000000000","message":"Thanks to the commit message, I know why this is happening. Thanks again","commit_id":"3237b6c593d1b15ae3a2976b9e7c6aee3b5b9d14"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"f324aec89c0019f79cad611c20f70285fc2ab943","unresolved":false,"context_lines":[{"line_number":2818,"context_line":"            mock_volume_api.terminate_connection.assert_called_once_with("},{"line_number":2819,"context_line":"                    self.context, uuids.volume_id, connector)"},{"line_number":2820,"context_line":""},{"line_number":2821,"context_line":"    def test_remove_volume_connection_cinder_v3_api(self):"},{"line_number":2822,"context_line":"        instance \u003d fake_instance.fake_instance_obj(self.context,"},{"line_number":2823,"context_line":"                                                   uuid\u003duuids.instance)"},{"line_number":2824,"context_line":"        volume_id \u003d uuids.volume"}],"source_content_type":"text/x-python","patch_set":16,"id":"1f1a1f67_1c7ddee6","line":2821,"range":{"start_line":2821,"start_character":8,"end_line":2821,"end_character":51},"updated":"2017-07-21 14:25:33.000000000","message":"Much better. Ta","commit_id":"98f98b9c86e2cf385444961f48fb4df0c7a8d34f"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"f324aec89c0019f79cad611c20f70285fc2ab943","unresolved":false,"context_lines":[{"line_number":2842,"context_line":""},{"line_number":2843,"context_line":"            mock_detach.assert_called_once_with(self.context, instance,"},{"line_number":2844,"context_line":"                                                self.compute.volume_api,"},{"line_number":2845,"context_line":"                                                mock_driver)"},{"line_number":2846,"context_line":"        _test()"},{"line_number":2847,"context_line":""},{"line_number":2848,"context_line":"    def test_delete_disk_metadata(self):"}],"source_content_type":"text/x-python","patch_set":16,"id":"1f1a1f67_dc5f6670","line":2845,"updated":"2017-07-21 14:25:33.000000000","message":"How come you no longer check to make sure terminate isn\u0027t called?","commit_id":"98f98b9c86e2cf385444961f48fb4df0c7a8d34f"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"56478947be61150a76c5678ba2f80c20059af7bf","unresolved":false,"context_lines":[{"line_number":2842,"context_line":""},{"line_number":2843,"context_line":"            mock_detach.assert_called_once_with(self.context, instance,"},{"line_number":2844,"context_line":"                                                self.compute.volume_api,"},{"line_number":2845,"context_line":"                                                mock_driver)"},{"line_number":2846,"context_line":"        _test()"},{"line_number":2847,"context_line":""},{"line_number":2848,"context_line":"    def test_delete_disk_metadata(self):"}],"source_content_type":"text/x-python","patch_set":16,"id":"1f1a1f67_db8beb65","line":2845,"in_reply_to":"1f1a1f67_dc5f6670","updated":"2017-07-21 15:10:07.000000000","message":"since it\u0027s not mocked, it will throw if it is unexpectedly called.","commit_id":"98f98b9c86e2cf385444961f48fb4df0c7a8d34f"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"601e18e8f806e4397f3fb8222c68c2fcfd62bf9f","unresolved":false,"context_lines":[{"line_number":2870,"context_line":"            mock_volume_api.terminate_connection.assert_called_once_with("},{"line_number":2871,"context_line":"                    self.context, uuids.volume_id, connector)"},{"line_number":2872,"context_line":""},{"line_number":2873,"context_line":"    def test_remove_volume_connection_cinder_v3_api(self):"},{"line_number":2874,"context_line":"        instance \u003d fake_instance.fake_instance_obj(self.context,"},{"line_number":2875,"context_line":"                                                   uuid\u003duuids.instance)"},{"line_number":2876,"context_line":"        volume_id \u003d uuids.volume"}],"source_content_type":"text/x-python","patch_set":20,"id":"7f515b1d_681022a9","line":2873,"range":{"start_line":2873,"start_character":8,"end_line":2873,"end_character":51},"updated":"2017-09-28 19:36:34.000000000","message":"Should probably assert that terminate_connection is not called in this test.","commit_id":"f261ce45d44e160f6988ee87981a2342fd018b2c"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"cd980af07479072cec1dd5e4c7f1e95e02e3be80","unresolved":false,"context_lines":[{"line_number":2870,"context_line":"            mock_volume_api.terminate_connection.assert_called_once_with("},{"line_number":2871,"context_line":"                    self.context, uuids.volume_id, connector)"},{"line_number":2872,"context_line":""},{"line_number":2873,"context_line":"    def test_remove_volume_connection_cinder_v3_api(self):"},{"line_number":2874,"context_line":"        instance \u003d fake_instance.fake_instance_obj(self.context,"},{"line_number":2875,"context_line":"                                                   uuid\u003duuids.instance)"},{"line_number":2876,"context_line":"        volume_id \u003d uuids.volume"}],"source_content_type":"text/x-python","patch_set":20,"id":"7f515b1d_a3687b1c","line":2873,"range":{"start_line":2873,"start_character":8,"end_line":2873,"end_character":51},"in_reply_to":"7f515b1d_681022a9","updated":"2017-09-28 23:44:46.000000000","message":"Done","commit_id":"f261ce45d44e160f6988ee87981a2342fd018b2c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"601e18e8f806e4397f3fb8222c68c2fcfd62bf9f","unresolved":false,"context_lines":[{"line_number":5936,"context_line":""},{"line_number":5937,"context_line":"        _test()"},{"line_number":5938,"context_line":""},{"line_number":5939,"context_line":"    def test_pre_live_migration_cinder_v3_api(self):"},{"line_number":5940,"context_line":"        compute \u003d manager.ComputeManager()"},{"line_number":5941,"context_line":""},{"line_number":5942,"context_line":"        instance \u003d fake_instance.fake_instance_obj(self.context,"}],"source_content_type":"text/x-python","patch_set":20,"id":"7f515b1d_c87a76ef","line":5939,"updated":"2017-09-28 19:36:34.000000000","message":"nit: in general, for larger tests like this it would be nice to have a docstring explaining what this is testing. Same below.","commit_id":"f261ce45d44e160f6988ee87981a2342fd018b2c"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"cd980af07479072cec1dd5e4c7f1e95e02e3be80","unresolved":false,"context_lines":[{"line_number":5936,"context_line":""},{"line_number":5937,"context_line":"        _test()"},{"line_number":5938,"context_line":""},{"line_number":5939,"context_line":"    def test_pre_live_migration_cinder_v3_api(self):"},{"line_number":5940,"context_line":"        compute \u003d manager.ComputeManager()"},{"line_number":5941,"context_line":""},{"line_number":5942,"context_line":"        instance \u003d fake_instance.fake_instance_obj(self.context,"}],"source_content_type":"text/x-python","patch_set":20,"id":"7f515b1d_23d92bf2","line":5939,"in_reply_to":"7f515b1d_c87a76ef","updated":"2017-09-28 23:44:46.000000000","message":"Done","commit_id":"f261ce45d44e160f6988ee87981a2342fd018b2c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"601e18e8f806e4397f3fb8222c68c2fcfd62bf9f","unresolved":false,"context_lines":[{"line_number":6051,"context_line":"            # is restored."},{"line_number":6052,"context_line":"            new_attachment_id \u003d uuids.attachment3"},{"line_number":6053,"context_line":"            mock_attach_create.side_effect \u003d [{\u0027id\u0027: new_attachment_id},"},{"line_number":6054,"context_line":"                                              exception.NovaException()]"},{"line_number":6055,"context_line":"            mock_get_bdms.return_value \u003d [vol1_bdm, vol2_bdm]"},{"line_number":6056,"context_line":"            mock_plm.return_value \u003d migrate_data"},{"line_number":6057,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"7f515b1d_08740efd","line":6054,"range":{"start_line":6054,"start_character":56,"end_line":6054,"end_character":69},"updated":"2017-09-28 19:36:34.000000000","message":"nit: I\u0027d use test.TestingException here so it\u0027s not so generic.","commit_id":"f261ce45d44e160f6988ee87981a2342fd018b2c"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"cd980af07479072cec1dd5e4c7f1e95e02e3be80","unresolved":false,"context_lines":[{"line_number":6051,"context_line":"            # is restored."},{"line_number":6052,"context_line":"            new_attachment_id \u003d uuids.attachment3"},{"line_number":6053,"context_line":"            mock_attach_create.side_effect \u003d [{\u0027id\u0027: new_attachment_id},"},{"line_number":6054,"context_line":"                                              exception.NovaException()]"},{"line_number":6055,"context_line":"            mock_get_bdms.return_value \u003d [vol1_bdm, vol2_bdm]"},{"line_number":6056,"context_line":"            mock_plm.return_value \u003d migrate_data"},{"line_number":6057,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"7f515b1d_c3bff75e","line":6054,"range":{"start_line":6054,"start_character":56,"end_line":6054,"end_character":69},"in_reply_to":"7f515b1d_08740efd","updated":"2017-09-28 23:44:46.000000000","message":"Done","commit_id":"f261ce45d44e160f6988ee87981a2342fd018b2c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"601e18e8f806e4397f3fb8222c68c2fcfd62bf9f","unresolved":false,"context_lines":[{"line_number":6055,"context_line":"            mock_get_bdms.return_value \u003d [vol1_bdm, vol2_bdm]"},{"line_number":6056,"context_line":"            mock_plm.return_value \u003d migrate_data"},{"line_number":6057,"context_line":""},{"line_number":6058,"context_line":"            self.assertRaises(exception.NovaException,"},{"line_number":6059,"context_line":"                              compute.pre_live_migration,"},{"line_number":6060,"context_line":"                              self.context, instance, False, {}, migrate_data)"},{"line_number":6061,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"7f515b1d_e870920a","line":6058,"range":{"start_line":6058,"start_character":40,"end_line":6058,"end_character":53},"updated":"2017-09-28 19:36:34.000000000","message":"same","commit_id":"f261ce45d44e160f6988ee87981a2342fd018b2c"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"cd980af07479072cec1dd5e4c7f1e95e02e3be80","unresolved":false,"context_lines":[{"line_number":6055,"context_line":"            mock_get_bdms.return_value \u003d [vol1_bdm, vol2_bdm]"},{"line_number":6056,"context_line":"            mock_plm.return_value \u003d migrate_data"},{"line_number":6057,"context_line":""},{"line_number":6058,"context_line":"            self.assertRaises(exception.NovaException,"},{"line_number":6059,"context_line":"                              compute.pre_live_migration,"},{"line_number":6060,"context_line":"                              self.context, instance, False, {}, migrate_data)"},{"line_number":6061,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"7f515b1d_a3c4fbf5","line":6058,"range":{"start_line":6058,"start_character":40,"end_line":6058,"end_character":53},"in_reply_to":"7f515b1d_e870920a","updated":"2017-09-28 23:44:46.000000000","message":"Done","commit_id":"f261ce45d44e160f6988ee87981a2342fd018b2c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"601e18e8f806e4397f3fb8222c68c2fcfd62bf9f","unresolved":false,"context_lines":[{"line_number":6284,"context_line":"                  mock_rpc, mock_get_bdm_info, mock_attach_delete,"},{"line_number":6285,"context_line":"                  mock_update_resource, mock_bdm_save, mock_rt):"},{"line_number":6286,"context_line":"            # Because live migration has succeeded, _post_live_migration"},{"line_number":6287,"context_line":"            # should call detach with the original/old attachment_id"},{"line_number":6288,"context_line":"            mock_rt.return_value \u003d mock.Mock()"},{"line_number":6289,"context_line":"            mock_get_bdms.return_value \u003d [vol_bdm, image_bdm]"},{"line_number":6290,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"7f515b1d_28022a3e","line":6287,"range":{"start_line":6287,"start_character":26,"end_line":6287,"end_character":32},"updated":"2017-09-28 19:36:34.000000000","message":"delete?","commit_id":"f261ce45d44e160f6988ee87981a2342fd018b2c"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"cd980af07479072cec1dd5e4c7f1e95e02e3be80","unresolved":false,"context_lines":[{"line_number":6284,"context_line":"                  mock_rpc, mock_get_bdm_info, mock_attach_delete,"},{"line_number":6285,"context_line":"                  mock_update_resource, mock_bdm_save, mock_rt):"},{"line_number":6286,"context_line":"            # Because live migration has succeeded, _post_live_migration"},{"line_number":6287,"context_line":"            # should call detach with the original/old attachment_id"},{"line_number":6288,"context_line":"            mock_rt.return_value \u003d mock.Mock()"},{"line_number":6289,"context_line":"            mock_get_bdms.return_value \u003d [vol_bdm, image_bdm]"},{"line_number":6290,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"7f515b1d_8338dfed","line":6287,"range":{"start_line":6287,"start_character":26,"end_line":6287,"end_character":32},"in_reply_to":"7f515b1d_28022a3e","updated":"2017-09-28 23:44:46.000000000","message":"Done","commit_id":"f261ce45d44e160f6988ee87981a2342fd018b2c"}],"nova/tests/unit/objects/test_migrate_data.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"601e18e8f806e4397f3fb8222c68c2fcfd62bf9f","unresolved":false,"context_lines":[{"line_number":224,"context_line":""},{"line_number":225,"context_line":"    def test_obj_make_compatible(self):"},{"line_number":226,"context_line":"        obj \u003d migrate_data.LibvirtLiveMigrateData("},{"line_number":227,"context_line":"            old_vol_attachment_ids\u003d{\u0027yes\u0027: \u0027no\u0027})"},{"line_number":228,"context_line":"        primitive \u003d obj.obj_to_primitive(target_version\u003d\u00271.0\u0027)"},{"line_number":229,"context_line":"        self.assertNotIn(\u0027old_vol_attachment_ids\u0027, primitive)"},{"line_number":230,"context_line":"        primitive \u003d obj.obj_to_primitive(target_version\u003d\u00271.3\u0027)"}],"source_content_type":"text/x-python","patch_set":20,"id":"7f515b1d_48b32699","line":227,"range":{"start_line":227,"start_character":36,"end_line":227,"end_character":47},"updated":"2017-09-28 19:36:34.000000000","message":"super nit: use uuids.volume_id: uuids.attachment_id\n\njust to make this more obvious the types of things we put in here\n\nsame below","commit_id":"f261ce45d44e160f6988ee87981a2342fd018b2c"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"cd980af07479072cec1dd5e4c7f1e95e02e3be80","unresolved":false,"context_lines":[{"line_number":224,"context_line":""},{"line_number":225,"context_line":"    def test_obj_make_compatible(self):"},{"line_number":226,"context_line":"        obj \u003d migrate_data.LibvirtLiveMigrateData("},{"line_number":227,"context_line":"            old_vol_attachment_ids\u003d{\u0027yes\u0027: \u0027no\u0027})"},{"line_number":228,"context_line":"        primitive \u003d obj.obj_to_primitive(target_version\u003d\u00271.0\u0027)"},{"line_number":229,"context_line":"        self.assertNotIn(\u0027old_vol_attachment_ids\u0027, primitive)"},{"line_number":230,"context_line":"        primitive \u003d obj.obj_to_primitive(target_version\u003d\u00271.3\u0027)"}],"source_content_type":"text/x-python","patch_set":20,"id":"7f515b1d_defc70b6","line":227,"range":{"start_line":227,"start_character":36,"end_line":227,"end_character":47},"in_reply_to":"7f515b1d_48b32699","updated":"2017-09-28 23:44:46.000000000","message":"Done","commit_id":"f261ce45d44e160f6988ee87981a2342fd018b2c"}],"nova/tests/unit/virt/libvirt/test_driver.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1889f63d0dd9a874fecbe18b12ad90ee9ff9cd27","unresolved":false,"context_lines":[{"line_number":10237,"context_line":"        @mock.patch.object(driver, \u0027block_device_info_get_mapping\u0027)"},{"line_number":10238,"context_line":"        @mock.patch.object(drvr, \u0027get_volume_connector\u0027)"},{"line_number":10239,"context_line":"        def _test(mock_get_connector, mock_get_mapping, mock_disconnect):"},{"line_number":10240,"context_line":"            instance \u003d {\u0027uuid\u0027: uuids.instance}"},{"line_number":10241,"context_line":"            disk_dev \u003d \u0027vdb\u0027"},{"line_number":10242,"context_line":""},{"line_number":10243,"context_line":"            # For the cinder v3 migration flow, when the attachment_id is not"}],"source_content_type":"text/x-python","patch_set":8,"id":"5f201791_87552953","line":10240,"range":{"start_line":10240,"start_character":23,"end_line":10240,"end_character":47},"updated":"2017-06-21 19:02:49.000000000","message":"Can we at least use an Instance object here please?\n\ninstance \u003d objects.Instance(uuid\u003duuids.instance)","commit_id":"373b01d5d432bbb0c2ddfb71a79d5129c1775b96"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"3f4ba5fa25a33b8f119e1b43fb92f46c6a614913","unresolved":false,"context_lines":[{"line_number":10237,"context_line":"        @mock.patch.object(driver, \u0027block_device_info_get_mapping\u0027)"},{"line_number":10238,"context_line":"        @mock.patch.object(drvr, \u0027get_volume_connector\u0027)"},{"line_number":10239,"context_line":"        def _test(mock_get_connector, mock_get_mapping, mock_disconnect):"},{"line_number":10240,"context_line":"            instance \u003d {\u0027uuid\u0027: uuids.instance}"},{"line_number":10241,"context_line":"            disk_dev \u003d \u0027vdb\u0027"},{"line_number":10242,"context_line":""},{"line_number":10243,"context_line":"            # For the cinder v3 migration flow, when the attachment_id is not"}],"source_content_type":"text/x-python","patch_set":8,"id":"5f201791_ab5a10f0","line":10240,"range":{"start_line":10240,"start_character":23,"end_line":10240,"end_character":47},"in_reply_to":"5f201791_87552953","updated":"2017-06-22 21:49:26.000000000","message":"Done","commit_id":"373b01d5d432bbb0c2ddfb71a79d5129c1775b96"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1889f63d0dd9a874fecbe18b12ad90ee9ff9cd27","unresolved":false,"context_lines":[{"line_number":10242,"context_line":""},{"line_number":10243,"context_line":"            # For the cinder v3 migration flow, when the attachment_id is not"},{"line_number":10244,"context_line":"            # None, the driver should not call volume_api.initialize_connection"},{"line_number":10245,"context_line":"            block_device_mapping \u003d ["},{"line_number":10246,"context_line":"                {\u0027connection_info\u0027:"},{"line_number":10247,"context_line":"                 {\u0027attachment_id\u0027: uuids.attachment, \u0027data\u0027: {}},"},{"line_number":10248,"context_line":"                 \u0027mount_device\u0027: \u0027/dev/%s\u0027 % disk_dev,"},{"line_number":10249,"context_line":"                 }]"},{"line_number":10250,"context_line":"            mock_get_mapping.return_value \u003d block_device_mapping"},{"line_number":10251,"context_line":""},{"line_number":10252,"context_line":"            drvr.post_live_migration(cntx, instance, None)"}],"source_content_type":"text/x-python","patch_set":8,"id":"5f201791_e7cbbd1b","line":10249,"range":{"start_line":10245,"start_character":12,"end_line":10249,"end_character":19},"updated":"2017-06-21 19:02:49.000000000","message":"Rather than a list of dicts, let\u0027s use a list with a single entry, which is a nova.virt.block_device.DriverVolumeBlockDevice which wraps a nova.objects.BlockDeviceMapping that would have attachment_id set on it, so we don\u0027t need to worry about the connection_info having the attachment_id.\n\nYes, the actual type of the block_device_mapping used anywhere in Nova is mind boggling.","commit_id":"373b01d5d432bbb0c2ddfb71a79d5129c1775b96"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"3f4ba5fa25a33b8f119e1b43fb92f46c6a614913","unresolved":false,"context_lines":[{"line_number":10242,"context_line":""},{"line_number":10243,"context_line":"            # For the cinder v3 migration flow, when the attachment_id is not"},{"line_number":10244,"context_line":"            # None, the driver should not call volume_api.initialize_connection"},{"line_number":10245,"context_line":"            block_device_mapping \u003d ["},{"line_number":10246,"context_line":"                {\u0027connection_info\u0027:"},{"line_number":10247,"context_line":"                 {\u0027attachment_id\u0027: uuids.attachment, \u0027data\u0027: {}},"},{"line_number":10248,"context_line":"                 \u0027mount_device\u0027: \u0027/dev/%s\u0027 % disk_dev,"},{"line_number":10249,"context_line":"                 }]"},{"line_number":10250,"context_line":"            mock_get_mapping.return_value \u003d block_device_mapping"},{"line_number":10251,"context_line":""},{"line_number":10252,"context_line":"            drvr.post_live_migration(cntx, instance, None)"}],"source_content_type":"text/x-python","patch_set":8,"id":"5f201791_cb92ec4b","line":10249,"range":{"start_line":10245,"start_character":12,"end_line":10249,"end_character":19},"in_reply_to":"5f201791_e7cbbd1b","updated":"2017-06-22 21:49:26.000000000","message":"This is what the driver is using- block_device_mapping in the driver is just a list of dicts. And since the attachment id is here (outside of the connection_info), this works ok.","commit_id":"373b01d5d432bbb0c2ddfb71a79d5129c1775b96"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1889f63d0dd9a874fecbe18b12ad90ee9ff9cd27","unresolved":false,"context_lines":[{"line_number":10251,"context_line":""},{"line_number":10252,"context_line":"            drvr.post_live_migration(cntx, instance, None)"},{"line_number":10253,"context_line":""},{"line_number":10254,"context_line":"            mock_disconnect.assert_called_once_with("},{"line_number":10255,"context_line":"                block_device_mapping[0][\u0027connection_info\u0027], disk_dev, instance)"},{"line_number":10256,"context_line":""},{"line_number":10257,"context_line":"        _test()"}],"source_content_type":"text/x-python","patch_set":8,"id":"5f201791_67d7adc1","line":10254,"updated":"2017-06-21 19:02:49.000000000","message":"Shouldn\u0027t you also mock out initialize_connection and assert that it is *not* called in the new style attachment case?","commit_id":"373b01d5d432bbb0c2ddfb71a79d5129c1775b96"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"3f4ba5fa25a33b8f119e1b43fb92f46c6a614913","unresolved":false,"context_lines":[{"line_number":10251,"context_line":""},{"line_number":10252,"context_line":"            drvr.post_live_migration(cntx, instance, None)"},{"line_number":10253,"context_line":""},{"line_number":10254,"context_line":"            mock_disconnect.assert_called_once_with("},{"line_number":10255,"context_line":"                block_device_mapping[0][\u0027connection_info\u0027], disk_dev, instance)"},{"line_number":10256,"context_line":""},{"line_number":10257,"context_line":"        _test()"}],"source_content_type":"text/x-python","patch_set":8,"id":"5f201791_5fc1f5e3","line":10254,"in_reply_to":"5f201791_67d7adc1","updated":"2017-06-22 21:49:26.000000000","message":"Done","commit_id":"373b01d5d432bbb0c2ddfb71a79d5129c1775b96"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"601e18e8f806e4397f3fb8222c68c2fcfd62bf9f","unresolved":false,"context_lines":[{"line_number":10409,"context_line":"            \u0027data\u0027: {\u0027multipath_id\u0027: \u0027dummy1\u0027},"},{"line_number":10410,"context_line":"            \u0027serial\u0027: vol_id}"},{"line_number":10411,"context_line":"        block_device_mapping \u003d ["},{"line_number":10412,"context_line":"            {\u0027attachment_id\u0027: uuids.attachment,"},{"line_number":10413,"context_line":"             \u0027mount_device\u0027: \u0027/dev/%s\u0027 % disk_dev,"},{"line_number":10414,"context_line":"             \u0027connection_info\u0027: connection_info}]"},{"line_number":10415,"context_line":"        old_attachment \u003d {"},{"line_number":10416,"context_line":"            \u0027connection_info\u0027: {"},{"line_number":10417,"context_line":"                \u0027data\u0027: {\u0027multipath_id\u0027: \u0027dummy1\u0027},"}],"source_content_type":"text/x-python","patch_set":20,"id":"7f515b1d_480fa636","line":10414,"range":{"start_line":10412,"start_character":12,"end_line":10414,"end_character":48},"updated":"2017-09-28 19:36:34.000000000","message":"This would actually be a variant of DriverVolumeBlockDevice in the real world. It would be nice to do the test with the same type of modeling.\n\nNote you can use nova.virt.driver.get_block_device_info to convert a BlockDeviceMappingList to what you need for testing here.","commit_id":"f261ce45d44e160f6988ee87981a2342fd018b2c"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"cd980af07479072cec1dd5e4c7f1e95e02e3be80","unresolved":false,"context_lines":[{"line_number":10409,"context_line":"            \u0027data\u0027: {\u0027multipath_id\u0027: \u0027dummy1\u0027},"},{"line_number":10410,"context_line":"            \u0027serial\u0027: vol_id}"},{"line_number":10411,"context_line":"        block_device_mapping \u003d ["},{"line_number":10412,"context_line":"            {\u0027attachment_id\u0027: uuids.attachment,"},{"line_number":10413,"context_line":"             \u0027mount_device\u0027: \u0027/dev/%s\u0027 % disk_dev,"},{"line_number":10414,"context_line":"             \u0027connection_info\u0027: connection_info}]"},{"line_number":10415,"context_line":"        old_attachment \u003d {"},{"line_number":10416,"context_line":"            \u0027connection_info\u0027: {"},{"line_number":10417,"context_line":"                \u0027data\u0027: {\u0027multipath_id\u0027: \u0027dummy1\u0027},"}],"source_content_type":"text/x-python","patch_set":20,"id":"7f515b1d_9e48352d","line":10414,"range":{"start_line":10412,"start_character":12,"end_line":10414,"end_character":48},"in_reply_to":"7f515b1d_480fa636","updated":"2017-09-28 23:44:46.000000000","message":"I worked on this for a while but finally ran out of time. The last problem I was hitting was that fake BlockDeviceMapping wants connection_info to be a string, but then I can\u0027t use that later for mock_get_bdms.return_value because libvirt wants it to be a dict. arggh.\n\nIn any case I will be happy to continue to work on this when I get back regardless of whether this makes it in or not.","commit_id":"f261ce45d44e160f6988ee87981a2342fd018b2c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"601e18e8f806e4397f3fb8222c68c2fcfd62bf9f","unresolved":false,"context_lines":[{"line_number":10413,"context_line":"             \u0027mount_device\u0027: \u0027/dev/%s\u0027 % disk_dev,"},{"line_number":10414,"context_line":"             \u0027connection_info\u0027: connection_info}]"},{"line_number":10415,"context_line":"        old_attachment \u003d {"},{"line_number":10416,"context_line":"            \u0027connection_info\u0027: {"},{"line_number":10417,"context_line":"                \u0027data\u0027: {\u0027multipath_id\u0027: \u0027dummy1\u0027},"},{"line_number":10418,"context_line":"                \u0027serial\u0027: vol_id}}"},{"line_number":10419,"context_line":"        instance \u003d uuids.instance"},{"line_number":10420,"context_line":""},{"line_number":10421,"context_line":"        migrate_data \u003d objects.LibvirtLiveMigrateData("}],"source_content_type":"text/x-python","patch_set":20,"id":"7f515b1d_23adabe8","line":10418,"range":{"start_line":10416,"start_character":31,"end_line":10418,"end_character":33},"updated":"2017-09-28 19:36:34.000000000","message":"You can re-use your connection_info variable here.","commit_id":"f261ce45d44e160f6988ee87981a2342fd018b2c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"601e18e8f806e4397f3fb8222c68c2fcfd62bf9f","unresolved":false,"context_lines":[{"line_number":10416,"context_line":"            \u0027connection_info\u0027: {"},{"line_number":10417,"context_line":"                \u0027data\u0027: {\u0027multipath_id\u0027: \u0027dummy1\u0027},"},{"line_number":10418,"context_line":"                \u0027serial\u0027: vol_id}}"},{"line_number":10419,"context_line":"        instance \u003d uuids.instance"},{"line_number":10420,"context_line":""},{"line_number":10421,"context_line":"        migrate_data \u003d objects.LibvirtLiveMigrateData("},{"line_number":10422,"context_line":"            is_shared_block_storage\u003dTrue,"}],"source_content_type":"text/x-python","patch_set":20,"id":"7f515b1d_03618fd3","line":10419,"range":{"start_line":10419,"start_character":8,"end_line":10419,"end_character":33},"updated":"2017-09-28 19:36:34.000000000","message":"Either pass an actual Instance object or at least a Mock, that would be better than a uuid.","commit_id":"f261ce45d44e160f6988ee87981a2342fd018b2c"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"cd980af07479072cec1dd5e4c7f1e95e02e3be80","unresolved":false,"context_lines":[{"line_number":10416,"context_line":"            \u0027connection_info\u0027: {"},{"line_number":10417,"context_line":"                \u0027data\u0027: {\u0027multipath_id\u0027: \u0027dummy1\u0027},"},{"line_number":10418,"context_line":"                \u0027serial\u0027: vol_id}}"},{"line_number":10419,"context_line":"        instance \u003d uuids.instance"},{"line_number":10420,"context_line":""},{"line_number":10421,"context_line":"        migrate_data \u003d objects.LibvirtLiveMigrateData("},{"line_number":10422,"context_line":"            is_shared_block_storage\u003dTrue,"}],"source_content_type":"text/x-python","patch_set":20,"id":"7f515b1d_de422d0f","line":10419,"range":{"start_line":10419,"start_character":8,"end_line":10419,"end_character":33},"in_reply_to":"7f515b1d_03618fd3","updated":"2017-09-28 23:44:46.000000000","message":"Done","commit_id":"f261ce45d44e160f6988ee87981a2342fd018b2c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"601e18e8f806e4397f3fb8222c68c2fcfd62bf9f","unresolved":false,"context_lines":[{"line_number":10432,"context_line":"            mock_get_bdms.return_value \u003d block_device_mapping"},{"line_number":10433,"context_line":"            mock_attachment_get.return_value \u003d old_attachment"},{"line_number":10434,"context_line":""},{"line_number":10435,"context_line":"            drvr.post_live_migration(cntx, instance, None,"},{"line_number":10436,"context_line":"                                     migrate_data\u003dmigrate_data)"},{"line_number":10437,"context_line":""},{"line_number":10438,"context_line":"            mock_attachment_get.assert_called_once_with(cntx,"}],"source_content_type":"text/x-python","patch_set":20,"id":"7f515b1d_a8373aff","line":10435,"range":{"start_line":10435,"start_character":53,"end_line":10435,"end_character":57},"updated":"2017-09-28 19:36:34.000000000","message":"You should pass something in for this, then you don\u0027t have to mock out block_device_info_get_mapping, which is basically just pulling block_device_mapping out of a dict using the \u0027block_device_mapping\u0027 key.","commit_id":"f261ce45d44e160f6988ee87981a2342fd018b2c"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"8040fbe9c2396e0170d19300c00c4a71690851b9","unresolved":false,"context_lines":[{"line_number":10432,"context_line":"            mock_get_bdms.return_value \u003d block_device_mapping"},{"line_number":10433,"context_line":"            mock_attachment_get.return_value \u003d old_attachment"},{"line_number":10434,"context_line":""},{"line_number":10435,"context_line":"            drvr.post_live_migration(cntx, instance, None,"},{"line_number":10436,"context_line":"                                     migrate_data\u003dmigrate_data)"},{"line_number":10437,"context_line":""},{"line_number":10438,"context_line":"            mock_attachment_get.assert_called_once_with(cntx,"}],"source_content_type":"text/x-python","patch_set":20,"id":"7f515b1d_5e707d08","line":10435,"range":{"start_line":10435,"start_character":53,"end_line":10435,"end_character":57},"in_reply_to":"7f515b1d_a8373aff","updated":"2017-09-28 23:47:37.000000000","message":"I\u0027ll address this when I get back to the bdm work above.","commit_id":"f261ce45d44e160f6988ee87981a2342fd018b2c"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"a2ca5a08c11d2b8e97d2485699d55824948caa4b","unresolved":false,"context_lines":[{"line_number":6890,"context_line":"            # If this is using the cinder v3 flow (attachment_id is"},{"line_number":6891,"context_line":"            # not None), then the volume disconnect happens during the detach"},{"line_number":6892,"context_line":"            # call made in compute_manager post_live_migration."},{"line_number":6893,"context_line":"            if vol.get(\u0027attachment_id\u0027) is None:"},{"line_number":6894,"context_line":"                # Cinder v2 api flow:"},{"line_number":6895,"context_line":"                # Retrieve connection info from Cinder\u0027s initialize_connection"},{"line_number":6896,"context_line":"                # API. The info returned will be accurate for the source"}],"source_content_type":"text/x-python","patch_set":3,"id":"ff0f0b1f_c948fd5d","line":6893,"updated":"2017-05-25 18:36:56.000000000","message":"s/b\nif vol[\u0027connection_info\u0027][\u0027attachment_id\u0027] is None:","commit_id":"a7d115d99dbf24ed68c0a390fd9bf56e88a8d2b8"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"2272d2f1ebe8af3bf929efdb8317f85a2fd36c90","unresolved":false,"context_lines":[{"line_number":6890,"context_line":"            # If this is using the cinder v3 flow (attachment_id is"},{"line_number":6891,"context_line":"            # not None), then the volume disconnect happens during the detach"},{"line_number":6892,"context_line":"            # call made in compute_manager post_live_migration."},{"line_number":6893,"context_line":"            if vol.get(\u0027attachment_id\u0027) is None:"},{"line_number":6894,"context_line":"                # Cinder v2 api flow:"},{"line_number":6895,"context_line":"                # Retrieve connection info from Cinder\u0027s initialize_connection"},{"line_number":6896,"context_line":"                # API. The info returned will be accurate for the source"}],"source_content_type":"text/x-python","patch_set":3,"id":"ff0f0b1f_d733f3f5","line":6893,"in_reply_to":"ff0f0b1f_c948fd5d","updated":"2017-05-25 20:18:30.000000000","message":"Done","commit_id":"a7d115d99dbf24ed68c0a390fd9bf56e88a8d2b8"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1889f63d0dd9a874fecbe18b12ad90ee9ff9cd27","unresolved":false,"context_lines":[{"line_number":6860,"context_line":"                    instance, CONF.libvirt.virt_type,"},{"line_number":6861,"context_line":"                    instance.image_meta, vol)"},{"line_number":6862,"context_line":""},{"line_number":6863,"context_line":"                bdmi \u003d objects.LibvirtLiveMigrateBDMInfo()"},{"line_number":6864,"context_line":"                bdmi.serial \u003d connection_info[\u0027serial\u0027]"},{"line_number":6865,"context_line":"                bdmi.connection_info \u003d connection_info"},{"line_number":6866,"context_line":"                bdmi.bus \u003d disk_info[\u0027bus\u0027]"}],"source_content_type":"text/x-python","patch_set":8,"id":"5f201791_c791e054","line":6863,"range":{"start_line":6863,"start_character":31,"end_line":6863,"end_character":56},"updated":"2017-06-21 19:02:49.000000000","message":"Maybe we should stash the bdm.attachment_id in this thing? But I don\u0027t see migrate_data used in post_live_migration so maybe that\u0027s not helpful. The migrate_data.bdms are used in nova.virt.libvirt.migration._update_volume_xml which is called during live migration itself.","commit_id":"373b01d5d432bbb0c2ddfb71a79d5129c1775b96"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1889f63d0dd9a874fecbe18b12ad90ee9ff9cd27","unresolved":false,"context_lines":[{"line_number":6981,"context_line":"        connector \u003d self.get_volume_connector(instance)"},{"line_number":6982,"context_line":"        volume_api \u003d self._volume_api"},{"line_number":6983,"context_line":"        for vol in block_device_mapping:"},{"line_number":6984,"context_line":"            connection_info \u003d vol[\u0027connection_info\u0027]"},{"line_number":6985,"context_line":"            if connection_info.get(\u0027attachment_id\u0027) is None:"},{"line_number":6986,"context_line":"                # Cinder v2 api flow:"},{"line_number":6987,"context_line":"                # Retrieve connection info from Cinder\u0027s initialize_connection"}],"source_content_type":"text/x-python","patch_set":8,"id":"5f201791_e77dfde7","line":6984,"range":{"start_line":6984,"start_character":30,"end_line":6984,"end_character":52},"updated":"2017-06-21 19:02:49.000000000","message":"We don\u0027t check bdm.connection_info anywhere else for the attachment_id, so why do we do it here? Why can\u0027t we just check if vol.attachment_id is None?\n\nPersonally I\u0027d like to avoid Nova ever having to deal with knowing that Cinder is returning the attachment_id in the connection_info and just focus on the bdm.attachment_id being set, which will be set when the volume is initially attached to the instance.\n\nNote that the type on vol here is a nova.virt.block_device.DriverVolumeBlockDevice which wraps a nova.objects.BlockDeviceMapping and passes through the attributes from the inner object:\n\nhttps://github.com/openstack/nova/blob/96e07267fe6ff6f3ea32586bbbe386019cc8b75a/nova/virt/block_device.py#L122\n\nMy guess is things aren\u0027t working for you to use vol.attachment_id since we don\u0027t have attachment_id set in the DriverVolumeBlockDevice._proxy_as_attr field:\n\nhttps://github.com/openstack/nova/blob/96e07267fe6ff6f3ea32586bbbe386019cc8b75a/nova/virt/block_device.py#L231\n\nAnd that\u0027s why you resorted to stashing the attachment_id in the connection_info dict, but I think that\u0027s the wrong way to go.\n\nWe want to rely less on the magical connection_info dict of random goodies.","commit_id":"373b01d5d432bbb0c2ddfb71a79d5129c1775b96"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"3f4ba5fa25a33b8f119e1b43fb92f46c6a614913","unresolved":false,"context_lines":[{"line_number":6981,"context_line":"        connector \u003d self.get_volume_connector(instance)"},{"line_number":6982,"context_line":"        volume_api \u003d self._volume_api"},{"line_number":6983,"context_line":"        for vol in block_device_mapping:"},{"line_number":6984,"context_line":"            connection_info \u003d vol[\u0027connection_info\u0027]"},{"line_number":6985,"context_line":"            if connection_info.get(\u0027attachment_id\u0027) is None:"},{"line_number":6986,"context_line":"                # Cinder v2 api flow:"},{"line_number":6987,"context_line":"                # Retrieve connection info from Cinder\u0027s initialize_connection"}],"source_content_type":"text/x-python","patch_set":8,"id":"5f201791_5a090c2f","line":6984,"range":{"start_line":6984,"start_character":30,"end_line":6984,"end_character":52},"in_reply_to":"5f201791_e77dfde7","updated":"2017-06-22 21:49:26.000000000","message":"I\u0027m not sure why I used the attachment_id in the connection_info (probably because I didn\u0027t realize that it shouldn\u0027t be depended on). It is already available off the vol so I\u0027ll change it.","commit_id":"373b01d5d432bbb0c2ddfb71a79d5129c1775b96"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"0400ffc081bf3f6c8c2095be689ee5bdaf7f481d","unresolved":false,"context_lines":[{"line_number":6999,"context_line":"            if vol[\u0027attachment_id\u0027] is None:"},{"line_number":7000,"context_line":"                # Cinder v2 api flow:"},{"line_number":7001,"context_line":"                # Retrieve connection info from Cinder\u0027s initialize_connection"},{"line_number":7002,"context_line":"                # API. The info returned will be accurate for the source"},{"line_number":7003,"context_line":"                # server."},{"line_number":7004,"context_line":"                volume_id \u003d connection_info[\u0027serial\u0027]"},{"line_number":7005,"context_line":"                connection_info \u003d volume_api.initialize_connection(context,"}],"source_content_type":"text/x-python","patch_set":13,"id":"3f1d235d_9b3b01c7","line":7002,"range":{"start_line":7002,"start_character":23,"end_line":7002,"end_character":72},"updated":"2017-07-12 03:45:36.000000000","message":"It may be not relative to this change as this exists for the V2 API flow. But could anyone help to clarify why it can\u0027t use the connection_info got from line 6998? Thanks.\n\nIn XenAPI, we can see it also retrieves BDM by invoking the virt function of block_device_info_get_mapping. The connection_info in it will be used to disconnect volumes from original VM.\nhttps://git.openstack.org/cgit/openstack/nova/tree/nova/virt/xenapi/vmops.py#n1354","commit_id":"6298563672edfa6b702ac650ddb25afc18df370b"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"14a84d9c5a9922b5ead1b131bfad3924616abfe8","unresolved":false,"context_lines":[{"line_number":6999,"context_line":"            if vol[\u0027attachment_id\u0027] is None:"},{"line_number":7000,"context_line":"                # Cinder v2 api flow:"},{"line_number":7001,"context_line":"                # Retrieve connection info from Cinder\u0027s initialize_connection"},{"line_number":7002,"context_line":"                # API. The info returned will be accurate for the source"},{"line_number":7003,"context_line":"                # server."},{"line_number":7004,"context_line":"                volume_id \u003d connection_info[\u0027serial\u0027]"},{"line_number":7005,"context_line":"                connection_info \u003d volume_api.initialize_connection(context,"}],"source_content_type":"text/x-python","patch_set":13,"id":"1f1a1f67_29706ba5","line":7002,"range":{"start_line":7002,"start_character":23,"end_line":7002,"end_character":72},"in_reply_to":"1f1a1f67_0fbc94da","updated":"2017-07-13 10:02:46.000000000","message":"Per the commit message from the following commit, the connection_info from DBM is for destination. So if the connection info for source side is different from destination, it will have problem. That\u0027s why libvirt changed to use initialize_connection.\nhttps://github.com/openstack/nova/commit/8b649aa86fb26e998d66e75e5cebfd19c396942d","commit_id":"6298563672edfa6b702ac650ddb25afc18df370b"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"5e72b733d8f703caa51d8e3228fccfa824b12ef9","unresolved":false,"context_lines":[{"line_number":6999,"context_line":"            if vol[\u0027attachment_id\u0027] is None:"},{"line_number":7000,"context_line":"                # Cinder v2 api flow:"},{"line_number":7001,"context_line":"                # Retrieve connection info from Cinder\u0027s initialize_connection"},{"line_number":7002,"context_line":"                # API. The info returned will be accurate for the source"},{"line_number":7003,"context_line":"                # server."},{"line_number":7004,"context_line":"                volume_id \u003d connection_info[\u0027serial\u0027]"},{"line_number":7005,"context_line":"                connection_info \u003d volume_api.initialize_connection(context,"}],"source_content_type":"text/x-python","patch_set":13,"id":"1f1a1f67_0fbc94da","line":7002,"range":{"start_line":7002,"start_character":23,"end_line":7002,"end_character":72},"in_reply_to":"1f1a1f67_1094d58a","updated":"2017-07-13 09:00:00.000000000","message":"The connection_info from bdm works well for XenAPI both in v2 and v3.27. It\u0027d be good if someone can help us to understand why libvirt needs initialize_connection.\n But it won\u0027t change my mind to agree this commit is very nice:-) \nThanks.\n-Jianghua","commit_id":"6298563672edfa6b702ac650ddb25afc18df370b"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"22b01c250a7d1fa4c3b7da2c4ae21d1d42582cf1","unresolved":false,"context_lines":[{"line_number":6999,"context_line":"            if vol[\u0027attachment_id\u0027] is None:"},{"line_number":7000,"context_line":"                # Cinder v2 api flow:"},{"line_number":7001,"context_line":"                # Retrieve connection info from Cinder\u0027s initialize_connection"},{"line_number":7002,"context_line":"                # API. The info returned will be accurate for the source"},{"line_number":7003,"context_line":"                # server."},{"line_number":7004,"context_line":"                volume_id \u003d connection_info[\u0027serial\u0027]"},{"line_number":7005,"context_line":"                connection_info \u003d volume_api.initialize_connection(context,"}],"source_content_type":"text/x-python","patch_set":13,"id":"1f1a1f67_ce98be1c","line":7002,"range":{"start_line":7002,"start_character":23,"end_line":7002,"end_character":72},"in_reply_to":"1f1a1f67_1db285e7","updated":"2017-07-14 01:33:22.000000000","message":"Ildiko, I agree with you:-) thanks.","commit_id":"6298563672edfa6b702ac650ddb25afc18df370b"},{"author":{"_account_id":9562,"name":"Ildiko Vancsa","email":"ildiko.vancsa@gmail.com","username":"ildikov"},"change_message_id":"ff606ea08f9e37c1c9f0d4a39d3c89afd3e8c557","unresolved":false,"context_lines":[{"line_number":6999,"context_line":"            if vol[\u0027attachment_id\u0027] is None:"},{"line_number":7000,"context_line":"                # Cinder v2 api flow:"},{"line_number":7001,"context_line":"                # Retrieve connection info from Cinder\u0027s initialize_connection"},{"line_number":7002,"context_line":"                # API. The info returned will be accurate for the source"},{"line_number":7003,"context_line":"                # server."},{"line_number":7004,"context_line":"                volume_id \u003d connection_info[\u0027serial\u0027]"},{"line_number":7005,"context_line":"                connection_info \u003d volume_api.initialize_connection(context,"}],"source_content_type":"text/x-python","patch_set":13,"id":"1f1a1f67_1db285e7","line":7002,"range":{"start_line":7002,"start_character":23,"end_line":7002,"end_character":72},"in_reply_to":"1f1a1f67_29706ba5","updated":"2017-07-13 12:09:33.000000000","message":"In my view this problem is independent from this change and we should figure out a better solution for it. It looks like something that should also affect all the drivers and not just Libvirt if it really happens.\n\nLooks more like a corner case, but we should definitely think about how to address it. It should be part of a follow up patch as it is not an issue introduced here or that would affect only the new flow.","commit_id":"6298563672edfa6b702ac650ddb25afc18df370b"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"22d06b66682f597191230a5c0623ce430e09ce58","unresolved":false,"context_lines":[{"line_number":6999,"context_line":"            if vol[\u0027attachment_id\u0027] is None:"},{"line_number":7000,"context_line":"                # Cinder v2 api flow:"},{"line_number":7001,"context_line":"                # Retrieve connection info from Cinder\u0027s initialize_connection"},{"line_number":7002,"context_line":"                # API. The info returned will be accurate for the source"},{"line_number":7003,"context_line":"                # server."},{"line_number":7004,"context_line":"                volume_id \u003d connection_info[\u0027serial\u0027]"},{"line_number":7005,"context_line":"                connection_info \u003d volume_api.initialize_connection(context,"}],"source_content_type":"text/x-python","patch_set":13,"id":"1f1a1f67_1094d58a","line":7002,"range":{"start_line":7002,"start_character":23,"end_line":7002,"end_character":72},"in_reply_to":"1f1a1f67_48f2ab08","updated":"2017-07-12 13:16:42.000000000","message":"I agree that it is strange that libvirt is the only driver talking to cinder. I don\u0027t know why this logic is here to get connection_info from cinder instead of the bdm. Migration works fine (in v3.27) without doing that. (Maybe Matt knows why this is here?)\n\nI left this in place as I did not want to change v2 behavior with this change. Once the v2 code is removed in some future release, this can be removed and the libvirt driver will not be calling cinder any more.","commit_id":"6298563672edfa6b702ac650ddb25afc18df370b"},{"author":{"_account_id":9562,"name":"Ildiko Vancsa","email":"ildiko.vancsa@gmail.com","username":"ildikov"},"change_message_id":"481d94227a748e7694065738339185b7f45dd4d1","unresolved":false,"context_lines":[{"line_number":6999,"context_line":"            if vol[\u0027attachment_id\u0027] is None:"},{"line_number":7000,"context_line":"                # Cinder v2 api flow:"},{"line_number":7001,"context_line":"                # Retrieve connection info from Cinder\u0027s initialize_connection"},{"line_number":7002,"context_line":"                # API. The info returned will be accurate for the source"},{"line_number":7003,"context_line":"                # server."},{"line_number":7004,"context_line":"                volume_id \u003d connection_info[\u0027serial\u0027]"},{"line_number":7005,"context_line":"                connection_info \u003d volume_api.initialize_connection(context,"}],"source_content_type":"text/x-python","patch_set":13,"id":"3f1d235d_eccb5da2","line":7002,"range":{"start_line":7002,"start_character":23,"end_line":7002,"end_character":72},"in_reply_to":"3f1d235d_9b3b01c7","updated":"2017-07-12 10:10:09.000000000","message":"In case of the new attach/detach API in Cinder, which is available from microversion 3.27 initialize_connection is part of attachment_create and attachment_update and is not used standalone anymore. Hence this switch here to the approach that XenAPI is using as well already.","commit_id":"6298563672edfa6b702ac650ddb25afc18df370b"},{"author":{"_account_id":8543,"name":"Lucian Petrut","email":"lpetrut@cloudbasesolutions.com","username":"plucian"},"change_message_id":"809e1866b58478910a318839d4b01b4eab0df146","unresolved":false,"context_lines":[{"line_number":6999,"context_line":"            if vol[\u0027attachment_id\u0027] is None:"},{"line_number":7000,"context_line":"                # Cinder v2 api flow:"},{"line_number":7001,"context_line":"                # Retrieve connection info from Cinder\u0027s initialize_connection"},{"line_number":7002,"context_line":"                # API. The info returned will be accurate for the source"},{"line_number":7003,"context_line":"                # server."},{"line_number":7004,"context_line":"                volume_id \u003d connection_info[\u0027serial\u0027]"},{"line_number":7005,"context_line":"                connection_info \u003d volume_api.initialize_connection(context,"}],"source_content_type":"text/x-python","patch_set":13,"id":"1f1a1f67_48f2ab08","line":7002,"range":{"start_line":7002,"start_character":23,"end_line":7002,"end_character":72},"in_reply_to":"3f1d235d_eccb5da2","updated":"2017-07-12 11:05:00.000000000","message":"I\u0027m a bit concerned about the v2 workflow here. What can we expect from the connection_info contained by the bdm? At this point, is it the connection_info used by the destination node? Trying to undestand why does the driver have to call the Cinder API here.\n\nFor the Hyper-V driver, we were never doing this, just using the connection info from the bdm. If this is in fact the conn info used by the destination node, we may have an issue. I guess we never noticed it as we\u0027re usually testing with backends for which the same connection info is valid on both sides. For FibreChannel, disconnecting volumes is a NOOP in our case, so that went silently as well.\n\nIf that\u0027s the case, shouldn\u0027t this kind of logic be moved up to the manager?","commit_id":"6298563672edfa6b702ac650ddb25afc18df370b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"54a5a32260552d80102acebfb8d180921119a0c6","unresolved":false,"context_lines":[{"line_number":7066,"context_line":"        self._fetch_instance_kernel_ramdisk("},{"line_number":7067,"context_line":"            context, instance, fallback_from_host\u003dfallback_from_host)"},{"line_number":7068,"context_line":""},{"line_number":7069,"context_line":"    def post_live_migration(self, context, instance, block_device_info,"},{"line_number":7070,"context_line":"                            migrate_data\u003dNone):"},{"line_number":7071,"context_line":"        # Disconnect from volume server"},{"line_number":7072,"context_line":"        block_device_mapping \u003d driver.block_device_info_get_mapping("}],"source_content_type":"text/x-python","patch_set":18,"id":"9f5c4f37_562483c6","line":7069,"range":{"start_line":7069,"start_character":8,"end_line":7069,"end_character":27},"updated":"2017-09-21 19:20:31.000000000","message":"OK so this is running on the source node, and is where we are disconnecting things from the source once we\u0027ve had a successful live migration to the destination. I always have to look this up again.\n\nhttps://photos.app.goo.gl/Q8JdpjM0PZhAzsv32\n\nTODO(me): ^ get that in the nova docs","commit_id":"f576578b2e1050def3ec2995fb4438b27a66aab4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"54a5a32260552d80102acebfb8d180921119a0c6","unresolved":false,"context_lines":[{"line_number":7074,"context_line":"        connector \u003d self.get_volume_connector(instance)"},{"line_number":7075,"context_line":"        volume_api \u003d self._volume_api"},{"line_number":7076,"context_line":"        for vol in block_device_mapping:"},{"line_number":7077,"context_line":"            connection_info \u003d vol[\u0027connection_info\u0027]"},{"line_number":7078,"context_line":"            if vol[\u0027attachment_id\u0027] is None:"},{"line_number":7079,"context_line":"                # Cinder v2 api flow:"},{"line_number":7080,"context_line":"                # Retrieve connection info from Cinder\u0027s initialize_connection"}],"source_content_type":"text/x-python","patch_set":18,"id":"7f515b1d_e6499e32","line":7077,"range":{"start_line":7077,"start_character":30,"end_line":7077,"end_character":52},"updated":"2017-09-21 19:20:31.000000000","message":"How do we know that this is accurate for the source host with a new style attachment? The connection_info is stored in cinder per volume/instance pair, so why don\u0027t we have to get the latest attachment data from cinder to we know we have the latest connection_info?\n\nIf there is something else with how pre_live_migration works for new style attachments, we should probably make a note of it here.","commit_id":"f576578b2e1050def3ec2995fb4438b27a66aab4"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"7b29f9a04263a131d2699345327692fe81fdc5c8","unresolved":false,"context_lines":[{"line_number":7074,"context_line":"        connector \u003d self.get_volume_connector(instance)"},{"line_number":7075,"context_line":"        volume_api \u003d self._volume_api"},{"line_number":7076,"context_line":"        for vol in block_device_mapping:"},{"line_number":7077,"context_line":"            connection_info \u003d vol[\u0027connection_info\u0027]"},{"line_number":7078,"context_line":"            if vol[\u0027attachment_id\u0027] is None:"},{"line_number":7079,"context_line":"                # Cinder v2 api flow:"},{"line_number":7080,"context_line":"                # Retrieve connection info from Cinder\u0027s initialize_connection"}],"source_content_type":"text/x-python","patch_set":18,"id":"7f515b1d_1f171154","line":7077,"range":{"start_line":7077,"start_character":30,"end_line":7077,"end_character":52},"in_reply_to":"7f515b1d_e6499e32","updated":"2017-09-25 20:57:58.000000000","message":"This was not correct. I now call attachment_get to the get old attachment\u0027s connection_info.","commit_id":"f576578b2e1050def3ec2995fb4438b27a66aab4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"54a5a32260552d80102acebfb8d180921119a0c6","unresolved":false,"context_lines":[{"line_number":7076,"context_line":"        for vol in block_device_mapping:"},{"line_number":7077,"context_line":"            connection_info \u003d vol[\u0027connection_info\u0027]"},{"line_number":7078,"context_line":"            if vol[\u0027attachment_id\u0027] is None:"},{"line_number":7079,"context_line":"                # Cinder v2 api flow:"},{"line_number":7080,"context_line":"                # Retrieve connection info from Cinder\u0027s initialize_connection"},{"line_number":7081,"context_line":"                # API. The info returned will be accurate for the source"},{"line_number":7082,"context_line":"                # server."}],"source_content_type":"text/x-python","patch_set":18,"id":"9f5c4f37_560da33c","line":7079,"range":{"start_line":7079,"start_character":16,"end_line":7079,"end_character":37},"updated":"2017-09-21 19:20:31.000000000","message":"I wonder if we should put a comment in here about what happens with a new style attachment? That presumably happens in the compute manager, right? (note: I haven\u0027t reviewed through the compute manager changes yet).","commit_id":"f576578b2e1050def3ec2995fb4438b27a66aab4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"54a5a32260552d80102acebfb8d180921119a0c6","unresolved":false,"context_lines":[{"line_number":7095,"context_line":"            # multipath_id can be placed into the connection info"},{"line_number":7096,"context_line":"            # because it is based off of the volume and will be the"},{"line_number":7097,"context_line":"            # same on the source and destination hosts."},{"line_number":7098,"context_line":"            if \u0027multipath_id\u0027 in vol[\u0027connection_info\u0027][\u0027data\u0027]:"},{"line_number":7099,"context_line":"                multipath_id \u003d vol[\u0027connection_info\u0027][\u0027data\u0027][\u0027multipath_id\u0027]"},{"line_number":7100,"context_line":"                connection_info[\u0027data\u0027][\u0027multipath_id\u0027] \u003d multipath_id"},{"line_number":7101,"context_line":""}],"source_content_type":"text/x-python","patch_set":18,"id":"9f5c4f37_7671c7e0","line":7098,"range":{"start_line":7098,"start_character":33,"end_line":7098,"end_character":55},"updated":"2017-09-21 19:20:31.000000000","message":"Just use connection_info now? I guess it wasn\u0027t before either. Seems weird if the connection_info could change when we initialize the connection (for the old flow I mean).","commit_id":"f576578b2e1050def3ec2995fb4438b27a66aab4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0084b9a0d261fbe810036ea28d162a3853348f19","unresolved":false,"context_lines":[{"line_number":7083,"context_line":"                # cinder v3.44 api flow: Retrieve the connection_info for"},{"line_number":7084,"context_line":"                # the old attachment from cinder."},{"line_number":7085,"context_line":"                old_attachment_id \u003d \\"},{"line_number":7086,"context_line":"                    migrate_data.old_vol_attachment_ids[volume_id]"},{"line_number":7087,"context_line":"                old_attachment \u003d volume_api.attachment_get("},{"line_number":7088,"context_line":"                    context, old_attachment_id)"},{"line_number":7089,"context_line":"                connection_info \u003d old_attachment[\u0027connection_info\u0027]"}],"source_content_type":"text/x-python","patch_set":20,"id":"7f515b1d_6f0b8c23","line":7086,"range":{"start_line":7086,"start_character":33,"end_line":7086,"end_character":55},"updated":"2017-09-28 16:13:34.000000000","message":"nit: the naming on \"old_vol_attachment_ids\" is plural, but we only get one value back. I think I commented on this in PS18 in the MigrateData object code, but this could be confusing. I think it\u0027s written with that name to set the groundwork for multi-attach? But still, migrate_data is per instance live migration operation, and volume:attachment:instance is 1:1 for that, so it seems we should just call that \"old_vol_attachment_id\", singular. Does that make sense?","commit_id":"f261ce45d44e160f6988ee87981a2342fd018b2c"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"2303fd1084aede17c3f4c0f2a78ac43a483773c4","unresolved":false,"context_lines":[{"line_number":7083,"context_line":"                # cinder v3.44 api flow: Retrieve the connection_info for"},{"line_number":7084,"context_line":"                # the old attachment from cinder."},{"line_number":7085,"context_line":"                old_attachment_id \u003d \\"},{"line_number":7086,"context_line":"                    migrate_data.old_vol_attachment_ids[volume_id]"},{"line_number":7087,"context_line":"                old_attachment \u003d volume_api.attachment_get("},{"line_number":7088,"context_line":"                    context, old_attachment_id)"},{"line_number":7089,"context_line":"                connection_info \u003d old_attachment[\u0027connection_info\u0027]"}],"source_content_type":"text/x-python","patch_set":20,"id":"7f515b1d_f270830a","line":7086,"range":{"start_line":7086,"start_character":33,"end_line":7086,"end_character":55},"in_reply_to":"7f515b1d_6f0b8c23","updated":"2017-09-28 17:31:26.000000000","message":"I made it plural because this will contain an attachment_id for each volume attached to the instance of which there can be many. So the structure itself is plural, the values inside are singular.","commit_id":"f261ce45d44e160f6988ee87981a2342fd018b2c"}]}