)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"c3457fe948b661bfc3f01e194aab76941e707875","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"07dee7a8_652072c8","updated":"2026-07-03 11:42:44.000000000","message":"recheck\nunrelated tests failed with timeout in devstack-plugin-nfs-tempest-full","commit_id":"b4394c0998cf4d0186d73cef93d4c3506a611eb8"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"d7a613079761eaf84305c0ed99e578f4a8ff9b7b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"987f8179_e427ff5e","updated":"2026-07-13 20:29:47.000000000","message":"removing my -1 so other reviews can take a look.","commit_id":"b4394c0998cf4d0186d73cef93d4c3506a611eb8"}],"cinder/volume/drivers/rbd.py":[{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"051abd211db6c1ebef916bace0df9c5fe9a6a48f","unresolved":true,"context_lines":[{"line_number":330,"context_line":"                     \u0027rbd_ceph_conf\u0027, \u0027rbd_pool\u0027]:"},{"line_number":331,"context_line":"            val \u003d getattr(self.configuration, attr)"},{"line_number":332,"context_line":"            if val is not None:"},{"line_number":333,"context_line":"                setattr(self.configuration, attr, utils.convert_str(val))"},{"line_number":334,"context_line":""},{"line_number":335,"context_line":"        self._backend_name \u003d (self.configuration.volume_backend_name or"},{"line_number":336,"context_line":"                              self.__class__.__name__)"}],"source_content_type":"text/x-python","patch_set":1,"id":"0086c256_60a189dc","line":333,"updated":"2026-07-03 08:53:37.000000000","message":"so I have doubt in this, in my devstack env, rbd_user in conf is set properly to cinder, and I beliive in prod also some username will be set.\nbut here its saying it can be None.\n\nwill this work for cinder to communicate with backend without user?","commit_id":"b4394c0998cf4d0186d73cef93d4c3506a611eb8"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"d055e1059cca86bd14780db3cb768644e64731e8","unresolved":true,"context_lines":[{"line_number":2041,"context_line":""},{"line_number":2042,"context_line":"            # Only delete and recreate for volume creation."},{"line_number":2043,"context_line":"            # For reimage, overwrite existing volume to preserve snapshots."},{"line_number":2044,"context_line":"            if not volume.get(\u0027previous_status\u0027):"},{"line_number":2045,"context_line":"                @utils.retry(exception.VolumeIsBusy,"},{"line_number":2046,"context_line":"                             self.configuration.rados_connection_interval,"},{"line_number":2047,"context_line":"                             self.configuration.rados_connection_retries)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7694ebc7_39f70583","line":2044,"range":{"start_line":2044,"start_character":0,"end_line":2044,"end_character":49},"updated":"2026-07-08 10:46:05.000000000","message":"This might not be future proof in the way that another operation starts calling _copy_image_to_volume then we won\u0027t be able to determine if \n1. it\u0027s a bootable volume creation\n2. reimage operation\n3. newly introduced operation","commit_id":"b4394c0998cf4d0186d73cef93d4c3506a611eb8"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"d2f637d68e2a841e6120fa34f0a6781249283315","unresolved":true,"context_lines":[{"line_number":2041,"context_line":""},{"line_number":2042,"context_line":"            # Only delete and recreate for volume creation."},{"line_number":2043,"context_line":"            # For reimage, overwrite existing volume to preserve snapshots."},{"line_number":2044,"context_line":"            if not volume.get(\u0027previous_status\u0027):"},{"line_number":2045,"context_line":"                @utils.retry(exception.VolumeIsBusy,"},{"line_number":2046,"context_line":"                             self.configuration.rados_connection_interval,"},{"line_number":2047,"context_line":"                             self.configuration.rados_connection_retries)"}],"source_content_type":"text/x-python","patch_set":1,"id":"fb2c1c1f_cf4a5509","line":2044,"range":{"start_line":2044,"start_character":0,"end_line":2044,"end_character":49},"in_reply_to":"7694ebc7_39f70583","updated":"2026-07-13 02:00:06.000000000","message":"so today this is only caled from create_volume and reimage.\nI think previous_status is standard(good) way to know current operation context/status of vol object.\n\nsurely if we decided to use this for another flow/use-case too, the existing tests will fail and catch the mismatch. \n\nstill, if you feel, we should have better segregation, we can add another argument in func signature reimage\u003dFalse for this check.","commit_id":"b4394c0998cf4d0186d73cef93d4c3506a611eb8"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"d1b5a925f3fd6a6c80d680e1a28814022d572098","unresolved":true,"context_lines":[{"line_number":2041,"context_line":""},{"line_number":2042,"context_line":"            # Only delete and recreate for volume creation."},{"line_number":2043,"context_line":"            # For reimage, overwrite existing volume to preserve snapshots."},{"line_number":2044,"context_line":"            if not volume.get(\u0027previous_status\u0027):"},{"line_number":2045,"context_line":"                @utils.retry(exception.VolumeIsBusy,"},{"line_number":2046,"context_line":"                             self.configuration.rados_connection_interval,"},{"line_number":2047,"context_line":"                             self.configuration.rados_connection_retries)"}],"source_content_type":"text/x-python","patch_set":1,"id":"c5add37a_8989785c","line":2044,"range":{"start_line":2044,"start_character":0,"end_line":2044,"end_character":49},"in_reply_to":"7c99078a_dc9efe88","updated":"2026-07-17 12:01:45.000000000","message":"replaced previous_status check with passing an explicit reimage\u003dTrue parameter.","commit_id":"b4394c0998cf4d0186d73cef93d4c3506a611eb8"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"d7a613079761eaf84305c0ed99e578f4a8ff9b7b","unresolved":true,"context_lines":[{"line_number":2041,"context_line":""},{"line_number":2042,"context_line":"            # Only delete and recreate for volume creation."},{"line_number":2043,"context_line":"            # For reimage, overwrite existing volume to preserve snapshots."},{"line_number":2044,"context_line":"            if not volume.get(\u0027previous_status\u0027):"},{"line_number":2045,"context_line":"                @utils.retry(exception.VolumeIsBusy,"},{"line_number":2046,"context_line":"                             self.configuration.rados_connection_interval,"},{"line_number":2047,"context_line":"                             self.configuration.rados_connection_retries)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7c99078a_dc9efe88","line":2044,"range":{"start_line":2044,"start_character":0,"end_line":2044,"end_character":49},"in_reply_to":"fb2c1c1f_cf4a5509","updated":"2026-07-13 20:29:47.000000000","message":"previous_status is a an intermin way to store the volume\u0027s status during a transition and rollback if needed. it is in no way associated with the reimage feature so I would incline towards differentiating the two workflows with a new parameter.","commit_id":"b4394c0998cf4d0186d73cef93d4c3506a611eb8"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"d055e1059cca86bd14780db3cb768644e64731e8","unresolved":true,"context_lines":[{"line_number":2062,"context_line":"                args.extend(self._ceph_args())"},{"line_number":2063,"context_line":"                self._try_execute(*args)"},{"line_number":2064,"context_line":"            else:"},{"line_number":2065,"context_line":"                # use qemu-img with -n flag to overwrite existing volume"},{"line_number":2066,"context_line":"                rbd_volume \u003d ("},{"line_number":2067,"context_line":"                    f\u0027rbd:{self.configuration.rbd_pool}/\u0027"},{"line_number":2068,"context_line":"                    f\u0027{volume.name}:\u0027"},{"line_number":2069,"context_line":"                    f\u0027conf\u003d{self.configuration.rbd_ceph_conf}\u0027"},{"line_number":2070,"context_line":"                )"},{"line_number":2071,"context_line":"                if self.configuration.rbd_user:"},{"line_number":2072,"context_line":"                    rbd_volume +\u003d f\u0027:id\u003d{self.configuration.rbd_user}\u0027"},{"line_number":2073,"context_line":""},{"line_number":2074,"context_line":"                image_utils.convert_image(tmp.name, rbd_volume, \u0027raw\u0027,"},{"line_number":2075,"context_line":"                                          run_as_root\u003dTrue,"},{"line_number":2076,"context_line":"                                          src_format\u003d\u0027raw\u0027,"},{"line_number":2077,"context_line":"                                          image_id\u003dimage_id,"},{"line_number":2078,"context_line":"                                          data\u003dNone,"},{"line_number":2079,"context_line":"                                          disable_sparse\u003ddisable_sparse,"},{"line_number":2080,"context_line":"                                          skip_create\u003dTrue)"},{"line_number":2081,"context_line":"        self._resize(volume)"},{"line_number":2082,"context_line":"        # We may need to re-enable replication features"},{"line_number":2083,"context_line":"        # after volume data modification."}],"source_content_type":"text/x-python","patch_set":1,"id":"94f6e43b_0c93de52","line":2080,"range":{"start_line":2065,"start_character":0,"end_line":2080,"end_character":59},"updated":"2026-07-08 10:46:05.000000000","message":"this approach seems to cover both cases, why aren\u0027t we using it for new volume creation workflow?\nThe only difference I see is\n\n1. \u0027--order\u0027, order\n2. --new-format\n\nI think --new-format is not needed anymore since Jewels, but order should be preserved but would be good to have this approach for both cases (instead of create volume, delete volume then using rbd import)","commit_id":"b4394c0998cf4d0186d73cef93d4c3506a611eb8"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"d2f637d68e2a841e6120fa34f0a6781249283315","unresolved":true,"context_lines":[{"line_number":2062,"context_line":"                args.extend(self._ceph_args())"},{"line_number":2063,"context_line":"                self._try_execute(*args)"},{"line_number":2064,"context_line":"            else:"},{"line_number":2065,"context_line":"                # use qemu-img with -n flag to overwrite existing volume"},{"line_number":2066,"context_line":"                rbd_volume \u003d ("},{"line_number":2067,"context_line":"                    f\u0027rbd:{self.configuration.rbd_pool}/\u0027"},{"line_number":2068,"context_line":"                    f\u0027{volume.name}:\u0027"},{"line_number":2069,"context_line":"                    f\u0027conf\u003d{self.configuration.rbd_ceph_conf}\u0027"},{"line_number":2070,"context_line":"                )"},{"line_number":2071,"context_line":"                if self.configuration.rbd_user:"},{"line_number":2072,"context_line":"                    rbd_volume +\u003d f\u0027:id\u003d{self.configuration.rbd_user}\u0027"},{"line_number":2073,"context_line":""},{"line_number":2074,"context_line":"                image_utils.convert_image(tmp.name, rbd_volume, \u0027raw\u0027,"},{"line_number":2075,"context_line":"                                          run_as_root\u003dTrue,"},{"line_number":2076,"context_line":"                                          src_format\u003d\u0027raw\u0027,"},{"line_number":2077,"context_line":"                                          image_id\u003dimage_id,"},{"line_number":2078,"context_line":"                                          data\u003dNone,"},{"line_number":2079,"context_line":"                                          disable_sparse\u003ddisable_sparse,"},{"line_number":2080,"context_line":"                                          skip_create\u003dTrue)"},{"line_number":2081,"context_line":"        self._resize(volume)"},{"line_number":2082,"context_line":"        # We may need to re-enable replication features"},{"line_number":2083,"context_line":"        # after volume data modification."}],"source_content_type":"text/x-python","patch_set":1,"id":"b7649fa8_525ae340","line":2080,"range":{"start_line":2065,"start_character":0,"end_line":2080,"end_character":59},"in_reply_to":"94f6e43b_0c93de52","updated":"2026-07-13 02:00:06.000000000","message":"ack, and yeah, it worked in my manual test too, using `qemu-img convert -n`.\n\nbut changing the creation path is a big change here to an exiting well tested flow, also comment at L2055-2066 - `keep using rbd import since it detects zeroes to preserve sparseness`\n\nSo though we can create new vol, `rbd import` was intentionally choosen, switching away from it needs proper validation of sparness handling, which is better done as a feature or improvement instead of bug fix.\n\nfor my ref, delete and comment was added in: https://review.opendev.org/c/openstack/cinder/+/30792","commit_id":"b4394c0998cf4d0186d73cef93d4c3506a611eb8"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"1ed7669bd134247d8cf07bcc01d651377d93bf3b","unresolved":true,"context_lines":[{"line_number":2062,"context_line":"                args.extend(self._ceph_args())"},{"line_number":2063,"context_line":"                self._try_execute(*args)"},{"line_number":2064,"context_line":"            else:"},{"line_number":2065,"context_line":"                # use qemu-img with -n flag to overwrite existing volume"},{"line_number":2066,"context_line":"                rbd_volume \u003d ("},{"line_number":2067,"context_line":"                    f\u0027rbd:{self.configuration.rbd_pool}/\u0027"},{"line_number":2068,"context_line":"                    f\u0027{volume.name}:\u0027"},{"line_number":2069,"context_line":"                    f\u0027conf\u003d{self.configuration.rbd_ceph_conf}\u0027"},{"line_number":2070,"context_line":"                )"},{"line_number":2071,"context_line":"                if self.configuration.rbd_user:"},{"line_number":2072,"context_line":"                    rbd_volume +\u003d f\u0027:id\u003d{self.configuration.rbd_user}\u0027"},{"line_number":2073,"context_line":""},{"line_number":2074,"context_line":"                image_utils.convert_image(tmp.name, rbd_volume, \u0027raw\u0027,"},{"line_number":2075,"context_line":"                                          run_as_root\u003dTrue,"},{"line_number":2076,"context_line":"                                          src_format\u003d\u0027raw\u0027,"},{"line_number":2077,"context_line":"                                          image_id\u003dimage_id,"},{"line_number":2078,"context_line":"                                          data\u003dNone,"},{"line_number":2079,"context_line":"                                          disable_sparse\u003ddisable_sparse,"},{"line_number":2080,"context_line":"                                          skip_create\u003dTrue)"},{"line_number":2081,"context_line":"        self._resize(volume)"},{"line_number":2082,"context_line":"        # We may need to re-enable replication features"},{"line_number":2083,"context_line":"        # after volume data modification."}],"source_content_type":"text/x-python","patch_set":1,"id":"93d12783_5f86524a","line":2080,"range":{"start_line":2065,"start_character":0,"end_line":2080,"end_character":59},"in_reply_to":"b7649fa8_525ae340","updated":"2026-07-14 07:08:14.000000000","message":"saw this https://qemu.readthedocs.io/en/master/tools/qemu-img.html#cmdoption-qemu-img-common-opts-S\n\nlater it also says - `SPARSE_SIZE is 0, the source will not be scanned for unallocated or zero sectors`","commit_id":"b4394c0998cf4d0186d73cef93d4c3506a611eb8"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"d7a613079761eaf84305c0ed99e578f4a8ff9b7b","unresolved":true,"context_lines":[{"line_number":2062,"context_line":"                args.extend(self._ceph_args())"},{"line_number":2063,"context_line":"                self._try_execute(*args)"},{"line_number":2064,"context_line":"            else:"},{"line_number":2065,"context_line":"                # use qemu-img with -n flag to overwrite existing volume"},{"line_number":2066,"context_line":"                rbd_volume \u003d ("},{"line_number":2067,"context_line":"                    f\u0027rbd:{self.configuration.rbd_pool}/\u0027"},{"line_number":2068,"context_line":"                    f\u0027{volume.name}:\u0027"},{"line_number":2069,"context_line":"                    f\u0027conf\u003d{self.configuration.rbd_ceph_conf}\u0027"},{"line_number":2070,"context_line":"                )"},{"line_number":2071,"context_line":"                if self.configuration.rbd_user:"},{"line_number":2072,"context_line":"                    rbd_volume +\u003d f\u0027:id\u003d{self.configuration.rbd_user}\u0027"},{"line_number":2073,"context_line":""},{"line_number":2074,"context_line":"                image_utils.convert_image(tmp.name, rbd_volume, \u0027raw\u0027,"},{"line_number":2075,"context_line":"                                          run_as_root\u003dTrue,"},{"line_number":2076,"context_line":"                                          src_format\u003d\u0027raw\u0027,"},{"line_number":2077,"context_line":"                                          image_id\u003dimage_id,"},{"line_number":2078,"context_line":"                                          data\u003dNone,"},{"line_number":2079,"context_line":"                                          disable_sparse\u003ddisable_sparse,"},{"line_number":2080,"context_line":"                                          skip_create\u003dTrue)"},{"line_number":2081,"context_line":"        self._resize(volume)"},{"line_number":2082,"context_line":"        # We may need to re-enable replication features"},{"line_number":2083,"context_line":"        # after volume data modification."}],"source_content_type":"text/x-python","patch_set":1,"id":"8c4731b8_91923f59","line":2080,"range":{"start_line":2065,"start_character":0,"end_line":2080,"end_character":59},"in_reply_to":"b7649fa8_525ae340","updated":"2026-07-13 20:29:47.000000000","message":"sparseness is already handled in qemu-img commands with -S 0 option.\nlet\u0027s see what other reviewers think about this.","commit_id":"b4394c0998cf4d0186d73cef93d4c3506a611eb8"}]}
