)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"6297a7c0a2768828d9b8d486e1c236d792d35f26","unresolved":true,"context_lines":[{"line_number":17,"context_line":"3. Create volume C from snapshot B."},{"line_number":18,"context_line":"4. Delete volume C.  The RBD driver calls trash_move()"},{"line_number":19,"context_line":"   on the RBD image corresponding to volume C."},{"line_number":20,"context_line":"5. Attempt to delete snapshot B."},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"The deletion of snapshot B will only succeed if the RBD"},{"line_number":23,"context_line":"trash has been purged after volume C was moved to the"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"a53db92c_4fccc702","line":20,"updated":"2022-02-07 21:23:50.000000000","message":"If rbd_flatten_volume_from_snapshot is enabled (it\u0027s off by default), then when creation of volume C completes at step 3, there should no longer be a relation between snapshot B and volume C, so even with volume C in the trash, the user should be able to delete snapshot B at step 5.  (I think.)\n\nThis would make volume-create take longer, but my impression is that users expect to request-\u003eget 202-\u003epoll-until-volume-goes-\u0027available\u0027.  And at least they\u0027re blocked on something visible, unlike this mysterious delete that should succeed but doesn\u0027t (I\u0027m talking about the current code without your fix).\n\nAs you\u0027ve probably gathered from my earlier comments, I really don\u0027t like the idea of having deferred delete ignored when an operator has explicitly enabled it.  How do you feel about enhancing the documentation around this instead of changing the code (at least short-term), and let operators decide what trade-off they\u0027d like to make?","commit_id":"29c10eae7648467be214e58bf12994dcbb0e5a76"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"6297a7c0a2768828d9b8d486e1c236d792d35f26","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"7d22cce8_a5adc9a0","updated":"2022-02-07 21:23:50.000000000","message":"Another thought inline.","commit_id":"29c10eae7648467be214e58bf12994dcbb0e5a76"},{"author":{"_account_id":33612,"name":"yuval","email":"yuval@lightbitslabs.com","username":"yuval"},"change_message_id":"94626d674b40b5e149c197c65bf6d78ecaf9466b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"7a80b1e8_9e66e3db","updated":"2022-01-13 14:13:18.000000000","message":"Hey, I really lack knowledge about the driver, but from an outsider point of view - you moved from something that happens only when the configuration is missing to always doing it. looks like a big change.\nwhat the RBDProxy().remove does?","commit_id":"29c10eae7648467be214e58bf12994dcbb0e5a76"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"b75adc81cfb9595aca4116e3118a48b3a4da8f83","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"f5172bc3_aefc2de1","updated":"2022-01-27 14:34:44.000000000","message":"I did a quick check with Devstack and empty volumes and this patch worked perfectly. However, we need to work on Brian comments. ","commit_id":"29c10eae7648467be214e58bf12994dcbb0e5a76"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"110c0765d314460dc3dde6a1848df91ca64bbfa4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"48421ac8_7fa34087","updated":"2022-01-20 00:53:16.000000000","message":"I\u0027m still worried about the unconditional remove().  I did some digging in cinder and put some info on an etherpad: https://etherpad.opendev.org/p/brief-history-of-cinder-rbd-driver-deferred-delete\n\nWe can have some discussion there and try to get a handle of how this is all supposed to hang together.","commit_id":"29c10eae7648467be214e58bf12994dcbb0e5a76"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"22d3dd635663c23fa5a0ebf21b74554eee171ee5","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"70b1d1dd_212257cd","updated":"2022-01-17 14:27:45.000000000","message":"Question inline.","commit_id":"29c10eae7648467be214e58bf12994dcbb0e5a76"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"2443b42e639d80542bdb1bf150b1ada86f85bf3a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"b076be65_155e5e1b","updated":"2022-01-20 15:02:15.000000000","message":"We had problems with source volume\u0027s clean up because of the trash_moved() so it\u0027s a good sign that it\u0027s passed. \n\n{0} tempest.scenario.test_volume_boot_pattern.TestVolumeBootPattern.test_volume_boot_pattern [164.279164s] ... ok","commit_id":"29c10eae7648467be214e58bf12994dcbb0e5a76"},{"author":{"_account_id":33807,"name":"Jacob Wang","email":"jacob_wang1@dell.com","username":"jacob0522"},"change_message_id":"1c6d683361e8df1a61bfd14bb3aada48a5d156be","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"8004c470_8c0c2931","updated":"2022-01-14 09:04:44.000000000","message":"run-DellEMC Unity CI","commit_id":"29c10eae7648467be214e58bf12994dcbb0e5a76"}],"cinder/tests/unit/volume/drivers/test_rbd.py":[{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"b75adc81cfb9595aca4116e3118a48b3a4da8f83","unresolved":true,"context_lines":[{"line_number":767,"context_line":"                self.assertFalse("},{"line_number":768,"context_line":"                    drv.rbd.Image.return_value.unprotect_snap.called)"},{"line_number":769,"context_line":"                self.assertEqual("},{"line_number":770,"context_line":"                    0, drv.rbd.RBD.return_value.trash_move.call_count)"},{"line_number":771,"context_line":"                self.assertEqual("},{"line_number":772,"context_line":"                    1, drv.rbd.RBD.return_value.remove.call_count)"},{"line_number":773,"context_line":"                drv.rbd.RBD.return_value.remove.assert_called_once_with("}],"source_content_type":"text/x-python","patch_set":5,"id":"02d800da_05ebc27f","line":770,"range":{"start_line":770,"start_character":19,"end_line":770,"end_character":23},"updated":"2022-01-27 14:34:44.000000000","message":"If the self.mock_rbd.ImageBusy is raised shouldn\u0027t the trash_move call be 1 instead of 0?","commit_id":"29c10eae7648467be214e58bf12994dcbb0e5a76"}],"cinder/volume/drivers/rbd.py":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"22d3dd635663c23fa5a0ebf21b74554eee171ee5","unresolved":true,"context_lines":[{"line_number":1226,"context_line":"                if self.configuration.enable_deferred_deletion:"},{"line_number":1227,"context_line":"                    delay \u003d self.configuration.deferred_deletion_delay"},{"line_number":1228,"context_line":"                try:"},{"line_number":1229,"context_line":"                    self.RBDProxy().remove(client.ioctx, volume_name)"},{"line_number":1230,"context_line":"                    return"},{"line_number":1231,"context_line":"                except (self.rbd.ImageHasSnapshots, self.rbd.ImageBusy):"},{"line_number":1232,"context_line":"                    pass"}],"source_content_type":"text/x-python","patch_set":5,"id":"8bd183dc_948276be","line":1229,"range":{"start_line":1229,"start_character":20,"end_line":1229,"end_character":69},"updated":"2022-01-17 14:27:45.000000000","message":"So if enable_deferred_deletion is set, we aren\u0027t going to actually use it unless the remove fails.  I am probably confusing rbd with a different driver, but I thought one reason for the deferred delete was that some deletes could take a long time in the backend, and we wanted a faster response?\n\nI looked at the rbd driver docs, and the only discussion of deferred delete is the help text for the config options, which don\u0027t say anything about why you\u0027d want to use it.  But it does look like this patch is going to change behavior--or, it\u0027s not going to change behavior but it\u0027s not obvious what the behavior is supposed to be.  What I\u0027m getting at is that I think we need to revise the help strings in the config options and/or say something explicitly about deferred delete in the rbd driver docs to make clear how this is supposed to work (if this patch doesn\u0027t change behavior), and include a release note (if it does change behavior).","commit_id":"29c10eae7648467be214e58bf12994dcbb0e5a76"}]}