)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"f3320a98cef4a1a36ee2f0f58241cf7734529c1f","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Kevin_Zheng \u003czhengzhenyu@huawei.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2018-12-21 16:46:35 +0800"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Don\u0027t call begin_detaching when detaching volume from sheved vm"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"When shelve an instance, if the instance has volume attached,"},{"line_number":10,"context_line":"with new attach/detach flow, we will delete the old attachment"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"3f79a3b5_0c9744eb","line":7,"range":{"start_line":7,"start_character":54,"end_line":7,"end_character":60},"updated":"2018-12-21 13:58:08.000000000","message":"shelved","commit_id":"1c9d745cb9aed1565e45002ff2a375bda9fa9417"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"f3320a98cef4a1a36ee2f0f58241cf7734529c1f","unresolved":false,"context_lines":[{"line_number":9,"context_line":"When shelve an instance, if the instance has volume attached,"},{"line_number":10,"context_line":"with new attach/detach flow, we will delete the old attachment"},{"line_number":11,"context_line":"and create a new attachment, the volume status will be ``reserved``"},{"line_number":12,"context_line":"in Cinder side, it will cause error if we call begin_detaching()"},{"line_number":13,"context_line":"as it will check whether the status is ``in-use``."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Acutally for shelved instances, we can just skip this step and"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"3f79a3b5_ac8170a3","line":12,"range":{"start_line":12,"start_character":14,"end_line":12,"end_character":18},"updated":"2018-12-21 13:58:08.000000000","message":". It","commit_id":"1c9d745cb9aed1565e45002ff2a375bda9fa9417"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"f3320a98cef4a1a36ee2f0f58241cf7734529c1f","unresolved":false,"context_lines":[{"line_number":9,"context_line":"When shelve an instance, if the instance has volume attached,"},{"line_number":10,"context_line":"with new attach/detach flow, we will delete the old attachment"},{"line_number":11,"context_line":"and create a new attachment, the volume status will be ``reserved``"},{"line_number":12,"context_line":"in Cinder side, it will cause error if we call begin_detaching()"},{"line_number":13,"context_line":"as it will check whether the status is ``in-use``."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Acutally for shelved instances, we can just skip this step and"},{"line_number":16,"context_line":"directly detach it."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"3f79a3b5_8c8274a2","line":13,"range":{"start_line":12,"start_character":19,"end_line":13,"end_character":50},"updated":"2018-12-21 13:58:08.000000000","message":"This is a bit confusing being munged into the first part about how we get the volume to reserved status. You\u0027re making it sound like begin_detaching is part of the attach process, which it\u0027s not. The first part is good - how the attached volume gets to \u0027reserved\u0027 status. What is missing is explaining that the failure is if the user tries to detach that volume, it fails because cinder does not allow a begin_detaching() call on a \u0027reserved\u0027 volume, only an \u0027in-use\u0027 volume.","commit_id":"1c9d745cb9aed1565e45002ff2a375bda9fa9417"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e1f40d3a78d0f83cf7b2449c1c13ab70b7939d59","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Zhenyu Zheng \u003czhengzhenyu@huawei.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2019-01-21 02:01:01 +0000"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Don\u0027t call begin_detaching when detaching volume from sheveld vm"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"When shelve an instance, if the instance has volume attached,"},{"line_number":10,"context_line":"with new attach/detach flow, we will delete the old attachment"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"9fdfeff1_e98286c8","line":7,"range":{"start_line":7,"start_character":54,"end_line":7,"end_character":61},"updated":"2019-01-22 22:16:47.000000000","message":"shelved","commit_id":"68d9f4baf745d0b54d80e6e750212a1e1c71e882"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e1f40d3a78d0f83cf7b2449c1c13ab70b7939d59","unresolved":false,"context_lines":[{"line_number":11,"context_line":"and create a new attachment, the volume status will be ``reserved``."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"If the user tries to detach these volumes, it fails due to that"},{"line_number":14,"context_line":"Cinder does not allow a begin_detaching() call on a `reserved` volume."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Acutally for shelved instances, we can just skip this step and"},{"line_number":17,"context_line":"directly detach it."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"9fdfeff1_2962ce3f","line":14,"updated":"2019-01-22 22:16:47.000000000","message":"Yup: https://github.com/openstack/cinder/blob/ee9cf8696794824a7b39cd5d78a978fe31190db2/cinder/volume/api.py#L725","commit_id":"68d9f4baf745d0b54d80e6e750212a1e1c71e882"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e1f40d3a78d0f83cf7b2449c1c13ab70b7939d59","unresolved":false,"context_lines":[{"line_number":13,"context_line":"If the user tries to detach these volumes, it fails due to that"},{"line_number":14,"context_line":"Cinder does not allow a begin_detaching() call on a `reserved` volume."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Acutally for shelved instances, we can just skip this step and"},{"line_number":17,"context_line":"directly detach it."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"Change-Id: Ib1799feebbd8f4b0f389168939df7e5e90c8add1"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"9fdfeff1_4967124f","line":16,"range":{"start_line":16,"start_character":0,"end_line":16,"end_character":8},"updated":"2019-01-22 22:16:47.000000000","message":"Actually","commit_id":"68d9f4baf745d0b54d80e6e750212a1e1c71e882"}],"nova/compute/api.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"f3320a98cef4a1a36ee2f0f58241cf7734529c1f","unresolved":false,"context_lines":[{"line_number":4295,"context_line":""},{"line_number":4296,"context_line":"        bdms \u003d [objects.BlockDeviceMapping.get_by_volume_id("},{"line_number":4297,"context_line":"            context, volume[\u0027id\u0027], instance.uuid)]"},{"line_number":4298,"context_line":"        if not bdms[0].attachment_id:"},{"line_number":4299,"context_line":"            try:"},{"line_number":4300,"context_line":"                self.volume_api.begin_detaching(context, volume[\u0027id\u0027])"},{"line_number":4301,"context_line":"            except exception.InvalidInput as exc:"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_ec61a810","line":4298,"updated":"2018-12-21 13:58:08.000000000","message":"A comment here would be good, i.e. \"The begin_detaching() call only works with in-use volumes, which will not be the case for volumes attached to a shelved offloaded server since those volumes will have \u0027reserved\u0027 status.\"","commit_id":"1c9d745cb9aed1565e45002ff2a375bda9fa9417"},{"author":{"_account_id":15888,"name":"Zhenyu Zheng","email":"zheng.zhenyu@outlook.com","username":"Kevin_Zheng"},"change_message_id":"69cfc35c48482e5f915fc0f450e9b60cbe054e1a","unresolved":false,"context_lines":[{"line_number":4295,"context_line":""},{"line_number":4296,"context_line":"        bdms \u003d [objects.BlockDeviceMapping.get_by_volume_id("},{"line_number":4297,"context_line":"            context, volume[\u0027id\u0027], instance.uuid)]"},{"line_number":4298,"context_line":"        if not bdms[0].attachment_id:"},{"line_number":4299,"context_line":"            try:"},{"line_number":4300,"context_line":"                self.volume_api.begin_detaching(context, volume[\u0027id\u0027])"},{"line_number":4301,"context_line":"            except exception.InvalidInput as exc:"}],"source_content_type":"text/x-python","patch_set":2,"id":"1f769fc5_5c2ccc46","line":4298,"in_reply_to":"3f79a3b5_ec61a810","updated":"2018-12-24 02:57:09.000000000","message":"Done","commit_id":"1c9d745cb9aed1565e45002ff2a375bda9fa9417"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e1f40d3a78d0f83cf7b2449c1c13ab70b7939d59","unresolved":false,"context_lines":[{"line_number":4287,"context_line":"        except exception.InvalidInput as exc:"},{"line_number":4288,"context_line":"            raise exception.InvalidVolume(reason\u003dexc.format_message())"},{"line_number":4289,"context_line":"        bdms \u003d [objects.BlockDeviceMapping.get_by_volume_id("},{"line_number":4290,"context_line":"                context, volume[\u0027id\u0027], instance.uuid)]"},{"line_number":4291,"context_line":"        self._record_action_start("},{"line_number":4292,"context_line":"            context, instance,"},{"line_number":4293,"context_line":"            instance_actions.DETACH_VOLUME)"}],"source_content_type":"text/x-python","patch_set":4,"id":"9fdfeff1_694f76bd","side":"PARENT","line":4290,"range":{"start_line":4290,"start_character":12,"end_line":4290,"end_character":16},"updated":"2019-01-22 22:16:47.000000000","message":"This indent changed.","commit_id":"99ca7d60b41f63446f86ed9be86440a2b250f330"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"de1bb707c02cebaafbfa1dabcbf0f8cee8201a6b","unresolved":false,"context_lines":[{"line_number":4287,"context_line":"        # The begin_detaching() call only works with in-use volumes,"},{"line_number":4288,"context_line":"        # which will not be the case for volumes attached to a shelved"},{"line_number":4289,"context_line":"        # offloaded server since those volumes will have `reserved` status."},{"line_number":4290,"context_line":"        if not bdms[0].attachment_id:"},{"line_number":4291,"context_line":"            try:"},{"line_number":4292,"context_line":"                self.volume_api.begin_detaching(context, volume[\u0027id\u0027])"},{"line_number":4293,"context_line":"            except exception.InvalidInput as exc:"}],"source_content_type":"text/x-python","patch_set":4,"id":"9fdfeff1_b635dcf9","line":4290,"range":{"start_line":4290,"start_character":15,"end_line":4290,"end_character":19},"updated":"2019-01-21 13:33:41.000000000","message":"what if this is []?\nwhat if there is more than block device?","commit_id":"68d9f4baf745d0b54d80e6e750212a1e1c71e882"},{"author":{"_account_id":15888,"name":"Zhenyu Zheng","email":"zheng.zhenyu@outlook.com","username":"Kevin_Zheng"},"change_message_id":"ddc9a89eef88a868cce91dd8294be05750240c52","unresolved":false,"context_lines":[{"line_number":4287,"context_line":"        # The begin_detaching() call only works with in-use volumes,"},{"line_number":4288,"context_line":"        # which will not be the case for volumes attached to a shelved"},{"line_number":4289,"context_line":"        # offloaded server since those volumes will have `reserved` status."},{"line_number":4290,"context_line":"        if not bdms[0].attachment_id:"},{"line_number":4291,"context_line":"            try:"},{"line_number":4292,"context_line":"                self.volume_api.begin_detaching(context, volume[\u0027id\u0027])"},{"line_number":4293,"context_line":"            except exception.InvalidInput as exc:"}],"source_content_type":"text/x-python","patch_set":4,"id":"9fdfeff1_9491a655","line":4290,"range":{"start_line":4290,"start_character":15,"end_line":4290,"end_character":19},"in_reply_to":"9fdfeff1_4eb5ef30","updated":"2019-01-22 01:34:30.000000000","message":"Thanks for clarify.\n@garyk, FYI, https://github.com/openstack/nova/blob/stable/rocky/nova/objects/block_device.py#L289","commit_id":"68d9f4baf745d0b54d80e6e750212a1e1c71e882"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"331e3d0bee86d6ce10a4bd6c08f456942e732a6b","unresolved":false,"context_lines":[{"line_number":4287,"context_line":"        # The begin_detaching() call only works with in-use volumes,"},{"line_number":4288,"context_line":"        # which will not be the case for volumes attached to a shelved"},{"line_number":4289,"context_line":"        # offloaded server since those volumes will have `reserved` status."},{"line_number":4290,"context_line":"        if not bdms[0].attachment_id:"},{"line_number":4291,"context_line":"            try:"},{"line_number":4292,"context_line":"                self.volume_api.begin_detaching(context, volume[\u0027id\u0027])"},{"line_number":4293,"context_line":"            except exception.InvalidInput as exc:"}],"source_content_type":"text/x-python","patch_set":4,"id":"9fdfeff1_4eb5ef30","line":4290,"range":{"start_line":4290,"start_character":15,"end_line":4290,"end_character":19},"in_reply_to":"9fdfeff1_b635dcf9","updated":"2019-01-21 21:58:55.000000000","message":"Neither of these would happen, BlockDeviceMapping.get_by_volume_id will either raise an exception if none are found or it will return exactly 1 BDM.","commit_id":"68d9f4baf745d0b54d80e6e750212a1e1c71e882"}],"nova/tests/unit/compute/test_compute.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e1f40d3a78d0f83cf7b2449c1c13ab70b7939d59","unresolved":false,"context_lines":[{"line_number":10773,"context_line":"                                           instance.uuid)"},{"line_number":10774,"context_line":"        self.assertTrue(mock_local_cleanup.called)"},{"line_number":10775,"context_line":""},{"line_number":10776,"context_line":"    @mock.patch.object(nova.volume.cinder.API, \u0027begin_detaching\u0027)"},{"line_number":10777,"context_line":"    @mock.patch.object(compute_api.API, \u0027_local_cleanup_bdm_volumes\u0027)"},{"line_number":10778,"context_line":"    @mock.patch.object(objects.BlockDeviceMapping, \u0027get_by_volume_id\u0027)"},{"line_number":10779,"context_line":"    def test_detach_volume_shelved_offloaded_new_flow(self,"}],"source_content_type":"text/x-python","patch_set":4,"id":"9fdfeff1_49f5d256","side":"PARENT","line":10776,"updated":"2019-01-22 22:16:47.000000000","message":"Could have left this in and asserted it\u0027s not called.","commit_id":"99ca7d60b41f63446f86ed9be86440a2b250f330"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"6504f129d29d74ac87282919cc526c7d73ceb3c1","unresolved":false,"context_lines":[{"line_number":10763,"context_line":"        self.compute_api._detach_volume_shelved_offloaded(self.context,"},{"line_number":10764,"context_line":"                                                          instance,"},{"line_number":10765,"context_line":"                                                          volume)"},{"line_number":10766,"context_line":"        mock_begin_detaching.assert_called_once_with(self.context,"},{"line_number":10767,"context_line":"                                                     volume[\u0027id\u0027])"},{"line_number":10768,"context_line":"        mock_record.assert_called_once_with("},{"line_number":10769,"context_line":"            self.context, instance, instance_actions.DETACH_VOLUME)"}],"source_content_type":"text/x-python","patch_set":5,"id":"9fdfeff1_fa8746b4","line":10766,"updated":"2019-01-28 19:53:12.000000000","message":"This would be covering the non-shelved case, so with the below assertion of not being called in the shelved case we should be covered.","commit_id":"41b982c9feec3105247bc72d23d1470bcabc3a0f"}]}