)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"a2784986f180a33042d79899892f7a12d1e0e5e7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"a531c51f_a3662283","updated":"2025-01-06 10:26:53.000000000","message":"Hi Fernando,\n\nplease find my response inline.","commit_id":"7d320764980a0e33a9abad14db41a4cdee71d57a"},{"author":{"_account_id":30555,"name":"Fernando Ferraz","display_name":"Fernando Ferraz","email":"fesilva@redhat.com","username":"fernandoperches"},"change_message_id":"836db126c08d9516d5402acccadf0261e1ea937b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"ce3ea08a_31df4445","updated":"2025-01-03 17:55:59.000000000","message":"Hi Rajat! Code and tests looks good. There is just one thing I haven\u0027t completely understood yet so I added a comment to clarify it.","commit_id":"7d320764980a0e33a9abad14db41a4cdee71d57a"},{"author":{"_account_id":30555,"name":"Fernando Ferraz","display_name":"Fernando Ferraz","email":"fesilva@redhat.com","username":"fernandoperches"},"change_message_id":"2eab09ff87bde0ad6428b4fa3367e658e77b3b04","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"9a0ed83e_e6bb0ad5","updated":"2025-01-03 17:39:53.000000000","message":"Hi Rajat! Code and tests looks good. There is just one thing I haven\u0027t completely understood yet, so I added a comment to clarify it.","commit_id":"7d320764980a0e33a9abad14db41a4cdee71d57a"},{"author":{"_account_id":10459,"name":"Luigi Toscano","email":"ltoscano@redhat.com","username":"ltoscano"},"change_message_id":"66d96d1b2a74c2d5087df268cd666b622eed9b0b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"c3735030_e48d20b8","updated":"2025-02-19 10:59:08.000000000","message":"The tempest tests which touches this codepath was merged. It failed without this patch and passed with this patch. Time to merge this if there are no other complains? And also backport it to all open branches, or tempest would fail on the older ones.","commit_id":"7d320764980a0e33a9abad14db41a4cdee71d57a"},{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"6fd9f14e4f12da2602b9683aace05f33c789f03a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"eb3ed974_a5b656ec","updated":"2025-03-26 14:29:23.000000000","message":"This looks okay to me, it addresses an important bug and doesn\u0027t make the original logic worse (tobias\u0027 comment is valid, this should be addressed in a separate patch)","commit_id":"7d320764980a0e33a9abad14db41a4cdee71d57a"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"1332e29016c7cea8292853b2bdad58aa18f7573f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"e3267aa2_6a0ee499","updated":"2024-12-27 08:21:59.000000000","message":"recheck ceph job failed again with nova test\n\ntempest.api.compute.servers.test_server_rescue.ServerStableDeviceRescueTest.test_stable_device_rescue_disk_virtio_with_volume_attached\n\nDB Error\n\u003cclass \u0027oslo_db.exception.DBConnectionError\u0027\u003e","commit_id":"7d320764980a0e33a9abad14db41a4cdee71d57a"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"75b1ddb3113736abaaf1836953fcf4a33e618caf","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"3dbc2685_2e55de45","updated":"2024-12-26 18:37:07.000000000","message":"recheck cinder-plugin-ceph-tempest failed and there are some signs related to this patch but not concrete\n\nDB Errors look unrelated\nDec 26 16:29:40.892615 np0039447322 cinder-volume[105692]: ERROR dbcounter [-] Failed to account for access to database \u0027cinder\u0027: (pymysql.err.OperationalError) (2003, \"Can\u0027t connect to MySQL server on \u0027127.0.0.1\u0027 ([Errno 111] ECONNREFUSED)\")\n\nThis might be related but we are just introducing the old workflow back so shouldn\u0027t be an issue\n\nDec 26 16:36:47.973126 np0039447322 cinder-volume[105692]: DEBUG cinder.utils [req-5458c473-9e7b-41d4-b707-591f22a9d437 req-34815525-0366-4c99-92da-38a73f86a84c tempest-TestVolumeBootPattern-513202553 None] Retrying cinder.volume.drivers.rbd.RBDDriver._try_remove_volume.\u003clocals\u003e._do_try_remove_volume in 5.0 seconds as it raised ImageBusy: [errno 16] RBD image is busy (error removing image). {{(pid\u003d105692) log_it /opt/stack/data/venv/lib/python3.12/site-packages/tenacity/before_sleep.py:65}}","commit_id":"7d320764980a0e33a9abad14db41a4cdee71d57a"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"91437e849c13612bedf3cb06b11daab65d89b6ff","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"3ba60b44_bf2c189c","updated":"2024-12-26 21:48:12.000000000","message":"recheck cinder-plugin-ceph-tempest failed but with different tests\nlooks like some issue with nova DB connection\n\n2024-12-26 19:29:44.810 113397 ERROR tempest.common.compute \u003cclass \u0027oslo_db.exception.DBConnectionError\u0027\u003e","commit_id":"7d320764980a0e33a9abad14db41a4cdee71d57a"}],"cinder/volume/drivers/rbd.py":[{"author":{"_account_id":30555,"name":"Fernando Ferraz","display_name":"Fernando Ferraz","email":"fesilva@redhat.com","username":"fernandoperches"},"change_message_id":"2eab09ff87bde0ad6428b4fa3367e658e77b3b04","unresolved":true,"context_lines":[{"line_number":2096,"context_line":"            volume_utils.upload_volume(context, image_service, image_meta,"},{"line_number":2097,"context_line":"                                       None, volume, volume_fd\u003dsource_handle)"},{"line_number":2098,"context_line":"        else:"},{"line_number":2099,"context_line":"            # When the image format is different from volume format, we will"},{"line_number":2100,"context_line":"            # fallback to the old workflow because of the following issues:"},{"line_number":2101,"context_line":"            # 1. Passing RBDVolumeIOWrapper to format inspector"},{"line_number":2102,"context_line":"            # We fail when calling privsep since RPC cannot serialize the"}],"source_content_type":"text/x-python","patch_set":2,"id":"66ae1f10_22e4b2f4","line":2099,"updated":"2025-01-03 17:39:53.000000000","message":"This states the fallback is required whenever the image format isn\u0027t the same as the volume format, but the code only checks for the output image format (`image_meta[\u0027disk_format\u0027] \u003d\u003d \u0027raw\u0027`), which I believe might lead to assuming a volume will always be \u0027raw.\u0027 Is that correct? I just may not be seeing the whole picture.\n\nIt would be more precise to state that any image format different from \u0027raw\u0027 results in a fallback.","commit_id":"7d320764980a0e33a9abad14db41a4cdee71d57a"},{"author":{"_account_id":30555,"name":"Fernando Ferraz","display_name":"Fernando Ferraz","email":"fesilva@redhat.com","username":"fernandoperches"},"change_message_id":"15e147ef996df42de36d4f1bfe15dc2b5afc3158","unresolved":false,"context_lines":[{"line_number":2096,"context_line":"            volume_utils.upload_volume(context, image_service, image_meta,"},{"line_number":2097,"context_line":"                                       None, volume, volume_fd\u003dsource_handle)"},{"line_number":2098,"context_line":"        else:"},{"line_number":2099,"context_line":"            # When the image format is different from volume format, we will"},{"line_number":2100,"context_line":"            # fallback to the old workflow because of the following issues:"},{"line_number":2101,"context_line":"            # 1. Passing RBDVolumeIOWrapper to format inspector"},{"line_number":2102,"context_line":"            # We fail when calling privsep since RPC cannot serialize the"}],"source_content_type":"text/x-python","patch_set":2,"id":"a178b63d_fe2d872e","line":2099,"in_reply_to":"2a47277b_330cb5fa","updated":"2025-01-06 12:22:06.000000000","message":"Thanks Rajat for making it clear. That is what I understood from the checking you added, but I wanted to ensure we are not missing any corner cases. I\u0027m dropping the -1.\n\nI\u0027m not 100% sure about it, but it might be worth including this information in the comments, as the phrase \u0027When the image format is different from volume format\u0027 may indicate that other format combinations are possible. Same for the release notes.","commit_id":"7d320764980a0e33a9abad14db41a4cdee71d57a"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"a2784986f180a33042d79899892f7a12d1e0e5e7","unresolved":true,"context_lines":[{"line_number":2096,"context_line":"            volume_utils.upload_volume(context, image_service, image_meta,"},{"line_number":2097,"context_line":"                                       None, volume, volume_fd\u003dsource_handle)"},{"line_number":2098,"context_line":"        else:"},{"line_number":2099,"context_line":"            # When the image format is different from volume format, we will"},{"line_number":2100,"context_line":"            # fallback to the old workflow because of the following issues:"},{"line_number":2101,"context_line":"            # 1. Passing RBDVolumeIOWrapper to format inspector"},{"line_number":2102,"context_line":"            # We fail when calling privsep since RPC cannot serialize the"}],"source_content_type":"text/x-python","patch_set":2,"id":"2a47277b_330cb5fa","line":2099,"in_reply_to":"66ae1f10_22e4b2f4","updated":"2025-01-06 10:26:53.000000000","message":"We only store RAW volumes in RBD so if the image format is any other than \u0027raw\u0027, it will require conversion.","commit_id":"7d320764980a0e33a9abad14db41a4cdee71d57a"},{"author":{"_account_id":16137,"name":"Tobias Urdin","email":"tobias.urdin@binero.com","username":"tobasco"},"change_message_id":"3c4ce044710a11567541f6731470baafdbfc3b01","unresolved":true,"context_lines":[{"line_number":2114,"context_line":"            tmp_dir \u003d volume_utils.image_conversion_dir()"},{"line_number":2115,"context_line":"            tmp_file \u003d os.path.join(tmp_dir,"},{"line_number":2116,"context_line":"                                    volume.name + \u0027-\u0027 + image_meta[\u0027id\u0027])"},{"line_number":2117,"context_line":"            with fileutils.remove_path_on_error(tmp_file):"},{"line_number":2118,"context_line":"                args \u003d [\u0027rbd\u0027, \u0027export\u0027,"},{"line_number":2119,"context_line":"                        \u0027--pool\u0027, self.configuration.rbd_pool,"},{"line_number":2120,"context_line":"                        volume.name, tmp_file]"}],"source_content_type":"text/x-python","patch_set":2,"id":"0fee7fec_5d523345","line":2117,"updated":"2025-03-12 19:46:47.000000000","message":"This will fail for larger volumes since it will fill up the local disk, should this be configurable and error out with a message API entry or similar? or should we check some kind of expected size on disk first? If multiple copy volume to image operations is happening at the same time it makes it even more likely to for the disk space to become full","commit_id":"7d320764980a0e33a9abad14db41a4cdee71d57a"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"4bc71fa5f7cafaf527fa6aaf54df1cfc9847fafc","unresolved":true,"context_lines":[{"line_number":2114,"context_line":"            tmp_dir \u003d volume_utils.image_conversion_dir()"},{"line_number":2115,"context_line":"            tmp_file \u003d os.path.join(tmp_dir,"},{"line_number":2116,"context_line":"                                    volume.name + \u0027-\u0027 + image_meta[\u0027id\u0027])"},{"line_number":2117,"context_line":"            with fileutils.remove_path_on_error(tmp_file):"},{"line_number":2118,"context_line":"                args \u003d [\u0027rbd\u0027, \u0027export\u0027,"},{"line_number":2119,"context_line":"                        \u0027--pool\u0027, self.configuration.rbd_pool,"},{"line_number":2120,"context_line":"                        volume.name, tmp_file]"}],"source_content_type":"text/x-python","patch_set":2,"id":"23e04ab0_a0d596bd","line":2117,"in_reply_to":"0fee7fec_5d523345","updated":"2025-03-26 14:22:12.000000000","message":"It\u0027s true that this is an issue if the conversion dir is not sufficiently large, but this has been the case for many years in a handful of places in Cinder.  (Including here in this driver before https://review.opendev.org/c/openstack/cinder/+/928024 was landed.)\n\nI would recommend flagging this for a later follow-up and not blocking this change for it.","commit_id":"7d320764980a0e33a9abad14db41a4cdee71d57a"},{"author":{"_account_id":16137,"name":"Tobias Urdin","email":"tobias.urdin@binero.com","username":"tobasco"},"change_message_id":"1774feb8bdbb1b65131664fad9bef4f65042d8bf","unresolved":false,"context_lines":[{"line_number":2114,"context_line":"            tmp_dir \u003d volume_utils.image_conversion_dir()"},{"line_number":2115,"context_line":"            tmp_file \u003d os.path.join(tmp_dir,"},{"line_number":2116,"context_line":"                                    volume.name + \u0027-\u0027 + image_meta[\u0027id\u0027])"},{"line_number":2117,"context_line":"            with fileutils.remove_path_on_error(tmp_file):"},{"line_number":2118,"context_line":"                args \u003d [\u0027rbd\u0027, \u0027export\u0027,"},{"line_number":2119,"context_line":"                        \u0027--pool\u0027, self.configuration.rbd_pool,"},{"line_number":2120,"context_line":"                        volume.name, tmp_file]"}],"source_content_type":"text/x-python","patch_set":2,"id":"71008d7c_7f85fd5e","line":2117,"in_reply_to":"23e04ab0_a0d596bd","updated":"2025-03-26 15:15:15.000000000","message":"Ack, makes sense if the issue already exists elsewhere we should have a combined effort for that in the future.","commit_id":"7d320764980a0e33a9abad14db41a4cdee71d57a"}]}