)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"c1d3aa21cfb7f7d5b982d087ee91517b7d0458ff","unresolved":false,"context_lines":[{"line_number":16,"context_line":"However, the check obviously needs to happen against every instance,"},{"line_number":17,"context_line":"not just the first to use a particular image. Consequently every image"},{"line_number":18,"context_line":"backend duplicates this check, making the check in fetch_to_raw both"},{"line_number":19,"context_line":"confusing and redundant."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"There are a couple of callers outside the libvirt driver. These do not"},{"line_number":22,"context_line":"pass the max_size argument, and are therefore unaffected."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"1aa78d24_3184645c","line":19,"updated":"2016-07-13 10:32:21.000000000","message":"Yes, the check is done by verify_base_size()","commit_id":"7fb25d50605f96b4ac1d3c20f4607bd0ae4b17d7"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"fc7379869b5d0df0eda7925f8380424df73df032","unresolved":false,"context_lines":[{"line_number":14,"context_line":"is used to download directly to the image cache, which means when"},{"line_number":15,"context_line":"booting multiple instances on the same compute it only executes once."},{"line_number":16,"context_line":"However, the check obviously needs to happen against every instance,"},{"line_number":17,"context_line":"not just the first to use a particular image. Consequently every image"},{"line_number":18,"context_line":"backend duplicates this check, making the check in fetch_to_raw both"},{"line_number":19,"context_line":"confusing and redundant."},{"line_number":20,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":12,"id":"dada55a8_560e07e6","line":17,"updated":"2016-07-22 14:48:50.000000000","message":"I\u0027m not sure about why the backends would duplicate this check since it should be a common thing, but at a minimum this is also checked here (for other reviewers\u0027 reference):\n\nhttps://github.com/openstack/nova/blob/master/nova/compute/api.py#L648-L650","commit_id":"cc2ae7526b35155687a5a1abbe4f2f80618c8f87"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"75b8fe16589408751b4c7db854b79d0e0d4c2607","unresolved":false,"context_lines":[{"line_number":14,"context_line":"is used to download directly to the image cache, which means when"},{"line_number":15,"context_line":"booting multiple instances on the same compute it only executes once."},{"line_number":16,"context_line":"However, the check obviously needs to happen against every instance,"},{"line_number":17,"context_line":"not just the first to use a particular image. Consequently every image"},{"line_number":18,"context_line":"backend duplicates this check, making the check in fetch_to_raw both"},{"line_number":19,"context_line":"confusing and redundant."},{"line_number":20,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":12,"id":"dada55a8_31e8af16","line":17,"in_reply_to":"dada55a8_560e07e6","updated":"2016-07-22 19:30:09.000000000","message":"I thought of the same check as Dan in the compute API.\n\nThe max_size in the imagebackend was added in icehouse:\n\nhttps://review.openstack.org/#/c/54765/\n\nThe weird thing is the check in the compute API was added before that, in Havana:\n\nhttps://review.openstack.org/#/c/29842/\n\nThe change in icehouse was for two CVEs though, so I\u0027m a bit hesitant to change things here without understanding the history.\n\nI think Phil Day is referring to the existing check in the compute API and why it wasn\u0027t sufficient here:\n\nhttps://bugs.launchpad.net/nova/+bug/1177830/comments/6\n\nThis was the other bug:\n\nhttps://bugs.launchpad.net/nova/+bug/1206081","commit_id":"cc2ae7526b35155687a5a1abbe4f2f80618c8f87"}],"nova/virt/libvirt/imagebackend.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"25ee9d57bcdf2ed226106af8abbf634d23ad8886","unresolved":false,"context_lines":[{"line_number":498,"context_line":"                prepare_template(target\u003dself.path, *args, **kwargs)"},{"line_number":499,"context_line":"        else:"},{"line_number":500,"context_line":"            if not os.path.exists(base):"},{"line_number":501,"context_line":"                prepare_template(target\u003dbase, *args, **kwargs)"},{"line_number":502,"context_line":""},{"line_number":503,"context_line":"            # NOTE(mikal): Update the mtime of the base file so the image"},{"line_number":504,"context_line":"            # cache manager knows it is in use."}],"source_content_type":"text/x-python","patch_set":12,"id":"dada55a8_0d99d66e","line":501,"updated":"2016-07-25 14:13:43.000000000","message":"This used to be passed max_size, which it would compare against the virtual_size of the disk (via inspection with qemu-img):\n\n https://github.com/openstack/nova/blob/414df1e56ea9df700756a1732125e06c5d97d792/nova/virt/images.py#L147\n\n(prepare_template ends up being images.fetch_to_raw).\n\nThis patch is removing that check (which didn\u0027t make sense and was ignored for the generate case above) in favor of the duplicated one below.\n\nAs you can see from the review where pbrady added it, max_size was passed from fetch_to_raw() (i.e. prepare_template here) to fetch() which didn\u0027t even use it:\n\n https://review.openstack.org/#/c/54765/1/nova/virt/images.py@64\n\nThis duplication is what Matt is trying to remove here.","commit_id":"cc2ae7526b35155687a5a1abbe4f2f80618c8f87"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"25ee9d57bcdf2ed226106af8abbf634d23ad8886","unresolved":false,"context_lines":[{"line_number":503,"context_line":"            # NOTE(mikal): Update the mtime of the base file so the image"},{"line_number":504,"context_line":"            # cache manager knows it is in use."},{"line_number":505,"context_line":"            libvirt_utils.update_mtime(base)"},{"line_number":506,"context_line":"            self.verify_base_size(base, size)"},{"line_number":507,"context_line":"            if not os.path.exists(self.path):"},{"line_number":508,"context_line":"                with fileutils.remove_path_on_error(self.path):"},{"line_number":509,"context_line":"                    copy_raw_image(base, self.path, size)"}],"source_content_type":"text/x-python","patch_set":12,"id":"dada55a8_72715344","line":506,"updated":"2016-07-25 14:13:43.000000000","message":"These verify_base_size() calls are the meat of the historic fixes for the verification of the image. They were added here:\n\n https://review.openstack.org/#/c/54765/1/nova/virt/libvirt/imagebackend.py\n\nThey inspect the image for virtual_size via qemu-img and compare to size. Since these calls are in all the backends (below) we aren\u0027t losing any checking per instance.\n\nThe code in images that pbrady added was to avoid flattening a sparse or qcow image and using up more space on the compute host than the size of the root disk of the flavor we\u0027re booting. As Matt said, there\u0027s really no reason to do that, as (a) the guest can already fault out all its disk blocks causing a 2x disk space footprint and (b) the image is being cached for use by other instances anyway, which may have been booted from a usable flavor.\n\nThe compute/api checks for the image size compare the image size-on-disk to the flavor, I think, which may not be accurate if the image is sparse or CoW, which is why we need to validate the virtual_size here before we commit to giving it to the instance. I guess if glance inspected the image and exposed virtual_size to us (maybe it does now?) then we could check it earlier. I still think we probably want these validations here though, especially since they\u0027re cheap to perform.","commit_id":"cc2ae7526b35155687a5a1abbe4f2f80618c8f87"}]}