)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":22257,"name":"Manish","email":"deolalkar.manish@hpe.com","username":"manishd"},"change_message_id":"8bd0f8dbb739e1ffa87f418492afaf33093627c0","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Fix resizing in imagebackend.cache()"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Under certain conditions, create_image() will not create a base image,"},{"line_number":10,"context_line":"so we cannot rely on it to exist lateron."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change-Id: I46b5658efafe558dd6b28c9910fb8fde830adec0"},{"line_number":13,"context_line":"Closes-Bug: 1608934"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"1ac06dbe_0c749bcd","line":10,"range":{"start_line":10,"start_character":33,"end_line":10,"end_character":40},"updated":"2016-08-19 11:06:16.000000000","message":"Small Change:--later on","commit_id":"73381de73db356038ccf33d1899b28c44025a96c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"651dc789781c8b72c955cd3e1ce078cfecb329f0","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Fix resizing in imagebackend.cache()"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Under certain conditions, create_image() will not create a base image,"},{"line_number":10,"context_line":"so we cannot rely on it to exist later on."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change-Id: I46b5658efafe558dd6b28c9910fb8fde830adec0"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"fa7ab95a_14a97624","line":9,"range":{"start_line":9,"start_character":0,"end_line":9,"end_character":24},"updated":"2016-08-29 15:06:46.000000000","message":"This is perfectly obscure and should be explained in better detail in the commit message. This is basically, \u0027trust me, some things happen, and this change fixes it, but you don\u0027t really know why\u0027.","commit_id":"928637293466210c6b8f3f2be87d552dd9a10ea7"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"2725f18d034c3e35aa888fd0a5ebcf993c2f2237","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Fix resizing in imagebackend.cache()"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Under certain conditions, create_image() will not create a base image,"},{"line_number":10,"context_line":"so we cannot rely on it to exist later on."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Closes-Bug: 1608934"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"9a89bdaa_8fedca27","line":9,"updated":"2016-09-09 17:51:39.000000000","message":"I\u0027d still like more information in the commit message about what the certain conditions are. It\u0027s not clear to me from the code or the various image backends. All of the various image backends and callback methods used throughout this code makes it non-trivial to sort out what the actual failure is, so if someone can articulate that series of issues in the commit message it should be in here.","commit_id":"9eb7c9bff3c7c64984dc7b6a47b1f405dea3d523"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"14f1f771e88c27b41adcf38fa9e986bc22c69e3d","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Fix resizing in imagebackend.cache()"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Under certain conditions, create_image() will not create a base image,"},{"line_number":10,"context_line":"so we cannot rely on it to exist later on."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Closes-Bug: 1608934"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"9a89bdaa_8a785891","line":9,"in_reply_to":"9a89bdaa_8fedca27","updated":"2016-09-09 18:10:25.000000000","message":"+1, it\u0027s difficult to review this properly without a clear explanation of what the bug is and why/how this solves it. (I didn\u0027t find a concise explanation in the launchpad bug either).","commit_id":"9eb7c9bff3c7c64984dc7b6a47b1f405dea3d523"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e8a2b358e86212e4fd4a518ef38abfaadf19cb2f","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Fix resizing in imagebackend.cache()"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Under certain conditions, create_image() will not create a base image,"},{"line_number":10,"context_line":"so we cannot rely on it to exist later on."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Closes-Bug: 1608934"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"9a89bdaa_0acb6873","line":9,"in_reply_to":"9a89bdaa_8fedca27","updated":"2016-09-09 18:14:51.000000000","message":"From reading the bug, it\u0027s an issue when running with the Flat backend (raw) and ephemeral or swap, so the issue is here?\n\nhttps://github.com/openstack/nova/blob/dd9a0026e85744cbfd16690f4c44083c81a6d628/nova/virt/libvirt/imagebackend.py#L494-L496\n\nAnd looking at the cache() method, it\u0027s checking self.exists() and the base:\n\nhttps://github.com/openstack/nova/blob/dd9a0026e85744cbfd16690f4c44083c81a6d628/nova/virt/libvirt/imagebackend.py#L213\n\nSo is the failure that self.exists() returns True but os.path.exists(base) returns False, so we call create_image() but create_image() in the non-image case (ephemeral/swap) sees self.exists() is True so it doesn\u0027t fetch the image.\n\nBut then here we check for the size of the base:\n\nhttps://github.com/openstack/nova/blob/dd9a0026e85744cbfd16690f4c44083c81a6d628/nova/virt/libvirt/imagebackend.py#L218\n\nWhich doesn\u0027t exist and we fail. Is that correct?","commit_id":"9eb7c9bff3c7c64984dc7b6a47b1f405dea3d523"}],"nova/virt/libvirt/imagebackend.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ee049779362fffedee636bf48f81c0385c743fd5","unresolved":false,"context_lines":[{"line_number":218,"context_line":"                              *args, **kwargs)"},{"line_number":219,"context_line":""},{"line_number":220,"context_line":"        if size:"},{"line_number":221,"context_line":"            if (\u0027image_id\u0027 in kwargs) and (size \u003e self.get_disk_size(base)):"},{"line_number":222,"context_line":"                self.resize_image(size)"},{"line_number":223,"context_line":""},{"line_number":224,"context_line":"            if (self.preallocate and self._can_fallocate() and"}],"source_content_type":"text/x-python","patch_set":1,"id":"3ac371cc_0410924d","line":221,"updated":"2016-08-16 13:51:01.000000000","message":"No unit test for this?\n\nAlso, if this was missed before, maybe add a comment what this is checking and why.","commit_id":"1fd1d35a552db6119129a6fd49e4a83ea25e3cfa"},{"author":{"_account_id":13252,"name":"Dr. Jens Harbott","display_name":"Jens Harbott (frickler)","email":"frickler@offenerstapel.de","username":"jrosenboom"},"change_message_id":"6938a04e4be893cb4cccaeefd480149c082c8a64","unresolved":false,"context_lines":[{"line_number":218,"context_line":"                              *args, **kwargs)"},{"line_number":219,"context_line":""},{"line_number":220,"context_line":"        if size:"},{"line_number":221,"context_line":"            if (\u0027image_id\u0027 in kwargs) and (size \u003e self.get_disk_size(base)):"},{"line_number":222,"context_line":"                self.resize_image(size)"},{"line_number":223,"context_line":""},{"line_number":224,"context_line":"            if (self.preallocate and self._can_fallocate() and"}],"source_content_type":"text/x-python","patch_set":1,"id":"3ac371cc_104605bf","line":221,"in_reply_to":"3ac371cc_0410924d","updated":"2016-08-16 15:34:26.000000000","message":"I added a comment, but I have really no idea how one would do a unit test for this. The existing tests are stubbing out everything so they fail to notice this issue.\n\nA more promising approach IMHO would be setting up functional tests for the various image_types. It seems currently there is only a single functional test involving an ephemeral disk, but it only uses the default qcow2 and even that is skipped most of the time.","commit_id":"1fd1d35a552db6119129a6fd49e4a83ea25e3cfa"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"e9ff467d818f055429d076cffcc8a45fb0246a3e","unresolved":false,"context_lines":[{"line_number":218,"context_line":"                              *args, **kwargs)"},{"line_number":219,"context_line":""},{"line_number":220,"context_line":"        if size:"},{"line_number":221,"context_line":"            if (\u0027image_id\u0027 in kwargs) and (size \u003e self.get_disk_size(base)):"},{"line_number":222,"context_line":"                self.resize_image(size)"},{"line_number":223,"context_line":""},{"line_number":224,"context_line":"            if (self.preallocate and self._can_fallocate() and"}],"source_content_type":"text/x-python","patch_set":1,"id":"3ac371cc_edc81dab","line":221,"in_reply_to":"3ac371cc_104605bf","updated":"2016-08-18 17:46:05.000000000","message":"We don\u0027t need to do more obtuse things in here based on random things which may have been shoved into kwargs who knows where. I\u0027m very strongly against this.\n\nCan\u0027t we just check if base exists before measuring it? Even that is a gross hack, so it would need a big comment right here explaining why it\u0027s there.\n\nAlso, I suggest that for the benefit of backports it might be better to revert the fix which broke it and fix it properly in an update.","commit_id":"1fd1d35a552db6119129a6fd49e4a83ea25e3cfa"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"9a1f69e632d0f57973fe2d7d7700f0f5187280fe","unresolved":false,"context_lines":[{"line_number":219,"context_line":""},{"line_number":220,"context_line":"        if size:"},{"line_number":221,"context_line":"            # create_image() only creates the base image if needed, so"},{"line_number":222,"context_line":"            # we cannot rely on it to exist here"},{"line_number":223,"context_line":"            if os.path.exists(base) and size \u003e self.get_disk_size(base):"},{"line_number":224,"context_line":"                self.resize_image(size)"},{"line_number":225,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"3ac371cc_cfd7f2eb","line":222,"updated":"2016-08-19 09:34:06.000000000","message":"I\u0027d probably add to this comment that the only cases which don\u0027t create base involve Flat and Lvm ephemeral disks, neither of which need to be resized anyway.","commit_id":"73381de73db356038ccf33d1899b28c44025a96c"},{"author":{"_account_id":14819,"name":"Timofey Durakov","email":"timofei.nd@gmail.com","username":"tdurakov"},"change_message_id":"a3a76fbe534cd9029b45b0a0f824a13fb3f87db9","unresolved":false,"context_lines":[{"line_number":218,"context_line":"                              *args, **kwargs)"},{"line_number":219,"context_line":""},{"line_number":220,"context_line":"        if size:"},{"line_number":221,"context_line":"            # create_image() only creates the base image if needed, so"},{"line_number":222,"context_line":"            # we cannot rely on it to exist here"},{"line_number":223,"context_line":"            if os.path.exists(base) and size \u003e self.get_disk_size(base):"},{"line_number":224,"context_line":"                self.resize_image(size)"},{"line_number":225,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"1ac06dbe_952670b8","line":222,"range":{"start_line":221,"start_character":12,"end_line":222,"end_character":48},"updated":"2016-08-25 15:24:28.000000000","message":"I\u0027d rephrase that with smth like:\ncreation of base file depends on the image type, so\nwe cannot rely on create_image()","commit_id":"928637293466210c6b8f3f2be87d552dd9a10ea7"},{"author":{"_account_id":13252,"name":"Dr. Jens Harbott","display_name":"Jens Harbott (frickler)","email":"frickler@offenerstapel.de","username":"jrosenboom"},"change_message_id":"b074ad0fae5f29aea41849ba6fe970edf2eb1e8e","unresolved":false,"context_lines":[{"line_number":219,"context_line":""},{"line_number":220,"context_line":"        if size:"},{"line_number":221,"context_line":"            # create_image() only creates the base image if needed, so"},{"line_number":222,"context_line":"            # we cannot rely on it to exist here"},{"line_number":223,"context_line":"            if os.path.exists(base) and size \u003e self.get_disk_size(base):"},{"line_number":224,"context_line":"                self.resize_image(size)"},{"line_number":225,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"1ac06dbe_7ef1116a","line":222,"in_reply_to":"1ac06dbe_952670b8","updated":"2016-08-25 16:14:27.000000000","message":"That would only be half true: For raw images, the base image does get created if an image_id is specified. For lvm, the base image gets created unless ephemeral_size is specified.","commit_id":"928637293466210c6b8f3f2be87d552dd9a10ea7"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"678e7c6ec0fd0344c345b155137922d5287fbefa","unresolved":false,"context_lines":[{"line_number":220,"context_line":"        if size:"},{"line_number":221,"context_line":"            # create_image() only creates the base image if needed, so"},{"line_number":222,"context_line":"            # we cannot rely on it to exist here"},{"line_number":223,"context_line":"            if os.path.exists(base) and size \u003e self.get_disk_size(base):"},{"line_number":224,"context_line":"                self.resize_image(size)"},{"line_number":225,"context_line":""},{"line_number":226,"context_line":"            if (self.preallocate and self._can_fallocate() and"}],"source_content_type":"text/x-python","patch_set":6,"id":"9a89bdaa_4ff9024d","line":223,"updated":"2016-09-09 17:58:23.000000000","message":"Should this be self.exists()? For something like rbd (which will still run this code, AFAICT), the base path is not a file right?","commit_id":"9eb7c9bff3c7c64984dc7b6a47b1f405dea3d523"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"cb1f3e035366d3032d6eca822ce4903a3ef279ad","unresolved":false,"context_lines":[{"line_number":220,"context_line":"        if size:"},{"line_number":221,"context_line":"            # create_image() only creates the base image if needed, so"},{"line_number":222,"context_line":"            # we cannot rely on it to exist here"},{"line_number":223,"context_line":"            if os.path.exists(base) and size \u003e self.get_disk_size(base):"},{"line_number":224,"context_line":"                self.resize_image(size)"},{"line_number":225,"context_line":""},{"line_number":226,"context_line":"            if (self.preallocate and self._can_fallocate() and"}],"source_content_type":"text/x-python","patch_set":6,"id":"9a89bdaa_654c2777","line":223,"in_reply_to":"9a89bdaa_4ff9024d","updated":"2016-09-09 18:43:21.000000000","message":"Matt beat me to it, but here\u0027s what I was just typing nonetheless.\n\nFor Rbd, those variables look something like the following, (when `disk_name\u003ddisk`):\n\n    base: instances/_base/sha1-image-uuid\n\n    path: rbd:FakePool/instance-uuid_disk:id\u003dFakeUser:conf\u003dFakeConf\n\n    rbd_name: instance-uuid_disk\n\nThe `path` variables are the ones that aren\u0027t always filesystem-y (for lack of a better word). The `base` variables are always filesystem-y.\n\nFor comparison, here\u0027s what other backends would look like:\n\n- LVM:\n\n    base: instances/_base/sha1-image-uuid\n\n    path: /dev/some-volume-group/instance-uuid_disk\n\n- Qcow2/Flat:\n\n    base: instances/_base/sha1-image-uuid\n\n    path: instances/instance-uuid/disk\n\n- Ploop:\n\n    base: instances/_base/sha1-image-uuid\n\n    path: instances/instance-uuid/disk\n\n    target: instances/instance-uuid/disk/root.hds\n\n\nPS. In only know this because I found it really confusing to follow, so I made myself a cheat-sheet a while back.","commit_id":"9eb7c9bff3c7c64984dc7b6a47b1f405dea3d523"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"6cb843d37dd65e49d4c5c987aec70ecfc2e69b45","unresolved":false,"context_lines":[{"line_number":220,"context_line":"        if size:"},{"line_number":221,"context_line":"            # create_image() only creates the base image if needed, so"},{"line_number":222,"context_line":"            # we cannot rely on it to exist here"},{"line_number":223,"context_line":"            if os.path.exists(base) and size \u003e self.get_disk_size(base):"},{"line_number":224,"context_line":"                self.resize_image(size)"},{"line_number":225,"context_line":""},{"line_number":226,"context_line":"            if (self.preallocate and self._can_fallocate() and"}],"source_content_type":"text/x-python","patch_set":6,"id":"9a89bdaa_0af52882","line":223,"in_reply_to":"9a89bdaa_4ff9024d","updated":"2016-09-09 18:21:02.000000000","message":"No, self.exists() would test the overlay, not the backing file.\n\nThis whole thing is gross (plug: and fixed by my refactor series). When we create a disk from *image*, the base file will be the size of the image, so what we\u0027re really testing here is whether we\u0027re creating a root disk which is bigger than its backing image.\n\nIf we\u0027re creating an ephemeral or swap disk, the backing file, if it is created at all, will always be the exact size originally requested. So in that code path this resize isn\u0027t required at all.\n\nThis is a Minimal Gross Hack(tm) which will cause it to work correctly in all cases. The better fix is obviously to treat these 2 code paths differently, but that\u0027s pretty invasive.","commit_id":"9eb7c9bff3c7c64984dc7b6a47b1f405dea3d523"}]}