)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"06bca7aa1e5297e2a7d28a6fcceac222694e9133","unresolved":true,"context_lines":[{"line_number":27,"context_line":"  2. If busy and the volume has child dependencies,"},{"line_number":28,"context_line":"     flatten those dependencies with RBD flatten()"},{"line_number":29,"context_line":"  3. Attempt RBD remove() again"},{"line_number":30,"context_line":"  4. Use trash_move() instead to move the image to the trash"},{"line_number":31,"context_line":"     instead of remove()."},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"This allows Cinder deletions to succeed in most scenarios where"},{"line_number":34,"context_line":"they would previously fail."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":17,"id":"f7cde6f4_6fd7644b","line":31,"range":{"start_line":30,"start_character":5,"end_line":31,"end_character":25},"updated":"2022-08-11 13:42:00.000000000","message":"TODO: add info on why this is needed  (it succeeds when you have trashed a snapshot, but remove() won\u0027t)","commit_id":"448ed0dd27c5afb387e239312b2c885fcdacbe09"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"852a55f25e7da2031ecf4207e0be834d8f519500","unresolved":false,"context_lines":[{"line_number":27,"context_line":"  2. If busy and the volume has child dependencies,"},{"line_number":28,"context_line":"     flatten those dependencies with RBD flatten()"},{"line_number":29,"context_line":"  3. Attempt RBD remove() again"},{"line_number":30,"context_line":"  4. Use trash_move() instead to move the image to the trash"},{"line_number":31,"context_line":"     instead of remove()."},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"This allows Cinder deletions to succeed in most scenarios where"},{"line_number":34,"context_line":"they would previously fail."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":17,"id":"3d6c22bb_15050818","line":31,"range":{"start_line":30,"start_character":5,"end_line":31,"end_character":25},"in_reply_to":"f7cde6f4_6fd7644b","updated":"2022-08-17 14:09:44.000000000","message":"Done","commit_id":"448ed0dd27c5afb387e239312b2c885fcdacbe09"},{"author":{"_account_id":14084,"name":"Einst Crazy","email":"yu.changcai@99cloud.net","username":"Einst"},"change_message_id":"84a4605b7cbbe5f43488b75ca69cdd129c3a00b4","unresolved":true,"context_lines":[{"line_number":35,"context_line":"     This allows Cinder deletion of a volume (volume1) to proceed"},{"line_number":36,"context_line":"     in the scenario where volume2 was cloned from snapshot s1 of"},{"line_number":37,"context_line":"     volume1, and snapshot s1 has been trashed and not fully"},{"line_number":38,"context_line":"     deleted from the RBD backend.  (Snapshots in the trash"},{"line_number":39,"context_line":"     namespace are no longer visible but are still in the"},{"line_number":40,"context_line":"     dependency chain.)"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"This allows Cinder deletions to succeed in most scenarios where"},{"line_number":43,"context_line":"they would previously fail."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":19,"id":"659535d4_678c6740","line":40,"range":{"start_line":38,"start_character":37,"end_line":40,"end_character":22},"updated":"2022-08-18 05:44:00.000000000","message":"when the snapshot will be delete in the backend.","commit_id":"1319919e2e08f56934fd709b0fa9b767a1031cdd"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"c0c888728f7cfe338396aa1912b4dca93c86f1d7","unresolved":false,"context_lines":[{"line_number":35,"context_line":"     This allows Cinder deletion of a volume (volume1) to proceed"},{"line_number":36,"context_line":"     in the scenario where volume2 was cloned from snapshot s1 of"},{"line_number":37,"context_line":"     volume1, and snapshot s1 has been trashed and not fully"},{"line_number":38,"context_line":"     deleted from the RBD backend.  (Snapshots in the trash"},{"line_number":39,"context_line":"     namespace are no longer visible but are still in the"},{"line_number":40,"context_line":"     dependency chain.)"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"This allows Cinder deletions to succeed in most scenarios where"},{"line_number":43,"context_line":"they would previously fail."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":19,"id":"9d1eaf13_36ef4dc0","line":40,"range":{"start_line":38,"start_character":37,"end_line":40,"end_character":22},"in_reply_to":"659535d4_678c6740","updated":"2022-08-19 14:34:03.000000000","message":"Snapshots are deleted from the trash when a trash purge operation occurs either initiated by the RBD driver in Cinder or on the backend.","commit_id":"1319919e2e08f56934fd709b0fa9b767a1031cdd"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"87573f0c343656464a549badf6b72814802cd08b","unresolved":true,"context_lines":[{"line_number":30,"context_line":"     flatten those dependencies with RBD flatten()"},{"line_number":31,"context_line":"  3. Attempt RBD remove() again"},{"line_number":32,"context_line":"     This will succeed in more cases than (1) would have."},{"line_number":33,"context_line":"  4. Use trash_move() instead to move the image to the trash"},{"line_number":34,"context_line":"     instead of remove()."},{"line_number":35,"context_line":"     This allows Cinder deletion of a volume (volume1) to proceed"},{"line_number":36,"context_line":"     in the scenario where volume2 was cloned from snapshot s1 of"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":26,"id":"b6108e0f_5f9a5912","line":33,"range":{"start_line":33,"start_character":5,"end_line":33,"end_character":8},"updated":"2022-09-05 09:50:40.000000000","message":"nit: If remove still fails, then","commit_id":"c9d1dcce870ee21b3a9c81e96445614a172a16ac"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"5c242a010c24998436f072bf3e3bf9a0e5316954","unresolved":true,"context_lines":[{"line_number":30,"context_line":"     flatten those dependencies with RBD flatten()"},{"line_number":31,"context_line":"  3. Attempt RBD remove() again"},{"line_number":32,"context_line":"     This will succeed in more cases than (1) would have."},{"line_number":33,"context_line":"  4. Use trash_move() instead to move the image to the trash"},{"line_number":34,"context_line":"     instead of remove()."},{"line_number":35,"context_line":"     This allows Cinder deletion of a volume (volume1) to proceed"},{"line_number":36,"context_line":"     in the scenario where volume2 was cloned from snapshot s1 of"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":26,"id":"d7bf5d4e_09475b8c","line":33,"range":{"start_line":33,"start_character":5,"end_line":33,"end_character":8},"in_reply_to":"b6108e0f_5f9a5912","updated":"2022-09-27 09:05:37.000000000","message":"+1","commit_id":"c9d1dcce870ee21b3a9c81e96445614a172a16ac"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"9ff8124a715a5a399ee862d4ea81bd6f146cc0f5","unresolved":false,"context_lines":[{"line_number":30,"context_line":"     flatten those dependencies with RBD flatten()"},{"line_number":31,"context_line":"  3. Attempt RBD remove() again"},{"line_number":32,"context_line":"     This will succeed in more cases than (1) would have."},{"line_number":33,"context_line":"  4. Use trash_move() instead to move the image to the trash"},{"line_number":34,"context_line":"     instead of remove()."},{"line_number":35,"context_line":"     This allows Cinder deletion of a volume (volume1) to proceed"},{"line_number":36,"context_line":"     in the scenario where volume2 was cloned from snapshot s1 of"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":26,"id":"912cbc82_df409142","line":33,"range":{"start_line":33,"start_character":5,"end_line":33,"end_character":8},"in_reply_to":"d7bf5d4e_09475b8c","updated":"2023-07-12 17:47:02.000000000","message":"Done","commit_id":"c9d1dcce870ee21b3a9c81e96445614a172a16ac"},{"author":{"_account_id":5997,"name":"Walt","display_name":"Hemna","email":"waboring@hemna.com","username":"walter-boring","status":"SAP"},"change_message_id":"b4ed03693e3607015ba0bb888db2d0b45e2ceffc","unresolved":true,"context_lines":[{"line_number":38,"context_line":"     deleted from the RBD backend.  (Snapshots in the trash"},{"line_number":39,"context_line":"     namespace are no longer visible but are still in the"},{"line_number":40,"context_line":"     dependency chain.)"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"This allows Cinder deletions to succeed in most scenarios where"},{"line_number":43,"context_line":"they would previously fail."},{"line_number":44,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":26,"id":"6f3dcc87_6db7da01","line":41,"updated":"2022-09-01 17:32:39.000000000","message":"so does this mean that rbd will have items in this trash bin occupying space that cinder can\u0027t then control and delete?  Will there be a discrepancy with the reported free space vs what cinder has allocated?\n\nWhen does this trash get emptied if ever?\n\nIF there is space occupied and there is no way for an operator to clean it up, that wouldn\u0027t be nice either, especially if as an operator I\u0027d want to drain a particular pool and move things elsewhere, or are getting full on space, but there is hidden space being occupied that cinder can\u0027t clean out.","commit_id":"c9d1dcce870ee21b3a9c81e96445614a172a16ac"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"87573f0c343656464a549badf6b72814802cd08b","unresolved":false,"context_lines":[{"line_number":38,"context_line":"     deleted from the RBD backend.  (Snapshots in the trash"},{"line_number":39,"context_line":"     namespace are no longer visible but are still in the"},{"line_number":40,"context_line":"     dependency chain.)"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"This allows Cinder deletions to succeed in most scenarios where"},{"line_number":43,"context_line":"they would previously fail."},{"line_number":44,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":26,"id":"6a33e8f3_3cdb011e","line":41,"in_reply_to":"6f3dcc87_6db7da01","updated":"2022-09-05 09:50:40.000000000","message":"Usage of the trash for deletion is already a Cinder RBD feature, and Cinder can periodically purge the trash (configuration options enable_deferred_deletion, deferred_deletion_delay, and deferred_deletion_purge_interval).\n\nDepending on the Ceph version, purging can also be scheduled on the Ceph cluster itself, and iirc an operator can actually purge a specific image in the trash manually as well.","commit_id":"c9d1dcce870ee21b3a9c81e96445614a172a16ac"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"5c242a010c24998436f072bf3e3bf9a0e5316954","unresolved":true,"context_lines":[{"line_number":16,"context_line":" volume1"},{"line_number":17,"context_line":"   -\u003e snapshot s1 of volume 1"},{"line_number":18,"context_line":"     -\u003e volume2 cloned from snapshot 1"},{"line_number":19,"context_line":"Deleting snapshot s1 would fail with ImageBusy."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"This is fixed by using RBD flatten operations to break"},{"line_number":22,"context_line":"dependencies between volumes/snapshots and other RBD volumes"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":28,"id":"63aabee5_33bbe68f","line":19,"range":{"start_line":19,"start_character":0,"end_line":19,"end_character":47},"updated":"2022-09-27 09:05:37.000000000","message":"are we sure about this? the rbd doc says it will raiseImageHasSnapshots\n\n\"Note that all snapshots must be deleted before the image can be removed. If there are snapshots left, ImageHasSnapshots is raised.\"\n\n--\nI checked in the code that we\u0027re catching both so we are good but the message can be improved here\n\n(self.rbd.ImageHasSnapshots, self.rbd.ImageBusy)","commit_id":"09555fe086beeb39ac2e08c2bbca56bfcd8d4eac"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"fc668616e0baaae0992dd67a4007112fb3eb7cda","unresolved":false,"context_lines":[{"line_number":16,"context_line":" volume1"},{"line_number":17,"context_line":"   -\u003e snapshot s1 of volume 1"},{"line_number":18,"context_line":"     -\u003e volume2 cloned from snapshot 1"},{"line_number":19,"context_line":"Deleting snapshot s1 would fail with ImageBusy."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"This is fixed by using RBD flatten operations to break"},{"line_number":22,"context_line":"dependencies between volumes/snapshots and other RBD volumes"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":28,"id":"be6a3a5c_f736b9e6","line":19,"range":{"start_line":19,"start_character":0,"end_line":19,"end_character":47},"in_reply_to":"63aabee5_33bbe68f","updated":"2023-08-08 18:47:57.000000000","message":"Image removal can raise ImageHasSnapshots, but I don\u0027t see that snapshot removal will?","commit_id":"09555fe086beeb39ac2e08c2bbca56bfcd8d4eac"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"5c242a010c24998436f072bf3e3bf9a0e5316954","unresolved":false,"context_lines":[{"line_number":23,"context_line":"or snapshots."},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"Delete now works as follows:"},{"line_number":26,"context_line":"  1. Attempt RBD remove()"},{"line_number":27,"context_line":"     This is the \"fast path\" for removing a simple volume"},{"line_number":28,"context_line":"     that involves no extra overhead."},{"line_number":29,"context_line":"  2. If busy and the volume has child dependencies,"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":28,"id":"4641298d_20095d44","line":26,"range":{"start_line":26,"start_character":17,"end_line":26,"end_character":25},"updated":"2022-09-27 09:05:37.000000000","message":"adding for reference\nhttps://docs.ceph.com/en/latest/rbd/api/librbdpy/#rbd.RBD.remove","commit_id":"09555fe086beeb39ac2e08c2bbca56bfcd8d4eac"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":12988,"name":"Peter Penchev","email":"openstack-dev@storpool.com","username":"ppenchev"},"change_message_id":"9d63f5c0745e77efc54cd6ba5019737f45dedc5b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"573165b6_13004e98","updated":"2022-06-15 09:55:07.000000000","message":"run-storpool-ci","commit_id":"aee544a22db370f7c0697e190828fec7a14b586a"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"337b450db34e0b50ea6e399393f9b5e54d74160e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"604f736b_ddf26b59","updated":"2022-06-15 21:24:14.000000000","message":"Still working on Alan\u0027s comments","commit_id":"16d2c28e472c4904743068e2cf21d6d45ec807c9"},{"author":{"_account_id":12988,"name":"Peter Penchev","email":"openstack-dev@storpool.com","username":"ppenchev"},"change_message_id":"22a75624ad07fececcff01fe273b2acf1ff92917","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"587f7312_f0a6f2b4","updated":"2022-06-16 09:50:54.000000000","message":"run-storpoolci","commit_id":"16d2c28e472c4904743068e2cf21d6d45ec807c9"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"6e4d55ee533ba7e7f6132fec2ec61d2900b5cb49","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"12b735b1_6c76c000","updated":"2022-06-24 14:16:18.000000000","message":"Zuul failed because of tempest.lib.exceptions.ServerFault: Got server fault. Rechecking","commit_id":"c61f7aa7928cd42b2a238c76ab0b565b840ce67c"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"54600e632ec7e84b29768e7c1d35d22f51968267","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"dc635948_451741d2","updated":"2022-06-24 14:16:39.000000000","message":"recheck cinder-tempest-plugin-lvm-lio-barbican","commit_id":"c61f7aa7928cd42b2a238c76ab0b565b840ce67c"},{"author":{"_account_id":33807,"name":"Jacob Wang","email":"jacob_wang1@dell.com","username":"jacob0522"},"change_message_id":"1eebefcc9bee9cae14654e43466e154f00d6acd1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":15,"id":"3619fbb4_b55274a4","updated":"2022-07-13 04:48:08.000000000","message":"run-DellEMC PowerStore CI","commit_id":"8839f09295c3963775d7460e676ea7cf8dfff16a"},{"author":{"_account_id":33807,"name":"Jacob Wang","email":"jacob_wang1@dell.com","username":"jacob0522"},"change_message_id":"4c9b0668fbc72df599a5f731b895b7caa56e9e20","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":15,"id":"7767e1f4_99f68cc5","updated":"2022-07-13 06:11:07.000000000","message":"run-DellEMC PowerStore CI","commit_id":"8839f09295c3963775d7460e676ea7cf8dfff16a"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"94019af109ce527f496a882b570e9635f4746e87","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":18,"id":"b47fc1d8_d42e51eb","updated":"2022-08-17 14:07:16.000000000","message":"Needs update for new pep8 rules.","commit_id":"b7c65dc02ea29d32b992677e0e4364532a979127"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"bedc2d0e9edd652f49ed4f7ea9466e08554890e2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":19,"id":"0f595a13_b377b78e","updated":"2022-08-17 19:13:48.000000000","message":"recheck","commit_id":"1319919e2e08f56934fd709b0fa9b767a1031cdd"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"3bd6c04f08b12c55b2a6e9db225cf9429672dba9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":21,"id":"34fa6ddb_e46eb9d3","updated":"2022-08-22 18:26:27.000000000","message":"Still working on issues noted in the previous patchset.","commit_id":"eab8364f0a2b4c79188297d7e41e08744d37718b"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"145308ef43a8dc421c701091cd7bd97f3840265d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":23,"id":"2a2b4309_112fd498","updated":"2022-08-23 21:47:18.000000000","message":"More to patch up here, but wanted to get some tempest results w/ new tests.","commit_id":"dc59a5540b37fd22fd5a70547c603b927cf33c27"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"8f0d1031558a1b9dc09a7b0b78904c19e9ade322","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":25,"id":"5af03dfd_aa4dd037","updated":"2022-08-25 15:41:32.000000000","message":"recheck\n\ncinder-grenade-mn-sub-volbak failed w/\n  More than one Router exists with the name \u0027neutron_grenade\u0027.","commit_id":"7dadf48553ba7e73f06b3f2b5bff6c38e09c6968"},{"author":{"_account_id":16137,"name":"Tobias Urdin","email":"tobias.urdin@binero.com","username":"tobasco"},"change_message_id":"e04bc4c2e1ada4d3d054f2ed82f2070a0c42abbe","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":26,"id":"64f62966_737bfd7e","updated":"2022-09-01 13:29:47.000000000","message":"I will update just based on the technical aspect from an operators view.\n\nThis makes a lot of sense, for us personally this would not incur any additional processing because we already flatten volumes created from snapshots, so we already do capacity planning based on actual usage since we potentially lose a little bit of storage doing that.\n\nBut I agree that this should be the default behaviour because it\u0027s annoying when you cannot delete the snapshot because it\u0027s been cloned from to a new volume.\n\nThe ratelimit part is I think very important, but I have a question, has it been considered to have this behaviour configurable but still be enabled by default?\n\nI can see people that does not want to flatten, or that has a very tight restriction on storage usage getting potential problems.","commit_id":"c9d1dcce870ee21b3a9c81e96445614a172a16ac"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"a943b3ed5f7c0b774ef2c475239bd055de8aea1b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":26,"id":"5b80c670_e8ef2e01","updated":"2022-09-15 15:05:53.000000000","message":"More updates/responses to follow.","commit_id":"c9d1dcce870ee21b3a9c81e96445614a172a16ac"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"87573f0c343656464a549badf6b72814802cd08b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":26,"id":"a20043b3_bf3ac657","updated":"2022-09-05 09:50:40.000000000","message":"Thanks for the big effort!!\n\n-1: Patch is missing a release note.\n\nI\u0027m sure that I\u0027m missing some things in my review (sorry in advance if I find them on the next patch review), but there are a couple of things that I think we need to fix (mostly calls to C libraries without proxy) and some nits that may be interesting to address.\n\nI think it would be good to document in doc/source/configuration/block-storage/drivers/ceph-rbd-volume-driver.rst the behavior of the trash usage of the Cinder driver as well as this delete with flattening behavior.  But that\u0027s something that can be done in a follow up patch.\n\nSomething else for a follow up patch would be taking into consideration the available space and the reserved_percentage before flattening, so VMs won\u0027t run out of space because something is being deleted.","commit_id":"c9d1dcce870ee21b3a9c81e96445614a172a16ac"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"31373a10258c2c3f44ca3fdd36bc1f1fa0b43f6c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":26,"id":"ba8d6480_34b66fe1","updated":"2022-09-01 13:44:27.000000000","message":"cinder-plugin-ceph-tempest failed with this Nova error:\n\nAug 31 17:08:35.289287 ubuntu-focal-inmotion-iad3-0030924357 nova-compute[110430]: WARNING nova.virt.block_device [None req-9802e610-2615-443a-aa21-e27bace190aa tempest-VolumesSnapshotTestJSON-1113571180 tempest-VolumesSnapshotTestJSON-1113571180-project] [instance: e25ce761-7e35-4e51-99ef-54fef2f56bc8] Guest refused to detach volume 3f46801d-39f6-41ef-9153-ce52c3f795c2: nova.exception.DeviceDetachFailed: Device detach failed for vdb: Run out of retry while detaching device vdb with device alias virtio-disk1 from instance e25ce761-7e35-4e51-99ef-54fef2f56bc8 from the live domain config. Device is still attached to the guest.\n","commit_id":"c9d1dcce870ee21b3a9c81e96445614a172a16ac"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"d27246d1704d166d5e9b294787a9786bb0cbab09","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":26,"id":"c132d1fb_0f688fb5","updated":"2022-08-31 16:05:06.000000000","message":"recheck grenade failed during devstack setup","commit_id":"c9d1dcce870ee21b3a9c81e96445614a172a16ac"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"e120eccdc10c3612feaa22f3935cc14adcfb28a6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":27,"id":"0d1b0571_93f1151a","updated":"2022-09-15 15:06:24.000000000","message":"Continue w/ comments from previous patchset.","commit_id":"493fb73ef09b2b6ec34e9d35f37236a2aa863aaf"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"9bea0e40f78270ceeec9d7b439f4c747438197c1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":27,"id":"e9cf36e9_34bb4432","updated":"2022-09-15 17:17:26.000000000","message":"recheck\n\ndebugging cinder-plugin-ceph-tempest","commit_id":"493fb73ef09b2b6ec34e9d35f37236a2aa863aaf"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"1dbd31b8e6aab7defa0bd9e4b3a580347aef6908","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":28,"id":"5ec955f5_ef8b7043","updated":"2022-10-06 10:28:46.000000000","message":"As pointed out by Eric, this needs more work","commit_id":"09555fe086beeb39ac2e08c2bbca56bfcd8d4eac"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"88105982035f99ff56f978b94c9d6dfa6c684092","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":28,"id":"27a38679_d5b5c5bd","updated":"2022-09-23 11:36:57.000000000","message":"I\u0027m not sure if this is discussed before but we will also need a releasenote since it\u0027s fixing a bug","commit_id":"09555fe086beeb39ac2e08c2bbca56bfcd8d4eac"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"774467c1177be3ee40691d35ce2972d1afa6d84f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":28,"id":"35cc7a83_e57e6ab1","updated":"2022-09-29 04:12:03.000000000","message":"LGTM, I really don\u0027t have anything to add that other reviewers haven\u0027t already noticed.  Looks like there are a few open comments you can address, otherwise, as far as I can tell, this looks ready.","commit_id":"09555fe086beeb39ac2e08c2bbca56bfcd8d4eac"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"5c242a010c24998436f072bf3e3bf9a0e5316954","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":28,"id":"6e2f1d7f_84ba4b4e","updated":"2022-09-27 09:05:37.000000000","message":"Looks good, I don\u0027t have any major concerns but the past comments needs to be addresssed.","commit_id":"09555fe086beeb39ac2e08c2bbca56bfcd8d4eac"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"f0e869ca5dde2a19772ade770c1aaa7612455884","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":28,"id":"67434cc7_52228f17","updated":"2022-09-29 09:19:32.000000000","message":"Since we\u0027re very close to the deadline (today) and the improvements mentioned are minor. The functionality seems fine so LGTM.","commit_id":"09555fe086beeb39ac2e08c2bbca56bfcd8d4eac"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"c9d65fc681dcceec193ebb7c5a6c10dcc78332fe","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":28,"id":"5520325b_78b2dcbb","updated":"2022-10-03 15:17:41.000000000","message":"Taking a further look at some of the review comments here.","commit_id":"09555fe086beeb39ac2e08c2bbca56bfcd8d4eac"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"ad92dae043244017ae982923ad050a05a7314cab","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":28,"id":"fc8c9705_3e6cff7f","updated":"2022-09-22 16:24:46.000000000","message":"recheck debugging rbd","commit_id":"09555fe086beeb39ac2e08c2bbca56bfcd8d4eac"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"c7472aacb66ce83c0e10069dc3ee52e8795dc25b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":29,"id":"3c60f3cb_01041cb2","updated":"2023-02-06 13:58:55.000000000","message":"Add a release note","commit_id":"6d083e182da722794afbfaee955e0bff774662c7"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"d2b3379c5dc1f0cba7986423a7d614ccab8e1d72","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":30,"id":"bdbd6463_8fddcc97","updated":"2023-02-07 04:52:16.000000000","message":"I see some coding level issues.\n\nAlso, some of the old comments might have been ignored, in particular what Rajat et.al wrote about the commit message.","commit_id":"8a0d456f1f8ecec9cfb816c720cfa23e3ea9f9aa"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"a8d39f1486d152d0a2b7d72c1aeb40ab9fa3bc48","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":34,"id":"2f6bd0ec_dac47b76","updated":"2023-08-16 17:51:17.000000000","message":"I have a few questions and thoughts to consider, but the overall approach looks fundamentally sound.","commit_id":"e7b5e1b80d99af31d802013a563633e230865ca8"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"97672b9af4fcd17ffae1bd9d7a7738e6488adae9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":34,"id":"906392ec_837e2b79","updated":"2023-08-14 23:47:30.000000000","message":"I\u0027m still going to +1 this, but it\u0027s such an amazing pile of baggage:\n- messages either say \"something volume\" or \"rbd volume\" or \"RBD volume\"\n- \"allowing deletion to proceed\" before the return is a bit weird\n- the retry functions with main exit (return True) from inside of an exception\n- depeted\u003dFalse way before it is set in a try:\n- self._delete_backup_snaps(rbd_image) is not covered by try/finally that closes rbd_image\n\nThe whole method is ripe for a clean-up.","commit_id":"e7b5e1b80d99af31d802013a563633e230865ca8"},{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"76b3563694f4009a04a48a8ed2996dd5f28ec223","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":36,"id":"34c098ad_2ebf53cb","updated":"2023-08-25 01:43:30.000000000","message":"I generally agree with Alan and Pete, there\u0027s a goo bit that could be\ncleaned up but the overall approach appears to be sound.  Additionally,\nsnapshot protection/unprotection isn\u0027t necessary any longer and the\nflatten routines could be consolidated.  But the bug this patch address\nI think is far more important than smaller issues that can be addressed\nin a follow up patch.","commit_id":"31429d294baf1d8dd983a80e69271d129fd9c4ee"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"353f3e3099398b065c407a994dd0c68ad0db820f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":36,"id":"f938d8f5_269747ac","updated":"2023-08-22 17:27:11.000000000","message":"recheck\n\ncinder-plugin-ceph-tempest c-vol hung","commit_id":"31429d294baf1d8dd983a80e69271d129fd9c4ee"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"75ff8465678e059929a1dafc2f8cadef4d23f848","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":36,"id":"42fc3150_a1360777","updated":"2023-08-21 19:04:00.000000000","message":"recheck\n\nopenstack-tox-functional-py39 rsync log failure","commit_id":"31429d294baf1d8dd983a80e69271d129fd9c4ee"},{"author":{"_account_id":33807,"name":"Jacob Wang","email":"jacob_wang1@dell.com","username":"jacob0522"},"change_message_id":"0e18e92325ea28628caa417f837ba9296bc0b4ee","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":36,"id":"af1ea4fc_39d9e0ac","updated":"2023-08-23 03:12:57.000000000","message":"run-DellEMC PowerFlex CI","commit_id":"31429d294baf1d8dd983a80e69271d129fd9c4ee"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"d71628dbbb32ee44aeb2194217ca561b1147e30f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":37,"id":"0479c9b6_5c9941bd","updated":"2023-08-29 03:09:01.000000000","message":"This has incorporated a lot of comments/suggestions from a lot of reviewers.  LGTM.","commit_id":"2eca5aa270b4a6dd1984648ca52e8964df992ed7"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"cda6d853bf62a5ba3dc6289ca465db04d0a36cbb","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":37,"id":"a828b7d0_5ac8c98f","updated":"2023-08-29 11:49:33.000000000","message":"recheck\n\ncinder-tempest-plugin-lvm-lio-barbican unrelated failure","commit_id":"2eca5aa270b4a6dd1984648ca52e8964df992ed7"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"08e3d74deea48ff70c2dcdd01fca6cbeeded10e8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":37,"id":"15bdda20_1cc9299d","updated":"2023-08-28 16:50:06.000000000","message":"recheck\n\ntempest-integrated-storage unrelated failure","commit_id":"2eca5aa270b4a6dd1984648ca52e8964df992ed7"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"a2125058ca05aafe2db95cae4352bb934c25ca28","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":38,"id":"9c2675fe_5dd32741","updated":"2023-08-29 16:30:43.000000000","message":"(apparently an unnecessary dependency was dropped, no code change)","commit_id":"06b06b9a483ac85aedd281b2cc20511e17fab65e"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"d4efdc0c881f2c9d022ec9e5f01874b50ea96c8f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":38,"id":"7e90ed71_bbcda148","updated":"2023-08-29 16:18:56.000000000","message":"Renewing my +2 after the tempest dependency was removed.","commit_id":"06b06b9a483ac85aedd281b2cc20511e17fab65e"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"f9d6fd4725c814f2dfa78ecc873c256ec2f3be54","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":38,"id":"9d5d2630_8927e41c","updated":"2023-09-05 12:13:18.000000000","message":"This patch needs a bit more analysis to understand if the failing gate jobs are caused by a problem introduced here.","commit_id":"06b06b9a483ac85aedd281b2cc20511e17fab65e"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"8ccd5418ab3d9e199204e9e366161ef82ee2e3a3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":38,"id":"f0d8333f_eb4a0971","updated":"2023-08-31 20:00:35.000000000","message":"When are we going to give up and escalate to the foundation\u0027s infra team?","commit_id":"06b06b9a483ac85aedd281b2cc20511e17fab65e"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"94054ce471ae877ef6ba886611e72a95f5323bcb","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":38,"id":"d30c1357_ca5947a9","updated":"2023-09-02 06:10:29.000000000","message":"recheck\n\nIt looks like all of the failures are in Tempest jobs, but actually these jobs were passing previously.","commit_id":"06b06b9a483ac85aedd281b2cc20511e17fab65e"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"6cdd703e6476027e166082f4355c8f1ab0ca7aba","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":38,"id":"14bb16ea_c8dc908c","updated":"2023-09-01 16:42:07.000000000","message":"recheck\n\ncinder-grenade-mn-sub-volbak TIMED_OUT","commit_id":"06b06b9a483ac85aedd281b2cc20511e17fab65e"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"8dbaafe866ad46298916a23cc64c7b3b14280763","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":38,"id":"0ed24f21_0c0648e9","updated":"2023-08-31 16:20:04.000000000","message":"recheck\n\ncinder-plugin-ceph-tempest c-vol hung","commit_id":"06b06b9a483ac85aedd281b2cc20511e17fab65e"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"9c40e370ed786eb9e9f52530cfa947483eefc9eb","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":38,"id":"16588d90_724a9843","updated":"2023-09-01 13:22:32.000000000","message":"recheck\n\ncinder-plugin-ceph-tempest c-vol hung","commit_id":"06b06b9a483ac85aedd281b2cc20511e17fab65e"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"1f9c53631566788029432c3c61038343c01b55bc","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":38,"id":"2ea10eda_a945572f","updated":"2023-08-30 22:09:40.000000000","message":"recheck\n\ncinder-plugin-ceph-tempest c-vol service hung","commit_id":"06b06b9a483ac85aedd281b2cc20511e17fab65e"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"57c7d224d6d95403f2cd19e045cb9ed3dac3bf1a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":38,"id":"3f68368f_a41e2716","updated":"2023-08-31 19:51:26.000000000","message":"recheck\n\ncinder-plugin-ceph-tempest failed with rados.ObjectNotFound: [errno 2] RADOS object not found (error calling conf_read_file)","commit_id":"06b06b9a483ac85aedd281b2cc20511e17fab65e"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"4d3392648c3820efa241e205d9cb30943ecf22a7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":38,"id":"1bb11ccf_ea466800","updated":"2023-08-30 16:34:33.000000000","message":"recheck\n\ncinder-plugin-ceph-tempest n-cpu hung","commit_id":"06b06b9a483ac85aedd281b2cc20511e17fab65e"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"7157fe83446b996858e4fe3f67209f4a43441047","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":38,"id":"b9c54c87_42e8df1d","updated":"2023-09-03 14:23:13.000000000","message":"recheck\n\nnext retry some time by midnight I guess","commit_id":"06b06b9a483ac85aedd281b2cc20511e17fab65e"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"e32b630558aff31fee63fafe6b636f5b8fe211a4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":38,"id":"9645e047_d45361a8","updated":"2023-08-30 18:51:30.000000000","message":"recheck\n\npy310 unrelated spurious unit test failure - cinder.tests.unit.volume.drivers.nec.v.test_internal_nec_rest_iscsi.VStorageRESTISCSIDriverTest.test_delete_group_snapshot","commit_id":"06b06b9a483ac85aedd281b2cc20511e17fab65e"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"901974c3e78d0cf2d8d2ed5f4bd8e190cb19eee6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":38,"id":"a7b6f226_f86bfb93","updated":"2023-08-31 12:06:39.000000000","message":"recheck\n\npy39 random failure cinder.tests.unit.backup.drivers.test_backup_s3.BackupS3TestCase.test_restore_sse TimeoutException","commit_id":"06b06b9a483ac85aedd281b2cc20511e17fab65e"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"5a52ee6557b17ec328583cdd0fe80c0641b70a5c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":38,"id":"8ac59f9b_87eb3f80","updated":"2023-08-29 19:44:03.000000000","message":"recheck\n\npy39 unit tests failed in pre-existing rbd test_deferred_deletion_periodic_task","commit_id":"06b06b9a483ac85aedd281b2cc20511e17fab65e"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"9298d7ed96b66b657e9ff89b819dbb021df2c7c4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":38,"id":"cff011e8_3b991e9c","updated":"2023-09-01 00:56:43.000000000","message":"recheck\n\nreleasenotes post failure","commit_id":"06b06b9a483ac85aedd281b2cc20511e17fab65e"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"0116fd080ad9272c7f4b840c304de12e93c61cfe","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":38,"id":"a247557b_394513d7","updated":"2023-08-29 22:02:26.000000000","message":"recheck tempest-integrated-storage - identity 500s, server faults, subnet in use errors","commit_id":"06b06b9a483ac85aedd281b2cc20511e17fab65e"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"4aabe54669e87e81656d00fba763f7cc47606aed","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":39,"id":"4c16da41_319d4e77","updated":"2023-09-14 18:34:28.000000000","message":"recheck\n\ngather more cinder-plugin-ceph-tempest info","commit_id":"750ad21bd9434befcee102dcb19423f7f1ff9e78"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"ff04f24b13ef59c98a6d88c96d6b72688778d805","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":39,"id":"5de2a681_8926f1b0","updated":"2023-09-11 17:40:29.000000000","message":"recheck\n\ntest with https://review.opendev.org/c/openstack/tempest/+/894269","commit_id":"750ad21bd9434befcee102dcb19423f7f1ff9e78"},{"author":{"_account_id":31779,"name":"Jean Pierre Roquesalane","display_name":"happystacker","email":"jeanpierre.roquesalane@dell.com","username":"happystacker"},"change_message_id":"8ef20e35f662bead760f87a5fdf0996157efc5d1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":39,"id":"2811367e_0a11ccd2","updated":"2023-09-11 10:06:03.000000000","message":"recheck cinder-plugin-ceph-tempest failure","commit_id":"750ad21bd9434befcee102dcb19423f7f1ff9e78"},{"author":{"_account_id":31779,"name":"Jean Pierre Roquesalane","display_name":"happystacker","email":"jeanpierre.roquesalane@dell.com","username":"happystacker"},"change_message_id":"c47472fdfcf6d3c0ad5e8feeae7d94b390f09fa5","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":39,"id":"e4d63f30_25dc9572","updated":"2023-09-12 13:57:53.000000000","message":"recheck cinder-plugin-ceph-tempest failure","commit_id":"750ad21bd9434befcee102dcb19423f7f1ff9e78"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"15bf81154d2dc49053d7699c3ce5147b16e1fd91","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":41,"id":"5cd42da9_50271359","updated":"2023-10-12 13:43:57.000000000","message":"Downgrading to +1 while we figure out the gate situation.","commit_id":"37e8a58790dd10c1bf1c168bdce6b34613c8d829"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"7c309e46185ed4207ed7da18f97a5d5da4067e85","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":42,"id":"1621a7e5_3c6a6358","updated":"2023-11-29 16:07:16.000000000","message":"This update resolves the gate failure seen on the previous patch -- the driver was leaking file descriptors due to RBDVolumeProxy being used as a regular class and not as a ContextManager.\n\nhttps://review.opendev.org/c/openstack/cinder/+/895851 runs the Ceph job a number of times against this patch.","commit_id":"94ef4accc4f9dc93ad3cb62d9a366145eff2c292"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"bb5c4c125c30122be3976e5d1d4cac0ebfda3d34","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":43,"id":"d51d0091_dc0c6bef","updated":"2023-11-30 23:14:27.000000000","message":"Code and tests look good (but I said that last time).  The -1 is because I think the new config opt needs to be mentioned in the release note.  Normally I\u0027d say it could be a followup, but since we know we\u0027ll want to backport this, might as well get it as part of this patch set.","commit_id":"77f8e2cc9584108e7b9cdac6f99d5b22103692d6"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"f57d3d9ff3d1bfc32a540a028aaddb195ee628fd","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":43,"id":"392fd7c2_93d9668f","updated":"2023-11-29 19:36:38.000000000","message":"OK, think it should be good to go at last.\n\nIf you make another round, I\u0027d love to see a couple of renames.\n\nFirst, I am with Alan regarding _flatten_volume and renaming to _flatten_dest_volume. That even preserves the comment, in a way.\n\nSecond, that _semaphore thing. The only way to find how it works is an exhaustive and exhausting search. I wish it were called _op_lim_sem or something like that. Just a tiny little clue as to nature of it, and its use inside the decorator.","commit_id":"77f8e2cc9584108e7b9cdac6f99d5b22103692d6"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"bee631c13cdca23c3ddbdbd4b0629ee8c543c8b0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":44,"id":"3ad79f13_112bf8e7","updated":"2023-12-14 13:53:33.000000000","message":"Revisions LGTM, and the CI is green.","commit_id":"1a675c9aa178c6d9c6ed10fd98f086c46d350d3f"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"3911b32175235b8db0a3db2685efa8a334f62606","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":44,"id":"b56cdbf0_bfab747c","updated":"2023-12-05 09:34:54.000000000","message":"The logic looks correct and is also summarized in the commit message. cinder-plugin-ceph-tempest job is passing. Not sure if we have any tempest test actually testing this scenario. Overall LGTM.","commit_id":"1a675c9aa178c6d9c6ed10fd98f086c46d350d3f"}],"cinder/tests/unit/volume/drivers/test_rbd.py":[{"author":{"_account_id":30615,"name":"Tushar Trambak Gite","email":"tushargite96@gmail.com","username":"tushargite96"},"change_message_id":"5313b757dc3dcaadaaa7b8700a1094797a7160e8","unresolved":true,"context_lines":[{"line_number":924,"context_line":"        mock_flatten \u003d self.mock_object(self.driver, \u0027_flatten\u0027)"},{"line_number":925,"context_line":""},{"line_number":926,"context_line":"        with mock.patch.object(self.driver, \u0027_get_clone_info\u0027) as \\"},{"line_number":927,"context_line":"                mock_get_clone_info:"},{"line_number":928,"context_line":"            mock_get_clone_info.return_value \u003d (None, None, None)"},{"line_number":929,"context_line":"            self.driver.rbd.Image.return_value.list_children.\\"},{"line_number":930,"context_line":"                return_value \u003d [(\u0027pool1\u0027, \u0027child1\u0027),"}],"source_content_type":"text/x-python","patch_set":30,"id":"ee198348_f21fd818","line":927,"updated":"2023-03-07 11:55:54.000000000","message":"indentation","commit_id":"8a0d456f1f8ecec9cfb816c720cfa23e3ea9f9aa"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"9ff8124a715a5a399ee862d4ea81bd6f146cc0f5","unresolved":false,"context_lines":[{"line_number":924,"context_line":"        mock_flatten \u003d self.mock_object(self.driver, \u0027_flatten\u0027)"},{"line_number":925,"context_line":""},{"line_number":926,"context_line":"        with mock.patch.object(self.driver, \u0027_get_clone_info\u0027) as \\"},{"line_number":927,"context_line":"                mock_get_clone_info:"},{"line_number":928,"context_line":"            mock_get_clone_info.return_value \u003d (None, None, None)"},{"line_number":929,"context_line":"            self.driver.rbd.Image.return_value.list_children.\\"},{"line_number":930,"context_line":"                return_value \u003d [(\u0027pool1\u0027, \u0027child1\u0027),"}],"source_content_type":"text/x-python","patch_set":30,"id":"1a7a7e6c_df0e27dc","line":927,"in_reply_to":"ee198348_f21fd818","updated":"2023-07-12 17:47:02.000000000","message":"Done","commit_id":"8a0d456f1f8ecec9cfb816c720cfa23e3ea9f9aa"}],"cinder/volume/drivers/rbd.py":[{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"fd42ddb6dd072a29ddd3bef31017ee89981218e6","unresolved":true,"context_lines":[{"line_number":1310,"context_line":""},{"line_number":1311,"context_line":"                if children:"},{"line_number":1312,"context_line":"                    for pool, child_name in children:"},{"line_number":1313,"context_line":"                        self._flatten(pool, child_name)"},{"line_number":1314,"context_line":"                    self.RBDProxy().remove(client.ioctx, volume_name)"},{"line_number":1315,"context_line":"                else:"},{"line_number":1316,"context_line":"                    LOG.debug(\"moving volume %s to trash\", volume_name)"}],"source_content_type":"text/x-python","patch_set":1,"id":"c869de9a_88f02ed2","line":1313,"updated":"2022-04-01 15:22:02.000000000","message":"We should limit concurrency here with utils.limit_operations() or so -- I\u0027ll take a look at this.","commit_id":"1352f453fca12bdb720ccd9460dc813e49adfce4"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"0e3ae803d3a347e6aea2aefa5812c013b1f6a8d9","unresolved":false,"context_lines":[{"line_number":1310,"context_line":""},{"line_number":1311,"context_line":"                if children:"},{"line_number":1312,"context_line":"                    for pool, child_name in children:"},{"line_number":1313,"context_line":"                        self._flatten(pool, child_name)"},{"line_number":1314,"context_line":"                    self.RBDProxy().remove(client.ioctx, volume_name)"},{"line_number":1315,"context_line":"                else:"},{"line_number":1316,"context_line":"                    LOG.debug(\"moving volume %s to trash\", volume_name)"}],"source_content_type":"text/x-python","patch_set":1,"id":"265bbb58_63792c00","line":1313,"in_reply_to":"9ff35229_a892abcd","updated":"2022-06-09 15:39:37.000000000","message":"Done","commit_id":"1352f453fca12bdb720ccd9460dc813e49adfce4"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"4cf46b7044ff672e6a3da148634065b9331e3b80","unresolved":true,"context_lines":[{"line_number":1310,"context_line":""},{"line_number":1311,"context_line":"                if children:"},{"line_number":1312,"context_line":"                    for pool, child_name in children:"},{"line_number":1313,"context_line":"                        self._flatten(pool, child_name)"},{"line_number":1314,"context_line":"                    self.RBDProxy().remove(client.ioctx, volume_name)"},{"line_number":1315,"context_line":"                else:"},{"line_number":1316,"context_line":"                    LOG.debug(\"moving volume %s to trash\", volume_name)"}],"source_content_type":"text/x-python","patch_set":1,"id":"9ff35229_a892abcd","line":1313,"in_reply_to":"c869de9a_88f02ed2","updated":"2022-04-28 19:14:55.000000000","message":"Seems like this one can be marked done.","commit_id":"1352f453fca12bdb720ccd9460dc813e49adfce4"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"4cf46b7044ff672e6a3da148634065b9331e3b80","unresolved":true,"context_lines":[{"line_number":290,"context_line":"        self.keyring_data: Optional[str] \u003d None"},{"line_number":291,"context_line":"        self._set_keyring_attributes()"},{"line_number":292,"context_line":""},{"line_number":293,"context_line":"        self._semaphore \u003d utils.semaphore_factory(3, 1)"},{"line_number":294,"context_line":""},{"line_number":295,"context_line":"    def _set_keyring_attributes(self) -\u003e None:"},{"line_number":296,"context_line":"        # The rbd_keyring_conf option is not available for OpenStack usage"}],"source_content_type":"text/x-python","patch_set":4,"id":"3ce23423_c1e14465","line":293,"updated":"2022-04-28 19:14:55.000000000","message":"It would help to name the arguments (limit\u003d3, concurrent_processes\u003d1).\n\nRegarding concurrent_processes, what if there are multiple RBD backends and they\u0027re using different pools but in the same cluster? I don\u0027t know if the load flattening load affects a specific pool, or the entire cluster.","commit_id":"618f2935e07065ec85ee002ed64289abdd8d09f5"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"0e3ae803d3a347e6aea2aefa5812c013b1f6a8d9","unresolved":true,"context_lines":[{"line_number":290,"context_line":"        self.keyring_data: Optional[str] \u003d None"},{"line_number":291,"context_line":"        self._set_keyring_attributes()"},{"line_number":292,"context_line":""},{"line_number":293,"context_line":"        self._semaphore \u003d utils.semaphore_factory(3, 1)"},{"line_number":294,"context_line":""},{"line_number":295,"context_line":"    def _set_keyring_attributes(self) -\u003e None:"},{"line_number":296,"context_line":"        # The rbd_keyring_conf option is not available for OpenStack usage"}],"source_content_type":"text/x-python","patch_set":4,"id":"6ce0558a_cb4f708c","line":293,"in_reply_to":"3ce23423_c1e14465","updated":"2022-06-09 15:39:37.000000000","message":"It would have I/O impact on the pool and cluster.  I\u0027m going to make the limit value configurable -- admins can choose an appropriate limit based on how much impact they want to allow on their cluster and how many c-vol nodes they are using that might simultaneously perform flatten operations.","commit_id":"618f2935e07065ec85ee002ed64289abdd8d09f5"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"99a24c7f71115b511f4aa59ce585f8372023ef04","unresolved":false,"context_lines":[{"line_number":290,"context_line":"        self.keyring_data: Optional[str] \u003d None"},{"line_number":291,"context_line":"        self._set_keyring_attributes()"},{"line_number":292,"context_line":""},{"line_number":293,"context_line":"        self._semaphore \u003d utils.semaphore_factory(3, 1)"},{"line_number":294,"context_line":""},{"line_number":295,"context_line":"    def _set_keyring_attributes(self) -\u003e None:"},{"line_number":296,"context_line":"        # The rbd_keyring_conf option is not available for OpenStack usage"}],"source_content_type":"text/x-python","patch_set":4,"id":"4019fe12_a1e6af85","line":293,"in_reply_to":"6ce0558a_cb4f708c","updated":"2022-06-14 23:34:58.000000000","message":"Done","commit_id":"618f2935e07065ec85ee002ed64289abdd8d09f5"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"4cf46b7044ff672e6a3da148634065b9331e3b80","unresolved":true,"context_lines":[{"line_number":1066,"context_line":"                      dict(pool\u003dpool, img\u003dvolume_name))"},{"line_number":1067,"context_line":"            with RBDVolumeProxy(self, volume_name) as vol:"},{"line_number":1068,"context_line":"                try:"},{"line_number":1069,"context_line":"                    vol.flatten()"},{"line_number":1070,"context_line":"                except self.rbd.InvalidArgument as e:"},{"line_number":1071,"context_line":"                    LOG.debug(e)"},{"line_number":1072,"context_line":"                    raise e"}],"source_content_type":"text/x-python","patch_set":4,"id":"31784c5f_7bcdb462","line":1069,"updated":"2022-04-28 19:14:55.000000000","message":"Maybe add another debug log message after the flatten. The before/after timestamps would provide insight on how long it takes, whether there\u0027s a bottleneck, etc.","commit_id":"618f2935e07065ec85ee002ed64289abdd8d09f5"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"0e3ae803d3a347e6aea2aefa5812c013b1f6a8d9","unresolved":true,"context_lines":[{"line_number":1066,"context_line":"                      dict(pool\u003dpool, img\u003dvolume_name))"},{"line_number":1067,"context_line":"            with RBDVolumeProxy(self, volume_name) as vol:"},{"line_number":1068,"context_line":"                try:"},{"line_number":1069,"context_line":"                    vol.flatten()"},{"line_number":1070,"context_line":"                except self.rbd.InvalidArgument as e:"},{"line_number":1071,"context_line":"                    LOG.debug(e)"},{"line_number":1072,"context_line":"                    raise e"}],"source_content_type":"text/x-python","patch_set":4,"id":"a2596b5a_d80470a3","line":1069,"in_reply_to":"31784c5f_7bcdb462","updated":"2022-06-09 15:39:37.000000000","message":"Good idea!","commit_id":"618f2935e07065ec85ee002ed64289abdd8d09f5"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"99a24c7f71115b511f4aa59ce585f8372023ef04","unresolved":false,"context_lines":[{"line_number":1066,"context_line":"                      dict(pool\u003dpool, img\u003dvolume_name))"},{"line_number":1067,"context_line":"            with RBDVolumeProxy(self, volume_name) as vol:"},{"line_number":1068,"context_line":"                try:"},{"line_number":1069,"context_line":"                    vol.flatten()"},{"line_number":1070,"context_line":"                except self.rbd.InvalidArgument as e:"},{"line_number":1071,"context_line":"                    LOG.debug(e)"},{"line_number":1072,"context_line":"                    raise e"}],"source_content_type":"text/x-python","patch_set":4,"id":"460d112d_6fcf45f0","line":1069,"in_reply_to":"a2596b5a_d80470a3","updated":"2022-06-14 23:34:58.000000000","message":"Done","commit_id":"618f2935e07065ec85ee002ed64289abdd8d09f5"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"4cf46b7044ff672e6a3da148634065b9331e3b80","unresolved":true,"context_lines":[{"line_number":1288,"context_line":"            self._delete_backup_snaps(rbd_image)"},{"line_number":1289,"context_line":""},{"line_number":1290,"context_line":"            # If the volume has non-clone snapshots this delete is expected to"},{"line_number":1291,"context_line":"            # raise VolumeIsBusy so do so straight away."},{"line_number":1292,"context_line":"            try:"},{"line_number":1293,"context_line":"                snaps \u003d rbd_image.list_snaps()"},{"line_number":1294,"context_line":"                for snap in snaps:"}],"source_content_type":"text/x-python","patch_set":4,"id":"3c0e8043_3ec55a52","line":1291,"range":{"start_line":1291,"start_character":14,"end_line":1291,"end_character":55},"updated":"2022-04-28 19:14:55.000000000","message":"This comment is no longer accurate.","commit_id":"618f2935e07065ec85ee002ed64289abdd8d09f5"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"0e3ae803d3a347e6aea2aefa5812c013b1f6a8d9","unresolved":false,"context_lines":[{"line_number":1288,"context_line":"            self._delete_backup_snaps(rbd_image)"},{"line_number":1289,"context_line":""},{"line_number":1290,"context_line":"            # If the volume has non-clone snapshots this delete is expected to"},{"line_number":1291,"context_line":"            # raise VolumeIsBusy so do so straight away."},{"line_number":1292,"context_line":"            try:"},{"line_number":1293,"context_line":"                snaps \u003d rbd_image.list_snaps()"},{"line_number":1294,"context_line":"                for snap in snaps:"}],"source_content_type":"text/x-python","patch_set":4,"id":"26d110ce_94cb811b","line":1291,"range":{"start_line":1291,"start_character":14,"end_line":1291,"end_character":55},"in_reply_to":"3c0e8043_3ec55a52","updated":"2022-06-09 15:39:37.000000000","message":"Done","commit_id":"618f2935e07065ec85ee002ed64289abdd8d09f5"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"4cf46b7044ff672e6a3da148634065b9331e3b80","unresolved":true,"context_lines":[{"line_number":1319,"context_line":"                    except (self.rbd.ImageHasSnapshots, self.rbd.ImageBusy):"},{"line_number":1320,"context_line":"                        delay \u003d 0"},{"line_number":1321,"context_line":""},{"line_number":1322,"context_line":"                # reopen  (TODO: close previous?)"},{"line_number":1323,"context_line":"                rbd_image \u003d self.rbd.Image(client.ioctx,"},{"line_number":1324,"context_line":"                                           volume_name,"},{"line_number":1325,"context_line":"                                           read_only\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":4,"id":"95df2bac_cd9b110d","line":1322,"updated":"2022-04-28 19:14:55.000000000","message":"Isn\u0027t that done on L1307?","commit_id":"618f2935e07065ec85ee002ed64289abdd8d09f5"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"0e3ae803d3a347e6aea2aefa5812c013b1f6a8d9","unresolved":true,"context_lines":[{"line_number":1319,"context_line":"                    except (self.rbd.ImageHasSnapshots, self.rbd.ImageBusy):"},{"line_number":1320,"context_line":"                        delay \u003d 0"},{"line_number":1321,"context_line":""},{"line_number":1322,"context_line":"                # reopen  (TODO: close previous?)"},{"line_number":1323,"context_line":"                rbd_image \u003d self.rbd.Image(client.ioctx,"},{"line_number":1324,"context_line":"                                           volume_name,"},{"line_number":1325,"context_line":"                                           read_only\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":4,"id":"a9e63046_63ed1308","line":1322,"in_reply_to":"95df2bac_cd9b110d","updated":"2022-06-09 15:39:37.000000000","message":"Need to look at this closer -- the concern was what happens on retry #2 if retry #1 failed with ImageBusy.","commit_id":"618f2935e07065ec85ee002ed64289abdd8d09f5"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"7e57f4a4babbd69730e94f2be9727bbcb9f72d1c","unresolved":false,"context_lines":[{"line_number":1319,"context_line":"                    except (self.rbd.ImageHasSnapshots, self.rbd.ImageBusy):"},{"line_number":1320,"context_line":"                        delay \u003d 0"},{"line_number":1321,"context_line":""},{"line_number":1322,"context_line":"                # reopen  (TODO: close previous?)"},{"line_number":1323,"context_line":"                rbd_image \u003d self.rbd.Image(client.ioctx,"},{"line_number":1324,"context_line":"                                           volume_name,"},{"line_number":1325,"context_line":"                                           read_only\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":4,"id":"59dcfd05_695c019f","line":1322,"in_reply_to":"a9e63046_63ed1308","updated":"2022-08-31 13:55:18.000000000","message":"Done","commit_id":"618f2935e07065ec85ee002ed64289abdd8d09f5"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"4cf46b7044ff672e6a3da148634065b9331e3b80","unresolved":true,"context_lines":[{"line_number":1323,"context_line":"                rbd_image \u003d self.rbd.Image(client.ioctx,"},{"line_number":1324,"context_line":"                                           volume_name,"},{"line_number":1325,"context_line":"                                           read_only\u003dTrue)"},{"line_number":1326,"context_line":"                children \u003d rbd_image.list_children()"},{"line_number":1327,"context_line":"                if children:"},{"line_number":1328,"context_line":"                    LOG.debug(\"children: %s\", children)"},{"line_number":1329,"context_line":"                    for pool, child_name in children:"}],"source_content_type":"text/x-python","patch_set":4,"id":"e4a5a188_54a700a1","line":1326,"updated":"2022-04-28 19:14:55.000000000","message":"I have a few several comments/question regarding the new flatten code (L1323..L1330)\n\n- Instead of reopening the image, could L1326 be moved up near L1293, when you already had the image open?\n\n- Does flattening children need to be done here (with the possibility of it being retried), or could it be done as a separate operation outside this function by moving the code down just prior to L1347?\n\n- With that in mind, should it be its own function? There\u0027s a self._delete_backup_snaps() function, this could be self._flatten_children(). Just a thought.\n\n- Consider the note at L1271: can you pass child_name directly to self._flatten() or do you need to pass utils.convert_str(child_name)","commit_id":"618f2935e07065ec85ee002ed64289abdd8d09f5"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"aa8ca0689957be6fe6c765971748f29dd295c553","unresolved":false,"context_lines":[{"line_number":1323,"context_line":"                rbd_image \u003d self.rbd.Image(client.ioctx,"},{"line_number":1324,"context_line":"                                           volume_name,"},{"line_number":1325,"context_line":"                                           read_only\u003dTrue)"},{"line_number":1326,"context_line":"                children \u003d rbd_image.list_children()"},{"line_number":1327,"context_line":"                if children:"},{"line_number":1328,"context_line":"                    LOG.debug(\"children: %s\", children)"},{"line_number":1329,"context_line":"                    for pool, child_name in children:"}],"source_content_type":"text/x-python","patch_set":4,"id":"37935af2_29d6fbef","line":1326,"in_reply_to":"cb98f1fb_80266cf9","updated":"2022-06-23 16:12:46.000000000","message":"Re: reopening the image -- we need it closed for the remove() call, so we will need to close it and reopen it if it fails.\n\nhttps://docs.ceph.com/en/latest/rbd/api/librbdpy/#rbd.RBD.remove\n\nMade a new method for flatten_children.\n\nWe don\u0027t need to call convert_str() since the string will already be in utf8.","commit_id":"618f2935e07065ec85ee002ed64289abdd8d09f5"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"0e3ae803d3a347e6aea2aefa5812c013b1f6a8d9","unresolved":true,"context_lines":[{"line_number":1323,"context_line":"                rbd_image \u003d self.rbd.Image(client.ioctx,"},{"line_number":1324,"context_line":"                                           volume_name,"},{"line_number":1325,"context_line":"                                           read_only\u003dTrue)"},{"line_number":1326,"context_line":"                children \u003d rbd_image.list_children()"},{"line_number":1327,"context_line":"                if children:"},{"line_number":1328,"context_line":"                    LOG.debug(\"children: %s\", children)"},{"line_number":1329,"context_line":"                    for pool, child_name in children:"}],"source_content_type":"text/x-python","patch_set":4,"id":"cb98f1fb_80266cf9","line":1326,"in_reply_to":"e4a5a188_54a700a1","updated":"2022-06-09 15:39:37.000000000","message":"Still looking at these, will hit on the next revision.","commit_id":"618f2935e07065ec85ee002ed64289abdd8d09f5"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"4cf46b7044ff672e6a3da148634065b9331e3b80","unresolved":true,"context_lines":[{"line_number":1328,"context_line":"                    LOG.debug(\"children: %s\", children)"},{"line_number":1329,"context_line":"                    for pool, child_name in children:"},{"line_number":1330,"context_line":"                        self._flatten(pool, child_name)"},{"line_number":1331,"context_line":"                    self.RBDProxy().remove(client.ioctx, volume_name)"},{"line_number":1332,"context_line":"                else:"},{"line_number":1333,"context_line":"                    LOG.debug(\"moving volume %s to trash\", volume_name)"},{"line_number":1334,"context_line":"                    # When using the RBD v2 clone api, deleting a volume"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a3541dd_fd8926ac","line":1331,"updated":"2022-04-28 19:14:55.000000000","message":"What if this throws an exception?\n\nThe current code seems to flatten children only if the original remove() on L1317 failed. Unless listing rbd_image.list_children() is expensive, would it make sense to flatten the kids before attempting the remove(), rather than doing it after the initial attempt to remove() failed and then trying a second time?","commit_id":"618f2935e07065ec85ee002ed64289abdd8d09f5"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"0e3ae803d3a347e6aea2aefa5812c013b1f6a8d9","unresolved":true,"context_lines":[{"line_number":1328,"context_line":"                    LOG.debug(\"children: %s\", children)"},{"line_number":1329,"context_line":"                    for pool, child_name in children:"},{"line_number":1330,"context_line":"                        self._flatten(pool, child_name)"},{"line_number":1331,"context_line":"                    self.RBDProxy().remove(client.ioctx, volume_name)"},{"line_number":1332,"context_line":"                else:"},{"line_number":1333,"context_line":"                    LOG.debug(\"moving volume %s to trash\", volume_name)"},{"line_number":1334,"context_line":"                    # When using the RBD v2 clone api, deleting a volume"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa576dca_e624dbbe","line":1331,"in_reply_to":"3a3541dd_fd8926ac","updated":"2022-06-09 15:39:37.000000000","message":"The intent was to only look for child images to flatten if we fail to do the original remove() successfully, which is the ideal case.  I think leaving that as a fast path makes sense.","commit_id":"618f2935e07065ec85ee002ed64289abdd8d09f5"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"7ef54e55a57293c8f3cb8769b55d489ddd6029f9","unresolved":false,"context_lines":[{"line_number":1328,"context_line":"                    LOG.debug(\"children: %s\", children)"},{"line_number":1329,"context_line":"                    for pool, child_name in children:"},{"line_number":1330,"context_line":"                        self._flatten(pool, child_name)"},{"line_number":1331,"context_line":"                    self.RBDProxy().remove(client.ioctx, volume_name)"},{"line_number":1332,"context_line":"                else:"},{"line_number":1333,"context_line":"                    LOG.debug(\"moving volume %s to trash\", volume_name)"},{"line_number":1334,"context_line":"                    # When using the RBD v2 clone api, deleting a volume"}],"source_content_type":"text/x-python","patch_set":4,"id":"1e197bf4_86fb99a7","line":1331,"in_reply_to":"fa576dca_e624dbbe","updated":"2022-06-24 18:06:52.000000000","message":"Ack","commit_id":"618f2935e07065ec85ee002ed64289abdd8d09f5"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"c25a2c155e06bd02da58c61e6ba501e1623a00fe","unresolved":true,"context_lines":[{"line_number":1341,"context_line":"                        delay \u003d 0"},{"line_number":1342,"context_line":""},{"line_number":1343,"context_line":"                if children:"},{"line_number":1344,"context_line":"                    self._flatten_children(children)"},{"line_number":1345,"context_line":"                    try:"},{"line_number":1346,"context_line":"                        self.RBDProxy().remove(client.ioctx, volume_name)"},{"line_number":1347,"context_line":"                    except (self.rbd.ImageHasSnapshots, self.rbd.ImageBusy):"}],"source_content_type":"text/x-python","patch_set":7,"id":"fbfac122_13c5a47c","line":1344,"range":{"start_line":1344,"start_character":25,"end_line":1344,"end_character":43},"updated":"2022-06-21 15:34:55.000000000","message":"IMO we should call list_children just before calling flatten -- otherwise we don\u0027t pick up potential changes in state from other delete operations occurring, and end up trying to call flatten operations on images that may no longer exist etc.","commit_id":"16d2c28e472c4904743068e2cf21d6d45ec807c9"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"7ef54e55a57293c8f3cb8769b55d489ddd6029f9","unresolved":false,"context_lines":[{"line_number":1341,"context_line":"                        delay \u003d 0"},{"line_number":1342,"context_line":""},{"line_number":1343,"context_line":"                if children:"},{"line_number":1344,"context_line":"                    self._flatten_children(children)"},{"line_number":1345,"context_line":"                    try:"},{"line_number":1346,"context_line":"                        self.RBDProxy().remove(client.ioctx, volume_name)"},{"line_number":1347,"context_line":"                    except (self.rbd.ImageHasSnapshots, self.rbd.ImageBusy):"}],"source_content_type":"text/x-python","patch_set":7,"id":"01907c55_2d3a2bea","line":1344,"range":{"start_line":1344,"start_character":25,"end_line":1344,"end_character":43},"in_reply_to":"fbfac122_13c5a47c","updated":"2022-06-24 18:06:52.000000000","message":"Ack","commit_id":"16d2c28e472c4904743068e2cf21d6d45ec807c9"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"aa8ca0689957be6fe6c765971748f29dd295c553","unresolved":false,"context_lines":[{"line_number":1340,"context_line":"                # flatten operations on images that may no longer exist."},{"line_number":1341,"context_line":"                rbd_image \u003d self.rbd.Image(client.ioctx, volume_name)"},{"line_number":1342,"context_line":"                children_list \u003d rbd_image.list_children()"},{"line_number":1343,"context_line":"                rbd_image.close()"},{"line_number":1344,"context_line":"                if children_list:"},{"line_number":1345,"context_line":"                    for (pool, child_name) in children_list:"},{"line_number":1346,"context_line":"                        LOG.info(\u0027Image %(pool)s/%(image)s is dependent \u0027"}],"source_content_type":"text/x-python","patch_set":9,"id":"94e540a2_59fbdf8a","line":1343,"range":{"start_line":1343,"start_character":26,"end_line":1343,"end_character":31},"updated":"2022-06-23 16:12:46.000000000","message":"We need to try: list_children() and have close() in a finally: block to ensure it is always closed.","commit_id":"a4350cdc720ae19e12d5bb7db35a4deffe7efe02"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"7ef54e55a57293c8f3cb8769b55d489ddd6029f9","unresolved":true,"context_lines":[{"line_number":1083,"context_line":"                    LOG.debug(\u0027flattening of %(pool)s/%(img)s has completed\u0027,"},{"line_number":1084,"context_line":"                              {\u0027pool\u0027: pool, \u0027img\u0027: volume_name})"},{"line_number":1085,"context_line":""},{"line_number":1086,"context_line":"        do_flatten(self, volume_name, pool)"},{"line_number":1087,"context_line":""},{"line_number":1088,"context_line":"    def _get_stripe_unit(self, ioctx: \u0027rados.Ioctx\u0027, volume_name: str) -\u003e int:"},{"line_number":1089,"context_line":"        \"\"\"Return the correct stripe unit for a cloned volume."}],"source_content_type":"text/x-python","patch_set":11,"id":"4dd0caba_72468dd4","line":1086,"updated":"2022-06-24 18:06:52.000000000","message":"The logs at L1074 and L1083 are good, because the timestamps will reveal how long the flatten operation takes.\n\nWith this in mind, I suggest adding another log message just prior to L1086, which will reveal when (and how long) the flatten operation is delayed by the limit_operations.","commit_id":"c61f7aa7928cd42b2a238c76ab0b565b840ce67c"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"721229a9f6fd49ab744e72b3c7387b66cb15859f","unresolved":true,"context_lines":[{"line_number":1083,"context_line":"                    LOG.debug(\u0027flattening of %(pool)s/%(img)s has completed\u0027,"},{"line_number":1084,"context_line":"                              {\u0027pool\u0027: pool, \u0027img\u0027: volume_name})"},{"line_number":1085,"context_line":""},{"line_number":1086,"context_line":"        do_flatten(self, volume_name, pool)"},{"line_number":1087,"context_line":""},{"line_number":1088,"context_line":"    def _get_stripe_unit(self, ioctx: \u0027rados.Ioctx\u0027, volume_name: str) -\u003e int:"},{"line_number":1089,"context_line":"        \"\"\"Return the correct stripe unit for a cloned volume."}],"source_content_type":"text/x-python","patch_set":11,"id":"8ec72a3d_fe3f4c90","line":1086,"in_reply_to":"4dd0caba_72468dd4","updated":"2022-07-18 23:12:15.000000000","message":"This comment, from an earlier patchset, still applies (though the line number references are off a bit).","commit_id":"c61f7aa7928cd42b2a238c76ab0b565b840ce67c"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"1878d22a3627e0d51fed4c488129052dc42c992b","unresolved":true,"context_lines":[{"line_number":1083,"context_line":"                    LOG.debug(\u0027flattening of %(pool)s/%(img)s has completed\u0027,"},{"line_number":1084,"context_line":"                              {\u0027pool\u0027: pool, \u0027img\u0027: volume_name})"},{"line_number":1085,"context_line":""},{"line_number":1086,"context_line":"        do_flatten(self, volume_name, pool)"},{"line_number":1087,"context_line":""},{"line_number":1088,"context_line":"    def _get_stripe_unit(self, ioctx: \u0027rados.Ioctx\u0027, volume_name: str) -\u003e int:"},{"line_number":1089,"context_line":"        \"\"\"Return the correct stripe unit for a cloned volume."}],"source_content_type":"text/x-python","patch_set":11,"id":"49ec69c6_1dae28d3","line":1086,"in_reply_to":"8ec72a3d_fe3f4c90","updated":"2022-08-19 20:52:33.000000000","message":"Any thoughts on this?","commit_id":"c61f7aa7928cd42b2a238c76ab0b565b840ce67c"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"9c8c6aa2b0bf8156303cd8b74f0efb2e55c352eb","unresolved":true,"context_lines":[{"line_number":1083,"context_line":"                    LOG.debug(\u0027flattening of %(pool)s/%(img)s has completed\u0027,"},{"line_number":1084,"context_line":"                              {\u0027pool\u0027: pool, \u0027img\u0027: volume_name})"},{"line_number":1085,"context_line":""},{"line_number":1086,"context_line":"        do_flatten(self, volume_name, pool)"},{"line_number":1087,"context_line":""},{"line_number":1088,"context_line":"    def _get_stripe_unit(self, ioctx: \u0027rados.Ioctx\u0027, volume_name: str) -\u003e int:"},{"line_number":1089,"context_line":"        \"\"\"Return the correct stripe unit for a cloned volume."}],"source_content_type":"text/x-python","patch_set":11,"id":"f33aec95_17aeb5e9","line":1086,"in_reply_to":"8ec72a3d_fe3f4c90","updated":"2022-08-19 23:06:36.000000000","message":"Not sure where L1086 was previously, but I think the extra log has to be ahead of the call to do_flatten(), before the decorator-provided wrapper is called.\n\nIt would be great if it were possible to see how many operations are hanging on semaphore at a given time. But I suppose this is impossible, unless someone has a bright idea. Maybe some kind of /info endpoint like in Swift.","commit_id":"c61f7aa7928cd42b2a238c76ab0b565b840ce67c"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"3bd6c04f08b12c55b2a6e9db225cf9429672dba9","unresolved":false,"context_lines":[{"line_number":1083,"context_line":"                    LOG.debug(\u0027flattening of %(pool)s/%(img)s has completed\u0027,"},{"line_number":1084,"context_line":"                              {\u0027pool\u0027: pool, \u0027img\u0027: volume_name})"},{"line_number":1085,"context_line":""},{"line_number":1086,"context_line":"        do_flatten(self, volume_name, pool)"},{"line_number":1087,"context_line":""},{"line_number":1088,"context_line":"    def _get_stripe_unit(self, ioctx: \u0027rados.Ioctx\u0027, volume_name: str) -\u003e int:"},{"line_number":1089,"context_line":"        \"\"\"Return the correct stripe unit for a cloned volume."}],"source_content_type":"text/x-python","patch_set":11,"id":"68a47dea_e7ca2661","line":1086,"in_reply_to":"f33aec95_17aeb5e9","updated":"2022-08-22 18:26:27.000000000","message":"Done","commit_id":"c61f7aa7928cd42b2a238c76ab0b565b840ce67c"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"7ef54e55a57293c8f3cb8769b55d489ddd6029f9","unresolved":true,"context_lines":[{"line_number":1303,"context_line":"        volume_name \u003d volume.name"},{"line_number":1304,"context_line":"        with RADOSClient(self) as client:"},{"line_number":1305,"context_line":"            try:"},{"line_number":1306,"context_line":"                rbd_image \u003d self.rbd.Image(client.ioctx,"},{"line_number":1307,"context_line":"                                           volume_name)"},{"line_number":1308,"context_line":"            except self.rbd.ImageNotFound:"},{"line_number":1309,"context_line":"                LOG.info(\"volume %s no longer exists in backend\","},{"line_number":1310,"context_line":"                         volume_name)"}],"source_content_type":"text/x-python","patch_set":11,"id":"461751a0_bd85ed2a","line":1307,"range":{"start_line":1306,"start_character":0,"end_line":1307,"end_character":55},"updated":"2022-06-24 18:06:52.000000000","message":"nit: this change isn\u0027t needed","commit_id":"c61f7aa7928cd42b2a238c76ab0b565b840ce67c"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"a275fcc7d02cc7d0361dbb0d25ce51cd766a469a","unresolved":false,"context_lines":[{"line_number":1303,"context_line":"        volume_name \u003d volume.name"},{"line_number":1304,"context_line":"        with RADOSClient(self) as client:"},{"line_number":1305,"context_line":"            try:"},{"line_number":1306,"context_line":"                rbd_image \u003d self.rbd.Image(client.ioctx,"},{"line_number":1307,"context_line":"                                           volume_name)"},{"line_number":1308,"context_line":"            except self.rbd.ImageNotFound:"},{"line_number":1309,"context_line":"                LOG.info(\"volume %s no longer exists in backend\","},{"line_number":1310,"context_line":"                         volume_name)"}],"source_content_type":"text/x-python","patch_set":11,"id":"55276ed3_7ee45949","line":1307,"range":{"start_line":1306,"start_character":0,"end_line":1307,"end_character":55},"in_reply_to":"461751a0_bd85ed2a","updated":"2022-08-31 13:52:12.000000000","message":"Done","commit_id":"c61f7aa7928cd42b2a238c76ab0b565b840ce67c"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"7ef54e55a57293c8f3cb8769b55d489ddd6029f9","unresolved":true,"context_lines":[{"line_number":1308,"context_line":"            except self.rbd.ImageNotFound:"},{"line_number":1309,"context_line":"                LOG.info(\"volume %s no longer exists in backend\","},{"line_number":1310,"context_line":"                         volume_name)"},{"line_number":1311,"context_line":"                return"},{"line_number":1312,"context_line":""},{"line_number":1313,"context_line":"            clone_snap \u003d None"},{"line_number":1314,"context_line":"            parent \u003d None"}],"source_content_type":"text/x-python","patch_set":11,"id":"c628367f_86a7e199","line":1311,"updated":"2022-06-24 18:06:52.000000000","message":"Should it call rbd_image.close() before returning?","commit_id":"c61f7aa7928cd42b2a238c76ab0b565b840ce67c"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"a275fcc7d02cc7d0361dbb0d25ce51cd766a469a","unresolved":false,"context_lines":[{"line_number":1308,"context_line":"            except self.rbd.ImageNotFound:"},{"line_number":1309,"context_line":"                LOG.info(\"volume %s no longer exists in backend\","},{"line_number":1310,"context_line":"                         volume_name)"},{"line_number":1311,"context_line":"                return"},{"line_number":1312,"context_line":""},{"line_number":1313,"context_line":"            clone_snap \u003d None"},{"line_number":1314,"context_line":"            parent \u003d None"}],"source_content_type":"text/x-python","patch_set":11,"id":"f818655a_6dc6bec3","line":1311,"in_reply_to":"272e0a65_287d3d1b","updated":"2022-08-31 13:52:12.000000000","message":"Done","commit_id":"c61f7aa7928cd42b2a238c76ab0b565b840ce67c"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"0b3ba827ab313d174fc276f5df4da82acf821f4c","unresolved":true,"context_lines":[{"line_number":1308,"context_line":"            except self.rbd.ImageNotFound:"},{"line_number":1309,"context_line":"                LOG.info(\"volume %s no longer exists in backend\","},{"line_number":1310,"context_line":"                         volume_name)"},{"line_number":1311,"context_line":"                return"},{"line_number":1312,"context_line":""},{"line_number":1313,"context_line":"            clone_snap \u003d None"},{"line_number":1314,"context_line":"            parent \u003d None"}],"source_content_type":"text/x-python","patch_set":11,"id":"272e0a65_287d3d1b","line":1311,"in_reply_to":"c628367f_86a7e199","updated":"2022-07-06 14:13:36.000000000","message":"I think not -- if it returned ImageNotFound, it couldn\u0027t have the image open.  Any other librbd bits will be closed up by the rbd_image object being deleted.","commit_id":"c61f7aa7928cd42b2a238c76ab0b565b840ce67c"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"7ef54e55a57293c8f3cb8769b55d489ddd6029f9","unresolved":true,"context_lines":[{"line_number":1354,"context_line":"                    self.RBDProxy().remove(client.ioctx, volume_name)"},{"line_number":1355,"context_line":"                    return"},{"line_number":1356,"context_line":"                except (self.rbd.ImageHasSnapshots, self.rbd.ImageBusy):"},{"line_number":1357,"context_line":"                    msg \u003d (_(\"ImageBusy error raised while deleting \""},{"line_number":1358,"context_line":"                             \"rbd volume. This may have been caused by \""},{"line_number":1359,"context_line":"                             \"a connection from a client that has \""},{"line_number":1360,"context_line":"                             \"crashed and, if so, may be resolved by \""}],"source_content_type":"text/x-python","patch_set":11,"id":"da70a8a5_f5bb99e2","line":1357,"updated":"2022-06-24 18:06:52.000000000","message":"It feels odd to see a repeat of the same message at L1384","commit_id":"c61f7aa7928cd42b2a238c76ab0b565b840ce67c"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"220b3986e308ffb899f13cf39f3a5b58f03129b3","unresolved":false,"context_lines":[{"line_number":1354,"context_line":"                    self.RBDProxy().remove(client.ioctx, volume_name)"},{"line_number":1355,"context_line":"                    return"},{"line_number":1356,"context_line":"                except (self.rbd.ImageHasSnapshots, self.rbd.ImageBusy):"},{"line_number":1357,"context_line":"                    msg \u003d (_(\"ImageBusy error raised while deleting \""},{"line_number":1358,"context_line":"                             \"rbd volume. This may have been caused by \""},{"line_number":1359,"context_line":"                             \"a connection from a client that has \""},{"line_number":1360,"context_line":"                             \"crashed and, if so, may be resolved by \""}],"source_content_type":"text/x-python","patch_set":11,"id":"19a62c19_2980d08f","line":1357,"in_reply_to":"da70a8a5_f5bb99e2","updated":"2022-09-26 13:44:11.000000000","message":"Done","commit_id":"c61f7aa7928cd42b2a238c76ab0b565b840ce67c"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"7ef54e55a57293c8f3cb8769b55d489ddd6029f9","unresolved":true,"context_lines":[{"line_number":1363,"context_line":"                    LOG.warning(msg)"},{"line_number":1364,"context_line":"                    # Now raise this so that the volume stays available"},{"line_number":1365,"context_line":"                    # and the deletion can be retried."},{"line_number":1366,"context_line":"                    raise exception.VolumeIsBusy(msg, volume_name\u003dvolume_name)"},{"line_number":1367,"context_line":""},{"line_number":1368,"context_line":"                LOG.debug(\"moving volume %s to trash\", volume_name)"},{"line_number":1369,"context_line":"                # When using the RBD v2 clone api, deleting a volume"}],"source_content_type":"text/x-python","patch_set":11,"id":"c2f765b8_0e6d239e","line":1366,"updated":"2022-06-24 18:06:52.000000000","message":"This line confuses me. You raise VolumeIsBusy, but the retry at L1336 only handles ImageBusy.","commit_id":"c61f7aa7928cd42b2a238c76ab0b565b840ce67c"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"487d30b0a9fbcf165cf42c2dcbfb0591cd07ad2d","unresolved":false,"context_lines":[{"line_number":1363,"context_line":"                    LOG.warning(msg)"},{"line_number":1364,"context_line":"                    # Now raise this so that the volume stays available"},{"line_number":1365,"context_line":"                    # and the deletion can be retried."},{"line_number":1366,"context_line":"                    raise exception.VolumeIsBusy(msg, volume_name\u003dvolume_name)"},{"line_number":1367,"context_line":""},{"line_number":1368,"context_line":"                LOG.debug(\"moving volume %s to trash\", volume_name)"},{"line_number":1369,"context_line":"                # When using the RBD v2 clone api, deleting a volume"}],"source_content_type":"text/x-python","patch_set":11,"id":"ad96b3df_df58b1e3","line":1366,"in_reply_to":"c2f765b8_0e6d239e","updated":"2022-08-31 13:54:49.000000000","message":"Fixed in current patchset.","commit_id":"c61f7aa7928cd42b2a238c76ab0b565b840ce67c"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"7ef54e55a57293c8f3cb8769b55d489ddd6029f9","unresolved":true,"context_lines":[{"line_number":1365,"context_line":"                    # and the deletion can be retried."},{"line_number":1366,"context_line":"                    raise exception.VolumeIsBusy(msg, volume_name\u003dvolume_name)"},{"line_number":1367,"context_line":""},{"line_number":1368,"context_line":"                LOG.debug(\"moving volume %s to trash\", volume_name)"},{"line_number":1369,"context_line":"                # When using the RBD v2 clone api, deleting a volume"},{"line_number":1370,"context_line":"                # that has a snapshot in the trash space raises a"},{"line_number":1371,"context_line":"                # busy exception."}],"source_content_type":"text/x-python","patch_set":11,"id":"c2332879_acdb4b1e","line":1368,"updated":"2022-06-24 18:06:52.000000000","message":"Maybe I\u0027m missing something, but how do we reach this line? Doesn\u0027t L1366 preempt it?","commit_id":"c61f7aa7928cd42b2a238c76ab0b565b840ce67c"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"743393ed0100e5e87059e0504ae01bbfe063cea4","unresolved":false,"context_lines":[{"line_number":1365,"context_line":"                    # and the deletion can be retried."},{"line_number":1366,"context_line":"                    raise exception.VolumeIsBusy(msg, volume_name\u003dvolume_name)"},{"line_number":1367,"context_line":""},{"line_number":1368,"context_line":"                LOG.debug(\"moving volume %s to trash\", volume_name)"},{"line_number":1369,"context_line":"                # When using the RBD v2 clone api, deleting a volume"},{"line_number":1370,"context_line":"                # that has a snapshot in the trash space raises a"},{"line_number":1371,"context_line":"                # busy exception."}],"source_content_type":"text/x-python","patch_set":11,"id":"694f1310_e29a0c0f","line":1368,"in_reply_to":"c2332879_acdb4b1e","updated":"2022-08-31 13:50:14.000000000","message":"This is now fixed.","commit_id":"c61f7aa7928cd42b2a238c76ab0b565b840ce67c"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"7ef54e55a57293c8f3cb8769b55d489ddd6029f9","unresolved":true,"context_lines":[{"line_number":1436,"context_line":"                    \"ImageNotFound: Unable to unprotect snapshot %s.\","},{"line_number":1437,"context_line":"                    snap_name)"},{"line_number":1438,"context_line":"            except self.rbd.ImageBusy as e:"},{"line_number":1439,"context_line":"                volume.close()"},{"line_number":1440,"context_line":"                with RBDVolumeProxy(self, volume_name) as volume:"},{"line_number":1441,"context_line":"                    children_list \u003d self._get_children_info(volume, snap_name)"},{"line_number":1442,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"d945986c_8f13a451","line":1439,"updated":"2022-06-24 18:06:52.000000000","message":"I haven\u0027t delved deeply into context managers with exceptions, so I\u0027ll just raise a question.\n\nIs the \u0027volume\u0027 available here? Or do you need to reorder L1427 and L1428?","commit_id":"c61f7aa7928cd42b2a238c76ab0b565b840ce67c"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"de0ce40e8f8e8f44d2c1bffd610dba2c9dacc1ca","unresolved":false,"context_lines":[{"line_number":1436,"context_line":"                    \"ImageNotFound: Unable to unprotect snapshot %s.\","},{"line_number":1437,"context_line":"                    snap_name)"},{"line_number":1438,"context_line":"            except self.rbd.ImageBusy as e:"},{"line_number":1439,"context_line":"                volume.close()"},{"line_number":1440,"context_line":"                with RBDVolumeProxy(self, volume_name) as volume:"},{"line_number":1441,"context_line":"                    children_list \u003d self._get_children_info(volume, snap_name)"},{"line_number":1442,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"cd9ee89e_26217fc2","line":1439,"in_reply_to":"8c41bbd7_cfb92e6c","updated":"2022-07-06 14:18:11.000000000","message":"Done","commit_id":"c61f7aa7928cd42b2a238c76ab0b565b840ce67c"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"3ead0af84170c8d893e5b00770dbc0fd112dbc9f","unresolved":true,"context_lines":[{"line_number":1436,"context_line":"                    \"ImageNotFound: Unable to unprotect snapshot %s.\","},{"line_number":1437,"context_line":"                    snap_name)"},{"line_number":1438,"context_line":"            except self.rbd.ImageBusy as e:"},{"line_number":1439,"context_line":"                volume.close()"},{"line_number":1440,"context_line":"                with RBDVolumeProxy(self, volume_name) as volume:"},{"line_number":1441,"context_line":"                    children_list \u003d self._get_children_info(volume, snap_name)"},{"line_number":1442,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"8c41bbd7_cfb92e6c","line":1439,"in_reply_to":"d945986c_8f13a451","updated":"2022-07-06 14:14:46.000000000","message":"This looks broken, need to fix still.","commit_id":"c61f7aa7928cd42b2a238c76ab0b565b840ce67c"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"721229a9f6fd49ab744e72b3c7387b66cb15859f","unresolved":true,"context_lines":[{"line_number":1322,"context_line":"            raise exception.VolumeIsBusy(msg, volume_name\u003dvolume_name)"},{"line_number":1323,"context_line":""},{"line_number":1324,"context_line":"    def _try_remove_volume(self, client, volume_name: str) -\u003e bool:"},{"line_number":1325,"context_line":"        @utils.retry((self.rbd.ImageBusy, self.rbd.ImageHasSnapshots),"},{"line_number":1326,"context_line":"                     self.configuration.rados_connection_interval,"},{"line_number":1327,"context_line":"                     self.configuration.rados_connection_retries)"},{"line_number":1328,"context_line":"        def _do_try_remove_volume(self, client, volume_name: str) -\u003e bool:"}],"source_content_type":"text/x-python","patch_set":15,"id":"3f4455df_7fc5a464","line":1325,"updated":"2022-07-18 23:12:15.000000000","message":"Why is this retried? I can\u0027t identify any code that attempts to fix the problem. The code at L1334..L1336 just logs the problem, so it\u0027s unclear to me how a subsequent retry would succeed.","commit_id":"8839f09295c3963775d7460e676ea7cf8dfff16a"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"f118314dbf34edcc440461b10b032422dbe1dbfe","unresolved":true,"context_lines":[{"line_number":1322,"context_line":"            raise exception.VolumeIsBusy(msg, volume_name\u003dvolume_name)"},{"line_number":1323,"context_line":""},{"line_number":1324,"context_line":"    def _try_remove_volume(self, client, volume_name: str) -\u003e bool:"},{"line_number":1325,"context_line":"        @utils.retry((self.rbd.ImageBusy, self.rbd.ImageHasSnapshots),"},{"line_number":1326,"context_line":"                     self.configuration.rados_connection_interval,"},{"line_number":1327,"context_line":"                     self.configuration.rados_connection_retries)"},{"line_number":1328,"context_line":"        def _do_try_remove_volume(self, client, volume_name: str) -\u003e bool:"}],"source_content_type":"text/x-python","patch_set":15,"id":"deb5ca11_e8e353fd","line":1325,"in_reply_to":"3f4455df_7fc5a464","updated":"2022-07-26 16:24:23.000000000","message":"This helps in the event of multiple simultaneous Cinder delete operations -- the dependency causing the ImageBusy error can be removed between retries.","commit_id":"8839f09295c3963775d7460e676ea7cf8dfff16a"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"3bd6c04f08b12c55b2a6e9db225cf9429672dba9","unresolved":false,"context_lines":[{"line_number":1322,"context_line":"            raise exception.VolumeIsBusy(msg, volume_name\u003dvolume_name)"},{"line_number":1323,"context_line":""},{"line_number":1324,"context_line":"    def _try_remove_volume(self, client, volume_name: str) -\u003e bool:"},{"line_number":1325,"context_line":"        @utils.retry((self.rbd.ImageBusy, self.rbd.ImageHasSnapshots),"},{"line_number":1326,"context_line":"                     self.configuration.rados_connection_interval,"},{"line_number":1327,"context_line":"                     self.configuration.rados_connection_retries)"},{"line_number":1328,"context_line":"        def _do_try_remove_volume(self, client, volume_name: str) -\u003e bool:"}],"source_content_type":"text/x-python","patch_set":15,"id":"28436ae9_720ab62a","line":1325,"in_reply_to":"49dc86b0_a7c2a257","updated":"2022-08-22 18:26:27.000000000","message":"Done","commit_id":"8839f09295c3963775d7460e676ea7cf8dfff16a"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"1878d22a3627e0d51fed4c488129052dc42c992b","unresolved":true,"context_lines":[{"line_number":1322,"context_line":"            raise exception.VolumeIsBusy(msg, volume_name\u003dvolume_name)"},{"line_number":1323,"context_line":""},{"line_number":1324,"context_line":"    def _try_remove_volume(self, client, volume_name: str) -\u003e bool:"},{"line_number":1325,"context_line":"        @utils.retry((self.rbd.ImageBusy, self.rbd.ImageHasSnapshots),"},{"line_number":1326,"context_line":"                     self.configuration.rados_connection_interval,"},{"line_number":1327,"context_line":"                     self.configuration.rados_connection_retries)"},{"line_number":1328,"context_line":"        def _do_try_remove_volume(self, client, volume_name: str) -\u003e bool:"}],"source_content_type":"text/x-python","patch_set":15,"id":"49dc86b0_a7c2a257","line":1325,"in_reply_to":"deb5ca11_e8e353fd","updated":"2022-08-19 20:52:33.000000000","message":"Ack, but I think a comment to that effect would help. It sounds like a case of \"don\u0027t give up on the first failure,\" but stop if multiple attempts still fail.","commit_id":"8839f09295c3963775d7460e676ea7cf8dfff16a"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"721229a9f6fd49ab744e72b3c7387b66cb15859f","unresolved":true,"context_lines":[{"line_number":1386,"context_line":"            if self.configuration.enable_deferred_deletion:"},{"line_number":1387,"context_line":"                delay \u003d self.configuration.deferred_deletion_delay"},{"line_number":1388,"context_line":"            try:"},{"line_number":1389,"context_line":"                self.RBDProxy().remove(client.ioctx, volume.name)"},{"line_number":1390,"context_line":"                return"},{"line_number":1391,"context_line":"            except (self.rbd.ImageHasSnapshots, self.rbd.ImageBusy):"},{"line_number":1392,"context_line":"                self._flatten_children(client.ioctx, volume.name)"}],"source_content_type":"text/x-python","patch_set":15,"id":"195536e3_049887d8","line":1389,"updated":"2022-07-18 23:12:15.000000000","message":"Seeing this function, which tries to remove the volume, and then a call to self._try_remove_volume() later, at L1400, leaves me confused.","commit_id":"8839f09295c3963775d7460e676ea7cf8dfff16a"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"f118314dbf34edcc440461b10b032422dbe1dbfe","unresolved":true,"context_lines":[{"line_number":1386,"context_line":"            if self.configuration.enable_deferred_deletion:"},{"line_number":1387,"context_line":"                delay \u003d self.configuration.deferred_deletion_delay"},{"line_number":1388,"context_line":"            try:"},{"line_number":1389,"context_line":"                self.RBDProxy().remove(client.ioctx, volume.name)"},{"line_number":1390,"context_line":"                return"},{"line_number":1391,"context_line":"            except (self.rbd.ImageHasSnapshots, self.rbd.ImageBusy):"},{"line_number":1392,"context_line":"                self._flatten_children(client.ioctx, volume.name)"}],"source_content_type":"text/x-python","patch_set":15,"id":"306dd98c_aa66adab","line":1389,"in_reply_to":"195536e3_049887d8","updated":"2022-07-26 16:24:23.000000000","message":"The key to this is in the self._flatten_children call below -- the later attempt may succeed.\n\nFirst we call .remove() for the simple happy path, the second call later to _try_remove_volume() will retry a couple of times to increase the chances of success when other operations are occurring.","commit_id":"8839f09295c3963775d7460e676ea7cf8dfff16a"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"9ff8124a715a5a399ee862d4ea81bd6f146cc0f5","unresolved":false,"context_lines":[{"line_number":1386,"context_line":"            if self.configuration.enable_deferred_deletion:"},{"line_number":1387,"context_line":"                delay \u003d self.configuration.deferred_deletion_delay"},{"line_number":1388,"context_line":"            try:"},{"line_number":1389,"context_line":"                self.RBDProxy().remove(client.ioctx, volume.name)"},{"line_number":1390,"context_line":"                return"},{"line_number":1391,"context_line":"            except (self.rbd.ImageHasSnapshots, self.rbd.ImageBusy):"},{"line_number":1392,"context_line":"                self._flatten_children(client.ioctx, volume.name)"}],"source_content_type":"text/x-python","patch_set":15,"id":"17fec501_712ae7c0","line":1389,"in_reply_to":"306dd98c_aa66adab","updated":"2023-07-12 17:47:02.000000000","message":"Done","commit_id":"8839f09295c3963775d7460e676ea7cf8dfff16a"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"f118314dbf34edcc440461b10b032422dbe1dbfe","unresolved":false,"context_lines":[{"line_number":1390,"context_line":"                return"},{"line_number":1391,"context_line":"            except (self.rbd.ImageHasSnapshots, self.rbd.ImageBusy):"},{"line_number":1392,"context_line":"                self._flatten_children(client.ioctx, volume.name)"},{"line_number":1393,"context_line":"                pass"},{"line_number":1394,"context_line":"            except self.rbd.ImageNotFound:"},{"line_number":1395,"context_line":"                LOG.info(\"RBD volume %s not found, allowing delete \""},{"line_number":1396,"context_line":"                         \"operation to proceed.\", volume.name)"}],"source_content_type":"text/x-python","patch_set":15,"id":"edbd2053_8efbf084","line":1393,"range":{"start_line":1393,"start_character":16,"end_line":1393,"end_character":20},"updated":"2022-07-26 16:24:23.000000000","message":"Should remove this line.","commit_id":"8839f09295c3963775d7460e676ea7cf8dfff16a"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"721229a9f6fd49ab744e72b3c7387b66cb15859f","unresolved":true,"context_lines":[{"line_number":1446,"context_line":"                    snap_name)"},{"line_number":1447,"context_line":"            except self.rbd.ImageBusy as e:"},{"line_number":1448,"context_line":"                with RBDVolumeProxy(self, volume_name) as volume:"},{"line_number":1449,"context_line":"                    children_list \u003d self._get_children_info(volume, snap_name)"},{"line_number":1450,"context_line":""},{"line_number":1451,"context_line":"                    if children_list:"},{"line_number":1452,"context_line":"                        for (pool, image) in children_list:"}],"source_content_type":"text/x-python","patch_set":15,"id":"bae5d4a3_9766063f","line":1449,"updated":"2022-07-18 23:12:15.000000000","message":"This chunk of code (L1449..L1459) looks like it\u0027s doing what the new self._flatten_children() function (L1289) does (conceptually they seem like they\u0027re doing the same thing). Seeing the same code inline in one location, and in a dedicated function elsewhere feels awkward.","commit_id":"8839f09295c3963775d7460e676ea7cf8dfff16a"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"9ff8124a715a5a399ee862d4ea81bd6f146cc0f5","unresolved":false,"context_lines":[{"line_number":1446,"context_line":"                    snap_name)"},{"line_number":1447,"context_line":"            except self.rbd.ImageBusy as e:"},{"line_number":1448,"context_line":"                with RBDVolumeProxy(self, volume_name) as volume:"},{"line_number":1449,"context_line":"                    children_list \u003d self._get_children_info(volume, snap_name)"},{"line_number":1450,"context_line":""},{"line_number":1451,"context_line":"                    if children_list:"},{"line_number":1452,"context_line":"                        for (pool, image) in children_list:"}],"source_content_type":"text/x-python","patch_set":15,"id":"e5ee81ab_090ad0c2","line":1449,"in_reply_to":"bae5d4a3_9766063f","updated":"2023-07-12 17:47:02.000000000","message":"Done","commit_id":"8839f09295c3963775d7460e676ea7cf8dfff16a"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"721229a9f6fd49ab744e72b3c7387b66cb15859f","unresolved":true,"context_lines":[{"line_number":1455,"context_line":"                                     {\u0027pool\u0027: pool,"},{"line_number":1456,"context_line":"                                      \u0027image\u0027: image,"},{"line_number":1457,"context_line":"                                      \u0027snap\u0027: snap_name})"},{"line_number":1458,"context_line":"                            volume.close()"},{"line_number":1459,"context_line":"                            self._flatten(pool, image)"},{"line_number":1460,"context_line":"                        raise e"},{"line_number":1461,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"128d1332_5fc4ea83","line":1458,"updated":"2022-07-18 23:12:15.000000000","message":"I suspect you\u0027re closing the volume in preparation for calling self._flatten(), but I don\u0027t think you should do it repeatedly inside the loop at L1452. Or am I missing something? I find it confusing to see the same context clause in 3 places: L1437, L1448 and L1466, and I hope they\u0027re not tripping over each other.","commit_id":"8839f09295c3963775d7460e676ea7cf8dfff16a"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"9ff8124a715a5a399ee862d4ea81bd6f146cc0f5","unresolved":false,"context_lines":[{"line_number":1455,"context_line":"                                     {\u0027pool\u0027: pool,"},{"line_number":1456,"context_line":"                                      \u0027image\u0027: image,"},{"line_number":1457,"context_line":"                                      \u0027snap\u0027: snap_name})"},{"line_number":1458,"context_line":"                            volume.close()"},{"line_number":1459,"context_line":"                            self._flatten(pool, image)"},{"line_number":1460,"context_line":"                        raise e"},{"line_number":1461,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"542b0281_10ccbcdc","line":1458,"in_reply_to":"128d1332_5fc4ea83","updated":"2023-07-12 17:47:02.000000000","message":"Done","commit_id":"8839f09295c3963775d7460e676ea7cf8dfff16a"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"94019af109ce527f496a882b570e9635f4746e87","unresolved":false,"context_lines":[{"line_number":1327,"context_line":"                     self.configuration.rados_connection_retries)"},{"line_number":1328,"context_line":"        def _do_try_remove_volume(self, client, volume_name: str) -\u003e bool:"},{"line_number":1329,"context_line":"            try:"},{"line_number":1330,"context_line":"                LOG.debug(f\u0027Trying to remove image {volume_name}\u0027)"},{"line_number":1331,"context_line":"                self.RBDProxy().remove(client.ioctx, volume_name)"},{"line_number":1332,"context_line":"                return True"},{"line_number":1333,"context_line":"            except (self.rbd.ImageHasSnapshots, self.rbd.ImageBusy):"}],"source_content_type":"text/x-python","patch_set":18,"id":"7b5a6433_0a023448","line":1330,"in_reply_to":"98a642e2_b700ddd3","updated":"2022-08-17 14:07:16.000000000","message":"\u003e pep8: G004 Logging statement uses f-string\n\nPlease fix.","commit_id":"b7c65dc02ea29d32b992677e0e4364532a979127"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"9c8c6aa2b0bf8156303cd8b74f0efb2e55c352eb","unresolved":true,"context_lines":[{"line_number":759,"context_line":"            self,"},{"line_number":760,"context_line":"            dest_name: str,"},{"line_number":761,"context_line":"            client: RADOSClient) -\u003e None:"},{"line_number":762,"context_line":"        LOG.info(\"maximum clone depth (%d) has been reached - \""},{"line_number":763,"context_line":"                 \"flattening dest volume\","},{"line_number":764,"context_line":"                 self.configuration.rbd_max_clone_depth)"},{"line_number":765,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"9d9036f7_c95b6e42","line":762,"updated":"2022-08-19 23:06:36.000000000","message":"I know this is a super-bikeshedding moment, but this log message me slightly annoyed. Is this function called _flatten_volume_when_maximum_depth_is_reached? No, it looks completely like a function that may be called whenever one wants to flatten. I\u0027d keep that log at the call site, where it belongs.","commit_id":"1319919e2e08f56934fd709b0fa9b767a1031cdd"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"03b66c24d0884a7147d9d5997121f4e36a4a24c2","unresolved":false,"context_lines":[{"line_number":759,"context_line":"            self,"},{"line_number":760,"context_line":"            dest_name: str,"},{"line_number":761,"context_line":"            client: RADOSClient) -\u003e None:"},{"line_number":762,"context_line":"        LOG.info(\"maximum clone depth (%d) has been reached - \""},{"line_number":763,"context_line":"                 \"flattening dest volume\","},{"line_number":764,"context_line":"                 self.configuration.rbd_max_clone_depth)"},{"line_number":765,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"7f618376_da296552","line":762,"in_reply_to":"9d9036f7_c95b6e42","updated":"2023-02-02 10:53:52.000000000","message":"Done","commit_id":"1319919e2e08f56934fd709b0fa9b767a1031cdd"},{"author":{"_account_id":14084,"name":"Einst Crazy","email":"yu.changcai@99cloud.net","username":"Einst"},"change_message_id":"84a4605b7cbbe5f43488b75ca69cdd129c3a00b4","unresolved":true,"context_lines":[{"line_number":1298,"context_line":"        if children_list:"},{"line_number":1299,"context_line":"            for (pool, child_name) in children_list:"},{"line_number":1300,"context_line":"                LOG.info(\u0027Image %(pool)s/%(image)s is dependent \u0027"},{"line_number":1301,"context_line":"                         \u0027on the image %(volume_name)s.\u0027,"},{"line_number":1302,"context_line":"                         {\u0027pool\u0027: pool,"},{"line_number":1303,"context_line":"                          \u0027image\u0027: child_name,"},{"line_number":1304,"context_line":"                          \u0027volume_name\u0027: volume_name})"}],"source_content_type":"text/x-python","patch_set":19,"id":"6d0fc28e_3c5fe73a","line":1301,"range":{"start_line":1301,"start_character":55,"end_line":1301,"end_character":57},"updated":"2022-08-18 05:44:00.000000000","message":"better to log follow with \"now to flattern it\"","commit_id":"1319919e2e08f56934fd709b0fa9b767a1031cdd"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"c0c888728f7cfe338396aa1912b4dca93c86f1d7","unresolved":false,"context_lines":[{"line_number":1298,"context_line":"        if children_list:"},{"line_number":1299,"context_line":"            for (pool, child_name) in children_list:"},{"line_number":1300,"context_line":"                LOG.info(\u0027Image %(pool)s/%(image)s is dependent \u0027"},{"line_number":1301,"context_line":"                         \u0027on the image %(volume_name)s.\u0027,"},{"line_number":1302,"context_line":"                         {\u0027pool\u0027: pool,"},{"line_number":1303,"context_line":"                          \u0027image\u0027: child_name,"},{"line_number":1304,"context_line":"                          \u0027volume_name\u0027: volume_name})"}],"source_content_type":"text/x-python","patch_set":19,"id":"bc24b44d_6d97f44a","line":1301,"range":{"start_line":1301,"start_character":55,"end_line":1301,"end_character":57},"in_reply_to":"6d0fc28e_3c5fe73a","updated":"2022-08-19 14:34:03.000000000","message":"The self._flatten() method logs that a flatten operation is occuring for that image.","commit_id":"1319919e2e08f56934fd709b0fa9b767a1031cdd"},{"author":{"_account_id":14084,"name":"Einst Crazy","email":"yu.changcai@99cloud.net","username":"Einst"},"change_message_id":"84a4605b7cbbe5f43488b75ca69cdd129c3a00b4","unresolved":true,"context_lines":[{"line_number":1377,"context_line":""},{"line_number":1378,"context_line":"        if clone_snap is not None:"},{"line_number":1379,"context_line":"            # If the volume has copy-on-write clones, keep it as a silent"},{"line_number":1380,"context_line":"            # volume which will be deleted when its snapshot and clones"},{"line_number":1381,"context_line":"            # are deleted."},{"line_number":1382,"context_line":"            new_name \u003d \"%s.deleted\" % (volume.name)"},{"line_number":1383,"context_line":"            self.RBDProxy().rename(client.ioctx, volume.name, new_name)"}],"source_content_type":"text/x-python","patch_set":19,"id":"fa916c3e_b73e6a57","line":1380,"range":{"start_line":1380,"start_character":52,"end_line":1380,"end_character":60},"updated":"2022-08-18 05:44:00.000000000","message":"maybe snapshots is better","commit_id":"1319919e2e08f56934fd709b0fa9b767a1031cdd"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"aa1da8b8ad12b1435fbdc12d79039bfaf0524fc1","unresolved":false,"context_lines":[{"line_number":1377,"context_line":""},{"line_number":1378,"context_line":"        if clone_snap is not None:"},{"line_number":1379,"context_line":"            # If the volume has copy-on-write clones, keep it as a silent"},{"line_number":1380,"context_line":"            # volume which will be deleted when its snapshot and clones"},{"line_number":1381,"context_line":"            # are deleted."},{"line_number":1382,"context_line":"            new_name \u003d \"%s.deleted\" % (volume.name)"},{"line_number":1383,"context_line":"            self.RBDProxy().rename(client.ioctx, volume.name, new_name)"}],"source_content_type":"text/x-python","patch_set":19,"id":"84d386cb_06e6f890","line":1380,"range":{"start_line":1380,"start_character":52,"end_line":1380,"end_character":60},"in_reply_to":"fa916c3e_b73e6a57","updated":"2022-08-19 14:35:06.000000000","message":"Done","commit_id":"1319919e2e08f56934fd709b0fa9b767a1031cdd"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"1878d22a3627e0d51fed4c488129052dc42c992b","unresolved":true,"context_lines":[{"line_number":1227,"context_line":""},{"line_number":1228,"context_line":"        return (None, None, None)"},{"line_number":1229,"context_line":""},{"line_number":1230,"context_line":"    def _get_children_info(self,"},{"line_number":1231,"context_line":"                           volume: \u0027rbd.Image\u0027,"},{"line_number":1232,"context_line":"                           snap: Optional[str]) -\u003e list[tuple]:"},{"line_number":1233,"context_line":"        \"\"\"List children for the given snapshot of a volume(image)."}],"source_content_type":"text/x-python","patch_set":20,"id":"e459b0e6_15db8e9c","line":1230,"updated":"2022-08-19 20:52:33.000000000","message":"I think this function is no longer called.","commit_id":"affc787e69c5ef742236db6594ca9b71f753f499"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"3bd6c04f08b12c55b2a6e9db225cf9429672dba9","unresolved":true,"context_lines":[{"line_number":1227,"context_line":""},{"line_number":1228,"context_line":"        return (None, None, None)"},{"line_number":1229,"context_line":""},{"line_number":1230,"context_line":"    def _get_children_info(self,"},{"line_number":1231,"context_line":"                           volume: \u0027rbd.Image\u0027,"},{"line_number":1232,"context_line":"                           snap: Optional[str]) -\u003e list[tuple]:"},{"line_number":1233,"context_line":"        \"\"\"List children for the given snapshot of a volume(image)."}],"source_content_type":"text/x-python","patch_set":20,"id":"ff761198_336a3f62","line":1230,"in_reply_to":"e459b0e6_15db8e9c","updated":"2022-08-22 18:26:27.000000000","message":"It\u0027s not, but it might need to be restored.  In trying to deduplicate code, I replaced a call to _get_children_info with a call to _flatten_children, but this method called volume.set_snap(snap) and the _flatten_children method doesn\u0027t.\n\nThis appears to potentially have an impact on the results here, will go look at that.","commit_id":"affc787e69c5ef742236db6594ca9b71f753f499"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"c4a479c3c421e1ce1dac13a7f0dcb251dda58270","unresolved":false,"context_lines":[{"line_number":1227,"context_line":""},{"line_number":1228,"context_line":"        return (None, None, None)"},{"line_number":1229,"context_line":""},{"line_number":1230,"context_line":"    def _get_children_info(self,"},{"line_number":1231,"context_line":"                           volume: \u0027rbd.Image\u0027,"},{"line_number":1232,"context_line":"                           snap: Optional[str]) -\u003e list[tuple]:"},{"line_number":1233,"context_line":"        \"\"\"List children for the given snapshot of a volume(image)."}],"source_content_type":"text/x-python","patch_set":20,"id":"cddb6908_bb57bad6","line":1230,"in_reply_to":"ff761198_336a3f62","updated":"2022-08-31 13:48:33.000000000","message":"Done","commit_id":"affc787e69c5ef742236db6594ca9b71f753f499"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"1878d22a3627e0d51fed4c488129052dc42c992b","unresolved":true,"context_lines":[{"line_number":1295,"context_line":"        finally:"},{"line_number":1296,"context_line":"            rbd_image.close()"},{"line_number":1297,"context_line":""},{"line_number":1298,"context_line":"        if children_list:"},{"line_number":1299,"context_line":"            for (pool, child_name) in children_list:"},{"line_number":1300,"context_line":"                LOG.info(\u0027Image %(pool)s/%(image)s is dependent \u0027"},{"line_number":1301,"context_line":"                         \u0027on the image %(volume_name)s.\u0027,"}],"source_content_type":"text/x-python","patch_set":20,"id":"15246091_b7835019","line":1298,"updated":"2022-08-19 20:52:33.000000000","message":"What does list_children() on L1294 returns when there aren\u0027t any? If it\u0027s an iterable [] then L1298 isn\u0027t required. Maybe list_children() returns None?","commit_id":"affc787e69c5ef742236db6594ca9b71f753f499"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"60b60b9b309f2cef9d4ceb53923ccb0dce7390b4","unresolved":true,"context_lines":[{"line_number":1295,"context_line":"        finally:"},{"line_number":1296,"context_line":"            rbd_image.close()"},{"line_number":1297,"context_line":""},{"line_number":1298,"context_line":"        if children_list:"},{"line_number":1299,"context_line":"            for (pool, child_name) in children_list:"},{"line_number":1300,"context_line":"                LOG.info(\u0027Image %(pool)s/%(image)s is dependent \u0027"},{"line_number":1301,"context_line":"                         \u0027on the image %(volume_name)s.\u0027,"}],"source_content_type":"text/x-python","patch_set":20,"id":"66fa5100_d96c8547","line":1298,"in_reply_to":"15246091_b7835019","updated":"2022-08-24 19:41:24.000000000","message":"It appears to return a list -- will take a look at this idea.\n  https://github.com/ceph/ceph/blob/f5c21acfdc16e8a519f58dccee808baf0c713c4d/src/pybind/rbd/rbd.pyx#L4140","commit_id":"affc787e69c5ef742236db6594ca9b71f753f499"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"6ef69c3929e087505d29d3dbd8cabdfcdf15fb73","unresolved":false,"context_lines":[{"line_number":1295,"context_line":"        finally:"},{"line_number":1296,"context_line":"            rbd_image.close()"},{"line_number":1297,"context_line":""},{"line_number":1298,"context_line":"        if children_list:"},{"line_number":1299,"context_line":"            for (pool, child_name) in children_list:"},{"line_number":1300,"context_line":"                LOG.info(\u0027Image %(pool)s/%(image)s is dependent \u0027"},{"line_number":1301,"context_line":"                         \u0027on the image %(volume_name)s.\u0027,"}],"source_content_type":"text/x-python","patch_set":20,"id":"22f78e07_f7143a79","line":1298,"in_reply_to":"66fa5100_d96c8547","updated":"2023-08-08 18:42:11.000000000","message":"Done","commit_id":"affc787e69c5ef742236db6594ca9b71f753f499"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"1878d22a3627e0d51fed4c488129052dc42c992b","unresolved":true,"context_lines":[{"line_number":1387,"context_line":"        deleted \u003d False"},{"line_number":1388,"context_line":""},{"line_number":1389,"context_line":"        try:"},{"line_number":1390,"context_line":"            self.RBDProxy().remove(client.ioctx, volume.name)"},{"line_number":1391,"context_line":"            return"},{"line_number":1392,"context_line":"        except (self.rbd.ImageHasSnapshots, self.rbd.ImageBusy):"},{"line_number":1393,"context_line":"            self._flatten_children(client.ioctx, volume.name)"},{"line_number":1394,"context_line":"        except self.rbd.ImageNotFound:"}],"source_content_type":"text/x-python","patch_set":20,"id":"ba930e4a_127dfada","line":1391,"range":{"start_line":1390,"start_character":0,"end_line":1391,"end_character":18},"updated":"2022-08-19 20:52:33.000000000","message":"This is the \"fast path,\" right?","commit_id":"affc787e69c5ef742236db6594ca9b71f753f499"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"c4a479c3c421e1ce1dac13a7f0dcb251dda58270","unresolved":false,"context_lines":[{"line_number":1387,"context_line":"        deleted \u003d False"},{"line_number":1388,"context_line":""},{"line_number":1389,"context_line":"        try:"},{"line_number":1390,"context_line":"            self.RBDProxy().remove(client.ioctx, volume.name)"},{"line_number":1391,"context_line":"            return"},{"line_number":1392,"context_line":"        except (self.rbd.ImageHasSnapshots, self.rbd.ImageBusy):"},{"line_number":1393,"context_line":"            self._flatten_children(client.ioctx, volume.name)"},{"line_number":1394,"context_line":"        except self.rbd.ImageNotFound:"}],"source_content_type":"text/x-python","patch_set":20,"id":"22bea7f9_dba8e122","line":1391,"range":{"start_line":1390,"start_character":0,"end_line":1391,"end_character":18},"in_reply_to":"ba930e4a_127dfada","updated":"2022-08-31 13:48:33.000000000","message":"Correct","commit_id":"affc787e69c5ef742236db6594ca9b71f753f499"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"1878d22a3627e0d51fed4c488129052dc42c992b","unresolved":true,"context_lines":[{"line_number":1398,"context_line":""},{"line_number":1399,"context_line":"        try:"},{"line_number":1400,"context_line":"            deleted \u003d self._try_remove_volume(client, volume.name)"},{"line_number":1401,"context_line":"        except self.rbd.ImageHasSnapshots:"},{"line_number":1402,"context_line":"            msg \u003d _(\u0027ImageHasSnapshots error while deleting volume\u0027)"},{"line_number":1403,"context_line":"            raise exception.VolumeIsBusy(msg, volume_name\u003dvolume.name)"},{"line_number":1404,"context_line":"        except self.rbd.ImageBusy:"},{"line_number":1405,"context_line":"            msg \u003d _(\u0027ImageBusy error raised while deleting rbd volume\u0027)"},{"line_number":1406,"context_line":"            raise exception.VolumeIsBusy(msg, volume_name\u003dvolume.name)"},{"line_number":1407,"context_line":""},{"line_number":1408,"context_line":"        if deleted:"},{"line_number":1409,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":20,"id":"cd430004_20991bf3","line":1406,"range":{"start_line":1401,"start_character":0,"end_line":1406,"end_character":70},"updated":"2022-08-19 20:52:33.000000000","message":"Oh, we raise ImageBusy here? My previous (possibly incorrect) assumption was that this is when we\u0027d try to trash the volume.\n\nReading ahead to the comment starting at L1415, we trash the volume only if \"a child snapshot of it has been trashed but not yet removed.\" But if that\u0027s the case then why didn\u0027t we see the ImageHasSnapshots exception?\n\nI guess the exception DOESN\u0027T happen when a child snapshot is \"not visible\" because it exists, but it\u0027s in the trash. This RBD stuff is weird.","commit_id":"affc787e69c5ef742236db6594ca9b71f753f499"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"c4a479c3c421e1ce1dac13a7f0dcb251dda58270","unresolved":false,"context_lines":[{"line_number":1398,"context_line":""},{"line_number":1399,"context_line":"        try:"},{"line_number":1400,"context_line":"            deleted \u003d self._try_remove_volume(client, volume.name)"},{"line_number":1401,"context_line":"        except self.rbd.ImageHasSnapshots:"},{"line_number":1402,"context_line":"            msg \u003d _(\u0027ImageHasSnapshots error while deleting volume\u0027)"},{"line_number":1403,"context_line":"            raise exception.VolumeIsBusy(msg, volume_name\u003dvolume.name)"},{"line_number":1404,"context_line":"        except self.rbd.ImageBusy:"},{"line_number":1405,"context_line":"            msg \u003d _(\u0027ImageBusy error raised while deleting rbd volume\u0027)"},{"line_number":1406,"context_line":"            raise exception.VolumeIsBusy(msg, volume_name\u003dvolume.name)"},{"line_number":1407,"context_line":""},{"line_number":1408,"context_line":"        if deleted:"},{"line_number":1409,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":20,"id":"10f81098_4b147425","line":1406,"range":{"start_line":1401,"start_character":0,"end_line":1406,"end_character":70},"in_reply_to":"a3ccbe4d_543b70d7","updated":"2022-08-31 13:48:33.000000000","message":"Done","commit_id":"affc787e69c5ef742236db6594ca9b71f753f499"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"60b60b9b309f2cef9d4ceb53923ccb0dce7390b4","unresolved":true,"context_lines":[{"line_number":1398,"context_line":""},{"line_number":1399,"context_line":"        try:"},{"line_number":1400,"context_line":"            deleted \u003d self._try_remove_volume(client, volume.name)"},{"line_number":1401,"context_line":"        except self.rbd.ImageHasSnapshots:"},{"line_number":1402,"context_line":"            msg \u003d _(\u0027ImageHasSnapshots error while deleting volume\u0027)"},{"line_number":1403,"context_line":"            raise exception.VolumeIsBusy(msg, volume_name\u003dvolume.name)"},{"line_number":1404,"context_line":"        except self.rbd.ImageBusy:"},{"line_number":1405,"context_line":"            msg \u003d _(\u0027ImageBusy error raised while deleting rbd volume\u0027)"},{"line_number":1406,"context_line":"            raise exception.VolumeIsBusy(msg, volume_name\u003dvolume.name)"},{"line_number":1407,"context_line":""},{"line_number":1408,"context_line":"        if deleted:"},{"line_number":1409,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":20,"id":"a3ccbe4d_543b70d7","line":1406,"range":{"start_line":1401,"start_character":0,"end_line":1406,"end_character":70},"in_reply_to":"cd430004_20991bf3","updated":"2022-08-24 19:41:24.000000000","message":"Your assumption was right, this code was just wrong before.","commit_id":"affc787e69c5ef742236db6594ca9b71f753f499"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"1878d22a3627e0d51fed4c488129052dc42c992b","unresolved":true,"context_lines":[{"line_number":1460,"context_line":"                    snap_name)"},{"line_number":1461,"context_line":"            except self.rbd.ImageBusy as e:"},{"line_number":1462,"context_line":"                with RADOSClient(self) as client:"},{"line_number":1463,"context_line":"                    self._flatten_children(client.ioctx, volume.name)"},{"line_number":1464,"context_line":""},{"line_number":1465,"context_line":"                raise e"},{"line_number":1466,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"ab3697bf_278b0d72","line":1463,"updated":"2022-08-19 20:52:33.000000000","message":"I just want to confirm that we\u0027re flattening the volume associated with the snapshot, and not the snapshot\u0027s parent volume. Right?","commit_id":"affc787e69c5ef742236db6594ca9b71f753f499"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"145308ef43a8dc421c701091cd7bd97f3840265d","unresolved":true,"context_lines":[{"line_number":1460,"context_line":"                    snap_name)"},{"line_number":1461,"context_line":"            except self.rbd.ImageBusy as e:"},{"line_number":1462,"context_line":"                with RADOSClient(self) as client:"},{"line_number":1463,"context_line":"                    self._flatten_children(client.ioctx, volume.name)"},{"line_number":1464,"context_line":""},{"line_number":1465,"context_line":"                raise e"},{"line_number":1466,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"08128a2b_36514968","line":1463,"range":{"start_line":1463,"start_character":57,"end_line":1463,"end_character":68},"updated":"2022-08-23 21:47:18.000000000","message":"volume.name is a bug here, should be volume_name.","commit_id":"affc787e69c5ef742236db6594ca9b71f753f499"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"ee4eead2369f8cf4d30fd097f64a2579136cfa91","unresolved":false,"context_lines":[{"line_number":1460,"context_line":"                    snap_name)"},{"line_number":1461,"context_line":"            except self.rbd.ImageBusy as e:"},{"line_number":1462,"context_line":"                with RADOSClient(self) as client:"},{"line_number":1463,"context_line":"                    self._flatten_children(client.ioctx, volume.name)"},{"line_number":1464,"context_line":""},{"line_number":1465,"context_line":"                raise e"},{"line_number":1466,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"832f7901_4c84189b","line":1463,"range":{"start_line":1463,"start_character":57,"end_line":1463,"end_character":68},"in_reply_to":"08128a2b_36514968","updated":"2022-08-24 19:33:43.000000000","message":"Done","commit_id":"affc787e69c5ef742236db6594ca9b71f753f499"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"9ff8124a715a5a399ee862d4ea81bd6f146cc0f5","unresolved":false,"context_lines":[{"line_number":1460,"context_line":"                    snap_name)"},{"line_number":1461,"context_line":"            except self.rbd.ImageBusy as e:"},{"line_number":1462,"context_line":"                with RADOSClient(self) as client:"},{"line_number":1463,"context_line":"                    self._flatten_children(client.ioctx, volume.name)"},{"line_number":1464,"context_line":""},{"line_number":1465,"context_line":"                raise e"},{"line_number":1466,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"206953e5_0894f0de","line":1463,"in_reply_to":"37d1752d_da713f4c","updated":"2023-07-12 17:47:02.000000000","message":"Done","commit_id":"affc787e69c5ef742236db6594ca9b71f753f499"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"3bd6c04f08b12c55b2a6e9db225cf9429672dba9","unresolved":true,"context_lines":[{"line_number":1460,"context_line":"                    snap_name)"},{"line_number":1461,"context_line":"            except self.rbd.ImageBusy as e:"},{"line_number":1462,"context_line":"                with RADOSClient(self) as client:"},{"line_number":1463,"context_line":"                    self._flatten_children(client.ioctx, volume.name)"},{"line_number":1464,"context_line":""},{"line_number":1465,"context_line":"                raise e"},{"line_number":1466,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"37d1752d_da713f4c","line":1463,"in_reply_to":"ab3697bf_278b0d72","updated":"2022-08-22 18:26:27.000000000","message":"See my above note about _get_children_info, going to check some details here.","commit_id":"affc787e69c5ef742236db6594ca9b71f753f499"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"87573f0c343656464a549badf6b72814802cd08b","unresolved":true,"context_lines":[{"line_number":133,"context_line":"    cfg.IntOpt(\u0027deferred_deletion_purge_interval\u0027, default\u003d60,"},{"line_number":134,"context_line":"               help\u003d\u0027Number of seconds between runs of the periodic task \u0027"},{"line_number":135,"context_line":"                    \u0027to purge volumes tagged for deletion.\u0027),"},{"line_number":136,"context_line":"    cfg.IntOpt(\u0027rbd_concurrent_flatten_operations\u0027, default\u003d3,"},{"line_number":137,"context_line":"               help\u003d\u0027Number of flatten operations that will run \u0027"},{"line_number":138,"context_line":"                    \u0027concurrently.\u0027)"},{"line_number":139,"context_line":"]"}],"source_content_type":"text/x-python","patch_set":26,"id":"02a91674_79f1d1a7","line":136,"updated":"2022-09-05 09:50:40.000000000","message":"nit: We should set a minimum value (no negative values). Should it be 1 or should we allow 0 in case we don\u0027t want all services in an A/A deployment doing it?","commit_id":"c9d1dcce870ee21b3a9c81e96445614a172a16ac"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"a943b3ed5f7c0b774ef2c475239bd055de8aea1b","unresolved":false,"context_lines":[{"line_number":133,"context_line":"    cfg.IntOpt(\u0027deferred_deletion_purge_interval\u0027, default\u003d60,"},{"line_number":134,"context_line":"               help\u003d\u0027Number of seconds between runs of the periodic task \u0027"},{"line_number":135,"context_line":"                    \u0027to purge volumes tagged for deletion.\u0027),"},{"line_number":136,"context_line":"    cfg.IntOpt(\u0027rbd_concurrent_flatten_operations\u0027, default\u003d3,"},{"line_number":137,"context_line":"               help\u003d\u0027Number of flatten operations that will run \u0027"},{"line_number":138,"context_line":"                    \u0027concurrently.\u0027)"},{"line_number":139,"context_line":"]"}],"source_content_type":"text/x-python","patch_set":26,"id":"00259ee5_034e47df","line":136,"in_reply_to":"02a91674_79f1d1a7","updated":"2022-09-15 15:05:53.000000000","message":"Done","commit_id":"c9d1dcce870ee21b3a9c81e96445614a172a16ac"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"87573f0c343656464a549badf6b72814802cd08b","unresolved":true,"context_lines":[{"line_number":135,"context_line":"                    \u0027to purge volumes tagged for deletion.\u0027),"},{"line_number":136,"context_line":"    cfg.IntOpt(\u0027rbd_concurrent_flatten_operations\u0027, default\u003d3,"},{"line_number":137,"context_line":"               help\u003d\u0027Number of flatten operations that will run \u0027"},{"line_number":138,"context_line":"                    \u0027concurrently.\u0027)"},{"line_number":139,"context_line":"]"},{"line_number":140,"context_line":""},{"line_number":141,"context_line":"CONF \u003d cfg.CONF"}],"source_content_type":"text/x-python","patch_set":26,"id":"f618f5fe_0dd00c2c","line":138,"updated":"2022-09-05 09:50:40.000000000","message":"nit: Maybe we should express (here or in the docs) that this is not a global value: \"concurrently for each RBD backend-service (eg: 2 RBD backends running Active-Active on 3 nodes will have 3*3*2 max concurrent operations.).\"","commit_id":"c9d1dcce870ee21b3a9c81e96445614a172a16ac"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"a943b3ed5f7c0b774ef2c475239bd055de8aea1b","unresolved":false,"context_lines":[{"line_number":135,"context_line":"                    \u0027to purge volumes tagged for deletion.\u0027),"},{"line_number":136,"context_line":"    cfg.IntOpt(\u0027rbd_concurrent_flatten_operations\u0027, default\u003d3,"},{"line_number":137,"context_line":"               help\u003d\u0027Number of flatten operations that will run \u0027"},{"line_number":138,"context_line":"                    \u0027concurrently.\u0027)"},{"line_number":139,"context_line":"]"},{"line_number":140,"context_line":""},{"line_number":141,"context_line":"CONF \u003d cfg.CONF"}],"source_content_type":"text/x-python","patch_set":26,"id":"b4676bc9_c80b574c","line":138,"in_reply_to":"f618f5fe_0dd00c2c","updated":"2022-09-15 15:05:53.000000000","message":"Done","commit_id":"c9d1dcce870ee21b3a9c81e96445614a172a16ac"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"87573f0c343656464a549badf6b72814802cd08b","unresolved":true,"context_lines":[{"line_number":758,"context_line":""},{"line_number":759,"context_line":"    def _flatten_volume("},{"line_number":760,"context_line":"            self,"},{"line_number":761,"context_line":"            dest_name: str,"},{"line_number":762,"context_line":"            client: RADOSClient) -\u003e None:"},{"line_number":763,"context_line":"        LOG.info(\"maximum clone depth (%d) has been reached - \""},{"line_number":764,"context_line":"                 \"flattening dest volume\","}],"source_content_type":"text/x-python","patch_set":26,"id":"3deb91b5_6cd36f13","line":761,"range":{"start_line":761,"start_character":12,"end_line":761,"end_character":21},"updated":"2022-09-05 09:50:40.000000000","message":"nit: Why call the variable dest_name instead of just image_name/volume_name or something generic like that?","commit_id":"c9d1dcce870ee21b3a9c81e96445614a172a16ac"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"a943b3ed5f7c0b774ef2c475239bd055de8aea1b","unresolved":false,"context_lines":[{"line_number":758,"context_line":""},{"line_number":759,"context_line":"    def _flatten_volume("},{"line_number":760,"context_line":"            self,"},{"line_number":761,"context_line":"            dest_name: str,"},{"line_number":762,"context_line":"            client: RADOSClient) -\u003e None:"},{"line_number":763,"context_line":"        LOG.info(\"maximum clone depth (%d) has been reached - \""},{"line_number":764,"context_line":"                 \"flattening dest volume\","}],"source_content_type":"text/x-python","patch_set":26,"id":"5318eade_2bc3fd9a","line":761,"range":{"start_line":761,"start_character":12,"end_line":761,"end_character":21},"in_reply_to":"3deb91b5_6cd36f13","updated":"2022-09-15 15:05:53.000000000","message":"Done","commit_id":"c9d1dcce870ee21b3a9c81e96445614a172a16ac"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"87573f0c343656464a549badf6b72814802cd08b","unresolved":true,"context_lines":[{"line_number":768,"context_line":"        try:"},{"line_number":769,"context_line":"            with RBDVolumeProxy(self, dest_name, client\u003dclient,"},{"line_number":770,"context_line":"                                ioctx\u003dclient.ioctx) as dest_volume:"},{"line_number":771,"context_line":"                LOG.debug(\"flattening dest volume %s\", dest_name)"},{"line_number":772,"context_line":"                dest_volume.flatten()"},{"line_number":773,"context_line":"        except Exception as e:"},{"line_number":774,"context_line":"            msg \u003d (_(\"Failed to flatten volume %(volume)s with \""}],"source_content_type":"text/x-python","patch_set":26,"id":"38a1bb1f_0295d530","line":771,"range":{"start_line":771,"start_character":38,"end_line":771,"end_character":42},"updated":"2022-09-05 09:50:40.000000000","message":"nit: should we remove \"dest\" here?","commit_id":"c9d1dcce870ee21b3a9c81e96445614a172a16ac"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"a943b3ed5f7c0b774ef2c475239bd055de8aea1b","unresolved":false,"context_lines":[{"line_number":768,"context_line":"        try:"},{"line_number":769,"context_line":"            with RBDVolumeProxy(self, dest_name, client\u003dclient,"},{"line_number":770,"context_line":"                                ioctx\u003dclient.ioctx) as dest_volume:"},{"line_number":771,"context_line":"                LOG.debug(\"flattening dest volume %s\", dest_name)"},{"line_number":772,"context_line":"                dest_volume.flatten()"},{"line_number":773,"context_line":"        except Exception as e:"},{"line_number":774,"context_line":"            msg \u003d (_(\"Failed to flatten volume %(volume)s with \""}],"source_content_type":"text/x-python","patch_set":26,"id":"5f461083_8d840c4a","line":771,"range":{"start_line":771,"start_character":38,"end_line":771,"end_character":42},"in_reply_to":"38a1bb1f_0295d530","updated":"2022-09-15 15:05:53.000000000","message":"Done","commit_id":"c9d1dcce870ee21b3a9c81e96445614a172a16ac"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"87573f0c343656464a549badf6b72814802cd08b","unresolved":true,"context_lines":[{"line_number":1075,"context_line":""},{"line_number":1076,"context_line":"        return volume_update"},{"line_number":1077,"context_line":""},{"line_number":1078,"context_line":"    def _flatten(self, pool: str, volume_name: str) -\u003e None:"},{"line_number":1079,"context_line":"        image \u003d pool + \u0027/\u0027 + volume_name"},{"line_number":1080,"context_line":""},{"line_number":1081,"context_line":"        @utils.limit_operations"}],"source_content_type":"text/x-python","patch_set":26,"id":"e3bb7e49_c494cb6d","line":1078,"updated":"2022-09-05 09:50:40.000000000","message":"nit: Should we group the 3 different flatten methods together and add a brief docstring explaining what each one does?","commit_id":"c9d1dcce870ee21b3a9c81e96445614a172a16ac"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"b346742be281a3f4e363a9fe02bf3333cbe15024","unresolved":false,"context_lines":[{"line_number":1075,"context_line":""},{"line_number":1076,"context_line":"        return volume_update"},{"line_number":1077,"context_line":""},{"line_number":1078,"context_line":"    def _flatten(self, pool: str, volume_name: str) -\u003e None:"},{"line_number":1079,"context_line":"        image \u003d pool + \u0027/\u0027 + volume_name"},{"line_number":1080,"context_line":""},{"line_number":1081,"context_line":"        @utils.limit_operations"}],"source_content_type":"text/x-python","patch_set":26,"id":"1caa4eb8_005b3350","line":1078,"in_reply_to":"e3bb7e49_c494cb6d","updated":"2023-02-07 19:15:14.000000000","message":"Will look into refactoring things for tidiness in a follow-up patch.","commit_id":"c9d1dcce870ee21b3a9c81e96445614a172a16ac"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"87573f0c343656464a549badf6b72814802cd08b","unresolved":true,"context_lines":[{"line_number":1094,"context_line":"                LOG.debug(\u0027image %s not found during flatten\u0027, volume_name)"},{"line_number":1095,"context_line":"                # do nothing"},{"line_number":1096,"context_line":""},{"line_number":1097,"context_line":"        LOG.debug(\u0027_flatten %s\u0027, image)"},{"line_number":1098,"context_line":"        do_flatten(self, volume_name, pool)"},{"line_number":1099,"context_line":""},{"line_number":1100,"context_line":"    def _get_stripe_unit(self, ioctx: \u0027rados.Ioctx\u0027, volume_name: str) -\u003e int:"}],"source_content_type":"text/x-python","patch_set":26,"id":"4e32d334_b4a73ff5","line":1097,"updated":"2022-09-05 09:50:40.000000000","message":"nit: Should we somehow reflect that the flattening is being queued? It may be helpful for those reading the logs without knowledge of the implementation:  \"Queuing %s for flattening\"","commit_id":"c9d1dcce870ee21b3a9c81e96445614a172a16ac"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"a943b3ed5f7c0b774ef2c475239bd055de8aea1b","unresolved":false,"context_lines":[{"line_number":1094,"context_line":"                LOG.debug(\u0027image %s not found during flatten\u0027, volume_name)"},{"line_number":1095,"context_line":"                # do nothing"},{"line_number":1096,"context_line":""},{"line_number":1097,"context_line":"        LOG.debug(\u0027_flatten %s\u0027, image)"},{"line_number":1098,"context_line":"        do_flatten(self, volume_name, pool)"},{"line_number":1099,"context_line":""},{"line_number":1100,"context_line":"    def _get_stripe_unit(self, ioctx: \u0027rados.Ioctx\u0027, volume_name: str) -\u003e int:"}],"source_content_type":"text/x-python","patch_set":26,"id":"fbf33c42_1bf64da0","line":1097,"in_reply_to":"4e32d334_b4a73ff5","updated":"2022-09-15 15:05:53.000000000","message":"Done","commit_id":"c9d1dcce870ee21b3a9c81e96445614a172a16ac"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"87573f0c343656464a549badf6b72814802cd08b","unresolved":true,"context_lines":[{"line_number":1279,"context_line":"                          client_ioctx: \u0027rados.Ioctx\u0027,"},{"line_number":1280,"context_line":"                          volume_name: str,"},{"line_number":1281,"context_line":"                          snap_name: Optional[str] \u003d None) -\u003e None:"},{"line_number":1282,"context_line":"        rbd_image \u003d self.rbd.Image(client_ioctx, volume_name)"},{"line_number":1283,"context_line":"        try:"},{"line_number":1284,"context_line":"            if snap_name is not None:"},{"line_number":1285,"context_line":"                rbd_image.set_snap(snap_name)"},{"line_number":1286,"context_line":""},{"line_number":1287,"context_line":"            children_list \u003d rbd_image.list_children()"},{"line_number":1288,"context_line":""},{"line_number":1289,"context_line":"            if snap_name is not None:"},{"line_number":1290,"context_line":"                rbd_image.set_snap(None)"},{"line_number":1291,"context_line":"        finally:"},{"line_number":1292,"context_line":"            rbd_image.close()"},{"line_number":1293,"context_line":""},{"line_number":1294,"context_line":"        if children_list:"},{"line_number":1295,"context_line":"            for (pool, child_name) in children_list:"}],"source_content_type":"text/x-python","patch_set":26,"id":"c04856d9_76b3c6ed","line":1292,"range":{"start_line":1282,"start_character":0,"end_line":1292,"end_character":29},"updated":"2022-09-05 09:50:40.000000000","message":"-1: All these methods need to be executed through a tpool proxy like the other calls.\n\n-1: Is it really necessary to call set_snap(\u0027none) before closing the image? If it isn\u0027t we can just remove it, and if it is then we cannot have that call inside the try, where an exception on the list_children would make the close be called without setting it.","commit_id":"c9d1dcce870ee21b3a9c81e96445614a172a16ac"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"a943b3ed5f7c0b774ef2c475239bd055de8aea1b","unresolved":false,"context_lines":[{"line_number":1279,"context_line":"                          client_ioctx: \u0027rados.Ioctx\u0027,"},{"line_number":1280,"context_line":"                          volume_name: str,"},{"line_number":1281,"context_line":"                          snap_name: Optional[str] \u003d None) -\u003e None:"},{"line_number":1282,"context_line":"        rbd_image \u003d self.rbd.Image(client_ioctx, volume_name)"},{"line_number":1283,"context_line":"        try:"},{"line_number":1284,"context_line":"            if snap_name is not None:"},{"line_number":1285,"context_line":"                rbd_image.set_snap(snap_name)"},{"line_number":1286,"context_line":""},{"line_number":1287,"context_line":"            children_list \u003d rbd_image.list_children()"},{"line_number":1288,"context_line":""},{"line_number":1289,"context_line":"            if snap_name is not None:"},{"line_number":1290,"context_line":"                rbd_image.set_snap(None)"},{"line_number":1291,"context_line":"        finally:"},{"line_number":1292,"context_line":"            rbd_image.close()"},{"line_number":1293,"context_line":""},{"line_number":1294,"context_line":"        if children_list:"},{"line_number":1295,"context_line":"            for (pool, child_name) in children_list:"}],"source_content_type":"text/x-python","patch_set":26,"id":"6001e6d5_d24d5214","line":1292,"range":{"start_line":1282,"start_character":0,"end_line":1292,"end_character":29},"in_reply_to":"c04856d9_76b3c6ed","updated":"2022-09-15 15:05:53.000000000","message":"Done","commit_id":"c9d1dcce870ee21b3a9c81e96445614a172a16ac"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"87573f0c343656464a549badf6b72814802cd08b","unresolved":true,"context_lines":[{"line_number":1329,"context_line":"        # this gives a window for other deletes of snapshots and images"},{"line_number":1330,"context_line":"        # to complete, freeing dependencies which allow this remove to"},{"line_number":1331,"context_line":"        # succeed."},{"line_number":1332,"context_line":"        @utils.retry((self.rbd.ImageBusy, self.rbd.ImageHasSnapshots),"},{"line_number":1333,"context_line":"                     self.configuration.rados_connection_interval,"},{"line_number":1334,"context_line":"                     self.configuration.rados_connection_retries)"},{"line_number":1335,"context_line":"        def _do_try_remove_volume(self, client, volume_name: str) -\u003e bool:"},{"line_number":1336,"context_line":"            try:"},{"line_number":1337,"context_line":"                LOG.debug(\u0027Trying to remove image %s\u0027, volume_name)"},{"line_number":1338,"context_line":"                self.RBDProxy().remove(client.ioctx, volume_name)"}],"source_content_type":"text/x-python","patch_set":26,"id":"7812ac07_f14ec61e","line":1335,"range":{"start_line":1332,"start_character":0,"end_line":1335,"end_character":74},"updated":"2022-09-05 09:50:40.000000000","message":"-1: Why have the _do_try_remove_volume with the decorators and requiring an extra method call instead of adding those same decorators to the _try_remove_volume method itself?","commit_id":"c9d1dcce870ee21b3a9c81e96445614a172a16ac"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"d336c2b1356ca7be47d08f41470d3628688bc206","unresolved":false,"context_lines":[{"line_number":1329,"context_line":"        # this gives a window for other deletes of snapshots and images"},{"line_number":1330,"context_line":"        # to complete, freeing dependencies which allow this remove to"},{"line_number":1331,"context_line":"        # succeed."},{"line_number":1332,"context_line":"        @utils.retry((self.rbd.ImageBusy, self.rbd.ImageHasSnapshots),"},{"line_number":1333,"context_line":"                     self.configuration.rados_connection_interval,"},{"line_number":1334,"context_line":"                     self.configuration.rados_connection_retries)"},{"line_number":1335,"context_line":"        def _do_try_remove_volume(self, client, volume_name: str) -\u003e bool:"},{"line_number":1336,"context_line":"            try:"},{"line_number":1337,"context_line":"                LOG.debug(\u0027Trying to remove image %s\u0027, volume_name)"},{"line_number":1338,"context_line":"                self.RBDProxy().remove(client.ioctx, volume_name)"}],"source_content_type":"text/x-python","patch_set":26,"id":"6089dc9f_301e05b0","line":1335,"range":{"start_line":1332,"start_character":0,"end_line":1335,"end_character":74},"in_reply_to":"7812ac07_f14ec61e","updated":"2022-09-15 18:56:23.000000000","message":"Because of the weird handling of rbd dependencies, which means that we need to reference self.rbd.ImageBusy etc., which aren\u0027t available if decorating the _try_remove_volume method itself.\n\n(Similar for self.configuration.)","commit_id":"c9d1dcce870ee21b3a9c81e96445614a172a16ac"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"87573f0c343656464a549badf6b72814802cd08b","unresolved":true,"context_lines":[{"line_number":1348,"context_line":""},{"line_number":1349,"context_line":"    @staticmethod"},{"line_number":1350,"context_line":"    def _find_clone_snap(rbd_image) -\u003e Optional[str]:"},{"line_number":1351,"context_line":"        snaps \u003d rbd_image.list_snaps()"},{"line_number":1352,"context_line":"        for snap in snaps:"},{"line_number":1353,"context_line":"            if snap[\u0027name\u0027].endswith(\u0027.clone_snap\u0027):"},{"line_number":1354,"context_line":"                LOG.debug(\"volume has clone snapshot(s)\")"}],"source_content_type":"text/x-python","patch_set":26,"id":"a89fc165_ae023b85","line":1351,"updated":"2022-09-05 09:50:40.000000000","message":"-1: This needs to be called through a proxy","commit_id":"c9d1dcce870ee21b3a9c81e96445614a172a16ac"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"a943b3ed5f7c0b774ef2c475239bd055de8aea1b","unresolved":false,"context_lines":[{"line_number":1348,"context_line":""},{"line_number":1349,"context_line":"    @staticmethod"},{"line_number":1350,"context_line":"    def _find_clone_snap(rbd_image) -\u003e Optional[str]:"},{"line_number":1351,"context_line":"        snaps \u003d rbd_image.list_snaps()"},{"line_number":1352,"context_line":"        for snap in snaps:"},{"line_number":1353,"context_line":"            if snap[\u0027name\u0027].endswith(\u0027.clone_snap\u0027):"},{"line_number":1354,"context_line":"                LOG.debug(\"volume has clone snapshot(s)\")"}],"source_content_type":"text/x-python","patch_set":26,"id":"ec6af2ef_f13d9b4d","line":1351,"in_reply_to":"a89fc165_ae023b85","updated":"2022-09-15 15:05:53.000000000","message":"Done","commit_id":"c9d1dcce870ee21b3a9c81e96445614a172a16ac"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"87573f0c343656464a549badf6b72814802cd08b","unresolved":true,"context_lines":[{"line_number":1382,"context_line":"        finally:"},{"line_number":1383,"context_line":"            rbd_image.close()"},{"line_number":1384,"context_line":""},{"line_number":1385,"context_line":"        if clone_snap is not None:"},{"line_number":1386,"context_line":"            # If the volume has copy-on-write clones, keep it as a silent"},{"line_number":1387,"context_line":"            # volume which will be deleted when its snapshots and clones"},{"line_number":1388,"context_line":"            # are deleted."},{"line_number":1389,"context_line":"            # TODO: only do this if it actually can\u0027t be deleted?"},{"line_number":1390,"context_line":"            new_name \u003d \"%s.deleted\" % (volume.name)"},{"line_number":1391,"context_line":"            self.RBDProxy().rename(client.ioctx, volume.name, new_name)"},{"line_number":1392,"context_line":"            return"},{"line_number":1393,"context_line":""},{"line_number":1394,"context_line":"        LOG.debug(\"deleting rbd volume %s\", volume.name)"},{"line_number":1395,"context_line":"        deleted \u003d False"}],"source_content_type":"text/x-python","patch_set":26,"id":"ed359155_97e8a484","line":1392,"range":{"start_line":1385,"start_character":0,"end_line":1392,"end_character":18},"updated":"2022-09-05 09:50:40.000000000","message":"-1: This is not mentioned in the commit message as one of the \"strategies\". Can\u0027t we just move it to the trash instead of trying to keep track ourselves of the \"deleted\" volumes?","commit_id":"c9d1dcce870ee21b3a9c81e96445614a172a16ac"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"bcd4b7583e7d8e06769343116e1136a2e39828ce","unresolved":false,"context_lines":[{"line_number":1382,"context_line":"        finally:"},{"line_number":1383,"context_line":"            rbd_image.close()"},{"line_number":1384,"context_line":""},{"line_number":1385,"context_line":"        if clone_snap is not None:"},{"line_number":1386,"context_line":"            # If the volume has copy-on-write clones, keep it as a silent"},{"line_number":1387,"context_line":"            # volume which will be deleted when its snapshots and clones"},{"line_number":1388,"context_line":"            # are deleted."},{"line_number":1389,"context_line":"            # TODO: only do this if it actually can\u0027t be deleted?"},{"line_number":1390,"context_line":"            new_name \u003d \"%s.deleted\" % (volume.name)"},{"line_number":1391,"context_line":"            self.RBDProxy().rename(client.ioctx, volume.name, new_name)"},{"line_number":1392,"context_line":"            return"},{"line_number":1393,"context_line":""},{"line_number":1394,"context_line":"        LOG.debug(\"deleting rbd volume %s\", volume.name)"},{"line_number":1395,"context_line":"        deleted \u003d False"}],"source_content_type":"text/x-python","patch_set":26,"id":"ee79081e_de076043","line":1392,"range":{"start_line":1385,"start_character":0,"end_line":1392,"end_character":18},"in_reply_to":"0683fa06_f2a3cb2e","updated":"2023-08-17 18:19:45.000000000","message":"You\u0027re right, I think there is a case here where we can do something better (remove/trash) but aren\u0027t yet.  I\u0027ll handle this in a follow-up patch and note it in the commit message here.","commit_id":"c9d1dcce870ee21b3a9c81e96445614a172a16ac"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"9ff8124a715a5a399ee862d4ea81bd6f146cc0f5","unresolved":true,"context_lines":[{"line_number":1382,"context_line":"        finally:"},{"line_number":1383,"context_line":"            rbd_image.close()"},{"line_number":1384,"context_line":""},{"line_number":1385,"context_line":"        if clone_snap is not None:"},{"line_number":1386,"context_line":"            # If the volume has copy-on-write clones, keep it as a silent"},{"line_number":1387,"context_line":"            # volume which will be deleted when its snapshots and clones"},{"line_number":1388,"context_line":"            # are deleted."},{"line_number":1389,"context_line":"            # TODO: only do this if it actually can\u0027t be deleted?"},{"line_number":1390,"context_line":"            new_name \u003d \"%s.deleted\" % (volume.name)"},{"line_number":1391,"context_line":"            self.RBDProxy().rename(client.ioctx, volume.name, new_name)"},{"line_number":1392,"context_line":"            return"},{"line_number":1393,"context_line":""},{"line_number":1394,"context_line":"        LOG.debug(\"deleting rbd volume %s\", volume.name)"},{"line_number":1395,"context_line":"        deleted \u003d False"}],"source_content_type":"text/x-python","patch_set":26,"id":"0683fa06_f2a3cb2e","line":1392,"range":{"start_line":1385,"start_character":0,"end_line":1392,"end_character":18},"in_reply_to":"ed359155_97e8a484","updated":"2023-07-12 17:47:02.000000000","message":"The .deleted renaming is the old strategy and is no longer done, but we need to maintain appropriate compatibility with it.  Let me think on how to document this...","commit_id":"c9d1dcce870ee21b3a9c81e96445614a172a16ac"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"87573f0c343656464a549badf6b72814802cd08b","unresolved":true,"context_lines":[{"line_number":1392,"context_line":"            return"},{"line_number":1393,"context_line":""},{"line_number":1394,"context_line":"        LOG.debug(\"deleting rbd volume %s\", volume.name)"},{"line_number":1395,"context_line":"        deleted \u003d False"},{"line_number":1396,"context_line":""},{"line_number":1397,"context_line":"        try:"},{"line_number":1398,"context_line":"            self.RBDProxy().remove(client.ioctx, volume.name)"}],"source_content_type":"text/x-python","patch_set":26,"id":"40a83448_c55c6c45","line":1395,"updated":"2022-09-05 09:50:40.000000000","message":"nit: Maybe move this to L1407 or L1411 replacing the pass","commit_id":"c9d1dcce870ee21b3a9c81e96445614a172a16ac"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"9ff8124a715a5a399ee862d4ea81bd6f146cc0f5","unresolved":false,"context_lines":[{"line_number":1392,"context_line":"            return"},{"line_number":1393,"context_line":""},{"line_number":1394,"context_line":"        LOG.debug(\"deleting rbd volume %s\", volume.name)"},{"line_number":1395,"context_line":"        deleted \u003d False"},{"line_number":1396,"context_line":""},{"line_number":1397,"context_line":"        try:"},{"line_number":1398,"context_line":"            self.RBDProxy().remove(client.ioctx, volume.name)"}],"source_content_type":"text/x-python","patch_set":26,"id":"f894aa19_994574fa","line":1395,"in_reply_to":"40a83448_c55c6c45","updated":"2023-07-12 17:47:02.000000000","message":"I don\u0027t really understand this one, but since it\u0027s a nit and this code has been worked on a bit since then, I\u0027m going to check it off the list for now.","commit_id":"c9d1dcce870ee21b3a9c81e96445614a172a16ac"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"87573f0c343656464a549badf6b72814802cd08b","unresolved":true,"context_lines":[{"line_number":1407,"context_line":"        try:"},{"line_number":1408,"context_line":"            deleted \u003d self._try_remove_volume(client, volume.name)"},{"line_number":1409,"context_line":"        except self.rbd.ImageHasSnapshots:"},{"line_number":1410,"context_line":"            # perform trash instead, which can succeed when snapshots exist"},{"line_number":1411,"context_line":"            pass"},{"line_number":1412,"context_line":"        except self.rbd.ImageBusy:"},{"line_number":1413,"context_line":"            msg \u003d _(\u0027ImageBusy error raised while deleting rbd volume\u0027)"}],"source_content_type":"text/x-python","patch_set":26,"id":"e5c9000b_4355f319","line":1410,"updated":"2022-09-05 09:50:40.000000000","message":"nit: Mention in the comment here that these snapshots are not visible because they are in the trash (what you say in the commit message an on L1423).  Otherwise it\u0027s hard to understand why we still have this issue after flattening.","commit_id":"c9d1dcce870ee21b3a9c81e96445614a172a16ac"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"9ff8124a715a5a399ee862d4ea81bd6f146cc0f5","unresolved":false,"context_lines":[{"line_number":1407,"context_line":"        try:"},{"line_number":1408,"context_line":"            deleted \u003d self._try_remove_volume(client, volume.name)"},{"line_number":1409,"context_line":"        except self.rbd.ImageHasSnapshots:"},{"line_number":1410,"context_line":"            # perform trash instead, which can succeed when snapshots exist"},{"line_number":1411,"context_line":"            pass"},{"line_number":1412,"context_line":"        except self.rbd.ImageBusy:"},{"line_number":1413,"context_line":"            msg \u003d _(\u0027ImageBusy error raised while deleting rbd volume\u0027)"}],"source_content_type":"text/x-python","patch_set":26,"id":"ec6b1482_e9567453","line":1410,"in_reply_to":"4440fceb_ee484574","updated":"2023-07-12 17:47:02.000000000","message":"Yes.","commit_id":"c9d1dcce870ee21b3a9c81e96445614a172a16ac"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"5c242a010c24998436f072bf3e3bf9a0e5316954","unresolved":true,"context_lines":[{"line_number":1407,"context_line":"        try:"},{"line_number":1408,"context_line":"            deleted \u003d self._try_remove_volume(client, volume.name)"},{"line_number":1409,"context_line":"        except self.rbd.ImageHasSnapshots:"},{"line_number":1410,"context_line":"            # perform trash instead, which can succeed when snapshots exist"},{"line_number":1411,"context_line":"            pass"},{"line_number":1412,"context_line":"        except self.rbd.ImageBusy:"},{"line_number":1413,"context_line":"            msg \u003d _(\u0027ImageBusy error raised while deleting rbd volume\u0027)"}],"source_content_type":"text/x-python","patch_set":26,"id":"4440fceb_ee484574","line":1410,"in_reply_to":"e5c9000b_4355f319","updated":"2022-09-27 09:05:37.000000000","message":"Isn\u0027t it mentioned on L#1510-1514?","commit_id":"c9d1dcce870ee21b3a9c81e96445614a172a16ac"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"87573f0c343656464a549badf6b72814802cd08b","unresolved":true,"context_lines":[{"line_number":1475,"context_line":"                raise e"},{"line_number":1476,"context_line":""},{"line_number":1477,"context_line":"        try:"},{"line_number":1478,"context_line":"            do_unprotect_snap(self, volume_name, snap_name)"},{"line_number":1479,"context_line":"        except self.rbd.ImageBusy:"},{"line_number":1480,"context_line":"            raise exception.SnapshotIsBusy(snapshot_name\u003dsnap_name)"},{"line_number":1481,"context_line":""}],"source_content_type":"text/x-python","patch_set":26,"id":"e519d1d5_32529d39","line":1478,"updated":"2022-09-05 09:50:40.000000000","message":"-1: There is an unnecessary call to remove_snap below when unprotect_snap raises ImageNotFound.  I know that we currently have this behavior as well, but we are refactoring the code, so imo we should fix it.","commit_id":"c9d1dcce870ee21b3a9c81e96445614a172a16ac"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"a943b3ed5f7c0b774ef2c475239bd055de8aea1b","unresolved":false,"context_lines":[{"line_number":1475,"context_line":"                raise e"},{"line_number":1476,"context_line":""},{"line_number":1477,"context_line":"        try:"},{"line_number":1478,"context_line":"            do_unprotect_snap(self, volume_name, snap_name)"},{"line_number":1479,"context_line":"        except self.rbd.ImageBusy:"},{"line_number":1480,"context_line":"            raise exception.SnapshotIsBusy(snapshot_name\u003dsnap_name)"},{"line_number":1481,"context_line":""}],"source_content_type":"text/x-python","patch_set":26,"id":"39a99a93_abfb462a","line":1478,"in_reply_to":"e519d1d5_32529d39","updated":"2022-09-15 15:05:53.000000000","message":"Done","commit_id":"c9d1dcce870ee21b3a9c81e96445614a172a16ac"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"774467c1177be3ee40691d35ce2972d1afa6d84f","unresolved":true,"context_lines":[{"line_number":905,"context_line":"            # volumes are always flattened."},{"line_number":906,"context_line":"            if (volume.use_quota and"},{"line_number":907,"context_line":"                    depth \u003e\u003d self.configuration.rbd_max_clone_depth):"},{"line_number":908,"context_line":""},{"line_number":909,"context_line":"                self._flatten_volume(dest_name, client)"},{"line_number":910,"context_line":""},{"line_number":911,"context_line":"                try:"}],"source_content_type":"text/x-python","patch_set":28,"id":"f3f0297f_ac6194af","line":908,"updated":"2022-09-29 04:12:03.000000000","message":"I agree with Pete, the log.info at line 830 should really be done here.","commit_id":"09555fe086beeb39ac2e08c2bbca56bfcd8d4eac"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"03b66c24d0884a7147d9d5997121f4e36a4a24c2","unresolved":false,"context_lines":[{"line_number":905,"context_line":"            # volumes are always flattened."},{"line_number":906,"context_line":"            if (volume.use_quota and"},{"line_number":907,"context_line":"                    depth \u003e\u003d self.configuration.rbd_max_clone_depth):"},{"line_number":908,"context_line":""},{"line_number":909,"context_line":"                self._flatten_volume(dest_name, client)"},{"line_number":910,"context_line":""},{"line_number":911,"context_line":"                try:"}],"source_content_type":"text/x-python","patch_set":28,"id":"c3fbdbbb_2382c596","line":908,"in_reply_to":"f3f0297f_ac6194af","updated":"2023-02-02 10:53:52.000000000","message":"Done","commit_id":"09555fe086beeb39ac2e08c2bbca56bfcd8d4eac"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"5c242a010c24998436f072bf3e3bf9a0e5316954","unresolved":true,"context_lines":[{"line_number":1396,"context_line":"                      client_ioctx: \u0027rados.Ioctx\u0027,"},{"line_number":1397,"context_line":"                      volume_name: str,"},{"line_number":1398,"context_line":"                      delay: int) -\u003e None:"},{"line_number":1399,"context_line":"        # trash_move() will succeed in some situations when a regular"},{"line_number":1400,"context_line":"        # remove() call will fail due to image dependencies"},{"line_number":1401,"context_line":"        LOG.debug(\"moving volume %s to trash\", volume_name)"},{"line_number":1402,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":28,"id":"38e25553_8fdba5d9","line":1399,"range":{"start_line":1399,"start_character":39,"end_line":1399,"end_character":54},"updated":"2022-09-27 09:05:37.000000000","message":"do we have cases where this also fails? this seems to be the last way of removing the image if the remove/(flatten,remove) fails.","commit_id":"09555fe086beeb39ac2e08c2bbca56bfcd8d4eac"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"fc688524291c3edf5f00af487592e563c8b7b912","unresolved":false,"context_lines":[{"line_number":1396,"context_line":"                      client_ioctx: \u0027rados.Ioctx\u0027,"},{"line_number":1397,"context_line":"                      volume_name: str,"},{"line_number":1398,"context_line":"                      delay: int) -\u003e None:"},{"line_number":1399,"context_line":"        # trash_move() will succeed in some situations when a regular"},{"line_number":1400,"context_line":"        # remove() call will fail due to image dependencies"},{"line_number":1401,"context_line":"        LOG.debug(\"moving volume %s to trash\", volume_name)"},{"line_number":1402,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":28,"id":"ae04438a_5460e91d","line":1399,"range":{"start_line":1399,"start_character":39,"end_line":1399,"end_character":54},"in_reply_to":"00d0e328_34ff2cbb","updated":"2023-03-20 13:23:43.000000000","message":"Done","commit_id":"09555fe086beeb39ac2e08c2bbca56bfcd8d4eac"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"03b66c24d0884a7147d9d5997121f4e36a4a24c2","unresolved":true,"context_lines":[{"line_number":1396,"context_line":"                      client_ioctx: \u0027rados.Ioctx\u0027,"},{"line_number":1397,"context_line":"                      volume_name: str,"},{"line_number":1398,"context_line":"                      delay: int) -\u003e None:"},{"line_number":1399,"context_line":"        # trash_move() will succeed in some situations when a regular"},{"line_number":1400,"context_line":"        # remove() call will fail due to image dependencies"},{"line_number":1401,"context_line":"        LOG.debug(\"moving volume %s to trash\", volume_name)"},{"line_number":1402,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":28,"id":"00d0e328_34ff2cbb","line":1399,"range":{"start_line":1399,"start_character":39,"end_line":1399,"end_character":54},"in_reply_to":"38e25553_8fdba5d9","updated":"2023-02-02 10:53:52.000000000","message":"There is probably some case where it fails when your cluster is broken -- not sure what you would like to know exactly?","commit_id":"09555fe086beeb39ac2e08c2bbca56bfcd8d4eac"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"5c242a010c24998436f072bf3e3bf9a0e5316954","unresolved":true,"context_lines":[{"line_number":1545,"context_line":"            try:"},{"line_number":1546,"context_line":"                with RBDVolumeProxy(self, volume_name) as volume:"},{"line_number":1547,"context_line":"                    volume.unprotect_snap(snap_name)"},{"line_number":1548,"context_line":"            except self.rbd.InvalidArgument:"},{"line_number":1549,"context_line":"                LOG.info("},{"line_number":1550,"context_line":"                    \"InvalidArgument: Unable to unprotect snapshot %s.\","},{"line_number":1551,"context_line":"                    snap_name)"}],"source_content_type":"text/x-python","patch_set":28,"id":"3b25823e_5c52120f","line":1548,"range":{"start_line":1548,"start_character":28,"end_line":1548,"end_character":43},"updated":"2022-09-27 09:05:37.000000000","message":"when do we get this exception? I can only see it raises two exceptions[1] i.e. IOError, ImageNotFound\n\n[1] https://github.com/ceph/ceph/blob/f5c21acfdc16e8a519f58dccee808baf0c713c4d/src/pybind/rbd/rbd.pyx#L3634","commit_id":"09555fe086beeb39ac2e08c2bbca56bfcd8d4eac"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"fc688524291c3edf5f00af487592e563c8b7b912","unresolved":false,"context_lines":[{"line_number":1545,"context_line":"            try:"},{"line_number":1546,"context_line":"                with RBDVolumeProxy(self, volume_name) as volume:"},{"line_number":1547,"context_line":"                    volume.unprotect_snap(snap_name)"},{"line_number":1548,"context_line":"            except self.rbd.InvalidArgument:"},{"line_number":1549,"context_line":"                LOG.info("},{"line_number":1550,"context_line":"                    \"InvalidArgument: Unable to unprotect snapshot %s.\","},{"line_number":1551,"context_line":"                    snap_name)"}],"source_content_type":"text/x-python","patch_set":28,"id":"2cc00d51_110e81ee","line":1548,"range":{"start_line":1548,"start_character":28,"end_line":1548,"end_character":43},"in_reply_to":"11c16f92_b9233dee","updated":"2023-03-20 13:23:43.000000000","message":"Done","commit_id":"09555fe086beeb39ac2e08c2bbca56bfcd8d4eac"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"03b66c24d0884a7147d9d5997121f4e36a4a24c2","unresolved":true,"context_lines":[{"line_number":1545,"context_line":"            try:"},{"line_number":1546,"context_line":"                with RBDVolumeProxy(self, volume_name) as volume:"},{"line_number":1547,"context_line":"                    volume.unprotect_snap(snap_name)"},{"line_number":1548,"context_line":"            except self.rbd.InvalidArgument:"},{"line_number":1549,"context_line":"                LOG.info("},{"line_number":1550,"context_line":"                    \"InvalidArgument: Unable to unprotect snapshot %s.\","},{"line_number":1551,"context_line":"                    snap_name)"}],"source_content_type":"text/x-python","patch_set":28,"id":"11c16f92_b9233dee","line":1548,"range":{"start_line":1548,"start_character":28,"end_line":1548,"end_character":43},"in_reply_to":"3b25823e_5c52120f","updated":"2023-02-02 10:53:52.000000000","message":"-EINVAL from snap_unprotect in src/librbd/Operations.cc is translated to InvalidArgument when trying to unprotect a snapshot that wasn\u0027t protected.","commit_id":"09555fe086beeb39ac2e08c2bbca56bfcd8d4eac"},{"author":{"_account_id":30615,"name":"Tushar Trambak Gite","email":"tushargite96@gmail.com","username":"tushargite96"},"change_message_id":"5b6760f24b8f62923fba54cab106d51ab5e1aadf","unresolved":true,"context_lines":[{"line_number":924,"context_line":"                    raise exception.VolumeBackendAPIException(data\u003dmsg)"},{"line_number":925,"context_line":""},{"line_number":926,"context_line":"            try:"},{"line_number":927,"context_line":"                volume_update \u003d self._setup_volume(volume)"},{"line_number":928,"context_line":"            except Exception:"},{"line_number":929,"context_line":"                self.RBDProxy().remove(client.ioctx, dest_name)"},{"line_number":930,"context_line":"                src_volume.unprotect_snap(clone_snap)"}],"source_content_type":"text/x-python","patch_set":30,"id":"a81ddcf6_afe42ce1","line":927,"updated":"2023-03-06 14:08:11.000000000","message":"indentation","commit_id":"8a0d456f1f8ecec9cfb816c720cfa23e3ea9f9aa"},{"author":{"_account_id":30615,"name":"Tushar Trambak Gite","email":"tushargite96@gmail.com","username":"tushargite96"},"change_message_id":"12c08666044a0894b5ac86f044899185641c6bc9","unresolved":false,"context_lines":[{"line_number":924,"context_line":"                    raise exception.VolumeBackendAPIException(data\u003dmsg)"},{"line_number":925,"context_line":""},{"line_number":926,"context_line":"            try:"},{"line_number":927,"context_line":"                volume_update \u003d self._setup_volume(volume)"},{"line_number":928,"context_line":"            except Exception:"},{"line_number":929,"context_line":"                self.RBDProxy().remove(client.ioctx, dest_name)"},{"line_number":930,"context_line":"                src_volume.unprotect_snap(clone_snap)"}],"source_content_type":"text/x-python","patch_set":30,"id":"f15e201f_fa664689","line":927,"in_reply_to":"a81ddcf6_afe42ce1","updated":"2023-03-07 12:42:03.000000000","message":"this was for this line https://review.opendev.org/c/openstack/cinder/+/835384/30/cinder/tests/unit/volume/drivers/test_rbd.py#927","commit_id":"8a0d456f1f8ecec9cfb816c720cfa23e3ea9f9aa"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"d2b3379c5dc1f0cba7986423a7d614ccab8e1d72","unresolved":true,"context_lines":[{"line_number":1171,"context_line":""},{"line_number":1172,"context_line":"        @utils.limit_operations"},{"line_number":1173,"context_line":"        def do_flatten(self, volume_name: str, pool: str) -\u003e None:"},{"line_number":1174,"context_line":"            LOG.debug(\u0027flattening %s\u0027, image)"},{"line_number":1175,"context_line":"            try:"},{"line_number":1176,"context_line":"                with RBDVolumeProxy(self, volume_name) as vol:"},{"line_number":1177,"context_line":"                    try:"}],"source_content_type":"text/x-python","patch_set":30,"id":"aa9795dd_b74668b0","line":1174,"updated":"2023-02-07 04:52:16.000000000","message":"This looks kind of jumbled up wrt. the pool. Maybe pass image as argument, not pool? As it is, pool is unused and image bypasses the argument, being in scope.","commit_id":"8a0d456f1f8ecec9cfb816c720cfa23e3ea9f9aa"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"b346742be281a3f4e363a9fe02bf3333cbe15024","unresolved":true,"context_lines":[{"line_number":1171,"context_line":""},{"line_number":1172,"context_line":"        @utils.limit_operations"},{"line_number":1173,"context_line":"        def do_flatten(self, volume_name: str, pool: str) -\u003e None:"},{"line_number":1174,"context_line":"            LOG.debug(\u0027flattening %s\u0027, image)"},{"line_number":1175,"context_line":"            try:"},{"line_number":1176,"context_line":"                with RBDVolumeProxy(self, volume_name) as vol:"},{"line_number":1177,"context_line":"                    try:"}],"source_content_type":"text/x-python","patch_set":30,"id":"b5d38c78_6e8b5454","line":1174,"in_reply_to":"aa9795dd_b74668b0","updated":"2023-02-07 19:15:14.000000000","message":"Hrm, this looks like a bug.  pool was previously passed to RBDVolumeProxy, and it should be still.","commit_id":"8a0d456f1f8ecec9cfb816c720cfa23e3ea9f9aa"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"9ff8124a715a5a399ee862d4ea81bd6f146cc0f5","unresolved":false,"context_lines":[{"line_number":1171,"context_line":""},{"line_number":1172,"context_line":"        @utils.limit_operations"},{"line_number":1173,"context_line":"        def do_flatten(self, volume_name: str, pool: str) -\u003e None:"},{"line_number":1174,"context_line":"            LOG.debug(\u0027flattening %s\u0027, image)"},{"line_number":1175,"context_line":"            try:"},{"line_number":1176,"context_line":"                with RBDVolumeProxy(self, volume_name) as vol:"},{"line_number":1177,"context_line":"                    try:"}],"source_content_type":"text/x-python","patch_set":30,"id":"09b410e7_b56f471c","line":1174,"in_reply_to":"b5d38c78_6e8b5454","updated":"2023-07-12 17:47:02.000000000","message":"Done","commit_id":"8a0d456f1f8ecec9cfb816c720cfa23e3ea9f9aa"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"d2b3379c5dc1f0cba7986423a7d614ccab8e1d72","unresolved":true,"context_lines":[{"line_number":1176,"context_line":"                with RBDVolumeProxy(self, volume_name) as vol:"},{"line_number":1177,"context_line":"                    try:"},{"line_number":1178,"context_line":"                        vol.flatten()"},{"line_number":1179,"context_line":"                    except self.rbd.InvalidArgument as e:"},{"line_number":1180,"context_line":"                        LOG.debug(e)"},{"line_number":1181,"context_line":"                        raise e"},{"line_number":1182,"context_line":"                    else:"}],"source_content_type":"text/x-python","patch_set":30,"id":"68dd86a5_f6389b95","line":1179,"updated":"2023-02-07 04:52:16.000000000","message":"What does this do? Looks like perhaps a forgotten chunk of some debugging session. Do you expect a particular problem in production at customers sites that we need to track?","commit_id":"8a0d456f1f8ecec9cfb816c720cfa23e3ea9f9aa"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"b346742be281a3f4e363a9fe02bf3333cbe15024","unresolved":true,"context_lines":[{"line_number":1176,"context_line":"                with RBDVolumeProxy(self, volume_name) as vol:"},{"line_number":1177,"context_line":"                    try:"},{"line_number":1178,"context_line":"                        vol.flatten()"},{"line_number":1179,"context_line":"                    except self.rbd.InvalidArgument as e:"},{"line_number":1180,"context_line":"                        LOG.debug(e)"},{"line_number":1181,"context_line":"                        raise e"},{"line_number":1182,"context_line":"                    else:"}],"source_content_type":"text/x-python","patch_set":30,"id":"fbfdfc38_b6188859","line":1179,"in_reply_to":"68dd86a5_f6389b95","updated":"2023-02-07 19:15:14.000000000","message":"Good question, I\u0027ll remove this.","commit_id":"8a0d456f1f8ecec9cfb816c720cfa23e3ea9f9aa"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"9ff8124a715a5a399ee862d4ea81bd6f146cc0f5","unresolved":false,"context_lines":[{"line_number":1176,"context_line":"                with RBDVolumeProxy(self, volume_name) as vol:"},{"line_number":1177,"context_line":"                    try:"},{"line_number":1178,"context_line":"                        vol.flatten()"},{"line_number":1179,"context_line":"                    except self.rbd.InvalidArgument as e:"},{"line_number":1180,"context_line":"                        LOG.debug(e)"},{"line_number":1181,"context_line":"                        raise e"},{"line_number":1182,"context_line":"                    else:"}],"source_content_type":"text/x-python","patch_set":30,"id":"e740a3d4_30dcf501","line":1179,"in_reply_to":"fbfdfc38_b6188859","updated":"2023-07-12 17:47:02.000000000","message":"Done","commit_id":"8a0d456f1f8ecec9cfb816c720cfa23e3ea9f9aa"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"72e63ae9d8dea61c5a1df7b381b90601019ddf41","unresolved":true,"context_lines":[{"line_number":1569,"context_line":"                with RADOSClient(self) as client:"},{"line_number":1570,"context_line":"                    self._flatten_children(client.ioctx,"},{"line_number":1571,"context_line":"                                           volume_name,"},{"line_number":1572,"context_line":"                                           snap_name)"},{"line_number":1573,"context_line":"                raise e"},{"line_number":1574,"context_line":""},{"line_number":1575,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":32,"id":"1a4c8cb4_e064d9ee","line":1572,"updated":"2023-07-15 04:20:10.000000000","message":"I\u0027m sorry that I\u0027m not good with my Python and nested exceptions, but what does happen here if _flatten() throws something?","commit_id":"18e665cd093e34da22c9bcc59034513a00a71696"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"4f8a9806ed1a91b36724b97c00374e251e4d7cd1","unresolved":false,"context_lines":[{"line_number":1569,"context_line":"                with RADOSClient(self) as client:"},{"line_number":1570,"context_line":"                    self._flatten_children(client.ioctx,"},{"line_number":1571,"context_line":"                                           volume_name,"},{"line_number":1572,"context_line":"                                           snap_name)"},{"line_number":1573,"context_line":"                raise e"},{"line_number":1574,"context_line":""},{"line_number":1575,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":32,"id":"8541949f_406fae0b","line":1572,"in_reply_to":"1a4c8cb4_e064d9ee","updated":"2023-08-07 19:32:56.000000000","message":"If it throws ImageBusy it will be retried (line 1552), otherwise the error will be handled at line 1575.","commit_id":"18e665cd093e34da22c9bcc59034513a00a71696"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"a8d39f1486d152d0a2b7d72c1aeb40ab9fa3bc48","unresolved":true,"context_lines":[{"line_number":1,"context_line":"#    Copyright 2013 OpenStack Foundation"},{"line_number":2,"context_line":"#    Copyright 2022 Red Hat, Inc."},{"line_number":3,"context_line":"#"},{"line_number":4,"context_line":"#    Licensed under the Apache License, Version 2.0 (the \"License\"); you may"},{"line_number":5,"context_line":"#    not use this file except in compliance with the License. You may obtain"}],"source_content_type":"text/x-python","patch_set":34,"id":"d716e1ba_baf2b04b","line":2,"updated":"2023-08-16 17:51:17.000000000","message":"My, how time flies :D","commit_id":"e7b5e1b80d99af31d802013a563633e230865ca8"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"bcd4b7583e7d8e06769343116e1136a2e39828ce","unresolved":false,"context_lines":[{"line_number":1,"context_line":"#    Copyright 2013 OpenStack Foundation"},{"line_number":2,"context_line":"#    Copyright 2022 Red Hat, Inc."},{"line_number":3,"context_line":"#"},{"line_number":4,"context_line":"#    Licensed under the Apache License, Version 2.0 (the \"License\"); you may"},{"line_number":5,"context_line":"#    not use this file except in compliance with the License. You may obtain"}],"source_content_type":"text/x-python","patch_set":34,"id":"1ed42006_97f9cbd2","line":2,"in_reply_to":"d716e1ba_baf2b04b","updated":"2023-08-17 18:19:45.000000000","message":"Ack","commit_id":"e7b5e1b80d99af31d802013a563633e230865ca8"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"a8d39f1486d152d0a2b7d72c1aeb40ab9fa3bc48","unresolved":true,"context_lines":[{"line_number":837,"context_line":"                       \u0027dst_size\u0027: volume.size})"},{"line_number":838,"context_line":"            self._resize(volume)"},{"line_number":839,"context_line":""},{"line_number":840,"context_line":"    def _flatten_volume("},{"line_number":841,"context_line":"            self,"},{"line_number":842,"context_line":"            image_name: str,"},{"line_number":843,"context_line":"            client: RADOSClient) -\u003e None:"}],"source_content_type":"text/x-python","patch_set":34,"id":"9ba8a21e_a537c52f","line":840,"updated":"2023-08-16 17:51:17.000000000","message":"At first I was going to just suggest renaming this to _flatten_dest_volume(), mainly to avoid confusion with the _flatten() function at L1197. Both functions flatten a volume, and this particular variant is for flattening a cloned volume.\n\nBut that has me wondering why we need two different flatten functions. The code is quite similar, just the use cases differ.\n\n- This function flattens a cloned volume when rbd_max_clone_depth is reached.\n- _flatten() pertains to flattening volumes created from a snapshot, either because rbd_flatten_volume_from_snapshot is set, or because you\u0027re flattening children in order to be able to delete the parent volume.\n\nDo we really need two flatten functions? I also note that _flatten() has the concurrency limit, but this function doesn\u0027t seem to do that.","commit_id":"e7b5e1b80d99af31d802013a563633e230865ca8"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"a8d39f1486d152d0a2b7d72c1aeb40ab9fa3bc48","unresolved":true,"context_lines":[{"line_number":1403,"context_line":"                LOG.error(e)"},{"line_number":1404,"context_line":"                raise"},{"line_number":1405,"context_line":""},{"line_number":1406,"context_line":"    def _trash_volume(self,"},{"line_number":1407,"context_line":"                      client_ioctx: \u0027rados.Ioctx\u0027,"},{"line_number":1408,"context_line":"                      volume_name: str,"},{"line_number":1409,"context_line":"                      delay: int) -\u003e None:"}],"source_content_type":"text/x-python","patch_set":34,"id":"a0ba1038_3e582604","line":1406,"updated":"2023-08-16 17:51:17.000000000","message":"fwiw, when used as a verb, \"trash[ing]\" a volume sounds destructive. I might have gone with something like _move_volume_to_trash().","commit_id":"e7b5e1b80d99af31d802013a563633e230865ca8"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"bcd4b7583e7d8e06769343116e1136a2e39828ce","unresolved":false,"context_lines":[{"line_number":1403,"context_line":"                LOG.error(e)"},{"line_number":1404,"context_line":"                raise"},{"line_number":1405,"context_line":""},{"line_number":1406,"context_line":"    def _trash_volume(self,"},{"line_number":1407,"context_line":"                      client_ioctx: \u0027rados.Ioctx\u0027,"},{"line_number":1408,"context_line":"                      volume_name: str,"},{"line_number":1409,"context_line":"                      delay: int) -\u003e None:"}],"source_content_type":"text/x-python","patch_set":34,"id":"3d696027_31d9f7a6","line":1406,"in_reply_to":"a0ba1038_3e582604","updated":"2023-08-17 18:19:45.000000000","message":"Done","commit_id":"e7b5e1b80d99af31d802013a563633e230865ca8"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"a8d39f1486d152d0a2b7d72c1aeb40ab9fa3bc48","unresolved":true,"context_lines":[{"line_number":1457,"context_line":""},{"line_number":1458,"context_line":"        return None"},{"line_number":1459,"context_line":""},{"line_number":1460,"context_line":"    def _delete_volume(self, volume: Volume, client) -\u003e None:"},{"line_number":1461,"context_line":"        try:"},{"line_number":1462,"context_line":"            rbd_image \u003d RBDVolumeProxy(self, volume.name, ioctx\u003dclient.ioctx)"},{"line_number":1463,"context_line":"        except self.rbd.ImageNotFound:"}],"source_content_type":"text/x-python","patch_set":34,"id":"de663a8e_d13f6906","line":1460,"updated":"2023-08-16 17:51:17.000000000","message":"Can you add a mypy annotation for the client arg?","commit_id":"e7b5e1b80d99af31d802013a563633e230865ca8"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"bcd4b7583e7d8e06769343116e1136a2e39828ce","unresolved":false,"context_lines":[{"line_number":1457,"context_line":""},{"line_number":1458,"context_line":"        return None"},{"line_number":1459,"context_line":""},{"line_number":1460,"context_line":"    def _delete_volume(self, volume: Volume, client) -\u003e None:"},{"line_number":1461,"context_line":"        try:"},{"line_number":1462,"context_line":"            rbd_image \u003d RBDVolumeProxy(self, volume.name, ioctx\u003dclient.ioctx)"},{"line_number":1463,"context_line":"        except self.rbd.ImageNotFound:"}],"source_content_type":"text/x-python","patch_set":34,"id":"87185994_100d9299","line":1460,"in_reply_to":"de663a8e_d13f6906","updated":"2023-08-17 18:19:45.000000000","message":"Done","commit_id":"e7b5e1b80d99af31d802013a563633e230865ca8"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"a8d39f1486d152d0a2b7d72c1aeb40ab9fa3bc48","unresolved":true,"context_lines":[{"line_number":1493,"context_line":"        deleted \u003d False"},{"line_number":1494,"context_line":""},{"line_number":1495,"context_line":"        try:"},{"line_number":1496,"context_line":"            self.RBDProxy().remove(client.ioctx, volume.name)"},{"line_number":1497,"context_line":"            return"},{"line_number":1498,"context_line":"        except (self.rbd.ImageHasSnapshots, self.rbd.ImageBusy):"},{"line_number":1499,"context_line":"            self._flatten_children(client.ioctx, volume.name)"}],"source_content_type":"text/x-python","patch_set":34,"id":"1f1853b1_e7f47247","line":1496,"updated":"2023-08-16 17:51:17.000000000","message":"I guess this is the \"fast path\" delete. Go for it, and worry about whether there\u0027s children only if this attempt fails.","commit_id":"e7b5e1b80d99af31d802013a563633e230865ca8"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"bcd4b7583e7d8e06769343116e1136a2e39828ce","unresolved":false,"context_lines":[{"line_number":1493,"context_line":"        deleted \u003d False"},{"line_number":1494,"context_line":""},{"line_number":1495,"context_line":"        try:"},{"line_number":1496,"context_line":"            self.RBDProxy().remove(client.ioctx, volume.name)"},{"line_number":1497,"context_line":"            return"},{"line_number":1498,"context_line":"        except (self.rbd.ImageHasSnapshots, self.rbd.ImageBusy):"},{"line_number":1499,"context_line":"            self._flatten_children(client.ioctx, volume.name)"}],"source_content_type":"text/x-python","patch_set":34,"id":"f89f231c_a91e1131","line":1496,"in_reply_to":"1f1853b1_e7f47247","updated":"2023-08-17 18:19:45.000000000","message":"Ack","commit_id":"e7b5e1b80d99af31d802013a563633e230865ca8"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"a8d39f1486d152d0a2b7d72c1aeb40ab9fa3bc48","unresolved":true,"context_lines":[{"line_number":1512,"context_line":"            raise exception.VolumeIsBusy(msg, volume_name\u003dvolume.name)"},{"line_number":1513,"context_line":""},{"line_number":1514,"context_line":"        if deleted:"},{"line_number":1515,"context_line":"            return"},{"line_number":1516,"context_line":""},{"line_number":1517,"context_line":"        delay \u003d 0"},{"line_number":1518,"context_line":"        if self.configuration.enable_deferred_deletion:"}],"source_content_type":"text/x-python","patch_set":34,"id":"0e17b1e7_eedcb093","line":1515,"updated":"2023-08-16 17:51:17.000000000","message":"Maybe the \u0027deleted\u0027 variable could be eliminated entirely if L1506 was replaced with something like this:\n\n             if self._try_remove_volume(client, volume.name):\n                 return","commit_id":"e7b5e1b80d99af31d802013a563633e230865ca8"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"bcd4b7583e7d8e06769343116e1136a2e39828ce","unresolved":true,"context_lines":[{"line_number":1512,"context_line":"            raise exception.VolumeIsBusy(msg, volume_name\u003dvolume.name)"},{"line_number":1513,"context_line":""},{"line_number":1514,"context_line":"        if deleted:"},{"line_number":1515,"context_line":"            return"},{"line_number":1516,"context_line":""},{"line_number":1517,"context_line":"        delay \u003d 0"},{"line_number":1518,"context_line":"        if self.configuration.enable_deferred_deletion:"}],"source_content_type":"text/x-python","patch_set":34,"id":"82ea4718_3bc232fd","line":1515,"in_reply_to":"0e17b1e7_eedcb093","updated":"2023-08-17 18:19:45.000000000","message":"Yeah, this makes sense.","commit_id":"e7b5e1b80d99af31d802013a563633e230865ca8"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"bb5c4c125c30122be3976e5d1d4cac0ebfda3d34","unresolved":false,"context_lines":[{"line_number":1512,"context_line":"            raise exception.VolumeIsBusy(msg, volume_name\u003dvolume.name)"},{"line_number":1513,"context_line":""},{"line_number":1514,"context_line":"        if deleted:"},{"line_number":1515,"context_line":"            return"},{"line_number":1516,"context_line":""},{"line_number":1517,"context_line":"        delay \u003d 0"},{"line_number":1518,"context_line":"        if self.configuration.enable_deferred_deletion:"}],"source_content_type":"text/x-python","patch_set":34,"id":"588615a8_aa49a48f","line":1515,"in_reply_to":"82ea4718_3bc232fd","updated":"2023-11-30 23:14:27.000000000","message":"Done","commit_id":"e7b5e1b80d99af31d802013a563633e230865ca8"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"a8d39f1486d152d0a2b7d72c1aeb40ab9fa3bc48","unresolved":true,"context_lines":[{"line_number":1556,"context_line":"            try:"},{"line_number":1557,"context_line":"                with RBDVolumeProxy(self, volume_name) as volume:"},{"line_number":1558,"context_line":"                    volume.unprotect_snap(snap_name)"},{"line_number":1559,"context_line":"            except self.rbd.InvalidArgument:"},{"line_number":1560,"context_line":"                LOG.info("},{"line_number":1561,"context_line":"                    \"InvalidArgument: Unable to unprotect snapshot %s.\","},{"line_number":1562,"context_line":"                    snap_name)"}],"source_content_type":"text/x-python","patch_set":34,"id":"faa9191c_c6552910","line":1559,"updated":"2023-08-16 17:51:17.000000000","message":"I realize the original code is similar, but don\u0027t you need to re-raise the InvalidArgument exception?","commit_id":"e7b5e1b80d99af31d802013a563633e230865ca8"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"735675fb9e8c13d72bffb24bafb182226c9a07e6","unresolved":false,"context_lines":[{"line_number":1556,"context_line":"            try:"},{"line_number":1557,"context_line":"                with RBDVolumeProxy(self, volume_name) as volume:"},{"line_number":1558,"context_line":"                    volume.unprotect_snap(snap_name)"},{"line_number":1559,"context_line":"            except self.rbd.InvalidArgument:"},{"line_number":1560,"context_line":"                LOG.info("},{"line_number":1561,"context_line":"                    \"InvalidArgument: Unable to unprotect snapshot %s.\","},{"line_number":1562,"context_line":"                    snap_name)"}],"source_content_type":"text/x-python","patch_set":34,"id":"d41569c4_b8151270","line":1559,"in_reply_to":"c9e9724b_9704b6b1","updated":"2023-11-29 15:27:32.000000000","message":"Done","commit_id":"e7b5e1b80d99af31d802013a563633e230865ca8"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"bcd4b7583e7d8e06769343116e1136a2e39828ce","unresolved":true,"context_lines":[{"line_number":1556,"context_line":"            try:"},{"line_number":1557,"context_line":"                with RBDVolumeProxy(self, volume_name) as volume:"},{"line_number":1558,"context_line":"                    volume.unprotect_snap(snap_name)"},{"line_number":1559,"context_line":"            except self.rbd.InvalidArgument:"},{"line_number":1560,"context_line":"                LOG.info("},{"line_number":1561,"context_line":"                    \"InvalidArgument: Unable to unprotect snapshot %s.\","},{"line_number":1562,"context_line":"                    snap_name)"}],"source_content_type":"text/x-python","patch_set":34,"id":"c9e9724b_9704b6b1","line":1559,"in_reply_to":"faa9191c_c6552910","updated":"2023-08-17 18:19:45.000000000","message":"The idea here is that this error would occur if the snapshot was not protected -- so we can ignore it and not do anything here.  (I think this is possible if, for example, an admin manually unprotects it.)\n\nAny problem caused by the unprotect call failing in this situation would presumably cause the remove_snap call below to fail.","commit_id":"e7b5e1b80d99af31d802013a563633e230865ca8"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"a8d39f1486d152d0a2b7d72c1aeb40ab9fa3bc48","unresolved":true,"context_lines":[{"line_number":2054,"context_line":"            @utils.retry(exception.VolumeIsBusy,"},{"line_number":2055,"context_line":"                         self.configuration.rados_connection_interval,"},{"line_number":2056,"context_line":"                         self.configuration.rados_connection_retries)"},{"line_number":2057,"context_line":"            def _delete_volume(volume: Volume) -\u003e None:"},{"line_number":2058,"context_line":"                self.delete_volume(volume)"},{"line_number":2059,"context_line":""},{"line_number":2060,"context_line":"            _delete_volume(volume)"}],"source_content_type":"text/x-python","patch_set":34,"id":"f6bbca9b_cf97c382","line":2057,"updated":"2023-08-16 17:51:17.000000000","message":"Ah, this is a local override of the new function at L1460, and it\u0027s purpose seems to be that it has a retry decorator.\n\nI mention this in case it raises questions about when/where we should retry deleting a volume when it\u0027s busy.\n\nUPDATE:\n\nI see retry logic is now built into the regular delete_volume() code, via the retry decorator at L1430. This suggests you could move the redundant retry from this function. I suppose that could be done as a follow-up patch, but we\u0027d need to remember to do it.","commit_id":"e7b5e1b80d99af31d802013a563633e230865ca8"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"f5fd275b86347221a174ff64bc4b66ebfa09d113","unresolved":true,"context_lines":[{"line_number":1382,"context_line":"                          client_ioctx: \u0027rados.Ioctx\u0027,"},{"line_number":1383,"context_line":"                          volume_name: str,"},{"line_number":1384,"context_line":"                          snap_name: Optional[str] \u003d None) -\u003e None:"},{"line_number":1385,"context_line":"        rbd_image \u003d RBDVolumeProxy(self, volume_name, ioctx\u003dclient_ioctx)"},{"line_number":1386,"context_line":"        try:"},{"line_number":1387,"context_line":"            if snap_name is not None:"},{"line_number":1388,"context_line":"                rbd_image.set_snap(snap_name)"}],"source_content_type":"text/x-python","patch_set":41,"id":"73442558_f2db34b9","line":1385,"range":{"start_line":1385,"start_character":8,"end_line":1385,"end_character":35},"updated":"2023-11-28 19:14:00.000000000","message":"This may be causing problems -- __exit__ is not called, so proper cleanup of the RBD connection is not done by RBDVolumeProxy.","commit_id":"37e8a58790dd10c1bf1c168bdce6b34613c8d829"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"ec3e2aa227d9c6dcf36fe231808d06d1cdfbb792","unresolved":true,"context_lines":[{"line_number":1382,"context_line":"                          client_ioctx: \u0027rados.Ioctx\u0027,"},{"line_number":1383,"context_line":"                          volume_name: str,"},{"line_number":1384,"context_line":"                          snap_name: Optional[str] \u003d None) -\u003e None:"},{"line_number":1385,"context_line":"        rbd_image \u003d RBDVolumeProxy(self, volume_name, ioctx\u003dclient_ioctx)"},{"line_number":1386,"context_line":"        try:"},{"line_number":1387,"context_line":"            if snap_name is not None:"},{"line_number":1388,"context_line":"                rbd_image.set_snap(snap_name)"}],"source_content_type":"text/x-python","patch_set":41,"id":"f889452e_80fcc7ba","line":1385,"range":{"start_line":1385,"start_character":8,"end_line":1385,"end_character":35},"in_reply_to":"2e7fbfe1_61a24d14","updated":"2023-11-28 21:27:36.000000000","message":"close() and other operations used in the driver (like create, create_snap, protect_snap, etc.) are routed to the proxied RBD image by RBDVolumeProxy\u0027s __getattr__ method.","commit_id":"37e8a58790dd10c1bf1c168bdce6b34613c8d829"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"a99ff9e7faf89055167003da6cf31b754bfeead9","unresolved":true,"context_lines":[{"line_number":1382,"context_line":"                          client_ioctx: \u0027rados.Ioctx\u0027,"},{"line_number":1383,"context_line":"                          volume_name: str,"},{"line_number":1384,"context_line":"                          snap_name: Optional[str] \u003d None) -\u003e None:"},{"line_number":1385,"context_line":"        rbd_image \u003d RBDVolumeProxy(self, volume_name, ioctx\u003dclient_ioctx)"},{"line_number":1386,"context_line":"        try:"},{"line_number":1387,"context_line":"            if snap_name is not None:"},{"line_number":1388,"context_line":"                rbd_image.set_snap(snap_name)"}],"source_content_type":"text/x-python","patch_set":41,"id":"2e7fbfe1_61a24d14","line":1385,"range":{"start_line":1385,"start_character":8,"end_line":1385,"end_character":35},"in_reply_to":"73442558_f2db34b9","updated":"2023-11-28 19:28:34.000000000","message":"Wait a moment, there\u0027s no close() method at all in RBDVolumeProxy. Is what happens that finally: always tracebacks, but nobody knows?","commit_id":"37e8a58790dd10c1bf1c168bdce6b34613c8d829"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"735675fb9e8c13d72bffb24bafb182226c9a07e6","unresolved":false,"context_lines":[{"line_number":1382,"context_line":"                          client_ioctx: \u0027rados.Ioctx\u0027,"},{"line_number":1383,"context_line":"                          volume_name: str,"},{"line_number":1384,"context_line":"                          snap_name: Optional[str] \u003d None) -\u003e None:"},{"line_number":1385,"context_line":"        rbd_image \u003d RBDVolumeProxy(self, volume_name, ioctx\u003dclient_ioctx)"},{"line_number":1386,"context_line":"        try:"},{"line_number":1387,"context_line":"            if snap_name is not None:"},{"line_number":1388,"context_line":"                rbd_image.set_snap(snap_name)"}],"source_content_type":"text/x-python","patch_set":41,"id":"2c1011e3_79c6a44f","line":1385,"range":{"start_line":1385,"start_character":8,"end_line":1385,"end_character":35},"in_reply_to":"f889452e_80fcc7ba","updated":"2023-11-29 15:27:32.000000000","message":"Done","commit_id":"37e8a58790dd10c1bf1c168bdce6b34613c8d829"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"f5fd275b86347221a174ff64bc4b66ebfa09d113","unresolved":true,"context_lines":[{"line_number":1460,"context_line":""},{"line_number":1461,"context_line":"    def _delete_volume(self, volume: Volume, client: RADOSClient) -\u003e None:"},{"line_number":1462,"context_line":"        try:"},{"line_number":1463,"context_line":"            rbd_image \u003d RBDVolumeProxy(self, volume.name, ioctx\u003dclient.ioctx)"},{"line_number":1464,"context_line":"        except self.rbd.ImageNotFound:"},{"line_number":1465,"context_line":"            LOG.info(\"volume %s no longer exists in backend\", volume.name)"},{"line_number":1466,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":41,"id":"bd830cf2_364979ac","line":1463,"range":{"start_line":1463,"start_character":12,"end_line":1463,"end_character":38},"updated":"2023-11-28 19:14:00.000000000","message":"Same issue as line 1385.","commit_id":"37e8a58790dd10c1bf1c168bdce6b34613c8d829"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"735675fb9e8c13d72bffb24bafb182226c9a07e6","unresolved":false,"context_lines":[{"line_number":1460,"context_line":""},{"line_number":1461,"context_line":"    def _delete_volume(self, volume: Volume, client: RADOSClient) -\u003e None:"},{"line_number":1462,"context_line":"        try:"},{"line_number":1463,"context_line":"            rbd_image \u003d RBDVolumeProxy(self, volume.name, ioctx\u003dclient.ioctx)"},{"line_number":1464,"context_line":"        except self.rbd.ImageNotFound:"},{"line_number":1465,"context_line":"            LOG.info(\"volume %s no longer exists in backend\", volume.name)"},{"line_number":1466,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":41,"id":"7b9fa3d8_da8f6c82","line":1463,"range":{"start_line":1463,"start_character":12,"end_line":1463,"end_character":38},"in_reply_to":"bd830cf2_364979ac","updated":"2023-11-29 15:27:32.000000000","message":"Done","commit_id":"37e8a58790dd10c1bf1c168bdce6b34613c8d829"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"22a5529bc637a4ee98eb55e47b44c2404ca9e322","unresolved":true,"context_lines":[{"line_number":1392,"context_line":"                children_list \u003d rbd_image.list_children()"},{"line_number":1393,"context_line":"            finally:"},{"line_number":1394,"context_line":"                rbd_image.close()"},{"line_number":1395,"context_line":""},{"line_number":1396,"context_line":"        for (pool, child_name) in children_list:"},{"line_number":1397,"context_line":"            LOG.info(\u0027Image %(pool)s/%(image)s%(snap)s is dependent \u0027"},{"line_number":1398,"context_line":"                     \u0027on the image %(volume_name)s.\u0027,"}],"source_content_type":"text/x-python","patch_set":42,"id":"36d9e071_22237b04","line":1395,"updated":"2023-11-29 16:25:21.000000000","message":"What in the world is this double-dipping? The context manager exists to avoid finally: in the first place! The __exit__ should be doing what your finally: clause does here.","commit_id":"94ef4accc4f9dc93ad3cb62d9a366145eff2c292"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"479f63d42286dc0a5a12f909add2ed38c033f07d","unresolved":false,"context_lines":[{"line_number":1392,"context_line":"                children_list \u003d rbd_image.list_children()"},{"line_number":1393,"context_line":"            finally:"},{"line_number":1394,"context_line":"                rbd_image.close()"},{"line_number":1395,"context_line":""},{"line_number":1396,"context_line":"        for (pool, child_name) in children_list:"},{"line_number":1397,"context_line":"            LOG.info(\u0027Image %(pool)s/%(image)s%(snap)s is dependent \u0027"},{"line_number":1398,"context_line":"                     \u0027on the image %(volume_name)s.\u0027,"}],"source_content_type":"text/x-python","patch_set":42,"id":"c9920a12_33f05996","line":1395,"in_reply_to":"36d9e071_22237b04","updated":"2023-11-29 18:23:41.000000000","message":"Yes, this is not needed here.","commit_id":"94ef4accc4f9dc93ad3cb62d9a366145eff2c292"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"bb5c4c125c30122be3976e5d1d4cac0ebfda3d34","unresolved":true,"context_lines":[{"line_number":134,"context_line":"    cfg.IntOpt(\u0027deferred_deletion_purge_interval\u0027, default\u003d60,"},{"line_number":135,"context_line":"               help\u003d\u0027Number of seconds between runs of the periodic task \u0027"},{"line_number":136,"context_line":"                    \u0027to purge volumes tagged for deletion.\u0027),"},{"line_number":137,"context_line":"    cfg.IntOpt(\u0027rbd_concurrent_flatten_operations\u0027, default\u003d3, min\u003d0,"},{"line_number":138,"context_line":"               help\u003d\u0027Number of flatten operations that will run \u0027"},{"line_number":139,"context_line":"                    \u0027concurrently on this volume service.\u0027)"},{"line_number":140,"context_line":"]"}],"source_content_type":"text/x-python","patch_set":43,"id":"60dd4a8c_54f0017d","line":137,"range":{"start_line":137,"start_character":16,"end_line":137,"end_character":49},"updated":"2023-11-30 23:14:27.000000000","message":"I should\u0027ve noticed this a long time ago, but this new option should be called out in the release note so operators know it exists and what the default is.  Looks like we usually use the \"features\" section for this, something like this note: add-backup-swift-container-storage-policy-8d4a268ed61b9fe2.yaml .  I\u0027d say it could be a followup, except that we\u0027ll want to backport this patch as far as it can reasonably go, so it would be better on this patch.","commit_id":"77f8e2cc9584108e7b9cdac6f99d5b22103692d6"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"d99530956604e740b15e229baa1b0a08b59a237e","unresolved":false,"context_lines":[{"line_number":134,"context_line":"    cfg.IntOpt(\u0027deferred_deletion_purge_interval\u0027, default\u003d60,"},{"line_number":135,"context_line":"               help\u003d\u0027Number of seconds between runs of the periodic task \u0027"},{"line_number":136,"context_line":"                    \u0027to purge volumes tagged for deletion.\u0027),"},{"line_number":137,"context_line":"    cfg.IntOpt(\u0027rbd_concurrent_flatten_operations\u0027, default\u003d3, min\u003d0,"},{"line_number":138,"context_line":"               help\u003d\u0027Number of flatten operations that will run \u0027"},{"line_number":139,"context_line":"                    \u0027concurrently on this volume service.\u0027)"},{"line_number":140,"context_line":"]"}],"source_content_type":"text/x-python","patch_set":43,"id":"0cb34287_f0348628","line":137,"range":{"start_line":137,"start_character":16,"end_line":137,"end_character":49},"in_reply_to":"60dd4a8c_54f0017d","updated":"2023-12-14 13:54:52.000000000","message":"Done","commit_id":"77f8e2cc9584108e7b9cdac6f99d5b22103692d6"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"3911b32175235b8db0a3db2685efa8a334f62606","unresolved":true,"context_lines":[{"line_number":1432,"context_line":"                     self.configuration.rados_connection_retries)"},{"line_number":1433,"context_line":"        def _do_try_remove_volume(self, client, volume_name: str) -\u003e bool:"},{"line_number":1434,"context_line":"            try:"},{"line_number":1435,"context_line":"                LOG.debug(\u0027Trying to remove image %s\u0027, volume_name)"},{"line_number":1436,"context_line":"                self.RBDProxy().remove(client.ioctx, volume_name)"},{"line_number":1437,"context_line":"                return True"},{"line_number":1438,"context_line":"            except (self.rbd.ImageHasSnapshots, self.rbd.ImageBusy):"}],"source_content_type":"text/x-python","patch_set":44,"id":"d4971b25_55fd3d2c","line":1435,"range":{"start_line":1435,"start_character":44,"end_line":1435,"end_character":49},"updated":"2023-12-05 09:34:54.000000000","message":"this can be confusing to the operator since RBD treats both volume and snapshots as images so we won\u0027t know what we are trying to delete with this log message unless looking at the volume and snap name which are different","commit_id":"1a675c9aa178c6d9c6ed10fd98f086c46d350d3f"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"3911b32175235b8db0a3db2685efa8a334f62606","unresolved":true,"context_lines":[{"line_number":1437,"context_line":"                return True"},{"line_number":1438,"context_line":"            except (self.rbd.ImageHasSnapshots, self.rbd.ImageBusy):"},{"line_number":1439,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":1440,"context_line":"                    msg \u003d _(\u0027deletion failed\u0027)"},{"line_number":1441,"context_line":"                    LOG.info(msg)"},{"line_number":1442,"context_line":""},{"line_number":1443,"context_line":"            return False"}],"source_content_type":"text/x-python","patch_set":44,"id":"2831caff_53eaa8bc","line":1440,"range":{"start_line":1440,"start_character":29,"end_line":1440,"end_character":44},"updated":"2023-12-05 09:34:54.000000000","message":"should we instead say, \"Volume deletion failed\" to differentiate that it\u0027s a volume we are deleting and not a snapshot","commit_id":"1a675c9aa178c6d9c6ed10fd98f086c46d350d3f"}],"releasenotes/notes/rbd-flatten-child-volumes-4cb0b7fcf3a1df5e.yaml":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"bee631c13cdca23c3ddbdbd4b0629ee8c543c8b0","unresolved":true,"context_lines":[{"line_number":5,"context_line":"    Therefore, deployments must either a) enable scheduled RBD trash purging on"},{"line_number":6,"context_line":"    the RBD backend or b) enable the Cinder RBD driver\u0027s enable_deferred_deletion"},{"line_number":7,"context_line":"    option to have Cinder purge the RBD trash."},{"line_number":8,"context_line":"    This adds the new configuration option \u0027rbd_concurrent_flatten_operations\u0027,"},{"line_number":9,"context_line":"    which limits how many RBD flattens the driver will run simultaneously."},{"line_number":10,"context_line":"    This can be used to prevent flatten operations from consuming too much I/O"},{"line_number":11,"context_line":"    capacity on the Ceph cluster.  It defaults to 3."}],"source_content_type":"text/x-yaml","patch_set":44,"id":"3b143f48_6a3d3fd0","line":8,"updated":"2023-12-14 13:53:33.000000000","message":"nit: without a blank line between lines 7 \u0026 8, this will be rendered as a single paragraph","commit_id":"1a675c9aa178c6d9c6ed10fd98f086c46d350d3f"}]}