)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":10439,"name":"Xi Yang","email":"yang.xi@99cloud.net","username":"phenom"},"change_message_id":"12c7229c2f415b2dcd3a10ebed7c8f9f2e184d74","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Vincent Hou \u003csbhou@cn.ibm.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2015-09-01 11:17:04 +0800"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Remove the detination volume check in delete_volume"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"If the volume migration fails in the middle of the progress,"},{"line_number":10,"context_line":"the destination volume will be cleaned up via"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"da20952f_e3cd5587","line":7,"updated":"2015-09-02 08:54:22.000000000","message":"Typo: detination -\u003e destination","commit_id":"361cef0b307b2255646dc8b1f966acde0a52a8cd"}],"cinder/volume/manager.py":[{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"ac0df2df92772c68bf355091bb45a1b691e33dee","unresolved":false,"context_lines":[{"line_number":536,"context_line":"           migrate_volume_completion properly."},{"line_number":537,"context_line":""},{"line_number":538,"context_line":"        3. Delete a migration destination volume"},{"line_number":539,"context_line":"           If deleting the destination volume in a migration, we want to"},{"line_number":540,"context_line":"           skip quotas but we need database updates for the volume."},{"line_number":541,"context_line":"      \"\"\""},{"line_number":542,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"da20952f_76a9297b","line":539,"updated":"2015-09-01 17:17:18.000000000","message":"Based on this help text it seems that we have two different expectations based on whether we are deleting a migration source or destination.  If it was the destination then the change you have made is correct but I think we will not get the right result for a source volume.  So, I think we need to do something different.\n\nCan we leave the code here as is and update the calling function to update the DB or not as desired?","commit_id":"361cef0b307b2255646dc8b1f966acde0a52a8cd"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"1c4c3f83ac5140fb113219cb6f3f9aff17f3a76b","unresolved":false,"context_lines":[{"line_number":536,"context_line":"           migrate_volume_completion properly."},{"line_number":537,"context_line":""},{"line_number":538,"context_line":"        3. Delete a migration destination volume"},{"line_number":539,"context_line":"           If deleting the destination volume in a migration, we want to"},{"line_number":540,"context_line":"           skip quotas but we need database updates for the volume."},{"line_number":541,"context_line":"      \"\"\""},{"line_number":542,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"da20952f_bff8e4c5","line":539,"in_reply_to":"da20952f_76a9297b","updated":"2015-09-01 23:36:09.000000000","message":"The code calls rpc to delete the volume ,which is asynchronous. We need to keep it in the db, as you see the code get the volume from db. If we remove the record before rpc, we will not delete the back end. If we remove the record after rpc, I have not idea which comes first, since rpc is asynchronous.\n\nPer the comments here, it is the behavior we need to remove the dest volume. For the source volume, we only remove it when a migration is successfully done. The if logic does not have anything to do with source volume.\n\nHowever, there is a compromise way to do it. Adding one new flag like clean db or something, if it is set, we force to clean the db. Not sure how much folks like it.","commit_id":"361cef0b307b2255646dc8b1f966acde0a52a8cd"},{"author":{"_account_id":10115,"name":"Mitsuhiro Tanino","email":"mitsuhiro.tanino@lycorp.co.jp","username":"mtanino"},"change_message_id":"485ddf5ceac5c2e2c6cf6fff67725d47bd5331be","unresolved":false,"context_lines":[{"line_number":536,"context_line":"           migrate_volume_completion properly."},{"line_number":537,"context_line":""},{"line_number":538,"context_line":"        3. Delete a migration destination volume"},{"line_number":539,"context_line":"           If deleting the destination volume in a migration, we want to"},{"line_number":540,"context_line":"           skip quotas but we need database updates for the volume."},{"line_number":541,"context_line":"      \"\"\""},{"line_number":542,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"da20952f_43438a13","line":539,"in_reply_to":"da20952f_830f86e0","updated":"2015-09-03 20:35:10.000000000","message":"Seems this patch cdf5e92f10fe8d097f91f6f63a2c77f6ab2bcdb8 changed the logic around Line 619, but the patch did not update this documentation.\nWe need to get Gorka\u0027s opinion.","commit_id":"361cef0b307b2255646dc8b1f966acde0a52a8cd"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"009299dbbdb8f53ce8c67d1e52d198690e3d6b8c","unresolved":false,"context_lines":[{"line_number":536,"context_line":"           migrate_volume_completion properly."},{"line_number":537,"context_line":""},{"line_number":538,"context_line":"        3. Delete a migration destination volume"},{"line_number":539,"context_line":"           If deleting the destination volume in a migration, we want to"},{"line_number":540,"context_line":"           skip quotas but we need database updates for the volume."},{"line_number":541,"context_line":"      \"\"\""},{"line_number":542,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"da20952f_cdee40b4","line":539,"in_reply_to":"da20952f_bff8e4c5","updated":"2015-09-02 08:13:28.000000000","message":"I am happened to encounter this problem.\nHave a try the following case:\n\n1. Fake to raise an exception before the line https://github.com/openstack/cinder/blob/master/cinder/volume/manager.py#L1481\n\nIt simulate an exception raised in vol_utils.copy_volume()\n\n2. Restart the volume service.\n\n3. Create a volume with type 1 with default lvmdriver-1 by devstack.\n\n4. run command \u0027cinder create type2\u0027\n\n4. cinder retype the volume into type2\ncinder retype --migration-policy on-demand b0d79b00-7da1-4507-8ce0-acd24fdc4535 lisa\n\n5. Wait above command finish.\n\nI found that with the condition check \nhttps://github.com/openstack/cinder/blob/master/cinder/volume/manager.py#L623\n\nIt has wrong result: only db entry is left, and the real storage volume in lvm is deleted.\n\nWhen I removed the check, everything goes well.\n\nVicent, may you can try above case in normal migration scenaio.\nPing me if you need.\n\nMeanwhile, @Jay, not sure \u0027 I think we will not get the right result for a source volume.\u0027.\nIf the volume to deleted is a source volume, I think is_migrating_dest is false. As the value is true when the volume is a target of migration.\n\nSo the change has no impact on source volume of migration.\n\nPlease correct me if I am wrong.","commit_id":"361cef0b307b2255646dc8b1f966acde0a52a8cd"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"41717d5a9deaa250a83cd58d1de8817bb8d04e70","unresolved":false,"context_lines":[{"line_number":536,"context_line":"           migrate_volume_completion properly."},{"line_number":537,"context_line":""},{"line_number":538,"context_line":"        3. Delete a migration destination volume"},{"line_number":539,"context_line":"           If deleting the destination volume in a migration, we want to"},{"line_number":540,"context_line":"           skip quotas but we need database updates for the volume."},{"line_number":541,"context_line":"      \"\"\""},{"line_number":542,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"da20952f_830f86e0","line":539,"in_reply_to":"da20952f_cdee40b4","updated":"2015-09-03 07:05:38.000000000","message":"IMO, I think we should remove the check Line 622 \u0027if not is_migrating_dest\u0027.\nAs you described, it makes no sense to leave the db record, but the back-end is gone.","commit_id":"361cef0b307b2255646dc8b1f966acde0a52a8cd"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"7e98a6dc6d8a2506344f376df74c3045d715d158","unresolved":false,"context_lines":[{"line_number":583,"context_line":"           If deleting the source volume in a migration, we want to skip"},{"line_number":584,"context_line":"           quotas. Also we want to skip other database updates for source"},{"line_number":585,"context_line":"           volume because these update will be handled at"},{"line_number":586,"context_line":"           migrate_volume_completion properly."},{"line_number":587,"context_line":""},{"line_number":588,"context_line":"        3. Delete a migration destination volume"},{"line_number":589,"context_line":"           If deleting the destination volume in a migration, we want to"}],"source_content_type":"text/x-python","patch_set":4,"id":"ba15a1d1_4bc5058c","line":586,"updated":"2015-09-22 14:22:31.000000000","message":"-1: Update this docstring.","commit_id":"4823a2805517d271b6671677a96cf56cf8a8d47c"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"8a17e13eed37f8444d2f2ba322078643979f1059","unresolved":false,"context_lines":[{"line_number":579,"context_line":"        1. Delete a volume(normal case)"},{"line_number":580,"context_line":"           Delete a volume and update quotas."},{"line_number":581,"context_line":""},{"line_number":582,"context_line":"        2. Delete a migration source volume"},{"line_number":583,"context_line":"           If deleting the source volume in a migration, we want to skip"},{"line_number":584,"context_line":"           quotas but we need database updates for the volume."},{"line_number":585,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"ba15a1d1_923d6991","line":582,"updated":"2015-09-23 09:05:11.000000000","message":"nit: It looks like 2 and 3 are the same thing, so maybe this should have been updated to just say \"Delete a migration volume\"","commit_id":"7e384b3be186baa7deb36131934f23377e1b5c49"},{"author":{"_account_id":9171,"name":"Vipin Balachandran","email":"vipin.bl@gmail.com","username":"vbala"},"change_message_id":"d79cc9fce21767c1150f9d7b1a3e0194bb534a09","unresolved":false,"context_lines":[{"line_number":669,"context_line":"        # If deleting the destination volume in a migration, we should skip"},{"line_number":670,"context_line":"        # database update here. In other cases, continue to update database"},{"line_number":671,"context_line":"        # entries."},{"line_number":672,"context_line":"        if not is_migrating_dest:"},{"line_number":673,"context_line":""},{"line_number":674,"context_line":"            # Delete glance metadata if it exists"},{"line_number":675,"context_line":"            self.db.volume_glance_metadata_delete_by_volume(context, volume_id)"}],"source_content_type":"text/x-python","patch_set":6,"id":"9a1a9d01_4a58f656","side":"PARENT","line":672,"updated":"2015-09-24 18:33:48.000000000","message":"Do we need to add/update any unit test for this change?","commit_id":"f2257e029fc484e87b5cecd1b29396baf2c7017b"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"848440eca35ef8249322468339f11c45e471d744","unresolved":false,"context_lines":[{"line_number":669,"context_line":"        # If deleting the destination volume in a migration, we should skip"},{"line_number":670,"context_line":"        # database update here. In other cases, continue to update database"},{"line_number":671,"context_line":"        # entries."},{"line_number":672,"context_line":"        if not is_migrating_dest:"},{"line_number":673,"context_line":""},{"line_number":674,"context_line":"            # Delete glance metadata if it exists"},{"line_number":675,"context_line":"            self.db.volume_glance_metadata_delete_by_volume(context, volume_id)"}],"source_content_type":"text/x-python","patch_set":6,"id":"9a1a9d01_57308d56","side":"PARENT","line":672,"in_reply_to":"9a1a9d01_4a58f656","updated":"2015-09-25 05:39:34.000000000","message":"It is better that I add one for the source/dest volume during migration.","commit_id":"f2257e029fc484e87b5cecd1b29396baf2c7017b"}]}