)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":16708,"name":"Kendall Nelson","display_name":"Kendall (diablo_rojo)","email":"kennelson11@gmail.com","username":"kjnelson"},"change_message_id":"be839baba96150a5a9e3c58df440feb1e272cd15","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Add retries to delete volume or snapshot"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"During volume or snapshot deletion we catch VolumeIsBusy or"},{"line_number":10,"context_line":"SnapshotIsBusy and ignore them setting a \u0027available\u0027 state."},{"line_number":11,"context_line":"Now after N tries of volume (or snapshot) deletion failed"},{"line_number":12,"context_line":"it\u0027s setting state to \u0027error-deleting\u0027."},{"line_number":13,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"1a4dcd0f_8884565e","line":10,"updated":"2015-08-05 16:20:58.000000000","message":"a -\u003e an","commit_id":"1bee208d90071481aa45ec8aa1f6d4b25266ea19"}],"cinder/volume/manager.py":[{"author":{"_account_id":16973,"name":"Skyler Berg","email":"skylertheberg@gmail.com","username":"sberg"},"change_message_id":"e96e12c545ae125019e5a1bcafbd46fb3429f3dd","unresolved":false,"context_lines":[{"line_number":545,"context_line":"        self._notify_about_volume_usage(context, volume_ref, \"delete.start\")"},{"line_number":546,"context_line":""},{"line_number":547,"context_line":"        @utils.retry(exception.VolumeIsBusy, retries\u003d3)"},{"line_number":548,"context_line":"        def _try_remove_volume(context, volume_ref):"},{"line_number":549,"context_line":"            try:"},{"line_number":550,"context_line":"                # NOTE(flaper87): Verify the driver is enabled"},{"line_number":551,"context_line":"                # before going forward. The exception will be caught"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a4dcd0f_6130459f","line":548,"updated":"2015-08-04 17:28:07.000000000","message":"Why are you using a nested function instead of adding a new private method?","commit_id":"faebff40632922fb878345b9a8dbff86aa5b15d3"},{"author":{"_account_id":14305,"name":"Yuriy Nesenenko","email":"ynesenenko@mirantis.com","username":"yuriy_n"},"change_message_id":"62dbdcd8029cbcdc645ee36bb07ecabf4c9aed56","unresolved":false,"context_lines":[{"line_number":545,"context_line":"        self._notify_about_volume_usage(context, volume_ref, \"delete.start\")"},{"line_number":546,"context_line":""},{"line_number":547,"context_line":"        @utils.retry(exception.VolumeIsBusy, retries\u003d3)"},{"line_number":548,"context_line":"        def _try_remove_volume(context, volume_ref):"},{"line_number":549,"context_line":"            try:"},{"line_number":550,"context_line":"                # NOTE(flaper87): Verify the driver is enabled"},{"line_number":551,"context_line":"                # before going forward. The exception will be caught"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a4dcd0f_c1300beb","line":548,"in_reply_to":"1a4dcd0f_6130459f","updated":"2015-08-05 12:34:21.000000000","message":"This function is used only here.","commit_id":"faebff40632922fb878345b9a8dbff86aa5b15d3"},{"author":{"_account_id":16973,"name":"Skyler Berg","email":"skylertheberg@gmail.com","username":"sberg"},"change_message_id":"e96e12c545ae125019e5a1bcafbd46fb3429f3dd","unresolved":false,"context_lines":[{"line_number":562,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":563,"context_line":"                    # If this is a destination volume, we have to clear the"},{"line_number":564,"context_line":"                    # database record to avoid user confusion."},{"line_number":565,"context_line":"                    self._clear_db(context, is_migrating_dest, volume_ref,"},{"line_number":566,"context_line":"                                   \u0027error_deleting\u0027)"},{"line_number":567,"context_line":""},{"line_number":568,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a4dcd0f_616285af","line":565,"updated":"2015-08-04 17:28:07.000000000","message":"If VolumeIsBusy is raised on the first try we will set the state to error_deleting. If it succeeds on the second attempt, won\u0027t it still be in the error_deleting state?","commit_id":"faebff40632922fb878345b9a8dbff86aa5b15d3"},{"author":{"_account_id":2759,"name":"Huang Zhiteng","email":"winston.d@gmail.com","username":"zhiteng-huang"},"change_message_id":"42df54ba909a77c0c227fe00fd0c478464c11b5b","unresolved":false,"context_lines":[{"line_number":562,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":563,"context_line":"                    # If this is a destination volume, we have to clear the"},{"line_number":564,"context_line":"                    # database record to avoid user confusion."},{"line_number":565,"context_line":"                    self._clear_db(context, is_migrating_dest, volume_ref,"},{"line_number":566,"context_line":"                                   \u0027error_deleting\u0027)"},{"line_number":567,"context_line":""},{"line_number":568,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a4dcd0f_a0b4f4d0","line":565,"in_reply_to":"1a4dcd0f_616285af","updated":"2015-08-05 02:33:02.000000000","message":"Right, _clear_db() shouldn\u0027t be call here between retries and volume status should remain unchanged.","commit_id":"faebff40632922fb878345b9a8dbff86aa5b15d3"},{"author":{"_account_id":8247,"name":"Thang Pham","email":"thang.g.pham@gmail.com","username":"thang.pham"},"change_message_id":"c4250a436535ed3d31528aee1f757886673ea7a9","unresolved":false,"context_lines":[{"line_number":546,"context_line":""},{"line_number":547,"context_line":"        @utils.retry(exception.VolumeIsBusy, retries\u003d3)"},{"line_number":548,"context_line":"        def _try_remove_volume(context, volume_ref):"},{"line_number":549,"context_line":"            utils.require_driver_initialized(self.driver)"},{"line_number":550,"context_line":"            self.driver.remove_export(context, volume_ref)"},{"line_number":551,"context_line":"            if unmanage_only:"},{"line_number":552,"context_line":"                self.driver.unmanage(volume_ref)"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a4dcd0f_14b82fc7","line":549,"updated":"2015-08-06 14:35:59.000000000","message":"Does utils.require_driver_initialized and self.driver.remove_export raise exception.VolumeIsBusy?  I am not seeing these methods raise that exception from a brief grep.  I am only seeing self.driver.delete_volume raise the exception.  In that case, they do not have to be in _try_remove_volume.","commit_id":"743388d49b221e2f9aa84497289888fc86c50917"},{"author":{"_account_id":8247,"name":"Thang Pham","email":"thang.g.pham@gmail.com","username":"thang.pham"},"change_message_id":"c4250a436535ed3d31528aee1f757886673ea7a9","unresolved":false,"context_lines":[{"line_number":710,"context_line":""},{"line_number":711,"context_line":"            # Pass context so that drivers that want to use it, can,"},{"line_number":712,"context_line":"            # but it is not a requirement for all drivers."},{"line_number":713,"context_line":"            snapshot.context \u003d context"},{"line_number":714,"context_line":"            snapshot.save()"},{"line_number":715,"context_line":"            self.driver.delete_snapshot(snapshot)"},{"line_number":716,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"1a4dcd0f_992798c9","line":713,"updated":"2015-08-06 14:35:59.000000000","message":"I do not believe it is necessary to update the snapshot.context to the same value for every try.  You could put this outside the _try_remove_snapshot.  Maybe before L718.","commit_id":"743388d49b221e2f9aa84497289888fc86c50917"},{"author":{"_account_id":2243,"name":"John Griffith","email":"john.griffith8@gmail.com","username":"john-griffith"},"change_message_id":"113cc06211c8acf3db06dccb39b57a5cbba76ba7","unresolved":false,"context_lines":[{"line_number":544,"context_line":"                                 \u0027target:\u0027))"},{"line_number":545,"context_line":"        self._notify_about_volume_usage(context, volume_ref, \"delete.start\")"},{"line_number":546,"context_line":""},{"line_number":547,"context_line":"        @utils.retry(exception.VolumeIsBusy, retries\u003d3)"},{"line_number":548,"context_line":"        def _try_remove_volume(context, volume_ref):"},{"line_number":549,"context_line":"            if unmanage_only:"},{"line_number":550,"context_line":"                self.driver.unmanage(volume_ref)"}],"source_content_type":"text/x-python","patch_set":4,"id":"1a4dcd0f_5ed0d1f7","line":547,"updated":"2015-08-07 17:55:33.000000000","message":"I have a couple problems with this... first there\u0027s only a handful of drivers that even use this exception; for the LVM driver for example the only place we use it is on \"delete_volume\" that has a snapshot associated with it.\n\nIn that case you can retry all you like but this is going to raise every time until the snapshots are deleted.  My comments in the bug refer to iscsi delete/remove-export failures that fail due to things like tgtadm (or even cases where we used to get lvm-remove failures) for device-busy.\n\nIMO those issues need to be addressed in the drivers themeselves, not in the manager.  The cases that are added here aren\u0027t really all that useful in the general case that I can see.","commit_id":"362560feca77fbbfcaa7aaa4ce9cf7019683a3fa"}]}