)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":10224,"name":"Feodor Tersin","email":"ftersin@hotmail.com","username":"ftersin"},"change_message_id":"12a1c3cfe7345cbbb6dea779decf5cbc0b889e51","unresolved":false,"context_lines":[{"line_number":45,"context_line":"that volume size isn\u0027t checked against flavor disk size, and one to"},{"line_number":46,"context_line":"check that volume size is checked against image metadata min_disk."},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"Closes-Bug: #1457517"},{"line_number":49,"context_line":"Change-Id: I264493172da20b664df571e32876030246c2a87c"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"3a50d1a3_c5978246","line":48,"updated":"2015-07-22 16:30:09.000000000","message":"Also these bugs are closed:\n\n Partial-Bug: #1469179\n Closes-Bug: #1459491\n Closes-Bug: #1466305","commit_id":"18f17b3af99977211bec0c9343f322a16ebfd66f"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"56b3d479a3795c61e08ff377c70c1b2b1260f0d0","unresolved":false,"context_lines":[{"line_number":45,"context_line":"that volume size isn\u0027t checked against flavor disk size, and one to"},{"line_number":46,"context_line":"check that volume size is checked against image metadata min_disk."},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"Closes-Bug: #1457517"},{"line_number":49,"context_line":"Change-Id: I264493172da20b664df571e32876030246c2a87c"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"3a50d1a3_cbdf9367","line":48,"in_reply_to":"3a50d1a3_c5978246","updated":"2015-07-22 16:46:02.000000000","message":"Excellent sleuthing, thanks!","commit_id":"18f17b3af99977211bec0c9343f322a16ebfd66f"}],"nova/compute/api.py":[{"author":{"_account_id":10224,"name":"Feodor Tersin","email":"ftersin@hotmail.com","username":"ftersin"},"change_message_id":"12a1c3cfe7345cbbb6dea779decf5cbc0b889e51","unresolved":false,"context_lines":[{"line_number":679,"context_line":"            # drivers. A value of 0 means don\u0027t check size."},{"line_number":680,"context_line":"            flavor_size \u003d (instance_type[\u0027root_gb\u0027] or 0) * (1024 ** 3)"},{"line_number":681,"context_line":"            if flavor_size !\u003d 0:"},{"line_number":682,"context_line":"                if flavor_size \u003c image_size:"},{"line_number":683,"context_line":"                    # The flavor disk is too small to contain the image"},{"line_number":684,"context_line":"                    raise exception.FlavorDiskSmallerThanImage("},{"line_number":685,"context_line":"                        flavor_size\u003dflavor_size, image_size\u003dimage_size)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a50d1a3_48b869c8","line":682,"updated":"2015-07-22 16:30:09.000000000","message":"This will fail in case of booting from volume-backed image. See also bug 1369854","commit_id":"18f17b3af99977211bec0c9343f322a16ebfd66f"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"53f2ba12842a0943545618f579bf6c5eee261ceb","unresolved":false,"context_lines":[{"line_number":679,"context_line":"            # drivers. A value of 0 means don\u0027t check size."},{"line_number":680,"context_line":"            flavor_size \u003d (instance_type[\u0027root_gb\u0027] or 0) * (1024 ** 3)"},{"line_number":681,"context_line":"            if flavor_size !\u003d 0:"},{"line_number":682,"context_line":"                if flavor_size \u003c image_size:"},{"line_number":683,"context_line":"                    # The flavor disk is too small to contain the image"},{"line_number":684,"context_line":"                    raise exception.FlavorDiskSmallerThanImage("},{"line_number":685,"context_line":"                        flavor_size\u003dflavor_size, image_size\u003dimage_size)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a50d1a3_718f1e75","line":682,"in_reply_to":"3a50d1a3_3101c405","updated":"2015-07-23 11:46:20.000000000","message":"I think the \u0027image_id is not None\u0027 check can just be replaced by \"is_volume_backed_instance\" check to deal with this.","commit_id":"18f17b3af99977211bec0c9343f322a16ebfd66f"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"56b3d479a3795c61e08ff377c70c1b2b1260f0d0","unresolved":false,"context_lines":[{"line_number":679,"context_line":"            # drivers. A value of 0 means don\u0027t check size."},{"line_number":680,"context_line":"            flavor_size \u003d (instance_type[\u0027root_gb\u0027] or 0) * (1024 ** 3)"},{"line_number":681,"context_line":"            if flavor_size !\u003d 0:"},{"line_number":682,"context_line":"                if flavor_size \u003c image_size:"},{"line_number":683,"context_line":"                    # The flavor disk is too small to contain the image"},{"line_number":684,"context_line":"                    raise exception.FlavorDiskSmallerThanImage("},{"line_number":685,"context_line":"                        flavor_size\u003dflavor_size, image_size\u003dimage_size)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a50d1a3_eb95af86","line":682,"in_reply_to":"3a50d1a3_48b869c8","updated":"2015-07-22 16:46:02.000000000","message":"This code is not executed if we are booting from a volume. See the comments both above and below around the guard \u0027if image_id is not None\u0027.","commit_id":"18f17b3af99977211bec0c9343f322a16ebfd66f"},{"author":{"_account_id":10224,"name":"Feodor Tersin","email":"ftersin@hotmail.com","username":"ftersin"},"change_message_id":"2bb863e5e8a4626cf6aabf700b222a81683c9137","unresolved":false,"context_lines":[{"line_number":679,"context_line":"            # drivers. A value of 0 means don\u0027t check size."},{"line_number":680,"context_line":"            flavor_size \u003d (instance_type[\u0027root_gb\u0027] or 0) * (1024 ** 3)"},{"line_number":681,"context_line":"            if flavor_size !\u003d 0:"},{"line_number":682,"context_line":"                if flavor_size \u003c image_size:"},{"line_number":683,"context_line":"                    # The flavor disk is too small to contain the image"},{"line_number":684,"context_line":"                    raise exception.FlavorDiskSmallerThanImage("},{"line_number":685,"context_line":"                        flavor_size\u003dflavor_size, image_size\u003dimage_size)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a50d1a3_3101c405","line":682,"in_reply_to":"3a50d1a3_b615ca40","updated":"2015-07-22 17:18:33.000000000","message":"See also https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/servers.py#L1095\n\nThat \u0027else\u0027 branch is related to volume-backed image with image_ref defined.","commit_id":"18f17b3af99977211bec0c9343f322a16ebfd66f"},{"author":{"_account_id":10224,"name":"Feodor Tersin","email":"ftersin@hotmail.com","username":"ftersin"},"change_message_id":"2b60f428d6a5d390b1f55e18f38f52f21518828e","unresolved":false,"context_lines":[{"line_number":679,"context_line":"            # drivers. A value of 0 means don\u0027t check size."},{"line_number":680,"context_line":"            flavor_size \u003d (instance_type[\u0027root_gb\u0027] or 0) * (1024 ** 3)"},{"line_number":681,"context_line":"            if flavor_size !\u003d 0:"},{"line_number":682,"context_line":"                if flavor_size \u003c image_size:"},{"line_number":683,"context_line":"                    # The flavor disk is too small to contain the image"},{"line_number":684,"context_line":"                    raise exception.FlavorDiskSmallerThanImage("},{"line_number":685,"context_line":"                        flavor_size\u003dflavor_size, image_size\u003dimage_size)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a50d1a3_b615ca40","line":682,"in_reply_to":"3a50d1a3_eb95af86","updated":"2015-07-22 16:59:24.000000000","message":"An instance can be booted from a volume-backed image. In this case image_id is not None. See the last (boot from a volume-backed image) part in https://review.openstack.org/#/c/196569/4/nova/tests/unit/compute/test_compute.py","commit_id":"18f17b3af99977211bec0c9343f322a16ebfd66f"},{"author":{"_account_id":10224,"name":"Feodor Tersin","email":"ftersin@hotmail.com","username":"ftersin"},"change_message_id":"2364d7848d764a291b2a92911cc4437705601c4b","unresolved":false,"context_lines":[{"line_number":785,"context_line":"                                       files_to_inject, is_vol_boot):"},{"line_number":786,"context_line":"        self._check_metadata_properties_quota(context, metadata)"},{"line_number":787,"context_line":"        self._check_injected_file_quota(context, files_to_inject)"},{"line_number":788,"context_line":"        if not is_vol_boot:"},{"line_number":789,"context_line":"            self._check_requested_image(context, image_id, image,"},{"line_number":790,"context_line":"                                        instance_type)"},{"line_number":791,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_0cc80a54","line":788,"updated":"2015-07-29 12:13:10.000000000","message":"This skips some useful checks, like min_ram","commit_id":"235a88f153ce30d61068a70fdc0368ca3829f0eb"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"107741688445f30827f17818727c61b730e566c3","unresolved":false,"context_lines":[{"line_number":785,"context_line":"                                       files_to_inject, is_vol_boot):"},{"line_number":786,"context_line":"        self._check_metadata_properties_quota(context, metadata)"},{"line_number":787,"context_line":"        self._check_injected_file_quota(context, files_to_inject)"},{"line_number":788,"context_line":"        if not is_vol_boot:"},{"line_number":789,"context_line":"            self._check_requested_image(context, image_id, image,"},{"line_number":790,"context_line":"                                        instance_type)"},{"line_number":791,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"1a4dcd0f_ca717be2","line":788,"in_reply_to":"3a50d1a3_0cc80a54","updated":"2015-08-03 15:10:24.000000000","message":"I\u0027ve proposed a new version which puts these back, and handles the case when creating a new volume at boot time with a bdm.","commit_id":"235a88f153ce30d61068a70fdc0368ca3829f0eb"},{"author":{"_account_id":10224,"name":"Feodor Tersin","email":"ftersin@hotmail.com","username":"ftersin"},"change_message_id":"2364d7848d764a291b2a92911cc4437705601c4b","unresolved":false,"context_lines":[{"line_number":1077,"context_line":""},{"line_number":1078,"context_line":"        is_vol_boot \u003d False"},{"line_number":1079,"context_line":"        if image_href:"},{"line_number":1080,"context_line":"            image_id, boot_meta \u003d self._get_image(context, image_href)"},{"line_number":1081,"context_line":"        else:"},{"line_number":1082,"context_line":"            image_id \u003d None"},{"line_number":1083,"context_line":"            (boot_meta, is_vol_boot) \u003d self._get_bdm_image_metadata("}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_4cfff265","line":1080,"updated":"2015-07-29 12:13:10.000000000","message":"is_vol_boot should be set to True for a volume-backed image","commit_id":"235a88f153ce30d61068a70fdc0368ca3829f0eb"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"000eff79bbcdef20592892af1359e75a613144ed","unresolved":false,"context_lines":[{"line_number":1020,"context_line":""},{"line_number":1021,"context_line":"        Returns a two element tuple, the first element contains"},{"line_number":1022,"context_line":"        the image metadata and the second element indicates"},{"line_number":1023,"context_line":"        whether the config represents boot-from-volume"},{"line_number":1024,"context_line":"        \"\"\""},{"line_number":1025,"context_line":"        if not block_device_mapping:"},{"line_number":1026,"context_line":"            return {}, None"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a4dcd0f_1da6c6c1","line":1023,"updated":"2015-08-03 16:44:26.000000000","message":"You forgot to update the comment here to say it returns the BDM now","commit_id":"a7d34122192d227c638280439ec14f267cdce9f9"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"b39aa10ba49da048f65bb50d9987618b13b46dfd","unresolved":false,"context_lines":[{"line_number":1020,"context_line":""},{"line_number":1021,"context_line":"        Returns a two element tuple, the first element contains"},{"line_number":1022,"context_line":"        the image metadata and the second element indicates"},{"line_number":1023,"context_line":"        whether the config represents boot-from-volume"},{"line_number":1024,"context_line":"        \"\"\""},{"line_number":1025,"context_line":"        if not block_device_mapping:"},{"line_number":1026,"context_line":"            return {}, None"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a4dcd0f_dd393e43","line":1023,"in_reply_to":"1a4dcd0f_1da6c6c1","updated":"2015-08-04 11:15:33.000000000","message":"Done","commit_id":"a7d34122192d227c638280439ec14f267cdce9f9"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"851cc732943ef4ea982e53513e750a85528e6683","unresolved":false,"context_lines":[{"line_number":666,"context_line":""},{"line_number":667,"context_line":"        # Image min_disk is in gb, size is in bytes. For sanity, have them both"},{"line_number":668,"context_line":"        # in bytes."},{"line_number":669,"context_line":"        image_min_disk \u003d int(image.get(\u0027min_disk\u0027) or 0) * (1024 ** 3)"},{"line_number":670,"context_line":"        image_size \u003d int(image.get(\u0027size\u0027) or 0)"},{"line_number":671,"context_line":""},{"line_number":672,"context_line":"        # N.B. root_bdm may be either a BlockDeviceDict or a BlockDeviceMapping"}],"source_content_type":"text/x-python","patch_set":7,"id":"1a4dcd0f_205332e3","line":669,"updated":"2015-08-04 11:21:14.000000000","message":"Use units.Gi instead of 1024 ** 3","commit_id":"0b2079f13f16fe1e58684b53d9858a7e3c6847c9"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"3b791b77b1fdb5db85125ac1c938ba0da73b0d0f","unresolved":false,"context_lines":[{"line_number":666,"context_line":""},{"line_number":667,"context_line":"        # Image min_disk is in gb, size is in bytes. For sanity, have them both"},{"line_number":668,"context_line":"        # in bytes."},{"line_number":669,"context_line":"        image_min_disk \u003d int(image.get(\u0027min_disk\u0027) or 0) * (1024 ** 3)"},{"line_number":670,"context_line":"        image_size \u003d int(image.get(\u0027size\u0027) or 0)"},{"line_number":671,"context_line":""},{"line_number":672,"context_line":"        # N.B. root_bdm may be either a BlockDeviceDict or a BlockDeviceMapping"}],"source_content_type":"text/x-python","patch_set":7,"id":"1a4dcd0f_6075bac9","line":669,"in_reply_to":"1a4dcd0f_205332e3","updated":"2015-08-04 11:32:01.000000000","message":"Yeah, I didn\u0027t use it because it wasn\u0027t there before, which was lazy. I\u0027ll fix it.","commit_id":"0b2079f13f16fe1e58684b53d9858a7e3c6847c9"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"851cc732943ef4ea982e53513e750a85528e6683","unresolved":false,"context_lines":[{"line_number":684,"context_line":"                assert(volume_id is not None)"},{"line_number":685,"context_line":"                volume \u003d self.volume_api.get(context, volume_id)"},{"line_number":686,"context_line":"                dest_size \u003d volume[\u0027size\u0027]"},{"line_number":687,"context_line":"            dest_size *\u003d 1024 ** 3"},{"line_number":688,"context_line":""},{"line_number":689,"context_line":"            if image_min_disk \u003e dest_size:"},{"line_number":690,"context_line":"                # This is exception is wrong, but it\u0027s the exception which"}],"source_content_type":"text/x-python","patch_set":7,"id":"1a4dcd0f_e03c6a95","line":687,"updated":"2015-08-04 11:21:14.000000000","message":"Use units.Gi instead of 1024 ** 3","commit_id":"0b2079f13f16fe1e58684b53d9858a7e3c6847c9"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"851cc732943ef4ea982e53513e750a85528e6683","unresolved":false,"context_lines":[{"line_number":694,"context_line":""},{"line_number":695,"context_line":"        # Target disk is a local disk whose size is taken from the flavor"},{"line_number":696,"context_line":"        else:"},{"line_number":697,"context_line":"            dest_size \u003d instance_type[\u0027root_gb\u0027] * (1024 ** 3)"},{"line_number":698,"context_line":""},{"line_number":699,"context_line":"            # NOTE(johannes): root_gb is allowed to be 0 for legacy reasons"},{"line_number":700,"context_line":"            # since libvirt interpreted the value differently than other"}],"source_content_type":"text/x-python","patch_set":7,"id":"1a4dcd0f_a0466221","line":697,"updated":"2015-08-04 11:21:14.000000000","message":"Use units.Gi instead of 1024 ** 3","commit_id":"0b2079f13f16fe1e58684b53d9858a7e3c6847c9"},{"author":{"_account_id":10224,"name":"Feodor Tersin","email":"ftersin@hotmail.com","username":"ftersin"},"change_message_id":"7211213b8832753824e8e1e991103562a289b792","unresolved":false,"context_lines":[{"line_number":681,"context_line":"            if dest_size is None:"},{"line_number":682,"context_line":"                # The only circumstance we handle is when the size hasn\u0027t been"},{"line_number":683,"context_line":"                # fetched for an existing volume"},{"line_number":684,"context_line":"                volume_id \u003d root_bdm[\u0027volume_id\u0027]"},{"line_number":685,"context_line":"                assert(volume_id is not None)"},{"line_number":686,"context_line":"                volume \u003d self.volume_api.get(context, volume_id)"},{"line_number":687,"context_line":"                dest_size \u003d volume[\u0027size\u0027]"}],"source_content_type":"text/x-python","patch_set":8,"id":"1a4dcd0f_6b96b140","line":684,"updated":"2015-08-05 13:32:50.000000000","message":"we can boot from a snapshot, so this will raise error","commit_id":"c3661f87bee0707ddb46dc5b7f7ff53c693db1b0"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"06d186f1cc77827029b8cb7eea513598198d5b57","unresolved":false,"context_lines":[{"line_number":681,"context_line":"            if dest_size is None:"},{"line_number":682,"context_line":"                # The only circumstance we handle is when the size hasn\u0027t been"},{"line_number":683,"context_line":"                # fetched for an existing volume"},{"line_number":684,"context_line":"                volume_id \u003d root_bdm[\u0027volume_id\u0027]"},{"line_number":685,"context_line":"                assert(volume_id is not None)"},{"line_number":686,"context_line":"                volume \u003d self.volume_api.get(context, volume_id)"},{"line_number":687,"context_line":"                dest_size \u003d volume[\u0027size\u0027]"}],"source_content_type":"text/x-python","patch_set":8,"id":"1a4dcd0f_d4976246","line":684,"in_reply_to":"1a4dcd0f_6b96b140","updated":"2015-08-05 14:13:49.000000000","message":"I\u0027ll look in to this, thanks. Strange this hasn\u0027t raised any errors in the gate.","commit_id":"c3661f87bee0707ddb46dc5b7f7ff53c693db1b0"},{"author":{"_account_id":10224,"name":"Feodor Tersin","email":"ftersin@hotmail.com","username":"ftersin"},"change_message_id":"5d077f6faed6d6ae85e3c89473f1c2f2cd1574ff","unresolved":false,"context_lines":[{"line_number":681,"context_line":"            if dest_size is None:"},{"line_number":682,"context_line":"                # The only circumstance we handle is when the size hasn\u0027t been"},{"line_number":683,"context_line":"                # fetched for an existing volume"},{"line_number":684,"context_line":"                volume_id \u003d root_bdm[\u0027volume_id\u0027]"},{"line_number":685,"context_line":"                assert(volume_id is not None)"},{"line_number":686,"context_line":"                volume \u003d self.volume_api.get(context, volume_id)"},{"line_number":687,"context_line":"                dest_size \u003d volume[\u0027size\u0027]"}],"source_content_type":"text/x-python","patch_set":8,"id":"1a4dcd0f_37f1e301","line":684,"in_reply_to":"1a4dcd0f_9aab4175","updated":"2015-08-05 15:54:16.000000000","message":"tests poor covers volume-backed cases, critical bugs often appear after changing of related code\n\nthat is why i proposed to add an integrated test case","commit_id":"c3661f87bee0707ddb46dc5b7f7ff53c693db1b0"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"77f11b0cf54d700d5a5c2c7d2748d03af7a46aea","unresolved":false,"context_lines":[{"line_number":681,"context_line":"            if dest_size is None:"},{"line_number":682,"context_line":"                # The only circumstance we handle is when the size hasn\u0027t been"},{"line_number":683,"context_line":"                # fetched for an existing volume"},{"line_number":684,"context_line":"                volume_id \u003d root_bdm[\u0027volume_id\u0027]"},{"line_number":685,"context_line":"                assert(volume_id is not None)"},{"line_number":686,"context_line":"                volume \u003d self.volume_api.get(context, volume_id)"},{"line_number":687,"context_line":"                dest_size \u003d volume[\u0027size\u0027]"}],"source_content_type":"text/x-python","patch_set":8,"id":"1a4dcd0f_9aab4175","line":684,"in_reply_to":"1a4dcd0f_d4976246","updated":"2015-08-05 14:41:25.000000000","message":"Confirmed.","commit_id":"c3661f87bee0707ddb46dc5b7f7ff53c693db1b0"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"a9bbe5f11825f690cd1c11237bfbe11cd98abee9","unresolved":false,"context_lines":[{"line_number":682,"context_line":"                # The only circumstance we handle is when the size hasn\u0027t been"},{"line_number":683,"context_line":"                # fetched for an existing volume"},{"line_number":684,"context_line":"                volume_id \u003d root_bdm[\u0027volume_id\u0027]"},{"line_number":685,"context_line":"                assert(volume_id is not None)"},{"line_number":686,"context_line":"                volume \u003d self.volume_api.get(context, volume_id)"},{"line_number":687,"context_line":"                dest_size \u003d volume[\u0027size\u0027]"},{"line_number":688,"context_line":"            dest_size *\u003d units.Gi"}],"source_content_type":"text/x-python","patch_set":8,"id":"1a4dcd0f_9c9b7c72","line":685,"updated":"2015-08-04 15:38:13.000000000","message":"do we need this assertion here?\nif this is important then we should raise a valid exception","commit_id":"c3661f87bee0707ddb46dc5b7f7ff53c693db1b0"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"7e0c1432a3eb415f5a5081cbceeae32633ae1e03","unresolved":false,"context_lines":[{"line_number":682,"context_line":"                # The only circumstance we handle is when the size hasn\u0027t been"},{"line_number":683,"context_line":"                # fetched for an existing volume"},{"line_number":684,"context_line":"                volume_id \u003d root_bdm[\u0027volume_id\u0027]"},{"line_number":685,"context_line":"                assert(volume_id is not None)"},{"line_number":686,"context_line":"                volume \u003d self.volume_api.get(context, volume_id)"},{"line_number":687,"context_line":"                dest_size \u003d volume[\u0027size\u0027]"},{"line_number":688,"context_line":"            dest_size *\u003d units.Gi"}],"source_content_type":"text/x-python","patch_set":8,"id":"1a4dcd0f_ff41520c","line":685,"in_reply_to":"1a4dcd0f_9c9b7c72","updated":"2015-08-04 15:54:21.000000000","message":"This is a pure assertion: it should never happen. If it ever happened it would be a programming error, and there would be nothing the user could do about it. However, if it did happen, the programmer would want to know about it at this point in the stack, rather than at some random subsquent point. I\u0027m happy with the choice of assert here.","commit_id":"c3661f87bee0707ddb46dc5b7f7ff53c693db1b0"},{"author":{"_account_id":10224,"name":"Feodor Tersin","email":"ftersin@hotmail.com","username":"ftersin"},"change_message_id":"7211213b8832753824e8e1e991103562a289b792","unresolved":false,"context_lines":[{"line_number":687,"context_line":"                dest_size \u003d volume[\u0027size\u0027]"},{"line_number":688,"context_line":"            dest_size *\u003d units.Gi"},{"line_number":689,"context_line":""},{"line_number":690,"context_line":"            if image_min_disk \u003e dest_size:"},{"line_number":691,"context_line":"                # This is exception is wrong, but it\u0027s the exception which"},{"line_number":692,"context_line":"                # calling code expects this function to raise. Due to the churn"},{"line_number":693,"context_line":"                # it causes this will be fixed up in a subsequent patch."}],"source_content_type":"text/x-python","patch_set":8,"id":"1a4dcd0f_8b477dbb","line":690,"updated":"2015-08-05 13:32:50.000000000","message":"this check has no sense because the volume size is not lesser than min_disk, because Cinder desn\u0027t allow to create volume from an image with size lesser than min_disk","commit_id":"c3661f87bee0707ddb46dc5b7f7ff53c693db1b0"},{"author":{"_account_id":10224,"name":"Feodor Tersin","email":"ftersin@hotmail.com","username":"ftersin"},"change_message_id":"5d077f6faed6d6ae85e3c89473f1c2f2cd1574ff","unresolved":false,"context_lines":[{"line_number":687,"context_line":"                dest_size \u003d volume[\u0027size\u0027]"},{"line_number":688,"context_line":"            dest_size *\u003d units.Gi"},{"line_number":689,"context_line":""},{"line_number":690,"context_line":"            if image_min_disk \u003e dest_size:"},{"line_number":691,"context_line":"                # This is exception is wrong, but it\u0027s the exception which"},{"line_number":692,"context_line":"                # calling code expects this function to raise. Due to the churn"},{"line_number":693,"context_line":"                # it causes this will be fixed up in a subsequent patch."}],"source_content_type":"text/x-python","patch_set":8,"id":"1a4dcd0f_4a23a6a7","line":690,"in_reply_to":"1a4dcd0f_5165a41b","updated":"2015-08-05 15:54:16.000000000","message":"In this case Nova works as a proxy. Glance handles the image, Cinder handles the volume (creates with checks, stores metadata). Nova just passes parameters.\n\nSince Cinder provides a complex solution for store data with metadta, and with internal checks of consitency, Nova can only double its logic (like store and check min_disk), rather than doing something unique which neither Cinder, nor Glance do. But this requires additional complex code (frankly, your solution is not very simple, and adds complexity to internal api). What is a stong reason to do this?","commit_id":"c3661f87bee0707ddb46dc5b7f7ff53c693db1b0"},{"author":{"_account_id":10224,"name":"Feodor Tersin","email":"ftersin@hotmail.com","username":"ftersin"},"change_message_id":"45087f8778a189e1b76723ef827232694197bdd8","unresolved":false,"context_lines":[{"line_number":687,"context_line":"                dest_size \u003d volume[\u0027size\u0027]"},{"line_number":688,"context_line":"            dest_size *\u003d units.Gi"},{"line_number":689,"context_line":""},{"line_number":690,"context_line":"            if image_min_disk \u003e dest_size:"},{"line_number":691,"context_line":"                # This is exception is wrong, but it\u0027s the exception which"},{"line_number":692,"context_line":"                # calling code expects this function to raise. Due to the churn"},{"line_number":693,"context_line":"                # it causes this will be fixed up in a subsequent patch."}],"source_content_type":"text/x-python","patch_set":8,"id":"1a4dcd0f_8ebc6b07","line":690,"in_reply_to":"1a4dcd0f_8b477dbb","updated":"2015-08-05 13:56:06.000000000","message":"if we create a new volume, Cinder checks the same later and raise an error by itself","commit_id":"c3661f87bee0707ddb46dc5b7f7ff53c693db1b0"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"06d186f1cc77827029b8cb7eea513598198d5b57","unresolved":false,"context_lines":[{"line_number":687,"context_line":"                dest_size \u003d volume[\u0027size\u0027]"},{"line_number":688,"context_line":"            dest_size *\u003d units.Gi"},{"line_number":689,"context_line":""},{"line_number":690,"context_line":"            if image_min_disk \u003e dest_size:"},{"line_number":691,"context_line":"                # This is exception is wrong, but it\u0027s the exception which"},{"line_number":692,"context_line":"                # calling code expects this function to raise. Due to the churn"},{"line_number":693,"context_line":"                # it causes this will be fixed up in a subsequent patch."}],"source_content_type":"text/x-python","patch_set":8,"id":"1a4dcd0f_5165a41b","line":690,"in_reply_to":"1a4dcd0f_8ebc6b07","updated":"2015-08-05 14:13:49.000000000","message":"Not if nova is creating the volume because we specified a bdm on the command line, because in that case the volume hasn\u0027t been created yet.\n\nAlso, I\u0027m happy with the belt and braces approach anyway.","commit_id":"c3661f87bee0707ddb46dc5b7f7ff53c693db1b0"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"a9bbe5f11825f690cd1c11237bfbe11cd98abee9","unresolved":false,"context_lines":[{"line_number":700,"context_line":"            # NOTE(johannes): root_gb is allowed to be 0 for legacy reasons"},{"line_number":701,"context_line":"            # since libvirt interpreted the value differently than other"},{"line_number":702,"context_line":"            # drivers. A value of 0 means don\u0027t check size."},{"line_number":703,"context_line":"            if dest_size !\u003d 0:"},{"line_number":704,"context_line":"                if image_size \u003e dest_size:"},{"line_number":705,"context_line":"                    raise exception.FlavorDiskTooSmall()"},{"line_number":706,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"1a4dcd0f_fce720e2","line":703,"updated":"2015-08-04 15:38:13.000000000","message":"if (dest_size !\u003d 0 and\n    (image_size \u003e dest_size or image_min_disk \u003e dest_size)):\n    raise exception.FlavorDiskTooSmall()","commit_id":"c3661f87bee0707ddb46dc5b7f7ff53c693db1b0"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"7e0c1432a3eb415f5a5081cbceeae32633ae1e03","unresolved":false,"context_lines":[{"line_number":700,"context_line":"            # NOTE(johannes): root_gb is allowed to be 0 for legacy reasons"},{"line_number":701,"context_line":"            # since libvirt interpreted the value differently than other"},{"line_number":702,"context_line":"            # drivers. A value of 0 means don\u0027t check size."},{"line_number":703,"context_line":"            if dest_size !\u003d 0:"},{"line_number":704,"context_line":"                if image_size \u003e dest_size:"},{"line_number":705,"context_line":"                    raise exception.FlavorDiskTooSmall()"},{"line_number":706,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"1a4dcd0f_9fffeea9","line":703,"in_reply_to":"1a4dcd0f_fce720e2","updated":"2015-08-04 15:54:21.000000000","message":"Look at the next patch in the series: these are actually 2 different exceptions. Also, this is clearer. Screen space is cheap :)","commit_id":"c3661f87bee0707ddb46dc5b7f7ff53c693db1b0"},{"author":{"_account_id":10224,"name":"Feodor Tersin","email":"ftersin@hotmail.com","username":"ftersin"},"change_message_id":"7211213b8832753824e8e1e991103562a289b792","unresolved":false,"context_lines":[{"line_number":1111,"context_line":""},{"line_number":1112,"context_line":"        root_bdm \u003d None"},{"line_number":1113,"context_line":"        if image_href:"},{"line_number":1114,"context_line":"            image_id, boot_meta \u003d self._get_image(context, image_href)"},{"line_number":1115,"context_line":"        else:"},{"line_number":1116,"context_line":"            image_id \u003d None"},{"line_number":1117,"context_line":"            (boot_meta, root_bdm) \u003d self._get_bdm_image_metadata("}],"source_content_type":"text/x-python","patch_set":8,"id":"1a4dcd0f_2b5d0979","line":1114,"updated":"2015-08-05 13:32:50.000000000","message":"launch from a volume-backed image is still not considered","commit_id":"c3661f87bee0707ddb46dc5b7f7ff53c693db1b0"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"a73204823db33486acbd17b81c4a29ac8c06d6fe","unresolved":false,"context_lines":[{"line_number":1111,"context_line":""},{"line_number":1112,"context_line":"        root_bdm \u003d None"},{"line_number":1113,"context_line":"        if image_href:"},{"line_number":1114,"context_line":"            image_id, boot_meta \u003d self._get_image(context, image_href)"},{"line_number":1115,"context_line":"        else:"},{"line_number":1116,"context_line":"            image_id \u003d None"},{"line_number":1117,"context_line":"            (boot_meta, root_bdm) \u003d self._get_bdm_image_metadata("}],"source_content_type":"text/x-python","patch_set":8,"id":"1a4dcd0f_ed3c0cca","line":1114,"in_reply_to":"1a4dcd0f_05d116fa","updated":"2015-08-05 16:05:42.000000000","message":"I don\u0027t think that test is valid. Can you give me a command line or api call which would result in both image_href being set here, and a bdm existing with a boot_index of 0?\n\nIt doesn\u0027t make sense in any case, and there\u0027s a specific check for it in _check_and_transform_bdm(). However, that check only checks the legacy \u0027device_name\u0027 field, which may mean it misses v2 bdms, which I guess would be a bug.\n\nIn any case, this is fine. If image_href is not None here, it means we are creating a local root disk, and volume checks are not relevant.","commit_id":"c3661f87bee0707ddb46dc5b7f7ff53c693db1b0"},{"author":{"_account_id":10224,"name":"Feodor Tersin","email":"ftersin@hotmail.com","username":"ftersin"},"change_message_id":"3cacbaa4875b061e6eac21b616363a0bf4e8343b","unresolved":false,"context_lines":[{"line_number":1111,"context_line":""},{"line_number":1112,"context_line":"        root_bdm \u003d None"},{"line_number":1113,"context_line":"        if image_href:"},{"line_number":1114,"context_line":"            image_id, boot_meta \u003d self._get_image(context, image_href)"},{"line_number":1115,"context_line":"        else:"},{"line_number":1116,"context_line":"            image_id \u003d None"},{"line_number":1117,"context_line":"            (boot_meta, root_bdm) \u003d self._get_bdm_image_metadata("}],"source_content_type":"text/x-python","patch_set":8,"id":"1a4dcd0f_bff70f58","line":1114,"in_reply_to":"1a4dcd0f_1455caf8","updated":"2015-08-05 14:48:10.000000000","message":"a snapshot of volume-backed instance\n\nsee fake image in 3rd case in L7348 of https://review.openstack.org/#/c/196569/4/nova/tests/unit/compute/test_compute.py","commit_id":"c3661f87bee0707ddb46dc5b7f7ff53c693db1b0"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"06d186f1cc77827029b8cb7eea513598198d5b57","unresolved":false,"context_lines":[{"line_number":1111,"context_line":""},{"line_number":1112,"context_line":"        root_bdm \u003d None"},{"line_number":1113,"context_line":"        if image_href:"},{"line_number":1114,"context_line":"            image_id, boot_meta \u003d self._get_image(context, image_href)"},{"line_number":1115,"context_line":"        else:"},{"line_number":1116,"context_line":"            image_id \u003d None"},{"line_number":1117,"context_line":"            (boot_meta, root_bdm) \u003d self._get_bdm_image_metadata("}],"source_content_type":"text/x-python","patch_set":8,"id":"1a4dcd0f_1455caf8","line":1114,"in_reply_to":"1a4dcd0f_2b5d0979","updated":"2015-08-05 14:13:49.000000000","message":"What do you mean by a volume-backed image? A glance image whose storage is in cinder? For the avoidance of ambiguity, can you give an example command line?\n\nA volume created from an image is handled in the branch below, because this branch is guarded by \u0027if image_href\u0027 on the line above.","commit_id":"c3661f87bee0707ddb46dc5b7f7ff53c693db1b0"},{"author":{"_account_id":10224,"name":"Feodor Tersin","email":"ftersin@hotmail.com","username":"ftersin"},"change_message_id":"417530f65e3487910a92b3b51f90ab4cce4a477c","unresolved":false,"context_lines":[{"line_number":1111,"context_line":""},{"line_number":1112,"context_line":"        root_bdm \u003d None"},{"line_number":1113,"context_line":"        if image_href:"},{"line_number":1114,"context_line":"            image_id, boot_meta \u003d self._get_image(context, image_href)"},{"line_number":1115,"context_line":"        else:"},{"line_number":1116,"context_line":"            image_id \u003d None"},{"line_number":1117,"context_line":"            (boot_meta, root_bdm) \u003d self._get_bdm_image_metadata("}],"source_content_type":"text/x-python","patch_set":8,"id":"1a4dcd0f_05d116fa","line":1114,"in_reply_to":"1a4dcd0f_bff70f58","updated":"2015-08-05 14:55:51.000000000","message":"look at description of bug 1481220 for an example how to create volume-backed snapshot","commit_id":"c3661f87bee0707ddb46dc5b7f7ff53c693db1b0"},{"author":{"_account_id":10224,"name":"Feodor Tersin","email":"ftersin@hotmail.com","username":"ftersin"},"change_message_id":"e8df4e7d12f5067bef1df361a19ebd90257fa4ea","unresolved":false,"context_lines":[{"line_number":1111,"context_line":""},{"line_number":1112,"context_line":"        root_bdm \u003d None"},{"line_number":1113,"context_line":"        if image_href:"},{"line_number":1114,"context_line":"            image_id, boot_meta \u003d self._get_image(context, image_href)"},{"line_number":1115,"context_line":"        else:"},{"line_number":1116,"context_line":"            image_id \u003d None"},{"line_number":1117,"context_line":"            (boot_meta, root_bdm) \u003d self._get_bdm_image_metadata("}],"source_content_type":"text/x-python","patch_set":8,"id":"1a4dcd0f_0dd138fa","line":1114,"in_reply_to":"1a4dcd0f_ed3c0cca","updated":"2015-08-05 16:15:04.000000000","message":"This is not a specific case. If you look at is_volume_backed_instance implementation, you\u0027ll see that image_ref could be not None for volume-backed instances.\n\n nova boot --block-device source\u003dimage,dest\u003dvolume,size\u003d1,bootindex\u003d0,id\u003d\u003cxxx\u003e --flavor m1.nano inst\n\n nova image-create inst volback\n\n openstack image show volback\n\n nova boot --image volback --flavor m1.nano inst1\n\nAlso see _test_flavor_disk_vs_volume_backed_instance in https://review.openstack.org/#/c/196570/10/tempest/api/compute/servers/test_create_server.py","commit_id":"c3661f87bee0707ddb46dc5b7f7ff53c693db1b0"},{"author":{"_account_id":10224,"name":"Feodor Tersin","email":"ftersin@hotmail.com","username":"ftersin"},"change_message_id":"f9613cdaf5e12e65183e77a312b9748704b5b96a","unresolved":false,"context_lines":[{"line_number":675,"context_line":"        if (root_bdm is not None and root_bdm.is_volume):"},{"line_number":676,"context_line":"            # There are 2 possibilities here: either the target volume already"},{"line_number":677,"context_line":"            # exists, or it doesn\u0027t, in which case the bdm will contain the"},{"line_number":678,"context_line":"            # intended volume size."},{"line_number":679,"context_line":"            #"},{"line_number":680,"context_line":"            # Cinder does its own check against min_disk, so if the target"},{"line_number":681,"context_line":"            # volume already exists this has already been done and we don\u0027t"}],"source_content_type":"text/x-python","patch_set":9,"id":"1a4dcd0f_5d7397e7","line":678,"updated":"2015-08-11 09:26:37.000000000","message":"If the volume is created from a snapshot, volume size may be omitted.","commit_id":"69d65c1908a3f2f0dd3e42444c6fef3952a9449f"},{"author":{"_account_id":10224,"name":"Feodor Tersin","email":"ftersin@hotmail.com","username":"ftersin"},"change_message_id":"884881aff729cba5e83e94e18dd4bebaf15ff6c3","unresolved":false,"context_lines":[{"line_number":675,"context_line":"        if (root_bdm is not None and root_bdm.is_volume):"},{"line_number":676,"context_line":"            # There are 2 possibilities here: either the target volume already"},{"line_number":677,"context_line":"            # exists, or it doesn\u0027t, in which case the bdm will contain the"},{"line_number":678,"context_line":"            # intended volume size."},{"line_number":679,"context_line":"            #"},{"line_number":680,"context_line":"            # Cinder does its own check against min_disk, so if the target"},{"line_number":681,"context_line":"            # volume already exists this has already been done and we don\u0027t"}],"source_content_type":"text/x-python","patch_set":9,"id":"1a4dcd0f_dea67d0e","line":678,"in_reply_to":"1a4dcd0f_4f6e6fca","updated":"2015-08-12 11:16:21.000000000","message":"I saw 3. But i thought that you just forgot to fix this when you where writing 3. So if now it looks as you want, it\u0027s ok.","commit_id":"69d65c1908a3f2f0dd3e42444c6fef3952a9449f"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"3b1f2214d198a062037e4793bb3a80aa6c0f9026","unresolved":false,"context_lines":[{"line_number":675,"context_line":"        if (root_bdm is not None and root_bdm.is_volume):"},{"line_number":676,"context_line":"            # There are 2 possibilities here: either the target volume already"},{"line_number":677,"context_line":"            # exists, or it doesn\u0027t, in which case the bdm will contain the"},{"line_number":678,"context_line":"            # intended volume size."},{"line_number":679,"context_line":"            #"},{"line_number":680,"context_line":"            # Cinder does its own check against min_disk, so if the target"},{"line_number":681,"context_line":"            # volume already exists this has already been done and we don\u0027t"}],"source_content_type":"text/x-python","patch_set":9,"id":"1a4dcd0f_4f6e6fca","line":678,"in_reply_to":"1a4dcd0f_5d7397e7","updated":"2015-08-11 11:40:37.000000000","message":"See possibility 3 :)\n\nBut seriously, I thought about this case. In order to have a snapshot, you first needed to have had a volume, which would have had min_disk checked by cinder. I think this is good anyway, but even if it\u0027s not it\u0027s just a UI issue rather than a correctness issue.","commit_id":"69d65c1908a3f2f0dd3e42444c6fef3952a9449f"},{"author":{"_account_id":10224,"name":"Feodor Tersin","email":"ftersin@hotmail.com","username":"ftersin"},"change_message_id":"f9613cdaf5e12e65183e77a312b9748704b5b96a","unresolved":false,"context_lines":[{"line_number":718,"context_line":"            # NOTE(johannes): root_gb is allowed to be 0 for legacy reasons"},{"line_number":719,"context_line":"            # since libvirt interpreted the value differently than other"},{"line_number":720,"context_line":"            # drivers. A value of 0 means don\u0027t check size."},{"line_number":721,"context_line":"            if dest_size !\u003d 0:"},{"line_number":722,"context_line":"                if image_size \u003e dest_size:"},{"line_number":723,"context_line":"                    raise exception.FlavorDiskTooSmall()"},{"line_number":724,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"1a4dcd0f_a0b394e8","line":721,"updated":"2015-08-11 09:26:37.000000000","message":"nit: imho master version is more consistent. The comment says about root_gb, and root_gb is tested for 0. So i\u0027d prefer to see something like:\n\n \u003ccomments\u003e\n elif instance_type[\u0027root_gb\u0027] !\u003d 0:\n     dest_size \u003d ...\n     if image_size \u003e dest_size:\n         ...\n     if image_min_disk \u003e dest_size:\n         ...\n\nBut this is just a subjective opinion.","commit_id":"69d65c1908a3f2f0dd3e42444c6fef3952a9449f"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"3b1f2214d198a062037e4793bb3a80aa6c0f9026","unresolved":false,"context_lines":[{"line_number":718,"context_line":"            # NOTE(johannes): root_gb is allowed to be 0 for legacy reasons"},{"line_number":719,"context_line":"            # since libvirt interpreted the value differently than other"},{"line_number":720,"context_line":"            # drivers. A value of 0 means don\u0027t check size."},{"line_number":721,"context_line":"            if dest_size !\u003d 0:"},{"line_number":722,"context_line":"                if image_size \u003e dest_size:"},{"line_number":723,"context_line":"                    raise exception.FlavorDiskTooSmall()"},{"line_number":724,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"1a4dcd0f_4ff3cf0a","line":721,"in_reply_to":"1a4dcd0f_a0b394e8","updated":"2015-08-11 11:40:37.000000000","message":"I think I see what you mean, but I like the simplicity of the 2 top-level branches being:\n\n  if root is a volume:\n    foo\n  else:\n    bar","commit_id":"69d65c1908a3f2f0dd3e42444c6fef3952a9449f"},{"author":{"_account_id":10224,"name":"Feodor Tersin","email":"ftersin@hotmail.com","username":"ftersin"},"change_message_id":"f9613cdaf5e12e65183e77a312b9748704b5b96a","unresolved":false,"context_lines":[{"line_number":854,"context_line":"                                         display_description, key_name,"},{"line_number":855,"context_line":"                                         key_data, security_groups,"},{"line_number":856,"context_line":"                                         availability_zone, forced_host,"},{"line_number":857,"context_line":"                                         user_data, metadata, injected_files,"},{"line_number":858,"context_line":"                                         access_ip_v4, access_ip_v6,"},{"line_number":859,"context_line":"                                         requested_networks, config_drive,"},{"line_number":860,"context_line":"                                         auto_disk_config, reservation_id,"}],"source_content_type":"text/x-python","patch_set":9,"id":"1a4dcd0f_20730417","line":857,"updated":"2015-08-11 09:26:37.000000000","message":"now you could remove injected_files arg","commit_id":"69d65c1908a3f2f0dd3e42444c6fef3952a9449f"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"3b1f2214d198a062037e4793bb3a80aa6c0f9026","unresolved":false,"context_lines":[{"line_number":854,"context_line":"                                         display_description, key_name,"},{"line_number":855,"context_line":"                                         key_data, security_groups,"},{"line_number":856,"context_line":"                                         availability_zone, forced_host,"},{"line_number":857,"context_line":"                                         user_data, metadata, injected_files,"},{"line_number":858,"context_line":"                                         access_ip_v4, access_ip_v6,"},{"line_number":859,"context_line":"                                         requested_networks, config_drive,"},{"line_number":860,"context_line":"                                         auto_disk_config, reservation_id,"}],"source_content_type":"text/x-python","patch_set":9,"id":"1a4dcd0f_6f448b1a","line":857,"in_reply_to":"1a4dcd0f_20730417","updated":"2015-08-11 11:40:37.000000000","message":"Ack.","commit_id":"69d65c1908a3f2f0dd3e42444c6fef3952a9449f"},{"author":{"_account_id":10224,"name":"Feodor Tersin","email":"ftersin@hotmail.com","username":"ftersin"},"change_message_id":"f9613cdaf5e12e65183e77a312b9748704b5b96a","unresolved":false,"context_lines":[{"line_number":1170,"context_line":"            base_options, instance_type, boot_meta, min_count, max_count,"},{"line_number":1171,"context_line":"            block_device_mapping, legacy_bdm)"},{"line_number":1172,"context_line":""},{"line_number":1173,"context_line":"        # We can\u0027t do this check earlier because we need bdms from all sources"},{"line_number":1174,"context_line":"        # to have been merged in order to get the root bdm."},{"line_number":1175,"context_line":"        self._checks_for_create_and_rebuild(context, image_id, boot_meta,"},{"line_number":1176,"context_line":"                instance_type, metadata, injected_files,"}],"source_content_type":"text/x-python","patch_set":9,"id":"1a4dcd0f_6ffed06c","line":1173,"updated":"2015-08-11 09:26:37.000000000","message":"It would be nice to have a test on that. To ensure that it\u0027s called after bdms parsing and merging, and a root bdm is provided indeed.","commit_id":"69d65c1908a3f2f0dd3e42444c6fef3952a9449f"},{"author":{"_account_id":10224,"name":"Feodor Tersin","email":"ftersin@hotmail.com","username":"ftersin"},"change_message_id":"884881aff729cba5e83e94e18dd4bebaf15ff6c3","unresolved":false,"context_lines":[{"line_number":1170,"context_line":"            base_options, instance_type, boot_meta, min_count, max_count,"},{"line_number":1171,"context_line":"            block_device_mapping, legacy_bdm)"},{"line_number":1172,"context_line":""},{"line_number":1173,"context_line":"        # We can\u0027t do this check earlier because we need bdms from all sources"},{"line_number":1174,"context_line":"        # to have been merged in order to get the root bdm."},{"line_number":1175,"context_line":"        self._checks_for_create_and_rebuild(context, image_id, boot_meta,"},{"line_number":1176,"context_line":"                instance_type, metadata, injected_files,"}],"source_content_type":"text/x-python","patch_set":9,"id":"1a4dcd0f_7b1e560d","line":1173,"in_reply_to":"1a4dcd0f_236e9c96","updated":"2015-08-12 11:16:21.000000000","message":"I didn\u0027t mean exactly explicit check of the order. It might be an integrated test, or a test which sets various bmds and checks which one is passed to _check_for_create_and_rebuild.\n\nAnyway i got your point. I\u0027m inclined to agree with you.","commit_id":"69d65c1908a3f2f0dd3e42444c6fef3952a9449f"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"3b1f2214d198a062037e4793bb3a80aa6c0f9026","unresolved":false,"context_lines":[{"line_number":1170,"context_line":"            base_options, instance_type, boot_meta, min_count, max_count,"},{"line_number":1171,"context_line":"            block_device_mapping, legacy_bdm)"},{"line_number":1172,"context_line":""},{"line_number":1173,"context_line":"        # We can\u0027t do this check earlier because we need bdms from all sources"},{"line_number":1174,"context_line":"        # to have been merged in order to get the root bdm."},{"line_number":1175,"context_line":"        self._checks_for_create_and_rebuild(context, image_id, boot_meta,"},{"line_number":1176,"context_line":"                instance_type, metadata, injected_files,"}],"source_content_type":"text/x-python","patch_set":9,"id":"1a4dcd0f_236e9c96","line":1173,"in_reply_to":"1a4dcd0f_6ffed06c","updated":"2015-08-11 11:40:37.000000000","message":"It looks to me like _create_instance is lacking some test coverage. If it existed already I think it would make sense to add it. Given that it doesn\u0027t, and I want to backport this fix, I don\u0027t want to add it to this patch.\n\nAgree it would be good to have, though. Well, that the correct root bdm is passed anyway. I don\u0027t like tests which assert ordering of private functions from afar.","commit_id":"69d65c1908a3f2f0dd3e42444c6fef3952a9449f"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"7d7f9c95c7094b69525149c323b5c8ea6838cfc4","unresolved":false,"context_lines":[{"line_number":699,"context_line":"            #"},{"line_number":700,"context_line":"            # We could reasonably refactor this check into _validate_bdm at"},{"line_number":701,"context_line":"            # some future date, as the various size logic is already split out"},{"line_number":702,"context_line":"            # in there."},{"line_number":703,"context_line":"            dest_size \u003d root_bdm.volume_size"},{"line_number":704,"context_line":"            if dest_size is not None:"},{"line_number":705,"context_line":"                dest_size *\u003d units.Gi"}],"source_content_type":"text/x-python","patch_set":10,"id":"1a4dcd0f_3ed98a57","line":702,"updated":"2015-08-12 13:51:11.000000000","message":"Hmmm so we only really actually know the snapshot size for sure after _validate_bdm because we don\u0027t require it in the API.\n\nShould we maybe move these checks there in this patch.","commit_id":"b46289053fa133c7d05c3a8daf4b967b96e29ceb"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"e201838fdf9615b0b617dab7120a2e987b05c36d","unresolved":false,"context_lines":[{"line_number":699,"context_line":"            #"},{"line_number":700,"context_line":"            # We could reasonably refactor this check into _validate_bdm at"},{"line_number":701,"context_line":"            # some future date, as the various size logic is already split out"},{"line_number":702,"context_line":"            # in there."},{"line_number":703,"context_line":"            dest_size \u003d root_bdm.volume_size"},{"line_number":704,"context_line":"            if dest_size is not None:"},{"line_number":705,"context_line":"                dest_size *\u003d units.Gi"}],"source_content_type":"text/x-python","patch_set":10,"id":"1a4dcd0f_59efd0b0","line":702,"in_reply_to":"1a4dcd0f_3ed98a57","updated":"2015-08-12 14:04:03.000000000","message":"Feodor brought this up in a comment against the previous patch. I don\u0027t think so.\n\nThe main reason is that (iiuc) in order for there to be a snapshot with image metadata, there first had to be a volume which was created from an image. That volume will have had its size checked against min_disk at creation, so the user will already have had their friendly error message. We should never hit it here, but if we did it would still be caught when the request gets to the host.\n\nThe other reason is that moving it into _validate_bdm is a larger refactor. As you\u0027ve pointed out in another review, moving _validate_bdm before instance creation requires changing the way volumes are attached, as well as moving significantly more code. I\u0027d like to avoid that for this bugfix. And the check is shared with rebuild, which would require careful consideration in the context of more bdm checks.","commit_id":"b46289053fa133c7d05c3a8daf4b967b96e29ceb"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"3041b5b103963e576c718ab4c93f75d02a5ef9fd","unresolved":false,"context_lines":[{"line_number":699,"context_line":"            #"},{"line_number":700,"context_line":"            # We could reasonably refactor this check into _validate_bdm at"},{"line_number":701,"context_line":"            # some future date, as the various size logic is already split out"},{"line_number":702,"context_line":"            # in there."},{"line_number":703,"context_line":"            dest_size \u003d root_bdm.volume_size"},{"line_number":704,"context_line":"            if dest_size is not None:"},{"line_number":705,"context_line":"                dest_size *\u003d units.Gi"}],"source_content_type":"text/x-python","patch_set":10,"id":"1a4dcd0f_ef27ee60","line":702,"in_reply_to":"1a4dcd0f_59efd0b0","updated":"2015-08-12 14:34:14.000000000","message":"I see... you seem to be correct. the only way we could get around the check performed at volume creation time is if there was a way to create a snapshot that is smaller than the volume which I think cannot be done so this should be fine.","commit_id":"b46289053fa133c7d05c3a8daf4b967b96e29ceb"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"7d7f9c95c7094b69525149c323b5c8ea6838cfc4","unresolved":false,"context_lines":[{"line_number":707,"context_line":"                if image_min_disk \u003e dest_size:"},{"line_number":708,"context_line":"                    # This is exception is wrong, but it\u0027s the exception which"},{"line_number":709,"context_line":"                    # calling code expects this function to raise. Due to the"},{"line_number":710,"context_line":"                    # churn it causes this will be fixed up in a subsequent"},{"line_number":711,"context_line":"                    # patch."},{"line_number":712,"context_line":"                    raise exception.FlavorDiskTooSmall()"},{"line_number":713,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"1a4dcd0f_23de5711","line":710,"updated":"2015-08-12 13:51:11.000000000","message":"This comment should be a TODO really but meh","commit_id":"b46289053fa133c7d05c3a8daf4b967b96e29ceb"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"e201838fdf9615b0b617dab7120a2e987b05c36d","unresolved":false,"context_lines":[{"line_number":707,"context_line":"                if image_min_disk \u003e dest_size:"},{"line_number":708,"context_line":"                    # This is exception is wrong, but it\u0027s the exception which"},{"line_number":709,"context_line":"                    # calling code expects this function to raise. Due to the"},{"line_number":710,"context_line":"                    # churn it causes this will be fixed up in a subsequent"},{"line_number":711,"context_line":"                    # patch."},{"line_number":712,"context_line":"                    raise exception.FlavorDiskTooSmall()"},{"line_number":713,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"1a4dcd0f_d9ef601b","line":710,"in_reply_to":"1a4dcd0f_23de5711","updated":"2015-08-12 14:04:03.000000000","message":"I\u0027ll change it as I\u0027m going to fix another nit anyway.","commit_id":"b46289053fa133c7d05c3a8daf4b967b96e29ceb"}],"nova/tests/unit/api/openstack/compute/contrib/test_block_device_mapping.py":[{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"000eff79bbcdef20592892af1359e75a613144ed","unresolved":false,"context_lines":[{"line_number":144,"context_line":"            mox.IgnoreArg(), mox.IgnoreArg(), mox.IgnoreArg(),"},{"line_number":145,"context_line":"            mox.IgnoreArg()).AndReturn(True)"},{"line_number":146,"context_line":"        compute_api.API._get_bdm_image_metadata("},{"line_number":147,"context_line":"            mox.IgnoreArg(), mox.IgnoreArg(), False).AndReturn(({}, True))"},{"line_number":148,"context_line":"        self.mox.ReplayAll()"},{"line_number":149,"context_line":""},{"line_number":150,"context_line":"        params \u003d {block_device_mapping.ATTRIBUTE_NAME: self.bdm}"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a4dcd0f_9d797632","line":147,"updated":"2015-08-03 16:44:26.000000000","message":"Needs to change to return a BDM","commit_id":"a7d34122192d227c638280439ec14f267cdce9f9"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"b39aa10ba49da048f65bb50d9987618b13b46dfd","unresolved":false,"context_lines":[{"line_number":144,"context_line":"            mox.IgnoreArg(), mox.IgnoreArg(), mox.IgnoreArg(),"},{"line_number":145,"context_line":"            mox.IgnoreArg()).AndReturn(True)"},{"line_number":146,"context_line":"        compute_api.API._get_bdm_image_metadata("},{"line_number":147,"context_line":"            mox.IgnoreArg(), mox.IgnoreArg(), False).AndReturn(({}, True))"},{"line_number":148,"context_line":"        self.mox.ReplayAll()"},{"line_number":149,"context_line":""},{"line_number":150,"context_line":"        params \u003d {block_device_mapping.ATTRIBUTE_NAME: self.bdm}"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a4dcd0f_e59a3c14","line":147,"in_reply_to":"1a4dcd0f_9d797632","updated":"2015-08-04 11:15:33.000000000","message":"Done","commit_id":"a7d34122192d227c638280439ec14f267cdce9f9"}],"nova/tests/unit/api/openstack/compute/contrib/test_block_device_mapping_v1.py":[{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"000eff79bbcdef20592892af1359e75a613144ed","unresolved":false,"context_lines":[{"line_number":128,"context_line":"        compute_api.API._get_bdm_image_metadata(mox.IgnoreArg(),"},{"line_number":129,"context_line":"                                                self.bdm,"},{"line_number":130,"context_line":"                                                True).AndReturn("},{"line_number":131,"context_line":"                                                    (volume, True))"},{"line_number":132,"context_line":"        params \u003d {\u0027block_device_mapping\u0027: self.bdm}"},{"line_number":133,"context_line":"        old_create \u003d compute_api.API.create"},{"line_number":134,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"1a4dcd0f_3d76223f","line":131,"updated":"2015-08-03 16:44:26.000000000","message":"Needs to return a BDM","commit_id":"a7d34122192d227c638280439ec14f267cdce9f9"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"b39aa10ba49da048f65bb50d9987618b13b46dfd","unresolved":false,"context_lines":[{"line_number":128,"context_line":"        compute_api.API._get_bdm_image_metadata(mox.IgnoreArg(),"},{"line_number":129,"context_line":"                                                self.bdm,"},{"line_number":130,"context_line":"                                                True).AndReturn("},{"line_number":131,"context_line":"                                                    (volume, True))"},{"line_number":132,"context_line":"        params \u003d {\u0027block_device_mapping\u0027: self.bdm}"},{"line_number":133,"context_line":"        old_create \u003d compute_api.API.create"},{"line_number":134,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"1a4dcd0f_e5c1dce5","line":131,"in_reply_to":"1a4dcd0f_3d76223f","updated":"2015-08-04 11:15:33.000000000","message":"This exposed a real bug, in that I wasn\u0027t handling the legacy bdm format. Fixed.","commit_id":"a7d34122192d227c638280439ec14f267cdce9f9"}],"nova/tests/unit/compute/test_compute.py":[{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"265fc1b05c977a42b53d7d76ff8aded515e8554a","unresolved":false,"context_lines":[{"line_number":569,"context_line":"            \u0027delete_on_termination\u0027: False,"},{"line_number":570,"context_line":"        }]"},{"line_number":571,"context_line":""},{"line_number":572,"context_line":"        (image_meta, is_vol_boot) \u003d \\"},{"line_number":573,"context_line":"            self.compute_api._get_bdm_image_metadata("},{"line_number":574,"context_line":"                self.context, block_device_mapping)"},{"line_number":575,"context_line":"        if metadata:"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_f197b545","line":572,"updated":"2015-07-29 11:12:14.000000000","message":"Consider:\n (image_meta,\n  is_vol_boot) \u003d self.compute_api._get_bdm_image_metadata(...","commit_id":"235a88f153ce30d61068a70fdc0368ca3829f0eb"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"107741688445f30827f17818727c61b730e566c3","unresolved":false,"context_lines":[{"line_number":569,"context_line":"            \u0027delete_on_termination\u0027: False,"},{"line_number":570,"context_line":"        }]"},{"line_number":571,"context_line":""},{"line_number":572,"context_line":"        (image_meta, is_vol_boot) \u003d \\"},{"line_number":573,"context_line":"            self.compute_api._get_bdm_image_metadata("},{"line_number":574,"context_line":"                self.context, block_device_mapping)"},{"line_number":575,"context_line":"        if metadata:"}],"source_content_type":"text/x-python","patch_set":4,"id":"1a4dcd0f_ca489b8a","line":572,"in_reply_to":"3a50d1a3_f197b545","updated":"2015-08-03 15:10:24.000000000","message":"Are we still recommending that? It\u0027s pretty ugly. Backslash is easier to read imho.","commit_id":"235a88f153ce30d61068a70fdc0368ca3829f0eb"},{"author":{"_account_id":10224,"name":"Feodor Tersin","email":"ftersin@hotmail.com","username":"ftersin"},"change_message_id":"f9613cdaf5e12e65183e77a312b9748704b5b96a","unresolved":false,"context_lines":[{"line_number":11643,"context_line":"        image_uuid \u003d str(uuid.uuid4())"},{"line_number":11644,"context_line":"        volume_uuid \u003d str(uuid.uuid4())"},{"line_number":11645,"context_line":"        image \u003d dict(id\u003dimage_uuid, status\u003d\u0027active\u0027,"},{"line_number":11646,"context_line":"                     size\u003d(2 * units.Gi), min_disk\u003d2)"},{"line_number":11647,"context_line":""},{"line_number":11648,"context_line":"        root_bdm \u003d block_device_obj.BlockDeviceMapping("},{"line_number":11649,"context_line":"            source_type\u003d\u0027volume\u0027, destination_type\u003d\u0027volume\u0027,"}],"source_content_type":"text/x-python","patch_set":9,"id":"1a4dcd0f_360fc52f","line":11646,"updated":"2015-08-11 09:26:37.000000000","message":"nit: Could we use self.instance_type[\u0027min_disk\u0027] in sizes evaluations? This will make the test insensitive to changes of the fake in class setup.","commit_id":"69d65c1908a3f2f0dd3e42444c6fef3952a9449f"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"3b1f2214d198a062037e4793bb3a80aa6c0f9026","unresolved":false,"context_lines":[{"line_number":11643,"context_line":"        image_uuid \u003d str(uuid.uuid4())"},{"line_number":11644,"context_line":"        volume_uuid \u003d str(uuid.uuid4())"},{"line_number":11645,"context_line":"        image \u003d dict(id\u003dimage_uuid, status\u003d\u0027active\u0027,"},{"line_number":11646,"context_line":"                     size\u003d(2 * units.Gi), min_disk\u003d2)"},{"line_number":11647,"context_line":""},{"line_number":11648,"context_line":"        root_bdm \u003d block_device_obj.BlockDeviceMapping("},{"line_number":11649,"context_line":"            source_type\u003d\u0027volume\u0027, destination_type\u003d\u0027volume\u0027,"}],"source_content_type":"text/x-python","patch_set":9,"id":"1a4dcd0f_38b6c78a","line":11646,"in_reply_to":"1a4dcd0f_360fc52f","updated":"2015-08-11 11:40:37.000000000","message":"Good idea.","commit_id":"69d65c1908a3f2f0dd3e42444c6fef3952a9449f"},{"author":{"_account_id":10224,"name":"Feodor Tersin","email":"ftersin@hotmail.com","username":"ftersin"},"change_message_id":"f9613cdaf5e12e65183e77a312b9748704b5b96a","unresolved":false,"context_lines":[{"line_number":11661,"context_line":"                     size\u003d(2 * units.Gi), min_disk\u003d3)"},{"line_number":11662,"context_line":""},{"line_number":11663,"context_line":"        root_bdm \u003d block_device_obj.BlockDeviceMapping("},{"line_number":11664,"context_line":"            source_type\u003d\u0027volume\u0027, destination_type\u003d\u0027volume\u0027,"},{"line_number":11665,"context_line":"            volume_id\u003dvolume_uuid, volume_size\u003d2)"},{"line_number":11666,"context_line":""},{"line_number":11667,"context_line":"        self.assertRaises(exception.FlavorDiskTooSmall,"}],"source_content_type":"text/x-python","patch_set":9,"id":"1a4dcd0f_19143a42","line":11664,"updated":"2015-08-11 09:26:37.000000000","message":"nit: i think image-\u003evolume mapping whould make more sense, because it correponds to a real case which behavior you want to improve by this patch.\n\nMoreover, volume-\u003evolume mapping doesn\u0027t allow volume_size attribute for boot instance operation iiamn. But of course, such combination occures for rebuild operation.","commit_id":"69d65c1908a3f2f0dd3e42444c6fef3952a9449f"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"3b1f2214d198a062037e4793bb3a80aa6c0f9026","unresolved":false,"context_lines":[{"line_number":11661,"context_line":"                     size\u003d(2 * units.Gi), min_disk\u003d3)"},{"line_number":11662,"context_line":""},{"line_number":11663,"context_line":"        root_bdm \u003d block_device_obj.BlockDeviceMapping("},{"line_number":11664,"context_line":"            source_type\u003d\u0027volume\u0027, destination_type\u003d\u0027volume\u0027,"},{"line_number":11665,"context_line":"            volume_id\u003dvolume_uuid, volume_size\u003d2)"},{"line_number":11666,"context_line":""},{"line_number":11667,"context_line":"        self.assertRaises(exception.FlavorDiskTooSmall,"}],"source_content_type":"text/x-python","patch_set":9,"id":"1a4dcd0f_58471377","line":11664,"in_reply_to":"1a4dcd0f_19143a42","updated":"2015-08-11 11:40:37.000000000","message":"Good point. This was a holdover, and I didn\u0027t update it.","commit_id":"69d65c1908a3f2f0dd3e42444c6fef3952a9449f"},{"author":{"_account_id":10224,"name":"Feodor Tersin","email":"ftersin@hotmail.com","username":"ftersin"},"change_message_id":"f9613cdaf5e12e65183e77a312b9748704b5b96a","unresolved":false,"context_lines":[{"line_number":11678,"context_line":""},{"line_number":11679,"context_line":"        root_bdm \u003d block_device_obj.BlockDeviceMapping("},{"line_number":11680,"context_line":"            source_type\u003d\u0027volume\u0027, destination_type\u003d\u0027volume\u0027,"},{"line_number":11681,"context_line":"            volume_id\u003dvolume_uuid, volume_size\u003dNone)"},{"line_number":11682,"context_line":""},{"line_number":11683,"context_line":"        self.compute_api._check_requested_image(self.context, image[\u0027id\u0027],"},{"line_number":11684,"context_line":"                image, self.instance_type, root_bdm)"}],"source_content_type":"text/x-python","patch_set":9,"id":"1a4dcd0f_b9bd06d9","line":11681,"updated":"2015-08-11 09:26:37.000000000","message":"nit: You could not specify volume_size arg at all.","commit_id":"69d65c1908a3f2f0dd3e42444c6fef3952a9449f"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"3b1f2214d198a062037e4793bb3a80aa6c0f9026","unresolved":false,"context_lines":[{"line_number":11678,"context_line":""},{"line_number":11679,"context_line":"        root_bdm \u003d block_device_obj.BlockDeviceMapping("},{"line_number":11680,"context_line":"            source_type\u003d\u0027volume\u0027, destination_type\u003d\u0027volume\u0027,"},{"line_number":11681,"context_line":"            volume_id\u003dvolume_uuid, volume_size\u003dNone)"},{"line_number":11682,"context_line":""},{"line_number":11683,"context_line":"        self.compute_api._check_requested_image(self.context, image[\u0027id\u0027],"},{"line_number":11684,"context_line":"                image, self.instance_type, root_bdm)"}],"source_content_type":"text/x-python","patch_set":9,"id":"1a4dcd0f_d82a4390","line":11681,"in_reply_to":"1a4dcd0f_b9bd06d9","updated":"2015-08-11 11:40:37.000000000","message":"Unfortunately you can\u0027t. If you omit it, the object marks it as not having been loaded, and tries to fetch it from the db when you access it.","commit_id":"69d65c1908a3f2f0dd3e42444c6fef3952a9449f"},{"author":{"_account_id":10224,"name":"Feodor Tersin","email":"ftersin@hotmail.com","username":"ftersin"},"change_message_id":"884881aff729cba5e83e94e18dd4bebaf15ff6c3","unresolved":false,"context_lines":[{"line_number":11678,"context_line":""},{"line_number":11679,"context_line":"        root_bdm \u003d block_device_obj.BlockDeviceMapping("},{"line_number":11680,"context_line":"            source_type\u003d\u0027volume\u0027, destination_type\u003d\u0027volume\u0027,"},{"line_number":11681,"context_line":"            volume_id\u003dvolume_uuid, volume_size\u003dNone)"},{"line_number":11682,"context_line":""},{"line_number":11683,"context_line":"        self.compute_api._check_requested_image(self.context, image[\u0027id\u0027],"},{"line_number":11684,"context_line":"                image, self.instance_type, root_bdm)"}],"source_content_type":"text/x-python","patch_set":9,"id":"1a4dcd0f_044ed86e","line":11681,"in_reply_to":"1a4dcd0f_d82a4390","updated":"2015-08-12 11:16:21.000000000","message":"it\u0027s right, my mistake","commit_id":"69d65c1908a3f2f0dd3e42444c6fef3952a9449f"},{"author":{"_account_id":10224,"name":"Feodor Tersin","email":"ftersin@hotmail.com","username":"ftersin"},"change_message_id":"f9613cdaf5e12e65183e77a312b9748704b5b96a","unresolved":false,"context_lines":[{"line_number":11683,"context_line":"        self.compute_api._check_requested_image(self.context, image[\u0027id\u0027],"},{"line_number":11684,"context_line":"                image, self.instance_type, root_bdm)"},{"line_number":11685,"context_line":""},{"line_number":11686,"context_line":""},{"line_number":11687,"context_line":"class ComputeHooksTestCase(test.BaseHookTestCase):"},{"line_number":11688,"context_line":"    def test_delete_instance_has_hook(self):"},{"line_number":11689,"context_line":"        delete_func \u003d compute_manager.ComputeManager._delete_instance"}],"source_content_type":"text/x-python","patch_set":9,"id":"1a4dcd0f_b9074680","line":11686,"updated":"2015-08-11 09:26:37.000000000","message":"nit: A test for image-\u003elocal root bdm is wanted.","commit_id":"69d65c1908a3f2f0dd3e42444c6fef3952a9449f"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"3b1f2214d198a062037e4793bb3a80aa6c0f9026","unresolved":false,"context_lines":[{"line_number":11683,"context_line":"        self.compute_api._check_requested_image(self.context, image[\u0027id\u0027],"},{"line_number":11684,"context_line":"                image, self.instance_type, root_bdm)"},{"line_number":11685,"context_line":""},{"line_number":11686,"context_line":""},{"line_number":11687,"context_line":"class ComputeHooksTestCase(test.BaseHookTestCase):"},{"line_number":11688,"context_line":"    def test_delete_instance_has_hook(self):"},{"line_number":11689,"context_line":"        delete_func \u003d compute_manager.ComputeManager._delete_instance"}],"source_content_type":"text/x-python","patch_set":9,"id":"1a4dcd0f_630ca43b","line":11686,"in_reply_to":"1a4dcd0f_b9074680","updated":"2015-08-11 11:40:37.000000000","message":"Yup.","commit_id":"69d65c1908a3f2f0dd3e42444c6fef3952a9449f"},{"author":{"_account_id":10224,"name":"Feodor Tersin","email":"ftersin@hotmail.com","username":"ftersin"},"change_message_id":"202ada3835bb095363b2aa5c8fe0b61616a05a1a","unresolved":false,"context_lines":[{"line_number":11642,"context_line":"        # disk"},{"line_number":11643,"context_line":"        image_uuid \u003d str(uuid.uuid4())"},{"line_number":11644,"context_line":"        image \u003d dict(id\u003dimage_uuid, status\u003d\u0027active\u0027,"},{"line_number":11645,"context_line":"                     size\u003dself.instance_type.root_gb * units.Gi, min_disk\u003d0)"},{"line_number":11646,"context_line":""},{"line_number":11647,"context_line":"        volume_uuid \u003d str(uuid.uuid4())"},{"line_number":11648,"context_line":"        root_bdm \u003d block_device_obj.BlockDeviceMapping("}],"source_content_type":"text/x-python","patch_set":10,"id":"1a4dcd0f_e49acc61","line":11645,"updated":"2015-08-12 11:15:50.000000000","message":"If you assign min_disk \u003d 0, you should have another test for min_disk \u003e root_gb case","commit_id":"b46289053fa133c7d05c3a8daf4b967b96e29ceb"},{"author":{"_account_id":10224,"name":"Feodor Tersin","email":"ftersin@hotmail.com","username":"ftersin"},"change_message_id":"fcd618ed94600530994a49d523883998b0e8455a","unresolved":false,"context_lines":[{"line_number":11642,"context_line":"        # disk"},{"line_number":11643,"context_line":"        image_uuid \u003d str(uuid.uuid4())"},{"line_number":11644,"context_line":"        image \u003d dict(id\u003dimage_uuid, status\u003d\u0027active\u0027,"},{"line_number":11645,"context_line":"                     size\u003dself.instance_type.root_gb * units.Gi, min_disk\u003d0)"},{"line_number":11646,"context_line":""},{"line_number":11647,"context_line":"        volume_uuid \u003d str(uuid.uuid4())"},{"line_number":11648,"context_line":"        root_bdm \u003d block_device_obj.BlockDeviceMapping("}],"source_content_type":"text/x-python","patch_set":10,"id":"1a4dcd0f_b475bbcb","line":11645,"in_reply_to":"1a4dcd0f_7e707240","updated":"2015-08-12 14:20:30.000000000","message":"It checks that the function fails on volume size is smaller than min_disk. But what test checks that the function passes on min_disk is greater than flavor\u0027s root_gb? Sorry if i missed something.","commit_id":"b46289053fa133c7d05c3a8daf4b967b96e29ceb"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"86466e1f589d270aefd9b72aa77dc47001d0dafc","unresolved":false,"context_lines":[{"line_number":11642,"context_line":"        # disk"},{"line_number":11643,"context_line":"        image_uuid \u003d str(uuid.uuid4())"},{"line_number":11644,"context_line":"        image \u003d dict(id\u003dimage_uuid, status\u003d\u0027active\u0027,"},{"line_number":11645,"context_line":"                     size\u003dself.instance_type.root_gb * units.Gi, min_disk\u003d0)"},{"line_number":11646,"context_line":""},{"line_number":11647,"context_line":"        volume_uuid \u003d str(uuid.uuid4())"},{"line_number":11648,"context_line":"        root_bdm \u003d block_device_obj.BlockDeviceMapping("}],"source_content_type":"text/x-python","patch_set":10,"id":"1a4dcd0f_af3806d4","line":11645,"in_reply_to":"1a4dcd0f_b475bbcb","updated":"2015-08-12 14:23:42.000000000","message":"Understood. I\u0027ll combine that into this test while I\u0027m fixing up the nit below.","commit_id":"b46289053fa133c7d05c3a8daf4b967b96e29ceb"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"e201838fdf9615b0b617dab7120a2e987b05c36d","unresolved":false,"context_lines":[{"line_number":11642,"context_line":"        # disk"},{"line_number":11643,"context_line":"        image_uuid \u003d str(uuid.uuid4())"},{"line_number":11644,"context_line":"        image \u003d dict(id\u003dimage_uuid, status\u003d\u0027active\u0027,"},{"line_number":11645,"context_line":"                     size\u003dself.instance_type.root_gb * units.Gi, min_disk\u003d0)"},{"line_number":11646,"context_line":""},{"line_number":11647,"context_line":"        volume_uuid \u003d str(uuid.uuid4())"},{"line_number":11648,"context_line":"        root_bdm \u003d block_device_obj.BlockDeviceMapping("}],"source_content_type":"text/x-python","patch_set":10,"id":"1a4dcd0f_7e707240","line":11645,"in_reply_to":"1a4dcd0f_e49acc61","updated":"2015-08-12 14:04:03.000000000","message":"That\u0027s test_volume_blockdevicemapping_min_disk below. min_disk is zero here only because I don\u0027t want it to do anything.","commit_id":"b46289053fa133c7d05c3a8daf4b967b96e29ceb"},{"author":{"_account_id":10224,"name":"Feodor Tersin","email":"ftersin@hotmail.com","username":"ftersin"},"change_message_id":"202ada3835bb095363b2aa5c8fe0b61616a05a1a","unresolved":false,"context_lines":[{"line_number":11647,"context_line":"        volume_uuid \u003d str(uuid.uuid4())"},{"line_number":11648,"context_line":"        root_bdm \u003d block_device_obj.BlockDeviceMapping("},{"line_number":11649,"context_line":"            source_type\u003d\u0027volume\u0027, destination_type\u003d\u0027volume\u0027,"},{"line_number":11650,"context_line":"            volume_id\u003dvolume_uuid, volume_size\u003d2)"},{"line_number":11651,"context_line":""},{"line_number":11652,"context_line":"        self.compute_api._check_requested_image(self.context, image[\u0027id\u0027],"},{"line_number":11653,"context_line":"                image, self.instance_type, root_bdm)"}],"source_content_type":"text/x-python","patch_set":10,"id":"1a4dcd0f_84cbc852","line":11650,"updated":"2015-08-12 11:15:50.000000000","message":"Did you forget to change volume_size\u003d2 here?","commit_id":"b46289053fa133c7d05c3a8daf4b967b96e29ceb"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"e201838fdf9615b0b617dab7120a2e987b05c36d","unresolved":false,"context_lines":[{"line_number":11647,"context_line":"        volume_uuid \u003d str(uuid.uuid4())"},{"line_number":11648,"context_line":"        root_bdm \u003d block_device_obj.BlockDeviceMapping("},{"line_number":11649,"context_line":"            source_type\u003d\u0027volume\u0027, destination_type\u003d\u0027volume\u0027,"},{"line_number":11650,"context_line":"            volume_id\u003dvolume_uuid, volume_size\u003d2)"},{"line_number":11651,"context_line":""},{"line_number":11652,"context_line":"        self.compute_api._check_requested_image(self.context, image[\u0027id\u0027],"},{"line_number":11653,"context_line":"                image, self.instance_type, root_bdm)"}],"source_content_type":"text/x-python","patch_set":10,"id":"1a4dcd0f_9e0d3e40","line":11650,"in_reply_to":"1a4dcd0f_84cbc852","updated":"2015-08-12 14:04:03.000000000","message":"Yes :)","commit_id":"b46289053fa133c7d05c3a8daf4b967b96e29ceb"}],"nova/tests/unit/compute/test_compute_api.py":[{"author":{"_account_id":10224,"name":"Feodor Tersin","email":"ftersin@hotmail.com","username":"ftersin"},"change_message_id":"f9613cdaf5e12e65183e77a312b9748704b5b96a","unresolved":false,"context_lines":[{"line_number":2303,"context_line":"                 \"properties\": {\u0027architecture\u0027: arch.X86_64}}"},{"line_number":2304,"context_line":"        admin_pass \u003d \u0027\u0027"},{"line_number":2305,"context_line":"        files_to_inject \u003d []"},{"line_number":2306,"context_line":"        bdms \u003d objects.BlockDeviceMappingList()"},{"line_number":2307,"context_line":""},{"line_number":2308,"context_line":"        _get_image.return_value \u003d (None, image)"},{"line_number":2309,"context_line":"        bdm_get_by_instance_uuid.return_value \u003d bdms"}],"source_content_type":"text/x-python","patch_set":9,"id":"1a4dcd0f_14b5e12f","line":2306,"updated":"2015-08-11 09:26:37.000000000","message":"nit: in fact it will have image-\u003elocal root mapping at least","commit_id":"69d65c1908a3f2f0dd3e42444c6fef3952a9449f"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"3b1f2214d198a062037e4793bb3a80aa6c0f9026","unresolved":false,"context_lines":[{"line_number":2303,"context_line":"                 \"properties\": {\u0027architecture\u0027: arch.X86_64}}"},{"line_number":2304,"context_line":"        admin_pass \u003d \u0027\u0027"},{"line_number":2305,"context_line":"        files_to_inject \u003d []"},{"line_number":2306,"context_line":"        bdms \u003d objects.BlockDeviceMappingList()"},{"line_number":2307,"context_line":""},{"line_number":2308,"context_line":"        _get_image.return_value \u003d (None, image)"},{"line_number":2309,"context_line":"        bdm_get_by_instance_uuid.return_value \u003d bdms"}],"source_content_type":"text/x-python","patch_set":9,"id":"1a4dcd0f_437c0872","line":2306,"in_reply_to":"1a4dcd0f_14b5e12f","updated":"2015-08-11 11:40:37.000000000","message":"Agreed. However, that was broken before and in any case isn\u0027t critical to this test. I don\u0027t think it\u0027s worth adding complication this patch.","commit_id":"69d65c1908a3f2f0dd3e42444c6fef3952a9449f"}]}