)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"f69ff8a85ac6b1e10563e31ff40440a430243eb5","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Ildiko Vancsa \u003cildiko.vancsa@ericsson.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2016-07-20 17:55:49 +0200"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"WIP Don\u0027t check cinder volume states during attach"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This patch changes how Nova interacts with volumes at attach time."},{"line_number":10,"context_line":"Nova should rely upon Cinder\u0027s os-reserve to determine if the"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"dada55a8_44b8ffe5","line":7,"range":{"start_line":7,"start_character":0,"end_line":7,"end_character":3},"updated":"2016-07-22 19:58:04.000000000","message":"When can we drop this? Could we do the TODO in a later change?\n\nWe agreed at the nova midcycle to get this in for newton as a bug fix.","commit_id":"5415d621c000630514c0bdb1008dcf9c97740599"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"f69ff8a85ac6b1e10563e31ff40440a430243eb5","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This patch changes how Nova interacts with volumes at attach time."},{"line_number":10,"context_line":"Nova should rely upon Cinder\u0027s os-reserve to determine if the"},{"line_number":11,"context_line":"state of the volume is in a good/valid state for attching."},{"line_number":12,"context_line":"This fixes a race between when nova fetches the volume and calls"},{"line_number":13,"context_line":"os-reserve."},{"line_number":14,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"dada55a8_c4c4ef60","line":11,"range":{"start_line":11,"start_character":49,"end_line":11,"end_character":57},"updated":"2016-07-22 19:58:04.000000000","message":"attaching","commit_id":"5415d621c000630514c0bdb1008dcf9c97740599"}],"nova/tests/unit/volume/test_cinder.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"f69ff8a85ac6b1e10563e31ff40440a430243eb5","unresolved":false,"context_lines":[{"line_number":154,"context_line":"                               instance: \u0027zone1\u0027) as mock_get_instance_az:"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"            CONF.set_override(\u0027cross_az_attach\u0027, False, group\u003d\u0027cinder\u0027)"},{"line_number":157,"context_line":"            volume[\u0027availability_zone\u0027] \u003d \u0027zone1\u0027"},{"line_number":158,"context_line":"            self.assertIsNone(self.api.check_availability_zone("},{"line_number":159,"context_line":"                self.ctx, volume, instance))"},{"line_number":160,"context_line":"            mock_get_instance_az.assert_called_once_with(self.ctx, instance)"},{"line_number":161,"context_line":"            mock_get_instance_az.reset_mock()"},{"line_number":162,"context_line":"            volume[\u0027availability_zone\u0027] \u003d \u0027zone2\u0027"},{"line_number":163,"context_line":"            self.assertRaises(exception.InvalidVolume,"},{"line_number":164,"context_line":"                              self.api.check_availability_zone,"}],"source_content_type":"text/x-python","patch_set":7,"id":"dada55a8_44a3df4f","line":161,"range":{"start_line":157,"start_character":12,"end_line":161,"end_character":45},"updated":"2016-07-22 19:58:04.000000000","message":"This is all existing test code, but it doesn\u0027t make sense given this is a negative test. The happy path is tested in test_check_attach right below this, so we could probably just clean this up (or I can do it separately and we can rebase this change on top).","commit_id":"5415d621c000630514c0bdb1008dcf9c97740599"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"f69ff8a85ac6b1e10563e31ff40440a430243eb5","unresolved":false,"context_lines":[{"line_number":165,"context_line":"                              self.ctx, volume, instance)"},{"line_number":166,"context_line":"            mock_get_instance_az.assert_called_once_with(self.ctx, instance)"},{"line_number":167,"context_line":"            mock_get_instance_az.reset_mock()"},{"line_number":168,"context_line":"            del instance[\u0027host\u0027]"},{"line_number":169,"context_line":"            volume[\u0027availability_zone\u0027] \u003d \u0027zone1\u0027"},{"line_number":170,"context_line":"            self.assertIsNone(self.api.check_availability_zone("},{"line_number":171,"context_line":"                self.ctx, volume, instance))"}],"source_content_type":"text/x-python","patch_set":7,"id":"dada55a8_a401fb3b","line":168,"range":{"start_line":168,"start_character":12,"end_line":168,"end_character":32},"updated":"2016-07-22 19:58:04.000000000","message":"I don\u0027t know what this has to do with anything, the check_availability_zone code doesn\u0027t check the instance host. This is probably legacy but it seems pointless.\n\n--\n\nOK, az.get_instance_availabilty_zone checks instance.host, but we mock that out so there isn\u0027t a point in having this in here anymore.","commit_id":"5415d621c000630514c0bdb1008dcf9c97740599"}],"nova/volume/cinder.py":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"5487313a85f5813202acf54e78a96035dcefd1d6","unresolved":false,"context_lines":[{"line_number":276,"context_line":"                                              \"status\": volume[\u0027status\u0027]}"},{"line_number":277,"context_line":"            raise exception.InvalidVolume(reason\u003dmsg)"},{"line_number":278,"context_line":""},{"line_number":279,"context_line":"    def check_attach(self, context, volume, instance\u003dNone):"},{"line_number":280,"context_line":"        # TODO(vish): abstract status checking?"},{"line_number":281,"context_line":"        if volume[\u0027status\u0027] !\u003d \"available\":"},{"line_number":282,"context_line":"            msg \u003d _(\"volume \u0027%(vol)s\u0027 status must be \u0027available\u0027. Currently \""}],"source_content_type":"text/x-python","patch_set":1,"id":"dab17558_a8c10250","line":279,"updated":"2016-05-17 17:45:31.000000000","message":"do we still need this for some case?","commit_id":"b1b4e135b91f66bab80cec438c1f940a0928346d"},{"author":{"_account_id":1865,"name":"Andrea Rosa","email":"andrea.rosa@gmail.com","username":"andrea-rosa-m"},"change_message_id":"15d9c08e4a8deeecf65e7bd6c371f92d10ebe002","unresolved":false,"context_lines":[{"line_number":277,"context_line":"            raise exception.InvalidVolume(reason\u003dmsg)"},{"line_number":278,"context_line":""},{"line_number":279,"context_line":"    def check_attach(self, context, volume, instance\u003dNone):"},{"line_number":280,"context_line":"        # TODO(vish): abstract status checking?"},{"line_number":281,"context_line":"        if volume[\u0027status\u0027] !\u003d \"available\":"},{"line_number":282,"context_line":"            msg \u003d _(\"volume \u0027%(vol)s\u0027 status must be \u0027available\u0027. Currently \""},{"line_number":283,"context_line":"                    \"in \u0027%(status)s\u0027\") % {\u0027vol\u0027: volume[\u0027id\u0027],"},{"line_number":284,"context_line":"                                          \u0027status\u0027: volume[\u0027status\u0027]}"},{"line_number":285,"context_line":"            raise exception.InvalidVolume(reason\u003dmsg)"},{"line_number":286,"context_line":"        if volume[\u0027attach_status\u0027] \u003d\u003d \"attached\":"},{"line_number":287,"context_line":"            msg \u003d _(\"volume %s already attached\") % volume[\u0027id\u0027]"},{"line_number":288,"context_line":"            raise exception.InvalidVolume(reason\u003dmsg)"},{"line_number":289,"context_line":""},{"line_number":290,"context_line":"        self.check_availability_zone(context, volume, instance)"},{"line_number":291,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"7aa08908_19a670c1","line":288,"range":{"start_line":280,"start_character":0,"end_line":288,"end_character":53},"updated":"2016-06-06 10:28:46.000000000","message":"I guess those checks will be removed, if I am right then I expect that the cinder reserve_volume will return an InvalidVolume exception if the volume is not in the right status for the reserve. If we do that then we have to add the managing of the new exception in the translate_volume_exception here:\nhttps://github.com/openstack/nova/blob/026468772672215d34a593e631d1e62d6a615aa4/nova/volume/cinder.py#L205","commit_id":"f5a6c7a85d70837ede1ff02614ac99d3d59ce5f3"},{"author":{"_account_id":9562,"name":"Ildiko Vancsa","email":"ildiko.vancsa@gmail.com","username":"ildikov"},"change_message_id":"58f677a87ac54f8e8b6aa39b80a68da4b60cb703","unresolved":false,"context_lines":[{"line_number":277,"context_line":"            raise exception.InvalidVolume(reason\u003dmsg)"},{"line_number":278,"context_line":""},{"line_number":279,"context_line":"    def check_attach(self, context, volume, instance\u003dNone):"},{"line_number":280,"context_line":"        # TODO(vish): abstract status checking?"},{"line_number":281,"context_line":"        if volume[\u0027status\u0027] !\u003d \"available\":"},{"line_number":282,"context_line":"            msg \u003d _(\"volume \u0027%(vol)s\u0027 status must be \u0027available\u0027. Currently \""},{"line_number":283,"context_line":"                    \"in \u0027%(status)s\u0027\") % {\u0027vol\u0027: volume[\u0027id\u0027],"},{"line_number":284,"context_line":"                                          \u0027status\u0027: volume[\u0027status\u0027]}"},{"line_number":285,"context_line":"            raise exception.InvalidVolume(reason\u003dmsg)"},{"line_number":286,"context_line":"        if volume[\u0027attach_status\u0027] \u003d\u003d \"attached\":"},{"line_number":287,"context_line":"            msg \u003d _(\"volume %s already attached\") % volume[\u0027id\u0027]"},{"line_number":288,"context_line":"            raise exception.InvalidVolume(reason\u003dmsg)"},{"line_number":289,"context_line":""},{"line_number":290,"context_line":"        self.check_availability_zone(context, volume, instance)"},{"line_number":291,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"3aaa91ec_30b3b20b","line":288,"range":{"start_line":280,"start_character":0,"end_line":288,"end_character":53},"in_reply_to":"7aa08908_19a670c1","updated":"2016-06-27 07:29:48.000000000","message":"You are right, it returns an InvalidVolume exception here: https://github.com/openstack/cinder/blob/master/cinder/volume/api.py#L562-L577\n\nIt needs to be managed as you pointed out, we might end up with different error message text, but that should be acceptable.","commit_id":"f5a6c7a85d70837ede1ff02614ac99d3d59ce5f3"}]}