)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4cab4f000ab02e8bd8e7752f391615a5a0e94ece","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Chris \u003ccm876n@att.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2017-01-04 09:38:47 -0600"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Bugfix: Changes to Instance Soft Delete"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Right now enabling soft delete opens up a potential DOS on"},{"line_number":10,"context_line":"compute hosts. This is because soft delete releases the"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"ba5201f7_31a2064c","line":7,"updated":"2017-01-05 15:40:40.000000000","message":"Please use a proper first line description for this. This line is what shows up in a short summary or log listing and this is extremely ambiguous.","commit_id":"546b35738f77fda997a66e2a546e9bd41ff00866"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4cab4f000ab02e8bd8e7752f391615a5a0e94ece","unresolved":false,"context_lines":[{"line_number":14,"context_line":""},{"line_number":15,"context_line":"This patch changes the functionality of soft-deletes to not"},{"line_number":16,"context_line":"release the resources until the soft-deleted instances have been"},{"line_number":17,"context_line":"reclaimed."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"Change-Id: I6850e6ad0fa5d075815b21e6116d2cf3f7949071"},{"line_number":20,"context_line":"Closes-Bug: 1501808"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"ba5201f7_d1bd3a69","line":17,"updated":"2017-01-05 15:40:40.000000000","message":"Even from this, without reading your code, I can\u0027t tell which \"resources\" you\u0027re talking about. Please expand.","commit_id":"546b35738f77fda997a66e2a546e9bd41ff00866"}],"nova/compute/api.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"3e4482384b42de62ebabd62c5eedf21fb1d29aef","unresolved":false,"context_lines":[{"line_number":1834,"context_line":"                     instance\u003dinstance)"},{"line_number":1835,"context_line":"            return"},{"line_number":1836,"context_line":""},{"line_number":1837,"context_line":"        # If there is an instance.host (or the instance is shelved-offloaded),"},{"line_number":1838,"context_line":"        # the instance has been scheduled and sent to a cell/compute which"},{"line_number":1839,"context_line":"        # means it was pulled from the cell db."},{"line_number":1840,"context_line":"        # Normal delete should be attempted."}],"source_content_type":"text/x-python","patch_set":7,"id":"7ffa3b31_b5e184d3","line":1837,"range":{"start_line":1837,"start_character":56,"end_line":1837,"end_character":58},"updated":"2017-04-14 13:55:26.000000000","message":"This should say, if *not* shelved-offloaded...","commit_id":"6525ded466b50f4230b4c7b4b9cb820ab15c9933"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"edd9e6351c56a204cac8ea7de0ef5ce687698cdd","unresolved":false,"context_lines":[{"line_number":1834,"context_line":"                     instance\u003dinstance)"},{"line_number":1835,"context_line":"            return"},{"line_number":1836,"context_line":""},{"line_number":1837,"context_line":"        # If there is an instance.host (or the instance is shelved-offloaded),"},{"line_number":1838,"context_line":"        # the instance has been scheduled and sent to a cell/compute which"},{"line_number":1839,"context_line":"        # means it was pulled from the cell db."},{"line_number":1840,"context_line":"        # Normal delete should be attempted."}],"source_content_type":"text/x-python","patch_set":7,"id":"7ffa3b31_676c6736","line":1837,"range":{"start_line":1837,"start_character":56,"end_line":1837,"end_character":58},"in_reply_to":"7ffa3b31_b5e184d3","updated":"2017-04-15 00:56:35.000000000","message":"Ignore this, it\u0027s actually OK.","commit_id":"6525ded466b50f4230b4c7b4b9cb820ab15c9933"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"3e4482384b42de62ebabd62c5eedf21fb1d29aef","unresolved":false,"context_lines":[{"line_number":1891,"context_line":"                                    self.notifier, context, instance):"},{"line_number":1892,"context_line":"                                instance.destroy()"},{"line_number":1893,"context_line":"                        # Now commit the quota reservation to decrement usage."},{"line_number":1894,"context_line":"                        with nova_context.target_cell(context, None):"},{"line_number":1895,"context_line":"                            quotas.commit()"},{"line_number":1896,"context_line":"                    except exception.InstanceNotFound:"},{"line_number":1897,"context_line":"                        with nova_context.target_cell(context, None):"},{"line_number":1898,"context_line":"                            quotas.rollback()"},{"line_number":1899,"context_line":"                    # The instance was deleted or is already gone."},{"line_number":1900,"context_line":"                    return"},{"line_number":1901,"context_line":"                if not instance:"}],"source_content_type":"text/x-python","patch_set":7,"id":"7ffa3b31_75ebfcaf","line":1898,"range":{"start_line":1894,"start_character":24,"end_line":1898,"end_character":45},"updated":"2017-04-14 13:55:26.000000000","message":"This is what I was referring to in the previous patch set. I don\u0027t think we handle soft delete at all in this flow though, which is unfortunate. I\u0027m actually trying to unwind a bunch of this code in another series:\n\nhttps://review.openstack.org/#/c/455007/\n\nBecause this block has basically duplicated local_delete but it\u0027s missing several things that _local_delete does, like cleaning up attached volumes and networks, and soft-delete of course.","commit_id":"6525ded466b50f4230b4c7b4b9cb820ab15c9933"}],"nova/compute/manager.py":[{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"f0f97d988bf05aa5c87d690e49785b2fc932ec37","unresolved":false,"context_lines":[{"line_number":2534,"context_line":"            instance.save(expected_task_state\u003d[task_states.SOFT_DELETING])"},{"line_number":2535,"context_line":"        except Exception:"},{"line_number":2536,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":2537,"context_line":"                quotas.rollback()"},{"line_number":2538,"context_line":"        quotas.commit()"},{"line_number":2539,"context_line":"        self._notify_about_instance_usage(context, instance, \"soft_delete.end\")"},{"line_number":2540,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"3a71b18c_cf35d0db","side":"PARENT","line":2537,"range":{"start_line":2537,"start_character":16,"end_line":2537,"end_character":33},"updated":"2016-12-09 17:20:10.000000000","message":"I think you need to promote this to the top level (below line 2521) and execute it unconditionally.\n\nMy comprehension of quotas is flaky, so this narrative is mostly for me, and also so you can point out specifically where I\u0027ve misunderstood :)\n\nWhen we call _delete, we create a bunch of negative reservations. These are entries in the DB which are considered in the context of quotas somehow(?), but haven\u0027t actually been rolled into the quota yet. These negative reservations, if committed, are what will release the quota used by this instance. However, because we\u0027re soft-deleting we don\u0027t want to commit those at all. When we come to hard-delete the instance, _delete will create a bunch of new negative reservations, and it will commit those. So if we don\u0027t delete these ones, they\u0027ll be floating around until presumably some scheduled task or something like that expires them(?). That seems inefficient and potentially temporarily wrong, which is why I think we need to kill them immediately.","commit_id":"18e997156c1cfd2af718b442c7015849ad54ae9c"},{"author":{"_account_id":21813,"name":"Andrey Volkov","email":"m@amadev.ru","username":"avolkov"},"change_message_id":"39f0ff6619127dc65e5d0c24e9eef11e502635a8","unresolved":false,"context_lines":[{"line_number":2525,"context_line":"            instance.vm_state \u003d vm_states.SOFT_DELETED"},{"line_number":2526,"context_line":"            instance.task_state \u003d None"},{"line_number":2527,"context_line":"            instance.save(expected_task_state\u003d[task_states.SOFT_DELETING])"},{"line_number":2528,"context_line":"        except Exception:"},{"line_number":2529,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":2530,"context_line":"                quotas.rollback()"},{"line_number":2531,"context_line":"        self._notify_about_instance_usage(context, instance, \"soft_delete.end\")"}],"source_content_type":"text/x-python","patch_set":4,"id":"1a6eadb0_e480bfc1","line":2528,"updated":"2016-12-15 14:41:16.000000000","message":"With such position of the first quotas.rollback exception handler here looks useless.\nI think it should either put rollback at end of function or remove try/except.","commit_id":"eea9e9abc504cc5401b3de901e2737f4cbf1d0ac"},{"author":{"_account_id":22771,"name":"Christopher Martin","email":"cm876n@att.com","username":"cm876n"},"change_message_id":"37281ff2ba42fb69fc145c2bacb08bc8a20f7203","unresolved":false,"context_lines":[{"line_number":2525,"context_line":"            instance.vm_state \u003d vm_states.SOFT_DELETED"},{"line_number":2526,"context_line":"            instance.task_state \u003d None"},{"line_number":2527,"context_line":"            instance.save(expected_task_state\u003d[task_states.SOFT_DELETING])"},{"line_number":2528,"context_line":"        except Exception:"},{"line_number":2529,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":2530,"context_line":"                quotas.rollback()"},{"line_number":2531,"context_line":"        self._notify_about_instance_usage(context, instance, \"soft_delete.end\")"}],"source_content_type":"text/x-python","patch_set":4,"id":"1a6eadb0_cda59f47","line":2528,"in_reply_to":"1a6eadb0_e480bfc1","updated":"2016-12-16 15:57:20.000000000","message":"Done","commit_id":"eea9e9abc504cc5401b3de901e2737f4cbf1d0ac"}],"nova/tests/unit/compute/test_compute_api.py":[{"author":{"_account_id":21813,"name":"Andrey Volkov","email":"m@amadev.ru","username":"avolkov"},"change_message_id":"39f0ff6619127dc65e5d0c24e9eef11e502635a8","unresolved":false,"context_lines":[{"line_number":1078,"context_line":"        if soft_delete:"},{"line_number":1079,"context_line":"            quota.QUOTAS.rollback(self.context, reservations,"},{"line_number":1080,"context_line":"                                  project_id\u003dinst.project_id,"},{"line_number":1081,"context_line":"                                  user_id\u003dinst.user_id)"},{"line_number":1082,"context_line":""},{"line_number":1083,"context_line":"        if commit_quotas:"},{"line_number":1084,"context_line":"            # Local delete or when we\u0027re testing API cell."}],"source_content_type":"text/x-python","patch_set":4,"id":"1a6eadb0_bf7a4e86","side":"PARENT","line":1081,"updated":"2016-12-15 14:41:16.000000000","message":"Besides that _test_delete is really not good I think\nwe still need to check quotas were rollbacked for soft delete. Did you consider to make separate test for that?","commit_id":"18e997156c1cfd2af718b442c7015849ad54ae9c"},{"author":{"_account_id":22771,"name":"Christopher Martin","email":"cm876n@att.com","username":"cm876n"},"change_message_id":"37281ff2ba42fb69fc145c2bacb08bc8a20f7203","unresolved":false,"context_lines":[{"line_number":1078,"context_line":"        if soft_delete:"},{"line_number":1079,"context_line":"            quota.QUOTAS.rollback(self.context, reservations,"},{"line_number":1080,"context_line":"                                  project_id\u003dinst.project_id,"},{"line_number":1081,"context_line":"                                  user_id\u003dinst.user_id)"},{"line_number":1082,"context_line":""},{"line_number":1083,"context_line":"        if commit_quotas:"},{"line_number":1084,"context_line":"            # Local delete or when we\u0027re testing API cell."}],"source_content_type":"text/x-python","patch_set":4,"id":"1a6eadb0_cd8e7fcf","side":"PARENT","line":1081,"in_reply_to":"1a6eadb0_bf7a4e86","updated":"2016-12-16 15:57:20.000000000","message":"No I didn\u0027t but I can do that.","commit_id":"18e997156c1cfd2af718b442c7015849ad54ae9c"}]}
