)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":23512,"name":"Alexander Pugachev","email":"alexander.pugachev@workday.com","username":"alexander.pugachev"},"change_message_id":"1394b125dd070ec6d05f2ff337473a666794ff24","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"virt: add numa constraint for emulator threads policy"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This commits update the part responsible to build the NUMA topology"},{"line_number":10,"context_line":"constrainst to take into account the emulator threads request."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"implements blueprint libvirt-emulator-threads-policy"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"da6895a0_232f71f7","line":9,"range":{"start_line":9,"start_character":5,"end_line":9,"end_character":19},"updated":"2016-10-24 10:33:45.000000000","message":"commit updates","commit_id":"4259ff17241bb3e0993d4ed7cde331d3a090c85d"}],"nova/exception.py":[{"author":{"_account_id":23512,"name":"Alexander Pugachev","email":"alexander.pugachev@workday.com","username":"alexander.pugachev"},"change_message_id":"1394b125dd070ec6d05f2ff337473a666794ff24","unresolved":false,"context_lines":[{"line_number":2161,"context_line":""},{"line_number":2162,"context_line":""},{"line_number":2163,"context_line":"class InvalidEmulatorThreadsPolicy(Invalid):"},{"line_number":2164,"context_line":"    msg_fmt \u003d _(\"CPU emulator threads requested invalid. \""},{"line_number":2165,"context_line":"                \"given: \u0027%(requested)s\u0027, available: \u0027%(available)s\u0027\")"},{"line_number":2166,"context_line":""},{"line_number":2167,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"da6895a0_83dcbde5","line":2164,"range":{"start_line":2164,"start_character":38,"end_line":2164,"end_character":57},"updated":"2016-10-24 10:33:45.000000000","message":"invalid what?","commit_id":"4259ff17241bb3e0993d4ed7cde331d3a090c85d"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"9ca1f33ee760a9bd3622858ec9bfbdf059cc275e","unresolved":false,"context_lines":[{"line_number":2161,"context_line":""},{"line_number":2162,"context_line":""},{"line_number":2163,"context_line":"class InvalidEmulatorThreadsPolicy(Invalid):"},{"line_number":2164,"context_line":"    msg_fmt \u003d _(\"CPU emulator threads requested invalid. \""},{"line_number":2165,"context_line":"                \"given: \u0027%(requested)s\u0027, available: \u0027%(available)s\u0027\")"},{"line_number":2166,"context_line":""},{"line_number":2167,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"ba5da102_89ba3be0","line":2164,"in_reply_to":"da6895a0_83dcbde5","updated":"2016-10-27 20:58:05.000000000","message":"What do you mean? I guess it\u0027s not clear as it is. The point is to raise an exception if the flavor extra-spec is configured with something that is not \u0027isolate\u0027 or \u0027shared\u0027. Don\u0027t hesitate to share with me any phrasing to make it better.","commit_id":"4259ff17241bb3e0993d4ed7cde331d3a090c85d"},{"author":{"_account_id":23512,"name":"Alexander Pugachev","email":"alexander.pugachev@workday.com","username":"alexander.pugachev"},"change_message_id":"1394b125dd070ec6d05f2ff337473a666794ff24","unresolved":false,"context_lines":[{"line_number":2166,"context_line":""},{"line_number":2167,"context_line":""},{"line_number":2168,"context_line":"class BadRequirementEmulatorThreadsPolicy(Invalid):"},{"line_number":2169,"context_line":"    msg_fmt \u003d _(\"An isolate CPU emulator threads require a dedicated \""},{"line_number":2170,"context_line":"                \"CPU policy\u0027\")"}],"source_content_type":"text/x-python","patch_set":3,"id":"da6895a0_03c8ad21","line":2169,"range":{"start_line":2169,"start_character":17,"end_line":2169,"end_character":32},"updated":"2016-10-24 10:33:45.000000000","message":"\"Isolated CPU emulator threads\"?","commit_id":"4259ff17241bb3e0993d4ed7cde331d3a090c85d"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"8ec530e9be2749af0f6bf2a65867ce46ddbe33de","unresolved":false,"context_lines":[{"line_number":2179,"context_line":"                \"given: \u0027%(requested)s\u0027, available: \u0027%(available)s\u0027.\")"},{"line_number":2180,"context_line":""},{"line_number":2181,"context_line":""},{"line_number":2182,"context_line":"class BadRequirementEmulatorThreadsPolicy(Invalid):"},{"line_number":2183,"context_line":"    msg_fmt \u003d _(\"An isolated CPU emulator threads option requires a dedicated \""},{"line_number":2184,"context_line":"                \"CPU policy option.\")"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a74a57a_a2ce4958","line":2182,"range":{"start_line":2182,"start_character":0,"end_line":2182,"end_character":51},"updated":"2016-11-23 12:27:48.000000000","message":"could you make this a subclass of \u0027InvalidEmulatorThreadsPolicy\u0027 (or define a common base) so we only have to catch one type of exception?","commit_id":"c389a556bfa2a2de5956895b14ec87bea11cbb2d"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"cbe7e7c26375ce9723f7b402447851c8b1953237","unresolved":false,"context_lines":[{"line_number":2179,"context_line":"                \"given: \u0027%(requested)s\u0027, available: \u0027%(available)s\u0027.\")"},{"line_number":2180,"context_line":""},{"line_number":2181,"context_line":""},{"line_number":2182,"context_line":"class BadRequirementEmulatorThreadsPolicy(Invalid):"},{"line_number":2183,"context_line":"    msg_fmt \u003d _(\"An isolated CPU emulator threads option requires a dedicated \""},{"line_number":2184,"context_line":"                \"CPU policy option.\")"}],"source_content_type":"text/x-python","patch_set":5,"id":"3a71b18c_866a4c19","line":2182,"in_reply_to":"5a74a57a_6df33974","updated":"2016-12-01 12:55:12.000000000","message":"Good point - I was thinking this would happen at the API level but I need to investigate. Let me get back to you on this","commit_id":"c389a556bfa2a2de5956895b14ec87bea11cbb2d"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"3c88c6f7f221c280ff66a3c4f56b1dfe7a9d59b8","unresolved":false,"context_lines":[{"line_number":2179,"context_line":"                \"given: \u0027%(requested)s\u0027, available: \u0027%(available)s\u0027.\")"},{"line_number":2180,"context_line":""},{"line_number":2181,"context_line":""},{"line_number":2182,"context_line":"class BadRequirementEmulatorThreadsPolicy(Invalid):"},{"line_number":2183,"context_line":"    msg_fmt \u003d _(\"An isolated CPU emulator threads option requires a dedicated \""},{"line_number":2184,"context_line":"                \"CPU policy option.\")"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a74a57a_6df33974","line":2182,"in_reply_to":"5a74a57a_a2ce4958","updated":"2016-11-23 15:22:14.000000000","message":"Sure, but before to do that where do we need to catch both ? - there are already sharing the same base class which is Invalid.","commit_id":"c389a556bfa2a2de5956895b14ec87bea11cbb2d"}],"nova/tests/unit/virt/test_hardware.py":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"4acfedb77481b5e222e68e926fc0985175b4fd00","unresolved":false,"context_lines":[{"line_number":1217,"context_line":"                },"},{"line_number":1218,"context_line":"                \"expect\": exception.BadRequirementEmulatorThreadsPolicy,"},{"line_number":1219,"context_line":"            },"},{"line_number":1220,"context_line":"            {   # We request emulator threads option without numa topology"},{"line_number":1221,"context_line":"                \"flavor\": objects.Flavor(vcpus\u003d4, memory_mb\u003d2048,"},{"line_number":1222,"context_line":"                                         extra_specs\u003d{"},{"line_number":1223,"context_line":"                    \"hw:emulator_threads_policy\": \"isolate\","}],"source_content_type":"text/x-python","patch_set":16,"id":"ba5201f7_afc9927a","line":1220,"updated":"2017-01-03 14:54:13.000000000","message":"The above comment is the same as line 1210. Did you mean to keep them the same?","commit_id":"ad2a127d4c2bfc41d00b2009504ad95623180287"}],"nova/virt/hardware.py":[{"author":{"_account_id":23512,"name":"Alexander Pugachev","email":"alexander.pugachev@workday.com","username":"alexander.pugachev"},"change_message_id":"1394b125dd070ec6d05f2ff337473a666794ff24","unresolved":false,"context_lines":[{"line_number":1140,"context_line":"    emu_threads_policy \u003d flavor.get(\u0027extra_specs\u0027, {}).get("},{"line_number":1141,"context_line":"        \u0027hw:cpu_emulator_threads_policy\u0027, None)"},{"line_number":1142,"context_line":""},{"line_number":1143,"context_line":"    if not emu_threads_policy:"},{"line_number":1144,"context_line":"        return"},{"line_number":1145,"context_line":""},{"line_number":1146,"context_line":"    if (emu_threads_policy !\u003d fields.CPUEmulatorThreadsPolicy.SHARED"}],"source_content_type":"text/x-python","patch_set":3,"id":"da6895a0_23993120","line":1143,"range":{"start_line":1143,"start_character":7,"end_line":1143,"end_character":29},"updated":"2016-10-24 10:33:45.000000000","message":"maybe \"is None\" would be better?","commit_id":"4259ff17241bb3e0993d4ed7cde331d3a090c85d"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"9ca1f33ee760a9bd3622858ec9bfbdf059cc275e","unresolved":false,"context_lines":[{"line_number":1140,"context_line":"    emu_threads_policy \u003d flavor.get(\u0027extra_specs\u0027, {}).get("},{"line_number":1141,"context_line":"        \u0027hw:cpu_emulator_threads_policy\u0027, None)"},{"line_number":1142,"context_line":""},{"line_number":1143,"context_line":"    if not emu_threads_policy:"},{"line_number":1144,"context_line":"        return"},{"line_number":1145,"context_line":""},{"line_number":1146,"context_line":"    if (emu_threads_policy !\u003d fields.CPUEmulatorThreadsPolicy.SHARED"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba5da102_69c6c768","line":1143,"in_reply_to":"da6895a0_23993120","updated":"2016-10-27 20:58:05.000000000","message":"I can do that to be more specific, yes.","commit_id":"4259ff17241bb3e0993d4ed7cde331d3a090c85d"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"8ec530e9be2749af0f6bf2a65867ce46ddbe33de","unresolved":false,"context_lines":[{"line_number":1138,"context_line":""},{"line_number":1139,"context_line":"def get_emulator_threads_constraint_from_flavor(flavor):"},{"line_number":1140,"context_line":"    \"\"\"Determines from flavor the emulator threads policy\"\"\""},{"line_number":1141,"context_line":"    emu_threads_policy \u003d flavor.get(\u0027extra_specs\u0027, {}).get("},{"line_number":1142,"context_line":"        \u0027hw:cpu_emulator_threads_policy\u0027, None)"},{"line_number":1143,"context_line":""},{"line_number":1144,"context_line":"    if not emu_threads_policy:"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a74a57a_053707b1","line":1141,"range":{"start_line":1141,"start_character":3,"end_line":1141,"end_character":59},"updated":"2016-11-23 12:27:48.000000000","message":"Wait - do we not support image-based properties? We should, if for no other reason that to prevent users specifying this in flavors and causing hassle for operators. We support image properties for cpu_policy, cpu_thread_policy and mem_page_size","commit_id":"c389a556bfa2a2de5956895b14ec87bea11cbb2d"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"63bbf4ccd2b644ade07cf37adcc0aee2bbaf3728","unresolved":false,"context_lines":[{"line_number":1138,"context_line":""},{"line_number":1139,"context_line":"def get_emulator_threads_constraint_from_flavor(flavor):"},{"line_number":1140,"context_line":"    \"\"\"Determines from flavor the emulator threads policy\"\"\""},{"line_number":1141,"context_line":"    emu_threads_policy \u003d flavor.get(\u0027extra_specs\u0027, {}).get("},{"line_number":1142,"context_line":"        \u0027hw:cpu_emulator_threads_policy\u0027, None)"},{"line_number":1143,"context_line":""},{"line_number":1144,"context_line":"    if not emu_threads_policy:"}],"source_content_type":"text/x-python","patch_set":5,"id":"3a71b18c_e65215dd","line":1141,"in_reply_to":"3a71b18c_06d8fc52","updated":"2016-12-01 15:38:47.000000000","message":"It has been agreed that we do not want that property in the image meta data for the reason I have explained to you before. Also adding such requirement at this step would also means to think about a way to restrict the usage of this feature for usersby operators. \n\nIf you still think we should have it in image metadata I recommend you to push a spec to explain your use case.\n\nSo far It\u0027s not expected, not in the approved spec and it\u0027s going to add code complexity.","commit_id":"c389a556bfa2a2de5956895b14ec87bea11cbb2d"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"893033631d7e02da755d3751ea3122a7e862529d","unresolved":false,"context_lines":[{"line_number":1138,"context_line":""},{"line_number":1139,"context_line":"def get_emulator_threads_constraint_from_flavor(flavor):"},{"line_number":1140,"context_line":"    \"\"\"Determines from flavor the emulator threads policy\"\"\""},{"line_number":1141,"context_line":"    emu_threads_policy \u003d flavor.get(\u0027extra_specs\u0027, {}).get("},{"line_number":1142,"context_line":"        \u0027hw:cpu_emulator_threads_policy\u0027, None)"},{"line_number":1143,"context_line":""},{"line_number":1144,"context_line":"    if not emu_threads_policy:"}],"source_content_type":"text/x-python","patch_set":5,"id":"3a71b18c_dff11a6c","line":1141,"in_reply_to":"3a71b18c_92216950","updated":"2016-12-02 16:48:43.000000000","message":"- We do not want emulator_threads_policy in image metadata like we do not have realtime in image metadata. Both of these options are related to how the host is going to make running the VMs, they do not influence the guest operating system/the image. In other words an application running in the guest can\u0027t take part of these improvements.\n- We want \u0027realtime_mask\u0027 or \u0027cpu_pinning\u0027 in image metadata because such options are related to the guest, which vCPUs will run realtime, which vCPUs will run on a dedicated pCPUs. In other words, an application running in the guest can be pre-configured to run on those selected vCPUs.","commit_id":"c389a556bfa2a2de5956895b14ec87bea11cbb2d"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"03da58ab5b9eef65c0f4b0e36c08e1f33b78e998","unresolved":false,"context_lines":[{"line_number":1138,"context_line":""},{"line_number":1139,"context_line":"def get_emulator_threads_constraint_from_flavor(flavor):"},{"line_number":1140,"context_line":"    \"\"\"Determines from flavor the emulator threads policy\"\"\""},{"line_number":1141,"context_line":"    emu_threads_policy \u003d flavor.get(\u0027extra_specs\u0027, {}).get("},{"line_number":1142,"context_line":"        \u0027hw:cpu_emulator_threads_policy\u0027, None)"},{"line_number":1143,"context_line":""},{"line_number":1144,"context_line":"    if not emu_threads_policy:"}],"source_content_type":"text/x-python","patch_set":5,"id":"3a71b18c_2f67ddb7","line":1141,"in_reply_to":"3a71b18c_96b2fa04","updated":"2016-12-06 08:42:29.000000000","message":"We have cpu_pinning, since we have the numa placement over image meta data so when users are using dedicated cpu_policy that is exactly what we do, and so the users may build it\u0027s image based on the instance topology.","commit_id":"c389a556bfa2a2de5956895b14ec87bea11cbb2d"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"7a2cb786ebb2c6663cad0881185fb5ed40faf45f","unresolved":false,"context_lines":[{"line_number":1138,"context_line":""},{"line_number":1139,"context_line":"def get_emulator_threads_constraint_from_flavor(flavor):"},{"line_number":1140,"context_line":"    \"\"\"Determines from flavor the emulator threads policy\"\"\""},{"line_number":1141,"context_line":"    emu_threads_policy \u003d flavor.get(\u0027extra_specs\u0027, {}).get("},{"line_number":1142,"context_line":"        \u0027hw:cpu_emulator_threads_policy\u0027, None)"},{"line_number":1143,"context_line":""},{"line_number":1144,"context_line":"    if not emu_threads_policy:"}],"source_content_type":"text/x-python","patch_set":5,"id":"3a71b18c_96b2fa04","line":1141,"in_reply_to":"3a71b18c_dff11a6c","updated":"2016-12-05 10:26:34.000000000","message":"\u003e We want \u0027realtime_mask\u0027 or \u0027cpu_pinning\u0027 in image metadata because such options are related to the guest, which vCPUs will run realtime, which vCPUs will run on a dedicated pCPUs. In other words, an application running in the guest can be pre-configured to run on those selected vCPUs.\n\nWe don\u0027t have \u0027cpu_pinning\u0027, rather we have \u0027cpu_policy\u0027. \u0027cpu_policy\u0027 determines if the instance vCPUs are pinned to host pCPUs or not, but it doesn\u0027t change the amount of CPUs on the guest or offer a way to say \"which vCPUs will run on a dedicated pCPUs\". It modifies the behavior of the existing resources to better suit certain workloads, but from the purely functional guest perspective there is no difference. \u0027emulator_threads_policy\u0027 is the same thing, and therefore should be configured the same way.","commit_id":"c389a556bfa2a2de5956895b14ec87bea11cbb2d"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"a80ae7d89fc9f2a23b271acad9ed419c8aa93941","unresolved":false,"context_lines":[{"line_number":1138,"context_line":""},{"line_number":1139,"context_line":"def get_emulator_threads_constraint_from_flavor(flavor):"},{"line_number":1140,"context_line":"    \"\"\"Determines from flavor the emulator threads policy\"\"\""},{"line_number":1141,"context_line":"    emu_threads_policy \u003d flavor.get(\u0027extra_specs\u0027, {}).get("},{"line_number":1142,"context_line":"        \u0027hw:cpu_emulator_threads_policy\u0027, None)"},{"line_number":1143,"context_line":""},{"line_number":1144,"context_line":"    if not emu_threads_policy:"}],"source_content_type":"text/x-python","patch_set":5,"id":"3a71b18c_92216950","line":1141,"in_reply_to":"3a71b18c_e65215dd","updated":"2016-12-02 11:21:02.000000000","message":"\u003e It has been agreed that we do not want that property in the image meta data for the reason I have explained to you before. \n\nYes, but your reason was that this is not related to the guest itself. I don\u0027t think that\u0027s a valid argument because none of the image metadata properties do this. Rather they change how libvirt or other hypervisors schedule said guest. For example, \u0027cpu_policy\u0027 dictates that the CPUs of the guest should be pinned to the host CPU. This won\u0027t actually change anything inside the guest itself, but it will change the characteristics of the guest\u0027s performance. IMO, emulator threads policy is the exact same thing.\n\n \u003e Also adding such requirement at this step would also means to think about a way to restrict the usage of this feature for usersby operators.\n\nSo for all the other NFV properties, we do things this exact same thing and have no issues. Providing image metadata properties actually provides operators with more control, because if both image metadata and flavor extra specs are set, they must be the same or else we raise an exception. This means operators of a cloud that allows configurable flavors can set image metadata properties to prevent users specifying this property in their flavor.\n\n \u003e If you still think we should have it in image metadata I recommend you to push a spec to explain your use case.\n\nIf I understand correctly, specs aren\u0027t to be seen as \"the gold standard\" for how something should be done and don\u0027t have to be perfect (that was jaypipes argument at the Barcelona summit, anyway). I think this is something that was missed during the spec review and is fair game to discuss now. However, I will push an amendment to the spec as it stands on the assumption that I\u0027m correct and this needs to be done. It can be abandoned in that\u0027s not the case","commit_id":"c389a556bfa2a2de5956895b14ec87bea11cbb2d"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"bb274802a14717e2f50cb4df3df9b76f10b4b377","unresolved":false,"context_lines":[{"line_number":1138,"context_line":""},{"line_number":1139,"context_line":"def get_emulator_threads_constraint_from_flavor(flavor):"},{"line_number":1140,"context_line":"    \"\"\"Determines from flavor the emulator threads policy\"\"\""},{"line_number":1141,"context_line":"    emu_threads_policy \u003d flavor.get(\u0027extra_specs\u0027, {}).get("},{"line_number":1142,"context_line":"        \u0027hw:cpu_emulator_threads_policy\u0027, None)"},{"line_number":1143,"context_line":""},{"line_number":1144,"context_line":"    if not emu_threads_policy:"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a74a57a_cd728d7c","line":1141,"in_reply_to":"5a74a57a_053707b1","updated":"2016-11-23 15:19:52.000000000","message":"There are no reason to support this feature in the image-based properties... It\u0027s clearly not related to the image running the guest where cpu_policy or cpu_thread_policy could be directly related.","commit_id":"c389a556bfa2a2de5956895b14ec87bea11cbb2d"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"8d3d97eeedef98901ab363b7e1c12ec27af575cc","unresolved":false,"context_lines":[{"line_number":1138,"context_line":""},{"line_number":1139,"context_line":"def get_emulator_threads_constraint_from_flavor(flavor):"},{"line_number":1140,"context_line":"    \"\"\"Determines from flavor the emulator threads policy\"\"\""},{"line_number":1141,"context_line":"    emu_threads_policy \u003d flavor.get(\u0027extra_specs\u0027, {}).get("},{"line_number":1142,"context_line":"        \u0027hw:cpu_emulator_threads_policy\u0027, None)"},{"line_number":1143,"context_line":""},{"line_number":1144,"context_line":"    if not emu_threads_policy:"}],"source_content_type":"text/x-python","patch_set":5,"id":"3a71b18c_06d8fc52","line":1141,"in_reply_to":"5a74a57a_cd728d7c","updated":"2016-12-01 12:54:05.000000000","message":"But I think there are. Image properties provide a way to distribute images that are preconfigured with certain properties (i.e. regardless of the flavors needed). The point of this option, if I understand it correctly, is to allow operators to say \"this application is real time/performance sensitive and the emulator threads need to stay the heck away from my instance vCPU\u0027s threads\" :) If this is the case, I don\u0027t see why we wouldn\u0027t want to provide an image meta data property such that folks could package an application with this property. This would allow those operators to ensure such constraints will always be applied regardless of flavor used. This is the exact reason we have \u0027cpu_policy\u0027, \u0027cpu_thread_policy\u0027, \u0027os_require_quiesce\u0027 and all the other fields in nova/objects/image_meta.py:ImageMetaProps.","commit_id":"c389a556bfa2a2de5956895b14ec87bea11cbb2d"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"8ec530e9be2749af0f6bf2a65867ce46ddbe33de","unresolved":false,"context_lines":[{"line_number":1139,"context_line":"def get_emulator_threads_constraint_from_flavor(flavor):"},{"line_number":1140,"context_line":"    \"\"\"Determines from flavor the emulator threads policy\"\"\""},{"line_number":1141,"context_line":"    emu_threads_policy \u003d flavor.get(\u0027extra_specs\u0027, {}).get("},{"line_number":1142,"context_line":"        \u0027hw:cpu_emulator_threads_policy\u0027, None)"},{"line_number":1143,"context_line":""},{"line_number":1144,"context_line":"    if not emu_threads_policy:"},{"line_number":1145,"context_line":"        return"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a74a57a_c5e7cf3e","line":1142,"range":{"start_line":1142,"start_character":40,"end_line":1142,"end_character":46},"updated":"2016-11-23 12:27:48.000000000","message":"You don\u0027t need this - it returns None by default","commit_id":"c389a556bfa2a2de5956895b14ec87bea11cbb2d"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"8ec530e9be2749af0f6bf2a65867ce46ddbe33de","unresolved":false,"context_lines":[{"line_number":1144,"context_line":"    if not emu_threads_policy:"},{"line_number":1145,"context_line":"        return"},{"line_number":1146,"context_line":""},{"line_number":1147,"context_line":"    if (emu_threads_policy !\u003d fields.CPUEmulatorThreadsPolicy.SHARED"},{"line_number":1148,"context_line":"        and emu_threads_policy !\u003d fields.CPUEmulatorThreadsPolicy.ISOLATE):"},{"line_number":1149,"context_line":"        raise exception.InvalidEmulatorThreadsPolicy("},{"line_number":1150,"context_line":"            requested\u003demu_threads_policy,"},{"line_number":1151,"context_line":"            available\u003dstr(fields.CPUEmulatorThreadsPolicy.ALL))"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a74a57a_05e9c71a","line":1148,"range":{"start_line":1147,"start_character":0,"end_line":1148,"end_character":75},"updated":"2016-11-23 12:27:48.000000000","message":"Let\u0027s try\n\n  if policy not in [fields.CPUEmulatorThreadsPolicy.SHARED,\n                    fields.CPUEmulatorThreadsPolicy.ISOLATE]:\n\ninstead?","commit_id":"c389a556bfa2a2de5956895b14ec87bea11cbb2d"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"826941e71fdc4f9d88fe84b9ce097f0466280e0e","unresolved":false,"context_lines":[{"line_number":1153,"context_line":"    if emu_threads_policy \u003d\u003d fields.CPUEmulatorThreadsPolicy.ISOLATE:"},{"line_number":1154,"context_line":"        # In order to make available emulator threads policy, a dedicated"},{"line_number":1155,"context_line":"        # CPU policy is necessary."},{"line_number":1156,"context_line":"        cpu_policy \u003d flavor.get(\u0027extra_specs\u0027, {}).get(\u0027hw:cpu_policy\u0027)"},{"line_number":1157,"context_line":"        if cpu_policy !\u003d fields.CPUAllocationPolicy.DEDICATED:"},{"line_number":1158,"context_line":"            raise exception.BadRequirementEmulatorThreadsPolicy()"},{"line_number":1159,"context_line":""},{"line_number":1160,"context_line":"    return emu_threads_policy"}],"source_content_type":"text/x-python","patch_set":5,"id":"3a71b18c_e9fe7304","line":1157,"range":{"start_line":1156,"start_character":0,"end_line":1157,"end_character":62},"updated":"2016-12-01 13:00:33.000000000","message":"You don\u0027t check image metadata here.","commit_id":"c389a556bfa2a2de5956895b14ec87bea11cbb2d"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"8ec530e9be2749af0f6bf2a65867ce46ddbe33de","unresolved":false,"context_lines":[{"line_number":1151,"context_line":"            available\u003dstr(fields.CPUEmulatorThreadsPolicy.ALL))"},{"line_number":1152,"context_line":""},{"line_number":1153,"context_line":"    if emu_threads_policy \u003d\u003d fields.CPUEmulatorThreadsPolicy.ISOLATE:"},{"line_number":1154,"context_line":"        # In order to make available emulator threads policy, a dedicated"},{"line_number":1155,"context_line":"        # CPU policy is necessary."},{"line_number":1156,"context_line":"        cpu_policy \u003d flavor.get(\u0027extra_specs\u0027, {}).get(\u0027hw:cpu_policy\u0027)"},{"line_number":1157,"context_line":"        if cpu_policy !\u003d fields.CPUAllocationPolicy.DEDICATED:"},{"line_number":1158,"context_line":"            raise exception.BadRequirementEmulatorThreadsPolicy()"},{"line_number":1159,"context_line":""},{"line_number":1160,"context_line":"    return emu_threads_policy"},{"line_number":1161,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"5a74a57a_c5626fd8","line":1158,"range":{"start_line":1154,"start_character":0,"end_line":1158,"end_character":65},"updated":"2016-11-23 12:27:48.000000000","message":"I don\u0027t believe this should be done here - the function is supposed to extract \"emulator thread constraints\". You should have another function to do the validation.","commit_id":"c389a556bfa2a2de5956895b14ec87bea11cbb2d"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"03da58ab5b9eef65c0f4b0e36c08e1f33b78e998","unresolved":false,"context_lines":[{"line_number":1155,"context_line":"        # CPU policy is necessary."},{"line_number":1156,"context_line":"        cpu_policy \u003d flavor.get(\u0027extra_specs\u0027, {}).get(\u0027hw:cpu_policy\u0027)"},{"line_number":1157,"context_line":"        if cpu_policy !\u003d fields.CPUAllocationPolicy.DEDICATED:"},{"line_number":1158,"context_line":"            raise exception.BadRequirementEmulatorThreadsPolicy()"},{"line_number":1159,"context_line":""},{"line_number":1160,"context_line":"    return emu_threads_policy"},{"line_number":1161,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"3a71b18c_af284d34","line":1158,"in_reply_to":"3a71b18c_461114ac","updated":"2016-12-06 08:42:29.000000000","message":"I don\u0027t see why we should do that sorry. That method already have a too big complexity, my plan is to rework it anyway.","commit_id":"c389a556bfa2a2de5956895b14ec87bea11cbb2d"},{"author":{"_account_id":8802,"name":"Vladik Romanovsky","email":"vromanso@redhat.com","username":"vladikr"},"change_message_id":"954d570bf161c112eb3364ab84607c57e8927260","unresolved":false,"context_lines":[{"line_number":1155,"context_line":"        # CPU policy is necessary."},{"line_number":1156,"context_line":"        cpu_policy \u003d flavor.get(\u0027extra_specs\u0027, {}).get(\u0027hw:cpu_policy\u0027)"},{"line_number":1157,"context_line":"        if cpu_policy !\u003d fields.CPUAllocationPolicy.DEDICATED:"},{"line_number":1158,"context_line":"            raise exception.BadRequirementEmulatorThreadsPolicy()"},{"line_number":1159,"context_line":""},{"line_number":1160,"context_line":"    return emu_threads_policy"},{"line_number":1161,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"3a71b18c_9a5ce27e","line":1158,"in_reply_to":"3a71b18c_af284d34","updated":"2016-12-10 05:43:56.000000000","message":"I don\u0027t see what\u0027s wrong with raising the exception here. It seems straight forward to me to check a value in the same method where you have actually retrieved it.","commit_id":"c389a556bfa2a2de5956895b14ec87bea11cbb2d"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"8d3d97eeedef98901ab363b7e1c12ec27af575cc","unresolved":false,"context_lines":[{"line_number":1155,"context_line":"        # CPU policy is necessary."},{"line_number":1156,"context_line":"        cpu_policy \u003d flavor.get(\u0027extra_specs\u0027, {}).get(\u0027hw:cpu_policy\u0027)"},{"line_number":1157,"context_line":"        if cpu_policy !\u003d fields.CPUAllocationPolicy.DEDICATED:"},{"line_number":1158,"context_line":"            raise exception.BadRequirementEmulatorThreadsPolicy()"},{"line_number":1159,"context_line":""},{"line_number":1160,"context_line":"    return emu_threads_policy"},{"line_number":1161,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"3a71b18c_461114ac","line":1158,"in_reply_to":"5a74a57a_4ddd9d13","updated":"2016-12-01 12:54:05.000000000","message":"Yes, this is definitely necessary. However, my complaint is that this function should only return the emulator_thread_policy here, and any cpu_policy/emulator_thread_policy conflict check should be done separately. This is what we do for cpu_policy/cpu_thread_policy  [2] and cpu_policy/rt [2] mismatches, and we should do the same thing here.\n\n[1] https://github.com/openstack/nova/blob/e83a3572/nova/virt/hardware.py#L1278-L1281\n[2] https://github.com/openstack/nova/blob/e83a3572/nova/virt/hardware.py#L1272-L1273","commit_id":"c389a556bfa2a2de5956895b14ec87bea11cbb2d"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"bb274802a14717e2f50cb4df3df9b76f10b4b377","unresolved":false,"context_lines":[{"line_number":1155,"context_line":"        # CPU policy is necessary."},{"line_number":1156,"context_line":"        cpu_policy \u003d flavor.get(\u0027extra_specs\u0027, {}).get(\u0027hw:cpu_policy\u0027)"},{"line_number":1157,"context_line":"        if cpu_policy !\u003d fields.CPUAllocationPolicy.DEDICATED:"},{"line_number":1158,"context_line":"            raise exception.BadRequirementEmulatorThreadsPolicy()"},{"line_number":1159,"context_line":""},{"line_number":1160,"context_line":"    return emu_threads_policy"},{"line_number":1161,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"5a74a57a_4ddd9d13","line":1158,"in_reply_to":"5a74a57a_c5626fd8","updated":"2016-11-23 15:19:52.000000000","message":"That method is supposed to return emulator threads policy configured from the flavor, if the method can\u0027t do it because it\u0027s badly configured so that method is raising an exception. I don\u0027t see any problem.","commit_id":"c389a556bfa2a2de5956895b14ec87bea11cbb2d"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"f5fad102bc65e28c40672138a6f09d8d3c1c008d","unresolved":false,"context_lines":[{"line_number":1170,"context_line":"    return objects.InstanceNUMATopology(cells\u003dcells)"},{"line_number":1171,"context_line":""},{"line_number":1172,"context_line":""},{"line_number":1173,"context_line":"def get_emulator_threads_constraint_from_flavor(flavor):"},{"line_number":1174,"context_line":"    \"\"\"Determines from flavor the emulator threads policy\"\"\""},{"line_number":1175,"context_line":"    emu_threads_policy \u003d flavor.get(\u0027extra_specs\u0027, {}).get("},{"line_number":1176,"context_line":"        \u0027hw:emulator_threads_policy\u0027)"},{"line_number":1177,"context_line":""},{"line_number":1178,"context_line":"    if not emu_threads_policy:"},{"line_number":1179,"context_line":"        return"},{"line_number":1180,"context_line":""},{"line_number":1181,"context_line":"    if (emu_threads_policy !\u003d fields.CPUEmulatorThreadsPolicy.SHARED"},{"line_number":1182,"context_line":"        and emu_threads_policy !\u003d fields.CPUEmulatorThreadsPolicy.ISOLATE):"},{"line_number":1183,"context_line":"        raise exception.InvalidEmulatorThreadsPolicy("},{"line_number":1184,"context_line":"            requested\u003demu_threads_policy,"},{"line_number":1185,"context_line":"            available\u003dstr(fields.CPUEmulatorThreadsPolicy.ALL))"},{"line_number":1186,"context_line":""},{"line_number":1187,"context_line":"    if emu_threads_policy \u003d\u003d fields.CPUEmulatorThreadsPolicy.ISOLATE:"},{"line_number":1188,"context_line":"        # In order to make available emulator threads policy, a dedicated"},{"line_number":1189,"context_line":"        # CPU policy is necessary."},{"line_number":1190,"context_line":"        cpu_policy \u003d flavor.get(\u0027extra_specs\u0027, {}).get(\u0027hw:cpu_policy\u0027)"},{"line_number":1191,"context_line":"        if cpu_policy !\u003d fields.CPUAllocationPolicy.DEDICATED:"},{"line_number":1192,"context_line":"            raise exception.BadRequirementEmulatorThreadsPolicy()"},{"line_number":1193,"context_line":""},{"line_number":1194,"context_line":"    return emu_threads_policy"},{"line_number":1195,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"3a71b18c_06363c03","line":1192,"range":{"start_line":1173,"start_character":0,"end_line":1192,"end_character":65},"updated":"2016-12-01 13:00:21.000000000","message":"Same three comments as PS5:\n\n- There should be an image metadata value\n- Use of \u0027x in [list]\u0027 is more readable\n- You shouldn\u0027t do cross-validation of different properties here","commit_id":"105011b1cbf6240c419677d5e0b090bb7fbe2049"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"0184a28b2b64593da67bd68f414b2813ff084db7","unresolved":false,"context_lines":[{"line_number":1189,"context_line":"        # CPU policy is necessary."},{"line_number":1190,"context_line":"        cpu_policy \u003d flavor.get(\u0027extra_specs\u0027, {}).get(\u0027hw:cpu_policy\u0027)"},{"line_number":1191,"context_line":"        if cpu_policy !\u003d fields.CPUAllocationPolicy.DEDICATED:"},{"line_number":1192,"context_line":"            raise exception.BadRequirementEmulatorThreadsPolicy()"},{"line_number":1193,"context_line":""},{"line_number":1194,"context_line":"    return emu_threads_policy"},{"line_number":1195,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"3a71b18c_61f8afe6","line":1192,"in_reply_to":"3a71b18c_06363c03","updated":"2016-12-01 15:55:02.000000000","message":"...\n\n  a) I explained to you we don\u0027t want that point.\n  b) why it\u0027s more readable ?\n  c) I explained to you on PS5 why it\u0027s like that.","commit_id":"105011b1cbf6240c419677d5e0b090bb7fbe2049"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"f5fad102bc65e28c40672138a6f09d8d3c1c008d","unresolved":false,"context_lines":[{"line_number":1194,"context_line":"    return emu_threads_policy"},{"line_number":1195,"context_line":""},{"line_number":1196,"context_line":""},{"line_number":1197,"context_line":"def _add_emulator_threads_constraint(flavor, numa_topology):"},{"line_number":1198,"context_line":"    emu_thread_policy \u003d get_emulator_threads_constraint_from_flavor(flavor)"},{"line_number":1199,"context_line":"    if emu_thread_policy and numa_topology:"},{"line_number":1200,"context_line":"        numa_topology.emulator_threads_policy \u003d emu_thread_policy"},{"line_number":1201,"context_line":"    return numa_topology"},{"line_number":1202,"context_line":""},{"line_number":1203,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"3a71b18c_86740cd0","line":1200,"range":{"start_line":1197,"start_character":0,"end_line":1200,"end_character":65},"updated":"2016-12-01 13:00:21.000000000","message":"This seems to be the same thing as \u0027_add_cpu_pinning_constraint\u0027, which we only just got rid of in \u002758db7d9c9\u0027. In addition to the comments on PS5, could you try to fold this into \u0027numa_get_constraints\u0027?","commit_id":"105011b1cbf6240c419677d5e0b090bb7fbe2049"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"0184a28b2b64593da67bd68f414b2813ff084db7","unresolved":false,"context_lines":[{"line_number":1197,"context_line":"def _add_emulator_threads_constraint(flavor, numa_topology):"},{"line_number":1198,"context_line":"    emu_thread_policy \u003d get_emulator_threads_constraint_from_flavor(flavor)"},{"line_number":1199,"context_line":"    if emu_thread_policy and numa_topology:"},{"line_number":1200,"context_line":"        numa_topology.emulator_threads_policy \u003d emu_thread_policy"},{"line_number":1201,"context_line":"    return numa_topology"},{"line_number":1202,"context_line":""},{"line_number":1203,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"3a71b18c_615d0fe9","line":1200,"in_reply_to":"3a71b18c_86740cd0","updated":"2016-12-01 15:55:02.000000000","message":"No, I don\u0027t think it make sens to add complexity in numa_get_constraints. Also it helps to have a small unit for the unit-tests.","commit_id":"105011b1cbf6240c419677d5e0b090bb7fbe2049"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"4acfedb77481b5e222e68e926fc0985175b4fd00","unresolved":false,"context_lines":[{"line_number":1180,"context_line":"        return"},{"line_number":1181,"context_line":""},{"line_number":1182,"context_line":"    if (emu_threads_policy not in [fields.CPUEmulatorThreadsPolicy.SHARE,"},{"line_number":1183,"context_line":"                                   fields.CPUEmulatorThreadsPolicy.ISOLATE]):"},{"line_number":1184,"context_line":"        raise exception.InvalidEmulatorThreadsPolicy("},{"line_number":1185,"context_line":"            requested\u003demu_threads_policy,"},{"line_number":1186,"context_line":"            available\u003dstr(fields.CPUEmulatorThreadsPolicy.ALL))"}],"source_content_type":"text/x-python","patch_set":16,"id":"ba5201f7_af77d238","line":1183,"updated":"2017-01-03 14:54:13.000000000","message":"Could future-proof the above code by just doing:\n\n if emu_threads_policy not in fields.CPUEmulatorThreadsPolicy.ALL:","commit_id":"ad2a127d4c2bfc41d00b2009504ad95623180287"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"47619bee7666a558dcfc7e4ec0504d0147cf1ba6","unresolved":false,"context_lines":[{"line_number":1170,"context_line":"    return objects.InstanceNUMATopology(cells\u003dcells)"},{"line_number":1171,"context_line":""},{"line_number":1172,"context_line":""},{"line_number":1173,"context_line":"def get_emulator_threads_constraint_from_flavor(flavor):"},{"line_number":1174,"context_line":"    \"\"\"Determines from flavor the emulator threads policy\"\"\""},{"line_number":1175,"context_line":"    emu_threads_policy \u003d flavor.get(\u0027extra_specs\u0027, {}).get("},{"line_number":1176,"context_line":"        \u0027hw:emulator_threads_policy\u0027)"}],"source_content_type":"text/x-python","patch_set":19,"id":"ba5201f7_c268a48a","line":1173,"range":{"start_line":1173,"start_character":0,"end_line":1173,"end_character":56},"updated":"2017-01-09 15:28:56.000000000","message":"Could this be private?","commit_id":"4e1a723488b2834e140c39b52ba2a120a98208ec"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"47619bee7666a558dcfc7e4ec0504d0147cf1ba6","unresolved":false,"context_lines":[{"line_number":1170,"context_line":"    return objects.InstanceNUMATopology(cells\u003dcells)"},{"line_number":1171,"context_line":""},{"line_number":1172,"context_line":""},{"line_number":1173,"context_line":"def get_emulator_threads_constraint_from_flavor(flavor):"},{"line_number":1174,"context_line":"    \"\"\"Determines from flavor the emulator threads policy\"\"\""},{"line_number":1175,"context_line":"    emu_threads_policy \u003d flavor.get(\u0027extra_specs\u0027, {}).get("},{"line_number":1176,"context_line":"        \u0027hw:emulator_threads_policy\u0027)"}],"source_content_type":"text/x-python","patch_set":19,"id":"ba5201f7_826e9c94","line":1173,"range":{"start_line":1173,"start_character":35,"end_line":1173,"end_character":47},"updated":"2017-01-09 15:28:56.000000000","message":"This seems superfluous","commit_id":"4e1a723488b2834e140c39b52ba2a120a98208ec"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"37e47d00c24740660afc94e3033d958fdbc5e64a","unresolved":false,"context_lines":[{"line_number":1170,"context_line":"    return objects.InstanceNUMATopology(cells\u003dcells)"},{"line_number":1171,"context_line":""},{"line_number":1172,"context_line":""},{"line_number":1173,"context_line":"def get_emulator_threads_constraint_from_flavor(flavor):"},{"line_number":1174,"context_line":"    \"\"\"Determines from flavor the emulator threads policy\"\"\""},{"line_number":1175,"context_line":"    emu_threads_policy \u003d flavor.get(\u0027extra_specs\u0027, {}).get("},{"line_number":1176,"context_line":"        \u0027hw:emulator_threads_policy\u0027)"}],"source_content_type":"text/x-python","patch_set":19,"id":"ba5201f7_24a07c90","line":1173,"in_reply_to":"ba5201f7_c268a48a","updated":"2017-01-10 10:44:17.000000000","message":"I don\u0027t think so, It\u0027s public, used by the driver layers","commit_id":"4e1a723488b2834e140c39b52ba2a120a98208ec"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"47619bee7666a558dcfc7e4ec0504d0147cf1ba6","unresolved":false,"context_lines":[{"line_number":1184,"context_line":"            requested\u003demu_threads_policy,"},{"line_number":1185,"context_line":"            available\u003dstr(fields.CPUEmulatorThreadsPolicy.ALL))"},{"line_number":1186,"context_line":""},{"line_number":1187,"context_line":"    if emu_threads_policy \u003d\u003d fields.CPUEmulatorThreadsPolicy.ISOLATE:"},{"line_number":1188,"context_line":"        # In order to make available emulator threads policy, a dedicated"},{"line_number":1189,"context_line":"        # CPU policy is necessary."},{"line_number":1190,"context_line":"        cpu_policy \u003d flavor.get(\u0027extra_specs\u0027, {}).get(\u0027hw:cpu_policy\u0027)"},{"line_number":1191,"context_line":"        if cpu_policy !\u003d fields.CPUAllocationPolicy.DEDICATED:"},{"line_number":1192,"context_line":"            raise exception.BadRequirementEmulatorThreadsPolicy()"},{"line_number":1193,"context_line":""},{"line_number":1194,"context_line":"    return emu_threads_policy"},{"line_number":1195,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"ba5201f7_c2ed04f1","line":1192,"range":{"start_line":1187,"start_character":0,"end_line":1192,"end_character":65},"updated":"2017-01-09 15:28:56.000000000","message":"This ignores CPU policies set via images. Could you move this out of the function and do the comparison in \u0027numa_get_constraints\u0027 or another function?","commit_id":"4e1a723488b2834e140c39b52ba2a120a98208ec"},{"author":{"_account_id":8802,"name":"Vladik Romanovsky","email":"vromanso@redhat.com","username":"vladikr"},"change_message_id":"2e283b65db3994fbf6a0ffdb0ad98e479d5cdd57","unresolved":false,"context_lines":[{"line_number":1184,"context_line":"            requested\u003demu_threads_policy,"},{"line_number":1185,"context_line":"            available\u003dstr(fields.CPUEmulatorThreadsPolicy.ALL))"},{"line_number":1186,"context_line":""},{"line_number":1187,"context_line":"    if emu_threads_policy \u003d\u003d fields.CPUEmulatorThreadsPolicy.ISOLATE:"},{"line_number":1188,"context_line":"        # In order to make available emulator threads policy, a dedicated"},{"line_number":1189,"context_line":"        # CPU policy is necessary."},{"line_number":1190,"context_line":"        cpu_policy \u003d flavor.get(\u0027extra_specs\u0027, {}).get(\u0027hw:cpu_policy\u0027)"},{"line_number":1191,"context_line":"        if cpu_policy !\u003d fields.CPUAllocationPolicy.DEDICATED:"},{"line_number":1192,"context_line":"            raise exception.BadRequirementEmulatorThreadsPolicy()"},{"line_number":1193,"context_line":""},{"line_number":1194,"context_line":"    return emu_threads_policy"},{"line_number":1195,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"ba5201f7_eece66b6","line":1192,"range":{"start_line":1187,"start_character":0,"end_line":1192,"end_character":65},"in_reply_to":"ba5201f7_c2ed04f1","updated":"2017-01-09 20:35:06.000000000","message":"I\u0027m probably missing something. get_emulator_threads_constraint_from_flavor is being called via  _add_emulator_threads_constraint in the of numa_get_constraints. Why would the CPU policies be ignored?","commit_id":"4e1a723488b2834e140c39b52ba2a120a98208ec"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"37e47d00c24740660afc94e3033d958fdbc5e64a","unresolved":false,"context_lines":[{"line_number":1189,"context_line":"        # CPU policy is necessary."},{"line_number":1190,"context_line":"        cpu_policy \u003d flavor.get(\u0027extra_specs\u0027, {}).get(\u0027hw:cpu_policy\u0027)"},{"line_number":1191,"context_line":"        if cpu_policy !\u003d fields.CPUAllocationPolicy.DEDICATED:"},{"line_number":1192,"context_line":"            raise exception.BadRequirementEmulatorThreadsPolicy()"},{"line_number":1193,"context_line":""},{"line_number":1194,"context_line":"    return emu_threads_policy"},{"line_number":1195,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"ba5201f7_5b2556ac","line":1192,"in_reply_to":"ba5201f7_eece66b6","updated":"2017-01-10 10:44:17.000000000","message":"Actually I think it\u0027s good point I should have also checked in image_meta for the cpu policy","commit_id":"4e1a723488b2834e140c39b52ba2a120a98208ec"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"47619bee7666a558dcfc7e4ec0504d0147cf1ba6","unresolved":false,"context_lines":[{"line_number":1316,"context_line":"    if cpu_policy \u003d\u003d fields.CPUAllocationPolicy.SHARED:"},{"line_number":1317,"context_line":"        if cpu_thread_policy:"},{"line_number":1318,"context_line":"            raise exception.CPUThreadPolicyConfigurationInvalid()"},{"line_number":1319,"context_line":"    else:"},{"line_number":1320,"context_line":"        if numa_topology:"},{"line_number":1321,"context_line":"            for cell in numa_topology.cells:"},{"line_number":1322,"context_line":"                cell.cpu_policy \u003d cpu_policy"},{"line_number":1323,"context_line":"                cell.cpu_thread_policy \u003d cpu_thread_policy"},{"line_number":1324,"context_line":"        else:"},{"line_number":1325,"context_line":"            single_cell \u003d objects.InstanceNUMACell("},{"line_number":1326,"context_line":"                id\u003d0,"},{"line_number":1327,"context_line":"                cpuset\u003dset(range(flavor.vcpus)),"},{"line_number":1328,"context_line":"                memory\u003dflavor.memory_mb,"},{"line_number":1329,"context_line":"                cpu_policy\u003dcpu_policy,"},{"line_number":1330,"context_line":"                cpu_thread_policy\u003dcpu_thread_policy)"},{"line_number":1331,"context_line":"            numa_topology \u003d objects.InstanceNUMATopology(cells\u003d[single_cell])"},{"line_number":1332,"context_line":""},{"line_number":1333,"context_line":"    numa_topology \u003d _add_emulator_threads_constraint(flavor, numa_topology)"},{"line_number":1334,"context_line":"    return numa_topology"}],"source_content_type":"text/x-python","patch_set":19,"id":"ba5201f7_a2b8002c","line":1331,"range":{"start_line":1319,"start_character":0,"end_line":1331,"end_character":77},"updated":"2017-01-09 15:28:56.000000000","message":"unrelated change?","commit_id":"4e1a723488b2834e140c39b52ba2a120a98208ec"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"37e47d00c24740660afc94e3033d958fdbc5e64a","unresolved":false,"context_lines":[{"line_number":1328,"context_line":"                memory\u003dflavor.memory_mb,"},{"line_number":1329,"context_line":"                cpu_policy\u003dcpu_policy,"},{"line_number":1330,"context_line":"                cpu_thread_policy\u003dcpu_thread_policy)"},{"line_number":1331,"context_line":"            numa_topology \u003d objects.InstanceNUMATopology(cells\u003d[single_cell])"},{"line_number":1332,"context_line":""},{"line_number":1333,"context_line":"    numa_topology \u003d _add_emulator_threads_constraint(flavor, numa_topology)"},{"line_number":1334,"context_line":"    return numa_topology"}],"source_content_type":"text/x-python","patch_set":19,"id":"ba5201f7_28812a90","line":1331,"in_reply_to":"ba5201f7_a2b8002c","updated":"2017-01-10 10:44:17.000000000","message":"No it\u0027s not an unrelated change","commit_id":"4e1a723488b2834e140c39b52ba2a120a98208ec"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"47619bee7666a558dcfc7e4ec0504d0147cf1ba6","unresolved":false,"context_lines":[{"line_number":1330,"context_line":"                cpu_thread_policy\u003dcpu_thread_policy)"},{"line_number":1331,"context_line":"            numa_topology \u003d objects.InstanceNUMATopology(cells\u003d[single_cell])"},{"line_number":1332,"context_line":""},{"line_number":1333,"context_line":"    numa_topology \u003d _add_emulator_threads_constraint(flavor, numa_topology)"},{"line_number":1334,"context_line":"    return numa_topology"},{"line_number":1335,"context_line":""},{"line_number":1336,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"ba5201f7_42a914df","line":1333,"range":{"start_line":1333,"start_character":4,"end_line":1333,"end_character":75},"updated":"2017-01-09 15:28:56.000000000","message":"Could we fold the four lines in this function in here?","commit_id":"4e1a723488b2834e140c39b52ba2a120a98208ec"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"37e47d00c24740660afc94e3033d958fdbc5e64a","unresolved":false,"context_lines":[{"line_number":1330,"context_line":"                cpu_thread_policy\u003dcpu_thread_policy)"},{"line_number":1331,"context_line":"            numa_topology \u003d objects.InstanceNUMATopology(cells\u003d[single_cell])"},{"line_number":1332,"context_line":""},{"line_number":1333,"context_line":"    numa_topology \u003d _add_emulator_threads_constraint(flavor, numa_topology)"},{"line_number":1334,"context_line":"    return numa_topology"},{"line_number":1335,"context_line":""},{"line_number":1336,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"ba5201f7_48558e1b","line":1333,"in_reply_to":"ba5201f7_42a914df","updated":"2017-01-10 10:44:17.000000000","message":"I don\u0027t think that is a good idea sorry","commit_id":"4e1a723488b2834e140c39b52ba2a120a98208ec"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"47619bee7666a558dcfc7e4ec0504d0147cf1ba6","unresolved":false,"context_lines":[{"line_number":1367,"context_line":"                   \u0027actual\u0027: len(host_topology)})"},{"line_number":1368,"context_line":"        return"},{"line_number":1369,"context_line":""},{"line_number":1370,"context_line":"    emulator_threads_policy \u003d None"},{"line_number":1371,"context_line":"    if \u0027emulator_threads_policy\u0027 in instance_topology:"},{"line_number":1372,"context_line":"        emulator_threads_policy \u003d instance_topology.emulator_threads_policy"},{"line_number":1373,"context_line":""},{"line_number":1374,"context_line":"    # TODO(ndipanov): We may want to sort permutations differently"},{"line_number":1375,"context_line":"    # depending on whether we want packing/spreading over NUMA nodes"}],"source_content_type":"text/x-python","patch_set":19,"id":"ba5201f7_02ed4c25","line":1372,"range":{"start_line":1370,"start_character":0,"end_line":1372,"end_character":75},"updated":"2017-01-09 15:28:56.000000000","message":"Do we need to do any validation here?","commit_id":"4e1a723488b2834e140c39b52ba2a120a98208ec"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"37e47d00c24740660afc94e3033d958fdbc5e64a","unresolved":false,"context_lines":[{"line_number":1369,"context_line":""},{"line_number":1370,"context_line":"    emulator_threads_policy \u003d None"},{"line_number":1371,"context_line":"    if \u0027emulator_threads_policy\u0027 in instance_topology:"},{"line_number":1372,"context_line":"        emulator_threads_policy \u003d instance_topology.emulator_threads_policy"},{"line_number":1373,"context_line":""},{"line_number":1374,"context_line":"    # TODO(ndipanov): We may want to sort permutations differently"},{"line_number":1375,"context_line":"    # depending on whether we want packing/spreading over NUMA nodes"}],"source_content_type":"text/x-python","patch_set":19,"id":"ba5201f7_6817b254","line":1372,"in_reply_to":"ba5201f7_02ed4c25","updated":"2017-01-10 10:44:17.000000000","message":"What kind of validation do you think we are missing?","commit_id":"4e1a723488b2834e140c39b52ba2a120a98208ec"},{"author":{"_account_id":21813,"name":"Andrey Volkov","email":"m@amadev.ru","username":"avolkov"},"change_message_id":"4d95d19bcfa9be1e06e9973a6d3ed97655d59c84","unresolved":false,"context_lines":[{"line_number":1170,"context_line":"    return objects.InstanceNUMATopology(cells\u003dcells)"},{"line_number":1171,"context_line":""},{"line_number":1172,"context_line":""},{"line_number":1173,"context_line":"def get_emulator_threads_constraint(flavor, image_meta):"},{"line_number":1174,"context_line":"    \"\"\"Determines the emulator threads policy\"\"\""},{"line_number":1175,"context_line":"    emu_threads_policy \u003d flavor.get(\u0027extra_specs\u0027, {}).get("},{"line_number":1176,"context_line":"        \u0027hw:emulator_threads_policy\u0027)"}],"source_content_type":"text/x-python","patch_set":20,"id":"ba5201f7_052b3295","line":1173,"updated":"2017-01-11 12:03:41.000000000","message":"This function has different than others naming.\nSee:\n   1056:def _get_cpu_policy_constraints(flavor, image_meta):\n   1075:def _get_cpu_thread_policy_constraints(flavor, image_meta):\n\nPossibly, _get_emulator_thread_policy_constraints?\n\nDo you really want it to be \"public\" especially it is called from \"private\" function?","commit_id":"77b251be9d17fecbdb7696523cbf649a5003862e"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"5d9e7da0380aff112ee96b47128baaafdcaf873b","unresolved":false,"context_lines":[{"line_number":1170,"context_line":"    return objects.InstanceNUMATopology(cells\u003dcells)"},{"line_number":1171,"context_line":""},{"line_number":1172,"context_line":""},{"line_number":1173,"context_line":"def get_emulator_threads_constraint(flavor, image_meta):"},{"line_number":1174,"context_line":"    \"\"\"Determines the emulator threads policy\"\"\""},{"line_number":1175,"context_line":"    emu_threads_policy \u003d flavor.get(\u0027extra_specs\u0027, {}).get("},{"line_number":1176,"context_line":"        \u0027hw:emulator_threads_policy\u0027)"}],"source_content_type":"text/x-python","patch_set":20,"id":"ba5201f7_71882772","line":1173,"in_reply_to":"ba5201f7_052b3295","updated":"2017-01-12 13:14:45.000000000","message":"I already replied to this question, please see previous comments","commit_id":"77b251be9d17fecbdb7696523cbf649a5003862e"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"4fa20bd8b890507fdbf01581c99eddd5699d12db","unresolved":false,"context_lines":[{"line_number":1315,"context_line":""},{"line_number":1316,"context_line":"    if cpu_policy \u003d\u003d fields.CPUAllocationPolicy.SHARED:"},{"line_number":1317,"context_line":"        if cpu_thread_policy:"},{"line_number":1318,"context_line":"            raise exception.CPUThreadPolicyConfigurationInvalid()"},{"line_number":1319,"context_line":"    else:"},{"line_number":1320,"context_line":"        if numa_topology:"},{"line_number":1321,"context_line":"            for cell in numa_topology.cells:"}],"source_content_type":"text/x-python","patch_set":20,"id":"ba5201f7_7935c2f5","line":1318,"updated":"2017-01-10 11:34:24.000000000","message":"The early return here seemed like the correct approach - it\u0027s not possible to add emulator thread constraints without a NUMA topology, right?","commit_id":"77b251be9d17fecbdb7696523cbf649a5003862e"},{"author":{"_account_id":21813,"name":"Andrey Volkov","email":"m@amadev.ru","username":"avolkov"},"change_message_id":"4d95d19bcfa9be1e06e9973a6d3ed97655d59c84","unresolved":false,"context_lines":[{"line_number":1317,"context_line":"        if cpu_thread_policy:"},{"line_number":1318,"context_line":"            raise exception.CPUThreadPolicyConfigurationInvalid()"},{"line_number":1319,"context_line":"    else:"},{"line_number":1320,"context_line":"        if numa_topology:"},{"line_number":1321,"context_line":"            for cell in numa_topology.cells:"},{"line_number":1322,"context_line":"                cell.cpu_policy \u003d cpu_policy"},{"line_number":1323,"context_line":"                cell.cpu_thread_policy \u003d cpu_thread_policy"}],"source_content_type":"text/x-python","patch_set":20,"id":"ba5201f7_b041c23b","line":1320,"updated":"2017-01-11 12:03:41.000000000","message":"This still can be done without else.\nSomething like:\n\nemu_thread_policy \u003d get_emulator_threads_constraint(flavor, image_meta)\n...\n    if numa_topology:\n        for cell in numa_topology.cells:\n            cell.cpu_policy \u003d cpu_policy\n            cell.cpu_thread_policy \u003d cpu_thread_policy\n    else:\n        single_cell \u003d objects.InstanceNUMACell(\n            id\u003d0,\n            cpuset\u003dset(range(flavor.vcpus)),\n            memory\u003dflavor.memory_mb,\n            cpu_policy\u003dcpu_policy,\n            cpu_thread_policy\u003dcpu_thread_policy)\n        numa_topology \u003d objects.InstanceNUMATopology(cells\u003d[single_cell])\n    if emu_thread_policy:\n        numa_topology.emulator_threads_policy \u003d emu_thread_policy\n\nSee:\nhttps://github.com/amadev/nova/commit/5790cd4261c09d20e1983385e6cbfc778c607c14","commit_id":"77b251be9d17fecbdb7696523cbf649a5003862e"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"5d9e7da0380aff112ee96b47128baaafdcaf873b","unresolved":false,"context_lines":[{"line_number":1317,"context_line":"        if cpu_thread_policy:"},{"line_number":1318,"context_line":"            raise exception.CPUThreadPolicyConfigurationInvalid()"},{"line_number":1319,"context_line":"    else:"},{"line_number":1320,"context_line":"        if numa_topology:"},{"line_number":1321,"context_line":"            for cell in numa_topology.cells:"},{"line_number":1322,"context_line":"                cell.cpu_policy \u003d cpu_policy"},{"line_number":1323,"context_line":"                cell.cpu_thread_policy \u003d cpu_thread_policy"}],"source_content_type":"text/x-python","patch_set":20,"id":"9a57fde8_dda5e6b5","line":1320,"in_reply_to":"ba5201f7_5a318559","updated":"2017-01-12 13:14:45.000000000","message":"You are removing a \u0027else\u0027 but add a \u0027if\u0027 ...\n\nI prefer my solution: The if..else are related to a context which is the cpu_*_policy.\n\nIn future I would like to see something like:\n\n  numa_topology \u003d _add_cpu_policy_constaint(...)\n  numa_topology \u003d _add_cpu_thread_policy_constaint(...)\n  numa_topology \u003d _add_emulator_threads_constraint(...)\n  numa_topology \u003d _add_my_feature_1_constraint(...)\n  ...\n\n  return numa_topology","commit_id":"77b251be9d17fecbdb7696523cbf649a5003862e"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"36f441128efcc67ee0201fd23dab1feabe760714","unresolved":false,"context_lines":[{"line_number":1317,"context_line":"        if cpu_thread_policy:"},{"line_number":1318,"context_line":"            raise exception.CPUThreadPolicyConfigurationInvalid()"},{"line_number":1319,"context_line":"    else:"},{"line_number":1320,"context_line":"        if numa_topology:"},{"line_number":1321,"context_line":"            for cell in numa_topology.cells:"},{"line_number":1322,"context_line":"                cell.cpu_policy \u003d cpu_policy"},{"line_number":1323,"context_line":"                cell.cpu_thread_policy \u003d cpu_thread_policy"}],"source_content_type":"text/x-python","patch_set":20,"id":"ba5201f7_5a318559","line":1320,"in_reply_to":"ba5201f7_b041c23b","updated":"2017-01-11 16:18:15.000000000","message":"+1 - flat is the new black, I hear ;)","commit_id":"77b251be9d17fecbdb7696523cbf649a5003862e"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"4fa20bd8b890507fdbf01581c99eddd5699d12db","unresolved":false,"context_lines":[{"line_number":1316,"context_line":"    if cpu_policy \u003d\u003d fields.CPUAllocationPolicy.SHARED:"},{"line_number":1317,"context_line":"        if cpu_thread_policy:"},{"line_number":1318,"context_line":"            raise exception.CPUThreadPolicyConfigurationInvalid()"},{"line_number":1319,"context_line":"    else:"},{"line_number":1320,"context_line":"        if numa_topology:"},{"line_number":1321,"context_line":"            for cell in numa_topology.cells:"},{"line_number":1322,"context_line":"                cell.cpu_policy \u003d cpu_policy"},{"line_number":1323,"context_line":"                cell.cpu_thread_policy \u003d cpu_thread_policy"},{"line_number":1324,"context_line":"        else:"},{"line_number":1325,"context_line":"            single_cell \u003d objects.InstanceNUMACell("},{"line_number":1326,"context_line":"                id\u003d0,"},{"line_number":1327,"context_line":"                cpuset\u003dset(range(flavor.vcpus)),"},{"line_number":1328,"context_line":"                memory\u003dflavor.memory_mb,"},{"line_number":1329,"context_line":"                cpu_policy\u003dcpu_policy,"},{"line_number":1330,"context_line":"                cpu_thread_policy\u003dcpu_thread_policy)"},{"line_number":1331,"context_line":"            numa_topology \u003d objects.InstanceNUMATopology(cells\u003d[single_cell])"},{"line_number":1332,"context_line":""},{"line_number":1333,"context_line":"    numa_topology \u003d _add_emulator_threads_constraint("},{"line_number":1334,"context_line":"        flavor, image_meta, numa_topology)"}],"source_content_type":"text/x-python","patch_set":20,"id":"ba5201f7_f97372e5","line":1331,"range":{"start_line":1319,"start_character":0,"end_line":1331,"end_character":77},"updated":"2017-01-10 11:34:24.000000000","message":"Per above, moving this into an else block isn\u0027t needed. Could you make this change separately, if you think it necessary.","commit_id":"77b251be9d17fecbdb7696523cbf649a5003862e"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"97a6350397bfb106d6f31188494795e29a56a9b0","unresolved":false,"context_lines":[{"line_number":1316,"context_line":"    if cpu_policy \u003d\u003d fields.CPUAllocationPolicy.SHARED:"},{"line_number":1317,"context_line":"        if cpu_thread_policy:"},{"line_number":1318,"context_line":"            raise exception.CPUThreadPolicyConfigurationInvalid()"},{"line_number":1319,"context_line":"    else:"},{"line_number":1320,"context_line":"        if numa_topology:"},{"line_number":1321,"context_line":"            for cell in numa_topology.cells:"},{"line_number":1322,"context_line":"                cell.cpu_policy \u003d cpu_policy"},{"line_number":1323,"context_line":"                cell.cpu_thread_policy \u003d cpu_thread_policy"},{"line_number":1324,"context_line":"        else:"},{"line_number":1325,"context_line":"            single_cell \u003d objects.InstanceNUMACell("},{"line_number":1326,"context_line":"                id\u003d0,"},{"line_number":1327,"context_line":"                cpuset\u003dset(range(flavor.vcpus)),"},{"line_number":1328,"context_line":"                memory\u003dflavor.memory_mb,"},{"line_number":1329,"context_line":"                cpu_policy\u003dcpu_policy,"},{"line_number":1330,"context_line":"                cpu_thread_policy\u003dcpu_thread_policy)"},{"line_number":1331,"context_line":"            numa_topology \u003d objects.InstanceNUMATopology(cells\u003d[single_cell])"},{"line_number":1332,"context_line":""},{"line_number":1333,"context_line":"    numa_topology \u003d _add_emulator_threads_constraint("},{"line_number":1334,"context_line":"        flavor, image_meta, numa_topology)"}],"source_content_type":"text/x-python","patch_set":20,"id":"ba5201f7_904937ba","line":1331,"range":{"start_line":1319,"start_character":0,"end_line":1331,"end_character":77},"in_reply_to":"ba5201f7_f97372e5","updated":"2017-01-10 23:19:38.000000000","message":"I thought the same thing. But then I figured out he put this here because the _add_emulator_threads_constraint() raises the new BadRequirementsEmulatorThreadsPolicy and InvalidEmulatorThreadsPolicy exceptions, even if no numa_topology is supplied. Returning numa_topology as was previously done on prior line 1288 would mean those exceptions would not be raised if no numa_topology is provided.","commit_id":"77b251be9d17fecbdb7696523cbf649a5003862e"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"5e9287b40f1323299a0da2366b5c7d09204c8abd","unresolved":false,"context_lines":[{"line_number":1330,"context_line":"                cpu_thread_policy\u003dcpu_thread_policy)"},{"line_number":1331,"context_line":"            numa_topology \u003d objects.InstanceNUMATopology(cells\u003d[single_cell])"},{"line_number":1332,"context_line":""},{"line_number":1333,"context_line":"    # TODO(sahid): This method needs to be reworked to align with"},{"line_number":1334,"context_line":"    # something like:"},{"line_number":1335,"context_line":"    #   numa_topology \u003d _add_cpu_policy_constaint(...)"},{"line_number":1336,"context_line":"    #   numa_topology \u003d _add_cpu_thread_policy_constaint(...)"},{"line_number":1337,"context_line":"    #   numa_topology \u003d _add_emulator_threads_constraint(...)"},{"line_number":1338,"context_line":"    #   numa_topology \u003d _add_my_feature_constraint(...)"},{"line_number":1339,"context_line":"    # So we keep one reference of numa topology and every features"},{"line_number":1340,"context_line":"    # update it to set their policies."},{"line_number":1341,"context_line":"    numa_topology \u003d _add_emulator_threads_constraint("},{"line_number":1342,"context_line":"        flavor, image_meta, numa_topology)"},{"line_number":1343,"context_line":"    return numa_topology"}],"source_content_type":"text/x-python","patch_set":25,"id":"5a3905b3_2462a4d8","line":1340,"range":{"start_line":1333,"start_character":0,"end_line":1340,"end_character":38},"updated":"2017-01-23 18:13:36.000000000","message":"I want to get this feature in, but I also don\u0027t like this combined validation-setting pattern and think we need to talk on it more. Can we please just do this later and stick with the existing style for this change?","commit_id":"0b8842719e1dc8fe7ff11d80cbedddc942a419bf"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"6ef037c89654cb7607db9220f6ea039483011395","unresolved":false,"context_lines":[{"line_number":1170,"context_line":"    return objects.InstanceNUMATopology(cells\u003dcells)"},{"line_number":1171,"context_line":""},{"line_number":1172,"context_line":""},{"line_number":1173,"context_line":"def get_emulator_threads_constraint(flavor, image_meta):"},{"line_number":1174,"context_line":"    \"\"\"Determines the emulator threads policy\"\"\""},{"line_number":1175,"context_line":"    emu_threads_policy \u003d flavor.get(\u0027extra_specs\u0027, {}).get("},{"line_number":1176,"context_line":"        \u0027hw:emulator_threads_policy\u0027)"}],"source_content_type":"text/x-python","patch_set":26,"id":"5a3905b3_0213980e","line":1173,"range":{"start_line":1173,"start_character":4,"end_line":1173,"end_character":35},"updated":"2017-01-24 19:43:05.000000000","message":"nit: this can (should?) be private","commit_id":"44eb1b64a06672908d29c0195aeefe253f477d77"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"62df5cfa18ac3bc0d546f925bf7587d5b3262aea","unresolved":false,"context_lines":[{"line_number":1170,"context_line":"    return objects.InstanceNUMATopology(cells\u003dcells)"},{"line_number":1171,"context_line":""},{"line_number":1172,"context_line":""},{"line_number":1173,"context_line":"def get_emulator_threads_constraint(flavor, image_meta):"},{"line_number":1174,"context_line":"    \"\"\"Determines the emulator threads policy\"\"\""},{"line_number":1175,"context_line":"    emu_threads_policy \u003d flavor.get(\u0027extra_specs\u0027, {}).get("},{"line_number":1176,"context_line":"        \u0027hw:emulator_threads_policy\u0027)"}],"source_content_type":"text/x-python","patch_set":26,"id":"5a3905b3_1ab920ed","line":1173,"range":{"start_line":1173,"start_character":4,"end_line":1173,"end_character":35},"in_reply_to":"5a3905b3_0213980e","updated":"2017-01-25 11:18:49.000000000","message":"+1","commit_id":"44eb1b64a06672908d29c0195aeefe253f477d77"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"79d21e9c9820fd8a5039ec64359bd7cb1fb5228f","unresolved":false,"context_lines":[{"line_number":1170,"context_line":"    return objects.InstanceNUMATopology(cells\u003dcells)"},{"line_number":1171,"context_line":""},{"line_number":1172,"context_line":""},{"line_number":1173,"context_line":"def get_emulator_threads_constraint(flavor, image_meta):"},{"line_number":1174,"context_line":"    \"\"\"Determines the emulator threads policy\"\"\""},{"line_number":1175,"context_line":"    emu_threads_policy \u003d flavor.get(\u0027extra_specs\u0027, {}).get("},{"line_number":1176,"context_line":"        \u0027hw:emulator_threads_policy\u0027)"}],"source_content_type":"text/x-python","patch_set":26,"id":"3a461143_3579b230","line":1173,"in_reply_to":"5a3905b3_1ab920ed","updated":"2017-01-25 14:12:11.000000000","message":"John I replied a bunch of time in this review about that, this method is not private because used [1], so far and because of the resource tracker we could have a reference of an instance or a flavor\u0027s ref passed.\n\n[1] https://review.openstack.org/#/c/385364/28/nova/virt/libvirt/driver.py","commit_id":"44eb1b64a06672908d29c0195aeefe253f477d77"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"6ef037c89654cb7607db9220f6ea039483011395","unresolved":false,"context_lines":[{"line_number":1184,"context_line":"            requested\u003demu_threads_policy,"},{"line_number":1185,"context_line":"            available\u003dstr(fields.CPUEmulatorThreadsPolicy.ALL))"},{"line_number":1186,"context_line":""},{"line_number":1187,"context_line":"    if emu_threads_policy \u003d\u003d fields.CPUEmulatorThreadsPolicy.ISOLATE:"},{"line_number":1188,"context_line":"        # In order to make available emulator threads policy, a dedicated"},{"line_number":1189,"context_line":"        # CPU policy is necessary."},{"line_number":1190,"context_line":"        cpu_policy \u003d _get_cpu_policy_constraints(flavor, image_meta)"},{"line_number":1191,"context_line":"        if cpu_policy !\u003d fields.CPUAllocationPolicy.DEDICATED:"},{"line_number":1192,"context_line":"            raise exception.BadRequirementEmulatorThreadsPolicy()"},{"line_number":1193,"context_line":""},{"line_number":1194,"context_line":"    return emu_threads_policy"},{"line_number":1195,"context_line":""}],"source_content_type":"text/x-python","patch_set":26,"id":"5a3905b3_a203a4bd","line":1192,"range":{"start_line":1187,"start_character":0,"end_line":1192,"end_character":65},"updated":"2017-01-24 19:43:05.000000000","message":"I still think this should go in numa_get_constraints, but we can meet half way","commit_id":"44eb1b64a06672908d29c0195aeefe253f477d77"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"62df5cfa18ac3bc0d546f925bf7587d5b3262aea","unresolved":false,"context_lines":[{"line_number":1184,"context_line":"            requested\u003demu_threads_policy,"},{"line_number":1185,"context_line":"            available\u003dstr(fields.CPUEmulatorThreadsPolicy.ALL))"},{"line_number":1186,"context_line":""},{"line_number":1187,"context_line":"    if emu_threads_policy \u003d\u003d fields.CPUEmulatorThreadsPolicy.ISOLATE:"},{"line_number":1188,"context_line":"        # In order to make available emulator threads policy, a dedicated"},{"line_number":1189,"context_line":"        # CPU policy is necessary."},{"line_number":1190,"context_line":"        cpu_policy \u003d _get_cpu_policy_constraints(flavor, image_meta)"},{"line_number":1191,"context_line":"        if cpu_policy !\u003d fields.CPUAllocationPolicy.DEDICATED:"},{"line_number":1192,"context_line":"            raise exception.BadRequirementEmulatorThreadsPolicy()"},{"line_number":1193,"context_line":""},{"line_number":1194,"context_line":"    return emu_threads_policy"},{"line_number":1195,"context_line":""}],"source_content_type":"text/x-python","patch_set":26,"id":"5a3905b3_da1f98b5","line":1192,"range":{"start_line":1187,"start_character":0,"end_line":1192,"end_character":65},"in_reply_to":"5a3905b3_a203a4bd","updated":"2017-01-25 11:18:49.000000000","message":"My preference would be to pass cpu_policy into this method, so we don\u0027t have to call _get_cpu_policy_constraints twice.\n\nWe should look at making these things more consistent ASAP.","commit_id":"44eb1b64a06672908d29c0195aeefe253f477d77"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"b6c11c76384d35037204735df1e3823b9f4a0900","unresolved":false,"context_lines":[{"line_number":1184,"context_line":"            requested\u003demu_threads_policy,"},{"line_number":1185,"context_line":"            available\u003dstr(fields.CPUEmulatorThreadsPolicy.ALL))"},{"line_number":1186,"context_line":""},{"line_number":1187,"context_line":"    if emu_threads_policy \u003d\u003d fields.CPUEmulatorThreadsPolicy.ISOLATE:"},{"line_number":1188,"context_line":"        # In order to make available emulator threads policy, a dedicated"},{"line_number":1189,"context_line":"        # CPU policy is necessary."},{"line_number":1190,"context_line":"        cpu_policy \u003d _get_cpu_policy_constraints(flavor, image_meta)"},{"line_number":1191,"context_line":"        if cpu_policy !\u003d fields.CPUAllocationPolicy.DEDICATED:"},{"line_number":1192,"context_line":"            raise exception.BadRequirementEmulatorThreadsPolicy()"},{"line_number":1193,"context_line":""},{"line_number":1194,"context_line":"    return emu_threads_policy"},{"line_number":1195,"context_line":""}],"source_content_type":"text/x-python","patch_set":26,"id":"5a3905b3_13c66b16","line":1192,"range":{"start_line":1187,"start_character":0,"end_line":1192,"end_character":65},"in_reply_to":"5a3905b3_da1f98b5","updated":"2017-01-25 12:28:16.000000000","message":"\u003e My preference would be to pass cpu_policy into this method, so we don\u0027t have to call _get_cpu_policy_constraints twice.\n\nPersonally, I think the extracting/validation of the parameter itself should be separate from handling conflicts with other parameters. However, I\u0027d held this up enough already that I was willing to overlook it here and fix it later.\n\n \u003e We should look at making these things more consistent ASAP.\n\nI have some of that work addressed already, I think?\n\nhttps://review.openstack.org/#/q/status:open+project:openstack/nova+branch:master+topic:feat/numa-refactor","commit_id":"44eb1b64a06672908d29c0195aeefe253f477d77"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"79d21e9c9820fd8a5039ec64359bd7cb1fb5228f","unresolved":false,"context_lines":[{"line_number":1189,"context_line":"        # CPU policy is necessary."},{"line_number":1190,"context_line":"        cpu_policy \u003d _get_cpu_policy_constraints(flavor, image_meta)"},{"line_number":1191,"context_line":"        if cpu_policy !\u003d fields.CPUAllocationPolicy.DEDICATED:"},{"line_number":1192,"context_line":"            raise exception.BadRequirementEmulatorThreadsPolicy()"},{"line_number":1193,"context_line":""},{"line_number":1194,"context_line":"    return emu_threads_policy"},{"line_number":1195,"context_line":""}],"source_content_type":"text/x-python","patch_set":26,"id":"3a461143_f5100aab","line":1192,"in_reply_to":"5a3905b3_da1f98b5","updated":"2017-01-25 14:12:11.000000000","message":"John I couldn\u0027t do that anyway because of what explained line 1173.\n\nThe good design for this method is purposed here but too much arguing so I finally reverted the work done [1]\n\n[1] https://review.openstack.org/#/c/385023/25/nova/virt/hardware.py,unified@1173","commit_id":"44eb1b64a06672908d29c0195aeefe253f477d77"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"1c9b7033c4e4d6d9319a613eee4e6ccef2db3557","unresolved":false,"context_lines":[{"line_number":1322,"context_line":"            cpuset\u003dset(range(flavor.vcpus)),"},{"line_number":1323,"context_line":"            memory\u003dflavor.memory_mb,"},{"line_number":1324,"context_line":"            cpu_policy\u003dcpu_policy,"},{"line_number":1325,"context_line":"            cpu_thread_policy\u003dcpu_thread_policy)"},{"line_number":1326,"context_line":"        numa_topology \u003d objects.InstanceNUMATopology(cells\u003d[single_cell])"},{"line_number":1327,"context_line":""},{"line_number":1328,"context_line":"    if emu_thread_policy:"}],"source_content_type":"text/x-python","patch_set":26,"id":"3a461143_9e6de59a","line":1325,"updated":"2017-01-26 17:20:54.000000000","message":"just a note for future, try to keep style-only changes like the above to separate patches, just to reduce reviewer cognitive load.","commit_id":"44eb1b64a06672908d29c0195aeefe253f477d77"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"17485aadbae521ba22221eb9c0156689973962d2","unresolved":false,"context_lines":[{"line_number":1202,"context_line":"    \"\"\"Determines the emulator threads policy\"\"\""},{"line_number":1203,"context_line":"    emu_threads_policy \u003d flavor.get(\u0027extra_specs\u0027, {}).get("},{"line_number":1204,"context_line":"        \u0027hw:emulator_threads_policy\u0027)"},{"line_number":1205,"context_line":"    LOG.debug(\"emulator threads policy constraint: %s\", emu_threads_policy)"},{"line_number":1206,"context_line":""},{"line_number":1207,"context_line":"    if not emu_threads_policy:"},{"line_number":1208,"context_line":"        return"}],"source_content_type":"text/x-python","patch_set":30,"id":"9a30ddce_b7246faf","line":1205,"updated":"2017-03-08 21:09:13.000000000","message":"Not going to block on this, but in a future patch, consider removing this debug log line or at the very least put it underneath line 1208.","commit_id":"338b3eaa08a10fe47666baf70009cbb76258e624"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"17485aadbae521ba22221eb9c0156689973962d2","unresolved":false,"context_lines":[{"line_number":1351,"context_line":"            cpuset\u003dset(range(flavor.vcpus)),"},{"line_number":1352,"context_line":"            memory\u003dflavor.memory_mb,"},{"line_number":1353,"context_line":"            cpu_policy\u003dcpu_policy,"},{"line_number":1354,"context_line":"            cpu_thread_policy\u003dcpu_thread_policy)"},{"line_number":1355,"context_line":"        numa_topology \u003d objects.InstanceNUMATopology(cells\u003d[single_cell])"},{"line_number":1356,"context_line":""},{"line_number":1357,"context_line":"    if emu_thread_policy:"}],"source_content_type":"text/x-python","patch_set":30,"id":"9a30ddce_37487f5b","line":1354,"updated":"2017-03-08 21:09:13.000000000","message":"As mentioned in previous comments, try to avoid style-only changes such as these because they add to the cognitive load of reviewers, especially ones that return to a patch after a long period of time ;)","commit_id":"338b3eaa08a10fe47666baf70009cbb76258e624"}]}