)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":20865,"name":"Gökhan","email":"gokhan.isik@tubitak.gov.tr","username":"gokhan.isik"},"change_message_id":"2b249d01cd9b39375958b058ff57b7d49099dd46","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"4a205b7a_10c439aa","updated":"2024-04-22 06:06:38.000000000","message":"Thanks Rajat, when it is ready for review, I can test immediately.","commit_id":"b331ef848eb0ab60315f79275c15255d6e9f6ad4"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"10407ecbef08c21ce8804de76ce2e3d40beaffad","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"c27d506e_2e51c759","in_reply_to":"4a205b7a_10c439aa","updated":"2024-04-22 10:07:29.000000000","message":"Hi Gokhan,\nI\u0027ve tested this along with the nova patch[1] and it works.\nThe reason I\u0027ve kept it as WIP is that the approach I\u0027ve taken might need some discussion to figure out that it works in all the cases.\n\n[1] https://review.opendev.org/c/openstack/nova/+/916409","commit_id":"b331ef848eb0ab60315f79275c15255d6e9f6ad4"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"30ad534d58d84e87ee1eeee2a6093b401526cc1e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"af40bc06_f7d286fc","updated":"2024-12-13 14:30:53.000000000","message":"But you know, when it\u0027s a \"reimage\" volume from a snapshot, and volume id \u003d snapshot_volume.id, it must mean that creating a temporary volume from a snapshot -\u003e copying to a volume must be the same as driver.snapshot. The result is simply the same, except that it could be significantly faster.","commit_id":"2df867cd7198156d1efcaa5c2efdc2ff5abf3628"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"1fd79edc00b2f773feaed1f473a3761a3a7db3e5","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"12b090e1_0c72314d","updated":"2024-12-13 11:39:46.000000000","message":"Thanks Gokhan and Michal","commit_id":"2df867cd7198156d1efcaa5c2efdc2ff5abf3628"},{"author":{"_account_id":20865,"name":"Gökhan","email":"gokhan.isik@tubitak.gov.tr","username":"gokhan.isik"},"change_message_id":"cacedf7360ef76b469f2dd8aead785ef22c75c7c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"85f5cca8_4b05094e","updated":"2024-12-11 12:57:11.000000000","message":"Thanks Michal for testing. I think we need to merge this commit. Michal, there is also a patch on nova side: https://review.opendev.org/c/openstack/nova/+/916409 can you test this with nova patch ?","commit_id":"2df867cd7198156d1efcaa5c2efdc2ff5abf3628"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"4bbe2211ac4fbfd2fdda38505f7c6fd7bfe47690","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"011629dd_433100dc","updated":"2024-12-11 11:24:40.000000000","message":"This is tested and working with small change in nova code which i will send and link it here ...\n\nIssue in nova was that nova is calculating timeout from image size which is 0 * reimage_timeout_per_gb \u003d 20 \u003d 0 ..so it timeouted in 0 seconds.\n\nNova patch: \n\n\n```\n--- /var/lib/kolla/venv/lib/python3/site-packages/nova/compute/manager.py\n+++ /var/lib/kolla/venv/lib/python3/site-packages/nova/compute/manager.py\n@@ -3627,6 +3627,12 @@\n                 instance_uuid\u003dinstance.uuid, reason\u003dmsg)\n         image_size \u003d int(math.ceil(float(image.get(\u0027size\u0027)) / units.Gi))\n         deadline \u003d CONF.reimage_timeout_per_gb * image_size\n+        if image_size:\n+            deadline \u003d CONF.reimage_timeout_per_gb * image_size\n+        else:\n+            deadline \u003d CONF.reimage_timeout_per_gb * root_bdm.volume_size\n+            LOG.debug(\u0027Image size is zero - using root disk size\u0027\n+                      \u0027 to calculate deadline for reimage event\u0027)\n         error_cb \u003d self._reimage_failed_callback\n \n         # Call cinder to perform reimage operation and wait until an\n```","commit_id":"2df867cd7198156d1efcaa5c2efdc2ff5abf3628"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"1fd79edc00b2f773feaed1f473a3761a3a7db3e5","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"320f0a05_f9368421","in_reply_to":"46c55a6b_47b5ae5e","updated":"2024-12-13 11:39:46.000000000","message":"Thanks for testing both the patches! I will update them to be in review-able state.","commit_id":"2df867cd7198156d1efcaa5c2efdc2ff5abf3628"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"474d7fe280bf3c8a7416f1c86fce6570c010c8ea","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"46c55a6b_47b5ae5e","in_reply_to":"85f5cca8_4b05094e","updated":"2024-12-11 13:40:22.000000000","message":"Working, tested on stable/2024.1 and cinder,nova patched.","commit_id":"2df867cd7198156d1efcaa5c2efdc2ff5abf3628"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"0dec3fb4e58fd3b9691e64c9a5185553d9a320e3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"1ebdcdea_86d76234","updated":"2024-12-15 21:31:44.000000000","message":"Thanks for the reviews and testing, I\u0027ve updated the nova and cinder patch with reno and proper UTs.","commit_id":"937467ddf4aedfefdaf806ee60bfff923c244183"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"12e285f7a02ed3e883521514d55c2f4035735610","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"3c944eea_251b9596","updated":"2024-12-17 05:10:08.000000000","message":"recheck POST_FAILURE in devstack-plugin-nfs-tempest-full","commit_id":"bcb28e671fb17ce91edccd2def4496a42cc2d38c"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"190d7507035430e7dec5f12beb1a260bd48d4994","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"fdcac8fe_a5971e63","updated":"2025-03-12 19:52:57.000000000","message":"Code and tests LGTM except for the issue noted in volume/api.py","commit_id":"4f2518044454ba4d44d526aabffcb85d418f9a33"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"dad5f584ceabb2aaf6fc421cf925f31b9f9706df","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"57a4cf82_ea1ae4d8","updated":"2025-03-12 19:55:33.000000000","message":"Forgot to mention that I agree with Rajat that optimizations can be done in a follow up, especially since we are right at RC-time.","commit_id":"4f2518044454ba4d44d526aabffcb85d418f9a33"},{"author":{"_account_id":20865,"name":"Gökhan","email":"gokhan.isik@tubitak.gov.tr","username":"gokhan.isik"},"change_message_id":"70abc6d2642a3b60a90fe3dbe162e6a835cc8da1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"4fbd06a7_4d06e403","updated":"2025-03-10 05:59:03.000000000","message":"can we merge please if it is ok ?","commit_id":"4f2518044454ba4d44d526aabffcb85d418f9a33"},{"author":{"_account_id":20865,"name":"Gökhan","email":"gokhan.isik@tubitak.gov.tr","username":"gokhan.isik"},"change_message_id":"86b416181215a8c662d4378b4c8e9c7404b22c4a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"35d4736a_a1575734","updated":"2024-12-18 13:02:23.000000000","message":"recheck","commit_id":"4f2518044454ba4d44d526aabffcb85d418f9a33"},{"author":{"_account_id":20865,"name":"Gökhan","email":"gokhan.isik@tubitak.gov.tr","username":"gokhan.isik"},"change_message_id":"a60c28b0cdf741bf16d50c2aa1b5fb5b38108440","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"17065f4c_b95d9629","updated":"2024-12-23 05:26:27.000000000","message":"recheck devstack-plugin-nfs-tempest-full post failure","commit_id":"4f2518044454ba4d44d526aabffcb85d418f9a33"},{"author":{"_account_id":20865,"name":"Gökhan","email":"gokhan.isik@tubitak.gov.tr","username":"gokhan.isik"},"change_message_id":"7931658687ad8e87392b1891102a958119458775","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"72f18edd_6dc65dec","updated":"2024-12-19 17:13:45.000000000","message":"recheck tempest-integrated-storage-ubuntu-jammy job timed out","commit_id":"4f2518044454ba4d44d526aabffcb85d418f9a33"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"f18bce3a2a622791f576957042263e2b5b47694b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"60423a55_cee53289","in_reply_to":"4fbd06a7_4d06e403","updated":"2025-03-10 08:33:58.000000000","message":"Thanks for bringing this up, I\u0027ve added it to the review requests section of this week\u0027s meeting agenda, will try to ask for reviews there.\n\nhttps://etherpad.opendev.org/p/cinder-epoxy-meetings#L73","commit_id":"4f2518044454ba4d44d526aabffcb85d418f9a33"},{"author":{"_account_id":20865,"name":"Gökhan","email":"gokhan.isik@tubitak.gov.tr","username":"gokhan.isik"},"change_message_id":"e768cc49ef303d8bfbd2bb3c2266d501b5b1ea9f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"17049122_9290d290","in_reply_to":"60423a55_cee53289","updated":"2025-03-11 05:13:42.000000000","message":"thanks Rajat 😊","commit_id":"4f2518044454ba4d44d526aabffcb85d418f9a33"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"8f56c7e4ae4e134d05e456dde3b1814add643bb3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"6d20c055_c11844db","updated":"2025-03-19 12:59:41.000000000","message":"Thanks Brian for the review!\nUpdated the code and tests based on suggestions.","commit_id":"b93812de4d095bebfad20b5a3203fa387fa29146"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"d8c17d665956270a76b2b38789b928bd7b27cb98","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"c92622ec_8bf4952a","updated":"2025-03-19 13:48:54.000000000","message":"Two nits noted inline, but otherwise, the revision LGTM.","commit_id":"b93812de4d095bebfad20b5a3203fa387fa29146"},{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"3c2f3e07337cc6091baa06e23fe3fa6984468575","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"0a71c468_69e8c509","updated":"2025-03-26 14:50:27.000000000","message":"Code looks good to me.","commit_id":"ae22195df7d674ba017f5e301868522ccb21c5b4"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"021171164354a08e617cb80849908d6e9a05999b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"6f8971ff_ef4350b2","updated":"2025-03-19 14:15:53.000000000","message":"Revisions LGTM.","commit_id":"ae22195df7d674ba017f5e301868522ccb21c5b4"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"d4f0252fb0dada26fc25d565cf2fc6429ceb66ef","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"23f8f31b_d7292ec9","updated":"2025-03-19 14:05:10.000000000","message":"Thanks again!","commit_id":"ae22195df7d674ba017f5e301868522ccb21c5b4"},{"author":{"_account_id":20865,"name":"Gökhan","email":"gokhan.isik@tubitak.gov.tr","username":"gokhan.isik"},"change_message_id":"389ae09ef47ba8e3cf2d92025f026a4483808e1c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"931d44b9_56fcd262","updated":"2025-03-20 09:44:51.000000000","message":"Thanks.","commit_id":"ae22195df7d674ba017f5e301868522ccb21c5b4"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"c204ff1c40c28aa48fd102e51e172eeea7d1b1c8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"330e0c9a_9f2fe0e2","updated":"2025-03-26 21:18:20.000000000","message":"recheck cinder-plugin-ceph-tempest - oom reaper killed mysql","commit_id":"ae22195df7d674ba017f5e301868522ccb21c5b4"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"ad17a8a4ea5fd82114e3cd57d141452cb5919293","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"fa1282cd_6660dc31","updated":"2025-03-27 01:18:52.000000000","message":"recheck devstack-plugin-nfs-tempest-full - post_failure, 3 failures in tempest.serial_tests.scenario.test_aggregates_basic_ops.TestAggregatesBasicOps","commit_id":"ae22195df7d674ba017f5e301868522ccb21c5b4"}],"cinder/api/contrib/volume_actions.py":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"190d7507035430e7dec5f12beb1a260bd48d4994","unresolved":true,"context_lines":[{"line_number":363,"context_line":"                            ) % boot_bdm[0].get(\u0027snapshot_id\u0027)"},{"line_number":364,"context_line":"                            raise webob.exc.HTTPNotFound("},{"line_number":365,"context_line":"                                explanation\u003dexplanation)"},{"line_number":366,"context_line":"            return image_snapshot"},{"line_number":367,"context_line":""},{"line_number":368,"context_line":"    @wsgi.Controller.api_version(mv.SUPPORT_REIMAGE_VOLUME)"},{"line_number":369,"context_line":"    @wsgi.response(HTTPStatus.ACCEPTED)"}],"source_content_type":"text/x-python","patch_set":6,"id":"b5407464_d57d8aab","line":366,"range":{"start_line":366,"start_character":12,"end_line":366,"end_character":33},"updated":"2025-03-12 19:52:57.000000000","message":"nit: this indentation is slightly odd ... if line 334 evalulates false, we won\u0027t hit this return, but will just use the no-explicit-return-returns-None feature of python.  If the other ifs evaluate false, they\u0027ll use this return, but if you make it to line 356 and don\u0027t raise, you\u0027ll use the return at line 358 instead.  Nothing wrong here, it just looks a bit weird.","commit_id":"4f2518044454ba4d44d526aabffcb85d418f9a33"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"8f56c7e4ae4e134d05e456dde3b1814add643bb3","unresolved":false,"context_lines":[{"line_number":363,"context_line":"                            ) % boot_bdm[0].get(\u0027snapshot_id\u0027)"},{"line_number":364,"context_line":"                            raise webob.exc.HTTPNotFound("},{"line_number":365,"context_line":"                                explanation\u003dexplanation)"},{"line_number":366,"context_line":"            return image_snapshot"},{"line_number":367,"context_line":""},{"line_number":368,"context_line":"    @wsgi.Controller.api_version(mv.SUPPORT_REIMAGE_VOLUME)"},{"line_number":369,"context_line":"    @wsgi.response(HTTPStatus.ACCEPTED)"}],"source_content_type":"text/x-python","patch_set":6,"id":"20d6ea44_5bd7ddcb","line":366,"range":{"start_line":366,"start_character":12,"end_line":366,"end_character":33},"in_reply_to":"b5407464_d57d8aab","updated":"2025-03-19 12:59:41.000000000","message":"Done","commit_id":"4f2518044454ba4d44d526aabffcb85d418f9a33"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"d8c17d665956270a76b2b38789b928bd7b27cb98","unresolved":true,"context_lines":[{"line_number":355,"context_line":"                                raise webob.exc.HTTPBadRequest(explanation\u003dmsg)"},{"line_number":356,"context_line":"                            image_snapshot \u003d self.volume_api.get_snapshot("},{"line_number":357,"context_line":"                                context, boot_bdm[0].get(\u0027snapshot_id\u0027))"},{"line_number":358,"context_line":"                            return image_snapshot"},{"line_number":359,"context_line":"                        except exception.NotFound:"},{"line_number":360,"context_line":"                            explanation \u003d _("},{"line_number":361,"context_line":"                                \u0027Nova specific image is found, but boot \u0027"}],"source_content_type":"text/x-python","patch_set":7,"id":"83f36aca_f38b7e01","line":358,"range":{"start_line":358,"start_character":27,"end_line":358,"end_character":49},"updated":"2025-03-19 13:48:54.000000000","message":"nit: don\u0027t really need this, since if there\u0027s no exception raised, line 366 will be the next line executed","commit_id":"b93812de4d095bebfad20b5a3203fa387fa29146"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"d4f0252fb0dada26fc25d565cf2fc6429ceb66ef","unresolved":false,"context_lines":[{"line_number":355,"context_line":"                                raise webob.exc.HTTPBadRequest(explanation\u003dmsg)"},{"line_number":356,"context_line":"                            image_snapshot \u003d self.volume_api.get_snapshot("},{"line_number":357,"context_line":"                                context, boot_bdm[0].get(\u0027snapshot_id\u0027))"},{"line_number":358,"context_line":"                            return image_snapshot"},{"line_number":359,"context_line":"                        except exception.NotFound:"},{"line_number":360,"context_line":"                            explanation \u003d _("},{"line_number":361,"context_line":"                                \u0027Nova specific image is found, but boot \u0027"}],"source_content_type":"text/x-python","patch_set":7,"id":"376c6ee8_c63e27b9","line":358,"range":{"start_line":358,"start_character":27,"end_line":358,"end_character":49},"in_reply_to":"83f36aca_f38b7e01","updated":"2025-03-19 14:05:10.000000000","message":"Done","commit_id":"b93812de4d095bebfad20b5a3203fa387fa29146"}],"cinder/volume/api.py":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"190d7507035430e7dec5f12beb1a260bd48d4994","unresolved":true,"context_lines":[{"line_number":2719,"context_line":"        image_meta \u003d self.image_service.show(context, image_id)"},{"line_number":2720,"context_line":"        if image_snap is None:"},{"line_number":2721,"context_line":"            try:"},{"line_number":2722,"context_line":"                volume_utils.check_image_metadata(image_meta, volume[\u0027size\u0027])"},{"line_number":2723,"context_line":"            # Currently we only raise InvalidInput and ImageUnacceptable"},{"line_number":2724,"context_line":"            # exceptions in the check_image_metadata call but having Exception"},{"line_number":2725,"context_line":"            # here makes it more generic since we want to roll back to original"}],"source_content_type":"text/x-python","patch_set":6,"id":"0fcfafe8_b0f80a3f","line":2722,"range":{"start_line":2722,"start_character":29,"end_line":2722,"end_character":49},"updated":"2025-03-12 19:52:57.000000000","message":"we\u0027re skipping this check if we have an image_snap ... you already checked the size back in volume_actions.py, and i guess we don\u0027t need to worry about min_disk because the image was already used to create the volume originally (though I wonder if you should do the min_disk check in volume_actions.py), but we should check somewhere that the image_snap is in \u0027active\u0027 status [0].  (Glance has a \u0027deactivate\u0027 action that an operator can use when investigating a problematic image; if an image is under suspicion for containing malware or something, then a savvy operator will probably also deactivate any snapshots created from that image, so we shouldn\u0027t consume it to re-image a volume.)\n\n[0] https://github.com/openstack/cinder/blob/2f90a5d345cf30fe438ea8421ad4a479dd40ab85/cinder/volume/volume_utils.py#L1103","commit_id":"4f2518044454ba4d44d526aabffcb85d418f9a33"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"8f56c7e4ae4e134d05e456dde3b1814add643bb3","unresolved":false,"context_lines":[{"line_number":2719,"context_line":"        image_meta \u003d self.image_service.show(context, image_id)"},{"line_number":2720,"context_line":"        if image_snap is None:"},{"line_number":2721,"context_line":"            try:"},{"line_number":2722,"context_line":"                volume_utils.check_image_metadata(image_meta, volume[\u0027size\u0027])"},{"line_number":2723,"context_line":"            # Currently we only raise InvalidInput and ImageUnacceptable"},{"line_number":2724,"context_line":"            # exceptions in the check_image_metadata call but having Exception"},{"line_number":2725,"context_line":"            # here makes it more generic since we want to roll back to original"}],"source_content_type":"text/x-python","patch_set":6,"id":"6223aa8f_840b413e","line":2722,"range":{"start_line":2722,"start_character":29,"end_line":2722,"end_character":49},"in_reply_to":"0fcfafe8_b0f80a3f","updated":"2025-03-19 12:59:41.000000000","message":"Thanks for catching this!\nI was trying to remember why i was skipping the metadata check and the only thing came to mind was, we already check the size in volume_actions but as you rightly pointed out, there are also other important checks like min_disk and image status.\nI\u0027ve added the metadata check again and tested the code against your concern (of the image being in \u0027deactivated\u0027 state) and now the metadata check catches it correctly and fails now (otherwise it succeeds skipping the size and virtual_size check).\nAlso added a comment to clarify which checks will be done if the image is backed by a volume snapshot.\n\n\n// Image stautus\u003ddeactivated\n| d5c6f56c-6137-4e94-9956-fd7786b96157 | test-vm                     | deactivated |\n\n// Try the reimage now (fails at image metadata check)\n\nMar 19 18:09:59 Ubuntu devstack@c-api.service[557638]: ERROR cinder.volume.api cinder.exception.InvalidInput: Invalid input received: Image d5c6f56c-6137-4e94-9956-fd7786b96157 is not active.","commit_id":"4f2518044454ba4d44d526aabffcb85d418f9a33"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"d8c17d665956270a76b2b38789b928bd7b27cb98","unresolved":true,"context_lines":[{"line_number":2719,"context_line":"        image_meta \u003d self.image_service.show(context, image_id)"},{"line_number":2720,"context_line":"        try:"},{"line_number":2721,"context_line":"            # If the source of the image is a volume snapshot"},{"line_number":2722,"context_line":"            # (image_snap is None), we will get image \u0027size\u0027 as 0 and"},{"line_number":2723,"context_line":"            # \u0027virtual_size\u0027 as None but at least we will verify the image"},{"line_number":2724,"context_line":"            # \u0027status\u0027 and \u0027min_disk\u0027 properties."},{"line_number":2725,"context_line":"            volume_utils.check_image_metadata(image_meta, volume[\u0027size\u0027])"}],"source_content_type":"text/x-python","patch_set":7,"id":"647f641a_f96c9c07","line":2722,"range":{"start_line":2722,"start_character":15,"end_line":2722,"end_character":33},"updated":"2025-03-19 13:48:54.000000000","message":"nit: don\u0027t you mean image_snap is *not* None ?","commit_id":"b93812de4d095bebfad20b5a3203fa387fa29146"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"d4f0252fb0dada26fc25d565cf2fc6429ceb66ef","unresolved":false,"context_lines":[{"line_number":2719,"context_line":"        image_meta \u003d self.image_service.show(context, image_id)"},{"line_number":2720,"context_line":"        try:"},{"line_number":2721,"context_line":"            # If the source of the image is a volume snapshot"},{"line_number":2722,"context_line":"            # (image_snap is None), we will get image \u0027size\u0027 as 0 and"},{"line_number":2723,"context_line":"            # \u0027virtual_size\u0027 as None but at least we will verify the image"},{"line_number":2724,"context_line":"            # \u0027status\u0027 and \u0027min_disk\u0027 properties."},{"line_number":2725,"context_line":"            volume_utils.check_image_metadata(image_meta, volume[\u0027size\u0027])"}],"source_content_type":"text/x-python","patch_set":7,"id":"5d9cbcec_770ca910","line":2722,"range":{"start_line":2722,"start_character":15,"end_line":2722,"end_character":33},"in_reply_to":"647f641a_f96c9c07","updated":"2025-03-19 14:05:10.000000000","message":"Done","commit_id":"b93812de4d095bebfad20b5a3203fa387fa29146"}],"cinder/volume/manager.py":[{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"366ec1070ffd16aceccbab2619861e1f35b7451c","unresolved":true,"context_lines":[{"line_number":5356,"context_line":"                # snapshot and do a copy so we are safe here."},{"line_number":5357,"context_line":"                # Size checks are already done on the API layer so we don\u0027t"},{"line_number":5358,"context_line":"                # need to worry about the image fitting into the volume."},{"line_number":5359,"context_line":"                self._revert_to_snapshot_generic(context, volume, image_snap)"},{"line_number":5360,"context_line":"            else:"},{"line_number":5361,"context_line":"                image_id \u003d image_meta[\u0027id\u0027]"},{"line_number":5362,"context_line":"                image_service, _ \u003d glance.get_remote_image_service("}],"source_content_type":"text/x-python","patch_set":3,"id":"087b9f59_be7957ed","line":5359,"range":{"start_line":5359,"start_character":0,"end_line":5359,"end_character":77},"updated":"2024-12-11 11:21:37.000000000","message":"I think there should be if conditional to check if volume.id \u003d\u003d image.snap.volume_id and if yes do self.driver.revert_to_snapshot(context, volume, image_snap) ; else ..... what there is now\n\nSomething as\n```\n                if volume.id \u003d\u003d image_snap.volume_id:\n                    LOG.debug(f\"Reverting volume id {volume.id} from snapshot {image_snap.volume_id}\")\n                    ```\n                    \nThis will ensure that if for example ceph is a backend ..it will be done on backend side instead of copy -\u003e create temp volume -\u003e volume. \n\nI\u0027ve tested and it\u0027s working and of course fast as expected.","commit_id":"2df867cd7198156d1efcaa5c2efdc2ff5abf3628"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"30ad534d58d84e87ee1eeee2a6093b401526cc1e","unresolved":true,"context_lines":[{"line_number":5356,"context_line":"                # snapshot and do a copy so we are safe here."},{"line_number":5357,"context_line":"                # Size checks are already done on the API layer so we don\u0027t"},{"line_number":5358,"context_line":"                # need to worry about the image fitting into the volume."},{"line_number":5359,"context_line":"                self._revert_to_snapshot_generic(context, volume, image_snap)"},{"line_number":5360,"context_line":"            else:"},{"line_number":5361,"context_line":"                image_id \u003d image_meta[\u0027id\u0027]"},{"line_number":5362,"context_line":"                image_service, _ \u003d glance.get_remote_image_service("}],"source_content_type":"text/x-python","patch_set":3,"id":"9df29f8f_089c9853","line":5359,"range":{"start_line":5359,"start_character":0,"end_line":5359,"end_character":77},"in_reply_to":"08607306_e9731956","updated":"2024-12-13 14:30:53.000000000","message":"But you know, when it\u0027s a \"reimage\" volume from a volume snapshot, and volume id \u003d snapshot.volume_id, it must mean that creating a temporary volume from a snapshot -\u003e copying to a volume must be the same as driver.revert_to_snapshot_non_generic. The result is simply the same, except that it could be significantly faster.\n\nI mean to say that it\u0027s also about speed.","commit_id":"2df867cd7198156d1efcaa5c2efdc2ff5abf3628"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"1fd79edc00b2f773feaed1f473a3761a3a7db3e5","unresolved":true,"context_lines":[{"line_number":5356,"context_line":"                # snapshot and do a copy so we are safe here."},{"line_number":5357,"context_line":"                # Size checks are already done on the API layer so we don\u0027t"},{"line_number":5358,"context_line":"                # need to worry about the image fitting into the volume."},{"line_number":5359,"context_line":"                self._revert_to_snapshot_generic(context, volume, image_snap)"},{"line_number":5360,"context_line":"            else:"},{"line_number":5361,"context_line":"                image_id \u003d image_meta[\u0027id\u0027]"},{"line_number":5362,"context_line":"                image_service, _ \u003d glance.get_remote_image_service("}],"source_content_type":"text/x-python","patch_set":3,"id":"08607306_e9731956","line":5359,"range":{"start_line":5359,"start_character":0,"end_line":5359,"end_character":77},"in_reply_to":"087b9f59_be7957ed","updated":"2024-12-13 11:39:46.000000000","message":"This path is not expected to be optimized. The reason for using the _revert_to_snapshot_generic method is it already provides the implementation required for doing the reimage i.e. create a temp volume and do the copy from temp to the original volume otherwise I would have added a new method for doing the same.\nThe optimization path would be good but requires more testing and has possibility to introduce regression in the reimage workflow which is not worth the effort of adding optimization for a specific case.","commit_id":"2df867cd7198156d1efcaa5c2efdc2ff5abf3628"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"46a0915ff5260e7026444eef1c95eb59591ec44f","unresolved":true,"context_lines":[{"line_number":5356,"context_line":"                # snapshot and do a copy so we are safe here."},{"line_number":5357,"context_line":"                # Size checks are already done on the API layer so we don\u0027t"},{"line_number":5358,"context_line":"                # need to worry about the image fitting into the volume."},{"line_number":5359,"context_line":"                self._revert_to_snapshot_generic(context, volume, image_snap)"},{"line_number":5360,"context_line":"            else:"},{"line_number":5361,"context_line":"                image_id \u003d image_meta[\u0027id\u0027]"},{"line_number":5362,"context_line":"                image_service, _ \u003d glance.get_remote_image_service("}],"source_content_type":"text/x-python","patch_set":3,"id":"4a73c6a2_a0afb716","line":5359,"range":{"start_line":5359,"start_character":0,"end_line":5359,"end_character":77},"in_reply_to":"71e97efc_05612773","updated":"2024-12-17 15:15:02.000000000","message":"What I wanted to say is (correct me if i am wrong), that if the volume_id \u003d\u003d snapshot.volume_id, it means you are going to create a volume from a snapshot of some volume -\u003e same volume. \n\nSoo, if you are using the LVM, or using ceph ..or whatever ..it means that it\u0027s same driver and same disk .. so you can be safe to leave it on driver how it will be done .. how snapshot will be reverted/data copied to the original volume ... cant\u0027t you ?","commit_id":"2df867cd7198156d1efcaa5c2efdc2ff5abf3628"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"0dec3fb4e58fd3b9691e64c9a5185553d9a320e3","unresolved":true,"context_lines":[{"line_number":5356,"context_line":"                # snapshot and do a copy so we are safe here."},{"line_number":5357,"context_line":"                # Size checks are already done on the API layer so we don\u0027t"},{"line_number":5358,"context_line":"                # need to worry about the image fitting into the volume."},{"line_number":5359,"context_line":"                self._revert_to_snapshot_generic(context, volume, image_snap)"},{"line_number":5360,"context_line":"            else:"},{"line_number":5361,"context_line":"                image_id \u003d image_meta[\u0027id\u0027]"},{"line_number":5362,"context_line":"                image_service, _ \u003d glance.get_remote_image_service("}],"source_content_type":"text/x-python","patch_set":3,"id":"71e97efc_05612773","line":5359,"range":{"start_line":5359,"start_character":0,"end_line":5359,"end_character":77},"in_reply_to":"9df29f8f_089c9853","updated":"2024-12-15 21:31:44.000000000","message":"To be clear, I\u0027m not against the idea of this optimization. What I\u0027m saying is it is outside the scope of my intended patch since it will require additional testing. If you plan to propose a follow up patch with the said optimization, I am happy to review it.","commit_id":"2df867cd7198156d1efcaa5c2efdc2ff5abf3628"}]}
