)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"a6b7e477ee5663f2c34f968c2f84f3413f322188","unresolved":false,"context_lines":[{"line_number":16,"context_line":"due to timeout, we need to terminate the connection. Otherwise, the dest"},{"line_number":17,"context_line":"volume will not be released and successfully deleted."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"closes-bug: #1398177"},{"line_number":20,"context_line":"Change-Id: I5bc75bd7652841fddd66481ee001fb682168215c"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":40,"id":"1a6ced46_04bceb68","line":19,"updated":"2015-03-23 17:25:57.000000000","message":"Closes-Bug","commit_id":"b2528e8b95b908dab1254ddf7276e90895df1bad"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"a99f2e2e92486806ebe36a4abccaf60a468d5ea4","unresolved":false,"context_lines":[{"line_number":16,"context_line":"due to timeout, we need to terminate the connection. Otherwise, the dest"},{"line_number":17,"context_line":"volume will not be released and successfully deleted."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"closes-bug: #1398177"},{"line_number":20,"context_line":"Change-Id: I5bc75bd7652841fddd66481ee001fb682168215c"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":40,"id":"1a6ced46_ce965b94","line":19,"in_reply_to":"1a6ced46_04bceb68","updated":"2015-03-24 05:50:33.000000000","message":"Done","commit_id":"b2528e8b95b908dab1254ddf7276e90895df1bad"}],"cinder/tests/test_volume.py":[{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"99e13464396dc3789b820c54d92734c6faf9d0be","unresolved":false,"context_lines":[{"line_number":3289,"context_line":""},{"line_number":3290,"context_line":"            # Exception case at the creation of the new temporary volume"},{"line_number":3291,"context_line":"            mock_create_volume.side_effect \u003d fake_create_volume"},{"line_number":3292,"context_line":"            #clean_temporary_volume.return_value \u003d fake_clean_temporary_volume"},{"line_number":3293,"context_line":"            volume \u003d tests_utils.create_volume(self.context, size\u003d0,"},{"line_number":3294,"context_line":"                                               host\u003dCONF.host)"},{"line_number":3295,"context_line":"            host_obj \u003d {\u0027host\u0027: \u0027newhost\u0027, \u0027capabilities\u0027: {}}"}],"source_content_type":"text/x-python","patch_set":21,"id":"9a80dd14_19722ead","line":3292,"updated":"2015-03-06 14:05:38.000000000","message":"Please remove commented out code.","commit_id":"5635e4181a8f9a7f1e8f3f5d2a500ce802bd7ae8"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"c6f39171bf4c1b4d3008f3636144e5dcf5a40b4f","unresolved":false,"context_lines":[{"line_number":3289,"context_line":""},{"line_number":3290,"context_line":"            # Exception case at the creation of the new temporary volume"},{"line_number":3291,"context_line":"            mock_create_volume.side_effect \u003d fake_create_volume"},{"line_number":3292,"context_line":"            #clean_temporary_volume.return_value \u003d fake_clean_temporary_volume"},{"line_number":3293,"context_line":"            volume \u003d tests_utils.create_volume(self.context, size\u003d0,"},{"line_number":3294,"context_line":"                                               host\u003dCONF.host)"},{"line_number":3295,"context_line":"            host_obj \u003d {\u0027host\u0027: \u0027newhost\u0027, \u0027capabilities\u0027: {}}"}],"source_content_type":"text/x-python","patch_set":21,"id":"9a80dd14_31e35da5","line":3292,"in_reply_to":"9a80dd14_19722ead","updated":"2015-03-09 03:06:15.000000000","message":"Done","commit_id":"5635e4181a8f9a7f1e8f3f5d2a500ce802bd7ae8"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"99e13464396dc3789b820c54d92734c6faf9d0be","unresolved":false,"context_lines":[{"line_number":3311,"context_line":"            db.volume_update(ctxt, volume[\u0027id\u0027],"},{"line_number":3312,"context_line":"                             {\u0027status\u0027: \u0027creating\u0027})"},{"line_number":3313,"context_line":""},{"line_number":3314,"context_line":"        with mock.patch.object(self.volume.driver, \u0027migrate_volume\u0027),\\"},{"line_number":3315,"context_line":"                mock.patch.object(volume_rpcapi.VolumeAPI, \u0027create_volume\u0027)\\"},{"line_number":3316,"context_line":"                as mock_create_volume,\\"},{"line_number":3317,"context_line":"                mock.patch.object(self.volume, \u0027_clean_temporary_volume\u0027) as \\"}],"source_content_type":"text/x-python","patch_set":21,"id":"9a80dd14_59dca673","line":3314,"updated":"2015-03-06 14:05:38.000000000","message":"Minor nit, but can you put a space between the , and the \\ ? For all of the line continuations in this with block it would nice to be consistent and make this as readable as possible until we can figure out how to makes things happy with this in ().","commit_id":"5635e4181a8f9a7f1e8f3f5d2a500ce802bd7ae8"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"c6f39171bf4c1b4d3008f3636144e5dcf5a40b4f","unresolved":false,"context_lines":[{"line_number":3311,"context_line":"            db.volume_update(ctxt, volume[\u0027id\u0027],"},{"line_number":3312,"context_line":"                             {\u0027status\u0027: \u0027creating\u0027})"},{"line_number":3313,"context_line":""},{"line_number":3314,"context_line":"        with mock.patch.object(self.volume.driver, \u0027migrate_volume\u0027),\\"},{"line_number":3315,"context_line":"                mock.patch.object(volume_rpcapi.VolumeAPI, \u0027create_volume\u0027)\\"},{"line_number":3316,"context_line":"                as mock_create_volume,\\"},{"line_number":3317,"context_line":"                mock.patch.object(self.volume, \u0027_clean_temporary_volume\u0027) as \\"}],"source_content_type":"text/x-python","patch_set":21,"id":"9a80dd14_51e0d195","line":3314,"in_reply_to":"9a80dd14_59dca673","updated":"2015-03-09 03:06:15.000000000","message":"Done","commit_id":"5635e4181a8f9a7f1e8f3f5d2a500ce802bd7ae8"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"a6b7e477ee5663f2c34f968c2f84f3413f322188","unresolved":false,"context_lines":[{"line_number":3633,"context_line":"            volume \u003d db.volume_get(context.get_admin_context(), volume[\u0027id\u0027])"},{"line_number":3634,"context_line":"            self.assertIsNone(volume[\u0027migration_status\u0027])"},{"line_number":3635,"context_line":"            self.assertEqual(\u0027available\u0027, volume[\u0027status\u0027])"},{"line_number":3636,"context_line":""},{"line_number":3637,"context_line":"    def test_migrate_volume_generic_create_volume_error(self):"},{"line_number":3638,"context_line":"        def fake_create_volume(ctxt, volume, host, req_spec, filters,"},{"line_number":3639,"context_line":"                               allow_reschedule\u003dTrue):"}],"source_content_type":"text/x-python","patch_set":40,"id":"1a6ced46_1f3a72cd","line":3636,"updated":"2015-03-23 17:25:57.000000000","message":"Please, add separate test for _clean_temporary_volume method","commit_id":"b2528e8b95b908dab1254ddf7276e90895df1bad"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"a6b7e477ee5663f2c34f968c2f84f3413f322188","unresolved":false,"context_lines":[{"line_number":3663,"context_line":"            self.assertTrue(clean_temporary_volume.called)"},{"line_number":3664,"context_line":""},{"line_number":3665,"context_line":"    def test_migrate_volume_generic_timeout_error(self):"},{"line_number":3666,"context_line":"        CONF.migration_create_volume_timeout_secs \u003d 2"},{"line_number":3667,"context_line":""},{"line_number":3668,"context_line":"        def fake_create_volume(ctxt, volume, host, req_spec, filters,"},{"line_number":3669,"context_line":"                               allow_reschedule\u003dTrue):"}],"source_content_type":"text/x-python","patch_set":40,"id":"1a6ced46_64aaff1a","line":3666,"updated":"2015-03-23 17:25:57.000000000","message":"Please, don\u0027t change CONF from tests. You should use mock for it","commit_id":"b2528e8b95b908dab1254ddf7276e90895df1bad"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"a379665d5c4ba944e2f573e54252cd0efb093293","unresolved":false,"context_lines":[{"line_number":3663,"context_line":"            self.assertTrue(clean_temporary_volume.called)"},{"line_number":3664,"context_line":""},{"line_number":3665,"context_line":"    def test_migrate_volume_generic_timeout_error(self):"},{"line_number":3666,"context_line":"        CONF.migration_create_volume_timeout_secs \u003d 2"},{"line_number":3667,"context_line":""},{"line_number":3668,"context_line":"        def fake_create_volume(ctxt, volume, host, req_spec, filters,"},{"line_number":3669,"context_line":"                               allow_reschedule\u003dTrue):"}],"source_content_type":"text/x-python","patch_set":40,"id":"1a6ced46_4e4eeb70","line":3666,"in_reply_to":"1a6ced46_64aaff1a","updated":"2015-03-24 04:00:49.000000000","message":"Ivan, I found out some other tests were using CONF.set_override(). Is it a proper approach as well?\nCould you some me some instructions on how to mock CONF? Because I tried mock.patch.object(CONF, \u0027migration_create_volume_timeout_secs\u0027\u0027) and mock.patch(CONF.migration_create_volume_timeout_secs), and they both failed.\nThx.","commit_id":"b2528e8b95b908dab1254ddf7276e90895df1bad"},{"author":{"_account_id":13900,"name":"Accela Zhao","email":"accelazh@gmail.com","username":"accelazh"},"change_message_id":"19c83c77f96e5cf25befbd7c6c6ebe4cd52aed89","unresolved":false,"context_lines":[{"line_number":3746,"context_line":"            self.assertIsNone(volume[\u0027migration_status\u0027])"},{"line_number":3747,"context_line":"            self.assertEqual(\u0027available\u0027, volume[\u0027status\u0027])"},{"line_number":3748,"context_line":""},{"line_number":3749,"context_line":"    def test__clean_temporary_volume(self):"},{"line_number":3750,"context_line":"        def fake_delete_volume(ctxt, volume):"},{"line_number":3751,"context_line":"            db.volume_destroy(ctxt, volume[\u0027id\u0027])"},{"line_number":3752,"context_line":""}],"source_content_type":"text/x-python","patch_set":48,"id":"baa041b7_7e6e7f30","line":3749,"updated":"2015-04-03 13:30:18.000000000","message":"Why did you use two under-scores?","commit_id":"304771544f75fbbeef9deb5a5ff0302eb97156b5"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"2de803e333a9b2a4d0c08c4019698225373371fc","unresolved":false,"context_lines":[{"line_number":3746,"context_line":"            self.assertIsNone(volume[\u0027migration_status\u0027])"},{"line_number":3747,"context_line":"            self.assertEqual(\u0027available\u0027, volume[\u0027status\u0027])"},{"line_number":3748,"context_line":""},{"line_number":3749,"context_line":"    def test__clean_temporary_volume(self):"},{"line_number":3750,"context_line":"        def fake_delete_volume(ctxt, volume):"},{"line_number":3751,"context_line":"            db.volume_destroy(ctxt, volume[\u0027id\u0027])"},{"line_number":3752,"context_line":""}],"source_content_type":"text/x-python","patch_set":48,"id":"baa041b7_c006afcd","line":3749,"in_reply_to":"baa041b7_23e9f014","updated":"2015-04-05 07:59:30.000000000","message":"_clean_temporary_volume is the method name.","commit_id":"304771544f75fbbeef9deb5a5ff0302eb97156b5"},{"author":{"_account_id":10263,"name":"Vilobh Meshram","email":"vilobhmm@yahoo-inc.com","username":"vilobhmm"},"change_message_id":"eed221db4cf8d80152ece1a1e651ce5581aa8997","unresolved":false,"context_lines":[{"line_number":3746,"context_line":"            self.assertIsNone(volume[\u0027migration_status\u0027])"},{"line_number":3747,"context_line":"            self.assertEqual(\u0027available\u0027, volume[\u0027status\u0027])"},{"line_number":3748,"context_line":""},{"line_number":3749,"context_line":"    def test__clean_temporary_volume(self):"},{"line_number":3750,"context_line":"        def fake_delete_volume(ctxt, volume):"},{"line_number":3751,"context_line":"            db.volume_destroy(ctxt, volume[\u0027id\u0027])"},{"line_number":3752,"context_line":""}],"source_content_type":"text/x-python","patch_set":48,"id":"baa041b7_23e9f014","line":3749,"in_reply_to":"baa041b7_7e6e7f30","updated":"2015-04-03 21:43:13.000000000","message":"Maybe seems a typo; again a good catch.","commit_id":"304771544f75fbbeef9deb5a5ff0302eb97156b5"},{"author":{"_account_id":6491,"name":"xing-yang","email":"xingyang105@gmail.com","username":"xing-yang"},"change_message_id":"3eeef368fb41d00037045dcf965c29fbb1e7dd25","unresolved":false,"context_lines":[{"line_number":3746,"context_line":"            self.assertIsNone(volume[\u0027migration_status\u0027])"},{"line_number":3747,"context_line":"            self.assertEqual(\u0027available\u0027, volume[\u0027status\u0027])"},{"line_number":3748,"context_line":""},{"line_number":3749,"context_line":"    def test__clean_temporary_volume(self):"},{"line_number":3750,"context_line":"        def fake_delete_volume(ctxt, volume):"},{"line_number":3751,"context_line":"            db.volume_destroy(ctxt, volume[\u0027id\u0027])"},{"line_number":3752,"context_line":""}],"source_content_type":"text/x-python","patch_set":48,"id":"baa041b7_55ac989f","line":3749,"in_reply_to":"baa041b7_c006afcd","updated":"2015-04-06 02:35:57.000000000","message":"You have two \u0027_\u0027 between \u0027test\u0027 and \u0027clean\u0027. Can you remove the extra one?","commit_id":"304771544f75fbbeef9deb5a5ff0302eb97156b5"}],"cinder/volume/driver.py":[{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"9f9b50079fd1a49ca7e905f67c21632b3dd2c835","unresolved":false,"context_lines":[{"line_number":640,"context_line":"            try:"},{"line_number":641,"context_line":"                conn \u003d rpcapi.initialize_connection(context, volume,"},{"line_number":642,"context_line":"                                                    properties)"},{"line_number":643,"context_line":"            except Exception as err:"},{"line_number":644,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":645,"context_line":"                    # It is possible that initialize_connection fails due to"},{"line_number":646,"context_line":"                    # timeout. In fact, the volume is already attached after"}],"source_content_type":"text/x-python","patch_set":29,"id":"9a80dd14_cd1102b1","line":643,"updated":"2015-03-11 15:32:59.000000000","message":"err variable is never used. Change both of these to just \"exception Exception:\".\n\nIt might be good to catch something more specific than Exception, but since you are just doing this to be able to log and cleanup from the error, then passing it back up, I think this is probably fine.","commit_id":"546628483231865fee62172731daaf3232410775"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"71415cab0a151cee54bf250f917d39bfae2f2ad4","unresolved":false,"context_lines":[{"line_number":640,"context_line":"            try:"},{"line_number":641,"context_line":"                conn \u003d rpcapi.initialize_connection(context, volume,"},{"line_number":642,"context_line":"                                                    properties)"},{"line_number":643,"context_line":"            except Exception as err:"},{"line_number":644,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":645,"context_line":"                    # It is possible that initialize_connection fails due to"},{"line_number":646,"context_line":"                    # timeout. In fact, the volume is already attached after"}],"source_content_type":"text/x-python","patch_set":29,"id":"9a80dd14_be4a1625","line":643,"in_reply_to":"9a80dd14_cd1102b1","updated":"2015-03-12 04:07:33.000000000","message":"Sean, this is also something I have been considering before. However, the exception is a bit difficult to get specific. I encountered a timeout error for initialize_connection, the code raised an oslo exception, which is hard for me to track what kind of exceptions this line of code may raise. Actually I need to try a connection termination for all the exceptions, so I just use the general exception instead","commit_id":"546628483231865fee62172731daaf3232410775"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"9f9b50079fd1a49ca7e905f67c21632b3dd2c835","unresolved":false,"context_lines":[{"line_number":652,"context_line":"                    except Exception as err:"},{"line_number":653,"context_line":"                        LOG.warn(_LW(\"Failed terminating the connection of \""},{"line_number":654,"context_line":"                                     \"volume %(volume_id)s, but it is \""},{"line_number":655,"context_line":"                                     \"acceptable.\") %"},{"line_number":656,"context_line":"                                 {\u0027volume_id\u0027: volume[\u0027id\u0027]})"},{"line_number":657,"context_line":"        else:"},{"line_number":658,"context_line":"            # Call local driver\u0027s create_export and initialize_connection."}],"source_content_type":"text/x-python","patch_set":29,"id":"9a80dd14_adee8eb2","line":655,"updated":"2015-03-11 15:32:59.000000000","message":"Please use , instead of % to make volume_id dict a parameter that gets passed in to the logger rather than formatting the string ahead of time.","commit_id":"546628483231865fee62172731daaf3232410775"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"71415cab0a151cee54bf250f917d39bfae2f2ad4","unresolved":false,"context_lines":[{"line_number":652,"context_line":"                    except Exception as err:"},{"line_number":653,"context_line":"                        LOG.warn(_LW(\"Failed terminating the connection of \""},{"line_number":654,"context_line":"                                     \"volume %(volume_id)s, but it is \""},{"line_number":655,"context_line":"                                     \"acceptable.\") %"},{"line_number":656,"context_line":"                                 {\u0027volume_id\u0027: volume[\u0027id\u0027]})"},{"line_number":657,"context_line":"        else:"},{"line_number":658,"context_line":"            # Call local driver\u0027s create_export and initialize_connection."}],"source_content_type":"text/x-python","patch_set":29,"id":"9a80dd14_1e8b2269","line":655,"in_reply_to":"9a80dd14_adee8eb2","updated":"2015-03-12 04:07:33.000000000","message":"Done","commit_id":"546628483231865fee62172731daaf3232410775"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"77e3d9ae0e465ee7fe1b930214ad49c68b6208d9","unresolved":false,"context_lines":[{"line_number":649,"context_line":"                    try:"},{"line_number":650,"context_line":"                        rpcapi.terminate_connection(context, volume,"},{"line_number":651,"context_line":"                                                    properties, force\u003dTrue)"},{"line_number":652,"context_line":"                    except Exception as err:"},{"line_number":653,"context_line":"                        LOG.warn(_LW(\"Failed terminating the connection of \""},{"line_number":654,"context_line":"                                     \"volume %(volume_id)s, but it is \""},{"line_number":655,"context_line":"                                     \"acceptable.\"),"}],"source_content_type":"text/x-python","patch_set":32,"id":"9a80dd14_92fe0cad","line":652,"updated":"2015-03-12 15:43:40.000000000","message":"Still have an unused variable here, but no harm. I would clean that out if you do another patch, but I wouldn\u0027t hold this up for it.","commit_id":"daa5da51a25274173f8add3c2d2260661c8b3de5"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"14df1d98c474722bf8999aa25ea43a075c83b210","unresolved":false,"context_lines":[{"line_number":649,"context_line":"                    try:"},{"line_number":650,"context_line":"                        rpcapi.terminate_connection(context, volume,"},{"line_number":651,"context_line":"                                                    properties, force\u003dTrue)"},{"line_number":652,"context_line":"                    except Exception as err:"},{"line_number":653,"context_line":"                        LOG.warn(_LW(\"Failed terminating the connection of \""},{"line_number":654,"context_line":"                                     \"volume %(volume_id)s, but it is \""},{"line_number":655,"context_line":"                                     \"acceptable.\"),"}],"source_content_type":"text/x-python","patch_set":32,"id":"9a80dd14_5d69e9bc","line":652,"in_reply_to":"9a80dd14_92fe0cad","updated":"2015-03-13 03:30:20.000000000","message":"Sorry, Need to drop it.","commit_id":"daa5da51a25274173f8add3c2d2260661c8b3de5"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"a6b7e477ee5663f2c34f968c2f84f3413f322188","unresolved":false,"context_lines":[{"line_number":652,"context_line":"                        rpcapi.terminate_connection(context, volume,"},{"line_number":653,"context_line":"                                                    properties, force\u003dTrue)"},{"line_number":654,"context_line":"                    except Exception:"},{"line_number":655,"context_line":"                        LOG.warn(_LW(\"Failed terminating the connection of \""},{"line_number":656,"context_line":"                                     \"volume %(volume_id)s, but it is \""},{"line_number":657,"context_line":"                                     \"acceptable.\"),"},{"line_number":658,"context_line":"                                 {\u0027volume_id\u0027: volume[\u0027id\u0027]})"}],"source_content_type":"text/x-python","patch_set":40,"id":"1a6ced46_044a2b80","line":655,"updated":"2015-03-23 17:25:57.000000000","message":"Could we handle more specific exception here?","commit_id":"b2528e8b95b908dab1254ddf7276e90895df1bad"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"a379665d5c4ba944e2f573e54252cd0efb093293","unresolved":false,"context_lines":[{"line_number":652,"context_line":"                        rpcapi.terminate_connection(context, volume,"},{"line_number":653,"context_line":"                                                    properties, force\u003dTrue)"},{"line_number":654,"context_line":"                    except Exception:"},{"line_number":655,"context_line":"                        LOG.warn(_LW(\"Failed terminating the connection of \""},{"line_number":656,"context_line":"                                     \"volume %(volume_id)s, but it is \""},{"line_number":657,"context_line":"                                     \"acceptable.\"),"},{"line_number":658,"context_line":"                                 {\u0027volume_id\u0027: volume[\u0027id\u0027]})"}],"source_content_type":"text/x-python","patch_set":40,"id":"1a6ced46_6e864767","line":655,"in_reply_to":"1a6ced46_044a2b80","updated":"2015-03-24 04:00:49.000000000","message":"Ivan, I used to give a comment for this line on Patch set 29, explaining why I leave the code like this.\nFor the exception here, it is really hard to expect what specific exceptions we will encounter here. The reason why I added the exception catch here is because I used to meet a timeout error for initialize_connection, the code raised an oslo exception, which is unpredictable for me to track what kind of exceptions this line of code may raise. I JUST met the timeout once, but I am not sure if there can be any other exceptions raised here.","commit_id":"b2528e8b95b908dab1254ddf7276e90895df1bad"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"f684f945dc1a49238450ae97a0b62ad10f1620a3","unresolved":false,"context_lines":[{"line_number":653,"context_line":"                        rpcapi.terminate_connection(context, volume,"},{"line_number":654,"context_line":"                                                    properties, force\u003dTrue)"},{"line_number":655,"context_line":"                    except Exception:"},{"line_number":656,"context_line":"                        LOG.warn(_LW(\"Failed terminating the connection of \""},{"line_number":657,"context_line":"                                     \"volume %(volume_id)s, but it is \""},{"line_number":658,"context_line":"                                     \"acceptable.\"),"},{"line_number":659,"context_line":"                                 {\u0027volume_id\u0027: volume[\u0027id\u0027]})"}],"source_content_type":"text/x-python","patch_set":46,"id":"baa041b7_17a41fa4","line":656,"updated":"2015-04-02 13:09:31.000000000","message":"LOG.warn is deprecated. Please use LOG.warning.","commit_id":"00d53d3d86e3d932c18544fe811fff4b357ec2d4"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"7ded54f91c5211170656baa7de384099617b6736","unresolved":false,"context_lines":[{"line_number":653,"context_line":"                        rpcapi.terminate_connection(context, volume,"},{"line_number":654,"context_line":"                                                    properties, force\u003dTrue)"},{"line_number":655,"context_line":"                    except Exception:"},{"line_number":656,"context_line":"                        LOG.warn(_LW(\"Failed terminating the connection of \""},{"line_number":657,"context_line":"                                     \"volume %(volume_id)s, but it is \""},{"line_number":658,"context_line":"                                     \"acceptable.\"),"},{"line_number":659,"context_line":"                                 {\u0027volume_id\u0027: volume[\u0027id\u0027]})"}],"source_content_type":"text/x-python","patch_set":46,"id":"baa041b7_12589431","line":656,"in_reply_to":"baa041b7_17a41fa4","updated":"2015-04-03 01:45:32.000000000","message":"Done","commit_id":"00d53d3d86e3d932c18544fe811fff4b357ec2d4"},{"author":{"_account_id":13900,"name":"Accela Zhao","email":"accelazh@gmail.com","username":"accelazh"},"change_message_id":"19c83c77f96e5cf25befbd7c6c6ebe4cd52aed89","unresolved":false,"context_lines":[{"line_number":650,"context_line":"                    # the timeout error is raised, so the connection worths"},{"line_number":651,"context_line":"                    # a try of terminating."},{"line_number":652,"context_line":"                    try:"},{"line_number":653,"context_line":"                        rpcapi.terminate_connection(context, volume,"},{"line_number":654,"context_line":"                                                    properties, force\u003dTrue)"},{"line_number":655,"context_line":"                    except Exception:"},{"line_number":656,"context_line":"                        LOG.warning(_LW(\"Failed terminating the connection \""}],"source_content_type":"text/x-python","patch_set":48,"id":"baa041b7_99cdddb9","line":653,"updated":"2015-04-03 13:30:18.000000000","message":"Do you think it is the vendor initialize_connection\u0027s responsibility to cleanup on failure or we should do it for them?","commit_id":"304771544f75fbbeef9deb5a5ff0302eb97156b5"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"2de803e333a9b2a4d0c08c4019698225373371fc","unresolved":false,"context_lines":[{"line_number":650,"context_line":"                    # the timeout error is raised, so the connection worths"},{"line_number":651,"context_line":"                    # a try of terminating."},{"line_number":652,"context_line":"                    try:"},{"line_number":653,"context_line":"                        rpcapi.terminate_connection(context, volume,"},{"line_number":654,"context_line":"                                                    properties, force\u003dTrue)"},{"line_number":655,"context_line":"                    except Exception:"},{"line_number":656,"context_line":"                        LOG.warning(_LW(\"Failed terminating the connection \""}],"source_content_type":"text/x-python","patch_set":48,"id":"baa041b7_c05fcfe3","line":653,"in_reply_to":"baa041b7_99cdddb9","updated":"2015-04-05 07:59:30.000000000","message":"As I explained in the comments and some of the previous patches, this is not possible to handle the failure in initialize_connection, because we have a timeout. For example, we wait for 5 seconds and no response coming back, then we raise a timeout exception and won\u0027t continue. But at the 6th second, the actual initialize_connection succeeded. For this case, we need yo try a terminate to cover this case.","commit_id":"304771544f75fbbeef9deb5a5ff0302eb97156b5"},{"author":{"_account_id":13900,"name":"Accela Zhao","email":"accelazh@gmail.com","username":"accelazh"},"change_message_id":"f9ea2ed24f5e44f6235b912cf0da238ebbd35227","unresolved":false,"context_lines":[{"line_number":650,"context_line":"                    # the timeout error is raised, so the connection worths"},{"line_number":651,"context_line":"                    # a try of terminating."},{"line_number":652,"context_line":"                    try:"},{"line_number":653,"context_line":"                        rpcapi.terminate_connection(context, volume,"},{"line_number":654,"context_line":"                                                    properties, force\u003dTrue)"},{"line_number":655,"context_line":"                    except Exception:"},{"line_number":656,"context_line":"                        LOG.warning(_LW(\"Failed terminating the connection \""}],"source_content_type":"text/x-python","patch_set":48,"id":"9aa53dc9_50386cd3","line":653,"in_reply_to":"baa041b7_c05fcfe3","updated":"2015-04-07 13:32:04.000000000","message":"Good point!","commit_id":"304771544f75fbbeef9deb5a5ff0302eb97156b5"},{"author":{"_account_id":13900,"name":"Accela Zhao","email":"accelazh@gmail.com","username":"accelazh"},"change_message_id":"19c83c77f96e5cf25befbd7c6c6ebe4cd52aed89","unresolved":false,"context_lines":[{"line_number":653,"context_line":"                        rpcapi.terminate_connection(context, volume,"},{"line_number":654,"context_line":"                                                    properties, force\u003dTrue)"},{"line_number":655,"context_line":"                    except Exception:"},{"line_number":656,"context_line":"                        LOG.warning(_LW(\"Failed terminating the connection \""},{"line_number":657,"context_line":"                                        \"of volume %(volume_id)s, but it is \""},{"line_number":658,"context_line":"                                        \"acceptable.\"),"},{"line_number":659,"context_line":"                                    {\u0027volume_id\u0027: volume[\u0027id\u0027]})"}],"source_content_type":"text/x-python","patch_set":48,"id":"baa041b7_99239da7","line":656,"updated":"2015-04-03 13:30:18.000000000","message":"\"Failed to terminate\"?","commit_id":"304771544f75fbbeef9deb5a5ff0302eb97156b5"},{"author":{"_account_id":13900,"name":"Accela Zhao","email":"accelazh@gmail.com","username":"accelazh"},"change_message_id":"f9ea2ed24f5e44f6235b912cf0da238ebbd35227","unresolved":false,"context_lines":[{"line_number":653,"context_line":"                        rpcapi.terminate_connection(context, volume,"},{"line_number":654,"context_line":"                                                    properties, force\u003dTrue)"},{"line_number":655,"context_line":"                    except Exception:"},{"line_number":656,"context_line":"                        LOG.warning(_LW(\"Failed terminating the connection \""},{"line_number":657,"context_line":"                                        \"of volume %(volume_id)s, but it is \""},{"line_number":658,"context_line":"                                        \"acceptable.\"),"},{"line_number":659,"context_line":"                                    {\u0027volume_id\u0027: volume[\u0027id\u0027]})"}],"source_content_type":"text/x-python","patch_set":48,"id":"9aa53dc9_70b01044","line":656,"in_reply_to":"baa041b7_8059c700","updated":"2015-04-07 13:32:04.000000000","message":"Oops, I mean \"Failed to terminate\" may be better syntax than \"Failed terminating\".","commit_id":"304771544f75fbbeef9deb5a5ff0302eb97156b5"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"2de803e333a9b2a4d0c08c4019698225373371fc","unresolved":false,"context_lines":[{"line_number":653,"context_line":"                        rpcapi.terminate_connection(context, volume,"},{"line_number":654,"context_line":"                                                    properties, force\u003dTrue)"},{"line_number":655,"context_line":"                    except Exception:"},{"line_number":656,"context_line":"                        LOG.warning(_LW(\"Failed terminating the connection \""},{"line_number":657,"context_line":"                                        \"of volume %(volume_id)s, but it is \""},{"line_number":658,"context_line":"                                        \"acceptable.\"),"},{"line_number":659,"context_line":"                                    {\u0027volume_id\u0027: volume[\u0027id\u0027]})"}],"source_content_type":"text/x-python","patch_set":48,"id":"baa041b7_8059c700","line":656,"in_reply_to":"baa041b7_99239da7","updated":"2015-04-05 07:59:30.000000000","message":"It does not matter the terminate fails, since it is tried to cover the possible case above.","commit_id":"304771544f75fbbeef9deb5a5ff0302eb97156b5"}],"cinder/volume/manager.py":[{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"99e13464396dc3789b820c54d92734c6faf9d0be","unresolved":false,"context_lines":[{"line_number":1166,"context_line":"                                              volume[\u0027id\u0027], new_volume[\u0027id\u0027])"},{"line_number":1167,"context_line":"        except Exception:"},{"line_number":1168,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":1169,"context_line":"                msg \u003d _(\"Failed to copy volume %(vol1)s to %(vol2)s\")"},{"line_number":1170,"context_line":"                LOG.error(msg % {\u0027vol1\u0027: volume[\u0027id\u0027],"},{"line_number":1171,"context_line":"                                 \u0027vol2\u0027: new_volume[\u0027id\u0027]})"},{"line_number":1172,"context_line":"                self._clean_temporary_volume(ctxt, volume[\u0027id\u0027],"}],"source_content_type":"text/x-python","patch_set":21,"id":"9a80dd14_99ec5ef5","line":1169,"updated":"2015-03-06 14:05:38.000000000","message":"I know this isn\u0027t part of your patch, but since you have to do another update could you change this from _() to _LE(), and in the LOG.error() change % to ,?\n\nE.G.:\nmsg \u003d _LE(...)\nLOG.error(msg, {\u0027vol1\u0027...})","commit_id":"5635e4181a8f9a7f1e8f3f5d2a500ce802bd7ae8"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"c6f39171bf4c1b4d3008f3636144e5dcf5a40b4f","unresolved":false,"context_lines":[{"line_number":1166,"context_line":"                                              volume[\u0027id\u0027], new_volume[\u0027id\u0027])"},{"line_number":1167,"context_line":"        except Exception:"},{"line_number":1168,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":1169,"context_line":"                msg \u003d _(\"Failed to copy volume %(vol1)s to %(vol2)s\")"},{"line_number":1170,"context_line":"                LOG.error(msg % {\u0027vol1\u0027: volume[\u0027id\u0027],"},{"line_number":1171,"context_line":"                                 \u0027vol2\u0027: new_volume[\u0027id\u0027]})"},{"line_number":1172,"context_line":"                self._clean_temporary_volume(ctxt, volume[\u0027id\u0027],"}],"source_content_type":"text/x-python","patch_set":21,"id":"9a80dd14_b1582d8b","line":1169,"in_reply_to":"9a80dd14_99ec5ef5","updated":"2015-03-09 03:06:15.000000000","message":"Done","commit_id":"5635e4181a8f9a7f1e8f3f5d2a500ce802bd7ae8"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"99e13464396dc3789b820c54d92734c6faf9d0be","unresolved":false,"context_lines":[{"line_number":1195,"context_line":"                volume \u003d self.db.volume_get(ctxt, new_volume_id)"},{"line_number":1196,"context_line":"                rpcapi.delete_volume(ctxt, volume)"},{"line_number":1197,"context_line":"        else:"},{"line_number":1198,"context_line":"                    # If we\u0027re in the completing phase don\u0027t delete the"},{"line_number":1199,"context_line":"                    # destination because we may have already deleted the"},{"line_number":1200,"context_line":"                    # source! But the migration_status in database should"},{"line_number":1201,"context_line":"                    # be cleared to handle volume after migration failure"}],"source_content_type":"text/x-python","patch_set":21,"id":"9a80dd14_39745267","line":1198,"updated":"2015-03-06 14:05:38.000000000","message":"Please align the comments with the code (shift left).","commit_id":"5635e4181a8f9a7f1e8f3f5d2a500ce802bd7ae8"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"c6f39171bf4c1b4d3008f3636144e5dcf5a40b4f","unresolved":false,"context_lines":[{"line_number":1195,"context_line":"                volume \u003d self.db.volume_get(ctxt, new_volume_id)"},{"line_number":1196,"context_line":"                rpcapi.delete_volume(ctxt, volume)"},{"line_number":1197,"context_line":"        else:"},{"line_number":1198,"context_line":"                    # If we\u0027re in the completing phase don\u0027t delete the"},{"line_number":1199,"context_line":"                    # destination because we may have already deleted the"},{"line_number":1200,"context_line":"                    # source! But the migration_status in database should"},{"line_number":1201,"context_line":"                    # be cleared to handle volume after migration failure"}],"source_content_type":"text/x-python","patch_set":21,"id":"9a80dd14_f1dc65e6","line":1198,"in_reply_to":"9a80dd14_39745267","updated":"2015-03-09 03:06:15.000000000","message":"Done","commit_id":"5635e4181a8f9a7f1e8f3f5d2a500ce802bd7ae8"},{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"b187b8399be019aa32d15260425ddd67f7740ba4","unresolved":false,"context_lines":[{"line_number":1211,"context_line":""},{"line_number":1212,"context_line":"                LOG.warn(_LW(\"Failed to migrate volume. The destination \""},{"line_number":1213,"context_line":"                             \"volume %(vol)s is not deleted since the \""},{"line_number":1214,"context_line":"                             \"source volume may have already deleted.\"),"},{"line_number":1215,"context_line":"                         {\u0027vol\u0027: new_volume_id})"},{"line_number":1216,"context_line":""},{"line_number":1217,"context_line":"    def migrate_volume_completion(self, ctxt, volume_id, new_volume_id,"}],"source_content_type":"text/x-python","patch_set":21,"id":"9a80dd14_013aa798","line":1214,"updated":"2015-03-06 20:18:59.000000000","message":"\"already deleted\" -\u003e \"already been deleted\"","commit_id":"5635e4181a8f9a7f1e8f3f5d2a500ce802bd7ae8"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"c6f39171bf4c1b4d3008f3636144e5dcf5a40b4f","unresolved":false,"context_lines":[{"line_number":1211,"context_line":""},{"line_number":1212,"context_line":"                LOG.warn(_LW(\"Failed to migrate volume. The destination \""},{"line_number":1213,"context_line":"                             \"volume %(vol)s is not deleted since the \""},{"line_number":1214,"context_line":"                             \"source volume may have already deleted.\"),"},{"line_number":1215,"context_line":"                         {\u0027vol\u0027: new_volume_id})"},{"line_number":1216,"context_line":""},{"line_number":1217,"context_line":"    def migrate_volume_completion(self, ctxt, volume_id, new_volume_id,"}],"source_content_type":"text/x-python","patch_set":21,"id":"9a80dd14_11dad9e7","line":1214,"in_reply_to":"9a80dd14_013aa798","updated":"2015-03-09 03:06:15.000000000","message":"Done","commit_id":"5635e4181a8f9a7f1e8f3f5d2a500ce802bd7ae8"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"97ef99fcfc7f15c666e638992db2e563a5f201b1","unresolved":false,"context_lines":[{"line_number":1133,"context_line":"            tries \u003d tries + 1"},{"line_number":1134,"context_line":"            now \u003d time.time()"},{"line_number":1135,"context_line":"            if new_volume[\u0027status\u0027] \u003d\u003d \u0027error\u0027:"},{"line_number":1136,"context_line":"                msg \u003d _LE(\"failed to create new_volume on destination host\")"},{"line_number":1137,"context_line":"                self._clean_temporary_volume(ctxt, volume[\u0027id\u0027],"},{"line_number":1138,"context_line":"                                             new_volume[\u0027id\u0027],"},{"line_number":1139,"context_line":"                                             clean_db_only\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":22,"id":"9a80dd14_dec2fc51","line":1136,"updated":"2015-03-09 13:50:31.000000000","message":"_LE should not be used for strings used for exception messages. This was correct as _().\n\nThe line you had updated on line 1169 needed _LE() because that string is sent to LOG.error. Strings used in exceptions, or both logs and exceptions, just use _(). Sorry if I had confused you earlier.","commit_id":"8c11652a49c24636d69f1a127da0333687717dea"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"a1f28c727a674f34e97a4a9d918b690b40e5177e","unresolved":false,"context_lines":[{"line_number":1133,"context_line":"            tries \u003d tries + 1"},{"line_number":1134,"context_line":"            now \u003d time.time()"},{"line_number":1135,"context_line":"            if new_volume[\u0027status\u0027] \u003d\u003d \u0027error\u0027:"},{"line_number":1136,"context_line":"                msg \u003d _LE(\"failed to create new_volume on destination host\")"},{"line_number":1137,"context_line":"                self._clean_temporary_volume(ctxt, volume[\u0027id\u0027],"},{"line_number":1138,"context_line":"                                             new_volume[\u0027id\u0027],"},{"line_number":1139,"context_line":"                                             clean_db_only\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":22,"id":"9a80dd14_2f874db6","line":1136,"in_reply_to":"9a80dd14_dec2fc51","updated":"2015-03-10 01:34:52.000000000","message":"Done","commit_id":"8c11652a49c24636d69f1a127da0333687717dea"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"97ef99fcfc7f15c666e638992db2e563a5f201b1","unresolved":false,"context_lines":[{"line_number":1139,"context_line":"                                             clean_db_only\u003dTrue)"},{"line_number":1140,"context_line":"                raise exception.VolumeMigrationFailed(reason\u003dmsg)"},{"line_number":1141,"context_line":"            elif now \u003e deadline:"},{"line_number":1142,"context_line":"                msg \u003d _LE(\"timeout creating new_volume on destination host\")"},{"line_number":1143,"context_line":"                self._clean_temporary_volume(ctxt, volume[\u0027id\u0027],"},{"line_number":1144,"context_line":"                                             new_volume[\u0027id\u0027],"},{"line_number":1145,"context_line":"                                             clean_db_only\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":22,"id":"9a80dd14_1ea8148c","line":1142,"updated":"2015-03-09 13:50:31.000000000","message":"Same as above.","commit_id":"8c11652a49c24636d69f1a127da0333687717dea"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"a1f28c727a674f34e97a4a9d918b690b40e5177e","unresolved":false,"context_lines":[{"line_number":1139,"context_line":"                                             clean_db_only\u003dTrue)"},{"line_number":1140,"context_line":"                raise exception.VolumeMigrationFailed(reason\u003dmsg)"},{"line_number":1141,"context_line":"            elif now \u003e deadline:"},{"line_number":1142,"context_line":"                msg \u003d _LE(\"timeout creating new_volume on destination host\")"},{"line_number":1143,"context_line":"                self._clean_temporary_volume(ctxt, volume[\u0027id\u0027],"},{"line_number":1144,"context_line":"                                             new_volume[\u0027id\u0027],"},{"line_number":1145,"context_line":"                                             clean_db_only\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":22,"id":"9a80dd14_4f92c1f6","line":1142,"in_reply_to":"9a80dd14_1ea8148c","updated":"2015-03-10 01:34:52.000000000","message":"Done","commit_id":"8c11652a49c24636d69f1a127da0333687717dea"},{"author":{"_account_id":10115,"name":"Mitsuhiro Tanino","email":"mitsuhiro.tanino@lycorp.co.jp","username":"mtanino"},"change_message_id":"340d419cbd988fdd23c80fddaae16f16b9dcff04","unresolved":false,"context_lines":[{"line_number":1251,"context_line":"                rpcapi.delete_volume(ctxt, volume)"},{"line_number":1252,"context_line":"                tries \u003d 0"},{"line_number":1253,"context_line":"                start \u003d time.time()"},{"line_number":1254,"context_line":"                deadline \u003d start + CONF.migration_create_volume_timeout_secs"},{"line_number":1255,"context_line":"                # If the volume is not successfully deleted, the information in"},{"line_number":1256,"context_line":"                # the database needs to be removed, because this volume is the"},{"line_number":1257,"context_line":"                # temporary destination volume, about which the user should"}],"source_content_type":"text/x-python","patch_set":30,"id":"9a80dd14_006ca59b","line":1254,"updated":"2015-03-12 02:58:35.000000000","message":"Could you let me know why we need timeout here? Can we execute self.db.volume_destroy() during rpcapi.delete_volume()?\n\nThis increases DB access and it should avoid in my understanding.","commit_id":"9296c67f5d7b53afed634d6ecddd2877b5bdc13d"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"f5e06fdd8f3b5a2983718b619c7cc27330312eb9","unresolved":false,"context_lines":[{"line_number":1251,"context_line":"                rpcapi.delete_volume(ctxt, volume)"},{"line_number":1252,"context_line":"                tries \u003d 0"},{"line_number":1253,"context_line":"                start \u003d time.time()"},{"line_number":1254,"context_line":"                deadline \u003d start + CONF.migration_create_volume_timeout_secs"},{"line_number":1255,"context_line":"                # If the volume is not successfully deleted, the information in"},{"line_number":1256,"context_line":"                # the database needs to be removed, because this volume is the"},{"line_number":1257,"context_line":"                # temporary destination volume, about which the user should"}],"source_content_type":"text/x-python","patch_set":30,"id":"9a80dd14_be94d644","line":1254,"in_reply_to":"9a80dd14_006ca59b","updated":"2015-03-12 04:13:30.000000000","message":"Good question here.\nI have just added them in the latest patch, because I did meet an exception raised for rpcapi.delete_volume, which ended up with the destination volume still in the database with the status of \"error_deleting\". This totally makes no sense to the user, since this destination volume is a temporary volume created by the migration. If the migration succeeds, it will replace the original source volume. If not, it should be removed. A redundant volume row with status of \"error_deleting\" left in the database will lead confusion to the user, so I need to remove the database record in the end.","commit_id":"9296c67f5d7b53afed634d6ecddd2877b5bdc13d"},{"author":{"_account_id":10115,"name":"Mitsuhiro Tanino","email":"mitsuhiro.tanino@lycorp.co.jp","username":"mtanino"},"change_message_id":"bf9252f8c3e2c74f095bd051297c9c47bb4f15bc","unresolved":false,"context_lines":[{"line_number":1251,"context_line":"                rpcapi.delete_volume(ctxt, volume)"},{"line_number":1252,"context_line":"                tries \u003d 0"},{"line_number":1253,"context_line":"                start \u003d time.time()"},{"line_number":1254,"context_line":"                deadline \u003d start + CONF.migration_create_volume_timeout_secs"},{"line_number":1255,"context_line":"                # If the volume is not successfully deleted, the information in"},{"line_number":1256,"context_line":"                # the database needs to be removed, because this volume is the"},{"line_number":1257,"context_line":"                # temporary destination volume, about which the user should"}],"source_content_type":"text/x-python","patch_set":30,"id":"9a80dd14_fed39c5c","line":1254,"in_reply_to":"9a80dd14_34272266","updated":"2015-03-12 13:02:44.000000000","message":"I think remaining \u0027error_deleting\u0027 volume(destination volume) is not a problem because something problem happened storage side. so admins needs to check the root cause of error deleting. This is notification for them.\n\nIf the redundant volume row which the status is \u0027error_deleting\u0027 can\u0027t remove using \u0027cinder delete\u0027 or \u0027cinder force-delete\u0027 anymore, we should solve it. Is this happen or not happen?","commit_id":"9296c67f5d7b53afed634d6ecddd2877b5bdc13d"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"de269922c0971c66d3026995d05ca072d459c9ca","unresolved":false,"context_lines":[{"line_number":1251,"context_line":"                rpcapi.delete_volume(ctxt, volume)"},{"line_number":1252,"context_line":"                tries \u003d 0"},{"line_number":1253,"context_line":"                start \u003d time.time()"},{"line_number":1254,"context_line":"                deadline \u003d start + CONF.migration_create_volume_timeout_secs"},{"line_number":1255,"context_line":"                # If the volume is not successfully deleted, the information in"},{"line_number":1256,"context_line":"                # the database needs to be removed, because this volume is the"},{"line_number":1257,"context_line":"                # temporary destination volume, about which the user should"}],"source_content_type":"text/x-python","patch_set":30,"id":"9a80dd14_34272266","line":1254,"in_reply_to":"9a80dd14_be94d644","updated":"2015-03-12 08:40:25.000000000","message":"I change  my mind. I will remove these line.","commit_id":"9296c67f5d7b53afed634d6ecddd2877b5bdc13d"},{"author":{"_account_id":10115,"name":"Mitsuhiro Tanino","email":"mitsuhiro.tanino@lycorp.co.jp","username":"mtanino"},"change_message_id":"7f6d09e829220eeb0a3132c35c97ee9debcc0c48","unresolved":false,"context_lines":[{"line_number":506,"context_line":"            if unmanage_only:"},{"line_number":507,"context_line":"                self.driver.unmanage(volume_ref)"},{"line_number":508,"context_line":"            else:"},{"line_number":509,"context_line":"                self.driver.delete_volume(volume_ref)"},{"line_number":510,"context_line":"        except exception.VolumeIsBusy:"},{"line_number":511,"context_line":"            LOG.error(_LE(\"Cannot delete volume %s: volume is busy\"),"},{"line_number":512,"context_line":"                      volume_ref[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":32,"id":"9a80dd14_b979f983","line":509,"updated":"2015-03-13 16:25:18.000000000","message":"If no one clear migration_status for failure case of destination volume deletion we should check here it and clear the migration_status. IMO.","commit_id":"daa5da51a25274173f8add3c2d2260661c8b3de5"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"62d20759ee84c89ac2587c0c1aec8cff07889059","unresolved":false,"context_lines":[{"line_number":506,"context_line":"            if unmanage_only:"},{"line_number":507,"context_line":"                self.driver.unmanage(volume_ref)"},{"line_number":508,"context_line":"            else:"},{"line_number":509,"context_line":"                self.driver.delete_volume(volume_ref)"},{"line_number":510,"context_line":"        except exception.VolumeIsBusy:"},{"line_number":511,"context_line":"            LOG.error(_LE(\"Cannot delete volume %s: volume is busy\"),"},{"line_number":512,"context_line":"                      volume_ref[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":32,"id":"9a80dd14_55bc1006","line":509,"in_reply_to":"9a80dd14_b979f983","updated":"2015-03-16 08:17:52.000000000","message":"I will add the db clearance in the exception handling for dest volume.","commit_id":"daa5da51a25274173f8add3c2d2260661c8b3de5"},{"author":{"_account_id":10115,"name":"Mitsuhiro Tanino","email":"mitsuhiro.tanino@lycorp.co.jp","username":"mtanino"},"change_message_id":"7f6d09e829220eeb0a3132c35c97ee9debcc0c48","unresolved":false,"context_lines":[{"line_number":512,"context_line":"                      volume_ref[\u0027id\u0027])"},{"line_number":513,"context_line":"            self.db.volume_update(context, volume_ref[\u0027id\u0027],"},{"line_number":514,"context_line":"                                  {\u0027status\u0027: \u0027available\u0027})"},{"line_number":515,"context_line":"            return True"},{"line_number":516,"context_line":"        except Exception:"},{"line_number":517,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":518,"context_line":"                self.db.volume_update(context,"}],"source_content_type":"text/x-python","patch_set":32,"id":"9a80dd14_b9f019fe","line":515,"updated":"2015-03-13 16:25:18.000000000","message":"ditto","commit_id":"daa5da51a25274173f8add3c2d2260661c8b3de5"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"62d20759ee84c89ac2587c0c1aec8cff07889059","unresolved":false,"context_lines":[{"line_number":512,"context_line":"                      volume_ref[\u0027id\u0027])"},{"line_number":513,"context_line":"            self.db.volume_update(context, volume_ref[\u0027id\u0027],"},{"line_number":514,"context_line":"                                  {\u0027status\u0027: \u0027available\u0027})"},{"line_number":515,"context_line":"            return True"},{"line_number":516,"context_line":"        except Exception:"},{"line_number":517,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":518,"context_line":"                self.db.volume_update(context,"}],"source_content_type":"text/x-python","patch_set":32,"id":"9a80dd14_35c15c80","line":515,"in_reply_to":"9a80dd14_b9f019fe","updated":"2015-03-16 08:17:52.000000000","message":"I will add the db clearance in the exception handling for dest volume.","commit_id":"daa5da51a25274173f8add3c2d2260661c8b3de5"},{"author":{"_account_id":10115,"name":"Mitsuhiro Tanino","email":"mitsuhiro.tanino@lycorp.co.jp","username":"mtanino"},"change_message_id":"925fa3e31051d60d0da551d0facf0165dd89fa4f","unresolved":false,"context_lines":[{"line_number":1248,"context_line":"                # The temporary volume is already created"},{"line_number":1249,"context_line":"                rpcapi \u003d volume_rpcapi.VolumeAPI()"},{"line_number":1250,"context_line":"                volume \u003d self.db.volume_get(ctxt, new_volume_id)"},{"line_number":1251,"context_line":"                rpcapi.delete_volume(ctxt, volume)"},{"line_number":1252,"context_line":"        else:"},{"line_number":1253,"context_line":"            # If we\u0027re in the completing phase don\u0027t delete the"},{"line_number":1254,"context_line":"            # destination because we may have already deleted the"}],"source_content_type":"text/x-python","patch_set":32,"id":"9a80dd14_b9edbeb9","line":1251,"updated":"2015-03-12 13:37:35.000000000","message":"Do we need try and except here because migration_status of destination volume is remaining? no?\n\ntry:\n    rpcapi.delete_volume()\nexcept Exception:\n    volume \u003d self.db.volume_get(ctxt, new_volume_id)\n    if volume[\u0027status\u0027] \u003d\u003d \u0027error_deleting\u0027\n        updates \u003d {\u0027migration_status\u0027: None}\n        self.db.volume_update(ctxt, new_volume_id, updates)","commit_id":"daa5da51a25274173f8add3c2d2260661c8b3de5"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"f5d483d834e6b1c1d9219c8637223cac18fe27a3","unresolved":false,"context_lines":[{"line_number":1248,"context_line":"                # The temporary volume is already created"},{"line_number":1249,"context_line":"                rpcapi \u003d volume_rpcapi.VolumeAPI()"},{"line_number":1250,"context_line":"                volume \u003d self.db.volume_get(ctxt, new_volume_id)"},{"line_number":1251,"context_line":"                rpcapi.delete_volume(ctxt, volume)"},{"line_number":1252,"context_line":"        else:"},{"line_number":1253,"context_line":"            # If we\u0027re in the completing phase don\u0027t delete the"},{"line_number":1254,"context_line":"            # destination because we may have already deleted the"}],"source_content_type":"text/x-python","patch_set":32,"id":"9a80dd14_7f7578f8","line":1251,"in_reply_to":"9a80dd14_191e850c","updated":"2015-03-16 02:47:48.000000000","message":"Hi Mitsuhiro,\nThe migration_status of the destination volume will be like \"target:src-volume-id\" , e.g. \"target:471cb9ba-25c5-44ee-bb51-ee8cb6bc972b\". \nIf the deletion of the dest volume, the status of this volume will be deleted; if the deletion fails, the status of this volume will be error_deleting. No matter what the migration_status will be, this dest volume won\u0027t take any effect, so what the migration_status is does not matter here.","commit_id":"daa5da51a25274173f8add3c2d2260661c8b3de5"},{"author":{"_account_id":10115,"name":"Mitsuhiro Tanino","email":"mitsuhiro.tanino@lycorp.co.jp","username":"mtanino"},"change_message_id":"f5948a169e0ee6f2e0b26cf5d3b32fd7d994b477","unresolved":false,"context_lines":[{"line_number":1248,"context_line":"                # The temporary volume is already created"},{"line_number":1249,"context_line":"                rpcapi \u003d volume_rpcapi.VolumeAPI()"},{"line_number":1250,"context_line":"                volume \u003d self.db.volume_get(ctxt, new_volume_id)"},{"line_number":1251,"context_line":"                rpcapi.delete_volume(ctxt, volume)"},{"line_number":1252,"context_line":"        else:"},{"line_number":1253,"context_line":"            # If we\u0027re in the completing phase don\u0027t delete the"},{"line_number":1254,"context_line":"            # destination because we may have already deleted the"}],"source_content_type":"text/x-python","patch_set":32,"id":"9a80dd14_7f7f38ab","line":1251,"in_reply_to":"9a80dd14_7f7578f8","updated":"2015-03-16 03:09:31.000000000","message":"In above case, we can see the dest volume via \"cinder list\". Right? \nIf the volume has \"target:src-volume-id\" in the migration_status,\nwe can\u0027t delete the volume using \"cinder delete volume\"\nanymore. I\u0027m concerning the situation that the unnecessary \nvolume entry keeps remaining on \"cinder list\".\n \nSo, my supposed case is,\n\n (1) failed to delete dest volume during volume migration with failure\n (2) Reset dest volume status to available(for deletion)\n (3) try to delete dest volume\n     -\u003e This volume deletion will be failed due to the migration_status.\n\nany thought?","commit_id":"daa5da51a25274173f8add3c2d2260661c8b3de5"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"f4c4dad092c6fd9e4581bcfac20c99e3cbaf6bc2","unresolved":false,"context_lines":[{"line_number":1248,"context_line":"                # The temporary volume is already created"},{"line_number":1249,"context_line":"                rpcapi \u003d volume_rpcapi.VolumeAPI()"},{"line_number":1250,"context_line":"                volume \u003d self.db.volume_get(ctxt, new_volume_id)"},{"line_number":1251,"context_line":"                rpcapi.delete_volume(ctxt, volume)"},{"line_number":1252,"context_line":"        else:"},{"line_number":1253,"context_line":"            # If we\u0027re in the completing phase don\u0027t delete the"},{"line_number":1254,"context_line":"            # destination because we may have already deleted the"}],"source_content_type":"text/x-python","patch_set":32,"id":"9a80dd14_fe6969be","line":1251,"in_reply_to":"9a80dd14_7f7f38ab","updated":"2015-03-16 04:12:16.000000000","message":"The reason why we reach Line 1251 rpcapi.delete_volume(ctxt, volume) is because the migration_failure as you proposed in (1), so we need to delete the dest volume. \nAfter that rpcapi.delete_volume(ctxt, volume) itself is going to delete the the dest volume, which is exactly the (3) step you proposed. \n\nImportant Notes: The volume deletion failure won\u0027t be caused by migration_status, this is something impossible to happen for volume deletion, no matter it is null or not null like \"target:src-volume-id\". It  must be some other  reasons, which causing that the volume fails to delete. For example, the volume backend is occupied  by other devices.","commit_id":"daa5da51a25274173f8add3c2d2260661c8b3de5"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"21a949269415c47c7fbde6a2d05a1ef47f1ba577","unresolved":false,"context_lines":[{"line_number":1248,"context_line":"                # The temporary volume is already created"},{"line_number":1249,"context_line":"                rpcapi \u003d volume_rpcapi.VolumeAPI()"},{"line_number":1250,"context_line":"                volume \u003d self.db.volume_get(ctxt, new_volume_id)"},{"line_number":1251,"context_line":"                rpcapi.delete_volume(ctxt, volume)"},{"line_number":1252,"context_line":"        else:"},{"line_number":1253,"context_line":"            # If we\u0027re in the completing phase don\u0027t delete the"},{"line_number":1254,"context_line":"            # destination because we may have already deleted the"}],"source_content_type":"text/x-python","patch_set":32,"id":"9a80dd14_e0777a9d","line":1251,"in_reply_to":"9a80dd14_b9edbeb9","updated":"2015-03-12 14:07:36.000000000","message":"rpcapi.delete_volume() is anyc cast call, so this line won\u0027t raise anything wrong here. Even if there is an error, the error is handled in the remote c-vol service.","commit_id":"daa5da51a25274173f8add3c2d2260661c8b3de5"},{"author":{"_account_id":10115,"name":"Mitsuhiro Tanino","email":"mitsuhiro.tanino@lycorp.co.jp","username":"mtanino"},"change_message_id":"d4cb7cb5379deed6e3bda3d6cb0da47c58df0f55","unresolved":false,"context_lines":[{"line_number":1248,"context_line":"                # The temporary volume is already created"},{"line_number":1249,"context_line":"                rpcapi \u003d volume_rpcapi.VolumeAPI()"},{"line_number":1250,"context_line":"                volume \u003d self.db.volume_get(ctxt, new_volume_id)"},{"line_number":1251,"context_line":"                rpcapi.delete_volume(ctxt, volume)"},{"line_number":1252,"context_line":"        else:"},{"line_number":1253,"context_line":"            # If we\u0027re in the completing phase don\u0027t delete the"},{"line_number":1254,"context_line":"            # destination because we may have already deleted the"}],"source_content_type":"text/x-python","patch_set":32,"id":"9a80dd14_e3a4e4ba","line":1251,"in_reply_to":"9a80dd14_e0777a9d","updated":"2015-03-12 14:28:13.000000000","message":"I understand. Thanks.\nIf error happens here how to reset to the migration_status for destination volume? during delete_volume() on remote c-vol service?","commit_id":"daa5da51a25274173f8add3c2d2260661c8b3de5"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"14df1d98c474722bf8999aa25ea43a075c83b210","unresolved":false,"context_lines":[{"line_number":1248,"context_line":"                # The temporary volume is already created"},{"line_number":1249,"context_line":"                rpcapi \u003d volume_rpcapi.VolumeAPI()"},{"line_number":1250,"context_line":"                volume \u003d self.db.volume_get(ctxt, new_volume_id)"},{"line_number":1251,"context_line":"                rpcapi.delete_volume(ctxt, volume)"},{"line_number":1252,"context_line":"        else:"},{"line_number":1253,"context_line":"            # If we\u0027re in the completing phase don\u0027t delete the"},{"line_number":1254,"context_line":"            # destination because we may have already deleted the"}],"source_content_type":"text/x-python","patch_set":32,"id":"9a80dd14_f8d9cb57","line":1251,"in_reply_to":"9a80dd14_e3a4e4ba","updated":"2015-03-13 03:30:20.000000000","message":"There are two status for a volume in the DB table: status and migration_status.\n\nThe status will be left error_deleting and the migration_status will be None, but in this situation, this dest volume won\u0027t take any quota and won\u0027t affect the user. \n\nI was considering whether I need to clean up the DB in Patch 30, but I finally drop that approach because it is not very necessary.","commit_id":"daa5da51a25274173f8add3c2d2260661c8b3de5"},{"author":{"_account_id":10115,"name":"Mitsuhiro Tanino","email":"mitsuhiro.tanino@lycorp.co.jp","username":"mtanino"},"change_message_id":"7f6d09e829220eeb0a3132c35c97ee9debcc0c48","unresolved":false,"context_lines":[{"line_number":1248,"context_line":"                # The temporary volume is already created"},{"line_number":1249,"context_line":"                rpcapi \u003d volume_rpcapi.VolumeAPI()"},{"line_number":1250,"context_line":"                volume \u003d self.db.volume_get(ctxt, new_volume_id)"},{"line_number":1251,"context_line":"                rpcapi.delete_volume(ctxt, volume)"},{"line_number":1252,"context_line":"        else:"},{"line_number":1253,"context_line":"            # If we\u0027re in the completing phase don\u0027t delete the"},{"line_number":1254,"context_line":"            # destination because we may have already deleted the"}],"source_content_type":"text/x-python","patch_set":32,"id":"9a80dd14_191e850c","line":1251,"in_reply_to":"9a80dd14_f8d9cb57","updated":"2015-03-13 16:25:18.000000000","message":"Hi, Thank you for the explanation.\n\nCould you let me know which line set migration_status to \"None\" for destination volume if the dest volume deletion will be failed?\nI suppose no one clear it for this error case.\nIf I\u0027 am wrong, please correct me.","commit_id":"daa5da51a25274173f8add3c2d2260661c8b3de5"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"a6b7e477ee5663f2c34f968c2f84f3413f322188","unresolved":false,"context_lines":[{"line_number":1239,"context_line":"        except Exception:"},{"line_number":1240,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":1241,"context_line":"                msg \u003d _LE(\"Failed to copy volume %(vol1)s to %(vol2)s\")"},{"line_number":1242,"context_line":"                LOG.error(msg % {\u0027vol1\u0027: volume[\u0027id\u0027],"},{"line_number":1243,"context_line":"                                 \u0027vol2\u0027: new_volume[\u0027id\u0027]})"},{"line_number":1244,"context_line":"                self._clean_temporary_volume(ctxt, volume[\u0027id\u0027],"},{"line_number":1245,"context_line":"                                             new_volume[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":40,"id":"1a6ced46_3f100e4e","line":1242,"updated":"2015-03-23 17:25:57.000000000","message":"Please, change \u0027%\u0027 to \u0027,\u0027","commit_id":"b2528e8b95b908dab1254ddf7276e90895df1bad"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"a379665d5c4ba944e2f573e54252cd0efb093293","unresolved":false,"context_lines":[{"line_number":1239,"context_line":"        except Exception:"},{"line_number":1240,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":1241,"context_line":"                msg \u003d _LE(\"Failed to copy volume %(vol1)s to %(vol2)s\")"},{"line_number":1242,"context_line":"                LOG.error(msg % {\u0027vol1\u0027: volume[\u0027id\u0027],"},{"line_number":1243,"context_line":"                                 \u0027vol2\u0027: new_volume[\u0027id\u0027]})"},{"line_number":1244,"context_line":"                self._clean_temporary_volume(ctxt, volume[\u0027id\u0027],"},{"line_number":1245,"context_line":"                                             new_volume[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":40,"id":"1a6ced46_ab9d915b","line":1242,"in_reply_to":"1a6ced46_3f100e4e","updated":"2015-03-24 04:00:49.000000000","message":"Done","commit_id":"b2528e8b95b908dab1254ddf7276e90895df1bad"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"be190fb96a3619bccdddaf2b5cc42c409f4255d5","unresolved":false,"context_lines":[{"line_number":530,"context_line":"        except exception.VolumeIsBusy:"},{"line_number":531,"context_line":"            LOG.error(_LE(\"Cannot delete volume %s: volume is busy\"),"},{"line_number":532,"context_line":"                      volume_ref[\u0027id\u0027])"},{"line_number":533,"context_line":"            # If this is a destination volume, we have to clear the database"},{"line_number":534,"context_line":"            # record to avoid user confusion."},{"line_number":535,"context_line":"            if is_migrating_dest:"},{"line_number":536,"context_line":"                self.db.volume_destroy(context, volume_id)"}],"source_content_type":"text/x-python","patch_set":42,"id":"da9b358b_1b5b0e89","line":533,"updated":"2015-03-31 08:41:52.000000000","message":"Code from lines #533-#543 is duplicated. Wouldn\u0027t it be better to move it to a function with status as a parameter?","commit_id":"7bdd3c0fd0f3df1de03bafe9065f1ce5199518d3"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"f684f945dc1a49238450ae97a0b62ad10f1620a3","unresolved":false,"context_lines":[{"line_number":1319,"context_line":"                             \"completion phase.\"),"},{"line_number":1320,"context_line":"                         {\u0027vol\u0027: new_volume_id})"},{"line_number":1321,"context_line":""},{"line_number":1322,"context_line":"                LOG.warn(_LW(\"Failed to migrate volume. The destination \""},{"line_number":1323,"context_line":"                             \"volume %(vol)s is not deleted since the \""},{"line_number":1324,"context_line":"                             \"source volume may have already been deleted.\"),"},{"line_number":1325,"context_line":"                         {\u0027vol\u0027: new_volume_id})"}],"source_content_type":"text/x-python","patch_set":46,"id":"baa041b7_17d2df00","line":1322,"updated":"2015-04-02 13:09:31.000000000","message":"/warn/warning/","commit_id":"00d53d3d86e3d932c18544fe811fff4b357ec2d4"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"7ded54f91c5211170656baa7de384099617b6736","unresolved":false,"context_lines":[{"line_number":1319,"context_line":"                             \"completion phase.\"),"},{"line_number":1320,"context_line":"                         {\u0027vol\u0027: new_volume_id})"},{"line_number":1321,"context_line":""},{"line_number":1322,"context_line":"                LOG.warn(_LW(\"Failed to migrate volume. The destination \""},{"line_number":1323,"context_line":"                             \"volume %(vol)s is not deleted since the \""},{"line_number":1324,"context_line":"                             \"source volume may have already been deleted.\"),"},{"line_number":1325,"context_line":"                         {\u0027vol\u0027: new_volume_id})"}],"source_content_type":"text/x-python","patch_set":46,"id":"baa041b7_3251580f","line":1322,"in_reply_to":"baa041b7_17d2df00","updated":"2015-04-03 01:45:32.000000000","message":"Done","commit_id":"00d53d3d86e3d932c18544fe811fff4b357ec2d4"},{"author":{"_account_id":13900,"name":"Accela Zhao","email":"accelazh@gmail.com","username":"accelazh"},"change_message_id":"19c83c77f96e5cf25befbd7c6c6ebe4cd52aed89","unresolved":false,"context_lines":[{"line_number":551,"context_line":"            # If this is a destination volume, we have to clear the database"},{"line_number":552,"context_line":"            # record to avoid user confusion."},{"line_number":553,"context_line":"            self._clear_db(context, is_migrating_dest, volume_id,"},{"line_number":554,"context_line":"                           volume_ref, \u0027available\u0027)"},{"line_number":555,"context_line":"            return True"},{"line_number":556,"context_line":"        except Exception:"},{"line_number":557,"context_line":"            with excutils.save_and_reraise_exception():"}],"source_content_type":"text/x-python","patch_set":48,"id":"baa041b7_d9e005cf","line":554,"updated":"2015-04-03 13:30:18.000000000","message":"Is there any constant to use instead of literal string?","commit_id":"304771544f75fbbeef9deb5a5ff0302eb97156b5"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"2de803e333a9b2a4d0c08c4019698225373371fc","unresolved":false,"context_lines":[{"line_number":551,"context_line":"            # If this is a destination volume, we have to clear the database"},{"line_number":552,"context_line":"            # record to avoid user confusion."},{"line_number":553,"context_line":"            self._clear_db(context, is_migrating_dest, volume_id,"},{"line_number":554,"context_line":"                           volume_ref, \u0027available\u0027)"},{"line_number":555,"context_line":"            return True"},{"line_number":556,"context_line":"        except Exception:"},{"line_number":557,"context_line":"            with excutils.save_and_reraise_exception():"}],"source_content_type":"text/x-python","patch_set":48,"id":"baa041b7_00781729","line":554,"in_reply_to":"baa041b7_d9e005cf","updated":"2015-04-05 07:59:30.000000000","message":"not yet.","commit_id":"304771544f75fbbeef9deb5a5ff0302eb97156b5"},{"author":{"_account_id":13900,"name":"Accela Zhao","email":"accelazh@gmail.com","username":"accelazh"},"change_message_id":"19c83c77f96e5cf25befbd7c6c6ebe4cd52aed89","unresolved":false,"context_lines":[{"line_number":618,"context_line":"                  status):"},{"line_number":619,"context_line":"        if is_migrating_dest:"},{"line_number":620,"context_line":"            self.db.volume_destroy(context, volume_id)"},{"line_number":621,"context_line":"            LOG.error(_LE(\"Unable to delete the destination volume %s \""},{"line_number":622,"context_line":"                          \"during volume migration, but the database \""},{"line_number":623,"context_line":"                          \"record needs to be deleted.\"),"},{"line_number":624,"context_line":"                      volume_ref[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":48,"id":"baa041b7_f9b629db","line":621,"updated":"2015-04-03 13:30:18.000000000","message":"The error will be logged even if volume_destroy succeeds?","commit_id":"304771544f75fbbeef9deb5a5ff0302eb97156b5"},{"author":{"_account_id":6491,"name":"xing-yang","email":"xingyang105@gmail.com","username":"xing-yang"},"change_message_id":"3eeef368fb41d00037045dcf965c29fbb1e7dd25","unresolved":false,"context_lines":[{"line_number":618,"context_line":"                  status):"},{"line_number":619,"context_line":"        if is_migrating_dest:"},{"line_number":620,"context_line":"            self.db.volume_destroy(context, volume_id)"},{"line_number":621,"context_line":"            LOG.error(_LE(\"Unable to delete the destination volume %s \""},{"line_number":622,"context_line":"                          \"during volume migration, but the database \""},{"line_number":623,"context_line":"                          \"record needs to be deleted.\"),"},{"line_number":624,"context_line":"                      volume_ref[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":48,"id":"baa041b7_d5c9c879","line":621,"in_reply_to":"baa041b7_201d5b0f","updated":"2015-04-06 02:35:57.000000000","message":"Can you add a comment saying _clear_db is called when driver.unmanage() or driver.delete_volume() fails in delete_volume()?  This way it is more clear why you log an error here.","commit_id":"304771544f75fbbeef9deb5a5ff0302eb97156b5"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"2de803e333a9b2a4d0c08c4019698225373371fc","unresolved":false,"context_lines":[{"line_number":618,"context_line":"                  status):"},{"line_number":619,"context_line":"        if is_migrating_dest:"},{"line_number":620,"context_line":"            self.db.volume_destroy(context, volume_id)"},{"line_number":621,"context_line":"            LOG.error(_LE(\"Unable to delete the destination volume %s \""},{"line_number":622,"context_line":"                          \"during volume migration, but the database \""},{"line_number":623,"context_line":"                          \"record needs to be deleted.\"),"},{"line_number":624,"context_line":"                      volume_ref[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":48,"id":"baa041b7_201d5b0f","line":621,"in_reply_to":"baa041b7_f9b629db","updated":"2015-04-05 07:59:30.000000000","message":"volume_destroy is to clean the db record, not the backend storage itself.\nThis method for the exception only, introduced by failing to delete the back end storage.","commit_id":"304771544f75fbbeef9deb5a5ff0302eb97156b5"},{"author":{"_account_id":13900,"name":"Accela Zhao","email":"accelazh@gmail.com","username":"accelazh"},"change_message_id":"19c83c77f96e5cf25befbd7c6c6ebe4cd52aed89","unresolved":false,"context_lines":[{"line_number":1237,"context_line":"        new_volume \u003d self.db.volume_get(ctxt, new_volume[\u0027id\u0027])"},{"line_number":1238,"context_line":"        tries \u003d 0"},{"line_number":1239,"context_line":"        while new_volume[\u0027status\u0027] !\u003d \u0027available\u0027:"},{"line_number":1240,"context_line":"            tries \u003d tries + 1"},{"line_number":1241,"context_line":"            now \u003d time.time()"},{"line_number":1242,"context_line":"            if new_volume[\u0027status\u0027] \u003d\u003d \u0027error\u0027:"},{"line_number":1243,"context_line":"                msg \u003d _(\"failed to create new_volume on destination host\")"}],"source_content_type":"text/x-python","patch_set":48,"id":"baa041b7_99d4bdbd","line":1240,"updated":"2015-04-03 13:30:18.000000000","message":"`tries +\u003d 1` would be better.","commit_id":"304771544f75fbbeef9deb5a5ff0302eb97156b5"},{"author":{"_account_id":13900,"name":"Accela Zhao","email":"accelazh@gmail.com","username":"accelazh"},"change_message_id":"19c83c77f96e5cf25befbd7c6c6ebe4cd52aed89","unresolved":false,"context_lines":[{"line_number":1241,"context_line":"            now \u003d time.time()"},{"line_number":1242,"context_line":"            if new_volume[\u0027status\u0027] \u003d\u003d \u0027error\u0027:"},{"line_number":1243,"context_line":"                msg \u003d _(\"failed to create new_volume on destination host\")"},{"line_number":1244,"context_line":"                self._clean_temporary_volume(ctxt, volume[\u0027id\u0027],"},{"line_number":1245,"context_line":"                                             new_volume[\u0027id\u0027],"},{"line_number":1246,"context_line":"                                             clean_db_only\u003dTrue)"},{"line_number":1247,"context_line":"                raise exception.VolumeMigrationFailed(reason\u003dmsg)"}],"source_content_type":"text/x-python","patch_set":48,"id":"baa041b7_798a19c1","line":1244,"updated":"2015-04-03 13:30:18.000000000","message":"What if self._clean_temporary_volume throws an exception?","commit_id":"304771544f75fbbeef9deb5a5ff0302eb97156b5"},{"author":{"_account_id":10263,"name":"Vilobh Meshram","email":"vilobhmm@yahoo-inc.com","username":"vilobhmm"},"change_message_id":"eed221db4cf8d80152ece1a1e651ce5581aa8997","unresolved":false,"context_lines":[{"line_number":1241,"context_line":"            now \u003d time.time()"},{"line_number":1242,"context_line":"            if new_volume[\u0027status\u0027] \u003d\u003d \u0027error\u0027:"},{"line_number":1243,"context_line":"                msg \u003d _(\"failed to create new_volume on destination host\")"},{"line_number":1244,"context_line":"                self._clean_temporary_volume(ctxt, volume[\u0027id\u0027],"},{"line_number":1245,"context_line":"                                             new_volume[\u0027id\u0027],"},{"line_number":1246,"context_line":"                                             clean_db_only\u003dTrue)"},{"line_number":1247,"context_line":"                raise exception.VolumeMigrationFailed(reason\u003dmsg)"}],"source_content_type":"text/x-python","patch_set":48,"id":"baa041b7_e300a803","line":1244,"in_reply_to":"baa041b7_798a19c1","updated":"2015-04-03 21:43:13.000000000","message":"Good point.","commit_id":"304771544f75fbbeef9deb5a5ff0302eb97156b5"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"a69bb67bf1b88ad3519be2b4dbf7b75285c91ff3","unresolved":false,"context_lines":[{"line_number":1241,"context_line":"            now \u003d time.time()"},{"line_number":1242,"context_line":"            if new_volume[\u0027status\u0027] \u003d\u003d \u0027error\u0027:"},{"line_number":1243,"context_line":"                msg \u003d _(\"failed to create new_volume on destination host\")"},{"line_number":1244,"context_line":"                self._clean_temporary_volume(ctxt, volume[\u0027id\u0027],"},{"line_number":1245,"context_line":"                                             new_volume[\u0027id\u0027],"},{"line_number":1246,"context_line":"                                             clean_db_only\u003dTrue)"},{"line_number":1247,"context_line":"                raise exception.VolumeMigrationFailed(reason\u003dmsg)"}],"source_content_type":"text/x-python","patch_set":48,"id":"9aa53dc9_335aa984","line":1244,"in_reply_to":"baa041b7_e300a803","updated":"2015-04-07 02:37:19.000000000","message":"I add a new try except in _clean_temporary_volume.","commit_id":"304771544f75fbbeef9deb5a5ff0302eb97156b5"},{"author":{"_account_id":6491,"name":"xing-yang","email":"xingyang105@gmail.com","username":"xing-yang"},"change_message_id":"07237d7045c4ffcda28b2f83f6a76bc47114e319","unresolved":false,"context_lines":[{"line_number":617,"context_line":""},{"line_number":618,"context_line":"    def _clear_db(self, context, is_migrating_dest, volume_id, volume_ref,"},{"line_number":619,"context_line":"                  status):"},{"line_number":620,"context_line":"        \"\"\"This method is called when driver.unmanage() or"},{"line_number":621,"context_line":"        driver.delete_volume() fails in delete_volume(), so it is already"},{"line_number":622,"context_line":"        in the exception handling part."},{"line_number":623,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":49,"id":"9aa53dc9_eea3389d","line":620,"updated":"2015-04-07 03:22:48.000000000","message":"Can you either follow the docstrings guideline \n(http://docs.openstack.org/developer/hacking/#docstrings) or just do comments (#)?","commit_id":"e9d0e88a2912d22058ff4daf02a66cdd045f5cfe"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"54da2faa20f2541c3bcacb42d8cb77f9fc814b48","unresolved":false,"context_lines":[{"line_number":617,"context_line":""},{"line_number":618,"context_line":"    def _clear_db(self, context, is_migrating_dest, volume_id, volume_ref,"},{"line_number":619,"context_line":"                  status):"},{"line_number":620,"context_line":"        \"\"\"This method is called when driver.unmanage() or"},{"line_number":621,"context_line":"        driver.delete_volume() fails in delete_volume(), so it is already"},{"line_number":622,"context_line":"        in the exception handling part."},{"line_number":623,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":49,"id":"9aa53dc9_ee3fb823","line":620,"in_reply_to":"9aa53dc9_eea3389d","updated":"2015-04-07 03:46:48.000000000","message":"I think just comment it.","commit_id":"e9d0e88a2912d22058ff4daf02a66cdd045f5cfe"},{"author":{"_account_id":6491,"name":"xing-yang","email":"xingyang105@gmail.com","username":"xing-yang"},"change_message_id":"07237d7045c4ffcda28b2f83f6a76bc47114e319","unresolved":false,"context_lines":[{"line_number":1313,"context_line":"            except exception.VolumeNotFound:"},{"line_number":1314,"context_line":"                LOG.info(_LI(\"Couldn\u0027t find the temporary volume \""},{"line_number":1315,"context_line":"                             \"%(vol)s in the database. There is no need \""},{"line_number":1316,"context_line":"                             \"to clean this volume.\"),"},{"line_number":1317,"context_line":"                         {\u0027vol\u0027: new_volume_id})"},{"line_number":1318,"context_line":"        else:"},{"line_number":1319,"context_line":"            # If we\u0027re in the completing phase don\u0027t delete the"}],"source_content_type":"text/x-python","patch_set":49,"id":"9aa53dc9_8e9cac6a","line":1316,"updated":"2015-04-07 03:22:48.000000000","message":"clean up","commit_id":"e9d0e88a2912d22058ff4daf02a66cdd045f5cfe"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"54da2faa20f2541c3bcacb42d8cb77f9fc814b48","unresolved":false,"context_lines":[{"line_number":1313,"context_line":"            except exception.VolumeNotFound:"},{"line_number":1314,"context_line":"                LOG.info(_LI(\"Couldn\u0027t find the temporary volume \""},{"line_number":1315,"context_line":"                             \"%(vol)s in the database. There is no need \""},{"line_number":1316,"context_line":"                             \"to clean this volume.\"),"},{"line_number":1317,"context_line":"                         {\u0027vol\u0027: new_volume_id})"},{"line_number":1318,"context_line":"        else:"},{"line_number":1319,"context_line":"            # If we\u0027re in the completing phase don\u0027t delete the"}],"source_content_type":"text/x-python","patch_set":49,"id":"9aa53dc9_ce3cf431","line":1316,"in_reply_to":"9aa53dc9_8e9cac6a","updated":"2015-04-07 03:46:48.000000000","message":"Done","commit_id":"e9d0e88a2912d22058ff4daf02a66cdd045f5cfe"},{"author":{"_account_id":10115,"name":"Mitsuhiro Tanino","email":"mitsuhiro.tanino@lycorp.co.jp","username":"mtanino"},"change_message_id":"4b111260fda188b248c03e62b21cdfb4b24706ca","unresolved":false,"context_lines":[{"line_number":616,"context_line":"        return True"},{"line_number":617,"context_line":""},{"line_number":618,"context_line":"    def _clear_db(self, context, is_migrating_dest, volume_id, volume_ref,"},{"line_number":619,"context_line":"                  status):"},{"line_number":620,"context_line":"        # This method is called when driver.unmanage() or"},{"line_number":621,"context_line":"        # driver.delete_volume() fails in delete_volume(), so it is already"},{"line_number":622,"context_line":"        # in the exception handling part."}],"source_content_type":"text/x-python","patch_set":50,"id":"9aa53dc9_20934d5d","line":619,"updated":"2015-04-07 16:59:50.000000000","message":"Nit.\n\nI think volume_ref[\u0027id\u0027] and volume_id are same value. The volume_ref[\u0027id\u0027] is proper parameter for this method and the volume_id is unnecessary.","commit_id":"980bef7c629cce45a716f1347459869db7c161a9"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"ee9cabacba244227e9fe9d56cfa731ab330a6ac7","unresolved":false,"context_lines":[{"line_number":616,"context_line":"        return True"},{"line_number":617,"context_line":""},{"line_number":618,"context_line":"    def _clear_db(self, context, is_migrating_dest, volume_id, volume_ref,"},{"line_number":619,"context_line":"                  status):"},{"line_number":620,"context_line":"        # This method is called when driver.unmanage() or"},{"line_number":621,"context_line":"        # driver.delete_volume() fails in delete_volume(), so it is already"},{"line_number":622,"context_line":"        # in the exception handling part."}],"source_content_type":"text/x-python","patch_set":50,"id":"9aa53dc9_43f3e49e","line":619,"in_reply_to":"9aa53dc9_20934d5d","updated":"2015-04-08 07:34:34.000000000","message":"Done.","commit_id":"980bef7c629cce45a716f1347459869db7c161a9"}]}