)]}'
{"nova/tests/unit/virt/test_block_device.py":[{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"91ca5ab40661588a3fed994ee52e02a5bf018857","unresolved":false,"context_lines":[{"line_number":431,"context_line":"                                       \u0027fake_uuid\u0027, bdm_dict[\u0027device_name\u0027],"},{"line_number":432,"context_line":"                                        mode\u003daccess_mode).AndRaise("},{"line_number":433,"context_line":"                                            test.TestingException)"},{"line_number":434,"context_line":"                self.virt_driver.detach_volume("},{"line_number":435,"context_line":"                        expected_conn_info, instance,"},{"line_number":436,"context_line":"                        bdm_dict[\u0027device_name\u0027],"},{"line_number":437,"context_line":"                        encryption\u003denc_data).AndReturn(None)"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa32b979_496fc35f","line":434,"updated":"2015-06-25 15:20:37.000000000","message":"I think a test with do_driver_attach\u003dFalse is needed here.","commit_id":"ec5c4eefe1b6aaca00ec11acefceb5ac23064ad8"},{"author":{"_account_id":9171,"name":"Vipin Balachandran","email":"vipin.bl@gmail.com","username":"vbala"},"change_message_id":"90db09962713666516fa5a0778609aed7a3ceeb3","unresolved":false,"context_lines":[{"line_number":431,"context_line":"                                       \u0027fake_uuid\u0027, bdm_dict[\u0027device_name\u0027],"},{"line_number":432,"context_line":"                                        mode\u003daccess_mode).AndRaise("},{"line_number":433,"context_line":"                                            test.TestingException)"},{"line_number":434,"context_line":"                self.virt_driver.detach_volume("},{"line_number":435,"context_line":"                        expected_conn_info, instance,"},{"line_number":436,"context_line":"                        bdm_dict[\u0027device_name\u0027],"},{"line_number":437,"context_line":"                        encryption\u003denc_data).AndReturn(None)"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa32b979_865351e2","line":434,"in_reply_to":"fa32b979_496fc35f","updated":"2015-06-26 10:14:14.000000000","message":"I think we have to do detach_volume irrespective of whether do_driver_detach is true or false.\n\nThe comment says:\n\n\"If do_driver_attach is False, we will attach a volume to an instance at boot time. So actual attach is done by instance creation code.\"","commit_id":"ec5c4eefe1b6aaca00ec11acefceb5ac23064ad8"}],"nova/virt/block_device.py":[{"author":{"_account_id":5638,"name":"Davanum Srinivas","email":"davanum@gmail.com","username":"dims-v"},"change_message_id":"f829f2d96927c6fc7dfe684f92a840b28590539c","unresolved":false,"context_lines":[{"line_number":278,"context_line":"            except Exception:"},{"line_number":279,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":280,"context_line":"                    if do_driver_attach:"},{"line_number":281,"context_line":"                        virt_driver.detach_volume(connection_info,"},{"line_number":282,"context_line":"                                                  instance,"},{"line_number":283,"context_line":"                                                  self[\u0027mount_device\u0027],"},{"line_number":284,"context_line":"                                                  encryption\u003dencryption)"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a016987_94ae702c","line":281,"updated":"2015-05-29 13:33:10.000000000","message":"Do we have to call volume_api.terminate_connection if detach_volume fails? (see except for attach_volume, we do that there)","commit_id":"fd7777fc0ea3ca208fc9c449c3d3ccd48df24a33"},{"author":{"_account_id":9171,"name":"Vipin Balachandran","email":"vipin.bl@gmail.com","username":"vbala"},"change_message_id":"daed2ba253be5c26101dd69e3c182714df35124f","unresolved":false,"context_lines":[{"line_number":278,"context_line":"            except Exception:"},{"line_number":279,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":280,"context_line":"                    if do_driver_attach:"},{"line_number":281,"context_line":"                        virt_driver.detach_volume(connection_info,"},{"line_number":282,"context_line":"                                                  instance,"},{"line_number":283,"context_line":"                                                  self[\u0027mount_device\u0027],"},{"line_number":284,"context_line":"                                                  encryption\u003dencryption)"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a016987_6fd8bd1e","line":281,"in_reply_to":"7a016987_94ae702c","updated":"2015-05-29 14:15:29.000000000","message":"I think irrespective of virt_driver.detach_volume success or failure, if there is a failure after line 239 (volume_api.initialize_connection) we should call volume_api.terminate_connection. Therefore we should call terminate_connection in volume_api.attach exception handling. But it won\u0027t result in data loss which this patch is trying to address. Anyway I will update the patch to call terminate_connection.","commit_id":"fd7777fc0ea3ca208fc9c449c3d3ccd48df24a33"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"28f2062fecc0d5c803be547d8b080f002aa04cee","unresolved":false,"context_lines":[{"line_number":284,"context_line":"                                                      self[\u0027mount_device\u0027],"},{"line_number":285,"context_line":"                                                      encryption\u003dencryption)"},{"line_number":286,"context_line":"                        except Exception:"},{"line_number":287,"context_line":"                            LOG.warn(_LE(\"Driver failed to detach volume \""},{"line_number":288,"context_line":"                                         \"%(volume_id)s at %(mount_point)s.\"),"},{"line_number":289,"context_line":"                                     {\u0027volume_id\u0027: volume_id,"},{"line_number":290,"context_line":"                                      \u0027mount_point\u0027: self[\u0027mount_device\u0027]},"}],"source_content_type":"text/x-python","patch_set":4,"id":"7a016987_68d414eb","line":287,"updated":"2015-05-31 14:53:06.000000000","message":"_LE \u003d\u003e _LW","commit_id":"7c40c73e86a49aa668b824476eeb6b4816b5ffff"},{"author":{"_account_id":9171,"name":"Vipin Balachandran","email":"vipin.bl@gmail.com","username":"vbala"},"change_message_id":"1a04cd58b63a131e376998920edddde67ca4b849","unresolved":false,"context_lines":[{"line_number":284,"context_line":"                                                      self[\u0027mount_device\u0027],"},{"line_number":285,"context_line":"                                                      encryption\u003dencryption)"},{"line_number":286,"context_line":"                        except Exception:"},{"line_number":287,"context_line":"                            LOG.warn(_LE(\"Driver failed to detach volume \""},{"line_number":288,"context_line":"                                         \"%(volume_id)s at %(mount_point)s.\"),"},{"line_number":289,"context_line":"                                     {\u0027volume_id\u0027: volume_id,"},{"line_number":290,"context_line":"                                      \u0027mount_point\u0027: self[\u0027mount_device\u0027]},"}],"source_content_type":"text/x-python","patch_set":4,"id":"7a016987_efa00392","line":287,"in_reply_to":"7a016987_68d414eb","updated":"2015-06-01 08:08:44.000000000","message":"Done","commit_id":"7c40c73e86a49aa668b824476eeb6b4816b5ffff"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"924fa3ab0348cd597da29922a42ed2ed142d7bbe","unresolved":false,"context_lines":[{"line_number":276,"context_line":"            try:"},{"line_number":277,"context_line":"                volume_api.attach(context, volume_id, instance.uuid,"},{"line_number":278,"context_line":"                                  self[\u0027mount_device\u0027], mode\u003dmode)"},{"line_number":279,"context_line":"            except Exception:"},{"line_number":280,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":281,"context_line":"                    if do_driver_attach:"},{"line_number":282,"context_line":"                        try:"}],"source_content_type":"text/x-python","patch_set":6,"id":"7a016987_4945cc6c","line":279,"updated":"2015-06-01 13:14:33.000000000","message":"Unfortunately the bug odes not report the traceback so we don\u0027t know what the exception is raised","commit_id":"b893e9dce478d7c498a6f4a84e54f6ca30f18d28"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"e6885b768d611e3d88d20872cf1eca1d893e2a28","unresolved":false,"context_lines":[{"line_number":276,"context_line":"            try:"},{"line_number":277,"context_line":"                volume_api.attach(context, volume_id, instance.uuid,"},{"line_number":278,"context_line":"                                  self[\u0027mount_device\u0027], mode\u003dmode)"},{"line_number":279,"context_line":"            except Exception:"},{"line_number":280,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":281,"context_line":"                    if do_driver_attach:"},{"line_number":282,"context_line":"                        try:"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa32b979_c97c3373","line":279,"in_reply_to":"7a016987_4945cc6c","updated":"2015-06-25 15:20:46.000000000","message":"This change is also a prerequisite to some fixes in Cinder - https://review.openstack.org/#/c/149894/ . We need to be able to return VolumeIsBusy from attach. So this should handle at least this one.","commit_id":"b893e9dce478d7c498a6f4a84e54f6ca30f18d28"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"924fa3ab0348cd597da29922a42ed2ed142d7bbe","unresolved":false,"context_lines":[{"line_number":284,"context_line":"                                                      instance,"},{"line_number":285,"context_line":"                                                      self[\u0027mount_device\u0027],"},{"line_number":286,"context_line":"                                                      encryption\u003dencryption)"},{"line_number":287,"context_line":"                        except Exception:"},{"line_number":288,"context_line":"                            LOG.warn(_LW(\"Driver failed to detach volume \""},{"line_number":289,"context_line":"                                         \"%(volume_id)s at %(mount_point)s.\"),"},{"line_number":290,"context_line":"                                     {\u0027volume_id\u0027: volume_id,"}],"source_content_type":"text/x-python","patch_set":6,"id":"7a016987_e9a8f821","line":287,"updated":"2015-06-01 13:14:33.000000000","message":"We can probably be more specific here? the exception should be NotImplementedError or NovaException.","commit_id":"b893e9dce478d7c498a6f4a84e54f6ca30f18d28"},{"author":{"_account_id":9171,"name":"Vipin Balachandran","email":"vipin.bl@gmail.com","username":"vbala"},"change_message_id":"8d7139306087a863522824143cf12adf213c2c72","unresolved":false,"context_lines":[{"line_number":284,"context_line":"                                                      instance,"},{"line_number":285,"context_line":"                                                      self[\u0027mount_device\u0027],"},{"line_number":286,"context_line":"                                                      encryption\u003dencryption)"},{"line_number":287,"context_line":"                        except Exception:"},{"line_number":288,"context_line":"                            LOG.warn(_LW(\"Driver failed to detach volume \""},{"line_number":289,"context_line":"                                         \"%(volume_id)s at %(mount_point)s.\"),"},{"line_number":290,"context_line":"                                     {\u0027volume_id\u0027: volume_id,"}],"source_content_type":"text/x-python","patch_set":6,"id":"7a016987_629d3abb","line":287,"in_reply_to":"7a016987_e9a8f821","updated":"2015-06-01 15:14:15.000000000","message":"Could you please see line 256? There are other places where we are following this pattern.\n\nIf there is an exception during detach, please tell me why our decision to terminate a previously initialized connection need to depend on the type of exception.","commit_id":"b893e9dce478d7c498a6f4a84e54f6ca30f18d28"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"b825a5eb8394f11cf8e7f9210c87b64053530608","unresolved":false,"context_lines":[{"line_number":290,"context_line":"                                     {\u0027volume_id\u0027: volume_id,"},{"line_number":291,"context_line":"                                      \u0027mount_point\u0027: self[\u0027mount_device\u0027]},"},{"line_number":292,"context_line":"                                     exc_info\u003dTrue)"},{"line_number":293,"context_line":"                    volume_api.terminate_connection(context, volume_id,"},{"line_number":294,"context_line":"                                                    connector)"},{"line_number":295,"context_line":""},{"line_number":296,"context_line":"    @update_db"}],"source_content_type":"text/x-python","patch_set":6,"id":"5afe65bd_661751ee","line":293,"updated":"2015-06-02 08:51:21.000000000","message":"Hi,\n\nWith this change volume will be detached from instance after exception in \u0027attach\u0027 call, but the cinder shows the status of volume as \u0027in-use\u0027.\n\nIMO, there is need to call \u0027volume_api.detach\u0027 after \u0027volume_api.terminate_connection\u0027.","commit_id":"b893e9dce478d7c498a6f4a84e54f6ca30f18d28"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"e6885b768d611e3d88d20872cf1eca1d893e2a28","unresolved":false,"context_lines":[{"line_number":290,"context_line":"                                     {\u0027volume_id\u0027: volume_id,"},{"line_number":291,"context_line":"                                      \u0027mount_point\u0027: self[\u0027mount_device\u0027]},"},{"line_number":292,"context_line":"                                     exc_info\u003dTrue)"},{"line_number":293,"context_line":"                    volume_api.terminate_connection(context, volume_id,"},{"line_number":294,"context_line":"                                                    connector)"},{"line_number":295,"context_line":""},{"line_number":296,"context_line":"    @update_db"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa32b979_89cb8b71","line":293,"in_reply_to":"5afe65bd_0f0d2490","updated":"2015-06-25 15:20:46.000000000","message":"I agree with Vipin here.","commit_id":"b893e9dce478d7c498a6f4a84e54f6ca30f18d28"},{"author":{"_account_id":9171,"name":"Vipin Balachandran","email":"vipin.bl@gmail.com","username":"vbala"},"change_message_id":"5580baace9d144cadab4863dae836c9fe0abf7c4","unresolved":false,"context_lines":[{"line_number":290,"context_line":"                                     {\u0027volume_id\u0027: volume_id,"},{"line_number":291,"context_line":"                                      \u0027mount_point\u0027: self[\u0027mount_device\u0027]},"},{"line_number":292,"context_line":"                                     exc_info\u003dTrue)"},{"line_number":293,"context_line":"                    volume_api.terminate_connection(context, volume_id,"},{"line_number":294,"context_line":"                                                    connector)"},{"line_number":295,"context_line":""},{"line_number":296,"context_line":"    @update_db"}],"source_content_type":"text/x-python","patch_set":6,"id":"5afe65bd_78792993","line":293,"in_reply_to":"5afe65bd_661751ee","updated":"2015-06-02 12:31:35.000000000","message":"Abhishek,\nWe change the volume state to in-use in Cinder attach. If volume_api.attach failed why do we need to call volume_api.detach?","commit_id":"b893e9dce478d7c498a6f4a84e54f6ca30f18d28"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"181b1697fd0196a3a319a9315bba38a9b272e2ed","unresolved":false,"context_lines":[{"line_number":290,"context_line":"                                     {\u0027volume_id\u0027: volume_id,"},{"line_number":291,"context_line":"                                      \u0027mount_point\u0027: self[\u0027mount_device\u0027]},"},{"line_number":292,"context_line":"                                     exc_info\u003dTrue)"},{"line_number":293,"context_line":"                    volume_api.terminate_connection(context, volume_id,"},{"line_number":294,"context_line":"                                                    connector)"},{"line_number":295,"context_line":""},{"line_number":296,"context_line":"    @update_db"}],"source_content_type":"text/x-python","patch_set":6,"id":"5afe65bd_8f56b4d0","line":293,"in_reply_to":"5afe65bd_78792993","updated":"2015-06-03 05:57:52.000000000","message":"Hi Vipin,\n\nThis could happen in a very rare scenario where cinder sets the volume status to \"in-use\"  and just before sending the response back to nova for attach request, it fails due to unknown reasons (like rabbitmq or cinder-vol down). Now, in this situation, the volume status is \"in-use\", bdm is removed and there is no way user can detach volume without admin\u0027s intervention.\n\nIf you attempt to detach volume on the cinder node as a rollback action, then there is a possibility the previous issue got fix due to HA configuration and it would then set the volume status to \"available\".","commit_id":"b893e9dce478d7c498a6f4a84e54f6ca30f18d28"},{"author":{"_account_id":9171,"name":"Vipin Balachandran","email":"vipin.bl@gmail.com","username":"vbala"},"change_message_id":"5868bcbd760a6686af772392ec02b3bdc12b95e6","unresolved":false,"context_lines":[{"line_number":290,"context_line":"                                     {\u0027volume_id\u0027: volume_id,"},{"line_number":291,"context_line":"                                      \u0027mount_point\u0027: self[\u0027mount_device\u0027]},"},{"line_number":292,"context_line":"                                     exc_info\u003dTrue)"},{"line_number":293,"context_line":"                    volume_api.terminate_connection(context, volume_id,"},{"line_number":294,"context_line":"                                                    connector)"},{"line_number":295,"context_line":""},{"line_number":296,"context_line":"    @update_db"}],"source_content_type":"text/x-python","patch_set":6,"id":"5afe65bd_0f0d2490","line":293,"in_reply_to":"5afe65bd_8f56b4d0","updated":"2015-06-03 06:13:08.000000000","message":"We have faced this problem before. It was due to RabbitMQ. If it is due to RPC timeout between c-api and c-vol, it\u0027s not guaranteed that the problem will  be resolved by the time  we call Cinder detach. Therefore, calling detach won\u0027t solve this problem completely. In our case, we were using heat and we handled it there.\n\nAnother issue is, if the attach didn\u0027t happen in c-vol (volume state never changed to in-use), then the detach can result in some confusing error messages.\n\nHaving said that I\u0027m feeling neutral about your suggestion and don\u0027t mind doing it if you feel strongly about it. But this patch is trying to handle the data-loss due to cleanup issues and it\u0027s better if we can implement your suggestion in a different patch.","commit_id":"b893e9dce478d7c498a6f4a84e54f6ca30f18d28"},{"author":{"_account_id":7173,"name":"Scott DAngelo","email":"scott.dangelo@gmail.com","username":"scottda"},"change_message_id":"d277649d3522bb5593916d961530ea21797cd775","unresolved":false,"context_lines":[{"line_number":292,"context_line":"                                 exc_info\u003dTrue)"},{"line_number":293,"context_line":"                    volume_api.terminate_connection(context, volume_id,"},{"line_number":294,"context_line":"                                                    connector)"},{"line_number":295,"context_line":""},{"line_number":296,"context_line":"    @update_db"},{"line_number":297,"context_line":"    def refresh_connection_info(self, context, instance,"},{"line_number":298,"context_line":"                                volume_api, virt_driver):"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba3cc151_2f276e13","line":295,"updated":"2015-07-07 15:59:32.000000000","message":"We should also call volume_api.detach(...)","commit_id":"d278ca3df4a39bc93c20e81e6d95099bb26f57b9"},{"author":{"_account_id":9171,"name":"Vipin Balachandran","email":"vipin.bl@gmail.com","username":"vbala"},"change_message_id":"1e64bfc50751a049f18d2f6ee5369c5726b8647d","unresolved":false,"context_lines":[{"line_number":292,"context_line":"                                 exc_info\u003dTrue)"},{"line_number":293,"context_line":"                    volume_api.terminate_connection(context, volume_id,"},{"line_number":294,"context_line":"                                                    connector)"},{"line_number":295,"context_line":""},{"line_number":296,"context_line":"    @update_db"},{"line_number":297,"context_line":"    def refresh_connection_info(self, context, instance,"},{"line_number":298,"context_line":"                                volume_api, virt_driver):"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba3cc151_fcb11e90","line":295,"in_reply_to":"ba3cc151_2f276e13","updated":"2015-07-08 08:57:45.000000000","message":"Done","commit_id":"d278ca3df4a39bc93c20e81e6d95099bb26f57b9"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d429f2aaa42fd186517bc1cc7d8ee5e02d53a935","unresolved":false,"context_lines":[{"line_number":285,"context_line":"                        virt_driver.detach_volume(connection_info,"},{"line_number":286,"context_line":"                                                  instance,"},{"line_number":287,"context_line":"                                                  self[\u0027mount_device\u0027],"},{"line_number":288,"context_line":"                                                  encryption\u003dencryption)"},{"line_number":289,"context_line":"                    except Exception:"},{"line_number":290,"context_line":"                        LOG.warn(_LW(\"Driver failed to detach volume \""},{"line_number":291,"context_line":"                                     \"%(volume_id)s at %(mount_point)s.\"),"}],"source_content_type":"text/x-python","patch_set":9,"id":"1a4dcd0f_73313bec","line":288,"updated":"2015-08-03 17:25:06.000000000","message":"If do_driver_attach is False, encryption is not a variable that\u0027s in scope.","commit_id":"e11043a6520d27a77b07f7c7221621f6b24b750d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"de1279c19d753dd18ed4281a76a8eefed6e7f815","unresolved":false,"context_lines":[{"line_number":285,"context_line":"                        virt_driver.detach_volume(connection_info,"},{"line_number":286,"context_line":"                                                  instance,"},{"line_number":287,"context_line":"                                                  self[\u0027mount_device\u0027],"},{"line_number":288,"context_line":"                                                  encryption\u003dencryption)"},{"line_number":289,"context_line":"                    except Exception:"},{"line_number":290,"context_line":"                        LOG.warn(_LW(\"Driver failed to detach volume \""},{"line_number":291,"context_line":"                                     \"%(volume_id)s at %(mount_point)s.\"),"}],"source_content_type":"text/x-python","patch_set":9,"id":"1a4dcd0f_b33ca330","line":288,"in_reply_to":"1a4dcd0f_73313bec","updated":"2015-08-03 17:29:56.000000000","message":"do_driver_attach is left as False during build/rebuild/unshelve by calling from _prep_block_devices:\n\nhttp://git.openstack.org/cgit/openstack/nova/tree/nova/compute/manager.py#n1716","commit_id":"e11043a6520d27a77b07f7c7221621f6b24b750d"},{"author":{"_account_id":9171,"name":"Vipin Balachandran","email":"vipin.bl@gmail.com","username":"vbala"},"change_message_id":"a4951639e5e3ae1c838fa87f630d8fe846970d1c","unresolved":false,"context_lines":[{"line_number":285,"context_line":"                        virt_driver.detach_volume(connection_info,"},{"line_number":286,"context_line":"                                                  instance,"},{"line_number":287,"context_line":"                                                  self[\u0027mount_device\u0027],"},{"line_number":288,"context_line":"                                                  encryption\u003dencryption)"},{"line_number":289,"context_line":"                    except Exception:"},{"line_number":290,"context_line":"                        LOG.warn(_LW(\"Driver failed to detach volume \""},{"line_number":291,"context_line":"                                     \"%(volume_id)s at %(mount_point)s.\"),"}],"source_content_type":"text/x-python","patch_set":9,"id":"fa1b9901_fd51a30f","line":288,"in_reply_to":"1a4dcd0f_b33ca330","updated":"2015-08-17 10:49:16.000000000","message":"Done\n\nComment says \"If do_driver_attach is False, we will attach a volume to an instance at boot time. So actual attach is done by instance creation code. \". So I assume that the driver attach will happen only later.","commit_id":"e11043a6520d27a77b07f7c7221621f6b24b750d"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"67e4851ec6a056992f40db9e5b480aa0b8673ceb","unresolved":false,"context_lines":[{"line_number":291,"context_line":"                                     \"%(volume_id)s at %(mount_point)s.\"),"},{"line_number":292,"context_line":"                                 {\u0027volume_id\u0027: volume_id,"},{"line_number":293,"context_line":"                                  \u0027mount_point\u0027: self[\u0027mount_device\u0027]},"},{"line_number":294,"context_line":"                                 exc_info\u003dTrue)"},{"line_number":295,"context_line":"                    volume_api.terminate_connection(context, volume_id,"},{"line_number":296,"context_line":"                                                    connector)"},{"line_number":297,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"ba3cc151_89e3dad7","line":294,"updated":"2015-07-08 16:39:53.000000000","message":"consider adding instance\u003dinstance\nthis will help with the log traces for the specific instance","commit_id":"e11043a6520d27a77b07f7c7221621f6b24b750d"},{"author":{"_account_id":9171,"name":"Vipin Balachandran","email":"vipin.bl@gmail.com","username":"vbala"},"change_message_id":"a4951639e5e3ae1c838fa87f630d8fe846970d1c","unresolved":false,"context_lines":[{"line_number":291,"context_line":"                                     \"%(volume_id)s at %(mount_point)s.\"),"},{"line_number":292,"context_line":"                                 {\u0027volume_id\u0027: volume_id,"},{"line_number":293,"context_line":"                                  \u0027mount_point\u0027: self[\u0027mount_device\u0027]},"},{"line_number":294,"context_line":"                                 exc_info\u003dTrue)"},{"line_number":295,"context_line":"                    volume_api.terminate_connection(context, volume_id,"},{"line_number":296,"context_line":"                                                    connector)"},{"line_number":297,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"fa1b9901_1d6257d9","line":294,"in_reply_to":"ba3cc151_89e3dad7","updated":"2015-08-17 10:49:16.000000000","message":"Done","commit_id":"e11043a6520d27a77b07f7c7221621f6b24b750d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d429f2aaa42fd186517bc1cc7d8ee5e02d53a935","unresolved":false,"context_lines":[{"line_number":291,"context_line":"                                     \"%(volume_id)s at %(mount_point)s.\"),"},{"line_number":292,"context_line":"                                 {\u0027volume_id\u0027: volume_id,"},{"line_number":293,"context_line":"                                  \u0027mount_point\u0027: self[\u0027mount_device\u0027]},"},{"line_number":294,"context_line":"                                 exc_info\u003dTrue)"},{"line_number":295,"context_line":"                    volume_api.terminate_connection(context, volume_id,"},{"line_number":296,"context_line":"                                                    connector)"},{"line_number":297,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"1a4dcd0f_b33ee3b6","line":294,"in_reply_to":"ba3cc151_89e3dad7","updated":"2015-08-03 17:25:06.000000000","message":"Yes, add instance\u003dinstance kwarg to log messages for debugging.","commit_id":"e11043a6520d27a77b07f7c7221621f6b24b750d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d429f2aaa42fd186517bc1cc7d8ee5e02d53a935","unresolved":false,"context_lines":[{"line_number":298,"context_line":"                    # Cinder-volume might have completed volume attach."},{"line_number":299,"context_line":"                    try:"},{"line_number":300,"context_line":"                        volume_api.detach(context, volume_id)"},{"line_number":301,"context_line":"                    except Exception:"},{"line_number":302,"context_line":"                        # Ignore the exception since detach would fail if"},{"line_number":303,"context_line":"                        # cinder-volume attach failed."},{"line_number":304,"context_line":"                        LOG.warn(_LW(\"Failed to detach volume: %s\"),"}],"source_content_type":"text/x-python","patch_set":9,"id":"1a4dcd0f_33d5f342","line":301,"updated":"2015-08-03 17:25:06.000000000","message":"Where is this unit tested?","commit_id":"e11043a6520d27a77b07f7c7221621f6b24b750d"},{"author":{"_account_id":9171,"name":"Vipin Balachandran","email":"vipin.bl@gmail.com","username":"vbala"},"change_message_id":"a4951639e5e3ae1c838fa87f630d8fe846970d1c","unresolved":false,"context_lines":[{"line_number":298,"context_line":"                    # Cinder-volume might have completed volume attach."},{"line_number":299,"context_line":"                    try:"},{"line_number":300,"context_line":"                        volume_api.detach(context, volume_id)"},{"line_number":301,"context_line":"                    except Exception:"},{"line_number":302,"context_line":"                        # Ignore the exception since detach would fail if"},{"line_number":303,"context_line":"                        # cinder-volume attach failed."},{"line_number":304,"context_line":"                        LOG.warn(_LW(\"Failed to detach volume: %s\"),"}],"source_content_type":"text/x-python","patch_set":9,"id":"fa1b9901_dd9a9fc5","line":301,"in_reply_to":"1a4dcd0f_33d5f342","updated":"2015-08-17 10:49:16.000000000","message":"Added two unit tests to cover additional cases.","commit_id":"e11043a6520d27a77b07f7c7221621f6b24b750d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d429f2aaa42fd186517bc1cc7d8ee5e02d53a935","unresolved":false,"context_lines":[{"line_number":302,"context_line":"                        # Ignore the exception since detach would fail if"},{"line_number":303,"context_line":"                        # cinder-volume attach failed."},{"line_number":304,"context_line":"                        LOG.warn(_LW(\"Failed to detach volume: %s\"),"},{"line_number":305,"context_line":"                                 volume_id, exc_info\u003dTrue)"},{"line_number":306,"context_line":""},{"line_number":307,"context_line":"    @update_db"},{"line_number":308,"context_line":"    def refresh_connection_info(self, context, instance,"}],"source_content_type":"text/x-python","patch_set":9,"id":"1a4dcd0f_139b17cb","line":305,"updated":"2015-08-03 17:25:06.000000000","message":"Do we need the exc_info here if we don\u0027t care about it?  We don\u0027t need the stacktrace, just add the exception to the log message.","commit_id":"e11043a6520d27a77b07f7c7221621f6b24b750d"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"67e4851ec6a056992f40db9e5b480aa0b8673ceb","unresolved":false,"context_lines":[{"line_number":302,"context_line":"                        # Ignore the exception since detach would fail if"},{"line_number":303,"context_line":"                        # cinder-volume attach failed."},{"line_number":304,"context_line":"                        LOG.warn(_LW(\"Failed to detach volume: %s\"),"},{"line_number":305,"context_line":"                                 volume_id, exc_info\u003dTrue)"},{"line_number":306,"context_line":""},{"line_number":307,"context_line":"    @update_db"},{"line_number":308,"context_line":"    def refresh_connection_info(self, context, instance,"}],"source_content_type":"text/x-python","patch_set":9,"id":"ba3cc151_69cf7656","line":305,"updated":"2015-07-08 16:39:53.000000000","message":"ditto","commit_id":"e11043a6520d27a77b07f7c7221621f6b24b750d"},{"author":{"_account_id":9171,"name":"Vipin Balachandran","email":"vipin.bl@gmail.com","username":"vbala"},"change_message_id":"a4951639e5e3ae1c838fa87f630d8fe846970d1c","unresolved":false,"context_lines":[{"line_number":302,"context_line":"                        # Ignore the exception since detach would fail if"},{"line_number":303,"context_line":"                        # cinder-volume attach failed."},{"line_number":304,"context_line":"                        LOG.warn(_LW(\"Failed to detach volume: %s\"),"},{"line_number":305,"context_line":"                                 volume_id, exc_info\u003dTrue)"},{"line_number":306,"context_line":""},{"line_number":307,"context_line":"    @update_db"},{"line_number":308,"context_line":"    def refresh_connection_info(self, context, instance,"}],"source_content_type":"text/x-python","patch_set":9,"id":"fa1b9901_fd9f63b2","line":305,"in_reply_to":"1a4dcd0f_139b17cb","updated":"2015-08-17 10:49:16.000000000","message":"Done","commit_id":"e11043a6520d27a77b07f7c7221621f6b24b750d"},{"author":{"_account_id":5997,"name":"Walt","display_name":"Hemna","email":"waboring@hemna.com","username":"walter-boring","status":"SAP"},"change_message_id":"8b315d906480262c6bdf4f78f00900406c907870","unresolved":false,"context_lines":[{"line_number":229,"context_line":"    def attach(self, context, instance, volume_api, virt_driver,"},{"line_number":230,"context_line":"               do_check_attach\u003dTrue, do_driver_attach\u003dFalse, **kwargs):"},{"line_number":231,"context_line":"        volume \u003d volume_api.get(context, self.volume_id)"},{"line_number":232,"context_line":"        if do_check_attach:"},{"line_number":233,"context_line":"            volume_api.check_attach(context, volume, instance\u003dinstance)"},{"line_number":234,"context_line":""},{"line_number":235,"context_line":"        volume_id \u003d volume[\u0027id\u0027]"}],"source_content_type":"text/x-python","patch_set":10,"id":"fa1b9901_3649518c","line":232,"updated":"2015-08-18 05:24:15.000000000","message":"If we get into check_attach here, we know the volume has to be in \u0027available\u0027 status.   Shouldn\u0027t we also do a reserve_volume afterwards, to make sure the volume is put into attaching state, before calling initialize_conneciton ?","commit_id":"155923774e825398af3f93ed23437d367321c417"},{"author":{"_account_id":9171,"name":"Vipin Balachandran","email":"vipin.bl@gmail.com","username":"vbala"},"change_message_id":"08ee005bf0d802395ca322cad7308d19c3aa7a46","unresolved":false,"context_lines":[{"line_number":229,"context_line":"    def attach(self, context, instance, volume_api, virt_driver,"},{"line_number":230,"context_line":"               do_check_attach\u003dTrue, do_driver_attach\u003dFalse, **kwargs):"},{"line_number":231,"context_line":"        volume \u003d volume_api.get(context, self.volume_id)"},{"line_number":232,"context_line":"        if do_check_attach:"},{"line_number":233,"context_line":"            volume_api.check_attach(context, volume, instance\u003dinstance)"},{"line_number":234,"context_line":""},{"line_number":235,"context_line":"        volume_id \u003d volume[\u0027id\u0027]"}],"source_content_type":"text/x-python","patch_set":10,"id":"fa1b9901_6b0cb494","line":232,"in_reply_to":"fa1b9901_3649518c","updated":"2015-08-18 10:31:07.000000000","message":"check_attach and reserve_volume are called in nova/compute/api.py. I\u0027m not sure why we need a check_attach here. If this is really needed, we also need reserve_volume as you said. But that is a different problem.","commit_id":"155923774e825398af3f93ed23437d367321c417"},{"author":{"_account_id":10224,"name":"Feodor Tersin","email":"ftersin@hotmail.com","username":"ftersin"},"change_message_id":"b850a63bb63fc460500b45ab2b2125fd1685bf84","unresolved":false,"context_lines":[{"line_number":299,"context_line":""},{"line_number":300,"context_line":"                    # Cinder-volume might have completed volume attach."},{"line_number":301,"context_line":"                    try:"},{"line_number":302,"context_line":"                        volume_api.detach(context, volume_id)"},{"line_number":303,"context_line":"                    except Exception as ex:"},{"line_number":304,"context_line":"                        # Ignore the exception since detach would fail if"},{"line_number":305,"context_line":"                        # cinder-volume attach failed."}],"source_content_type":"text/x-python","patch_set":10,"id":"da20952f_663cf22f","line":302,"updated":"2015-09-02 09:54:47.000000000","message":"What is a case when this has to be called?","commit_id":"155923774e825398af3f93ed23437d367321c417"},{"author":{"_account_id":9171,"name":"Vipin Balachandran","email":"vipin.bl@gmail.com","username":"vbala"},"change_message_id":"3679072988115128676bcfd26857e1cdf5ca44bf","unresolved":false,"context_lines":[{"line_number":299,"context_line":""},{"line_number":300,"context_line":"                    # Cinder-volume might have completed volume attach."},{"line_number":301,"context_line":"                    try:"},{"line_number":302,"context_line":"                        volume_api.detach(context, volume_id)"},{"line_number":303,"context_line":"                    except Exception as ex:"},{"line_number":304,"context_line":"                        # Ignore the exception since detach would fail if"},{"line_number":305,"context_line":"                        # cinder-volume attach failed."}],"source_content_type":"text/x-python","patch_set":10,"id":"da20952f_8cb5a57f","line":302,"in_reply_to":"da20952f_663cf22f","updated":"2015-09-02 10:53:27.000000000","message":"This was added to address some review comments.\n\nSee\n\nhttps://review.openstack.org/#/c/186742/6/nova/virt/block_device.py\n\nand\n\nhttps://review.openstack.org/#/c/186742/8/nova/virt/block_device.py","commit_id":"155923774e825398af3f93ed23437d367321c417"},{"author":{"_account_id":10224,"name":"Feodor Tersin","email":"ftersin@hotmail.com","username":"ftersin"},"change_message_id":"c638ae6746cd2370656211cc73b2d951df0d0d64","unresolved":false,"context_lines":[{"line_number":299,"context_line":""},{"line_number":300,"context_line":"                    # Cinder-volume might have completed volume attach."},{"line_number":301,"context_line":"                    try:"},{"line_number":302,"context_line":"                        volume_api.detach(context, volume_id)"},{"line_number":303,"context_line":"                    except Exception as ex:"},{"line_number":304,"context_line":"                        # Ignore the exception since detach would fail if"},{"line_number":305,"context_line":"                        # cinder-volume attach failed."}],"source_content_type":"text/x-python","patch_set":10,"id":"da20952f_d2f2586f","line":302,"in_reply_to":"da20952f_8cb5a57f","updated":"2015-09-02 11:43:52.000000000","message":"aha, unknown reason, very rare case... Well, probably it makes sense. But since this reason is not clear, is it possible to add a comment here with an explanation of the reason?","commit_id":"155923774e825398af3f93ed23437d367321c417"},{"author":{"_account_id":10224,"name":"Feodor Tersin","email":"ftersin@hotmail.com","username":"ftersin"},"change_message_id":"b850a63bb63fc460500b45ab2b2125fd1685bf84","unresolved":false,"context_lines":[{"line_number":300,"context_line":"                    # Cinder-volume might have completed volume attach."},{"line_number":301,"context_line":"                    try:"},{"line_number":302,"context_line":"                        volume_api.detach(context, volume_id)"},{"line_number":303,"context_line":"                    except Exception as ex:"},{"line_number":304,"context_line":"                        # Ignore the exception since detach would fail if"},{"line_number":305,"context_line":"                        # cinder-volume attach failed."},{"line_number":306,"context_line":"                        LOG.warn(_LW(\"Error: %(error)s occurred while \""}],"source_content_type":"text/x-python","patch_set":10,"id":"da20952f_26efdab1","line":303,"updated":"2015-09-02 09:54:47.000000000","message":"Since https://review.openstack.org/#/c/212193/ is merged, you can do not handle this.","commit_id":"155923774e825398af3f93ed23437d367321c417"},{"author":{"_account_id":9171,"name":"Vipin Balachandran","email":"vipin.bl@gmail.com","username":"vbala"},"change_message_id":"3679072988115128676bcfd26857e1cdf5ca44bf","unresolved":false,"context_lines":[{"line_number":300,"context_line":"                    # Cinder-volume might have completed volume attach."},{"line_number":301,"context_line":"                    try:"},{"line_number":302,"context_line":"                        volume_api.detach(context, volume_id)"},{"line_number":303,"context_line":"                    except Exception as ex:"},{"line_number":304,"context_line":"                        # Ignore the exception since detach would fail if"},{"line_number":305,"context_line":"                        # cinder-volume attach failed."},{"line_number":306,"context_line":"                        LOG.warn(_LW(\"Error: %(error)s occurred while \""}],"source_content_type":"text/x-python","patch_set":10,"id":"da20952f_d72a4a7e","line":303,"in_reply_to":"da20952f_26efdab1","updated":"2015-09-02 10:53:27.000000000","message":"There can be other exceptions, for example the RPC call from c-api to c-vol can fail. An exception is also raised if driver.remove_export fails. There are several other cases. What is wrong in logging a warning in these cases?","commit_id":"155923774e825398af3f93ed23437d367321c417"},{"author":{"_account_id":10224,"name":"Feodor Tersin","email":"ftersin@hotmail.com","username":"ftersin"},"change_message_id":"c638ae6746cd2370656211cc73b2d951df0d0d64","unresolved":false,"context_lines":[{"line_number":300,"context_line":"                    # Cinder-volume might have completed volume attach."},{"line_number":301,"context_line":"                    try:"},{"line_number":302,"context_line":"                        volume_api.detach(context, volume_id)"},{"line_number":303,"context_line":"                    except Exception as ex:"},{"line_number":304,"context_line":"                        # Ignore the exception since detach would fail if"},{"line_number":305,"context_line":"                        # cinder-volume attach failed."},{"line_number":306,"context_line":"                        LOG.warn(_LW(\"Error: %(error)s occurred while \""}],"source_content_type":"text/x-python","patch_set":10,"id":"da20952f_bd03f575","line":303,"in_reply_to":"da20952f_d72a4a7e","updated":"2015-09-02 11:43:52.000000000","message":"But you don\u0027t handle errors for terminate_connection call. Why? Is this call safe? Or it doesn\u0027t raise an error which should be logged here? Or all detach errors should be suppressed in favor of an original exception (see save_and_reraise_exception behavior), but terminate_connection errors should not?\n\nIn other words i see the difference between these calls, but can not understand a reason of it.\n\nThis handling made sense when Cinder was failing on detaching of a not attached volume, but now it just takes space and raises questions.","commit_id":"155923774e825398af3f93ed23437d367321c417"},{"author":{"_account_id":6491,"name":"xing-yang","email":"xingyang105@gmail.com","username":"xing-yang"},"change_message_id":"a26b760d4851df1cd6c2f7fef09c42539692179a","unresolved":false,"context_lines":[{"line_number":235,"context_line":"        volume_id \u003d volume[\u0027id\u0027]"},{"line_number":236,"context_line":"        context \u003d context.elevated()"},{"line_number":237,"context_line":""},{"line_number":238,"context_line":"        connector \u003d virt_driver.get_volume_connector(instance)"},{"line_number":239,"context_line":"        connection_info \u003d volume_api.initialize_connection(context,"},{"line_number":240,"context_line":"                                                           volume_id,"},{"line_number":241,"context_line":"                                                           connector)"}],"source_content_type":"text/x-python","patch_set":11,"id":"ba15a1d1_2d193722","line":238,"updated":"2015-09-18 15:31:10.000000000","message":"So the problem we were trying to address is timeout in initialize_connection:\n\nhttps://review.openstack.org/#/c/138664/6/nova/volume/cinder.py","commit_id":"aa899506366cd84d5170cb913f33c0c0ca7a8b27"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"9e5c6348caf86b222a3bf83ff034f615ef415cd6","unresolved":false,"context_lines":[{"line_number":279,"context_line":"            try:"},{"line_number":280,"context_line":"                volume_api.attach(context, volume_id, instance.uuid,"},{"line_number":281,"context_line":"                                  self[\u0027mount_device\u0027], mode\u003dmode)"},{"line_number":282,"context_line":"            except Exception:"},{"line_number":283,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":284,"context_line":"                    if do_driver_attach:"},{"line_number":285,"context_line":"                        try:"}],"source_content_type":"text/x-python","patch_set":11,"id":"da20952f_dbadda84","line":282,"updated":"2015-09-07 17:25:12.000000000","message":"seems like we also do this error handling:\nhttps://github.com/openstack/nova/blob/master/nova/compute/manager.py#L4544\n\nIs that the correct order still?","commit_id":"aa899506366cd84d5170cb913f33c0c0ca7a8b27"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"7aca8a03af86e7640b048aa565f77a0b76cb081f","unresolved":false,"context_lines":[{"line_number":279,"context_line":"            try:"},{"line_number":280,"context_line":"                volume_api.attach(context, volume_id, instance.uuid,"},{"line_number":281,"context_line":"                                  self[\u0027mount_device\u0027], mode\u003dmode)"},{"line_number":282,"context_line":"            except Exception:"},{"line_number":283,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":284,"context_line":"                    if do_driver_attach:"},{"line_number":285,"context_line":"                        try:"}],"source_content_type":"text/x-python","patch_set":11,"id":"ba15a1d1_47da4a18","line":282,"in_reply_to":"da20952f_dbadda84","updated":"2015-09-18 15:03:44.000000000","message":"That line number has changed, what do you mean specifically about the error handling in the compute manager?  The only thing I see there is the volume is unreserved in cinder, which is undoing what nova-api does before casting off to nova-compute to do the driver attach.  And the block device mapping is deleted.","commit_id":"aa899506366cd84d5170cb913f33c0c0ca7a8b27"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"9e5c6348caf86b222a3bf83ff034f615ef415cd6","unresolved":false,"context_lines":[{"line_number":288,"context_line":"                                                      self[\u0027mount_device\u0027],"},{"line_number":289,"context_line":"                                                      encryption\u003dencryption)"},{"line_number":290,"context_line":"                        except Exception:"},{"line_number":291,"context_line":"                            LOG.warn(_LW(\"Driver failed to detach volume \""},{"line_number":292,"context_line":"                                         \"%(volume_id)s at %(mount_point)s.\"),"},{"line_number":293,"context_line":"                                     {\u0027volume_id\u0027: volume_id,"},{"line_number":294,"context_line":"                                      \u0027mount_point\u0027: self[\u0027mount_device\u0027]},"}],"source_content_type":"text/x-python","patch_set":11,"id":"da20952f_fb6e9ec6","line":291,"updated":"2015-09-07 17:25:12.000000000","message":"afraid we are past string freeze now, can we please move this to a debug log, with a TODO to update this to a warning log message?","commit_id":"aa899506366cd84d5170cb913f33c0c0ca7a8b27"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"727b3452404f7c5500efeba26b3dcdab6cfb075b","unresolved":false,"context_lines":[{"line_number":288,"context_line":"                                                      self[\u0027mount_device\u0027],"},{"line_number":289,"context_line":"                                                      encryption\u003dencryption)"},{"line_number":290,"context_line":"                        except Exception:"},{"line_number":291,"context_line":"                            LOG.warn(_LW(\"Driver failed to detach volume \""},{"line_number":292,"context_line":"                                         \"%(volume_id)s at %(mount_point)s.\"),"},{"line_number":293,"context_line":"                                     {\u0027volume_id\u0027: volume_id,"},{"line_number":294,"context_line":"                                      \u0027mount_point\u0027: self[\u0027mount_device\u0027]},"}],"source_content_type":"text/x-python","patch_set":11,"id":"ba15a1d1_72fceab7","line":291,"in_reply_to":"da20952f_fb6e9ec6","updated":"2015-09-18 15:12:25.000000000","message":"But now this is OK right?","commit_id":"aa899506366cd84d5170cb913f33c0c0ca7a8b27"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"19a4be454b17188ba1f6f07b6c77227652e1ba31","unresolved":false,"context_lines":[{"line_number":288,"context_line":"                                                      self[\u0027mount_device\u0027],"},{"line_number":289,"context_line":"                                                      encryption\u003dencryption)"},{"line_number":290,"context_line":"                        except Exception:"},{"line_number":291,"context_line":"                            LOG.warn(_LW(\"Driver failed to detach volume \""},{"line_number":292,"context_line":"                                         \"%(volume_id)s at %(mount_point)s.\"),"},{"line_number":293,"context_line":"                                     {\u0027volume_id\u0027: volume_id,"},{"line_number":294,"context_line":"                                      \u0027mount_point\u0027: self[\u0027mount_device\u0027]},"}],"source_content_type":"text/x-python","patch_set":11,"id":"ba15a1d1_f29d5abc","line":291,"in_reply_to":"da20952f_fb6e9ec6","updated":"2015-09-18 15:14:37.000000000","message":"yes, it is now.","commit_id":"aa899506366cd84d5170cb913f33c0c0ca7a8b27"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"9e5c6348caf86b222a3bf83ff034f615ef415cd6","unresolved":false,"context_lines":[{"line_number":300,"context_line":"                    # Cinder-volume might have completed volume attach. So"},{"line_number":301,"context_line":"                    # we should detach the volume. If the attach did not"},{"line_number":302,"context_line":"                    # happen, the detach request will be ignored."},{"line_number":303,"context_line":"                    volume_api.detach(context, volume_id)"},{"line_number":304,"context_line":""},{"line_number":305,"context_line":"    @update_db"},{"line_number":306,"context_line":"    def refresh_connection_info(self, context, instance,"}],"source_content_type":"text/x-python","patch_set":11,"id":"da20952f_db55dac1","line":303,"updated":"2015-09-07 17:25:12.000000000","message":"I think you are missing the volume delete in the case of delete_on_termination.\n\nNow I have a feeling we have way way too much duplicated code here:\nhttps://github.com/openstack/nova/blob/master/nova/compute/api.py#L1831\n\nCan we share code here? somehow?\n\nWhat about with here too really:\nhttps://github.com/openstack/nova/blob/master/nova/compute/manager.py#L2275\n\nand this bit:\nhttps://github.com/openstack/nova/blob/master/nova/compute/manager.py#L2302","commit_id":"aa899506366cd84d5170cb913f33c0c0ca7a8b27"},{"author":{"_account_id":9171,"name":"Vipin Balachandran","email":"vipin.bl@gmail.com","username":"vbala"},"change_message_id":"4828b72f61ee9b10c834c76fcc703ffa218a2a1e","unresolved":false,"context_lines":[{"line_number":300,"context_line":"                    # Cinder-volume might have completed volume attach. So"},{"line_number":301,"context_line":"                    # we should detach the volume. If the attach did not"},{"line_number":302,"context_line":"                    # happen, the detach request will be ignored."},{"line_number":303,"context_line":"                    volume_api.detach(context, volume_id)"},{"line_number":304,"context_line":""},{"line_number":305,"context_line":"    @update_db"},{"line_number":306,"context_line":"    def refresh_connection_info(self, context, instance,"}],"source_content_type":"text/x-python","patch_set":11,"id":"ba15a1d1_9c5da97b","line":303,"in_reply_to":"ba15a1d1_0d3d538e","updated":"2015-09-22 13:00:19.000000000","message":"\u003eIf volume attach didn\u0027t complete and we call volume detach here, it could cause exceptions. \n\nThere was exception handling in previous patch sets. But the following Cinder patch ensures that there will not be any exception if the attachment is not found during volume detach.\n\nhttps://review.openstack.org/#/c/212193/\n\n\u003eNot sure if every Cinder driver ignores it\n\nThe driver detach happens after the db query to retrieve the attachment. If there is no attachment, we ignore the exception and return without calling the driver detach.","commit_id":"aa899506366cd84d5170cb913f33c0c0ca7a8b27"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fe51e8ad7e273cc4b8e2f9a8b2623f80926b73c3","unresolved":false,"context_lines":[{"line_number":300,"context_line":"                    # Cinder-volume might have completed volume attach. So"},{"line_number":301,"context_line":"                    # we should detach the volume. If the attach did not"},{"line_number":302,"context_line":"                    # happen, the detach request will be ignored."},{"line_number":303,"context_line":"                    volume_api.detach(context, volume_id)"},{"line_number":304,"context_line":""},{"line_number":305,"context_line":"    @update_db"},{"line_number":306,"context_line":"    def refresh_connection_info(self, context, instance,"}],"source_content_type":"text/x-python","patch_set":11,"id":"ba15a1d1_6d05ef43","line":303,"in_reply_to":"ba15a1d1_0d3d538e","updated":"2015-09-18 15:37:52.000000000","message":"I think it\u0027s OK given we\u0027re in a with excutils.save_and_reraise_exception(): block, so we are already going to be raising the original volume_api.attach failure - which is how we got here.","commit_id":"aa899506366cd84d5170cb913f33c0c0ca7a8b27"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"df5358dad6e367eb03adedc4b95d19f39046726e","unresolved":false,"context_lines":[{"line_number":300,"context_line":"                    # Cinder-volume might have completed volume attach. So"},{"line_number":301,"context_line":"                    # we should detach the volume. If the attach did not"},{"line_number":302,"context_line":"                    # happen, the detach request will be ignored."},{"line_number":303,"context_line":"                    volume_api.detach(context, volume_id)"},{"line_number":304,"context_line":""},{"line_number":305,"context_line":"    @update_db"},{"line_number":306,"context_line":"    def refresh_connection_info(self, context, instance,"}],"source_content_type":"text/x-python","patch_set":11,"id":"ba15a1d1_588a2765","line":303,"in_reply_to":"ba15a1d1_6d05ef43","updated":"2015-09-18 15:46:10.000000000","message":"@John - I\u0027ve opened bug 1497343 to track consolidated the duplicate detach code.","commit_id":"aa899506366cd84d5170cb913f33c0c0ca7a8b27"},{"author":{"_account_id":6491,"name":"xing-yang","email":"xingyang105@gmail.com","username":"xing-yang"},"change_message_id":"a26b760d4851df1cd6c2f7fef09c42539692179a","unresolved":false,"context_lines":[{"line_number":300,"context_line":"                    # Cinder-volume might have completed volume attach. So"},{"line_number":301,"context_line":"                    # we should detach the volume. If the attach did not"},{"line_number":302,"context_line":"                    # happen, the detach request will be ignored."},{"line_number":303,"context_line":"                    volume_api.detach(context, volume_id)"},{"line_number":304,"context_line":""},{"line_number":305,"context_line":"    @update_db"},{"line_number":306,"context_line":"    def refresh_connection_info(self, context, instance,"}],"source_content_type":"text/x-python","patch_set":11,"id":"ba15a1d1_0d3d538e","line":303,"in_reply_to":"ba15a1d1_87614207","updated":"2015-09-18 15:31:10.000000000","message":"I think we should catch any exceptions from volume_api.detach and pass.  If volume attach didn\u0027t complete and we call volume detach here, it could cause exceptions.  Not sure if every Cinder driver ignores it if we try to detach a volume that is not attached.","commit_id":"aa899506366cd84d5170cb913f33c0c0ca7a8b27"},{"author":{"_account_id":6491,"name":"xing-yang","email":"xingyang105@gmail.com","username":"xing-yang"},"change_message_id":"c3eca6e50b9123ec7c41cabd97c8b2ce58c36f0f","unresolved":false,"context_lines":[{"line_number":300,"context_line":"                    # Cinder-volume might have completed volume attach. So"},{"line_number":301,"context_line":"                    # we should detach the volume. If the attach did not"},{"line_number":302,"context_line":"                    # happen, the detach request will be ignored."},{"line_number":303,"context_line":"                    volume_api.detach(context, volume_id)"},{"line_number":304,"context_line":""},{"line_number":305,"context_line":"    @update_db"},{"line_number":306,"context_line":"    def refresh_connection_info(self, context, instance,"}],"source_content_type":"text/x-python","patch_set":11,"id":"ba15a1d1_e5852859","line":303,"in_reply_to":"ba15a1d1_9c5da97b","updated":"2015-09-22 13:54:39.000000000","message":"Thanks Vipin.  I\u0027m more thinking about when db and array are out of sync.  So it says attached in db, but it is not the case on the array.  self.driver.detach_volume(context, volume, attachment) could still throw exception.  I did run into the situation when db and array are out of sync when initialize_connection times out, but I haven\u0027t seen that in this particular code path that you are covering.  This is fine as the original exception will be raised any way.","commit_id":"aa899506366cd84d5170cb913f33c0c0ca7a8b27"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"7aca8a03af86e7640b048aa565f77a0b76cb081f","unresolved":false,"context_lines":[{"line_number":300,"context_line":"                    # Cinder-volume might have completed volume attach. So"},{"line_number":301,"context_line":"                    # we should detach the volume. If the attach did not"},{"line_number":302,"context_line":"                    # happen, the detach request will be ignored."},{"line_number":303,"context_line":"                    volume_api.detach(context, volume_id)"},{"line_number":304,"context_line":""},{"line_number":305,"context_line":"    @update_db"},{"line_number":306,"context_line":"    def refresh_connection_info(self, context, instance,"}],"source_content_type":"text/x-python","patch_set":11,"id":"ba15a1d1_87614207","line":303,"in_reply_to":"da20952f_db55dac1","updated":"2015-09-18 15:03:44.000000000","message":"I don\u0027t think we\u0027re supposed to delete the volume, we only do that if we created it, which is only in the image/snapshot/blank cases, see how _call_wait_func is used below.\n\nI agree that the code in _shutdown_instance in nova.compute.manager could probably be moved into this module, but at this point in the release I\u0027m not sure we want to do that now - we could refactor and consolidate that code in a follow up change.  That kind of refactor in the bug fix also makes this harder to backport (thinking ahead to mitaka if this doesn\u0027t land in liberty, we\u0027d want to backport to stable/liberty).","commit_id":"aa899506366cd84d5170cb913f33c0c0ca7a8b27"}]}