)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"827524339d22e1f95d1a5c3fb86cc8941f7a7e3d","unresolved":false,"context_lines":[{"line_number":14,"context_line":"This is achieved by converting the parameter and property into a"},{"line_number":15,"context_line":"resource request for one slot of the MEM_ENCRYPTION_CONTEXT inventory"},{"line_number":16,"context_line":"provided by Ib1304e65f1."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"Change-Id: I8c63b5cc5ad97ce831adb2eb96a995ebc798ecb7"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"9fb8cfa7_55073dcb","line":17,"updated":"2019-06-10 20:51:17.000000000","message":"blueprint: amd-sev-libvirt-support","commit_id":"a662fd015ea6e48d316a9fa5cf5e282b4a81735f"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"168d23260646c7feef98a68090bc74f4e618bc6e","unresolved":false,"context_lines":[{"line_number":14,"context_line":"This is achieved by converting the parameter and property into a"},{"line_number":15,"context_line":"resource request for one slot of the MEM_ENCRYPTION_CONTEXT inventory"},{"line_number":16,"context_line":"provided by Ib1304e65f1."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"Change-Id: I8c63b5cc5ad97ce831adb2eb96a995ebc798ecb7"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"9fb8cfa7_95193523","line":17,"in_reply_to":"9fb8cfa7_55073dcb","updated":"2019-06-11 15:46:12.000000000","message":"Doh, I had a feeling there was something missing but couldn\u0027t pin it down :-)","commit_id":"a662fd015ea6e48d316a9fa5cf5e282b4a81735f"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"2d1773e21fa73211de61cce706e73e447582ba94","unresolved":false,"context_lines":[{"line_number":18,"context_line":""},{"line_number":19,"context_line":"Since future commits adding support for SEV to guest XML config will"},{"line_number":20,"context_line":"also need to know at launch-time whether memory encryption has been"},{"line_number":21,"context_line":"requested, add a reusable mem_encryption_requesters() function to the"},{"line_number":22,"context_line":"nova.virt.hardware library for detecting which of the extra spec /"},{"line_number":23,"context_line":"image property (if either) have requested encrypted memory."},{"line_number":24,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":12,"id":"9fb8cfa7_dd395ea3","line":21,"range":{"start_line":21,"start_character":49,"end_line":21,"end_character":51},"updated":"2019-06-20 14:07:02.000000000","message":"d","commit_id":"5032dc87e81d77033db36f79e4d56d78b9ad0155"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"9cd8dbf7c96d69a5f66ece9cedcdc79471e8a91a","unresolved":false,"context_lines":[{"line_number":18,"context_line":""},{"line_number":19,"context_line":"Since future commits adding support for SEV to guest XML config will"},{"line_number":20,"context_line":"also need to know at launch-time whether memory encryption has been"},{"line_number":21,"context_line":"requested, add a reusable mem_encryption_requesters() function to the"},{"line_number":22,"context_line":"nova.virt.hardware library for detecting which of the extra spec /"},{"line_number":23,"context_line":"image property (if either) have requested encrypted memory."},{"line_number":24,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":12,"id":"9fb8cfa7_44803a47","line":21,"range":{"start_line":21,"start_character":49,"end_line":21,"end_character":51},"in_reply_to":"9fb8cfa7_dd395ea3","updated":"2019-06-20 15:23:06.000000000","message":"Done","commit_id":"5032dc87e81d77033db36f79e4d56d78b9ad0155"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"2d1773e21fa73211de61cce706e73e447582ba94","unresolved":false,"context_lines":[{"line_number":27,"context_line":"a new generic FlavorImageConflict exception, which can also be useful"},{"line_number":28,"context_line":"in the future for handling other similar conflicts.  In this"},{"line_number":29,"context_line":"particular use case, FlavorImageConflict is raised by"},{"line_number":30,"context_line":"mem_encryption_requesters(), and then if caught during the request"},{"line_number":31,"context_line":"filter phase is re-raised as a RequestFilterFailed exception, or if"},{"line_number":32,"context_line":"caught during API call validation, it\u0027s re-raised as HTTPBadRequest."},{"line_number":33,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":12,"id":"9fb8cfa7_1dba9609","line":30,"range":{"start_line":30,"start_character":23,"end_line":30,"end_character":25},"updated":"2019-06-20 14:07:02.000000000","message":"d","commit_id":"5032dc87e81d77033db36f79e4d56d78b9ad0155"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"9cd8dbf7c96d69a5f66ece9cedcdc79471e8a91a","unresolved":false,"context_lines":[{"line_number":27,"context_line":"a new generic FlavorImageConflict exception, which can also be useful"},{"line_number":28,"context_line":"in the future for handling other similar conflicts.  In this"},{"line_number":29,"context_line":"particular use case, FlavorImageConflict is raised by"},{"line_number":30,"context_line":"mem_encryption_requesters(), and then if caught during the request"},{"line_number":31,"context_line":"filter phase is re-raised as a RequestFilterFailed exception, or if"},{"line_number":32,"context_line":"caught during API call validation, it\u0027s re-raised as HTTPBadRequest."},{"line_number":33,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":12,"id":"9fb8cfa7_e4960e95","line":30,"range":{"start_line":30,"start_character":23,"end_line":30,"end_character":25},"in_reply_to":"9fb8cfa7_1dba9609","updated":"2019-06-20 15:23:06.000000000","message":"Done","commit_id":"5032dc87e81d77033db36f79e4d56d78b9ad0155"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"76c6538dc5f828399684b8fbe835ae1a62cd439d","unresolved":false,"context_lines":[{"line_number":21,"context_line":"Since future commits adding support for SEV to guest XML config will"},{"line_number":22,"context_line":"also need to know at launch-time whether memory encryption has been"},{"line_number":23,"context_line":"requested, add a reusable mem_encryption_requested() function to the"},{"line_number":24,"context_line":"nova.virt.hardware library for detecting which of the extra spec /"},{"line_number":25,"context_line":"image property (if either) have requested encrypted memory."},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"If both the extra spec parameter and the image property are explicitly"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":31,"id":"7faddb67_2dbadfe0","line":24,"range":{"start_line":24,"start_character":0,"end_line":24,"end_character":18},"updated":"2019-08-09 15:20:10.000000000","message":"++ this is exactly where this should live","commit_id":"adec7baaf4d94ddea461558169359d152b6223fc"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"76c6538dc5f828399684b8fbe835ae1a62cd439d","unresolved":false,"context_lines":[{"line_number":26,"context_line":""},{"line_number":27,"context_line":"If both the extra spec parameter and the image property are explicitly"},{"line_number":28,"context_line":"specified, and they contradict each other, then log an error and raise"},{"line_number":29,"context_line":"a new generic FlavorImageConflict exception, which can also be useful"},{"line_number":30,"context_line":"in the future for handling other similar conflicts.  In this"},{"line_number":31,"context_line":"particular use case, FlavorImageConflict is raised by"},{"line_number":32,"context_line":"mem_encryption_requested(), and then if caught during the request"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":31,"id":"7faddb67_edb36709","line":29,"range":{"start_line":29,"start_character":14,"end_line":29,"end_character":33},"updated":"2019-08-09 15:20:10.000000000","message":"\\o/","commit_id":"adec7baaf4d94ddea461558169359d152b6223fc"}],"lower-constraints.txt":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"e5598df8e539473c81257cbdd7601b700ff00866","unresolved":false,"context_lines":[{"line_number":66,"context_line":"openstacksdk\u003d\u003d0.12.0"},{"line_number":67,"context_line":"os-brick\u003d\u003d2.6.1"},{"line_number":68,"context_line":"os-client-config\u003d\u003d1.29.0"},{"line_number":69,"context_line":"os-resource-classes\u003d\u003d0.4.0"},{"line_number":70,"context_line":"os-service-types\u003d\u003d1.2.0"},{"line_number":71,"context_line":"os-traits\u003d\u003d0.15.0"},{"line_number":72,"context_line":"os-vif\u003d\u003d1.14.0"}],"source_content_type":"text/plain","patch_set":19,"id":"7faddb67_d752f934","line":69,"updated":"2019-07-19 19:28:38.000000000","message":"*Technically* you don\u0027t need this until the last patch [1], because that\u0027s the first place you actually reference MEM_ENCRYPTION_CONTEXT from os-resource-classes. (Here you just reference it explicitly inside a string.)\n\n[1] https://review.opendev.org/#/c/666616/","commit_id":"e055df5f10e03498d27fa8a1063557562c857ac8"}],"nova/compute/api.py":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"e5598df8e539473c81257cbdd7601b700ff00866","unresolved":false,"context_lines":[{"line_number":566,"context_line":"            return"},{"line_number":567,"context_line":""},{"line_number":568,"context_line":"        image_properties \u003d image.get(\u0027properties\u0027, {})"},{"line_number":569,"context_line":""},{"line_number":570,"context_line":"        config_drive_option \u003d image_properties.get("},{"line_number":571,"context_line":"            \u0027img_config_drive\u0027, \u0027optional\u0027)"},{"line_number":572,"context_line":"        if config_drive_option not in [\u0027optional\u0027, \u0027mandatory\u0027]:"}],"source_content_type":"text/x-python","patch_set":19,"id":"7faddb67_b7e2dd9e","line":569,"updated":"2019-07-19 19:28:38.000000000","message":"unrelated whitespace change","commit_id":"e055df5f10e03498d27fa8a1063557562c857ac8"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"9732a31b1526e8a29133e24495970eb551e29145","unresolved":false,"context_lines":[{"line_number":566,"context_line":"            return"},{"line_number":567,"context_line":""},{"line_number":568,"context_line":"        image_properties \u003d image.get(\u0027properties\u0027, {})"},{"line_number":569,"context_line":""},{"line_number":570,"context_line":"        config_drive_option \u003d image_properties.get("},{"line_number":571,"context_line":"            \u0027img_config_drive\u0027, \u0027optional\u0027)"},{"line_number":572,"context_line":"        if config_drive_option not in [\u0027optional\u0027, \u0027mandatory\u0027]:"}],"source_content_type":"text/x-python","patch_set":19,"id":"7faddb67_7b7f39ab","line":569,"in_reply_to":"7faddb67_b7e2dd9e","updated":"2019-07-24 12:46:28.000000000","message":"Done","commit_id":"e055df5f10e03498d27fa8a1063557562c857ac8"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"e5598df8e539473c81257cbdd7601b700ff00866","unresolved":false,"context_lines":[{"line_number":652,"context_line":"                        servers_policies.ZERO_DISK_FLAVOR, fatal\u003dFalse):"},{"line_number":653,"context_line":"                    raise exception.BootFromVolumeRequiredForZeroDiskFlavor()"},{"line_number":654,"context_line":""},{"line_number":655,"context_line":"        image_meta \u003d _get_image_meta_obj(image)"},{"line_number":656,"context_line":"        API._validate_flavor_image_mem_encryption(instance_type, image_meta)"},{"line_number":657,"context_line":""},{"line_number":658,"context_line":"        API._validate_flavor_image_numa_pci("}],"source_content_type":"text/x-python","patch_set":19,"id":"7faddb67_b7909dee","line":655,"updated":"2019-07-19 19:28:38.000000000","message":"NTS: since PS13, this was moved from L569","commit_id":"e055df5f10e03498d27fa8a1063557562c857ac8"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"9732a31b1526e8a29133e24495970eb551e29145","unresolved":false,"context_lines":[{"line_number":652,"context_line":"                        servers_policies.ZERO_DISK_FLAVOR, fatal\u003dFalse):"},{"line_number":653,"context_line":"                    raise exception.BootFromVolumeRequiredForZeroDiskFlavor()"},{"line_number":654,"context_line":""},{"line_number":655,"context_line":"        image_meta \u003d _get_image_meta_obj(image)"},{"line_number":656,"context_line":"        API._validate_flavor_image_mem_encryption(instance_type, image_meta)"},{"line_number":657,"context_line":""},{"line_number":658,"context_line":"        API._validate_flavor_image_numa_pci("}],"source_content_type":"text/x-python","patch_set":19,"id":"7faddb67_7b2df9a6","line":655,"in_reply_to":"7faddb67_b7909dee","updated":"2019-07-24 12:46:28.000000000","message":"IIRC this was necessary in order to keep the tests green.  If this validation happened earlier, its (expected, correct) failure would break the tests which test the above validations.","commit_id":"e055df5f10e03498d27fa8a1063557562c857ac8"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"76c6538dc5f828399684b8fbe835ae1a62cd439d","unresolved":false,"context_lines":[{"line_number":656,"context_line":"            image, instance_type, validate_numa\u003dvalidate_numa,"},{"line_number":657,"context_line":"            validate_pci\u003dvalidate_pci)"},{"line_number":658,"context_line":""},{"line_number":659,"context_line":"    @staticmethod"},{"line_number":660,"context_line":"    def _validate_flavor_image_mem_encryption(instance_type, image):"},{"line_number":661,"context_line":"        \"\"\"Validate that the flavor and image don\u0027t make contradictory"},{"line_number":662,"context_line":"        requests regarding memory encryption."},{"line_number":663,"context_line":""},{"line_number":664,"context_line":"        :param instance_type: Flavor object"},{"line_number":665,"context_line":"        :param image: a dict representation of the image including properties"},{"line_number":666,"context_line":"        :raises: nova.exception.FlavorImageConflict"},{"line_number":667,"context_line":"        \"\"\""},{"line_number":668,"context_line":"        # This library function will raise the exception for us if"},{"line_number":669,"context_line":"        # necessary; if not, we can ignore the result returned."},{"line_number":670,"context_line":"        hardware.mem_encryption_requested(instance_type, image)"},{"line_number":671,"context_line":""},{"line_number":672,"context_line":"    @staticmethod"},{"line_number":673,"context_line":"    def _validate_flavor_image_numa_pci(image, instance_type,"}],"source_content_type":"text/x-python","patch_set":31,"id":"7faddb67_cd8f8b2c","line":670,"range":{"start_line":659,"start_character":0,"end_line":670,"end_character":63},"updated":"2019-08-09 15:20:10.000000000","message":"I said this to Sean Mooney in another review: the below function is already doing non-NUMA stuff (see: real-time), so would it make sense to rename that function and do this check as part of that?","commit_id":"adec7baaf4d94ddea461558169359d152b6223fc"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"42e91f1a8a93669cbf3ecf52ce9cd921e97265e4","unresolved":false,"context_lines":[{"line_number":656,"context_line":"            image, instance_type, validate_numa\u003dvalidate_numa,"},{"line_number":657,"context_line":"            validate_pci\u003dvalidate_pci)"},{"line_number":658,"context_line":""},{"line_number":659,"context_line":"    @staticmethod"},{"line_number":660,"context_line":"    def _validate_flavor_image_mem_encryption(instance_type, image):"},{"line_number":661,"context_line":"        \"\"\"Validate that the flavor and image don\u0027t make contradictory"},{"line_number":662,"context_line":"        requests regarding memory encryption."},{"line_number":663,"context_line":""},{"line_number":664,"context_line":"        :param instance_type: Flavor object"},{"line_number":665,"context_line":"        :param image: a dict representation of the image including properties"},{"line_number":666,"context_line":"        :raises: nova.exception.FlavorImageConflict"},{"line_number":667,"context_line":"        \"\"\""},{"line_number":668,"context_line":"        # This library function will raise the exception for us if"},{"line_number":669,"context_line":"        # necessary; if not, we can ignore the result returned."},{"line_number":670,"context_line":"        hardware.mem_encryption_requested(instance_type, image)"},{"line_number":671,"context_line":""},{"line_number":672,"context_line":"    @staticmethod"},{"line_number":673,"context_line":"    def _validate_flavor_image_numa_pci(image, instance_type,"}],"source_content_type":"text/x-python","patch_set":31,"id":"7faddb67_2869eabc","line":670,"range":{"start_line":659,"start_character":0,"end_line":670,"end_character":63},"in_reply_to":"7faddb67_cd8f8b2c","updated":"2019-08-19 13:10:31.000000000","message":"As mentioned before I\u0027m a strong believer in single responsibility functions, so if the below is doing non-NUMA stuff then I think it would be better to split the below into smaller pieces rather than making it even bigger.","commit_id":"adec7baaf4d94ddea461558169359d152b6223fc"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"4d2a8dca052c6d3cf68ef9f5afa91aaef8bdd754","unresolved":false,"context_lines":[{"line_number":650,"context_line":"                        servers_policies.ZERO_DISK_FLAVOR, fatal\u003dFalse):"},{"line_number":651,"context_line":"                    raise exception.BootFromVolumeRequiredForZeroDiskFlavor()"},{"line_number":652,"context_line":""},{"line_number":653,"context_line":"        image_meta \u003d _get_image_meta_obj(image)"},{"line_number":654,"context_line":"        API._validate_flavor_image_mem_encryption(instance_type, image_meta)"},{"line_number":655,"context_line":""},{"line_number":656,"context_line":"        API._validate_flavor_image_numa_pci("},{"line_number":657,"context_line":"            image, instance_type, validate_numa\u003dvalidate_numa,"}],"source_content_type":"text/x-python","patch_set":40,"id":"7faddb67_b63186a4","line":654,"range":{"start_line":653,"start_character":0,"end_line":654,"end_character":76},"updated":"2019-08-21 17:05:50.000000000","message":"I would _much_ rather we did this in \u0027_validate_flavor_image_numa_pci\u0027 (which needs to be renamed), and it seems mriedem agrees [1]. That function needs to be renamed but it can be a separate change after this has landed\n\n[1] https://review.opendev.org/#/c/671338/7/nova/compute/api.py","commit_id":"8706c91037b3bb636723e1f9fdf55996151792b2"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"4920ed0a766e20cf77f6a321891ae458895335c2","unresolved":false,"context_lines":[{"line_number":650,"context_line":"                        servers_policies.ZERO_DISK_FLAVOR, fatal\u003dFalse):"},{"line_number":651,"context_line":"                    raise exception.BootFromVolumeRequiredForZeroDiskFlavor()"},{"line_number":652,"context_line":""},{"line_number":653,"context_line":"        image_meta \u003d _get_image_meta_obj(image)"},{"line_number":654,"context_line":"        API._validate_flavor_image_mem_encryption(instance_type, image_meta)"},{"line_number":655,"context_line":""},{"line_number":656,"context_line":"        API._validate_flavor_image_numa_pci("},{"line_number":657,"context_line":"            image, instance_type, validate_numa\u003dvalidate_numa,"}],"source_content_type":"text/x-python","patch_set":40,"id":"7faddb67_d903150f","line":654,"range":{"start_line":653,"start_character":0,"end_line":654,"end_character":76},"in_reply_to":"7faddb67_196a0db2","updated":"2019-08-21 18:35:15.000000000","message":"matt pointed out that resize calls _validate_flavor_image_numa_pci but not this so that was his man concern whit how i extended this function.\n\nthat till applies.\ni would keep _validate_flavor_image_mem_encryption for now\nand call it in _validate_flavor_image_numa_pci\n\nand then we can rename _validate_flavor_image_numa_pci ot _validate_flavor_image_hw_properties to indicate it is validating anything in the HW: or HW_ namspaces and is nolnger numa or pci specific as sev and vpmem is not related to either numa or pci passhtouhg.","commit_id":"8706c91037b3bb636723e1f9fdf55996151792b2"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"ab67b7a01dc1c44f8fdc99c969f9d8a66e245b71","unresolved":false,"context_lines":[{"line_number":650,"context_line":"                        servers_policies.ZERO_DISK_FLAVOR, fatal\u003dFalse):"},{"line_number":651,"context_line":"                    raise exception.BootFromVolumeRequiredForZeroDiskFlavor()"},{"line_number":652,"context_line":""},{"line_number":653,"context_line":"        image_meta \u003d _get_image_meta_obj(image)"},{"line_number":654,"context_line":"        API._validate_flavor_image_mem_encryption(instance_type, image_meta)"},{"line_number":655,"context_line":""},{"line_number":656,"context_line":"        API._validate_flavor_image_numa_pci("},{"line_number":657,"context_line":"            image, instance_type, validate_numa\u003dvalidate_numa,"}],"source_content_type":"text/x-python","patch_set":40,"id":"7faddb67_196a0db2","line":654,"range":{"start_line":653,"start_character":0,"end_line":654,"end_character":76},"in_reply_to":"7faddb67_b63186a4","updated":"2019-08-21 17:50:25.000000000","message":"But *why* do you have that preference?  As I already mentioned the first time you stated it[1], I have a strong preference for single responsibility functions.  The advantages of this are numerous and (I\u0027m assuming) obvious (let me know if not), but I can\u0027t understand what counter-advantages there would be in this case by lumping multiple things into the same function.  If I\u0027m missing something I\u0027d really appreciate help understanding what.\n\nIIUC mriedem\u0027s comment was about a slightly but significantly different situation, because he was asking why the new validation was placed in the \"parent\" method _validate_flavor_image_nostatus() rather than in the to-be-renamed \"child\" method _validate_flavor_image_numa_pci().  In contrast, my patch adds a *new* child method and calls that from the parent, thereby avoiding further bloating either of the existing methods.\n\n[1] https://review.opendev.org/#/c/664420/31/nova/compute/api.py@670","commit_id":"8706c91037b3bb636723e1f9fdf55996151792b2"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"915842f7cf5a5337d6201addf8ae8b8db794f357","unresolved":false,"context_lines":[{"line_number":650,"context_line":"                        servers_policies.ZERO_DISK_FLAVOR, fatal\u003dFalse):"},{"line_number":651,"context_line":"                    raise exception.BootFromVolumeRequiredForZeroDiskFlavor()"},{"line_number":652,"context_line":""},{"line_number":653,"context_line":"        image_meta \u003d _get_image_meta_obj(image)"},{"line_number":654,"context_line":"        API._validate_flavor_image_mem_encryption(instance_type, image_meta)"},{"line_number":655,"context_line":""},{"line_number":656,"context_line":"        API._validate_flavor_image_numa_pci("},{"line_number":657,"context_line":"            image, instance_type, validate_numa\u003dvalidate_numa,"}],"source_content_type":"text/x-python","patch_set":40,"id":"7faddb67_842bf0e8","line":654,"range":{"start_line":653,"start_character":0,"end_line":654,"end_character":76},"in_reply_to":"7faddb67_d903150f","updated":"2019-08-21 19:17:49.000000000","message":"Done","commit_id":"8706c91037b3bb636723e1f9fdf55996151792b2"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"54fd07dd751a6f2a6a285de21b3c454e3b412d0c","unresolved":false,"context_lines":[{"line_number":650,"context_line":"                        servers_policies.ZERO_DISK_FLAVOR, fatal\u003dFalse):"},{"line_number":651,"context_line":"                    raise exception.BootFromVolumeRequiredForZeroDiskFlavor()"},{"line_number":652,"context_line":""},{"line_number":653,"context_line":"        image_meta \u003d _get_image_meta_obj(image)"},{"line_number":654,"context_line":"        API._validate_flavor_image_mem_encryption(instance_type, image_meta)"},{"line_number":655,"context_line":""},{"line_number":656,"context_line":"        API._validate_flavor_image_numa_pci("},{"line_number":657,"context_line":"            image, instance_type, validate_numa\u003dvalidate_numa,"}],"source_content_type":"text/x-python","patch_set":40,"id":"7faddb67_448098f7","line":654,"range":{"start_line":653,"start_character":0,"end_line":654,"end_character":76},"in_reply_to":"7faddb67_d903150f","updated":"2019-08-21 19:03:33.000000000","message":"OK thanks Sean, that makes a lot more sense to me now.","commit_id":"8706c91037b3bb636723e1f9fdf55996151792b2"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"4920ed0a766e20cf77f6a321891ae458895335c2","unresolved":false,"context_lines":[{"line_number":668,"context_line":"        \"\"\""},{"line_number":669,"context_line":"        # This library function will raise the exception for us if"},{"line_number":670,"context_line":"        # necessary; if not, we can ignore the result returned."},{"line_number":671,"context_line":"        hardware.get_mem_encryption_constraint(instance_type, image)"},{"line_number":672,"context_line":""},{"line_number":673,"context_line":"    @staticmethod"},{"line_number":674,"context_line":"    def _validate_flavor_image_numa_pci(image, instance_type,"}],"source_content_type":"text/x-python","patch_set":40,"id":"7faddb67_c4180877","line":671,"range":{"start_line":671,"start_character":6,"end_line":671,"end_character":68},"updated":"2019-08-21 18:35:15.000000000","message":"with that said since you are just calling a singe fucntion i can see the argument for not haveing it be separate but i would counter that with its easier to test.","commit_id":"8706c91037b3bb636723e1f9fdf55996151792b2"}],"nova/objects/image_meta.py":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"827524339d22e1f95d1a5c3fb86cc8941f7a7e3d","unresolved":false,"context_lines":[{"line_number":305,"context_line":"        # boolean indicating that the guest needs to be booted with"},{"line_number":306,"context_line":"        # encrypted memory"},{"line_number":307,"context_line":"        \u0027hw_mem_encryption\u0027: fields.FlexibleBooleanField(),"},{"line_number":308,"context_line":""},{"line_number":309,"context_line":"        # One of the magic strings \u0027small\u0027, \u0027any\u0027, \u0027large\u0027"},{"line_number":310,"context_line":"        # or an explicit page size in KB (eg 4, 2048, ...)"},{"line_number":311,"context_line":"        \u0027hw_mem_page_size\u0027: fields.StringField(),"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_1558e5f7","line":308,"updated":"2019-06-10 20:51:17.000000000","message":"This delta seems sane.\n\nYou\u0027ll need to fix up the hash in test_objects.py.","commit_id":"a662fd015ea6e48d316a9fa5cf5e282b4a81735f"}],"nova/scheduler/request_filter.py":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"827524339d22e1f95d1a5c3fb86cc8941f7a7e3d","unresolved":false,"context_lines":[{"line_number":160,"context_line":"    \"\"\"When the hw:mem_encryption extra spec or the hw_mem_encryption"},{"line_number":161,"context_line":"    image property are requested, require hosts which can support"},{"line_number":162,"context_line":"    encryption of the guest memory."},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"    Currently AMD SEV is the only technology providing this which nova"},{"line_number":165,"context_line":"    supports."},{"line_number":166,"context_line":"    \"\"\""},{"line_number":167,"context_line":"    def _mem_encryption_extra_spec(request_spec):"},{"line_number":168,"context_line":"        if \u0027flavor\u0027 not in request_spec:"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_d5424d3e","line":165,"range":{"start_line":163,"start_character":0,"end_line":165,"end_character":13},"updated":"2019-06-10 20:51:17.000000000","message":"Not sure this is worth including. Since support by other technologies might not need to touch this filter, it would be easy to miss updating this.","commit_id":"a662fd015ea6e48d316a9fa5cf5e282b4a81735f"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"168d23260646c7feef98a68090bc74f4e618bc6e","unresolved":false,"context_lines":[{"line_number":160,"context_line":"    \"\"\"When the hw:mem_encryption extra spec or the hw_mem_encryption"},{"line_number":161,"context_line":"    image property are requested, require hosts which can support"},{"line_number":162,"context_line":"    encryption of the guest memory."},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"    Currently AMD SEV is the only technology providing this which nova"},{"line_number":165,"context_line":"    supports."},{"line_number":166,"context_line":"    \"\"\""},{"line_number":167,"context_line":"    def _mem_encryption_extra_spec(request_spec):"},{"line_number":168,"context_line":"        if \u0027flavor\u0027 not in request_spec:"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_35ae4998","line":165,"range":{"start_line":163,"start_character":0,"end_line":165,"end_character":13},"in_reply_to":"9fb8cfa7_d5424d3e","updated":"2019-06-11 15:46:12.000000000","message":"Done","commit_id":"a662fd015ea6e48d316a9fa5cf5e282b4a81735f"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"827524339d22e1f95d1a5c3fb86cc8941f7a7e3d","unresolved":false,"context_lines":[{"line_number":171,"context_line":"        if \u0027extra_specs\u0027 not in request_spec.flavor:"},{"line_number":172,"context_line":"            return False"},{"line_number":173,"context_line":""},{"line_number":174,"context_line":"        return strutils.bool_from_string("},{"line_number":175,"context_line":"            request_spec.flavor.extra_specs.get(\u0027hw:mem_encryption\u0027, False))"},{"line_number":176,"context_line":""},{"line_number":177,"context_line":"    def _mem_encryption_image_prop(request_spec):"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_35a889d9","line":174,"range":{"start_line":174,"start_character":15,"end_line":174,"end_character":40},"updated":"2019-06-10 20:51:17.000000000","message":"this is (apparently) unnecessary because the FlexibleBooleanField type automagically coerces for you.","commit_id":"a662fd015ea6e48d316a9fa5cf5e282b4a81735f"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"168d23260646c7feef98a68090bc74f4e618bc6e","unresolved":false,"context_lines":[{"line_number":171,"context_line":"        if \u0027extra_specs\u0027 not in request_spec.flavor:"},{"line_number":172,"context_line":"            return False"},{"line_number":173,"context_line":""},{"line_number":174,"context_line":"        return strutils.bool_from_string("},{"line_number":175,"context_line":"            request_spec.flavor.extra_specs.get(\u0027hw:mem_encryption\u0027, False))"},{"line_number":176,"context_line":""},{"line_number":177,"context_line":"    def _mem_encryption_image_prop(request_spec):"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_f593d159","line":174,"range":{"start_line":174,"start_character":15,"end_line":174,"end_character":40},"in_reply_to":"9fb8cfa7_35a889d9","updated":"2019-06-11 15:46:12.000000000","message":"Nope, tried removing this and it broke stuff.  Turns out the extra_specs field is actually a DictOfStringsField.\n\nHowever, the image property is indeed a FlexibleBooleanField, so I can remove it there.","commit_id":"a662fd015ea6e48d316a9fa5cf5e282b4a81735f"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"d163373fcb53335e3dd76ef14f7bd3bf61aa2165","unresolved":false,"context_lines":[{"line_number":171,"context_line":"        if \u0027extra_specs\u0027 not in request_spec.flavor:"},{"line_number":172,"context_line":"            return False"},{"line_number":173,"context_line":""},{"line_number":174,"context_line":"        return strutils.bool_from_string("},{"line_number":175,"context_line":"            request_spec.flavor.extra_specs.get(\u0027hw:mem_encryption\u0027, False))"},{"line_number":176,"context_line":""},{"line_number":177,"context_line":"    def _mem_encryption_image_prop(request_spec):"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_32e96db6","line":174,"range":{"start_line":174,"start_character":15,"end_line":174,"end_character":40},"in_reply_to":"9fb8cfa7_f593d159","updated":"2019-06-11 16:26:18.000000000","message":"oh, sorry, that\u0027s what I meant","commit_id":"a662fd015ea6e48d316a9fa5cf5e282b4a81735f"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"827524339d22e1f95d1a5c3fb86cc8941f7a7e3d","unresolved":false,"context_lines":[{"line_number":184,"context_line":"        return strutils.bool_from_string("},{"line_number":185,"context_line":"            request_spec.image.properties.get(\u0027hw_mem_encryption\u0027, False))"},{"line_number":186,"context_line":""},{"line_number":187,"context_line":"    if _mem_encryption_extra_spec(request_spec) or \\"},{"line_number":188,"context_line":"       _mem_encryption_image_prop(request_spec):"},{"line_number":189,"context_line":"        request_spec.flavor.extra_specs["},{"line_number":190,"context_line":"            \u0027resources:MEM_ENCRYPTION_CONTEXT\u0027] \u003d \u00271\u0027"},{"line_number":191,"context_line":"        return True"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_75a281ff","line":188,"range":{"start_line":187,"start_character":0,"end_line":188,"end_character":48},"updated":"2019-06-10 20:51:17.000000000","message":"1) Prefer parens to backslash\n\n2) We should (again) consider what happens if both the flavor and the image specify the key, but with conflicting values. Specifically:\n\n- If the admin (via the flavor) explicitly said False, is it okay for the user (via the image) to override that by saying True? Thereby consuming a potentially precious resource (a MEM_ENCRYPTION_CONTEXT) and hiding his guest\u0027s memory from the admin who might have wanted the ability to snoop it (hey, stranger things have happened).\n- If the admin explicitly said True, is it okay for the user to say False? Thereby allowing snooping of the guest\u0027s memory when the admin didn\u0027t want that.\n\nCorrect answer in both cases: no.\n\nSo if that happens, do we a) log a warning and give the flavor precedence, or b) raise an exception and refuse to continue?\n\nIn this case, since we\u0027re talking about security stuff, I say b), fail hard and fast.","commit_id":"a662fd015ea6e48d316a9fa5cf5e282b4a81735f"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"168d23260646c7feef98a68090bc74f4e618bc6e","unresolved":false,"context_lines":[{"line_number":184,"context_line":"        return strutils.bool_from_string("},{"line_number":185,"context_line":"            request_spec.image.properties.get(\u0027hw_mem_encryption\u0027, False))"},{"line_number":186,"context_line":""},{"line_number":187,"context_line":"    if _mem_encryption_extra_spec(request_spec) or \\"},{"line_number":188,"context_line":"       _mem_encryption_image_prop(request_spec):"},{"line_number":189,"context_line":"        request_spec.flavor.extra_specs["},{"line_number":190,"context_line":"            \u0027resources:MEM_ENCRYPTION_CONTEXT\u0027] \u003d \u00271\u0027"},{"line_number":191,"context_line":"        return True"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_eb5e9242","line":188,"range":{"start_line":187,"start_character":0,"end_line":188,"end_character":48},"in_reply_to":"9fb8cfa7_75a281ff","updated":"2019-06-11 15:46:12.000000000","message":"\u003e 1) Prefer parens to backslash\n\nDone.\n\n \u003e 2) We should (again) consider what happens if both the flavor and the image specify the key, but with conflicting values.\n\nExcellent point which hadn\u0027t ever occurred to me!\n\n \u003e Specifically:\n \u003e \n \u003e - If the admin (via the flavor) explicitly said False, is it okay for the user (via the image) to override that by saying True? Thereby consuming a potentially precious resource (a MEM_ENCRYPTION_CONTEXT) and hiding his guest\u0027s memory from the admin who might have wanted the ability to snoop it (hey, stranger things have happened).\n\nIf the flavor explicitly said hw:mem_encryption\u003dFalse, the user could probably just pick another flavor.  And it seems very unlikely that every flavor would explicitly have False; if the admin wanted to prevent usage of SEV, they would just set num_memory_encrypted_guests to 0, or configure the kernel parameter to 0, or (much less sensibly) run a version of QEMU/libvirt/nova which didn\u0027t support it, or one of several other options.\n\n \u003e - If the admin explicitly said True, is it okay for the user to say False? Thereby allowing snooping of the guest\u0027s memory when the admin didn\u0027t want that.\n\nWell, again flavor characteristics aren\u0027t purely the choice of the admin, because the user chooses which flavor to use.  And it\u0027s hard to imagine a scenarios where the admin wants to force all instances to have encrypted memory, because encrypted memory is intended for the benefit of the user, protecting them against a malicious admin rather than vice versa.  One could maybe make the argument that an admin might want all instances to be encrypted so that the user can\u0027t ever accuse the admin of violating their privacy by prying into guest memory contents, but I\u0027m not sure that the attestation process would allow the admin to prove that they couldn\u0027t have engaged in such privacy violations, since it\u0027s designed to allow the *user* to prove that.\n\n \u003e Correct answer in both cases: no.\n\nI don\u0027t necessarily disagree, but I\u0027m not sure it\u0027s quite as cut and dried as you portray.\n\n \u003e So if that happens, do we a) log a warning and give the flavor precedence, or b) raise an exception and refuse to continue?\n \u003e \n \u003e In this case, since we\u0027re talking about security stuff, I say b), fail hard and fast.\n\nI\u0027m fine with that and I\u0027ve implemented it in the next patch set, but I think this warrants a bit more research (a.k.a. asking my AMD / SUSE contacts) how they envisage these use cases working out.  I\u0027ve just been on a conf call with AMD and their initial reaction to this scenario is to agree with your suggestion of bailing when there\u0027s a conflict regarding what has been requested.","commit_id":"a662fd015ea6e48d316a9fa5cf5e282b4a81735f"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"827524339d22e1f95d1a5c3fb86cc8941f7a7e3d","unresolved":false,"context_lines":[{"line_number":188,"context_line":"       _mem_encryption_image_prop(request_spec):"},{"line_number":189,"context_line":"        request_spec.flavor.extra_specs["},{"line_number":190,"context_line":"            \u0027resources:MEM_ENCRYPTION_CONTEXT\u0027] \u003d \u00271\u0027"},{"line_number":191,"context_line":"        return True"},{"line_number":192,"context_line":""},{"line_number":193,"context_line":"    return False"},{"line_number":194,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_f5499130","line":191,"updated":"2019-06-10 20:51:17.000000000","message":"Wanna throw some debug logging in here? See (recent additions to) other filters.","commit_id":"a662fd015ea6e48d316a9fa5cf5e282b4a81735f"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"168d23260646c7feef98a68090bc74f4e618bc6e","unresolved":false,"context_lines":[{"line_number":188,"context_line":"       _mem_encryption_image_prop(request_spec):"},{"line_number":189,"context_line":"        request_spec.flavor.extra_specs["},{"line_number":190,"context_line":"            \u0027resources:MEM_ENCRYPTION_CONTEXT\u0027] \u003d \u00271\u0027"},{"line_number":191,"context_line":"        return True"},{"line_number":192,"context_line":""},{"line_number":193,"context_line":"    return False"},{"line_number":194,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_15b0e5dc","line":191,"in_reply_to":"9fb8cfa7_f5499130","updated":"2019-06-11 15:46:12.000000000","message":"Done","commit_id":"a662fd015ea6e48d316a9fa5cf5e282b4a81735f"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"d163373fcb53335e3dd76ef14f7bd3bf61aa2165","unresolved":false,"context_lines":[{"line_number":193,"context_line":"    # First check for conflicts between explicit requirements"},{"line_number":194,"context_line":"    # regarding memory encryption."},{"line_number":195,"context_line":"    if extra_spec is not None and image_prop is not None:"},{"line_number":196,"context_line":"        if extra_spec !\u003d image_prop:"},{"line_number":197,"context_line":"            msg \u003d _("},{"line_number":198,"context_line":"                \"Flavor %(flavor_name)s has hw:mem_encryption extra spec \""},{"line_number":199,"context_line":"                \"explicitly set to %(flavor_val)s, conflicting with \""}],"source_content_type":"text/x-python","patch_set":2,"id":"9fb8cfa7_12e789ae","line":196,"range":{"start_line":196,"start_character":8,"end_line":196,"end_character":36},"updated":"2019-06-11 16:26:18.000000000","message":"nit: could combine this with previous condition","commit_id":"28513132d9afcb053247b7a15e04d32be57f8862"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"e3223b6d45b7be656235889eab8b888d37f4d4f1","unresolved":false,"context_lines":[{"line_number":193,"context_line":"    # First check for conflicts between explicit requirements"},{"line_number":194,"context_line":"    # regarding memory encryption."},{"line_number":195,"context_line":"    if extra_spec is not None and image_prop is not None:"},{"line_number":196,"context_line":"        if extra_spec !\u003d image_prop:"},{"line_number":197,"context_line":"            msg \u003d _("},{"line_number":198,"context_line":"                \"Flavor %(flavor_name)s has hw:mem_encryption extra spec \""},{"line_number":199,"context_line":"                \"explicitly set to %(flavor_val)s, conflicting with \""}],"source_content_type":"text/x-python","patch_set":2,"id":"9fb8cfa7_8ba7c4bd","line":196,"range":{"start_line":196,"start_character":8,"end_line":196,"end_character":36},"in_reply_to":"9fb8cfa7_12e789ae","updated":"2019-06-11 19:03:34.000000000","message":"Done","commit_id":"28513132d9afcb053247b7a15e04d32be57f8862"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"d163373fcb53335e3dd76ef14f7bd3bf61aa2165","unresolved":false,"context_lines":[{"line_number":209,"context_line":"                    request_spec.image.properties.get(\u0027hw_mem_encryption\u0027)"},{"line_number":210,"context_line":"            }"},{"line_number":211,"context_line":"            LOG.error(msg, data)"},{"line_number":212,"context_line":"            raise exception.RequestFilterFailed(reason\u003dmsg % data)"},{"line_number":213,"context_line":""},{"line_number":214,"context_line":"    # If we get this far, either the extra spec or image property explicitly"},{"line_number":215,"context_line":"    # specified a requirement regarding memory encryption, and if both did,"}],"source_content_type":"text/x-python","patch_set":2,"id":"9fb8cfa7_75efcf1d","line":212,"updated":"2019-06-11 16:26:18.000000000","message":"I know it sucks, but we don\u0027t translate logs, so you\u0027re going to have to use separate strings for the log and the exception.","commit_id":"28513132d9afcb053247b7a15e04d32be57f8862"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"e3223b6d45b7be656235889eab8b888d37f4d4f1","unresolved":false,"context_lines":[{"line_number":209,"context_line":"                    request_spec.image.properties.get(\u0027hw_mem_encryption\u0027)"},{"line_number":210,"context_line":"            }"},{"line_number":211,"context_line":"            LOG.error(msg, data)"},{"line_number":212,"context_line":"            raise exception.RequestFilterFailed(reason\u003dmsg % data)"},{"line_number":213,"context_line":""},{"line_number":214,"context_line":"    # If we get this far, either the extra spec or image property explicitly"},{"line_number":215,"context_line":"    # specified a requirement regarding memory encryption, and if both did,"}],"source_content_type":"text/x-python","patch_set":2,"id":"9fb8cfa7_6bde1012","line":212,"in_reply_to":"9fb8cfa7_75efcf1d","updated":"2019-06-11 19:03:34.000000000","message":"So basically the majority of https://docs.openstack.org/oslo.i18n/latest/user/guidelines.html#log-translation is wrong :-(  Burying a small warning message in the middle of a long page isn\u0027t sufficient to negate large chunks of advice which follow it.  I even saw the warning, discussed it on IRC, and still came to the conclusion this was the right thing to do.\n\nI\u0027ve submitted https://review.opendev.org/#/c/664670/ to address this.","commit_id":"28513132d9afcb053247b7a15e04d32be57f8862"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"d163373fcb53335e3dd76ef14f7bd3bf61aa2165","unresolved":false,"context_lines":[{"line_number":214,"context_line":"    # If we get this far, either the extra spec or image property explicitly"},{"line_number":215,"context_line":"    # specified a requirement regarding memory encryption, and if both did,"},{"line_number":216,"context_line":"    # they are asking for the same thing."},{"line_number":217,"context_line":"    if not extra_spec and not image_prop:"},{"line_number":218,"context_line":"        # Neither require memory encryption, so no further action required."},{"line_number":219,"context_line":"        return False"},{"line_number":220,"context_line":""},{"line_number":221,"context_line":"    request_spec.flavor.extra_specs[\u0027resources:MEM_ENCRYPTION_CONTEXT\u0027] \u003d \u00271\u0027"},{"line_number":222,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"9fb8cfa7_12cea92c","line":219,"range":{"start_line":217,"start_character":0,"end_line":219,"end_character":20},"updated":"2019-06-11 16:26:18.000000000","message":"nit, could do this one first","commit_id":"28513132d9afcb053247b7a15e04d32be57f8862"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"e3223b6d45b7be656235889eab8b888d37f4d4f1","unresolved":false,"context_lines":[{"line_number":214,"context_line":"    # If we get this far, either the extra spec or image property explicitly"},{"line_number":215,"context_line":"    # specified a requirement regarding memory encryption, and if both did,"},{"line_number":216,"context_line":"    # they are asking for the same thing."},{"line_number":217,"context_line":"    if not extra_spec and not image_prop:"},{"line_number":218,"context_line":"        # Neither require memory encryption, so no further action required."},{"line_number":219,"context_line":"        return False"},{"line_number":220,"context_line":""},{"line_number":221,"context_line":"    request_spec.flavor.extra_specs[\u0027resources:MEM_ENCRYPTION_CONTEXT\u0027] \u003d \u00271\u0027"},{"line_number":222,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"9fb8cfa7_4bad4c99","line":219,"range":{"start_line":217,"start_character":0,"end_line":219,"end_character":20},"in_reply_to":"9fb8cfa7_12cea92c","updated":"2019-06-11 19:03:34.000000000","message":"Done","commit_id":"28513132d9afcb053247b7a15e04d32be57f8862"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"d163373fcb53335e3dd76ef14f7bd3bf61aa2165","unresolved":false,"context_lines":[{"line_number":230,"context_line":"        LOG.error(\"BUG: require_encrypted_memory_support \""},{"line_number":231,"context_line":"                  \"request filter was applied for unknown reason\")"},{"line_number":232,"context_line":""},{"line_number":233,"context_line":"    LOG.debug(\u0027require_encrypted_memory_support request filter added \u0027"},{"line_number":234,"context_line":"              \u0027required resource MEM_ENCRYPTION_CONTEXT due to %s\u0027,"},{"line_number":235,"context_line":"              \u0027 and \u0027.join(requesters))"},{"line_number":236,"context_line":"    return True"}],"source_content_type":"text/x-python","patch_set":2,"id":"9fb8cfa7_955123dc","line":233,"updated":"2019-06-11 16:26:18.000000000","message":"nit: else","commit_id":"28513132d9afcb053247b7a15e04d32be57f8862"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"4154137f0fb7dcf57b2399ce4713d47c8a90e7ad","unresolved":false,"context_lines":[{"line_number":230,"context_line":"        LOG.error(\"BUG: require_encrypted_memory_support \""},{"line_number":231,"context_line":"                  \"request filter was applied for unknown reason\")"},{"line_number":232,"context_line":""},{"line_number":233,"context_line":"    LOG.debug(\u0027require_encrypted_memory_support request filter added \u0027"},{"line_number":234,"context_line":"              \u0027required resource MEM_ENCRYPTION_CONTEXT due to %s\u0027,"},{"line_number":235,"context_line":"              \u0027 and \u0027.join(requesters))"},{"line_number":236,"context_line":"    return True"}],"source_content_type":"text/x-python","patch_set":2,"id":"9fb8cfa7_a6567f29","line":233,"in_reply_to":"9fb8cfa7_86901b4c","updated":"2019-06-11 19:32:38.000000000","message":"I mean\n\n if not requesters:\n     # This shouldn\u0027t happen\n     LOG.error(\"filter applied for unknown reason\")\n else:\n     LOG.debug(\"filter applied due to %s\", ...)\n\nOtherwise if the theoretically-unreachable code is reached, this message will end up as \"require_encrypted_memory_support request filter added required resource MEM_ENCRYPTION_CONTEXT due to \" and be emitted in addition to the error log saying the same thing (but more clearly).\n\nLike I said, a nit, considering how unlikely it is to happen.","commit_id":"28513132d9afcb053247b7a15e04d32be57f8862"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"e3223b6d45b7be656235889eab8b888d37f4d4f1","unresolved":false,"context_lines":[{"line_number":230,"context_line":"        LOG.error(\"BUG: require_encrypted_memory_support \""},{"line_number":231,"context_line":"                  \"request filter was applied for unknown reason\")"},{"line_number":232,"context_line":""},{"line_number":233,"context_line":"    LOG.debug(\u0027require_encrypted_memory_support request filter added \u0027"},{"line_number":234,"context_line":"              \u0027required resource MEM_ENCRYPTION_CONTEXT due to %s\u0027,"},{"line_number":235,"context_line":"              \u0027 and \u0027.join(requesters))"},{"line_number":236,"context_line":"    return True"}],"source_content_type":"text/x-python","patch_set":2,"id":"9fb8cfa7_86901b4c","line":233,"in_reply_to":"9fb8cfa7_955123dc","updated":"2019-06-11 19:03:34.000000000","message":"Not sure what you mean here?","commit_id":"28513132d9afcb053247b7a15e04d32be57f8862"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"48b3f249c7590ce695b9fc53702414e40fafd65e","unresolved":false,"context_lines":[{"line_number":230,"context_line":"        LOG.error(\"BUG: require_encrypted_memory_support \""},{"line_number":231,"context_line":"                  \"request filter was applied for unknown reason\")"},{"line_number":232,"context_line":""},{"line_number":233,"context_line":"    LOG.debug(\u0027require_encrypted_memory_support request filter added \u0027"},{"line_number":234,"context_line":"              \u0027required resource MEM_ENCRYPTION_CONTEXT due to %s\u0027,"},{"line_number":235,"context_line":"              \u0027 and \u0027.join(requesters))"},{"line_number":236,"context_line":"    return True"}],"source_content_type":"text/x-python","patch_set":2,"id":"9fb8cfa7_b34d483f","line":233,"in_reply_to":"9fb8cfa7_a6567f29","updated":"2019-06-12 16:38:54.000000000","message":"Ah!  Nice suggestion thanks - done.","commit_id":"28513132d9afcb053247b7a15e04d32be57f8862"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"4154137f0fb7dcf57b2399ce4713d47c8a90e7ad","unresolved":false,"context_lines":[{"line_number":220,"context_line":"                request_spec.image.properties.get(\u0027hw_mem_encryption\u0027)"},{"line_number":221,"context_line":"        }"},{"line_number":222,"context_line":"        LOG.error(lmsg, data)"},{"line_number":223,"context_line":"        raise exception.RequestFilterFailed(reason\u003demsg % data)"},{"line_number":224,"context_line":""},{"line_number":225,"context_line":"    # If we get this far, either the extra spec or image property explicitly"},{"line_number":226,"context_line":"    # specified a requirement regarding memory encryption, and if both did,"}],"source_content_type":"text/x-python","patch_set":3,"id":"9fb8cfa7_e141c13a","line":223,"updated":"2019-06-11 19:32:38.000000000","message":"Now that I\u0027ve made you go through the trouble of doing this, do we have an understanding of how this will appear in the wild? If the caller does a LOG.exception or similar, using the exception content, then the message will show up twice in the log, which is unnecessary/redundant.\n\n[Later] Looks like this gets converted to NoValidHosts, with the exception message embedded in it.","commit_id":"3905ff94bf250349ebe04256c36e800999884b2e"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"5dda86721a594b5e7768487ea89196c8f2cf1553","unresolved":false,"context_lines":[{"line_number":220,"context_line":"                request_spec.image.properties.get(\u0027hw_mem_encryption\u0027)"},{"line_number":221,"context_line":"        }"},{"line_number":222,"context_line":"        LOG.error(lmsg, data)"},{"line_number":223,"context_line":"        raise exception.RequestFilterFailed(reason\u003demsg % data)"},{"line_number":224,"context_line":""},{"line_number":225,"context_line":"    # If we get this far, either the extra spec or image property explicitly"},{"line_number":226,"context_line":"    # specified a requirement regarding memory encryption, and if both did,"}],"source_content_type":"text/x-python","patch_set":4,"id":"9fb8cfa7_b32ead8f","line":223,"updated":"2019-06-12 12:55:54.000000000","message":"This will lead to error status instance.\n\nSo prefer to validate the extra spec in the API layer, something like this https://github.com/openstack/nova/blob/master/nova/compute/api.py#L820","commit_id":"6d9e494a6a6477a60d933fab29fa9cfc6b688c1f"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"c8088118b6c45b0783c1a92c31446a4e66b2a2ee","unresolved":false,"context_lines":[{"line_number":220,"context_line":"                request_spec.image.properties.get(\u0027hw_mem_encryption\u0027)"},{"line_number":221,"context_line":"        }"},{"line_number":222,"context_line":"        LOG.error(lmsg, data)"},{"line_number":223,"context_line":"        raise exception.RequestFilterFailed(reason\u003demsg % data)"},{"line_number":224,"context_line":""},{"line_number":225,"context_line":"    # If we get this far, either the extra spec or image property explicitly"},{"line_number":226,"context_line":"    # specified a requirement regarding memory encryption, and if both did,"}],"source_content_type":"text/x-python","patch_set":4,"id":"9fb8cfa7_9fca3e18","line":223,"in_reply_to":"9fb8cfa7_59d98b54","updated":"2019-06-17 18:15:20.000000000","message":"Since PS6 moved the check into a reusable function in nova.virt.hardware, it was easy to add the API layer check to PS8.","commit_id":"6d9e494a6a6477a60d933fab29fa9cfc6b688c1f"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"5bcf5c1bebff45e005caad5a9930b52714564107","unresolved":false,"context_lines":[{"line_number":220,"context_line":"                request_spec.image.properties.get(\u0027hw_mem_encryption\u0027)"},{"line_number":221,"context_line":"        }"},{"line_number":222,"context_line":"        LOG.error(lmsg, data)"},{"line_number":223,"context_line":"        raise exception.RequestFilterFailed(reason\u003demsg % data)"},{"line_number":224,"context_line":""},{"line_number":225,"context_line":"    # If we get this far, either the extra spec or image property explicitly"},{"line_number":226,"context_line":"    # specified a requirement regarding memory encryption, and if both did,"}],"source_content_type":"text/x-python","patch_set":4,"id":"9fb8cfa7_f36e60c7","line":223,"in_reply_to":"9fb8cfa7_b32ead8f","updated":"2019-06-12 16:28:42.000000000","message":"@Eric, what do you think about this?","commit_id":"6d9e494a6a6477a60d933fab29fa9cfc6b688c1f"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"e47f18cf920c7d7ef29e4ea3af251444bbf75b8e","unresolved":false,"context_lines":[{"line_number":220,"context_line":"                request_spec.image.properties.get(\u0027hw_mem_encryption\u0027)"},{"line_number":221,"context_line":"        }"},{"line_number":222,"context_line":"        LOG.error(lmsg, data)"},{"line_number":223,"context_line":"        raise exception.RequestFilterFailed(reason\u003demsg % data)"},{"line_number":224,"context_line":""},{"line_number":225,"context_line":"    # If we get this far, either the extra spec or image property explicitly"},{"line_number":226,"context_line":"    # specified a requirement regarding memory encryption, and if both did,"}],"source_content_type":"text/x-python","patch_set":4,"id":"9fb8cfa7_59d98b54","line":223,"in_reply_to":"9fb8cfa7_f36e60c7","updated":"2019-06-12 18:19:30.000000000","message":"It would be a good thing to add a check in the API layer *as well*, probably via a separate change.\n\nBut we should keep this one.\n\nHow is this different from what\u0027s happening on L87 above?","commit_id":"6d9e494a6a6477a60d933fab29fa9cfc6b688c1f"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"70e57837d45b7cd65a3803f3dabd45aa926a4189","unresolved":false,"context_lines":[{"line_number":156,"context_line":""},{"line_number":157,"context_line":""},{"line_number":158,"context_line":"@trace_request_filter"},{"line_number":159,"context_line":"def require_encrypted_memory_support(ctxt, request_spec):"},{"line_number":160,"context_line":"    \"\"\"When the hw:mem_encryption extra spec or the hw_mem_encryption"},{"line_number":161,"context_line":"    image property are requested, require hosts which can support"},{"line_number":162,"context_line":"    encryption of the guest memory."}],"source_content_type":"text/x-python","patch_set":11,"id":"9fb8cfa7_0ff82dc2","line":159,"range":{"start_line":159,"start_character":4,"end_line":159,"end_character":12},"updated":"2019-06-19 20:50:23.000000000","message":"micronit, I would remove the \u0027require_\u0027 prefix here. It kind of implies you\u0027re doing this with a trait, which you\u0027re not.","commit_id":"31f4a45e1864c0175daf0ddb47f5a9a04b3beb08"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"18bc4382e0b7ba94f6f44669ec76d62526561e0e","unresolved":false,"context_lines":[{"line_number":156,"context_line":""},{"line_number":157,"context_line":""},{"line_number":158,"context_line":"@trace_request_filter"},{"line_number":159,"context_line":"def require_encrypted_memory_support(ctxt, request_spec):"},{"line_number":160,"context_line":"    \"\"\"When the hw:mem_encryption extra spec or the hw_mem_encryption"},{"line_number":161,"context_line":"    image property are requested, require hosts which can support"},{"line_number":162,"context_line":"    encryption of the guest memory."}],"source_content_type":"text/x-python","patch_set":11,"id":"9fb8cfa7_f5bfa8a7","line":159,"range":{"start_line":159,"start_character":4,"end_line":159,"end_character":12},"in_reply_to":"9fb8cfa7_0ff82dc2","updated":"2019-06-20 12:09:46.000000000","message":"Will do.","commit_id":"31f4a45e1864c0175daf0ddb47f5a9a04b3beb08"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"ed39148aba4c7aff9f1044511d03e65b6c8c1fa0","unresolved":false,"context_lines":[{"line_number":156,"context_line":""},{"line_number":157,"context_line":""},{"line_number":158,"context_line":"@trace_request_filter"},{"line_number":159,"context_line":"def require_encrypted_memory_support(ctxt, request_spec):"},{"line_number":160,"context_line":"    \"\"\"When the hw:mem_encryption extra spec or the hw_mem_encryption"},{"line_number":161,"context_line":"    image property are requested, require hosts which can support"},{"line_number":162,"context_line":"    encryption of the guest memory."}],"source_content_type":"text/x-python","patch_set":11,"id":"9fb8cfa7_67ca256f","line":159,"range":{"start_line":159,"start_character":4,"end_line":159,"end_character":12},"in_reply_to":"9fb8cfa7_f5bfa8a7","updated":"2019-06-20 13:53:45.000000000","message":"Done","commit_id":"31f4a45e1864c0175daf0ddb47f5a9a04b3beb08"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"e5598df8e539473c81257cbdd7601b700ff00866","unresolved":false,"context_lines":[{"line_number":204,"context_line":"        # No memory encryption required, so no further action required."},{"line_number":205,"context_line":"        return False"},{"line_number":206,"context_line":""},{"line_number":207,"context_line":"    request_spec.flavor.extra_specs[\u0027resources:MEM_ENCRYPTION_CONTEXT\u0027] \u003d \u00271\u0027"},{"line_number":208,"context_line":""},{"line_number":209,"context_line":"    LOG.debug(\"encrypted_memory_support request filter added \""},{"line_number":210,"context_line":"              \"required resource MEM_ENCRYPTION_CONTEXT\")"}],"source_content_type":"text/x-python","patch_set":19,"id":"7faddb67_b72c1dac","line":207,"range":{"start_line":207,"start_character":47,"end_line":207,"end_character":69},"updated":"2019-07-19 19:28:38.000000000","message":"nit: you could use os_resource_classes.MEM_ENCRYPTION_CONTEXT explicitly here. That would justify bumping l-c in this patch instead of in [1].\n\n[1] https://review.opendev.org/#/c/666616/","commit_id":"e055df5f10e03498d27fa8a1063557562c857ac8"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"9732a31b1526e8a29133e24495970eb551e29145","unresolved":false,"context_lines":[{"line_number":204,"context_line":"        # No memory encryption required, so no further action required."},{"line_number":205,"context_line":"        return False"},{"line_number":206,"context_line":""},{"line_number":207,"context_line":"    request_spec.flavor.extra_specs[\u0027resources:MEM_ENCRYPTION_CONTEXT\u0027] \u003d \u00271\u0027"},{"line_number":208,"context_line":""},{"line_number":209,"context_line":"    LOG.debug(\"encrypted_memory_support request filter added \""},{"line_number":210,"context_line":"              \"required resource MEM_ENCRYPTION_CONTEXT\")"}],"source_content_type":"text/x-python","patch_set":19,"id":"7faddb67_5ea58bb4","line":207,"range":{"start_line":207,"start_character":47,"end_line":207,"end_character":69},"in_reply_to":"7faddb67_b72c1dac","updated":"2019-07-24 12:46:28.000000000","message":"Oh, nice idea - thanks!  Done.","commit_id":"e055df5f10e03498d27fa8a1063557562c857ac8"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"76c6538dc5f828399684b8fbe835ae1a62cd439d","unresolved":false,"context_lines":[{"line_number":185,"context_line":"    encryption of the guest memory."},{"line_number":186,"context_line":"    \"\"\""},{"line_number":187,"context_line":""},{"line_number":188,"context_line":"    if \u0027flavor\u0027 in request_spec:"},{"line_number":189,"context_line":"        flavor \u003d request_spec.flavor"},{"line_number":190,"context_line":"    else:"},{"line_number":191,"context_line":"        flavor \u003d objects.Flavor()"},{"line_number":192,"context_line":""},{"line_number":193,"context_line":"    if \u0027image\u0027 in request_spec:"},{"line_number":194,"context_line":"        image_meta \u003d request_spec.image"}],"source_content_type":"text/x-python","patch_set":31,"id":"7faddb67_4d7cfbea","line":191,"range":{"start_line":188,"start_character":0,"end_line":191,"end_character":33},"updated":"2019-08-09 15:20:10.000000000","message":"Looking at the object definition in \u0027nova/objects/request_spec.py\u0027, this isn\u0027t nullable. Why would this be unset","commit_id":"adec7baaf4d94ddea461558169359d152b6223fc"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"42e91f1a8a93669cbf3ecf52ce9cd921e97265e4","unresolved":false,"context_lines":[{"line_number":185,"context_line":"    encryption of the guest memory."},{"line_number":186,"context_line":"    \"\"\""},{"line_number":187,"context_line":""},{"line_number":188,"context_line":"    if \u0027flavor\u0027 in request_spec:"},{"line_number":189,"context_line":"        flavor \u003d request_spec.flavor"},{"line_number":190,"context_line":"    else:"},{"line_number":191,"context_line":"        flavor \u003d objects.Flavor()"},{"line_number":192,"context_line":""},{"line_number":193,"context_line":"    if \u0027image\u0027 in request_spec:"},{"line_number":194,"context_line":"        image_meta \u003d request_spec.image"}],"source_content_type":"text/x-python","patch_set":31,"id":"7faddb67_68c7e299","line":191,"range":{"start_line":188,"start_character":0,"end_line":191,"end_character":33},"in_reply_to":"7faddb67_4d7cfbea","updated":"2019-08-19 13:10:31.000000000","message":"Good point.  It was only unset in the tests, but I\u0027ve fixed them to be more like real world environments.","commit_id":"adec7baaf4d94ddea461558169359d152b6223fc"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"76c6538dc5f828399684b8fbe835ae1a62cd439d","unresolved":false,"context_lines":[{"line_number":190,"context_line":"    else:"},{"line_number":191,"context_line":"        flavor \u003d objects.Flavor()"},{"line_number":192,"context_line":""},{"line_number":193,"context_line":"    if \u0027image\u0027 in request_spec:"},{"line_number":194,"context_line":"        image_meta \u003d request_spec.image"},{"line_number":195,"context_line":"    else:"},{"line_number":196,"context_line":"        image_meta \u003d objects.ImageMeta("},{"line_number":197,"context_line":"            properties\u003dobjects.ImageMetaProps())"},{"line_number":198,"context_line":""},{"line_number":199,"context_line":"    try:"},{"line_number":200,"context_line":"        requested \u003d mem_encryption_requested(flavor, image_meta)"}],"source_content_type":"text/x-python","patch_set":31,"id":"7faddb67_8d1c33c2","line":197,"range":{"start_line":193,"start_character":0,"end_line":197,"end_character":48},"updated":"2019-08-09 15:20:10.000000000","message":"Apparently this is nullable, but why would it be unset?","commit_id":"adec7baaf4d94ddea461558169359d152b6223fc"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"42e91f1a8a93669cbf3ecf52ce9cd921e97265e4","unresolved":false,"context_lines":[{"line_number":190,"context_line":"    else:"},{"line_number":191,"context_line":"        flavor \u003d objects.Flavor()"},{"line_number":192,"context_line":""},{"line_number":193,"context_line":"    if \u0027image\u0027 in request_spec:"},{"line_number":194,"context_line":"        image_meta \u003d request_spec.image"},{"line_number":195,"context_line":"    else:"},{"line_number":196,"context_line":"        image_meta \u003d objects.ImageMeta("},{"line_number":197,"context_line":"            properties\u003dobjects.ImageMetaProps())"},{"line_number":198,"context_line":""},{"line_number":199,"context_line":"    try:"},{"line_number":200,"context_line":"        requested \u003d mem_encryption_requested(flavor, image_meta)"}],"source_content_type":"text/x-python","patch_set":31,"id":"7faddb67_c83b7699","line":197,"range":{"start_line":193,"start_character":0,"end_line":197,"end_character":48},"in_reply_to":"7faddb67_8d1c33c2","updated":"2019-08-19 13:10:31.000000000","message":"Because some tests don\u0027t set it.  AFAICS it seems to be standard practice in nova to use incomplete fixtures which only contain the bits needed for the test(s) which use them, and that seems fine to me.","commit_id":"adec7baaf4d94ddea461558169359d152b6223fc"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"76c6538dc5f828399684b8fbe835ae1a62cd439d","unresolved":false,"context_lines":[{"line_number":196,"context_line":"        image_meta \u003d objects.ImageMeta("},{"line_number":197,"context_line":"            properties\u003dobjects.ImageMetaProps())"},{"line_number":198,"context_line":""},{"line_number":199,"context_line":"    try:"},{"line_number":200,"context_line":"        requested \u003d mem_encryption_requested(flavor, image_meta)"},{"line_number":201,"context_line":"    except exception.FlavorImageConflict as exc:"},{"line_number":202,"context_line":"        raise exception.RequestFilterFailed(reason\u003dexc.message)"},{"line_number":203,"context_line":""},{"line_number":204,"context_line":"    if not requested:"},{"line_number":205,"context_line":"        # No memory encryption required, so no further action required."}],"source_content_type":"text/x-python","patch_set":31,"id":"7faddb67_0d38433b","line":202,"range":{"start_line":199,"start_character":0,"end_line":202,"end_character":63},"updated":"2019-08-09 15:20:10.000000000","message":"Isn\u0027t this already handled at the API layer? If so, we don\u0027t need to care about this here\n\nLater: yes, we are. Kill it, perhaps with a comment saying that this has already been done","commit_id":"adec7baaf4d94ddea461558169359d152b6223fc"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"42e91f1a8a93669cbf3ecf52ce9cd921e97265e4","unresolved":false,"context_lines":[{"line_number":196,"context_line":"        image_meta \u003d objects.ImageMeta("},{"line_number":197,"context_line":"            properties\u003dobjects.ImageMetaProps())"},{"line_number":198,"context_line":""},{"line_number":199,"context_line":"    try:"},{"line_number":200,"context_line":"        requested \u003d mem_encryption_requested(flavor, image_meta)"},{"line_number":201,"context_line":"    except exception.FlavorImageConflict as exc:"},{"line_number":202,"context_line":"        raise exception.RequestFilterFailed(reason\u003dexc.message)"},{"line_number":203,"context_line":""},{"line_number":204,"context_line":"    if not requested:"},{"line_number":205,"context_line":"        # No memory encryption required, so no further action required."}],"source_content_type":"text/x-python","patch_set":31,"id":"7faddb67_28e6ca17","line":202,"range":{"start_line":199,"start_character":0,"end_line":202,"end_character":63},"in_reply_to":"7faddb67_0d38433b","updated":"2019-08-19 13:10:31.000000000","message":"We already agreed to do it this way:\n\nhttp://eavesdrop.openstack.org/irclogs/%23openstack-nova/%23openstack-nova.2019-06-12.log.html#t2019-06-12T17:58:34\n\nIt\u0027s better to be safe than sorry; the check is extremely low cost, and provides extra defence against potential bugs which might allow the API request through when it shouldn\u0027t be allowed.","commit_id":"adec7baaf4d94ddea461558169359d152b6223fc"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"76c6538dc5f828399684b8fbe835ae1a62cd439d","unresolved":false,"context_lines":[{"line_number":205,"context_line":"        # No memory encryption required, so no further action required."},{"line_number":206,"context_line":"        return False"},{"line_number":207,"context_line":""},{"line_number":208,"context_line":"    spec \u003d \u0027resources:\u0027 + orc.MEM_ENCRYPTION_CONTEXT"},{"line_number":209,"context_line":"    request_spec.flavor.extra_specs[spec] \u003d \u00271\u0027"},{"line_number":210,"context_line":""},{"line_number":211,"context_line":"    LOG.debug(\"encrypted_memory_support request filter added \""},{"line_number":212,"context_line":"              \"required resource MEM_ENCRYPTION_CONTEXT\")"}],"source_content_type":"text/x-python","patch_set":31,"id":"7faddb67_adc9ef5a","line":209,"range":{"start_line":208,"start_character":0,"end_line":209,"end_character":47},"updated":"2019-08-09 15:20:10.000000000","message":"Will this be persisted? Comments in [1] suggest we need to do more than this.\n\nAlso, what if the user has explicitly requested this extra_spec. Shouldn\u0027t we check to make sure that this isn\u0027t the case and error out if isn\u0027t?\n\nhttps://review.opendev.org/#/c/665775/10/nova/scheduler/request_filter.py@61","commit_id":"adec7baaf4d94ddea461558169359d152b6223fc"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"42e91f1a8a93669cbf3ecf52ce9cd921e97265e4","unresolved":false,"context_lines":[{"line_number":205,"context_line":"        # No memory encryption required, so no further action required."},{"line_number":206,"context_line":"        return False"},{"line_number":207,"context_line":""},{"line_number":208,"context_line":"    spec \u003d \u0027resources:\u0027 + orc.MEM_ENCRYPTION_CONTEXT"},{"line_number":209,"context_line":"    request_spec.flavor.extra_specs[spec] \u003d \u00271\u0027"},{"line_number":210,"context_line":""},{"line_number":211,"context_line":"    LOG.debug(\"encrypted_memory_support request filter added \""},{"line_number":212,"context_line":"              \"required resource MEM_ENCRYPTION_CONTEXT\")"}],"source_content_type":"text/x-python","patch_set":31,"id":"7faddb67_48c8e653","line":209,"range":{"start_line":208,"start_character":0,"end_line":209,"end_character":47},"in_reply_to":"7faddb67_adc9ef5a","updated":"2019-08-19 13:10:31.000000000","message":"\u003e Will this be persisted?\n\nNo, it must not be persisted.  This is a deliberate part of the design, because it could come from a user-uploaded image property:\n\nhttp://eavesdrop.openstack.org/irclogs/%23openstack-nova/%23openstack-nova.2019-04-23.log.html#t2019-04-23T20:19:56\n\nhttp://eavesdrop.openstack.org/irclogs/%23openstack-nova/%23openstack-nova.2019-04-25.log.html#t2019-04-25T15:04:42\n\n \u003e Comments in [1] suggest we need to do more\n \u003e than this.\n \u003e\n \u003e https://review.opendev.org/#/c/665775/10/nova/scheduler/request_filter.py@61\n\n\nSorry, you lost me there.  Any chance you could elaborate?\n\n \u003e Also, what if the user has explicitly requested this extra_spec.\n \u003e Shouldn\u0027t we check to make sure that this isn\u0027t the case and error\n \u003e out if isn\u0027t?\n\nHmm, not sure I follow 100% but sounds like you might have a point here.  Are you talking about if the flavor has two extra specs:\n\n  - hw:mem_encryption\u003dTrue\n  - resources:MEM_ENCRYPTION_CONTEXT\u003d0\n\n?  I think that could only happen with an extremely confused operator.  How to do it right will be clearly documented, e.g.\n\nhttps://review.opendev.org/#/c/666616/28/doc/source/user/flavors.rst\n\nand MEM_ENCRYPTION_CONTEXT will only be used as an internal implementation detail.","commit_id":"adec7baaf4d94ddea461558169359d152b6223fc"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"76c6538dc5f828399684b8fbe835ae1a62cd439d","unresolved":false,"context_lines":[{"line_number":208,"context_line":"    spec \u003d \u0027resources:\u0027 + orc.MEM_ENCRYPTION_CONTEXT"},{"line_number":209,"context_line":"    request_spec.flavor.extra_specs[spec] \u003d \u00271\u0027"},{"line_number":210,"context_line":""},{"line_number":211,"context_line":"    LOG.debug(\"encrypted_memory_support request filter added \""},{"line_number":212,"context_line":"              \"required resource MEM_ENCRYPTION_CONTEXT\")"},{"line_number":213,"context_line":""},{"line_number":214,"context_line":"    return True"}],"source_content_type":"text/x-python","patch_set":31,"id":"7faddb67_ad256f0c","line":211,"range":{"start_line":211,"start_character":14,"end_line":211,"end_character":15},"updated":"2019-08-09 15:20:10.000000000","message":"cough \u0027 cough","commit_id":"adec7baaf4d94ddea461558169359d152b6223fc"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"42e91f1a8a93669cbf3ecf52ce9cd921e97265e4","unresolved":false,"context_lines":[{"line_number":208,"context_line":"    spec \u003d \u0027resources:\u0027 + orc.MEM_ENCRYPTION_CONTEXT"},{"line_number":209,"context_line":"    request_spec.flavor.extra_specs[spec] \u003d \u00271\u0027"},{"line_number":210,"context_line":""},{"line_number":211,"context_line":"    LOG.debug(\"encrypted_memory_support request filter added \""},{"line_number":212,"context_line":"              \"required resource MEM_ENCRYPTION_CONTEXT\")"},{"line_number":213,"context_line":""},{"line_number":214,"context_line":"    return True"}],"source_content_type":"text/x-python","patch_set":31,"id":"7faddb67_28f9ea7e","line":211,"range":{"start_line":211,"start_character":14,"end_line":211,"end_character":15},"in_reply_to":"7faddb67_ad256f0c","updated":"2019-08-19 13:10:31.000000000","message":"Are you saying that single quotes are preferred for string literals?  There\u0027s nothing in HACKING.rst about this, nor in https://docs.openstack.org/hacking/latest/user/hacking.html, nor is there any consensus for this in the existing codebase.\n\nFurthermore, single quotes can lead to horrible situations, e.g.\n\n    LOG.debug(\u0027Don\\\u0027t use log messages with apostrophes in, \u0027\n              \u0027because you can\\\u0027t mention them without escaping\u0027\n              \u0027which looks ugly as hell, doesn\\\u0027t it?\u0027)\n\nIn contrast:\n\n    LOG.debug(\"You won\u0027t have any problem with apostrophes \"\n              \"if you surround the string with double quotes\")","commit_id":"adec7baaf4d94ddea461558169359d152b6223fc"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"76c6538dc5f828399684b8fbe835ae1a62cd439d","unresolved":false,"context_lines":[{"line_number":209,"context_line":"    request_spec.flavor.extra_specs[spec] \u003d \u00271\u0027"},{"line_number":210,"context_line":""},{"line_number":211,"context_line":"    LOG.debug(\"encrypted_memory_support request filter added \""},{"line_number":212,"context_line":"              \"required resource MEM_ENCRYPTION_CONTEXT\")"},{"line_number":213,"context_line":""},{"line_number":214,"context_line":"    return True"},{"line_number":215,"context_line":""}],"source_content_type":"text/x-python","patch_set":31,"id":"7faddb67_4d171ba5","line":212,"range":{"start_line":212,"start_character":33,"end_line":212,"end_character":55},"updated":"2019-08-09 15:20:10.000000000","message":"We have the constant so let\u0027s use it.\n\n  %s\u0027, orc.MEM_ENCRYPTION_CONTEXT)","commit_id":"adec7baaf4d94ddea461558169359d152b6223fc"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"42e91f1a8a93669cbf3ecf52ce9cd921e97265e4","unresolved":false,"context_lines":[{"line_number":209,"context_line":"    request_spec.flavor.extra_specs[spec] \u003d \u00271\u0027"},{"line_number":210,"context_line":""},{"line_number":211,"context_line":"    LOG.debug(\"encrypted_memory_support request filter added \""},{"line_number":212,"context_line":"              \"required resource MEM_ENCRYPTION_CONTEXT\")"},{"line_number":213,"context_line":""},{"line_number":214,"context_line":"    return True"},{"line_number":215,"context_line":""}],"source_content_type":"text/x-python","patch_set":31,"id":"7faddb67_68304256","line":212,"range":{"start_line":212,"start_character":33,"end_line":212,"end_character":55},"in_reply_to":"7faddb67_4d171ba5","updated":"2019-08-19 13:10:31.000000000","message":"Good idea, thanks - done.","commit_id":"adec7baaf4d94ddea461558169359d152b6223fc"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"4d2a8dca052c6d3cf68ef9f5afa91aaef8bdd754","unresolved":false,"context_lines":[{"line_number":21,"context_line":"from nova import exception"},{"line_number":22,"context_line":"from nova.i18n import _"},{"line_number":23,"context_line":"from nova import objects"},{"line_number":24,"context_line":"from nova.virt.hardware import get_mem_encryption_constraint"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"CONF \u003d nova.conf.CONF"}],"source_content_type":"text/x-python","patch_set":40,"id":"7faddb67_361d960e","line":24,"range":{"start_line":24,"start_character":0,"end_line":24,"end_character":60},"updated":"2019-08-21 17:05:50.000000000","message":"You can\u0027t import functions, only modules. I\u0027m surprised flake8 isn\u0027t moaning about this","commit_id":"8706c91037b3bb636723e1f9fdf55996151792b2"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"ab67b7a01dc1c44f8fdc99c969f9d8a66e245b71","unresolved":false,"context_lines":[{"line_number":21,"context_line":"from nova import exception"},{"line_number":22,"context_line":"from nova.i18n import _"},{"line_number":23,"context_line":"from nova import objects"},{"line_number":24,"context_line":"from nova.virt.hardware import get_mem_encryption_constraint"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"CONF \u003d nova.conf.CONF"}],"source_content_type":"text/x-python","patch_set":40,"id":"7faddb67_d98d557c","line":24,"range":{"start_line":24,"start_character":0,"end_line":24,"end_character":60},"in_reply_to":"7faddb67_361d960e","updated":"2019-08-21 17:50:25.000000000","message":"Well, it\u0027s technically possible, otherwise the code below would trigger an error;-)  I assume you mean this is a style decision made by either OpenStack or nova specifically, but yeah flake8 is fine with it.\n\n... *checks* ...\n\nYep, sure enough: https://docs.openstack.org/hacking/latest/user/hacking.html#imports\n\nOK, so that was news to me.  Definitely seems like a gap in the hacking module.","commit_id":"8706c91037b3bb636723e1f9fdf55996151792b2"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"915842f7cf5a5337d6201addf8ae8b8db794f357","unresolved":false,"context_lines":[{"line_number":21,"context_line":"from nova import exception"},{"line_number":22,"context_line":"from nova.i18n import _"},{"line_number":23,"context_line":"from nova import objects"},{"line_number":24,"context_line":"from nova.virt.hardware import get_mem_encryption_constraint"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"CONF \u003d nova.conf.CONF"}],"source_content_type":"text/x-python","patch_set":40,"id":"7faddb67_841450a1","line":24,"range":{"start_line":24,"start_character":0,"end_line":24,"end_character":60},"in_reply_to":"7faddb67_d98d557c","updated":"2019-08-21 19:17:49.000000000","message":"Done","commit_id":"8706c91037b3bb636723e1f9fdf55996151792b2"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"4d2a8dca052c6d3cf68ef9f5afa91aaef8bdd754","unresolved":false,"context_lines":[{"line_number":193,"context_line":"        image_meta \u003d objects.ImageMeta("},{"line_number":194,"context_line":"            properties\u003dobjects.ImageMetaProps())"},{"line_number":195,"context_line":""},{"line_number":196,"context_line":"    try:"},{"line_number":197,"context_line":"        requested \u003d get_mem_encryption_constraint(flavor, image_meta)"},{"line_number":198,"context_line":"    except exception.FlavorImageConflict as exc:"},{"line_number":199,"context_line":"        raise exception.RequestFilterFailed(reason\u003dexc.message)"},{"line_number":200,"context_line":""},{"line_number":201,"context_line":"    if not requested:"},{"line_number":202,"context_line":"        # No memory encryption required, so no further action required."}],"source_content_type":"text/x-python","patch_set":40,"id":"7faddb67_76278e64","line":199,"range":{"start_line":196,"start_character":0,"end_line":199,"end_character":63},"updated":"2019-08-21 17:05:50.000000000","message":"I don\u0027t think we need to do this since the API should have raised this error already","commit_id":"8706c91037b3bb636723e1f9fdf55996151792b2"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"ab67b7a01dc1c44f8fdc99c969f9d8a66e245b71","unresolved":false,"context_lines":[{"line_number":193,"context_line":"        image_meta \u003d objects.ImageMeta("},{"line_number":194,"context_line":"            properties\u003dobjects.ImageMetaProps())"},{"line_number":195,"context_line":""},{"line_number":196,"context_line":"    try:"},{"line_number":197,"context_line":"        requested \u003d get_mem_encryption_constraint(flavor, image_meta)"},{"line_number":198,"context_line":"    except exception.FlavorImageConflict as exc:"},{"line_number":199,"context_line":"        raise exception.RequestFilterFailed(reason\u003dexc.message)"},{"line_number":200,"context_line":""},{"line_number":201,"context_line":"    if not requested:"},{"line_number":202,"context_line":"        # No memory encryption required, so no further action required."}],"source_content_type":"text/x-python","patch_set":40,"id":"7faddb67_79f22107","line":199,"range":{"start_line":196,"start_character":0,"end_line":199,"end_character":63},"in_reply_to":"7faddb67_76278e64","updated":"2019-08-21 17:50:25.000000000","message":"Erm, groundhog day ;-)\n\nhttps://review.opendev.org/#/c/664420/31/nova/scheduler/request_filter.py@202","commit_id":"8706c91037b3bb636723e1f9fdf55996151792b2"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"16ae5284d7d70eb620eadf518bba07ce16dcecfc","unresolved":false,"context_lines":[{"line_number":196,"context_line":"    try:"},{"line_number":197,"context_line":"        requested \u003d hw.get_mem_encryption_constraint(flavor, image_meta)"},{"line_number":198,"context_line":"    except exception.FlavorImageConflict as exc:"},{"line_number":199,"context_line":"        raise exception.RequestFilterFailed(reason\u003dexc.message)"},{"line_number":200,"context_line":""},{"line_number":201,"context_line":"    if not requested:"},{"line_number":202,"context_line":"        # No memory encryption required, so no further action required."}],"source_content_type":"text/x-python","patch_set":42,"id":"7faddb67_e640a137","line":199,"range":{"start_line":199,"start_character":8,"end_line":199,"end_character":63},"updated":"2019-08-22 07:11:04.000000000","message":"this shoudn\u0027t happend, right?","commit_id":"ac44947c5e0d6896032409f951f05989ec64444b"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"7c8dceccb830cf98d3141683a0859f1896b5c1a5","unresolved":false,"context_lines":[{"line_number":196,"context_line":"    try:"},{"line_number":197,"context_line":"        requested \u003d hw.get_mem_encryption_constraint(flavor, image_meta)"},{"line_number":198,"context_line":"    except exception.FlavorImageConflict as exc:"},{"line_number":199,"context_line":"        raise exception.RequestFilterFailed(reason\u003dexc.message)"},{"line_number":200,"context_line":""},{"line_number":201,"context_line":"    if not requested:"},{"line_number":202,"context_line":"        # No memory encryption required, so no further action required."}],"source_content_type":"text/x-python","patch_set":42,"id":"7faddb67_41b71f86","line":199,"range":{"start_line":199,"start_character":8,"end_line":199,"end_character":63},"in_reply_to":"7faddb67_e640a137","updated":"2019-08-22 09:39:02.000000000","message":"Please see:\n\nhttps://review.opendev.org/#/c/664420/31/nova/scheduler/request_filter.py@202\n\nhttps://review.opendev.org/#/c/664420/40/nova/scheduler/request_filter.py@199","commit_id":"ac44947c5e0d6896032409f951f05989ec64444b"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"16ae5284d7d70eb620eadf518bba07ce16dcecfc","unresolved":false,"context_lines":[{"line_number":203,"context_line":"        return False"},{"line_number":204,"context_line":""},{"line_number":205,"context_line":"    spec \u003d \u0027resources:\u0027 + orc.MEM_ENCRYPTION_CONTEXT"},{"line_number":206,"context_line":"    request_spec.flavor.extra_specs[spec] \u003d \u00271\u0027"},{"line_number":207,"context_line":""},{"line_number":208,"context_line":"    LOG.debug(\"encrypted_memory_support request filter added \""},{"line_number":209,"context_line":"              \"required resource %s\", orc.MEM_ENCRYPTION_CONTEXT)"}],"source_content_type":"text/x-python","patch_set":42,"id":"7faddb67_46a41507","line":206,"updated":"2019-08-22 07:11:04.000000000","message":"will this override existing extra specs?\n\nI kind of feeling the request filter isn\u0027t the right place to do this. Maybe we should do that in \u0027nova.scheduler.utils.resources_from_request_spec\u0027","commit_id":"ac44947c5e0d6896032409f951f05989ec64444b"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"7c8dceccb830cf98d3141683a0859f1896b5c1a5","unresolved":false,"context_lines":[{"line_number":203,"context_line":"        return False"},{"line_number":204,"context_line":""},{"line_number":205,"context_line":"    spec \u003d \u0027resources:\u0027 + orc.MEM_ENCRYPTION_CONTEXT"},{"line_number":206,"context_line":"    request_spec.flavor.extra_specs[spec] \u003d \u00271\u0027"},{"line_number":207,"context_line":""},{"line_number":208,"context_line":"    LOG.debug(\"encrypted_memory_support request filter added \""},{"line_number":209,"context_line":"              \"required resource %s\", orc.MEM_ENCRYPTION_CONTEXT)"}],"source_content_type":"text/x-python","patch_set":42,"id":"7faddb67_46b892e7","line":206,"in_reply_to":"7faddb67_46a41507","updated":"2019-08-22 09:39:02.000000000","message":"\u003e will this override existing extra specs?\n\nDo you mean if resources:mem_encryption_context\u003d0 already exists in the flavor?  (There is no other possibility since max is set to 1 during inventory.)  If so, please see the previous discussion on this:\n\nhttps://review.opendev.org/#/c/664420/31/nova/scheduler/request_filter.py@209\n\nI can\u0027t imagine a situation in which the operator would be so confused that they would do this.  It will clearly be documented how to set up SEV, and it will not even mention this internal-only resource.\n\n \u003e I kind of feeling the request filter isn\u0027t the right place to do\n \u003e this. Maybe we should do that in \u0027nova.scheduler.utils.resources_from_request_spec\u0027\n\nOh interesting, thanks!  I didn\u0027t know about that method and I was recommended to put it in request_filter.py:\n\nhttp://eavesdrop.openstack.org/irclogs/%23openstack-nova/%23openstack-nova.2019-04-23.log.html#t2019-04-23T17:41:18\n\nbut I could move it maybe.  What are the pros and cons of both?","commit_id":"ac44947c5e0d6896032409f951f05989ec64444b"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"53f8024fd3dfedb4b0d0aa8ed787d418d08eaaf1","unresolved":false,"context_lines":[{"line_number":203,"context_line":"        return False"},{"line_number":204,"context_line":""},{"line_number":205,"context_line":"    spec \u003d \u0027resources:\u0027 + orc.MEM_ENCRYPTION_CONTEXT"},{"line_number":206,"context_line":"    request_spec.flavor.extra_specs[spec] \u003d \u00271\u0027"},{"line_number":207,"context_line":""},{"line_number":208,"context_line":"    LOG.debug(\"encrypted_memory_support request filter added \""},{"line_number":209,"context_line":"              \"required resource %s\", orc.MEM_ENCRYPTION_CONTEXT)"}],"source_content_type":"text/x-python","patch_set":42,"id":"7faddb67_8d15429e","line":206,"in_reply_to":"7faddb67_46b892e7","updated":"2019-08-22 15:35:28.000000000","message":"As discussed in IRC just now, we agreed to keep it in the current place for now:\n\nhttp://eavesdrop.openstack.org/irclogs/%23openstack-nova/%23openstack-nova.2019-08-22.log.html#t2019-08-22T15:18:47","commit_id":"ac44947c5e0d6896032409f951f05989ec64444b"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"dcc41238522c5a8426f6018c4eeccaebec442cbc","unresolved":false,"context_lines":[{"line_number":203,"context_line":"        return False"},{"line_number":204,"context_line":""},{"line_number":205,"context_line":"    spec \u003d \u0027resources:\u0027 + orc.MEM_ENCRYPTION_CONTEXT"},{"line_number":206,"context_line":"    request_spec.flavor.extra_specs[spec] \u003d \u00271\u0027"},{"line_number":207,"context_line":""},{"line_number":208,"context_line":"    LOG.debug(\"encrypted_memory_support request filter added \""},{"line_number":209,"context_line":"              \"required resource %s\", orc.MEM_ENCRYPTION_CONTEXT)"}],"source_content_type":"text/x-python","patch_set":42,"id":"7faddb67_69854b07","line":206,"in_reply_to":"7faddb67_74cd2ab6","updated":"2019-08-23 07:45:55.000000000","message":"OK, that\u0027s totally fine by me as long as https://review.opendev.org/#/c/674894/ doesn\u0027t block the whole SEV series so that it misses Train :-)","commit_id":"ac44947c5e0d6896032409f951f05989ec64444b"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"2ec5c870901be5b251efa0455dc7d49f4bdd145d","unresolved":false,"context_lines":[{"line_number":203,"context_line":"        return False"},{"line_number":204,"context_line":""},{"line_number":205,"context_line":"    spec \u003d \u0027resources:\u0027 + orc.MEM_ENCRYPTION_CONTEXT"},{"line_number":206,"context_line":"    request_spec.flavor.extra_specs[spec] \u003d \u00271\u0027"},{"line_number":207,"context_line":""},{"line_number":208,"context_line":"    LOG.debug(\"encrypted_memory_support request filter added \""},{"line_number":209,"context_line":"              \"required resource %s\", orc.MEM_ENCRYPTION_CONTEXT)"}],"source_content_type":"text/x-python","patch_set":42,"id":"7faddb67_74cd2ab6","line":206,"in_reply_to":"7faddb67_8d15429e","updated":"2019-08-22 21:15:02.000000000","message":"After spending some more time in the cpu-resources series, I think Alex is completely correct about this.\n\nWe should cut it over as soon as https://review.opendev.org/#/c/674894/ merges.","commit_id":"ac44947c5e0d6896032409f951f05989ec64444b"}],"nova/scheduler/utils.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"56e3989c25eb781e1d54dd86e7225608fc88b87e","unresolved":false,"context_lines":[{"line_number":93,"context_line":""},{"line_number":94,"context_line":"        image \u003d request_spec.image if \u0027image\u0027 in request_spec else None"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"        self._translate_memory_encryption(request_spec.flavor, image)"},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"        # Parse the flavor extra specs"},{"line_number":99,"context_line":"        self._process_extra_specs(request_spec.flavor)"}],"source_content_type":"text/x-python","patch_set":46,"id":"7faddb67_202ff3f6","line":96,"updated":"2019-08-30 09:51:37.000000000","message":"style nit: We could do this at the end of the function since it won\u0027t affect VCPU/MEMORY_MB/DISK_GB allocations. I think I\u0027d prefer this, since it lets us get the highest priority stuff out of the way first","commit_id":"3f53fed18665b6295d84d196ce681870617b87c5"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"73b3f273224446132f461a49039a83c617e93300","unresolved":false,"context_lines":[{"line_number":93,"context_line":""},{"line_number":94,"context_line":"        image \u003d request_spec.image if \u0027image\u0027 in request_spec else None"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"        self._translate_memory_encryption(request_spec.flavor, image)"},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"        # Parse the flavor extra specs"},{"line_number":99,"context_line":"        self._process_extra_specs(request_spec.flavor)"}],"source_content_type":"text/x-python","patch_set":46,"id":"7faddb67_11048bc5","line":96,"in_reply_to":"7faddb67_202ff3f6","updated":"2019-08-30 13:08:17.000000000","message":"Done","commit_id":"3f53fed18665b6295d84d196ce681870617b87c5"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"56e3989c25eb781e1d54dd86e7225608fc88b87e","unresolved":false,"context_lines":[{"line_number":167,"context_line":"        resources:MEM_ENCRYPTION_CONTEXT\u003d1 which requires a slot on a"},{"line_number":168,"context_line":"        host which can support encryption of the guest memory."},{"line_number":169,"context_line":"        \"\"\""},{"line_number":170,"context_line":"        if image is None:"},{"line_number":171,"context_line":"            image \u003d objects.ImageMeta(properties\u003dobjects.ImageMetaProps())"},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"        # NOTE(aspiers): In theory this could raise FlavorImageConflict,"},{"line_number":174,"context_line":"        # but we already check it in the API layer, so that should never"}],"source_content_type":"text/x-python","patch_set":46,"id":"7faddb67_806b471d","line":171,"range":{"start_line":170,"start_character":0,"end_line":171,"end_character":74},"updated":"2019-08-30 09:51:37.000000000","message":"I wonder if we should be doing this in the caller (i.e. replace \"image \u003d request_spec.image if \u0027image\u0027 in request_spec else None\"). This sounds like something others will use down the line. I mean, I\u0027m doing the same thing (in a different way) already in [1] so there are two users straight off the bat\n\n[1] https://review.opendev.org/#/c/671801/32/nova/scheduler/utils.py@172","commit_id":"3f53fed18665b6295d84d196ce681870617b87c5"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"73b3f273224446132f461a49039a83c617e93300","unresolved":false,"context_lines":[{"line_number":167,"context_line":"        resources:MEM_ENCRYPTION_CONTEXT\u003d1 which requires a slot on a"},{"line_number":168,"context_line":"        host which can support encryption of the guest memory."},{"line_number":169,"context_line":"        \"\"\""},{"line_number":170,"context_line":"        if image is None:"},{"line_number":171,"context_line":"            image \u003d objects.ImageMeta(properties\u003dobjects.ImageMetaProps())"},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"        # NOTE(aspiers): In theory this could raise FlavorImageConflict,"},{"line_number":174,"context_line":"        # but we already check it in the API layer, so that should never"}],"source_content_type":"text/x-python","patch_set":46,"id":"7faddb67_f1dc4f46","line":171,"range":{"start_line":170,"start_character":0,"end_line":171,"end_character":74},"in_reply_to":"7faddb67_806b471d","updated":"2019-08-30 13:08:17.000000000","message":"Done","commit_id":"3f53fed18665b6295d84d196ce681870617b87c5"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"f62a6e8afb98253601ae4619d59936b415b11f46","unresolved":false,"context_lines":[{"line_number":177,"context_line":"            # No memory encryption required, so no further action required."},{"line_number":178,"context_line":"            return"},{"line_number":179,"context_line":""},{"line_number":180,"context_line":"        spec \u003d \u0027resources:\u0027 + orc.MEM_ENCRYPTION_CONTEXT"},{"line_number":181,"context_line":"        flavor.extra_specs[spec] \u003d \u00271\u0027"},{"line_number":182,"context_line":""},{"line_number":183,"context_line":"        LOG.debug(\"Added %s\u003d1 to requested resources\","},{"line_number":184,"context_line":"                  orc.MEM_ENCRYPTION_CONTEXT)"}],"source_content_type":"text/x-python","patch_set":46,"id":"7faddb67_b0c6d89b","line":181,"range":{"start_line":180,"start_character":8,"end_line":181,"end_character":38},"updated":"2019-08-30 05:10:59.000000000","message":"we should follow this way https://review.opendev.org/#/c/671801/32/nova/scheduler/utils.py@167\n\nAdd your resource to ResourceRequest obj.","commit_id":"3f53fed18665b6295d84d196ce681870617b87c5"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"73b3f273224446132f461a49039a83c617e93300","unresolved":false,"context_lines":[{"line_number":177,"context_line":"            # No memory encryption required, so no further action required."},{"line_number":178,"context_line":"            return"},{"line_number":179,"context_line":""},{"line_number":180,"context_line":"        spec \u003d \u0027resources:\u0027 + orc.MEM_ENCRYPTION_CONTEXT"},{"line_number":181,"context_line":"        flavor.extra_specs[spec] \u003d \u00271\u0027"},{"line_number":182,"context_line":""},{"line_number":183,"context_line":"        LOG.debug(\"Added %s\u003d1 to requested resources\","},{"line_number":184,"context_line":"                  orc.MEM_ENCRYPTION_CONTEXT)"}],"source_content_type":"text/x-python","patch_set":46,"id":"7faddb67_f1b52fee","line":181,"range":{"start_line":180,"start_character":8,"end_line":181,"end_character":38},"in_reply_to":"7faddb67_0013d7b9","updated":"2019-08-30 13:08:17.000000000","message":"I was thinking that if _process_extra_specs() ever wanted to do any extra processing which might affect this resource class, that would require that logic to be duplicated here.  That\u0027s why I did the translation *before* calling _process_extra_specs().  But I\u0027m outvoted so I\u0027ve changed it.","commit_id":"3f53fed18665b6295d84d196ce681870617b87c5"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"56e3989c25eb781e1d54dd86e7225608fc88b87e","unresolved":false,"context_lines":[{"line_number":177,"context_line":"            # No memory encryption required, so no further action required."},{"line_number":178,"context_line":"            return"},{"line_number":179,"context_line":""},{"line_number":180,"context_line":"        spec \u003d \u0027resources:\u0027 + orc.MEM_ENCRYPTION_CONTEXT"},{"line_number":181,"context_line":"        flavor.extra_specs[spec] \u003d \u00271\u0027"},{"line_number":182,"context_line":""},{"line_number":183,"context_line":"        LOG.debug(\"Added %s\u003d1 to requested resources\","},{"line_number":184,"context_line":"                  orc.MEM_ENCRYPTION_CONTEXT)"}],"source_content_type":"text/x-python","patch_set":46,"id":"7faddb67_0013d7b9","line":181,"range":{"start_line":180,"start_character":8,"end_line":181,"end_character":38},"in_reply_to":"7faddb67_b0c6d89b","updated":"2019-08-30 09:51:37.000000000","message":"+1 - you want to be calling \u0027_add_resource\u0027 rather than modifying the flavor","commit_id":"3f53fed18665b6295d84d196ce681870617b87c5"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"63195ccd9f27e58893981c815941c5b6d564c57c","unresolved":false,"context_lines":[{"line_number":92,"context_line":"        # with destination.aggregates handling in resources_from_request_spec"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"        image \u003d (request_spec.image if \u0027image\u0027 in request_spec"},{"line_number":95,"context_line":"                 else objects.ImageMeta(properties\u003dobjects.ImageMetaProps()))"},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"        # Parse the flavor extra specs"},{"line_number":98,"context_line":"        self._process_extra_specs(request_spec.flavor)"}],"source_content_type":"text/x-python","patch_set":47,"id":"7faddb67_cc5418c9","line":95,"updated":"2019-08-30 14:50:21.000000000","message":"RequestSpec.image is nullable, so this could still conceivably be set to None. We should handle that too. I\u0027m okay with tackling it in a follow-up though","commit_id":"4a22e424de8b56b4a355fddfb71ba25106aa39c9"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"9331bcb83481bdacd3c1c28441f957fa69694de8","unresolved":false,"context_lines":[{"line_number":92,"context_line":"        # with destination.aggregates handling in resources_from_request_spec"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"        image \u003d (request_spec.image if \u0027image\u0027 in request_spec"},{"line_number":95,"context_line":"                 else objects.ImageMeta(properties\u003dobjects.ImageMetaProps()))"},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"        # Parse the flavor extra specs"},{"line_number":98,"context_line":"        self._process_extra_specs(request_spec.flavor)"}],"source_content_type":"text/x-python","patch_set":49,"id":"7faddb67_0093be5d","line":95,"updated":"2019-09-02 10:15:03.000000000","message":"Per PS47\n\n \u003e RequestSpec.image is nullable, so this could still conceivably\n \u003e be set to None. We should handle that too. I\u0027m okay with\n \u003e tackling it in a follow-up though","commit_id":"b4905467db4dfbf476d13c8535322f3f7ce2ad18"}],"nova/tests/unit/api/openstack/compute/test_serversV21.py":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"70e57837d45b7cd65a3803f3dabd45aa926a4189","unresolved":false,"context_lines":[{"line_number":6020,"context_line":"                          self.req, body\u003dself.body)"},{"line_number":6021,"context_line":""},{"line_number":6022,"context_line":"    @mock.patch(\u0027nova.virt.hardware.mem_encryption_requesters\u0027,"},{"line_number":6023,"context_line":"                side_effect\u003dexception.FlavorImageConflict(\"reason\"))"},{"line_number":6024,"context_line":"    def test_create_instance_raise_flavor_image_conflict("},{"line_number":6025,"context_line":"            self, mock_conflict):"},{"line_number":6026,"context_line":"        self.assertRaises(webob.exc.HTTPBadRequest,"}],"source_content_type":"text/x-python","patch_set":11,"id":"9fb8cfa7_4f838505","line":6023,"range":{"start_line":6023,"start_character":28,"end_line":6023,"end_character":67},"updated":"2019-06-19 20:50:23.000000000","message":"hm, I thought we had a hacking check (or something) that made sure you created exceptions with the right kwargs. Anyway, you should probably do that here for good form.","commit_id":"31f4a45e1864c0175daf0ddb47f5a9a04b3beb08"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"18bc4382e0b7ba94f6f44669ec76d62526561e0e","unresolved":false,"context_lines":[{"line_number":6020,"context_line":"                          self.req, body\u003dself.body)"},{"line_number":6021,"context_line":""},{"line_number":6022,"context_line":"    @mock.patch(\u0027nova.virt.hardware.mem_encryption_requesters\u0027,"},{"line_number":6023,"context_line":"                side_effect\u003dexception.FlavorImageConflict(\"reason\"))"},{"line_number":6024,"context_line":"    def test_create_instance_raise_flavor_image_conflict("},{"line_number":6025,"context_line":"            self, mock_conflict):"},{"line_number":6026,"context_line":"        self.assertRaises(webob.exc.HTTPBadRequest,"}],"source_content_type":"text/x-python","patch_set":11,"id":"9fb8cfa7_550d548e","line":6023,"range":{"start_line":6023,"start_character":28,"end_line":6023,"end_character":67},"in_reply_to":"9fb8cfa7_4f838505","updated":"2019-06-20 12:09:46.000000000","message":"No kwargs are required here, because the raw message is provided.  This is deliberate, because the message template in the exception class (which I copied from your patch elsewhere) isn\u0027t detailed enough to describe this particular type of conflict, hence overriding it with a custom message.\n\nHaving said that, I\u0027ve added a message\u003d prefix for clarity.","commit_id":"31f4a45e1864c0175daf0ddb47f5a9a04b3beb08"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"31c9343c8611dd2bd60aae6e0127c4960eca3e43","unresolved":false,"context_lines":[{"line_number":6020,"context_line":"                          self.req, body\u003dself.body)"},{"line_number":6021,"context_line":""},{"line_number":6022,"context_line":"    @mock.patch(\u0027nova.virt.hardware.mem_encryption_requesters\u0027,"},{"line_number":6023,"context_line":"                side_effect\u003dexception.FlavorImageConflict(\"reason\"))"},{"line_number":6024,"context_line":"    def test_create_instance_raise_flavor_image_conflict("},{"line_number":6025,"context_line":"            self, mock_conflict):"},{"line_number":6026,"context_line":"        self.assertRaises(webob.exc.HTTPBadRequest,"}],"source_content_type":"text/x-python","patch_set":11,"id":"9fb8cfa7_8c6c1e09","line":6023,"range":{"start_line":6023,"start_character":28,"end_line":6023,"end_character":67},"in_reply_to":"9fb8cfa7_550d548e","updated":"2019-06-20 12:22:40.000000000","message":"ah, sorry, yeah, my bad.","commit_id":"31f4a45e1864c0175daf0ddb47f5a9a04b3beb08"}],"nova/tests/unit/image/fake.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"4d2a8dca052c6d3cf68ef9f5afa91aaef8bdd754","unresolved":false,"context_lines":[{"line_number":326,"context_line":"    return image_service"},{"line_number":327,"context_line":""},{"line_number":328,"context_line":""},{"line_number":329,"context_line":"def fake_image_obj(default_image_meta\u003dNone, default_image_props\u003dNone,"},{"line_number":330,"context_line":"                       variable_image_props\u003dNone):"},{"line_number":331,"context_line":"    \"\"\"Helper for constructing a test ImageMeta object with attributes and"},{"line_number":332,"context_line":"    properties coming from a combination of (probably hard-coded)"},{"line_number":333,"context_line":"    values within a test, and (optionally) variable values from the"},{"line_number":334,"context_line":"    test\u0027s caller, if the test is actually a helper written to be"},{"line_number":335,"context_line":"    reusable and run multiple times with different parameters from"},{"line_number":336,"context_line":"    different \"wrapper\" tests."},{"line_number":337,"context_line":"    \"\"\""},{"line_number":338,"context_line":"    image_meta_props \u003d default_image_props or {}"},{"line_number":339,"context_line":"    if variable_image_props:"},{"line_number":340,"context_line":"        image_meta_props.update(variable_image_props)"},{"line_number":341,"context_line":""},{"line_number":342,"context_line":"    test_image_meta \u003d default_image_meta or {\"disk_format\": \"raw\"}"},{"line_number":343,"context_line":"    if \u0027name\u0027 not in test_image_meta:"},{"line_number":344,"context_line":"        # NOTE(aspiers): the name is specified here in case it\u0027s needed"},{"line_number":345,"context_line":"        # by the logging in nova.virt.hardware.get_mem_encryption_constraint()"},{"line_number":346,"context_line":"        test_image_meta[\u0027name\u0027] \u003d \u0027fake_image\u0027"},{"line_number":347,"context_line":"    test_image_meta.update({"},{"line_number":348,"context_line":"        \u0027properties\u0027: image_meta_props,"},{"line_number":349,"context_line":"    })"},{"line_number":350,"context_line":""},{"line_number":351,"context_line":"    return objects.ImageMeta.from_dict(test_image_meta)"}],"source_content_type":"text/x-python","patch_set":40,"id":"7faddb67_56b29207","line":351,"range":{"start_line":329,"start_character":0,"end_line":351,"end_character":55},"updated":"2019-08-21 17:05:50.000000000","message":"This should probably have gone into a new \u0027nova/tests/unit/fake_image.py\u0027 file. Alternatively, it seems \u0027fake_request_spec.py\u0027 has an \u0027IMAGE_META\u0027 attribute you could use. Much of a muchness though","commit_id":"8706c91037b3bb636723e1f9fdf55996151792b2"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"ab67b7a01dc1c44f8fdc99c969f9d8a66e245b71","unresolved":false,"context_lines":[{"line_number":326,"context_line":"    return image_service"},{"line_number":327,"context_line":""},{"line_number":328,"context_line":""},{"line_number":329,"context_line":"def fake_image_obj(default_image_meta\u003dNone, default_image_props\u003dNone,"},{"line_number":330,"context_line":"                       variable_image_props\u003dNone):"},{"line_number":331,"context_line":"    \"\"\"Helper for constructing a test ImageMeta object with attributes and"},{"line_number":332,"context_line":"    properties coming from a combination of (probably hard-coded)"},{"line_number":333,"context_line":"    values within a test, and (optionally) variable values from the"},{"line_number":334,"context_line":"    test\u0027s caller, if the test is actually a helper written to be"},{"line_number":335,"context_line":"    reusable and run multiple times with different parameters from"},{"line_number":336,"context_line":"    different \"wrapper\" tests."},{"line_number":337,"context_line":"    \"\"\""},{"line_number":338,"context_line":"    image_meta_props \u003d default_image_props or {}"},{"line_number":339,"context_line":"    if variable_image_props:"},{"line_number":340,"context_line":"        image_meta_props.update(variable_image_props)"},{"line_number":341,"context_line":""},{"line_number":342,"context_line":"    test_image_meta \u003d default_image_meta or {\"disk_format\": \"raw\"}"},{"line_number":343,"context_line":"    if \u0027name\u0027 not in test_image_meta:"},{"line_number":344,"context_line":"        # NOTE(aspiers): the name is specified here in case it\u0027s needed"},{"line_number":345,"context_line":"        # by the logging in nova.virt.hardware.get_mem_encryption_constraint()"},{"line_number":346,"context_line":"        test_image_meta[\u0027name\u0027] \u003d \u0027fake_image\u0027"},{"line_number":347,"context_line":"    test_image_meta.update({"},{"line_number":348,"context_line":"        \u0027properties\u0027: image_meta_props,"},{"line_number":349,"context_line":"    })"},{"line_number":350,"context_line":""},{"line_number":351,"context_line":"    return objects.ImageMeta.from_dict(test_image_meta)"}],"source_content_type":"text/x-python","patch_set":40,"id":"7faddb67_99d27d5b","line":351,"range":{"start_line":329,"start_character":0,"end_line":351,"end_character":55},"in_reply_to":"7faddb67_56b29207","updated":"2019-08-21 17:50:25.000000000","message":"Why have two separate files for generating fake image objects?  I did consider adding a new one first, but then I found this existing one and couldn\u0027t see a good reason for having two files performing essentially the same role.","commit_id":"8706c91037b3bb636723e1f9fdf55996151792b2"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"63195ccd9f27e58893981c815941c5b6d564c57c","unresolved":false,"context_lines":[{"line_number":327,"context_line":""},{"line_number":328,"context_line":""},{"line_number":329,"context_line":"def fake_image_obj(default_image_meta\u003dNone, default_image_props\u003dNone,"},{"line_number":330,"context_line":"                       variable_image_props\u003dNone):"},{"line_number":331,"context_line":"    \"\"\"Helper for constructing a test ImageMeta object with attributes and"},{"line_number":332,"context_line":"    properties coming from a combination of (probably hard-coded)"},{"line_number":333,"context_line":"    values within a test, and (optionally) variable values from the"}],"source_content_type":"text/x-python","patch_set":47,"id":"7faddb67_4c682887","line":330,"range":{"start_line":330,"start_character":19,"end_line":330,"end_character":23},"updated":"2019-08-30 14:50:21.000000000","message":"nit","commit_id":"4a22e424de8b56b4a355fddfb71ba25106aa39c9"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"49d145c142846098afbe3306240cdc8de3f9e60f","unresolved":false,"context_lines":[{"line_number":327,"context_line":""},{"line_number":328,"context_line":""},{"line_number":329,"context_line":"def fake_image_obj(default_image_meta\u003dNone, default_image_props\u003dNone,"},{"line_number":330,"context_line":"                       variable_image_props\u003dNone):"},{"line_number":331,"context_line":"    \"\"\"Helper for constructing a test ImageMeta object with attributes and"},{"line_number":332,"context_line":"    properties coming from a combination of (probably hard-coded)"},{"line_number":333,"context_line":"    values within a test, and (optionally) variable values from the"}],"source_content_type":"text/x-python","patch_set":47,"id":"7faddb67_a2f52385","line":330,"range":{"start_line":330,"start_character":19,"end_line":330,"end_character":23},"in_reply_to":"7faddb67_4c682887","updated":"2019-08-30 15:18:55.000000000","message":"Done","commit_id":"4a22e424de8b56b4a355fddfb71ba25106aa39c9"}],"nova/tests/unit/scheduler/test_request_filter.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"4d2a8dca052c6d3cf68ef9f5afa91aaef8bdd754","unresolved":false,"context_lines":[{"line_number":254,"context_line":"        # NOTE(aspiers): RequestSpec.flavor is not nullable, but"},{"line_number":255,"context_line":"        # RequestSpec.image is, so the caller should always support a"},{"line_number":256,"context_line":"        # flavor, even if it has no fields set."},{"line_number":257,"context_line":"        assert \u0027flavor\u0027 in kwargs"},{"line_number":258,"context_line":"        reqspec \u003d objects.RequestSpec(**kwargs)"},{"line_number":259,"context_line":"        filter_result \u003d request_filter.encrypted_memory_support("},{"line_number":260,"context_line":"                self.context, reqspec)"}],"source_content_type":"text/x-python","patch_set":40,"id":"7faddb67_96a1eaab","line":257,"range":{"start_line":257,"start_character":0,"end_line":257,"end_character":33},"updated":"2019-08-21 17:05:50.000000000","message":"Generally, you can\u0027t rely on asserts because they can be optimized away, but this is test code so I guess it\u0027s okay?","commit_id":"8706c91037b3bb636723e1f9fdf55996151792b2"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"5060a4484efd8680da348ea52d745f013908da2e","unresolved":false,"context_lines":[{"line_number":254,"context_line":"        # NOTE(aspiers): RequestSpec.flavor is not nullable, but"},{"line_number":255,"context_line":"        # RequestSpec.image is, so the caller should always support a"},{"line_number":256,"context_line":"        # flavor, even if it has no fields set."},{"line_number":257,"context_line":"        assert \u0027flavor\u0027 in kwargs"},{"line_number":258,"context_line":"        reqspec \u003d objects.RequestSpec(**kwargs)"},{"line_number":259,"context_line":"        filter_result \u003d request_filter.encrypted_memory_support("},{"line_number":260,"context_line":"                self.context, reqspec)"}],"source_content_type":"text/x-python","patch_set":40,"id":"7faddb67_c4f9e84d","line":257,"range":{"start_line":257,"start_character":0,"end_line":257,"end_character":33},"in_reply_to":"7faddb67_59c885a6","updated":"2019-08-21 20:22:29.000000000","message":"Done","commit_id":"8706c91037b3bb636723e1f9fdf55996151792b2"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"ab67b7a01dc1c44f8fdc99c969f9d8a66e245b71","unresolved":false,"context_lines":[{"line_number":254,"context_line":"        # NOTE(aspiers): RequestSpec.flavor is not nullable, but"},{"line_number":255,"context_line":"        # RequestSpec.image is, so the caller should always support a"},{"line_number":256,"context_line":"        # flavor, even if it has no fields set."},{"line_number":257,"context_line":"        assert \u0027flavor\u0027 in kwargs"},{"line_number":258,"context_line":"        reqspec \u003d objects.RequestSpec(**kwargs)"},{"line_number":259,"context_line":"        filter_result \u003d request_filter.encrypted_memory_support("},{"line_number":260,"context_line":"                self.context, reqspec)"}],"source_content_type":"text/x-python","patch_set":40,"id":"7faddb67_59c885a6","line":257,"range":{"start_line":257,"start_character":0,"end_line":257,"end_character":33},"in_reply_to":"7faddb67_96a1eaab","updated":"2019-08-21 17:50:25.000000000","message":"Interesting, didn\u0027t know that.  I could change it to assertIn I guess.","commit_id":"8706c91037b3bb636723e1f9fdf55996151792b2"}],"nova/tests/unit/virt/test_hardware.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"76c6538dc5f828399684b8fbe835ae1a62cd439d","unresolved":false,"context_lines":[{"line_number":3630,"context_line":"        self._test_encrypted_memory_support_not_required()"},{"line_number":3631,"context_line":""},{"line_number":3632,"context_line":"    def test_require_false_extra_spec(self):"},{"line_number":3633,"context_line":"        for extra_spec in (\u00270\u0027, \u0027false\u0027, \u0027False\u0027):"},{"line_number":3634,"context_line":"            self._test_encrypted_memory_support_not_required("},{"line_number":3635,"context_line":"                objects.Flavor(extra_specs\u003d{\u0027hw:mem_encryption\u0027: extra_spec})"},{"line_number":3636,"context_line":"            )"}],"source_content_type":"text/x-python","patch_set":31,"id":"7faddb67_edb5a77f","line":3633,"range":{"start_line":3633,"start_character":0,"end_line":3633,"end_character":50},"updated":"2019-08-09 15:20:10.000000000","message":"Technically we\u0027re testing \u0027strutils.bool_from_string\u0027 behavior here so I don\u0027t know if this is necessary, but is\u0027s done now so ¯\\_(ツ)_/¯","commit_id":"adec7baaf4d94ddea461558169359d152b6223fc"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"42e91f1a8a93669cbf3ecf52ce9cd921e97265e4","unresolved":false,"context_lines":[{"line_number":3630,"context_line":"        self._test_encrypted_memory_support_not_required()"},{"line_number":3631,"context_line":""},{"line_number":3632,"context_line":"    def test_require_false_extra_spec(self):"},{"line_number":3633,"context_line":"        for extra_spec in (\u00270\u0027, \u0027false\u0027, \u0027False\u0027):"},{"line_number":3634,"context_line":"            self._test_encrypted_memory_support_not_required("},{"line_number":3635,"context_line":"                objects.Flavor(extra_specs\u003d{\u0027hw:mem_encryption\u0027: extra_spec})"},{"line_number":3636,"context_line":"            )"}],"source_content_type":"text/x-python","patch_set":31,"id":"7faddb67_2827aa14","line":3633,"range":{"start_line":3633,"start_character":0,"end_line":3633,"end_character":50},"in_reply_to":"7faddb67_edb5a77f","updated":"2019-08-19 13:10:31.000000000","message":"Well, the point of the test is to check it does the right thing without too much knowledge of the implementation :-)  This prevents future code changes from accidentally removing usage of bool_from_string without noticing, which could have happened if all 3 cases weren\u0027t tested.  Yeah I know it\u0027s thorough but in cases like this where it can be done cheaply I prefer to err on the side of too many tests.","commit_id":"adec7baaf4d94ddea461558169359d152b6223fc"}],"nova/virt/hardware.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b39eba5a51fb0ef8f493c0af99f6bbfa99b96dd2","unresolved":false,"context_lines":[{"line_number":1140,"context_line":""},{"line_number":1141,"context_line":"    if image_meta:"},{"line_number":1142,"context_line":"        if isinstance(image_meta, dict):"},{"line_number":1143,"context_line":"            # NOTE(aspiers): This case is required for some tests to"},{"line_number":1144,"context_line":"            # succeed which pass in a fake image_meta as a dict."},{"line_number":1145,"context_line":"            props \u003d image_meta.get(\u0027properties\u0027, {})"},{"line_number":1146,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":10,"id":"9fb8cfa7_c8077018","line":1143,"updated":"2019-06-19 18:15:59.000000000","message":"We shouldn\u0027t add more compat code like this. If we have tests passing in a dict for image_meta, we should have a cleanup patch before this in the series that converts those tests to use an ImageMeta object, which you can probably easily convert using image_meta \u003d ImageMeta.from_dict(image_meta).","commit_id":"885cc8ed955be7b10c57f14be9b0aa14f3609279"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"c02ba78e6e5f391738f6bde3312385468f885a3a","unresolved":false,"context_lines":[{"line_number":1140,"context_line":""},{"line_number":1141,"context_line":"    if image_meta:"},{"line_number":1142,"context_line":"        if isinstance(image_meta, dict):"},{"line_number":1143,"context_line":"            # NOTE(aspiers): This case is required for some tests to"},{"line_number":1144,"context_line":"            # succeed which pass in a fake image_meta as a dict."},{"line_number":1145,"context_line":"            props \u003d image_meta.get(\u0027properties\u0027, {})"},{"line_number":1146,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":10,"id":"9fb8cfa7_e317f750","line":1143,"in_reply_to":"9fb8cfa7_c8077018","updated":"2019-06-19 20:03:29.000000000","message":"Turns out I had misread the code - this function should *always* be passed an ImageMeta, and never a dict, so this conditional is not necessary.  It was getting passed a dict during my test runs because I had missed an ImageMeta.from_dict() conversion in api.py.  Sorry for the noise, but you helped me learn a bunch of stuff in the process :-)","commit_id":"885cc8ed955be7b10c57f14be9b0aa14f3609279"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"70e57837d45b7cd65a3803f3dabd45aa926a4189","unresolved":false,"context_lines":[{"line_number":1135,"context_line":"    flavor_key \u003d \u0027:\u0027.join([\u0027hw\u0027, key])"},{"line_number":1136,"context_line":"    image_key \u003d \u0027_\u0027.join([\u0027hw\u0027, key])"},{"line_number":1137,"context_line":""},{"line_number":1138,"context_line":"    extra_specs \u003d (flavor or {}).get(\u0027extra_specs\u0027, {})"},{"line_number":1139,"context_line":"    flavor_policy \u003d extra_specs.get(flavor_key, default)"},{"line_number":1140,"context_line":""},{"line_number":1141,"context_line":"    if image_meta:"},{"line_number":1142,"context_line":"        image_policy \u003d image_meta.properties.get(image_key, default)"},{"line_number":1143,"context_line":"    else:"},{"line_number":1144,"context_line":"        image_policy \u003d default"},{"line_number":1145,"context_line":""},{"line_number":1146,"context_line":"    return flavor_policy, image_policy"},{"line_number":1147,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"9fb8cfa7_efbfd9a7","line":1144,"range":{"start_line":1138,"start_character":0,"end_line":1144,"end_character":30},"updated":"2019-06-19 20:50:23.000000000","message":"Sorry, why was this change necessary? I can\u0027t see anywhere flavor or image_meta could be None.","commit_id":"31f4a45e1864c0175daf0ddb47f5a9a04b3beb08"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"ed39148aba4c7aff9f1044511d03e65b6c8c1fa0","unresolved":false,"context_lines":[{"line_number":1135,"context_line":"    flavor_key \u003d \u0027:\u0027.join([\u0027hw\u0027, key])"},{"line_number":1136,"context_line":"    image_key \u003d \u0027_\u0027.join([\u0027hw\u0027, key])"},{"line_number":1137,"context_line":""},{"line_number":1138,"context_line":"    extra_specs \u003d (flavor or {}).get(\u0027extra_specs\u0027, {})"},{"line_number":1139,"context_line":"    flavor_policy \u003d extra_specs.get(flavor_key, default)"},{"line_number":1140,"context_line":""},{"line_number":1141,"context_line":"    if image_meta:"},{"line_number":1142,"context_line":"        image_policy \u003d image_meta.properties.get(image_key, default)"},{"line_number":1143,"context_line":"    else:"},{"line_number":1144,"context_line":"        image_policy \u003d default"},{"line_number":1145,"context_line":""},{"line_number":1146,"context_line":"    return flavor_policy, image_policy"},{"line_number":1147,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"9fb8cfa7_a7b81de5","line":1144,"range":{"start_line":1138,"start_character":0,"end_line":1144,"end_character":30},"in_reply_to":"9fb8cfa7_8c3adee4","updated":"2019-06-20 13:53:45.000000000","message":"Yes, makes sense.  I think I might have managed that - let\u0027s see if Zuul agrees.","commit_id":"31f4a45e1864c0175daf0ddb47f5a9a04b3beb08"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"31c9343c8611dd2bd60aae6e0127c4960eca3e43","unresolved":false,"context_lines":[{"line_number":1135,"context_line":"    flavor_key \u003d \u0027:\u0027.join([\u0027hw\u0027, key])"},{"line_number":1136,"context_line":"    image_key \u003d \u0027_\u0027.join([\u0027hw\u0027, key])"},{"line_number":1137,"context_line":""},{"line_number":1138,"context_line":"    extra_specs \u003d (flavor or {}).get(\u0027extra_specs\u0027, {})"},{"line_number":1139,"context_line":"    flavor_policy \u003d extra_specs.get(flavor_key, default)"},{"line_number":1140,"context_line":""},{"line_number":1141,"context_line":"    if image_meta:"},{"line_number":1142,"context_line":"        image_policy \u003d image_meta.properties.get(image_key, default)"},{"line_number":1143,"context_line":"    else:"},{"line_number":1144,"context_line":"        image_policy \u003d default"},{"line_number":1145,"context_line":""},{"line_number":1146,"context_line":"    return flavor_policy, image_policy"},{"line_number":1147,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"9fb8cfa7_8c3adee4","line":1144,"range":{"start_line":1138,"start_character":0,"end_line":1144,"end_character":30},"in_reply_to":"9fb8cfa7_d54e24c0","updated":"2019-06-20 12:22:40.000000000","message":"Can we make the unit tests not allow None?\n\nBasically, I\u0027m saying I don\u0027t want to change this logic. Right now we would blow up if we got None - which presumably points to a bug in the code path. This delta would cause that bug to be ignored silently and trigger \"default\" behavior (whatever that is).","commit_id":"31f4a45e1864c0175daf0ddb47f5a9a04b3beb08"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"98da8a59f4040609344a42d1bcfb17bfd9c4a8df","unresolved":false,"context_lines":[{"line_number":1135,"context_line":"    flavor_key \u003d \u0027:\u0027.join([\u0027hw\u0027, key])"},{"line_number":1136,"context_line":"    image_key \u003d \u0027_\u0027.join([\u0027hw\u0027, key])"},{"line_number":1137,"context_line":""},{"line_number":1138,"context_line":"    extra_specs \u003d (flavor or {}).get(\u0027extra_specs\u0027, {})"},{"line_number":1139,"context_line":"    flavor_policy \u003d extra_specs.get(flavor_key, default)"},{"line_number":1140,"context_line":""},{"line_number":1141,"context_line":"    if image_meta:"},{"line_number":1142,"context_line":"        image_policy \u003d image_meta.properties.get(image_key, default)"},{"line_number":1143,"context_line":"    else:"},{"line_number":1144,"context_line":"        image_policy \u003d default"},{"line_number":1145,"context_line":""},{"line_number":1146,"context_line":"    return flavor_policy, image_policy"},{"line_number":1147,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"9fb8cfa7_f2ea5ed6","line":1144,"range":{"start_line":1138,"start_character":0,"end_line":1144,"end_character":30},"in_reply_to":"9fb8cfa7_efbfd9a7","updated":"2019-06-19 20:56:46.000000000","message":"Oh, is it because of [1]? Yeah, if that\u0027s a thing, I would rather you did it in _sev_required:\n\n    def _sev_required(self, flavor, image):\n        if not self.supports_amd_sev:\n            return False\n\n        return bool(hardware.mem_encryption_requesters(\n            flavor or {},\n            image or {}))\n\n(Except s/bool/any/ if you take my advice below.)\n\n[1] https://review.opendev.org/#/c/644565/9/nova/virt/libvirt/driver.py","commit_id":"31f4a45e1864c0175daf0ddb47f5a9a04b3beb08"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"18bc4382e0b7ba94f6f44669ec76d62526561e0e","unresolved":false,"context_lines":[{"line_number":1135,"context_line":"    flavor_key \u003d \u0027:\u0027.join([\u0027hw\u0027, key])"},{"line_number":1136,"context_line":"    image_key \u003d \u0027_\u0027.join([\u0027hw\u0027, key])"},{"line_number":1137,"context_line":""},{"line_number":1138,"context_line":"    extra_specs \u003d (flavor or {}).get(\u0027extra_specs\u0027, {})"},{"line_number":1139,"context_line":"    flavor_policy \u003d extra_specs.get(flavor_key, default)"},{"line_number":1140,"context_line":""},{"line_number":1141,"context_line":"    if image_meta:"},{"line_number":1142,"context_line":"        image_policy \u003d image_meta.properties.get(image_key, default)"},{"line_number":1143,"context_line":"    else:"},{"line_number":1144,"context_line":"        image_policy \u003d default"},{"line_number":1145,"context_line":""},{"line_number":1146,"context_line":"    return flavor_policy, image_policy"},{"line_number":1147,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"9fb8cfa7_d54e24c0","line":1144,"range":{"start_line":1138,"start_character":0,"end_line":1144,"end_character":30},"in_reply_to":"9fb8cfa7_f2ea5ed6","updated":"2019-06-20 12:09:46.000000000","message":"No, it\u0027s because of unit tests which allow them to be None.  These tests are now hitting this code path because of the extra validation check added to the API layer.","commit_id":"31f4a45e1864c0175daf0ddb47f5a9a04b3beb08"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"70e57837d45b7cd65a3803f3dabd45aa926a4189","unresolved":false,"context_lines":[{"line_number":2138,"context_line":"    # specified a requirement regarding memory encryption, and if both did,"},{"line_number":2139,"context_line":"    # they are asking for the same thing."},{"line_number":2140,"context_line":"    requesters \u003d []"},{"line_number":2141,"context_line":"    if flavor_mem_enc:"},{"line_number":2142,"context_line":"        requesters.append(\"hw:mem_encryption extra spec\")"},{"line_number":2143,"context_line":"    if image_mem_enc:"},{"line_number":2144,"context_line":"        requesters.append(\"hw_mem_encryption image property\")"},{"line_number":2145,"context_line":""},{"line_number":2146,"context_line":"    if not requesters:"},{"line_number":2147,"context_line":"        # This should never happen if the logic above is correct"}],"source_content_type":"text/x-python","patch_set":11,"id":"9fb8cfa7_cf069521","line":2144,"range":{"start_line":2141,"start_character":0,"end_line":2144,"end_character":61},"updated":"2019-06-19 20:50:23.000000000","message":"mmph, I have a couple of problems with this.\n\nI first came across the method name \"mem_encryption_requestors\" and assumed it was going to return something like (True, False) or {\u0027flavor\u0027: True, \u0027image\u0027: False} or...\n\nThen I saw how the result was being used in the request filter (boolean-ish) and was like wha...? But I didn\u0027t yet notice how it was being used in the log.\n\nIt wasn\u0027t until I got to the implementation that I realized the semantic is:\n- Return a list of zero, one, or two strings\n- If the list is empty, don\u0027t do mem encryption\n- Otherwise, do memory encryption, and the list can be used to construct a message, but you have to know exactly how that message needs to be put together.\n\nThat seems really odd, and tightly coupled to the consuming code.\n\nMy second issue is the way message construction is being done. It\u0027s generally a no-no to put otherwise-translatable words/phrases into format vars, because a) they don\u0027t get translated, so the message winds up being a mix of English and $language; and b) assuming the injected text is comprehensible to the reader, the whole may not make any sense due to differences in sentence structure etc.\n\nNow, in this case, we\u0027re only using the result for a debug log, which isn\u0027t translated, so the second point is moot... until some other consumer decides to use it for an exception, or we decide to start translating logs again, or...\n\nI\u0027m not going to block on the second thing. But I am going to downvote on the tight coupling and general ickiness.\n\nI would prefer if this method returned ($bool, $bool) indicating whether memory encryption was requested by the (flavor, image) -- described explicitly in the docstring. The caller can do the switch-on logic with any(); and then can do all the message assembly locally.","commit_id":"31f4a45e1864c0175daf0ddb47f5a9a04b3beb08"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"31c9343c8611dd2bd60aae6e0127c4960eca3e43","unresolved":false,"context_lines":[{"line_number":2138,"context_line":"    # specified a requirement regarding memory encryption, and if both did,"},{"line_number":2139,"context_line":"    # they are asking for the same thing."},{"line_number":2140,"context_line":"    requesters \u003d []"},{"line_number":2141,"context_line":"    if flavor_mem_enc:"},{"line_number":2142,"context_line":"        requesters.append(\"hw:mem_encryption extra spec\")"},{"line_number":2143,"context_line":"    if image_mem_enc:"},{"line_number":2144,"context_line":"        requesters.append(\"hw_mem_encryption image property\")"},{"line_number":2145,"context_line":""},{"line_number":2146,"context_line":"    if not requesters:"},{"line_number":2147,"context_line":"        # This should never happen if the logic above is correct"}],"source_content_type":"text/x-python","patch_set":11,"id":"9fb8cfa7_cc621617","line":2144,"range":{"start_line":2141,"start_character":0,"end_line":2144,"end_character":61},"in_reply_to":"9fb8cfa7_0c76ee07","updated":"2019-06-20 12:22:40.000000000","message":"I had a better idea in my sleep last night:\n\nLet\u0027s make this method mem_encryption_requested, and have it return a single boolean (or raise), and have it do the logging itself.\n\nThe request filter can tack on another log just saying \"requesting mem encryption\" which, in the context of the other logs, is all that\u0027s necessary.","commit_id":"31f4a45e1864c0175daf0ddb47f5a9a04b3beb08"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"ed39148aba4c7aff9f1044511d03e65b6c8c1fa0","unresolved":false,"context_lines":[{"line_number":2138,"context_line":"    # specified a requirement regarding memory encryption, and if both did,"},{"line_number":2139,"context_line":"    # they are asking for the same thing."},{"line_number":2140,"context_line":"    requesters \u003d []"},{"line_number":2141,"context_line":"    if flavor_mem_enc:"},{"line_number":2142,"context_line":"        requesters.append(\"hw:mem_encryption extra spec\")"},{"line_number":2143,"context_line":"    if image_mem_enc:"},{"line_number":2144,"context_line":"        requesters.append(\"hw_mem_encryption image property\")"},{"line_number":2145,"context_line":""},{"line_number":2146,"context_line":"    if not requesters:"},{"line_number":2147,"context_line":"        # This should never happen if the logic above is correct"}],"source_content_type":"text/x-python","patch_set":11,"id":"9fb8cfa7_67f305e9","line":2144,"range":{"start_line":2141,"start_character":0,"end_line":2144,"end_character":61},"in_reply_to":"9fb8cfa7_cc621617","updated":"2019-06-20 13:53:45.000000000","message":"Done","commit_id":"31f4a45e1864c0175daf0ddb47f5a9a04b3beb08"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"18bc4382e0b7ba94f6f44669ec76d62526561e0e","unresolved":false,"context_lines":[{"line_number":2138,"context_line":"    # specified a requirement regarding memory encryption, and if both did,"},{"line_number":2139,"context_line":"    # they are asking for the same thing."},{"line_number":2140,"context_line":"    requesters \u003d []"},{"line_number":2141,"context_line":"    if flavor_mem_enc:"},{"line_number":2142,"context_line":"        requesters.append(\"hw:mem_encryption extra spec\")"},{"line_number":2143,"context_line":"    if image_mem_enc:"},{"line_number":2144,"context_line":"        requesters.append(\"hw_mem_encryption image property\")"},{"line_number":2145,"context_line":""},{"line_number":2146,"context_line":"    if not requesters:"},{"line_number":2147,"context_line":"        # This should never happen if the logic above is correct"}],"source_content_type":"text/x-python","patch_set":11,"id":"9fb8cfa7_0c76ee07","line":2144,"range":{"start_line":2141,"start_character":0,"end_line":2144,"end_character":61},"in_reply_to":"9fb8cfa7_cf069521","updated":"2019-06-20 12:09:46.000000000","message":"Agreed on all points - will fix.","commit_id":"31f4a45e1864c0175daf0ddb47f5a9a04b3beb08"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"70e57837d45b7cd65a3803f3dabd45aa926a4189","unresolved":false,"context_lines":[{"line_number":2145,"context_line":""},{"line_number":2146,"context_line":"    if not requesters:"},{"line_number":2147,"context_line":"        # This should never happen if the logic above is correct"},{"line_number":2148,"context_line":"        LOG.error(\"BUG: require_encrypted_memory_support \""},{"line_number":2149,"context_line":"                  \"request filter was applied for unknown reason\")"},{"line_number":2150,"context_line":""},{"line_number":2151,"context_line":"    return requesters"}],"source_content_type":"text/x-python","patch_set":11,"id":"9fb8cfa7_d2e5da39","line":2149,"range":{"start_line":2148,"start_character":24,"end_line":2149,"end_character":45},"updated":"2019-06-19 20:50:23.000000000","message":"This is now out of place\n\nWe can probably just get rid of this block","commit_id":"31f4a45e1864c0175daf0ddb47f5a9a04b3beb08"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"31c9343c8611dd2bd60aae6e0127c4960eca3e43","unresolved":false,"context_lines":[{"line_number":2145,"context_line":""},{"line_number":2146,"context_line":"    if not requesters:"},{"line_number":2147,"context_line":"        # This should never happen if the logic above is correct"},{"line_number":2148,"context_line":"        LOG.error(\"BUG: require_encrypted_memory_support \""},{"line_number":2149,"context_line":"                  \"request filter was applied for unknown reason\")"},{"line_number":2150,"context_line":""},{"line_number":2151,"context_line":"    return requesters"}],"source_content_type":"text/x-python","patch_set":11,"id":"9fb8cfa7_8cb73e87","line":2149,"range":{"start_line":2148,"start_character":24,"end_line":2149,"end_character":45},"in_reply_to":"9fb8cfa7_2c71720d","updated":"2019-06-20 12:22:40.000000000","message":"I just meant that this method has nothing to do with request filters being applied. The message should talk about the logic that got us here.\n\nBut like I said, we can probably just remove this condition anyway, as being overly paranoid and unreachable.","commit_id":"31f4a45e1864c0175daf0ddb47f5a9a04b3beb08"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"ed39148aba4c7aff9f1044511d03e65b6c8c1fa0","unresolved":false,"context_lines":[{"line_number":2145,"context_line":""},{"line_number":2146,"context_line":"    if not requesters:"},{"line_number":2147,"context_line":"        # This should never happen if the logic above is correct"},{"line_number":2148,"context_line":"        LOG.error(\"BUG: require_encrypted_memory_support \""},{"line_number":2149,"context_line":"                  \"request filter was applied for unknown reason\")"},{"line_number":2150,"context_line":""},{"line_number":2151,"context_line":"    return requesters"}],"source_content_type":"text/x-python","patch_set":11,"id":"9fb8cfa7_670d4534","line":2149,"range":{"start_line":2148,"start_character":24,"end_line":2149,"end_character":45},"in_reply_to":"9fb8cfa7_8cb73e87","updated":"2019-06-20 13:53:45.000000000","message":"Ohhh sorry, missed your highlighting :-)  Normally I\u0027d prefer to keep it, as every single damn time I add a sanity check which seems overly paranoid and has me thinking \"this can\u0027t ever possibly be reached\", Murphy\u0027s Law kicks in ...\n\nBut maybe this case is particularly paranoid, so I can live without it.","commit_id":"31f4a45e1864c0175daf0ddb47f5a9a04b3beb08"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"18bc4382e0b7ba94f6f44669ec76d62526561e0e","unresolved":false,"context_lines":[{"line_number":2145,"context_line":""},{"line_number":2146,"context_line":"    if not requesters:"},{"line_number":2147,"context_line":"        # This should never happen if the logic above is correct"},{"line_number":2148,"context_line":"        LOG.error(\"BUG: require_encrypted_memory_support \""},{"line_number":2149,"context_line":"                  \"request filter was applied for unknown reason\")"},{"line_number":2150,"context_line":""},{"line_number":2151,"context_line":"    return requesters"}],"source_content_type":"text/x-python","patch_set":11,"id":"9fb8cfa7_2c71720d","line":2149,"range":{"start_line":2148,"start_character":24,"end_line":2149,"end_character":45},"in_reply_to":"9fb8cfa7_d2e5da39","updated":"2019-06-20 12:09:46.000000000","message":"Why is it now out of place?","commit_id":"31f4a45e1864c0175daf0ddb47f5a9a04b3beb08"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"76c6538dc5f828399684b8fbe835ae1a62cd439d","unresolved":false,"context_lines":[{"line_number":2075,"context_line":"    return updated_numa_topology"},{"line_number":2076,"context_line":""},{"line_number":2077,"context_line":""},{"line_number":2078,"context_line":"def mem_encryption_requested(flavor, image_meta):"},{"line_number":2079,"context_line":"    \"\"\"Return a boolean indicating whether encryption of guest memory was"},{"line_number":2080,"context_line":"    requested, either via the hw:mem_encryption extra spec or the"},{"line_number":2081,"context_line":"    hw_mem_encryption image property (or both)."}],"source_content_type":"text/x-python","patch_set":31,"id":"7faddb67_ed59278e","line":2078,"range":{"start_line":2078,"start_character":4,"end_line":2078,"end_character":28},"updated":"2019-08-09 15:20:10.000000000","message":"This is such a nit, but I\u0027d really like to see this called \u0027get_mem_encryption_constraint\u0027 since we have a pattern here already for this stuff. Also, maybe move it up with all those (that\u0027s a personal one, since I\u0027m removing vast swathes from the end of this file in cpu-resources)","commit_id":"adec7baaf4d94ddea461558169359d152b6223fc"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"42e91f1a8a93669cbf3ecf52ce9cd921e97265e4","unresolved":false,"context_lines":[{"line_number":2075,"context_line":"    return updated_numa_topology"},{"line_number":2076,"context_line":""},{"line_number":2077,"context_line":""},{"line_number":2078,"context_line":"def mem_encryption_requested(flavor, image_meta):"},{"line_number":2079,"context_line":"    \"\"\"Return a boolean indicating whether encryption of guest memory was"},{"line_number":2080,"context_line":"    requested, either via the hw:mem_encryption extra spec or the"},{"line_number":2081,"context_line":"    hw_mem_encryption image property (or both)."}],"source_content_type":"text/x-python","patch_set":31,"id":"7faddb67_68ccc233","line":2078,"range":{"start_line":2078,"start_character":4,"end_line":2078,"end_character":28},"in_reply_to":"7faddb67_ed59278e","updated":"2019-08-19 13:10:31.000000000","message":"Done","commit_id":"adec7baaf4d94ddea461558169359d152b6223fc"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"76c6538dc5f828399684b8fbe835ae1a62cd439d","unresolved":false,"context_lines":[{"line_number":2087,"context_line":"    flavor_mem_enc_str, image_mem_enc \u003d _get_flavor_image_meta("},{"line_number":2088,"context_line":"        \u0027mem_encryption\u0027, flavor, image_meta)"},{"line_number":2089,"context_line":""},{"line_number":2090,"context_line":"    if flavor_mem_enc_str is None:"},{"line_number":2091,"context_line":"        flavor_mem_enc \u003d None"},{"line_number":2092,"context_line":"    else:"},{"line_number":2093,"context_line":"        flavor_mem_enc \u003d strutils.bool_from_string(flavor_mem_enc_str)"},{"line_number":2094,"context_line":""},{"line_number":2095,"context_line":"    # Image property is a FlexibleBooleanField, so coercion to a"}],"source_content_type":"text/x-python","patch_set":31,"id":"7faddb67_0dd4c3bc","line":2092,"range":{"start_line":2090,"start_character":0,"end_line":2092,"end_character":9},"updated":"2019-08-09 15:20:10.000000000","message":"femtonit:\n\n  flavor_mem_enc \u003d None\n  if flavor_mem_enc_str is not None:\n      flavor_mem_enc \u003d strutils.bool_from_string(flavor_mem_enc_str)","commit_id":"adec7baaf4d94ddea461558169359d152b6223fc"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"42e91f1a8a93669cbf3ecf52ce9cd921e97265e4","unresolved":false,"context_lines":[{"line_number":2087,"context_line":"    flavor_mem_enc_str, image_mem_enc \u003d _get_flavor_image_meta("},{"line_number":2088,"context_line":"        \u0027mem_encryption\u0027, flavor, image_meta)"},{"line_number":2089,"context_line":""},{"line_number":2090,"context_line":"    if flavor_mem_enc_str is None:"},{"line_number":2091,"context_line":"        flavor_mem_enc \u003d None"},{"line_number":2092,"context_line":"    else:"},{"line_number":2093,"context_line":"        flavor_mem_enc \u003d strutils.bool_from_string(flavor_mem_enc_str)"},{"line_number":2094,"context_line":""},{"line_number":2095,"context_line":"    # Image property is a FlexibleBooleanField, so coercion to a"}],"source_content_type":"text/x-python","patch_set":31,"id":"7faddb67_e8df52d4","line":2092,"range":{"start_line":2090,"start_character":0,"end_line":2092,"end_character":9},"in_reply_to":"7faddb67_0dd4c3bc","updated":"2019-08-19 13:10:31.000000000","message":"femtonit but nevertheless nicer, I agree.  Done.","commit_id":"adec7baaf4d94ddea461558169359d152b6223fc"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"76c6538dc5f828399684b8fbe835ae1a62cd439d","unresolved":false,"context_lines":[{"line_number":2102,"context_line":"    # memory encryption."},{"line_number":2103,"context_line":"    if (flavor_mem_enc is not None and image_mem_enc is not None and"},{"line_number":2104,"context_line":"            flavor_mem_enc !\u003d image_mem_enc):"},{"line_number":2105,"context_line":"        # Unfortunately there\u0027s no way to avoid this duplication :-("},{"line_number":2106,"context_line":"        lmsg \u003d ("},{"line_number":2107,"context_line":"            \"Flavor %(flavor_name)s has hw:mem_encryption extra spec \""},{"line_number":2108,"context_line":"            \"explicitly set to %(flavor_val)s, conflicting with \""},{"line_number":2109,"context_line":"            \"image %(image_name)s which has hw_mem_encryption property \""},{"line_number":2110,"context_line":"            \"explicitly set to %(image_val)s\""},{"line_number":2111,"context_line":"        )"},{"line_number":2112,"context_line":"        emsg \u003d _("},{"line_number":2113,"context_line":"            \"Flavor %(flavor_name)s has hw:mem_encryption extra spec \""},{"line_number":2114,"context_line":"            \"explicitly set to %(flavor_val)s, conflicting with \""}],"source_content_type":"text/x-python","patch_set":31,"id":"7faddb67_4df39b74","line":2111,"range":{"start_line":2105,"start_character":0,"end_line":2111,"end_character":9},"updated":"2019-08-09 15:20:10.000000000","message":"Do we need the log when we have the exception? I mean, we\u0027re not doing it anywhere else. Do we realllllly need it?","commit_id":"adec7baaf4d94ddea461558169359d152b6223fc"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"42e91f1a8a93669cbf3ecf52ce9cd921e97265e4","unresolved":false,"context_lines":[{"line_number":2102,"context_line":"    # memory encryption."},{"line_number":2103,"context_line":"    if (flavor_mem_enc is not None and image_mem_enc is not None and"},{"line_number":2104,"context_line":"            flavor_mem_enc !\u003d image_mem_enc):"},{"line_number":2105,"context_line":"        # Unfortunately there\u0027s no way to avoid this duplication :-("},{"line_number":2106,"context_line":"        lmsg \u003d ("},{"line_number":2107,"context_line":"            \"Flavor %(flavor_name)s has hw:mem_encryption extra spec \""},{"line_number":2108,"context_line":"            \"explicitly set to %(flavor_val)s, conflicting with \""},{"line_number":2109,"context_line":"            \"image %(image_name)s which has hw_mem_encryption property \""},{"line_number":2110,"context_line":"            \"explicitly set to %(image_val)s\""},{"line_number":2111,"context_line":"        )"},{"line_number":2112,"context_line":"        emsg \u003d _("},{"line_number":2113,"context_line":"            \"Flavor %(flavor_name)s has hw:mem_encryption extra spec \""},{"line_number":2114,"context_line":"            \"explicitly set to %(flavor_val)s, conflicting with \""}],"source_content_type":"text/x-python","patch_set":31,"id":"7faddb67_8309df2b","line":2111,"range":{"start_line":2105,"start_character":0,"end_line":2111,"end_character":9},"in_reply_to":"7faddb67_4df39b74","updated":"2019-08-19 13:10:31.000000000","message":"Heh, remember these?\n\nhttps://review.opendev.org/#/c/664670/\nhttps://review.opendev.org/#/c/665112/\n\nThey came from trying to figure out how to do this exact bit of code right.\n\nBut OK, you are asking whether we need both in this particular situation.  Honest answer - I don\u0027t really know, because I\u0027m not sure what are all the places which might catch and handle this exception, and whether they would log it appropriately.  Suggestions welcome.  I guess some more manual testing is needed.","commit_id":"adec7baaf4d94ddea461558169359d152b6223fc"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"76c6538dc5f828399684b8fbe835ae1a62cd439d","unresolved":false,"context_lines":[{"line_number":2131,"context_line":"    if flavor_mem_enc:"},{"line_number":2132,"context_line":"        requesters.append(\"hw:mem_encryption extra spec in %s flavor\" %"},{"line_number":2133,"context_line":"                          flavor.name)"},{"line_number":2134,"context_line":"    if image_mem_enc:"},{"line_number":2135,"context_line":"        requesters.append(\"hw_mem_encryption property of image %s\" %"},{"line_number":2136,"context_line":"                          image_meta.name)"},{"line_number":2137,"context_line":""}],"source_content_type":"text/x-python","patch_set":31,"id":"7faddb67_ad86cfb9","line":2134,"range":{"start_line":2134,"start_character":0,"end_line":2134,"end_character":21},"updated":"2019-08-09 15:20:10.000000000","message":"else (since we know we have one or the other here, right?)","commit_id":"adec7baaf4d94ddea461558169359d152b6223fc"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"42e91f1a8a93669cbf3ecf52ce9cd921e97265e4","unresolved":false,"context_lines":[{"line_number":2131,"context_line":"    if flavor_mem_enc:"},{"line_number":2132,"context_line":"        requesters.append(\"hw:mem_encryption extra spec in %s flavor\" %"},{"line_number":2133,"context_line":"                          flavor.name)"},{"line_number":2134,"context_line":"    if image_mem_enc:"},{"line_number":2135,"context_line":"        requesters.append(\"hw_mem_encryption property of image %s\" %"},{"line_number":2136,"context_line":"                          image_meta.name)"},{"line_number":2137,"context_line":""}],"source_content_type":"text/x-python","patch_set":31,"id":"7faddb67_08f66e5e","line":2134,"range":{"start_line":2134,"start_character":0,"end_line":2134,"end_character":21},"in_reply_to":"7faddb67_ad86cfb9","updated":"2019-08-19 13:10:31.000000000","message":"No, we could have both.  There are test cases specifically for that, in fact.","commit_id":"adec7baaf4d94ddea461558169359d152b6223fc"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"4d2a8dca052c6d3cf68ef9f5afa91aaef8bdd754","unresolved":false,"context_lines":[{"line_number":1221,"context_line":"            \u0027image_name\u0027: image_meta.name,"},{"line_number":1222,"context_line":"            \u0027image_val\u0027: image_mem_enc,"},{"line_number":1223,"context_line":"        }"},{"line_number":1224,"context_line":"        LOG.error(lmsg, data)"},{"line_number":1225,"context_line":"        raise exception.FlavorImageConflict(emsg % data)"},{"line_number":1226,"context_line":""},{"line_number":1227,"context_line":""}],"source_content_type":"text/x-python","patch_set":40,"id":"7faddb67_d69a026d","line":1224,"updated":"2019-08-21 17:05:50.000000000","message":"I asked this before and you probably replied, in which case I\u0027ll hunt out the reply tomorrow, but do we _really_ need to do this? Wouldn\u0027t an exception be good enough, assuming we catch and log those at the API level anyway?","commit_id":"8706c91037b3bb636723e1f9fdf55996151792b2"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"ab67b7a01dc1c44f8fdc99c969f9d8a66e245b71","unresolved":false,"context_lines":[{"line_number":1221,"context_line":"            \u0027image_name\u0027: image_meta.name,"},{"line_number":1222,"context_line":"            \u0027image_val\u0027: image_mem_enc,"},{"line_number":1223,"context_line":"        }"},{"line_number":1224,"context_line":"        LOG.error(lmsg, data)"},{"line_number":1225,"context_line":"        raise exception.FlavorImageConflict(emsg % data)"},{"line_number":1226,"context_line":""},{"line_number":1227,"context_line":""}],"source_content_type":"text/x-python","patch_set":40,"id":"7faddb67_9937ddb0","line":1224,"in_reply_to":"7faddb67_d69a026d","updated":"2019-08-21 17:50:25.000000000","message":"Yes, I did reply to this:\n\nhttps://review.opendev.org/#/c/664420/31/nova/virt/hardware.py@2111\n\nand also replied in the same patch set to a whole bunch of other feedback you previously gave.","commit_id":"8706c91037b3bb636723e1f9fdf55996151792b2"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"16ae5284d7d70eb620eadf518bba07ce16dcecfc","unresolved":false,"context_lines":[{"line_number":1192,"context_line":""},{"line_number":1193,"context_line":"    _check_mem_encryption_uses_uefi_image(requesters, image_meta)"},{"line_number":1194,"context_line":""},{"line_number":1195,"context_line":"    LOG.debug(\"Memory encryption requested by %s\", \" and \".join(requesters))"},{"line_number":1196,"context_line":"    return True"},{"line_number":1197,"context_line":""},{"line_number":1198,"context_line":""}],"source_content_type":"text/x-python","patch_set":42,"id":"7faddb67_ab7cb4b5","line":1195,"updated":"2019-08-22 07:11:04.000000000","message":"not sure this info is useful or not, since it isn\u0027t bind to instance. we don\u0027t this is about which instance.","commit_id":"ac44947c5e0d6896032409f951f05989ec64444b"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"7c8dceccb830cf98d3141683a0859f1896b5c1a5","unresolved":false,"context_lines":[{"line_number":1192,"context_line":""},{"line_number":1193,"context_line":"    _check_mem_encryption_uses_uefi_image(requesters, image_meta)"},{"line_number":1194,"context_line":""},{"line_number":1195,"context_line":"    LOG.debug(\"Memory encryption requested by %s\", \" and \".join(requesters))"},{"line_number":1196,"context_line":"    return True"},{"line_number":1197,"context_line":""},{"line_number":1198,"context_line":""}],"source_content_type":"text/x-python","patch_set":42,"id":"7faddb67_c679e230","line":1195,"in_reply_to":"7faddb67_ab7cb4b5","updated":"2019-08-22 09:39:02.000000000","message":"At very least, it\u0027s useful:\n\n- in unit tests\n- in real environments where this message is expected but not seen, it tells us that something was wrong with the flavor or image metadata.\n\nAlso individual DEBUG messages are not always 100% helpful by themselves ;-)  That is normal, because it is expected they will be read by developers who can figure out the context.","commit_id":"ac44947c5e0d6896032409f951f05989ec64444b"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"16ae5284d7d70eb620eadf518bba07ce16dcecfc","unresolved":false,"context_lines":[{"line_number":1221,"context_line":"            \u0027image_name\u0027: image_meta.name,"},{"line_number":1222,"context_line":"            \u0027image_val\u0027: image_mem_enc,"},{"line_number":1223,"context_line":"        }"},{"line_number":1224,"context_line":"        LOG.error(lmsg, data)"},{"line_number":1225,"context_line":"        raise exception.FlavorImageConflict(emsg % data)"},{"line_number":1226,"context_line":""},{"line_number":1227,"context_line":""}],"source_content_type":"text/x-python","patch_set":42,"id":"7faddb67_2b69a4f6","line":1224,"updated":"2019-08-22 07:11:04.000000000","message":"we needn\u0027t this error message, this isn\u0027t any system error. It is just user doesn\u0027t input valid value.","commit_id":"ac44947c5e0d6896032409f951f05989ec64444b"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"d65671b3ba719ae2b29dc443a64233899fcfe6b2","unresolved":false,"context_lines":[{"line_number":1221,"context_line":"            \u0027image_name\u0027: image_meta.name,"},{"line_number":1222,"context_line":"            \u0027image_val\u0027: image_mem_enc,"},{"line_number":1223,"context_line":"        }"},{"line_number":1224,"context_line":"        LOG.error(lmsg, data)"},{"line_number":1225,"context_line":"        raise exception.FlavorImageConflict(emsg % data)"},{"line_number":1226,"context_line":""},{"line_number":1227,"context_line":""}],"source_content_type":"text/x-python","patch_set":42,"id":"7faddb67_86f9aa4f","line":1224,"in_reply_to":"7faddb67_269a3662","updated":"2019-08-22 11:48:41.000000000","message":"Done","commit_id":"ac44947c5e0d6896032409f951f05989ec64444b"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"7c8dceccb830cf98d3141683a0859f1896b5c1a5","unresolved":false,"context_lines":[{"line_number":1221,"context_line":"            \u0027image_name\u0027: image_meta.name,"},{"line_number":1222,"context_line":"            \u0027image_val\u0027: image_mem_enc,"},{"line_number":1223,"context_line":"        }"},{"line_number":1224,"context_line":"        LOG.error(lmsg, data)"},{"line_number":1225,"context_line":"        raise exception.FlavorImageConflict(emsg % data)"},{"line_number":1226,"context_line":""},{"line_number":1227,"context_line":""}],"source_content_type":"text/x-python","patch_set":42,"id":"7faddb67_269a3662","line":1224,"in_reply_to":"7faddb67_2b69a4f6","updated":"2019-08-22 09:39:02.000000000","message":"Please see:\n\nhttps://review.opendev.org/#/c/664420/31/nova/virt/hardware.py@2111\n\nbut OK, I will remove it.","commit_id":"ac44947c5e0d6896032409f951f05989ec64444b"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"16ae5284d7d70eb620eadf518bba07ce16dcecfc","unresolved":false,"context_lines":[{"line_number":1222,"context_line":"            \u0027image_val\u0027: image_mem_enc,"},{"line_number":1223,"context_line":"        }"},{"line_number":1224,"context_line":"        LOG.error(lmsg, data)"},{"line_number":1225,"context_line":"        raise exception.FlavorImageConflict(emsg % data)"},{"line_number":1226,"context_line":""},{"line_number":1227,"context_line":""},{"line_number":1228,"context_line":"def _check_mem_encryption_uses_uefi_image(requesters, image_meta):"}],"source_content_type":"text/x-python","patch_set":42,"id":"7faddb67_eb24ecc7","line":1225,"updated":"2019-08-22 07:11:04.000000000","message":"not sure we should use this generic exception. For other extra spec and image metadata, like cpu_policy, it also have conflict check, but it use it own exception.","commit_id":"ac44947c5e0d6896032409f951f05989ec64444b"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"7c8dceccb830cf98d3141683a0859f1896b5c1a5","unresolved":false,"context_lines":[{"line_number":1222,"context_line":"            \u0027image_val\u0027: image_mem_enc,"},{"line_number":1223,"context_line":"        }"},{"line_number":1224,"context_line":"        LOG.error(lmsg, data)"},{"line_number":1225,"context_line":"        raise exception.FlavorImageConflict(emsg % data)"},{"line_number":1226,"context_line":""},{"line_number":1227,"context_line":""},{"line_number":1228,"context_line":"def _check_mem_encryption_uses_uefi_image(requesters, image_meta):"}],"source_content_type":"text/x-python","patch_set":42,"id":"7faddb67_86ef6ab0","line":1225,"in_reply_to":"7faddb67_eb24ecc7","updated":"2019-08-22 09:39:02.000000000","message":"Eric asked me to do this:\n\nhttp://eavesdrop.openstack.org/irclogs/%23openstack-nova/%23openstack-nova.2019-06-12.log.html#t2019-06-12T18:25:43\n\nHowever right now the exception is not very generic since this patch is the only user.  Maybe in the future https://review.opendev.org/#/c/663365/ will also use it.  I don\u0027t really see huge value in separate exceptions for each of those use cases, but I don\u0027t mind either way so I\u0027ll leave you to debate that with Eric :-)","commit_id":"ac44947c5e0d6896032409f951f05989ec64444b"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"16ae5284d7d70eb620eadf518bba07ce16dcecfc","unresolved":false,"context_lines":[{"line_number":1240,"context_line":"    )"},{"line_number":1241,"context_line":"    data \u003d {\u0027requesters\u0027: \" and \".join(requesters),"},{"line_number":1242,"context_line":"            \u0027image_name\u0027: image_meta.name}"},{"line_number":1243,"context_line":"    LOG.error(lmsg, data)"},{"line_number":1244,"context_line":"    raise exception.FlavorImageConflict(emsg % data)"},{"line_number":1245,"context_line":""},{"line_number":1246,"context_line":""}],"source_content_type":"text/x-python","patch_set":42,"id":"7faddb67_4b6c2006","line":1243,"updated":"2019-08-22 07:11:04.000000000","message":"we needn\u0027t this error log also.","commit_id":"ac44947c5e0d6896032409f951f05989ec64444b"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"7c8dceccb830cf98d3141683a0859f1896b5c1a5","unresolved":false,"context_lines":[{"line_number":1240,"context_line":"    )"},{"line_number":1241,"context_line":"    data \u003d {\u0027requesters\u0027: \" and \".join(requesters),"},{"line_number":1242,"context_line":"            \u0027image_name\u0027: image_meta.name}"},{"line_number":1243,"context_line":"    LOG.error(lmsg, data)"},{"line_number":1244,"context_line":"    raise exception.FlavorImageConflict(emsg % data)"},{"line_number":1245,"context_line":""},{"line_number":1246,"context_line":""}],"source_content_type":"text/x-python","patch_set":42,"id":"7faddb67_86444aba","line":1243,"in_reply_to":"7faddb67_4b6c2006","updated":"2019-08-22 09:39:02.000000000","message":"OK according to file:///home/adam/SUSE/cloud/OpenStack/git/oslo.log/doc/build/html/user/guidelines.html#definition-of-log-levels you are right, I will remove.","commit_id":"ac44947c5e0d6896032409f951f05989ec64444b"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"d65671b3ba719ae2b29dc443a64233899fcfe6b2","unresolved":false,"context_lines":[{"line_number":1240,"context_line":"    )"},{"line_number":1241,"context_line":"    data \u003d {\u0027requesters\u0027: \" and \".join(requesters),"},{"line_number":1242,"context_line":"            \u0027image_name\u0027: image_meta.name}"},{"line_number":1243,"context_line":"    LOG.error(lmsg, data)"},{"line_number":1244,"context_line":"    raise exception.FlavorImageConflict(emsg % data)"},{"line_number":1245,"context_line":""},{"line_number":1246,"context_line":""}],"source_content_type":"text/x-python","patch_set":42,"id":"7faddb67_46ec1292","line":1243,"in_reply_to":"7faddb67_86444aba","updated":"2019-08-22 11:48:41.000000000","message":"Done","commit_id":"ac44947c5e0d6896032409f951f05989ec64444b"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"56e3989c25eb781e1d54dd86e7225608fc88b87e","unresolved":false,"context_lines":[{"line_number":1214,"context_line":"        raise exception.FlavorImageConflict(emsg % data)"},{"line_number":1215,"context_line":""},{"line_number":1216,"context_line":""},{"line_number":1217,"context_line":"def _check_mem_encryption_uses_uefi_image(requesters, image_meta):"},{"line_number":1218,"context_line":"    if image_meta.properties.hw_firmware_type \u003d\u003d \u0027uefi\u0027:"},{"line_number":1219,"context_line":"        return"},{"line_number":1220,"context_line":""}],"source_content_type":"text/x-python","patch_set":46,"id":"7faddb67_8087c7b6","line":1217,"range":{"start_line":1217,"start_character":0,"end_line":1217,"end_character":66},"updated":"2019-08-30 09:51:37.000000000","message":"I would so much rather roll this into the previous one, because passing partial log messages around seems silly. However, we\u0027ve had this come up a few times so this is merely me \"just sayin\u0027\" :)","commit_id":"3f53fed18665b6295d84d196ce681870617b87c5"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"49d145c142846098afbe3306240cdc8de3f9e60f","unresolved":false,"context_lines":[{"line_number":1214,"context_line":"        raise exception.FlavorImageConflict(emsg % data)"},{"line_number":1215,"context_line":""},{"line_number":1216,"context_line":""},{"line_number":1217,"context_line":"def _check_mem_encryption_uses_uefi_image(requesters, image_meta):"},{"line_number":1218,"context_line":"    if image_meta.properties.hw_firmware_type \u003d\u003d \u0027uefi\u0027:"},{"line_number":1219,"context_line":"        return"},{"line_number":1220,"context_line":""}],"source_content_type":"text/x-python","patch_set":46,"id":"7faddb67_e26c7b08","line":1217,"range":{"start_line":1217,"start_character":0,"end_line":1217,"end_character":66},"in_reply_to":"7faddb67_47a7a1a9","updated":"2019-08-30 15:18:55.000000000","message":"\u003e Understood, but this feels like we\u0027re reducing the discussion down to \"small functions are good and big functions are bad\", which is of course a huge oversimplification.\n\nAgreed.\n\n\u003e I\u0027ve clearly got many years less professional experience (a mere 6) but things like coupling vs. cohesion and cyclomatic complexity are fortunately taught from the get go and in my opinion splitting things up like we\u0027re doing here reduces the cohesiveness/readability of the function\n\nI think it significantly increases readability / maintainability / testability but yeah, we can debate that over a beer sometime ;-)\n\n\u003e Put it this way: if I were to insert a couple of functions between this function and the one above, such that they ended up on different pages, would you find this implementation as easy to grok as if they were merged?\n\nIdeally that wouldn\u0027t happen, but honestly yes, I would definitely still find it easier to grok than if they were merged.  And anyway these days it\u0027s just a keystroke to jump from a method invocation to its definition.\n\n\u003e And again, none of this matters since it\u0027s all just talk at this point.\n\nRight :-)  Another time/place.","commit_id":"3f53fed18665b6295d84d196ce681870617b87c5"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"73b3f273224446132f461a49039a83c617e93300","unresolved":false,"context_lines":[{"line_number":1214,"context_line":"        raise exception.FlavorImageConflict(emsg % data)"},{"line_number":1215,"context_line":""},{"line_number":1216,"context_line":""},{"line_number":1217,"context_line":"def _check_mem_encryption_uses_uefi_image(requesters, image_meta):"},{"line_number":1218,"context_line":"    if image_meta.properties.hw_firmware_type \u003d\u003d \u0027uefi\u0027:"},{"line_number":1219,"context_line":"        return"},{"line_number":1220,"context_line":""}],"source_content_type":"text/x-python","patch_set":46,"id":"7faddb67_d12fd3f2","line":1217,"range":{"start_line":1217,"start_character":0,"end_line":1217,"end_character":66},"in_reply_to":"7faddb67_8087c7b6","updated":"2019-08-30 13:08:17.000000000","message":"\u003e I would so much rather roll this into the previous one, because passing partial log messages around seems silly.\n\nWhy?\n\n\u003e However, we\u0027ve had this come up a few times so this is merely me \"just sayin\u0027\" :)\n\nYeah I know - thanks for being open to tolerating differences of opinion ;-)  And if you didn\u0027t already, please consider the various costs of long methods which I have mentioned several times before.  I strongly believe it\u0027s way too easy to underestimate how damaging they are (and IMHO the vast majority of engineers *do* underestimate this).  If I was only allowed to give a single piece of advice to coders starting out their career (which obviously excludes you, since you\u0027re already extremely experienced) it would probably be \"write shorter functions/methods\".  Of course you\u0027re entitled to your own opinion.  FWIW mine is based on ~29 years of professional coding experience - just sayin\u0027 ;-)","commit_id":"3f53fed18665b6295d84d196ce681870617b87c5"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"63195ccd9f27e58893981c815941c5b6d564c57c","unresolved":false,"context_lines":[{"line_number":1214,"context_line":"        raise exception.FlavorImageConflict(emsg % data)"},{"line_number":1215,"context_line":""},{"line_number":1216,"context_line":""},{"line_number":1217,"context_line":"def _check_mem_encryption_uses_uefi_image(requesters, image_meta):"},{"line_number":1218,"context_line":"    if image_meta.properties.hw_firmware_type \u003d\u003d \u0027uefi\u0027:"},{"line_number":1219,"context_line":"        return"},{"line_number":1220,"context_line":""}],"source_content_type":"text/x-python","patch_set":46,"id":"7faddb67_47a7a1a9","line":1217,"range":{"start_line":1217,"start_character":0,"end_line":1217,"end_character":66},"in_reply_to":"7faddb67_d12fd3f2","updated":"2019-08-30 14:50:21.000000000","message":"\u003e \u003e I would so much rather roll this into the previous one, because\n \u003e passing partial log messages around seems silly.\n \u003e \n \u003e Why?\n\nMostly because it\u0027s incohesive (to me). See below.\n\n \u003e \u003e However, we\u0027ve had this come up a few times so this is merely me\n \u003e \"just sayin\u0027\" :)\n \u003e \n \u003e Yeah I know - thanks for being open to tolerating differences of\n \u003e opinion ;-)  And if you didn\u0027t already, please consider the various\n \u003e costs of long methods which I have mentioned several times before. \n \u003e I strongly believe it\u0027s way too easy to underestimate how damaging\n \u003e they are (and IMHO the vast majority of engineers *do*\n \u003e underestimate this).  If I was only allowed to give a single piece\n \u003e of advice to coders starting out their career (which obviously\n \u003e excludes you, since you\u0027re already extremely experienced) it would\n \u003e probably be \"write shorter functions/methods\".  Of course you\u0027re\n \u003e entitled to your own opinion.  FWIW mine is based on ~29 years of\n \u003e professional coding experience - just sayin\u0027 ;-)\n\nUnderstood, but this feels like we\u0027re reducing the discussion down to \"small functions are good and big functions are bad\", which is of course a huge oversimplification. I\u0027ve clearly got many years less professional experience (a mere 6) but things like coupling vs. cohesion and cyclomatic complexity are fortunately taught from the get go and in my opinion splitting things up like we\u0027re doing here reduces the cohesiveness/readability of the function and does extremely little w.r.t. reducing something like cyclomatic complexity.\n\nPut it this way: if I were to insert a couple of functions between this function and the one above, such that they ended up on different pages, would you find this implementation as easy to grok as if they were merged? I don\u0027t think it would (otherwise I wouldn\u0027t ask the question :)) and, more importantly, I don\u0027t think the costs of merging things together _in this instance_ outweigh the impact on grokability that splitting them out does.\n\nAnd again, none of this matters since it\u0027s all just talk at this point.","commit_id":"3f53fed18665b6295d84d196ce681870617b87c5"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"56e3989c25eb781e1d54dd86e7225608fc88b87e","unresolved":false,"context_lines":[{"line_number":1218,"context_line":"    if image_meta.properties.hw_firmware_type \u003d\u003d \u0027uefi\u0027:"},{"line_number":1219,"context_line":"        return"},{"line_number":1220,"context_line":""},{"line_number":1221,"context_line":"    # Unfortunately there\u0027s no way to avoid this duplication :-("},{"line_number":1222,"context_line":"    emsg \u003d _("},{"line_number":1223,"context_line":"        \"Memory encryption requested by %(requesters)s but \""},{"line_number":1224,"context_line":"        \"image %(image_name)s doesn\u0027t have hw_firmware_type set to \u0027uefi\u0027\""}],"source_content_type":"text/x-python","patch_set":46,"id":"7faddb67_00ef5787","line":1221,"range":{"start_line":1221,"start_character":0,"end_line":1221,"end_character":64},"updated":"2019-08-30 09:51:37.000000000","message":"Don\u0027t think anything is duplicated now","commit_id":"3f53fed18665b6295d84d196ce681870617b87c5"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"73b3f273224446132f461a49039a83c617e93300","unresolved":false,"context_lines":[{"line_number":1218,"context_line":"    if image_meta.properties.hw_firmware_type \u003d\u003d \u0027uefi\u0027:"},{"line_number":1219,"context_line":"        return"},{"line_number":1220,"context_line":""},{"line_number":1221,"context_line":"    # Unfortunately there\u0027s no way to avoid this duplication :-("},{"line_number":1222,"context_line":"    emsg \u003d _("},{"line_number":1223,"context_line":"        \"Memory encryption requested by %(requesters)s but \""},{"line_number":1224,"context_line":"        \"image %(image_name)s doesn\u0027t have hw_firmware_type set to \u0027uefi\u0027\""}],"source_content_type":"text/x-python","patch_set":46,"id":"7faddb67_5143e3b0","line":1221,"range":{"start_line":1221,"start_character":0,"end_line":1221,"end_character":64},"in_reply_to":"7faddb67_00ef5787","updated":"2019-08-30 13:08:17.000000000","message":"Oops, good catch - fixed.","commit_id":"3f53fed18665b6295d84d196ce681870617b87c5"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"56e3989c25eb781e1d54dd86e7225608fc88b87e","unresolved":false,"context_lines":[{"line_number":1221,"context_line":"    # Unfortunately there\u0027s no way to avoid this duplication :-("},{"line_number":1222,"context_line":"    emsg \u003d _("},{"line_number":1223,"context_line":"        \"Memory encryption requested by %(requesters)s but \""},{"line_number":1224,"context_line":"        \"image %(image_name)s doesn\u0027t have hw_firmware_type set to \u0027uefi\u0027\""},{"line_number":1225,"context_line":"    )"},{"line_number":1226,"context_line":"    data \u003d {\u0027requesters\u0027: \" and \".join(requesters),"},{"line_number":1227,"context_line":"            \u0027image_name\u0027: image_meta.name}"}],"source_content_type":"text/x-python","patch_set":46,"id":"7faddb67_20f4d35a","line":1224,"range":{"start_line":1224,"start_character":43,"end_line":1224,"end_character":59},"updated":"2019-08-30 09:51:37.000000000","message":"\u0027hw_firmware_type\u0027 ?","commit_id":"3f53fed18665b6295d84d196ce681870617b87c5"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"73b3f273224446132f461a49039a83c617e93300","unresolved":false,"context_lines":[{"line_number":1221,"context_line":"    # Unfortunately there\u0027s no way to avoid this duplication :-("},{"line_number":1222,"context_line":"    emsg \u003d _("},{"line_number":1223,"context_line":"        \"Memory encryption requested by %(requesters)s but \""},{"line_number":1224,"context_line":"        \"image %(image_name)s doesn\u0027t have hw_firmware_type set to \u0027uefi\u0027\""},{"line_number":1225,"context_line":"    )"},{"line_number":1226,"context_line":"    data \u003d {\u0027requesters\u0027: \" and \".join(requesters),"},{"line_number":1227,"context_line":"            \u0027image_name\u0027: image_meta.name}"}],"source_content_type":"text/x-python","patch_set":46,"id":"7faddb67_d1b27396","line":1224,"range":{"start_line":1224,"start_character":43,"end_line":1224,"end_character":59},"in_reply_to":"7faddb67_20f4d35a","updated":"2019-08-30 13:08:17.000000000","message":"You mean add quotes?  OK.","commit_id":"3f53fed18665b6295d84d196ce681870617b87c5"}]}