)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"871df0101f00c2f6c5b684bae2ffc7169a389d12","unresolved":false,"context_lines":[{"line_number":3,"context_line":"AuthorDate: 2017-01-06 13:54:53 +0800"},{"line_number":4,"context_line":"Commit:     Tao Li \u003clitao3721@126.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2017-01-09 11:10:38 +0800"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"When using ceph storage in nova, the libvirt driver should not"},{"line_number":8,"context_line":"cleanup the root disk except booting scenario such as evacuating"},{"line_number":9,"context_line":"instance failed. If the root disk is removed, maybe some user"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"ba5201f7_9648ac8c","line":6,"updated":"2017-01-09 10:22:37.000000000","message":"This commit message is missing a subject line.","commit_id":"7111b8e7d7f9ec92d11d0a2a3fdd733d7cbec798"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"a11ac64265298351207d3bcd353914ecd52ec9b1","unresolved":false,"context_lines":[{"line_number":996,"context_line":"                self._cleanup_lvm(instance, block_device_info)"},{"line_number":997,"context_line":"            if CONF.libvirt.images_type \u003d\u003d \u0027rbd\u0027 and \\"},{"line_number":998,"context_line":"                            instance.task_state \u003d\u003d task_states.SPAWNING:"},{"line_number":999,"context_line":"                # Only remove the root disk when booting instance failed."},{"line_number":1000,"context_line":"                # Otherwise, the data in root disk will be lost."},{"line_number":1001,"context_line":"                self._cleanup_rbd(instance)"},{"line_number":1002,"context_line":""},{"line_number":1003,"context_line":"        is_shared_block_storage \u003d False"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba5201f7_2d16dc4b","line":1000,"range":{"start_line":999,"start_character":0,"end_line":1000,"end_character":64},"updated":"2017-01-09 01:29:06.000000000","message":"This should be placed above the \u0027if\u0027 code.","commit_id":"a61aeafcaa555883f54f57231d280196fdcabac1"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"e274cd62f497f4d21fed12921c4e6895c867a0ad","unresolved":false,"context_lines":[{"line_number":997,"context_line":"            # Only remove the root disk when booting instance failed."},{"line_number":998,"context_line":"            # Otherwise, the data in root disk will be lost."},{"line_number":999,"context_line":"            if CONF.libvirt.images_type \u003d\u003d \u0027rbd\u0027 and \\"},{"line_number":1000,"context_line":"                            instance.task_state \u003d\u003d task_states.SPAWNING:"},{"line_number":1001,"context_line":"                self._cleanup_rbd(instance)"},{"line_number":1002,"context_line":""},{"line_number":1003,"context_line":"        is_shared_block_storage \u003d False"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba5201f7_6548eccd","line":1000,"updated":"2017-01-09 12:17:50.000000000","message":"I\u0027m not in favour of this because this is the wrong place for this logic to live. This works round the problem for RBD, but it will not fix the same problem with NFS. More immediately this further overloads the semantics of instance.task_state, which is already fragile and difficult to reason about.\n\nThe fundamental issue we have here is that by the time we get here we\u0027ve lost the context of whether or not we\u0027re rebuilding. We don\u0027t want to delete disks if we\u0027re rebuilding and we\u0027re on shared storage. This is independent of whether we want to delete instance files, meaning the logic of destroy_disks in this method is incorrect.\n\nI think that in order to do this correctly spawn() needs to learn about rebuilds (e.g. take the recreate argument passed elsewhere), and to know if it\u0027s expecting the storage to exist already. This context is available at the call site in ComputeManager._rebuild_default_impl(). Incidentally, this context is also required if spawn is to know whether disk creation should actually create disks or not.\n\nSorry to open a can of worms for you, but I\u0027m concerned that this is a hack which fails to address an underlying problem with multiple symptoms apart from this one. Worse, it adds a little more tech debt, and makes the existing code more fragile and harder to understand.","commit_id":"7111b8e7d7f9ec92d11d0a2a3fdd733d7cbec798"}]}