)]}'
{"cinder/image/image_utils.py":[{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"3e6dfdabe9dbf83f35535d70bd8a4ff9733b9575","unresolved":true,"context_lines":[{"line_number":951,"context_line":"    LOG.debug(f\u0027Copying image from {src} to volume {dest} - size: {size}\u0027)"},{"line_number":952,"context_line":"    if (isinstance(src, str) and isinstance(dest, str)):"},{"line_number":953,"context_line":"        if os.path.dirname(src) \u003d\u003d os.path.dirname(dest):"},{"line_number":954,"context_line":"            utils.execute(\u0027mv\u0027, src, dest, run_as_root\u003drun_as_root)"},{"line_number":955,"context_line":"            return"},{"line_number":956,"context_line":"    image_size_m \u003d math.ceil(float(size) / units.Mi)"},{"line_number":957,"context_line":"    volume_utils.copy_volume(src, dest, image_size_m, blocksize)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f04a003_8bea9605","line":954,"updated":"2025-05-14 12:26:32.000000000","message":"This changes a method which is called \"copy\" into a move in some cases -- this will introduce a good bit of confusion and is pretty likely to result in some surprising bugs.  If we think it\u0027s useful to make this a \"mv\" I don\u0027t think it should be done this way.","commit_id":"50a2f0e9dd8c88bcc09ccca55a97558bbe032102"},{"author":{"_account_id":28619,"name":"Dmitriy Rabotyagov","email":"noonedeadpunk@gmail.com","username":"noonedeadpunk"},"change_message_id":"dbcaa46089549ac7b81a98234429b6d1750b35b9","unresolved":true,"context_lines":[{"line_number":951,"context_line":"    LOG.debug(f\u0027Copying image from {src} to volume {dest} - size: {size}\u0027)"},{"line_number":952,"context_line":"    if (isinstance(src, str) and isinstance(dest, str)):"},{"line_number":953,"context_line":"        if os.path.dirname(src) \u003d\u003d os.path.dirname(dest):"},{"line_number":954,"context_line":"            utils.execute(\u0027mv\u0027, src, dest, run_as_root\u003drun_as_root)"},{"line_number":955,"context_line":"            return"},{"line_number":956,"context_line":"    image_size_m \u003d math.ceil(float(size) / units.Mi)"},{"line_number":957,"context_line":"    volume_utils.copy_volume(src, dest, image_size_m, blocksize)"}],"source_content_type":"text/x-python","patch_set":2,"id":"7cc109bd_b58d29f0","line":954,"in_reply_to":"3f04a003_8bea9605","updated":"2025-08-27 11:49:34.000000000","message":"I somehow struggle to think of more efficient way of achieving similar goal - not waste diskspace when there is no need for it.","commit_id":"50a2f0e9dd8c88bcc09ccca55a97558bbe032102"},{"author":{"_account_id":36171,"name":"jayaanand borra","display_name":"jayaanand borra","email":"jayaanand.borra@netapp.com","username":"jayaanan","status":"netapp"},"change_message_id":"d328d7a5c6ba2f66b13eb64e862ad2c1f6e5ae8f","unresolved":true,"context_lines":[{"line_number":951,"context_line":"    LOG.debug(f\u0027Copying image from {src} to volume {dest} - size: {size}\u0027)"},{"line_number":952,"context_line":"    if (isinstance(src, str) and isinstance(dest, str)):"},{"line_number":953,"context_line":"        if os.path.dirname(src) \u003d\u003d os.path.dirname(dest):"},{"line_number":954,"context_line":"            utils.execute(\u0027mv\u0027, src, dest, run_as_root\u003drun_as_root)"},{"line_number":955,"context_line":"            return"},{"line_number":956,"context_line":"    image_size_m \u003d math.ceil(float(size) / units.Mi)"},{"line_number":957,"context_line":"    volume_utils.copy_volume(src, dest, image_size_m, blocksize)"}],"source_content_type":"text/x-python","patch_set":2,"id":"232c83b6_41121a31","line":954,"in_reply_to":"68bb8b92_e724f90d","updated":"2026-09-27 17:10:51.000000000","message":"This mv is taken whenever os.path.dirname(src) \u003d\u003d os.path.dirname(dest). With the default image_conversion_dir ($state_path/conversion), that is true for RBD: RBDDriver._copy_image_to_volume stages the image with NamedTemporaryFile(dir\u003dvolume_utils.image_conversion_dir()) (rbd.py:2030-2034), and both temporary_file() and TemporaryImages.fetch() create their files with mkstemp(dir\u003dCONF.image_conversion_dir) (image_utils.py:1319 and 1454).\n\nOn the create-from-image path the source is not the private image_download_* file. CreateVolumeFromSpecTask._create_from_image_cache_or_download holds TemporaryImages.fetch (create_volume.py:991-1023) across the driver copy. fetch_to_volume_format then does tmp \u003d tmp_images.get(context, image_id) (image_utils.py:1092-1094) and this mv renames that path. get() returns temporary_images[context.user_id][image_id] (image_utils.py:1468-1472). A second in-flight fetch for that same user and image overwrites the one dict slot (image_utils.py:1460). This mv then renames whichever path is in the slot, including the other request\u0027s file, while that request is still inside its fetch. copy_volume() leaves the source in place, so those two creates both succeed today. The image bytes are the same image_id; the failure is the other create\u0027s path disappearing. qemu-img info and signature verification in the same with-block run before _create_from_image_download, so one create still reaches rbd import. Two overlapping downloads do not.\n\nThe {image_id} lock does not cover this. It is only on _prepare_image_cache_entry (create_volume.py:874). Encrypted volumes skip that call (create_volume.py:1134, not volume_is_encrypted). Cache-disabled backends never take it. After prepare returns without cloning, the fallback _create_from_image_cache_or_download at create_volume.py:1149 downloads with no lock. reimage (manager.py:5416) does not use TemporaryImages, so there the source really is the private temporary_file; please do not use that path to justify renaming the cached one.\n\ndirname equality does not check that dest is a disposable regular file, and it is not the \"same filesystem\" / \"same volume type\" check in the commit message. Please keep this helper as a byte copy. The second full-size file in the conversion dir is real for RBD RAW images (check_available_space only requires one image_size, image_utils.py:1272). Avoid that second file in RBDDriver._copy_image_to_volume, or rename only when src is the private temporary_file from this with-block and not TemporaryImages.get().\n\nThe calls at lines 1120 and 1146 do not pass run_as_root, so the new argument is unused; dd in _copy_volume_with_path already forces run_as_root\u003dTrue (volume_utils.py:501). The debug log on this path still says \"Copying\".","commit_id":"50a2f0e9dd8c88bcc09ccca55a97558bbe032102"},{"author":{"_account_id":28619,"name":"Dmitriy Rabotyagov","email":"noonedeadpunk@gmail.com","username":"noonedeadpunk"},"change_message_id":"cebebc3ec1dc7954ed51776e289556623d5ec2b9","unresolved":true,"context_lines":[{"line_number":951,"context_line":"    LOG.debug(f\u0027Copying image from {src} to volume {dest} - size: {size}\u0027)"},{"line_number":952,"context_line":"    if (isinstance(src, str) and isinstance(dest, str)):"},{"line_number":953,"context_line":"        if os.path.dirname(src) \u003d\u003d os.path.dirname(dest):"},{"line_number":954,"context_line":"            utils.execute(\u0027mv\u0027, src, dest, run_as_root\u003drun_as_root)"},{"line_number":955,"context_line":"            return"},{"line_number":956,"context_line":"    image_size_m \u003d math.ceil(float(size) / units.Mi)"},{"line_number":957,"context_line":"    volume_utils.copy_volume(src, dest, image_size_m, blocksize)"}],"source_content_type":"text/x-python","patch_set":2,"id":"68bb8b92_e724f90d","line":954,"in_reply_to":"7cc109bd_b58d29f0","updated":"2025-08-27 12:02:20.000000000","message":"I\u0027ve renamed the method to reduce confusion at least... But if you have some idea - I\u0027m very open to check on implementation.\n\nAs while my example mentions 50GB image, for Windows it\u0027s \"normal\" to do such operations with 250Gb images as well, which becomes not only extremely time consuming, but also when it\u0027s done several times in parallel - become very problematic.","commit_id":"50a2f0e9dd8c88bcc09ccca55a97558bbe032102"}],"cinder/tests/unit/test_image_utils.py":[{"author":{"_account_id":36171,"name":"jayaanand borra","display_name":"jayaanand borra","email":"jayaanand.borra@netapp.com","username":"jayaanan","status":"netapp"},"change_message_id":"d328d7a5c6ba2f66b13eb64e862ad2c1f6e5ae8f","unresolved":true,"context_lines":[{"line_number":1665,"context_line":""},{"line_number":1666,"context_line":"    @mock.patch(\u0027cinder.image.image_utils.volume_utils.copy_volume\u0027)"},{"line_number":1667,"context_line":"    @mock.patch(\u0027cinder.image.image_utils.utils.execute\u0027)"},{"line_number":1668,"context_line":"    def test_move_image_to_volume(self, mock_exec, mock_copy):"},{"line_number":1669,"context_line":"        image_size \u003d 41126400"},{"line_number":1670,"context_line":"        blocksize \u003d mock.sentinel.blocksize"},{"line_number":1671,"context_line":"        src_file \u003d \"/var/lib/cinder/conversion/image\""}],"source_content_type":"text/x-python","patch_set":6,"id":"f13341fc_faaa1e61","line":1668,"updated":"2026-09-27 17:10:51.000000000","message":"test_move_image_to_volume only checks that two hardcoded paths under /var/lib/cinder/conversion make utils.execute(\u0027mv\u0027, ...) run, and test_copy_image_to_volume checks that /tmp versus /var/lib/cinder/volumes still calls copy_volume. Neither test runs fetch_to_volume_format while TemporaryImages.fetch has registered the source. Please add that case: after copy_or_move_image_to_volume, the path stored by TemporaryImages for that user and image_id must still exist. Also assert the failing execute() path leaves dest unchanged. The same-directory mock call does not show that the create-from-image cache file survives.","commit_id":"c6b1504ce1da04f0abe456d9f686a1237d1ae388"}]}
