)]}'
{"cinder/image/image_utils.py":[{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"0413a4693cd211e68931f87b64620316ca4f0b41","unresolved":false,"context_lines":[{"line_number":488,"context_line":"                image_id\u003dimage_id,"},{"line_number":489,"context_line":"                reason\u003d_(\"fmt\u003d%(fmt)s backed by:%(backing_file)s\")"},{"line_number":490,"context_line":"                % {\u0027fmt\u0027: fmt, \u0027backing_file\u0027: backing_file, })"},{"line_number":491,"context_line":""},{"line_number":492,"context_line":"        # NOTE(jdg): I\u0027m using qemu-img convert to write"},{"line_number":493,"context_line":"        # to the volume regardless if it *needs* conversion or not"},{"line_number":494,"context_line":"        # TODO(avishay): We can speed this up by checking if the image is raw"}],"source_content_type":"text/x-python","patch_set":1,"id":"df7087c5_727e191a","line":491,"updated":"2018-03-22 16:41:05.000000000","message":"Based on the removed comment, this check should be done on the image conversion directory at the appropriate place.\n\nCan you confirm it\u0027s actually called there?","commit_id":"076687f35e324ead5fdca7650a1e720b9ce0c81b"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"6f7facd73f3006da73e3e54400edf4946fb34e47","unresolved":false,"context_lines":[{"line_number":488,"context_line":"                image_id\u003dimage_id,"},{"line_number":489,"context_line":"                reason\u003d_(\"fmt\u003d%(fmt)s backed by:%(backing_file)s\")"},{"line_number":490,"context_line":"                % {\u0027fmt\u0027: fmt, \u0027backing_file\u0027: backing_file, })"},{"line_number":491,"context_line":""},{"line_number":492,"context_line":"        # NOTE(jdg): I\u0027m using qemu-img convert to write"},{"line_number":493,"context_line":"        # to the volume regardless if it *needs* conversion or not"},{"line_number":494,"context_line":"        # TODO(avishay): We can speed this up by checking if the image is raw"}],"source_content_type":"text/x-python","patch_set":1,"id":"df7087c5_a65d4f48","line":491,"in_reply_to":"df7087c5_48dec2da","updated":"2018-03-22 19:22:03.000000000","message":"I think there are two checks being attempted with check_available_space(): one for the conversion directory and one for the actual volume destination if the volume is being created on an NFS backend (here).\n\nI\u0027m not sure what the impact on NFS is if we remove this check.","commit_id":"076687f35e324ead5fdca7650a1e720b9ce0c81b"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"9f7fbc33e4534fb170a8b7a90f8334a6a661bc19","unresolved":false,"context_lines":[{"line_number":488,"context_line":"                image_id\u003dimage_id,"},{"line_number":489,"context_line":"                reason\u003d_(\"fmt\u003d%(fmt)s backed by:%(backing_file)s\")"},{"line_number":490,"context_line":"                % {\u0027fmt\u0027: fmt, \u0027backing_file\u0027: backing_file, })"},{"line_number":491,"context_line":""},{"line_number":492,"context_line":"        # NOTE(jdg): I\u0027m using qemu-img convert to write"},{"line_number":493,"context_line":"        # to the volume regardless if it *needs* conversion or not"},{"line_number":494,"context_line":"        # TODO(avishay): We can speed this up by checking if the image is raw"}],"source_content_type":"text/x-python","patch_set":1,"id":"df7087c5_48dec2da","line":491,"in_reply_to":"df7087c5_7256195f","updated":"2018-03-22 17:53:19.000000000","message":"btw, the check on the conversion directory was initially failing in my environment. I thought it was failing for the disk. So that\u0027s why it would be good to have the message be a bit clearer (a different review is fine).","commit_id":"076687f35e324ead5fdca7650a1e720b9ce0c81b"},{"author":{"_account_id":23561,"name":"iain MacDonnell","email":"iain.macdonnell@oracle.com","username":"imacdonn"},"change_message_id":"09f1d32693276dfb678b4fe02deef6ca333369d5","unresolved":false,"context_lines":[{"line_number":488,"context_line":"                image_id\u003dimage_id,"},{"line_number":489,"context_line":"                reason\u003d_(\"fmt\u003d%(fmt)s backed by:%(backing_file)s\")"},{"line_number":490,"context_line":"                % {\u0027fmt\u0027: fmt, \u0027backing_file\u0027: backing_file, })"},{"line_number":491,"context_line":""},{"line_number":492,"context_line":"        # NOTE(jdg): I\u0027m using qemu-img convert to write"},{"line_number":493,"context_line":"        # to the volume regardless if it *needs* conversion or not"},{"line_number":494,"context_line":"        # TODO(avishay): We can speed this up by checking if the image is raw"}],"source_content_type":"text/x-python","patch_set":1,"id":"df7087c5_7256195f","line":491,"in_reply_to":"df7087c5_727e191a","updated":"2018-03-22 16:45:54.000000000","message":"I believe that that\u0027s covered at https://github.com/openstack/cinder/blob/master/cinder/volume/flows/manager/create_volume.py#L733-L735","commit_id":"076687f35e324ead5fdca7650a1e720b9ce0c81b"},{"author":{"_account_id":23561,"name":"iain MacDonnell","email":"iain.macdonnell@oracle.com","username":"imacdonn"},"change_message_id":"607a5977e96cc9f28afaa74d81d94363f82597d8","unresolved":false,"context_lines":[{"line_number":488,"context_line":"                image_id\u003dimage_id,"},{"line_number":489,"context_line":"                reason\u003d_(\"fmt\u003d%(fmt)s backed by:%(backing_file)s\")"},{"line_number":490,"context_line":"                % {\u0027fmt\u0027: fmt, \u0027backing_file\u0027: backing_file, })"},{"line_number":491,"context_line":""},{"line_number":492,"context_line":"        # NOTE(jdg): I\u0027m using qemu-img convert to write"},{"line_number":493,"context_line":"        # to the volume regardless if it *needs* conversion or not"},{"line_number":494,"context_line":"        # TODO(avishay): We can speed this up by checking if the image is raw"}],"source_content_type":"text/x-python","patch_set":1,"id":"df7087c5_973e37b6","line":491,"in_reply_to":"df7087c5_a65d4f48","updated":"2018-03-22 20:41:06.000000000","message":"Seems that the NFS driver checks for sufficient space in the filesystem (share) before creating the volume, here: https://github.com/openstack/cinder/blob/master/cinder/volume/drivers/nfs.py#L271-L276","commit_id":"076687f35e324ead5fdca7650a1e720b9ce0c81b"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"96cd46b3f747695b33f489a7ca2c5150a0e50e15","unresolved":false,"context_lines":[{"line_number":578,"context_line":"def check_available_space(dest, image_size, image_id):"},{"line_number":579,"context_line":"    # TODO(e0ne): replace psutil with shutil.disk_usage when we drop"},{"line_number":580,"context_line":"    # Python 2.7 support."},{"line_number":581,"context_line":"    if not os.path.isdir(dest):"},{"line_number":582,"context_line":"        dest \u003d os.path.dirname(dest)"},{"line_number":583,"context_line":""},{"line_number":584,"context_line":"    free_space \u003d psutil.disk_usage(dest).free"}],"source_content_type":"text/x-python","patch_set":1,"id":"df7087c5_d94cf477","line":581,"updated":"2018-03-22 11:27:11.000000000","message":"This is going to be an incorrect check whenever a /dev/sdx device is passed in here as dest. It might be better to do something like:\n\n    if utils.is_blk_device(dest):\n        # destination is a block device\n       return\n\nto avoid the bogus check whenever someone passes in a device path.\n\nBut unfortunately, you lose the check to see if the disk is going to be big enough for the image. Unfortunately, I wasn\u0027t able to find an easy way in python to get the size of a disk using the device path.","commit_id":"076687f35e324ead5fdca7650a1e720b9ce0c81b"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"9f7fbc33e4534fb170a8b7a90f8334a6a661bc19","unresolved":false,"context_lines":[{"line_number":578,"context_line":"def check_available_space(dest, image_size, image_id):"},{"line_number":579,"context_line":"    # TODO(e0ne): replace psutil with shutil.disk_usage when we drop"},{"line_number":580,"context_line":"    # Python 2.7 support."},{"line_number":581,"context_line":"    if not os.path.isdir(dest):"},{"line_number":582,"context_line":"        dest \u003d os.path.dirname(dest)"},{"line_number":583,"context_line":""},{"line_number":584,"context_line":"    free_space \u003d psutil.disk_usage(dest).free"}],"source_content_type":"text/x-python","patch_set":1,"id":"df7087c5_08f68a64","line":581,"in_reply_to":"df7087c5_122ebd91","updated":"2018-03-22 17:53:19.000000000","message":"That seems reasonable.","commit_id":"076687f35e324ead5fdca7650a1e720b9ce0c81b"},{"author":{"_account_id":23561,"name":"iain MacDonnell","email":"iain.macdonnell@oracle.com","username":"imacdonn"},"change_message_id":"b32c8cd4bad562f657536a436ed369e3fcf5205c","unresolved":false,"context_lines":[{"line_number":578,"context_line":"def check_available_space(dest, image_size, image_id):"},{"line_number":579,"context_line":"    # TODO(e0ne): replace psutil with shutil.disk_usage when we drop"},{"line_number":580,"context_line":"    # Python 2.7 support."},{"line_number":581,"context_line":"    if not os.path.isdir(dest):"},{"line_number":582,"context_line":"        dest \u003d os.path.dirname(dest)"},{"line_number":583,"context_line":""},{"line_number":584,"context_line":"    free_space \u003d psutil.disk_usage(dest).free"}],"source_content_type":"text/x-python","patch_set":1,"id":"df7087c5_122ebd91","line":581,"in_reply_to":"df7087c5_d94cf477","updated":"2018-03-22 16:26:49.000000000","message":"IMO, it\u0027s up to the backend to provide a volume of the requested size. There\u0027s already a check, before copying an image to a volume, that the volume is not smaller than the virtual size of the image. If the backend delivers a volume that\u0027s smaller than what was requested, that\u0027s a problem in the backend. I think that this function should be used only when checking for local temporary space on the cinder-volume host.","commit_id":"076687f35e324ead5fdca7650a1e720b9ce0c81b"},{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"96cd46b3f747695b33f489a7ca2c5150a0e50e15","unresolved":false,"context_lines":[{"line_number":583,"context_line":""},{"line_number":584,"context_line":"    free_space \u003d psutil.disk_usage(dest).free"},{"line_number":585,"context_line":"    if free_space \u003c\u003d image_size:"},{"line_number":586,"context_line":"        msg \u003d (\u0027There is no space to convert image. \u0027"},{"line_number":587,"context_line":"               \u0027Requested: %(image_size)s, available: %(free_space)s\u0027"},{"line_number":588,"context_line":"               ) % {\u0027image_size\u0027: image_size, \u0027free_space\u0027: free_space}"},{"line_number":589,"context_line":"        raise exception.ImageTooBig(image_id\u003dimage_id, reason\u003dmsg)"}],"source_content_type":"text/x-python","patch_set":1,"id":"df7087c5_5969c4bb","line":586,"updated":"2018-03-22 11:27:11.000000000","message":"It would also be good to add the dest to this error message so the user can see where there was not enough space.","commit_id":"076687f35e324ead5fdca7650a1e720b9ce0c81b"},{"author":{"_account_id":23561,"name":"iain MacDonnell","email":"iain.macdonnell@oracle.com","username":"imacdonn"},"change_message_id":"b32c8cd4bad562f657536a436ed369e3fcf5205c","unresolved":false,"context_lines":[{"line_number":583,"context_line":""},{"line_number":584,"context_line":"    free_space \u003d psutil.disk_usage(dest).free"},{"line_number":585,"context_line":"    if free_space \u003c\u003d image_size:"},{"line_number":586,"context_line":"        msg \u003d (\u0027There is no space to convert image. \u0027"},{"line_number":587,"context_line":"               \u0027Requested: %(image_size)s, available: %(free_space)s\u0027"},{"line_number":588,"context_line":"               ) % {\u0027image_size\u0027: image_size, \u0027free_space\u0027: free_space}"},{"line_number":589,"context_line":"        raise exception.ImageTooBig(image_id\u003dimage_id, reason\u003dmsg)"}],"source_content_type":"text/x-python","patch_set":1,"id":"df7087c5_3232e1f3","line":586,"in_reply_to":"df7087c5_5969c4bb","updated":"2018-03-22 16:26:49.000000000","message":"Totally agree, but I think it\u0027s out of scope for this change (which is focused on  *removing* use of this function. I may submit another change to improve the exception message for any legitimate uses of this function.","commit_id":"076687f35e324ead5fdca7650a1e720b9ce0c81b"}]}