)]}'
{"cinder/db/sqlalchemy/api.py":[{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"21c71b678266e8524b2445dd74d6452fbe275558","unresolved":false,"context_lines":[{"line_number":1139,"context_line":""},{"line_number":1140,"context_line":"@require_admin_context"},{"line_number":1141,"context_line":"def finish_volume_migration(context, src_vol_id, dest_vol_id):"},{"line_number":1142,"context_line":"    \"\"\"Swap almost all columns between dest and source.\"\"\""},{"line_number":1143,"context_line":"    session \u003d get_session()"},{"line_number":1144,"context_line":"    with session.begin():"},{"line_number":1145,"context_line":"        src_volume_ref \u003d _volume_get(context, src_vol_id, session\u003dsession)"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a4dcd0f_f98e2ecf","line":1142,"updated":"2015-08-11 07:52:35.000000000","message":"I am not sure why you are swapping the source and dest volumes.\nCan you give more explanations?","commit_id":"3624eab1df82a7881b3e5485d458e0bc1c8e7595"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"3a83c6bb98a67d923d82408c437fdefb2c252817","unresolved":false,"context_lines":[{"line_number":1139,"context_line":""},{"line_number":1140,"context_line":"@require_admin_context"},{"line_number":1141,"context_line":"def finish_volume_migration(context, src_vol_id, dest_vol_id):"},{"line_number":1142,"context_line":"    \"\"\"Swap almost all columns between dest and source.\"\"\""},{"line_number":1143,"context_line":"    session \u003d get_session()"},{"line_number":1144,"context_line":"    with session.begin():"},{"line_number":1145,"context_line":"        src_volume_ref \u003d _volume_get(context, src_vol_id, session\u003dsession)"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a4dcd0f_f7961b4b","line":1142,"in_reply_to":"1a4dcd0f_f98e2ecf","updated":"2015-08-11 15:41:26.000000000","message":"Right now when we do a migration we create a destination/temporary volume on the destination host/backend and when the data is finished copying we copy DB data from the destination to the source DB entry so we can maintain the same ID we were using before but now pointing to the new backend.\n\nThis means that we end up with 2 exact entries in the DB (except for the ID) and we completely lose track of the original volume information.\n\nIn order to fix the 2 bugs referenced in the commit message we need to keep track of the original information.  This way we still keep a reference to the new volume and the old volume and each one has its own entry in the DB.","commit_id":"3624eab1df82a7881b3e5485d458e0bc1c8e7595"},{"author":{"_account_id":12608,"name":"Curt Bruns","email":"curt.e.bruns@intel.com","username":"cebruns"},"change_message_id":"117bd6693b6e59d663bc55c8b3dafcbb0afe0e74","unresolved":false,"context_lines":[{"line_number":1143,"context_line":"    session \u003d get_session()"},{"line_number":1144,"context_line":"    with session.begin():"},{"line_number":1145,"context_line":"        src_volume_ref \u003d _volume_get(context, src_vol_id, session\u003dsession)"},{"line_number":1146,"context_line":"        src_original_data \u003d dict(src_volume_ref.iteritems())"},{"line_number":1147,"context_line":"        dest_volume_ref \u003d _volume_get(context, dest_vol_id, session\u003dsession)"},{"line_number":1148,"context_line":""},{"line_number":1149,"context_line":"        # NOTE(rpodolyaka): we should copy only column values, while model"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a4dcd0f_bc951889","line":1146,"updated":"2015-08-14 21:42:59.000000000","message":"This only performs a shallow copy, which I think is fine in this case, just noting it.","commit_id":"3624eab1df82a7881b3e5485d458e0bc1c8e7595"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"99421f42e6e7dd250daa898d91c93cfce740d6fc","unresolved":false,"context_lines":[{"line_number":1143,"context_line":"    session \u003d get_session()"},{"line_number":1144,"context_line":"    with session.begin():"},{"line_number":1145,"context_line":"        src_volume_ref \u003d _volume_get(context, src_vol_id, session\u003dsession)"},{"line_number":1146,"context_line":"        src_original_data \u003d dict(src_volume_ref.iteritems())"},{"line_number":1147,"context_line":"        dest_volume_ref \u003d _volume_get(context, dest_vol_id, session\u003dsession)"},{"line_number":1148,"context_line":""},{"line_number":1149,"context_line":"        # NOTE(rpodolyaka): we should copy only column values, while model"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa1b9901_95ddd372","line":1146,"in_reply_to":"1a4dcd0f_bc951889","updated":"2015-08-17 14:45:21.000000000","message":"Yes, shallow copy is all we need","commit_id":"3624eab1df82a7881b3e5485d458e0bc1c8e7595"},{"author":{"_account_id":2759,"name":"Huang Zhiteng","email":"winston.d@gmail.com","username":"zhiteng-huang"},"change_message_id":"c2f140b71c038cf0b5d5406055333c856c4fb2c7","unresolved":false,"context_lines":[{"line_number":1139,"context_line":""},{"line_number":1140,"context_line":"@require_admin_context"},{"line_number":1141,"context_line":"def finish_volume_migration(context, src_vol_id, dest_vol_id):"},{"line_number":1142,"context_line":"    \"\"\"Swap almost all columns between dest and source.\"\"\""},{"line_number":1143,"context_line":"    session \u003d get_session()"},{"line_number":1144,"context_line":"    with session.begin():"},{"line_number":1145,"context_line":"        src_volume_ref \u003d _volume_get(context, src_vol_id, session\u003dsession)"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa1b9901_6d660fef","line":1142,"updated":"2015-08-18 08:46:06.000000000","message":"At least document the side effort: dest volume status and migration status would be changed to \u0027deleting\u0027.","commit_id":"e015ba76600606fc58ebe9b70996bd30e787ff49"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"e83b0c85e8309f85ba557f2a8752833466cafe1e","unresolved":false,"context_lines":[{"line_number":1139,"context_line":""},{"line_number":1140,"context_line":"@require_admin_context"},{"line_number":1141,"context_line":"def finish_volume_migration(context, src_vol_id, dest_vol_id):"},{"line_number":1142,"context_line":"    \"\"\"Swap almost all columns between dest and source.\"\"\""},{"line_number":1143,"context_line":"    session \u003d get_session()"},{"line_number":1144,"context_line":"    with session.begin():"},{"line_number":1145,"context_line":"        src_volume_ref \u003d _volume_get(context, src_vol_id, session\u003dsession)"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa1b9901_79ca51a3","line":1142,"in_reply_to":"fa1b9901_6d660fef","updated":"2015-08-18 10:01:10.000000000","message":"Done","commit_id":"e015ba76600606fc58ebe9b70996bd30e787ff49"}],"cinder/volume/api.py":[{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"21c71b678266e8524b2445dd74d6452fbe275558","unresolved":false,"context_lines":[{"line_number":355,"context_line":"                      \u0027vol_status\u0027: volume[\u0027status\u0027]})"},{"line_number":356,"context_line":"            raise exception.InvalidVolume(reason\u003dmsg)"},{"line_number":357,"context_line":""},{"line_number":358,"context_line":"        if volume[\u0027migration_status\u0027] not in (None, \u0027deleting\u0027):"},{"line_number":359,"context_line":"            # Volume is migrating, wait until done"},{"line_number":360,"context_line":"            LOG.info(_LI(\u0027Unable to delete volume: %s, \u0027"},{"line_number":361,"context_line":"                         \u0027volume is currently migrating.\u0027), volume[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a4dcd0f_f9d62ef0","line":358,"updated":"2015-08-11 07:52:35.000000000","message":"I am not sure if this is necessary to add, because \u0027deleting\u0027 only happens on the \u0027new_volume\u0027 aka dest volume, which does not have the chance to become the REAL\" volume counted for the \u0027REAL\u0027 user and tenant.\nJust have this concern here.","commit_id":"3624eab1df82a7881b3e5485d458e0bc1c8e7595"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"3a83c6bb98a67d923d82408c437fdefb2c252817","unresolved":false,"context_lines":[{"line_number":355,"context_line":"                      \u0027vol_status\u0027: volume[\u0027status\u0027]})"},{"line_number":356,"context_line":"            raise exception.InvalidVolume(reason\u003dmsg)"},{"line_number":357,"context_line":""},{"line_number":358,"context_line":"        if volume[\u0027migration_status\u0027] not in (None, \u0027deleting\u0027):"},{"line_number":359,"context_line":"            # Volume is migrating, wait until done"},{"line_number":360,"context_line":"            LOG.info(_LI(\u0027Unable to delete volume: %s, \u0027"},{"line_number":361,"context_line":"                         \u0027volume is currently migrating.\u0027), volume[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a4dcd0f_f727fb0f","line":358,"in_reply_to":"1a4dcd0f_f9d62ef0","updated":"2015-08-11 15:41:26.000000000","message":"We have introduced a new \u0027deleting\u0027 value for migration status for the original source volume (which after swapping DB contents is now pointed by the new/temporary DB entry) so if async delete fails and we want to retry deleting it or force-delete the volume we need to explicitly allow this.","commit_id":"3624eab1df82a7881b3e5485d458e0bc1c8e7595"}],"cinder/volume/manager.py":[{"author":{"_account_id":2759,"name":"Huang Zhiteng","email":"winston.d@gmail.com","username":"zhiteng-huang"},"change_message_id":"ec6efbede98d76a620786290e0a5de4b2cb82415","unresolved":false,"context_lines":[{"line_number":1394,"context_line":"        # Swap src and dest DB records so we can continue using the src id and"},{"line_number":1395,"context_line":"        # asynchronously delete the destination id"},{"line_number":1396,"context_line":"        __, updated_new \u003d self.db.finish_volume_migration("},{"line_number":1397,"context_line":"            ctxt, volume_id, new_volume_id)"},{"line_number":1398,"context_line":"        if orig_volume_status \u003d\u003d \u0027in-use\u0027:"},{"line_number":1399,"context_line":"            updates \u003d {\u0027migration_status\u0027: \u0027completing\u0027,"},{"line_number":1400,"context_line":"                       \u0027status\u0027: orig_volume_status}"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a4dcd0f_b91b0b80","line":1397,"updated":"2015-08-14 09:38:53.000000000","message":"Can we explicitly set src volume status to \u0027deleting\u0027 here in manager instead of doing in it in finish_volume_migration()?","commit_id":"3624eab1df82a7881b3e5485d458e0bc1c8e7595"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"8ee071878e3a593fa301d5a4fecf75b624e64efa","unresolved":false,"context_lines":[{"line_number":1394,"context_line":"        # Swap src and dest DB records so we can continue using the src id and"},{"line_number":1395,"context_line":"        # asynchronously delete the destination id"},{"line_number":1396,"context_line":"        __, updated_new \u003d self.db.finish_volume_migration("},{"line_number":1397,"context_line":"            ctxt, volume_id, new_volume_id)"},{"line_number":1398,"context_line":"        if orig_volume_status \u003d\u003d \u0027in-use\u0027:"},{"line_number":1399,"context_line":"            updates \u003d {\u0027migration_status\u0027: \u0027completing\u0027,"},{"line_number":1400,"context_line":"                       \u0027status\u0027: orig_volume_status}"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a4dcd0f_b6df550b","line":1397,"in_reply_to":"1a4dcd0f_0b22e401","updated":"2015-08-17 07:17:50.000000000","message":"I gradually understand why you add the volume info swap.\nThere is test case for finish_volume_migration. I think you need to change the test case for sure.","commit_id":"3624eab1df82a7881b3e5485d458e0bc1c8e7595"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"99421f42e6e7dd250daa898d91c93cfce740d6fc","unresolved":false,"context_lines":[{"line_number":1394,"context_line":"        # Swap src and dest DB records so we can continue using the src id and"},{"line_number":1395,"context_line":"        # asynchronously delete the destination id"},{"line_number":1396,"context_line":"        __, updated_new \u003d self.db.finish_volume_migration("},{"line_number":1397,"context_line":"            ctxt, volume_id, new_volume_id)"},{"line_number":1398,"context_line":"        if orig_volume_status \u003d\u003d \u0027in-use\u0027:"},{"line_number":1399,"context_line":"            updates \u003d {\u0027migration_status\u0027: \u0027completing\u0027,"},{"line_number":1400,"context_line":"                       \u0027status\u0027: orig_volume_status}"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa1b9901_55fc1b3e","line":1397,"in_reply_to":"1a4dcd0f_b6df550b","updated":"2015-08-17 14:45:21.000000000","message":"Thanks!  I missed that one.","commit_id":"3624eab1df82a7881b3e5485d458e0bc1c8e7595"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"f6b91fb85ef4e45e16c1510401abaedb91b03cc0","unresolved":false,"context_lines":[{"line_number":1394,"context_line":"        # Swap src and dest DB records so we can continue using the src id and"},{"line_number":1395,"context_line":"        # asynchronously delete the destination id"},{"line_number":1396,"context_line":"        __, updated_new \u003d self.db.finish_volume_migration("},{"line_number":1397,"context_line":"            ctxt, volume_id, new_volume_id)"},{"line_number":1398,"context_line":"        if orig_volume_status \u003d\u003d \u0027in-use\u0027:"},{"line_number":1399,"context_line":"            updates \u003d {\u0027migration_status\u0027: \u0027completing\u0027,"},{"line_number":1400,"context_line":"                       \u0027status\u0027: orig_volume_status}"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a4dcd0f_0b22e401","line":1397,"in_reply_to":"1a4dcd0f_b91b0b80","updated":"2015-08-14 10:31:51.000000000","message":"I\u0027d rather not, because then migration_status and status will be out of sync when we change migration_status to deleting in finish_volume_migration.\n\nAnd changing migration_status here to fix that out of sync situation would not be correct.","commit_id":"3624eab1df82a7881b3e5485d458e0bc1c8e7595"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"21c71b678266e8524b2445dd74d6452fbe275558","unresolved":false,"context_lines":[{"line_number":1414,"context_line":"        # Asynchronous deletion of the source volume in the back-end (now"},{"line_number":1415,"context_line":"        # pointed by the target volume id)"},{"line_number":1416,"context_line":"        try:"},{"line_number":1417,"context_line":"            rpcapi.delete_volume(ctxt, updated_new)"},{"line_number":1418,"context_line":"        except Exception as ex:"},{"line_number":1419,"context_line":"            LOG.error(_LE(\u0027Failed to request async delete of migration source \u0027"},{"line_number":1420,"context_line":"                          \u0027vol %(vol)s: %(err)s\u0027),"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a4dcd0f_b48f4d22","line":1417,"updated":"2015-08-11 07:52:35.000000000","message":"delete_volume is using cast. I do not think you can catch the exception here even if it fails on deleting the back-end.\n\nupdated_new keeps the information of the source vol, and this method is running on the source node. Why not just use self.delete_volume??","commit_id":"3624eab1df82a7881b3e5485d458e0bc1c8e7595"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"3a83c6bb98a67d923d82408c437fdefb2c252817","unresolved":false,"context_lines":[{"line_number":1414,"context_line":"        # Asynchronous deletion of the source volume in the back-end (now"},{"line_number":1415,"context_line":"        # pointed by the target volume id)"},{"line_number":1416,"context_line":"        try:"},{"line_number":1417,"context_line":"            rpcapi.delete_volume(ctxt, updated_new)"},{"line_number":1418,"context_line":"        except Exception as ex:"},{"line_number":1419,"context_line":"            LOG.error(_LE(\u0027Failed to request async delete of migration source \u0027"},{"line_number":1420,"context_line":"                          \u0027vol %(vol)s: %(err)s\u0027),"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a4dcd0f_17014738","line":1417,"in_reply_to":"1a4dcd0f_b48f4d22","updated":"2015-08-11 15:41:26.000000000","message":"This is not trying to catch the error of deletion but an error on the rpc call itself so that we don\u0027t fail the migration even if that happens.\n\nBecause using self.delete_volume is what we had before and is what is creating one of the 2 bugs this patch fixes.  When delete_volume takes more than rpc timeout (1 minute by default) the migration of in-use volumes will stall/fail.","commit_id":"3624eab1df82a7881b3e5485d458e0bc1c8e7595"},{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"a4931b753a6b44a653b40ee7d29aad220589e93e","unresolved":false,"context_lines":[{"line_number":2697,"context_line":"            self.db.volume_update(ctxt.elevated(), volume[\u0027id\u0027],"},{"line_number":2698,"context_line":"                                  model_update)"},{"line_number":2699,"context_line":"            # Swap keys that were changed in the source so we keep their values"},{"line_number":2700,"context_line":"            # in the temporaty volume\u0027s DB record."},{"line_number":2701,"context_line":"            model_update_new \u003d {key: volume[key]"},{"line_number":2702,"context_line":"                                for key in model_update.iterkeys()}"},{"line_number":2703,"context_line":"            self.db.volume_update(ctxt.elevated(), new_volume[\u0027id\u0027],"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a4dcd0f_42be5d00","line":2700,"updated":"2015-08-10 23:57:34.000000000","message":"Typo: temporary.","commit_id":"3624eab1df82a7881b3e5485d458e0bc1c8e7595"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"3a83c6bb98a67d923d82408c437fdefb2c252817","unresolved":false,"context_lines":[{"line_number":2697,"context_line":"            self.db.volume_update(ctxt.elevated(), volume[\u0027id\u0027],"},{"line_number":2698,"context_line":"                                  model_update)"},{"line_number":2699,"context_line":"            # Swap keys that were changed in the source so we keep their values"},{"line_number":2700,"context_line":"            # in the temporaty volume\u0027s DB record."},{"line_number":2701,"context_line":"            model_update_new \u003d {key: volume[key]"},{"line_number":2702,"context_line":"                                for key in model_update.iterkeys()}"},{"line_number":2703,"context_line":"            self.db.volume_update(ctxt.elevated(), new_volume[\u0027id\u0027],"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a4dcd0f_f78a9bbe","line":2700,"in_reply_to":"1a4dcd0f_42be5d00","updated":"2015-08-11 15:41:26.000000000","message":"I\u0027ll fix it if I have to submit a new patch.","commit_id":"3624eab1df82a7881b3e5485d458e0bc1c8e7595"}]}
