)]}'
{"cinder/tests/unit/test_volume.py":[{"author":{"_account_id":7173,"name":"Scott DAngelo","email":"scott.dangelo@gmail.com","username":"scottda"},"change_message_id":"51b0f4f2fa5924a66ff81d4aec968eec79a122f7","unresolved":false,"context_lines":[{"line_number":3872,"context_line":"                          volume,"},{"line_number":3873,"context_line":"                          3)"},{"line_number":3874,"context_line":""},{"line_number":3875,"context_line":"        db.volume_update(self.context, volume[\u0027id\u0027], {\u0027status\u0027: \u0027available\u0027})"},{"line_number":3876,"context_line":"        # Extend fails when new_size \u003c orig_size"},{"line_number":3877,"context_line":"        self.assertRaises(exception.InvalidInput,"},{"line_number":3878,"context_line":"                          volume_api.extend,"}],"source_content_type":"text/x-python","patch_set":36,"id":"5a18252c_2efdd73a","line":3875,"updated":"2016-04-12 15:12:29.000000000","message":"I think you can use volume.id if you do another patch.","commit_id":"226dcb3881717564191111648a9e0acea455979b"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"ffea4b810c6fbe3e09e1069388e7e0f46760c58a","unresolved":false,"context_lines":[{"line_number":3872,"context_line":"                          volume,"},{"line_number":3873,"context_line":"                          3)"},{"line_number":3874,"context_line":""},{"line_number":3875,"context_line":"        db.volume_update(self.context, volume[\u0027id\u0027], {\u0027status\u0027: \u0027available\u0027})"},{"line_number":3876,"context_line":"        # Extend fails when new_size \u003c orig_size"},{"line_number":3877,"context_line":"        self.assertRaises(exception.InvalidInput,"},{"line_number":3878,"context_line":"                          volume_api.extend,"}],"source_content_type":"text/x-python","patch_set":36,"id":"5a18252c_26945b41","line":3875,"in_reply_to":"5a18252c_2efdd73a","updated":"2016-04-14 16:06:46.000000000","message":"Done","commit_id":"226dcb3881717564191111648a9e0acea455979b"},{"author":{"_account_id":7173,"name":"Scott DAngelo","email":"scott.dangelo@gmail.com","username":"scottda"},"change_message_id":"51b0f4f2fa5924a66ff81d4aec968eec79a122f7","unresolved":false,"context_lines":[{"line_number":3996,"context_line":"        except exception.QuotaUsageNotFound:"},{"line_number":3997,"context_line":"            volumes_in_use \u003d 0"},{"line_number":3998,"context_line":"        self.assertEqual(100, volumes_in_use)"},{"line_number":3999,"context_line":"        db.volume_update(self.context, volume[\u0027id\u0027], {\u0027status\u0027: \u0027available\u0027})"},{"line_number":4000,"context_line":""},{"line_number":4001,"context_line":"        volume_api.extend(self.context, volume, 200)"},{"line_number":4002,"context_line":"        mock_rpc_extend.called_once_with(self.context, volume, 200, mock.ANY)"}],"source_content_type":"text/x-python","patch_set":36,"id":"5a18252c_6e634f1b","line":3999,"updated":"2016-04-12 15:12:29.000000000","message":"volume.id","commit_id":"226dcb3881717564191111648a9e0acea455979b"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"ffea4b810c6fbe3e09e1069388e7e0f46760c58a","unresolved":false,"context_lines":[{"line_number":3996,"context_line":"        except exception.QuotaUsageNotFound:"},{"line_number":3997,"context_line":"            volumes_in_use \u003d 0"},{"line_number":3998,"context_line":"        self.assertEqual(100, volumes_in_use)"},{"line_number":3999,"context_line":"        db.volume_update(self.context, volume[\u0027id\u0027], {\u0027status\u0027: \u0027available\u0027})"},{"line_number":4000,"context_line":""},{"line_number":4001,"context_line":"        volume_api.extend(self.context, volume, 200)"},{"line_number":4002,"context_line":"        mock_rpc_extend.called_once_with(self.context, volume, 200, mock.ANY)"}],"source_content_type":"text/x-python","patch_set":36,"id":"5a18252c_869d6f68","line":3999,"in_reply_to":"5a18252c_6e634f1b","updated":"2016-04-14 16:06:46.000000000","message":"Done","commit_id":"226dcb3881717564191111648a9e0acea455979b"}],"cinder/volume/api.py":[{"author":{"_account_id":9171,"name":"Vipin Balachandran","email":"vipin.bl@gmail.com","username":"vbala"},"change_message_id":"f3c02d22c7e8766fcd6e8f4dd2993f63859aef89","unresolved":false,"context_lines":[{"line_number":1301,"context_line":"            \u0027id\u0027: volume[\u0027id\u0027],"},{"line_number":1302,"context_line":"        }"},{"line_number":1303,"context_line":""},{"line_number":1304,"context_line":"        # Loop until we make a decission on whether to go forward with the"},{"line_number":1305,"context_line":"        # deletion or raise and error"},{"line_number":1306,"context_line":"        # TODO(geguileo): Once we are using versioned objects we should change"},{"line_number":1307,"context_line":"        # this to use volume.conditional_update and volume.model"}],"source_content_type":"text/x-python","patch_set":11,"id":"da20952f_7af20c08","line":1304,"updated":"2015-09-10 13:06:13.000000000","message":"decision","commit_id":"ebfef1181fb60a7699319fda4cf61e22ae61035b"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"27a20908d07bdee47a0f96298af078a0f1af03da","unresolved":false,"context_lines":[{"line_number":1301,"context_line":"            \u0027id\u0027: volume[\u0027id\u0027],"},{"line_number":1302,"context_line":"        }"},{"line_number":1303,"context_line":""},{"line_number":1304,"context_line":"        # Loop until we make a decission on whether to go forward with the"},{"line_number":1305,"context_line":"        # deletion or raise and error"},{"line_number":1306,"context_line":"        # TODO(geguileo): Once we are using versioned objects we should change"},{"line_number":1307,"context_line":"        # this to use volume.conditional_update and volume.model"}],"source_content_type":"text/x-python","patch_set":11,"id":"da20952f_0d72aae3","line":1304,"in_reply_to":"da20952f_7af20c08","updated":"2015-09-10 19:35:31.000000000","message":"done","commit_id":"ebfef1181fb60a7699319fda4cf61e22ae61035b"},{"author":{"_account_id":9171,"name":"Vipin Balachandran","email":"vipin.bl@gmail.com","username":"vbala"},"change_message_id":"f3c02d22c7e8766fcd6e8f4dd2993f63859aef89","unresolved":false,"context_lines":[{"line_number":1318,"context_line":"            # NOTE(geguileo): If we haven\u0027t raised an error even though we"},{"line_number":1319,"context_line":"            # couldn\u0027t update the DB it means that between the conditional"},{"line_number":1320,"context_line":"            # update and the error checking to raise the right exception DB"},{"line_number":1321,"context_line":"            # has changed and now the conditional update will work, so we loop"},{"line_number":1322,"context_line":"            # again to update it."},{"line_number":1323,"context_line":""},{"line_number":1324,"context_line":"        # NOTE(geguileo): To mimic current behavior we will rollback status"}],"source_content_type":"text/x-python","patch_set":11,"id":"da20952f_1aec30f2","line":1321,"updated":"2015-09-10 13:06:13.000000000","message":"No guarantee that it *will* work. It will if there is no other thread.","commit_id":"ebfef1181fb60a7699319fda4cf61e22ae61035b"},{"author":{"_account_id":9171,"name":"Vipin Balachandran","email":"vipin.bl@gmail.com","username":"vbala"},"change_message_id":"23aea78e4e1f71e59c6047e6dbdbf6fdada43e51","unresolved":false,"context_lines":[{"line_number":1318,"context_line":"            # NOTE(geguileo): If we haven\u0027t raised an error even though we"},{"line_number":1319,"context_line":"            # couldn\u0027t update the DB it means that between the conditional"},{"line_number":1320,"context_line":"            # update and the error checking to raise the right exception DB"},{"line_number":1321,"context_line":"            # has changed and now the conditional update will work, so we loop"},{"line_number":1322,"context_line":"            # again to update it."},{"line_number":1323,"context_line":""},{"line_number":1324,"context_line":"        # NOTE(geguileo): To mimic current behavior we will rollback status"}],"source_content_type":"text/x-python","patch_set":11,"id":"da20952f_172387be","line":1321,"in_reply_to":"da20952f_0d4f0abf","updated":"2015-09-11 08:10:47.000000000","message":"Ok agree, but what is the probability that _check_volume_availability will not raise an exception? I think it is also less than the probability of you winning the lottery :D\n\nSo why can\u0027t we simply raise InvalidVolume here instead of calling _check_volume_availability. Assume that _check_volume_availability raises an exception and I can argue that we should catch that and try once more :)\n\nI can\u0027t see the value in checking the status again or I\u0027m missing something.\nBut I\u0027m not going to downvote because of this optimism :)","commit_id":"ebfef1181fb60a7699319fda4cf61e22ae61035b"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"722f264fc11d002e937149d9afdc2969ef5ae986","unresolved":false,"context_lines":[{"line_number":1318,"context_line":"            # NOTE(geguileo): If we haven\u0027t raised an error even though we"},{"line_number":1319,"context_line":"            # couldn\u0027t update the DB it means that between the conditional"},{"line_number":1320,"context_line":"            # update and the error checking to raise the right exception DB"},{"line_number":1321,"context_line":"            # has changed and now the conditional update will work, so we loop"},{"line_number":1322,"context_line":"            # again to update it."},{"line_number":1323,"context_line":""},{"line_number":1324,"context_line":"        # NOTE(geguileo): To mimic current behavior we will rollback status"}],"source_content_type":"text/x-python","patch_set":11,"id":"da20952f_1a408ba4","line":1321,"in_reply_to":"da20952f_172387be","updated":"2015-09-11 09:00:35.000000000","message":"Yep, that\u0027s also unlikely but it can happen and I want to be sure we have it covered.\n\nThe reason to call _check_volume_availability is to have the right error msg instead of a generic one.\n\nIf we didn\u0027t have to raise good errors, all the API races code in all the patches would be so much easier, it would be an if on the conditional update and if it didn\u0027t change in the DB I would just raise and exception and be done with it.  :-(\n\nPS: We agree, I\u0027m not winning the lottery and getting rich.  ;-)","commit_id":"ebfef1181fb60a7699319fda4cf61e22ae61035b"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"27a20908d07bdee47a0f96298af078a0f1af03da","unresolved":false,"context_lines":[{"line_number":1318,"context_line":"            # NOTE(geguileo): If we haven\u0027t raised an error even though we"},{"line_number":1319,"context_line":"            # couldn\u0027t update the DB it means that between the conditional"},{"line_number":1320,"context_line":"            # update and the error checking to raise the right exception DB"},{"line_number":1321,"context_line":"            # has changed and now the conditional update will work, so we loop"},{"line_number":1322,"context_line":"            # again to update it."},{"line_number":1323,"context_line":""},{"line_number":1324,"context_line":"        # NOTE(geguileo): To mimic current behavior we will rollback status"}],"source_content_type":"text/x-python","patch_set":11,"id":"da20952f_0d4f0abf","line":1321,"in_reply_to":"da20952f_1aec30f2","updated":"2015-09-10 19:35:31.000000000","message":"Well, nothing in life is guaranteed, but there are more chances of me winning the lottery than this having to do another pass after the next one, and I don\u0027t buy tickets.  ;-)","commit_id":"ebfef1181fb60a7699319fda4cf61e22ae61035b"},{"author":{"_account_id":9171,"name":"Vipin Balachandran","email":"vipin.bl@gmail.com","username":"vbala"},"change_message_id":"f3c02d22c7e8766fcd6e8f4dd2993f63859aef89","unresolved":false,"context_lines":[{"line_number":1384,"context_line":"                    \u0027to extend, but current status is: \u0027"},{"line_number":1385,"context_line":"                    \u0027%(vol_status)s.\u0027) % {\u0027vol_id\u0027: volume[\u0027id\u0027],"},{"line_number":1386,"context_line":"                                          \u0027vol_status\u0027: volume[\u0027status\u0027]}"},{"line_number":1387,"context_line":"            raise exception.InvalidVolume(reason\u003dmsg)"},{"line_number":1388,"context_line":""},{"line_number":1389,"context_line":"        # NOTE(geguileo): To mimic current behavior we will rollback status"},{"line_number":1390,"context_line":"        # if quota reservation fails"}],"source_content_type":"text/x-python","patch_set":11,"id":"da20952f_1aed10b7","line":1387,"updated":"2015-09-10 13:06:13.000000000","message":"Like the previous case (copy volume to image), the volume status might have changed now and if we loop, the update may return True. I think what you are doing here is right which means you can consider replacing self._check_volume_availability() in the previous method with raise InvalidVolume.","commit_id":"ebfef1181fb60a7699319fda4cf61e22ae61035b"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"27a20908d07bdee47a0f96298af078a0f1af03da","unresolved":false,"context_lines":[{"line_number":1384,"context_line":"                    \u0027to extend, but current status is: \u0027"},{"line_number":1385,"context_line":"                    \u0027%(vol_status)s.\u0027) % {\u0027vol_id\u0027: volume[\u0027id\u0027],"},{"line_number":1386,"context_line":"                                          \u0027vol_status\u0027: volume[\u0027status\u0027]}"},{"line_number":1387,"context_line":"            raise exception.InvalidVolume(reason\u003dmsg)"},{"line_number":1388,"context_line":""},{"line_number":1389,"context_line":"        # NOTE(geguileo): To mimic current behavior we will rollback status"},{"line_number":1390,"context_line":"        # if quota reservation fails"}],"source_content_type":"text/x-python","patch_set":11,"id":"da20952f_ad929ef4","line":1387,"in_reply_to":"da20952f_1aed10b7","updated":"2015-09-10 19:35:31.000000000","message":"I don\u0027t understand what you mean, _check_volume_availability already raises InvalidVolume when pertinent.","commit_id":"ebfef1181fb60a7699319fda4cf61e22ae61035b"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"722f264fc11d002e937149d9afdc2969ef5ae986","unresolved":false,"context_lines":[{"line_number":1384,"context_line":"                    \u0027to extend, but current status is: \u0027"},{"line_number":1385,"context_line":"                    \u0027%(vol_status)s.\u0027) % {\u0027vol_id\u0027: volume[\u0027id\u0027],"},{"line_number":1386,"context_line":"                                          \u0027vol_status\u0027: volume[\u0027status\u0027]}"},{"line_number":1387,"context_line":"            raise exception.InvalidVolume(reason\u003dmsg)"},{"line_number":1388,"context_line":""},{"line_number":1389,"context_line":"        # NOTE(geguileo): To mimic current behavior we will rollback status"},{"line_number":1390,"context_line":"        # if quota reservation fails"}],"source_content_type":"text/x-python","patch_set":11,"id":"da20952f_da73f314","line":1387,"in_reply_to":"da20952f_377ecb1b","updated":"2015-09-11 09:00:35.000000000","message":"Yep, on line 1380 is where I check for the available state.","commit_id":"ebfef1181fb60a7699319fda4cf61e22ae61035b"},{"author":{"_account_id":9171,"name":"Vipin Balachandran","email":"vipin.bl@gmail.com","username":"vbala"},"change_message_id":"23aea78e4e1f71e59c6047e6dbdbf6fdada43e51","unresolved":false,"context_lines":[{"line_number":1384,"context_line":"                    \u0027to extend, but current status is: \u0027"},{"line_number":1385,"context_line":"                    \u0027%(vol_status)s.\u0027) % {\u0027vol_id\u0027: volume[\u0027id\u0027],"},{"line_number":1386,"context_line":"                                          \u0027vol_status\u0027: volume[\u0027status\u0027]}"},{"line_number":1387,"context_line":"            raise exception.InvalidVolume(reason\u003dmsg)"},{"line_number":1388,"context_line":""},{"line_number":1389,"context_line":"        # NOTE(geguileo): To mimic current behavior we will rollback status"},{"line_number":1390,"context_line":"        # if quota reservation fails"}],"source_content_type":"text/x-python","patch_set":11,"id":"da20952f_377ecb1b","line":1387,"in_reply_to":"da20952f_ad929ef4","updated":"2015-09-11 08:10:47.000000000","message":"Sorry that I was not clear. In this case, you don\u0027t bother to read the status again as done in _check_volume_availability and check if the status has changed after the failure of conditional_update. But I overlooked the if condition above.","commit_id":"ebfef1181fb60a7699319fda4cf61e22ae61035b"},{"author":{"_account_id":16708,"name":"Kendall Nelson","display_name":"Kendall (diablo_rojo)","email":"kennelson11@gmail.com","username":"kjnelson"},"change_message_id":"4869bd9b276e13975d058c4a24dc7a307d88912d","unresolved":false,"context_lines":[{"line_number":1361,"context_line":"                                                    \u0027size\u0027: volume[\u0027size\u0027]})"},{"line_number":1362,"context_line":"            raise exception.InvalidInput(reason\u003dmsg)"},{"line_number":1363,"context_line":""},{"line_number":1364,"context_line":"        # Loop until we make a decission on whether to go forward with the"},{"line_number":1365,"context_line":"        # extend or raise and error"},{"line_number":1366,"context_line":"        # TODO(geguileo): Once we are using versioned objects we should change"},{"line_number":1367,"context_line":"        # this to use volume.conditional_update and remove the id from the"}],"source_content_type":"text/x-python","patch_set":12,"id":"da20952f_0ef83649","line":1364,"updated":"2015-09-11 18:44:55.000000000","message":"decission -\u003e decision","commit_id":"f540283b708c7785e51d1198d2b0518b0721dccc"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"b1d4b157833a84b6c08cd05e17b06d3e744230a2","unresolved":false,"context_lines":[{"line_number":1167,"context_line":"        try:"},{"line_number":1168,"context_line":"            recv_metadata \u003d self.image_service.create("},{"line_number":1169,"context_line":"                context, self.image_service._translate_to_glance(metadata))"},{"line_number":1170,"context_line":"        except Exception:"},{"line_number":1171,"context_line":"            # NOTE(geguileo): To mimic behavior before conditional_update we"},{"line_number":1172,"context_line":"            # will rollback status if image create fails"},{"line_number":1173,"context_line":"            with excutils.save_and_reraise_exception():"}],"source_content_type":"text/x-python","patch_set":24,"id":"da6ed579_8bd13f25","line":1170,"updated":"2016-01-15 15:06:10.000000000","message":"Can we catch more specific exception here?","commit_id":"68efc82ca8de71aaa1ddc3865d44ed2c36f83fdc"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"e74cd45d573dbcbfe3e5083fbad835211e8c8ea9","unresolved":false,"context_lines":[{"line_number":1167,"context_line":"        try:"},{"line_number":1168,"context_line":"            recv_metadata \u003d self.image_service.create("},{"line_number":1169,"context_line":"                context, self.image_service._translate_to_glance(metadata))"},{"line_number":1170,"context_line":"        except Exception:"},{"line_number":1171,"context_line":"            # NOTE(geguileo): To mimic behavior before conditional_update we"},{"line_number":1172,"context_line":"            # will rollback status if image create fails"},{"line_number":1173,"context_line":"            with excutils.save_and_reraise_exception():"}],"source_content_type":"text/x-python","patch_set":24,"id":"da6ed579_66b875e6","line":1170,"in_reply_to":"da6ed579_8bd13f25","updated":"2016-01-18 11:18:35.000000000","message":"I believe we shouldn\u0027t catch a more specific exception, since we are trying to reproduce the same behavior we had before, with means that if there\u0027s *any exception* on `image_service_create` we should have status in its original state.\n\nBefore this was done by changing the status after storing the image data, but that way we can\u0027t prevent API races.","commit_id":"68efc82ca8de71aaa1ddc3865d44ed2c36f83fdc"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"b5520dcbd0d6e9f8a827d27a4640923e62fe0d33","unresolved":false,"context_lines":[{"line_number":1139,"context_line":"        glance_core_properties \u003d CONF.glance_core_properties"},{"line_number":1140,"context_line":"        if glance_core_properties:"},{"line_number":1141,"context_line":"            try:"},{"line_number":1142,"context_line":"                volume_image_metadata \u003d self.get_volume_image_metadata(context,"},{"line_number":1143,"context_line":"                                                                       volume)"},{"line_number":1144,"context_line":"                custom_property_set \u003d (set(volume_image_metadata).difference"},{"line_number":1145,"context_line":"                                       (set(glance_core_properties)))"},{"line_number":1146,"context_line":"                if custom_property_set:"}],"source_content_type":"text/x-python","patch_set":28,"id":"7af24918_20ccf7b9","line":1143,"range":{"start_line":1142,"start_character":0,"end_line":1143,"end_character":78},"updated":"2016-03-04 12:24:21.000000000","message":"Being paranoid - volume_image_metadata are freely editable, so there\u0027s a race condition here. Unfortunately this flies to Glance API, so there\u0027s no way to ensure nothing changed.\n\nAnd actually I don\u0027t think it hurts much.","commit_id":"5a9bb353d55dd7c0dc11a30feb959c81f8a12407"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"c76ec012739869f94ca32eb484e831c4d57c4bdc","unresolved":false,"context_lines":[{"line_number":1139,"context_line":"        glance_core_properties \u003d CONF.glance_core_properties"},{"line_number":1140,"context_line":"        if glance_core_properties:"},{"line_number":1141,"context_line":"            try:"},{"line_number":1142,"context_line":"                volume_image_metadata \u003d self.get_volume_image_metadata(context,"},{"line_number":1143,"context_line":"                                                                       volume)"},{"line_number":1144,"context_line":"                custom_property_set \u003d (set(volume_image_metadata).difference"},{"line_number":1145,"context_line":"                                       (set(glance_core_properties)))"},{"line_number":1146,"context_line":"                if custom_property_set:"}],"source_content_type":"text/x-python","patch_set":28,"id":"5aef4532_4d163c56","line":1143,"range":{"start_line":1142,"start_character":0,"end_line":1143,"end_character":78},"in_reply_to":"5aef4532_dc9a44c5","updated":"2016-03-08 12:30:30.000000000","message":"You are right, I didn\u0027t see it.\n\nI agree with you that it doesn\u0027t hurt much, and we cannot prevent those races without adding a new DB status \"updating metadata\" or a locking mechanism.\n\nWhat I\u0027ve done in the latest patch is reduce the chances of some races from happening.","commit_id":"5a9bb353d55dd7c0dc11a30feb959c81f8a12407"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"bc77eb25b0020e8276f03ea2202222521df55bc1","unresolved":false,"context_lines":[{"line_number":1139,"context_line":"        glance_core_properties \u003d CONF.glance_core_properties"},{"line_number":1140,"context_line":"        if glance_core_properties:"},{"line_number":1141,"context_line":"            try:"},{"line_number":1142,"context_line":"                volume_image_metadata \u003d self.get_volume_image_metadata(context,"},{"line_number":1143,"context_line":"                                                                       volume)"},{"line_number":1144,"context_line":"                custom_property_set \u003d (set(volume_image_metadata).difference"},{"line_number":1145,"context_line":"                                       (set(glance_core_properties)))"},{"line_number":1146,"context_line":"                if custom_property_set:"}],"source_content_type":"text/x-python","patch_set":28,"id":"7af24918_4205bd43","line":1143,"range":{"start_line":1142,"start_character":0,"end_line":1143,"end_character":78},"in_reply_to":"7af24918_20ccf7b9","updated":"2016-03-04 15:35:24.000000000","message":"AFAIK there is no race condition here... This metadata is stored in Cinder DB and the only time it is touched is on volume creation.","commit_id":"5a9bb353d55dd7c0dc11a30feb959c81f8a12407"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"76807e4428a7eeba6624410d263811dcc5fdcda5","unresolved":false,"context_lines":[{"line_number":1139,"context_line":"        glance_core_properties \u003d CONF.glance_core_properties"},{"line_number":1140,"context_line":"        if glance_core_properties:"},{"line_number":1141,"context_line":"            try:"},{"line_number":1142,"context_line":"                volume_image_metadata \u003d self.get_volume_image_metadata(context,"},{"line_number":1143,"context_line":"                                                                       volume)"},{"line_number":1144,"context_line":"                custom_property_set \u003d (set(volume_image_metadata).difference"},{"line_number":1145,"context_line":"                                       (set(glance_core_properties)))"},{"line_number":1146,"context_line":"                if custom_property_set:"}],"source_content_type":"text/x-python","patch_set":28,"id":"5aef4532_dc9a44c5","line":1143,"range":{"start_line":1142,"start_character":0,"end_line":1143,"end_character":78},"in_reply_to":"7af24918_4205bd43","updated":"2016-03-07 10:44:13.000000000","message":"I think that command below edits them. AFAIK it was added in Liberty.\n\nmdulko:cinder/ (master) $ cinder help image-metadata\n usage: cinder image-metadata \u003cvolume\u003e \u003caction\u003e \u003ckey\u003dvalue\u003e [\u003ckey\u003dvalue\u003e ...]\n\n Sets or deletes volume image metadata.\n\n Positional arguments:\n   \u003cvolume\u003e     Name or ID of volume for which to update metadata.\n   \u003caction\u003e     The action. Valid values are \u0027set\u0027 or \u0027unset.\u0027\n   \u003ckey\u003dvalue\u003e  Metadata key and value pair to set or unset. For unset, specify\n               only the key.","commit_id":"5a9bb353d55dd7c0dc11a30feb959c81f8a12407"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"b5520dcbd0d6e9f8a827d27a4640923e62fe0d33","unresolved":false,"context_lines":[{"line_number":1219,"context_line":"            reservations \u003d None"},{"line_number":1220,"context_line":"            reserve_opts \u003d {\u0027gigabytes\u0027: size_increase}"},{"line_number":1221,"context_line":"            QUOTAS.add_volume_type_opts(context, reserve_opts,"},{"line_number":1222,"context_line":"                                        volume.volume_type_id)"},{"line_number":1223,"context_line":"            reservations \u003d QUOTAS.reserve(context,"},{"line_number":1224,"context_line":"                                          project_id\u003dvolume.project_id,"},{"line_number":1225,"context_line":"                                          **reserve_opts)"}],"source_content_type":"text/x-python","patch_set":28,"id":"7af24918_a07be7bc","line":1222,"range":{"start_line":1222,"start_character":40,"end_line":1222,"end_character":61},"updated":"2016-03-04 12:24:21.000000000","message":"And again adding some paranoia - what if volume was retyped between it was fetched from DB and status changed? We\u0027ll be reserving quota for wrong volume type.\n\n(And no, I really don\u0027t know if that\u0027s possible in real world).","commit_id":"5a9bb353d55dd7c0dc11a30feb959c81f8a12407"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"76807e4428a7eeba6624410d263811dcc5fdcda5","unresolved":false,"context_lines":[{"line_number":1219,"context_line":"            reservations \u003d None"},{"line_number":1220,"context_line":"            reserve_opts \u003d {\u0027gigabytes\u0027: size_increase}"},{"line_number":1221,"context_line":"            QUOTAS.add_volume_type_opts(context, reserve_opts,"},{"line_number":1222,"context_line":"                                        volume.volume_type_id)"},{"line_number":1223,"context_line":"            reservations \u003d QUOTAS.reserve(context,"},{"line_number":1224,"context_line":"                                          project_id\u003dvolume.project_id,"},{"line_number":1225,"context_line":"                                          **reserve_opts)"}],"source_content_type":"text/x-python","patch_set":28,"id":"5aef4532_5cdd946f","line":1222,"range":{"start_line":1222,"start_character":40,"end_line":1222,"end_character":61},"in_reply_to":"7af24918_584810b3","updated":"2016-03-07 10:44:13.000000000","message":"I again agree that doing quotas right would be best.","commit_id":"5a9bb353d55dd7c0dc11a30feb959c81f8a12407"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"d3a62ad9da46acef369437cedb827103cc31d3f6","unresolved":false,"context_lines":[{"line_number":1219,"context_line":"            reservations \u003d None"},{"line_number":1220,"context_line":"            reserve_opts \u003d {\u0027gigabytes\u0027: size_increase}"},{"line_number":1221,"context_line":"            QUOTAS.add_volume_type_opts(context, reserve_opts,"},{"line_number":1222,"context_line":"                                        volume.volume_type_id)"},{"line_number":1223,"context_line":"            reservations \u003d QUOTAS.reserve(context,"},{"line_number":1224,"context_line":"                                          project_id\u003dvolume.project_id,"},{"line_number":1225,"context_line":"                                          **reserve_opts)"}],"source_content_type":"text/x-python","patch_set":28,"id":"7af24918_584810b3","line":1222,"range":{"start_line":1222,"start_character":40,"end_line":1222,"end_character":61},"in_reply_to":"7af24918_82198524","updated":"2016-03-04 16:04:18.000000000","message":"I have just realized what you meant.  The best solution would be to change the way we do the quotas so that they work referencing the volume type that\u0027s on the DB in the volume, but removing races from quotas is another whole different pain.  Because the alternative is to retrieve the volume from the DB after we have changed the status and I really dislike that solution. :-(","commit_id":"5a9bb353d55dd7c0dc11a30feb959c81f8a12407"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"bc77eb25b0020e8276f03ea2202222521df55bc1","unresolved":false,"context_lines":[{"line_number":1219,"context_line":"            reservations \u003d None"},{"line_number":1220,"context_line":"            reserve_opts \u003d {\u0027gigabytes\u0027: size_increase}"},{"line_number":1221,"context_line":"            QUOTAS.add_volume_type_opts(context, reserve_opts,"},{"line_number":1222,"context_line":"                                        volume.volume_type_id)"},{"line_number":1223,"context_line":"            reservations \u003d QUOTAS.reserve(context,"},{"line_number":1224,"context_line":"                                          project_id\u003dvolume.project_id,"},{"line_number":1225,"context_line":"                                          **reserve_opts)"}],"source_content_type":"text/x-python","patch_set":28,"id":"7af24918_82198524","line":1222,"range":{"start_line":1222,"start_character":40,"end_line":1222,"end_character":61},"in_reply_to":"7af24918_a07be7bc","updated":"2016-03-04 15:35:24.000000000","message":"It cannot happen because a retype requires status to be available or in-use and we have atomically changed it to extending on L1213.","commit_id":"5a9bb353d55dd7c0dc11a30feb959c81f8a12407"},{"author":{"_account_id":10439,"name":"Xi Yang","email":"yang.xi@99cloud.net","username":"phenom"},"change_message_id":"2f2f22b6270b336bc8d16b0036a65145a3a4ae90","unresolved":false,"context_lines":[{"line_number":973,"context_line":"                               key, meta_type\u003dcommon.METADATA_TYPES.user):"},{"line_number":974,"context_line":"        \"\"\"Delete the given metadata item from a volume.\"\"\""},{"line_number":975,"context_line":"        if volume[\u0027status\u0027] in (\u0027maintenance\u0027, \u0027uploading\u0027):"},{"line_number":976,"context_line":"            LOG.info(_LI(\u0027Unable to delete the volume metadata, \u0027"},{"line_number":977,"context_line":"                         \u0027because it is in %s.\u0027) % volume[\u0027status\u0027],"},{"line_number":978,"context_line":"                     resource\u003dvolume)"},{"line_number":979,"context_line":"            msg \u003d _(\"The volume metadata cannot be deleted when the volume \""}],"source_content_type":"text/x-python","patch_set":30,"id":"5aef4532_3bd4a545","line":976,"updated":"2016-03-09 03:23:09.000000000","message":"Since we throw an exception here, should we change the log level from info to exception?","commit_id":"caabd4a8bcc5d34aff9440228c2267663b904e0f"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"43ded39b0eaa3c11a122887f058b7372953cc2b8","unresolved":false,"context_lines":[{"line_number":973,"context_line":"                               key, meta_type\u003dcommon.METADATA_TYPES.user):"},{"line_number":974,"context_line":"        \"\"\"Delete the given metadata item from a volume.\"\"\""},{"line_number":975,"context_line":"        if volume[\u0027status\u0027] in (\u0027maintenance\u0027, \u0027uploading\u0027):"},{"line_number":976,"context_line":"            LOG.info(_LI(\u0027Unable to delete the volume metadata, \u0027"},{"line_number":977,"context_line":"                         \u0027because it is in %s.\u0027) % volume[\u0027status\u0027],"},{"line_number":978,"context_line":"                     resource\u003dvolume)"},{"line_number":979,"context_line":"            msg \u003d _(\"The volume metadata cannot be deleted when the volume \""}],"source_content_type":"text/x-python","patch_set":30,"id":"5aef4532_4c4f226b","line":976,"in_reply_to":"5aef4532_3bd4a545","updated":"2016-03-09 11:51:33.000000000","message":"I don\u0027t think we should change the level, because this is not an error that concerns admins, it\u0027s just an invalid request (like non-forced delete of a volume with snapshots).","commit_id":"caabd4a8bcc5d34aff9440228c2267663b904e0f"},{"author":{"_account_id":10439,"name":"Xi Yang","email":"yang.xi@99cloud.net","username":"phenom"},"change_message_id":"2f2f22b6270b336bc8d16b0036a65145a3a4ae90","unresolved":false,"context_lines":[{"line_number":977,"context_line":"                         \u0027because it is in %s.\u0027) % volume[\u0027status\u0027],"},{"line_number":978,"context_line":"                     resource\u003dvolume)"},{"line_number":979,"context_line":"            msg \u003d _(\"The volume metadata cannot be deleted when the volume \""},{"line_number":980,"context_line":"                    \"is in maintenance mode.\")"},{"line_number":981,"context_line":"            raise exception.InvalidVolume(reason\u003dmsg)"},{"line_number":982,"context_line":"        self.db.volume_metadata_delete(context, volume[\u0027id\u0027], key, meta_type)"},{"line_number":983,"context_line":"        LOG.info(_LI(\"Delete volume metadata completed successfully.\"),"}],"source_content_type":"text/x-python","patch_set":30,"id":"5aef4532_9bf4d9f2","line":980,"updated":"2016-03-09 03:23:09.000000000","message":"Should we modify this msg too?","commit_id":"caabd4a8bcc5d34aff9440228c2267663b904e0f"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"43ded39b0eaa3c11a122887f058b7372953cc2b8","unresolved":false,"context_lines":[{"line_number":977,"context_line":"                         \u0027because it is in %s.\u0027) % volume[\u0027status\u0027],"},{"line_number":978,"context_line":"                     resource\u003dvolume)"},{"line_number":979,"context_line":"            msg \u003d _(\"The volume metadata cannot be deleted when the volume \""},{"line_number":980,"context_line":"                    \"is in maintenance mode.\")"},{"line_number":981,"context_line":"            raise exception.InvalidVolume(reason\u003dmsg)"},{"line_number":982,"context_line":"        self.db.volume_metadata_delete(context, volume[\u0027id\u0027], key, meta_type)"},{"line_number":983,"context_line":"        LOG.info(_LI(\"Delete volume metadata completed successfully.\"),"}],"source_content_type":"text/x-python","patch_set":30,"id":"5aef4532_79b90279","line":980,"in_reply_to":"5aef4532_9bf4d9f2","updated":"2016-03-09 11:51:33.000000000","message":"You are right, I don\u0027t know how I missed it.","commit_id":"caabd4a8bcc5d34aff9440228c2267663b904e0f"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"43ded39b0eaa3c11a122887f058b7372953cc2b8","unresolved":false,"context_lines":[{"line_number":1016,"context_line":"                         \u0027because it is in %s.\u0027) % volume[\u0027status\u0027],"},{"line_number":1017,"context_line":"                     resource\u003dvolume)"},{"line_number":1018,"context_line":"            msg \u003d _(\"The volume metadata cannot be updated when the volume \""},{"line_number":1019,"context_line":"                    \"is in maintenance mode.\")"},{"line_number":1020,"context_line":"            raise exception.InvalidVolume(reason\u003dmsg)"},{"line_number":1021,"context_line":"        self._check_metadata_properties(metadata)"},{"line_number":1022,"context_line":"        db_meta \u003d self.db.volume_metadata_update(context, volume[\u0027id\u0027],"}],"source_content_type":"text/x-python","patch_set":30,"id":"5aef4532_6ca81edc","line":1019,"updated":"2016-03-09 11:51:33.000000000","message":"And I missed it here as well.","commit_id":"caabd4a8bcc5d34aff9440228c2267663b904e0f"},{"author":{"_account_id":7173,"name":"Scott DAngelo","email":"scott.dangelo@gmail.com","username":"scottda"},"change_message_id":"51b0f4f2fa5924a66ff81d4aec968eec79a122f7","unresolved":false,"context_lines":[{"line_number":973,"context_line":"    def delete_volume_metadata(self, context, volume,"},{"line_number":974,"context_line":"                               key, meta_type\u003dcommon.METADATA_TYPES.user):"},{"line_number":975,"context_line":"        \"\"\"Delete the given metadata item from a volume.\"\"\""},{"line_number":976,"context_line":"        if volume[\u0027status\u0027] in (\u0027maintenance\u0027, \u0027uploading\u0027):"},{"line_number":977,"context_line":"            msg \u003d _(\u0027Deleting volume metadata is not allowed for volumes in \u0027"},{"line_number":978,"context_line":"                    \u0027%s status.\u0027) % volume[\u0027status\u0027]"},{"line_number":979,"context_line":"            LOG.info(msg, resource\u003dvolume)"}],"source_content_type":"text/x-python","patch_set":36,"id":"5a18252c_8e17cb50","line":976,"updated":"2016-04-12 15:12:29.000000000","message":"volume.status ?","commit_id":"226dcb3881717564191111648a9e0acea455979b"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"ffea4b810c6fbe3e09e1069388e7e0f46760c58a","unresolved":false,"context_lines":[{"line_number":973,"context_line":"    def delete_volume_metadata(self, context, volume,"},{"line_number":974,"context_line":"                               key, meta_type\u003dcommon.METADATA_TYPES.user):"},{"line_number":975,"context_line":"        \"\"\"Delete the given metadata item from a volume.\"\"\""},{"line_number":976,"context_line":"        if volume[\u0027status\u0027] in (\u0027maintenance\u0027, \u0027uploading\u0027):"},{"line_number":977,"context_line":"            msg \u003d _(\u0027Deleting volume metadata is not allowed for volumes in \u0027"},{"line_number":978,"context_line":"                    \u0027%s status.\u0027) % volume[\u0027status\u0027]"},{"line_number":979,"context_line":"            LOG.info(msg, resource\u003dvolume)"}],"source_content_type":"text/x-python","patch_set":36,"id":"5a18252c_664493b6","line":976,"in_reply_to":"5a18252c_8e17cb50","updated":"2016-04-14 16:06:46.000000000","message":"Done","commit_id":"226dcb3881717564191111648a9e0acea455979b"},{"author":{"_account_id":7173,"name":"Scott DAngelo","email":"scott.dangelo@gmail.com","username":"scottda"},"change_message_id":"51b0f4f2fa5924a66ff81d4aec968eec79a122f7","unresolved":false,"context_lines":[{"line_number":975,"context_line":"        \"\"\"Delete the given metadata item from a volume.\"\"\""},{"line_number":976,"context_line":"        if volume[\u0027status\u0027] in (\u0027maintenance\u0027, \u0027uploading\u0027):"},{"line_number":977,"context_line":"            msg \u003d _(\u0027Deleting volume metadata is not allowed for volumes in \u0027"},{"line_number":978,"context_line":"                    \u0027%s status.\u0027) % volume[\u0027status\u0027]"},{"line_number":979,"context_line":"            LOG.info(msg, resource\u003dvolume)"},{"line_number":980,"context_line":"            raise exception.InvalidVolume(reason\u003dmsg)"},{"line_number":981,"context_line":"        self.db.volume_metadata_delete(context, volume[\u0027id\u0027], key, meta_type)"}],"source_content_type":"text/x-python","patch_set":36,"id":"5a18252c_ae3ba7ea","line":978,"updated":"2016-04-12 15:12:29.000000000","message":"volume.status","commit_id":"226dcb3881717564191111648a9e0acea455979b"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"ffea4b810c6fbe3e09e1069388e7e0f46760c58a","unresolved":false,"context_lines":[{"line_number":975,"context_line":"        \"\"\"Delete the given metadata item from a volume.\"\"\""},{"line_number":976,"context_line":"        if volume[\u0027status\u0027] in (\u0027maintenance\u0027, \u0027uploading\u0027):"},{"line_number":977,"context_line":"            msg \u003d _(\u0027Deleting volume metadata is not allowed for volumes in \u0027"},{"line_number":978,"context_line":"                    \u0027%s status.\u0027) % volume[\u0027status\u0027]"},{"line_number":979,"context_line":"            LOG.info(msg, resource\u003dvolume)"},{"line_number":980,"context_line":"            raise exception.InvalidVolume(reason\u003dmsg)"},{"line_number":981,"context_line":"        self.db.volume_metadata_delete(context, volume[\u0027id\u0027], key, meta_type)"}],"source_content_type":"text/x-python","patch_set":36,"id":"5a18252c_c613e7b6","line":978,"in_reply_to":"5a18252c_ae3ba7ea","updated":"2016-04-14 16:06:46.000000000","message":"Done","commit_id":"226dcb3881717564191111648a9e0acea455979b"}]}
