)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"16a2b3c75cd38dea1412f5d3d2251c29880d19b3","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Detect that SEV is required and enable iommu for devices"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"As was previously agreed, HW_CPU_AMD_SEV trait in flavor indicates that"},{"line_number":10,"context_line":"SEV must be used."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"When SEV is requested, set all virtio devices\u0027 iommu attribute to ``on``."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"9fb8cfa7_bce07598","line":9,"range":{"start_line":9,"start_character":26,"end_line":9,"end_character":40},"updated":"2019-06-01 00:13:41.000000000","message":"That was the agreement for the Stein spec, but it\u0027s no longer true.  Our plans changed in Train (and not just because we pseudo-renamed the trait to HW_CPU_X86_AMD_SEV).  All the details are in the Train spec.\n\nhttp://specs.openstack.org/openstack/nova-specs/specs/train/approved/amd-sev-libvirt-support.html","commit_id":"786e72d7eab10ed6046aa6a94a18a050eff1fd95"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"ef2c3b14bd3e3d1b46637a6456ceda48a9b611d8","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Detect that SEV is required and enable iommu for devices"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"As was previously agreed, HW_CPU_AMD_SEV trait in flavor indicates that"},{"line_number":10,"context_line":"SEV must be used."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"When SEV is requested, set all virtio devices\u0027 iommu attribute to ``on``."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"9fb8cfa7_b5b22748","line":9,"range":{"start_line":9,"start_character":26,"end_line":9,"end_character":40},"in_reply_to":"9fb8cfa7_bce07598","updated":"2019-06-12 11:35:17.000000000","message":"Done","commit_id":"786e72d7eab10ed6046aa6a94a18a050eff1fd95"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"e9d35b8774b034bf7a588d1e9275f3ebc27d1771","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Apply SEV-specific guest config when SEV is required"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Add a new _sev_required() method to the libvirt driver to detect that"},{"line_number":10,"context_line":"SEV is required and return True iff the following are both true:"},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"  a) the supports_amd_sev instance variable in the driver is"},{"line_number":13,"context_line":"     true, *and*"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":18,"id":"7faddb67_a4d2de26","line":10,"range":{"start_line":10,"start_character":32,"end_line":10,"end_character":35},"updated":"2019-07-17 01:22:44.000000000","message":"s/if","commit_id":"175f40b057c0c75f87f01ec30bfd084246039fa8"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"0250d2c5e797f9d3a8738217f6acd97b5358b839","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Apply SEV-specific guest config when SEV is required"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Add a new _sev_required() method to the libvirt driver to detect that"},{"line_number":10,"context_line":"SEV is required and return True iff the following are both true:"},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"  a) the supports_amd_sev instance variable in the driver is"},{"line_number":13,"context_line":"     true, *and*"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":18,"id":"7faddb67_52501a33","line":10,"range":{"start_line":10,"start_character":32,"end_line":10,"end_character":35},"in_reply_to":"7faddb67_68535418","updated":"2019-07-24 15:38:21.000000000","message":"This should not have been changed to \"if\"; I deliberately wrote \"iff\"\n\nhttps://en.wikipedia.org/wiki/Iff","commit_id":"175f40b057c0c75f87f01ec30bfd084246039fa8"},{"author":{"_account_id":13478,"name":"Boris Bobrov","email":"b.bobrov@sap.com","username":"bbobrov"},"change_message_id":"5eda2a6343a4ef2a447d596d8002249bcc08998f","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Apply SEV-specific guest config when SEV is required"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Add a new _sev_required() method to the libvirt driver to detect that"},{"line_number":10,"context_line":"SEV is required and return True iff the following are both true:"},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"  a) the supports_amd_sev instance variable in the driver is"},{"line_number":13,"context_line":"     true, *and*"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":18,"id":"7faddb67_68535418","line":10,"range":{"start_line":10,"start_character":32,"end_line":10,"end_character":35},"in_reply_to":"7faddb67_a4d2de26","updated":"2019-07-17 11:36:41.000000000","message":"Done","commit_id":"175f40b057c0c75f87f01ec30bfd084246039fa8"}],"nova/tests/unit/virt/libvirt/test_designer.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"7492ba9961c40eb4e61ff0c9cec7a07ac2a34f35","unresolved":false,"context_lines":[{"line_number":238,"context_line":"                                config.LibvirtConfigGuestVideo,"},{"line_number":239,"context_line":"                                config.LibvirtConfigGuestSerial)"},{"line_number":240,"context_line":""},{"line_number":241,"context_line":"        for i, dev in enumerate(conf.devices):"},{"line_number":242,"context_line":"            for ignored in ignored_driver_types:"},{"line_number":243,"context_line":"                if isinstance(dev, ignored):"},{"line_number":244,"context_line":"                    break"}],"source_content_type":"text/x-python","patch_set":49,"id":"7faddb67_777d88e4","line":241,"range":{"start_line":241,"start_character":12,"end_line":241,"end_character":13},"updated":"2019-09-03 15:41:43.000000000","message":"unused so enumerate is not needed","commit_id":"285046f01cc5cc4f74c9ffab65c0a8074448b8fb"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"2fe8ed783a148a4e793a73de92f84ee308652990","unresolved":false,"context_lines":[{"line_number":238,"context_line":"                                config.LibvirtConfigGuestVideo,"},{"line_number":239,"context_line":"                                config.LibvirtConfigGuestSerial)"},{"line_number":240,"context_line":""},{"line_number":241,"context_line":"        for i, dev in enumerate(conf.devices):"},{"line_number":242,"context_line":"            for ignored in ignored_driver_types:"},{"line_number":243,"context_line":"                if isinstance(dev, ignored):"},{"line_number":244,"context_line":"                    break"}],"source_content_type":"text/x-python","patch_set":49,"id":"7faddb67_773588f5","line":241,"range":{"start_line":241,"start_character":12,"end_line":241,"end_character":13},"in_reply_to":"7faddb67_777d88e4","updated":"2019-09-03 16:14:51.000000000","message":"Done in next PS.","commit_id":"285046f01cc5cc4f74c9ffab65c0a8074448b8fb"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"7492ba9961c40eb4e61ff0c9cec7a07ac2a34f35","unresolved":false,"context_lines":[{"line_number":246,"context_line":"                self.assertTrue("},{"line_number":247,"context_line":"                    dev.driver_iommu,"},{"line_number":248,"context_line":"                    \"expected device to have driver_iommu enabled\\n%s\" %"},{"line_number":249,"context_line":"                    dev.to_xml())"}],"source_content_type":"text/x-python","patch_set":49,"id":"7faddb67_975b6484","line":249,"updated":"2019-09-03 15:41:43.000000000","message":"Could you change the test case not to repeat the code under test so closely? It feels like this test case needs a unit test to see how it work. Could you simply assert what is in the conf.devices after L232?\n\nBtw as far as I see FAKE_KVM_GUEST only has a disk device nothing else. So this test does not cover the case when set_driver_iommu_for_sev ignores devices","commit_id":"285046f01cc5cc4f74c9ffab65c0a8074448b8fb"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"17ad84efc24b8bcccf6ba04619f9d3786fe19d7d","unresolved":false,"context_lines":[{"line_number":246,"context_line":"                self.assertTrue("},{"line_number":247,"context_line":"                    dev.driver_iommu,"},{"line_number":248,"context_line":"                    \"expected device to have driver_iommu enabled\\n%s\" %"},{"line_number":249,"context_line":"                    dev.to_xml())"}],"source_content_type":"text/x-python","patch_set":49,"id":"5faad753_68c0fb51","line":249,"in_reply_to":"7faddb67_4293e85c","updated":"2019-09-05 22:26:28.000000000","message":"\u003e \u003e Could you change the test case not to repeat the code under test so closely?\n\u003e \n\u003e Yes, that bothered me too but wasn\u0027t entirely sure how to improve.\n\nNow done in patchset 51:\n\nhttps://review.opendev.org/#/c/644565/51..52/nova/tests/unit/virt/libvirt/test_designer.py","commit_id":"285046f01cc5cc4f74c9ffab65c0a8074448b8fb"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"2fe8ed783a148a4e793a73de92f84ee308652990","unresolved":false,"context_lines":[{"line_number":246,"context_line":"                self.assertTrue("},{"line_number":247,"context_line":"                    dev.driver_iommu,"},{"line_number":248,"context_line":"                    \"expected device to have driver_iommu enabled\\n%s\" %"},{"line_number":249,"context_line":"                    dev.to_xml())"}],"source_content_type":"text/x-python","patch_set":49,"id":"7faddb67_4293e85c","line":249,"in_reply_to":"7faddb67_975b6484","updated":"2019-09-03 16:14:51.000000000","message":"\u003e Could you change the test case not to repeat the code under test so closely?\n\nYes, that bothered me too but wasn\u0027t entirely sure how to improve.\n\n\u003e It feels like this test case needs a unit test to see how it work. Could you simply assert what is in the conf.devices after L232?\n\nI\u0027ll try to do that.\n\n\u003e Btw as far as I see FAKE_KVM_GUEST only has a disk device nothing else. So this test does not cover the case when set_driver_iommu_for_sev ignores devices\n\nWell spotted, I already had this fixed for the next patch set :-)","commit_id":"285046f01cc5cc4f74c9ffab65c0a8074448b8fb"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"f3924b01af92d8f22ed91203cb8bcb06b8e03353","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":52,"id":"5faad753_62a32f51","line":249,"updated":"2019-09-06 11:39:09.000000000","message":"Was going to suggest checking the values of these elements before too, but we\u0027re kind of doing that already elsewhere with [1]\n\n[1] https://review.opendev.org/#/c/680526/1/nova/tests/unit/virt/libvirt/test_config.py","commit_id":"61268d8f334a734dfe23882b4db5305867fa9caa"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"ac0b0630918feaf36a17ea22e516296bebbb568b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":53,"id":"5faad753_5dccb134","line":249,"updated":"2019-09-09 14:45:21.000000000","message":"What about devices[5] and devices[7]?","commit_id":"71316d15bc0ee1dc1afb2a76da5e0fed006c0e0e"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"6d80c0c4b14091e0d42bd8df4e3a68f93d162e10","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":53,"id":"5faad753_d9e09ecf","line":249,"in_reply_to":"5faad753_0bbb4fe2","updated":"2019-09-10 12:28:06.000000000","message":"Thanks for the explanation.","commit_id":"71316d15bc0ee1dc1afb2a76da5e0fed006c0e0e"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"4afe95625e78267a215ab924e1d56797992a2fbf","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":53,"id":"5faad753_0bbb4fe2","line":249,"in_reply_to":"5faad753_5dccb134","updated":"2019-09-09 15:33:32.000000000","message":"They don\u0027t have a driver_iommu attribute.","commit_id":"71316d15bc0ee1dc1afb2a76da5e0fed006c0e0e"}],"nova/tests/unit/virt/libvirt/test_driver.py":[{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"55146ddceda17c44b6f118493667865b445f13f4","unresolved":false,"context_lines":[{"line_number":2603,"context_line":"                                     image_meta, disk_info,"},{"line_number":2604,"context_line":"                                     context\u003dctxt)"},{"line_number":2605,"context_line":"        # all disks are expected to be virtio, thus iommu should be on"},{"line_number":2606,"context_line":"        self.assertTrue(cfg.devices[0].driver_iommu)"},{"line_number":2607,"context_line":"        self.assertTrue(cfg.devices[1].driver_iommu)"},{"line_number":2608,"context_line":"        self.assertTrue(cfg.devices[2].driver_iommu)"},{"line_number":2609,"context_line":"        # interface"},{"line_number":2610,"context_line":"        self.assertTrue(cfg.devices[3].driver_iommu)"},{"line_number":2611,"context_line":"        # memballoon"},{"line_number":2612,"context_line":"        self.assertTrue(cfg.devices[8].driver_iommu)"},{"line_number":2613,"context_line":""},{"line_number":2614,"context_line":"    def test_get_guest_memory_backing_config_file_backed(self):"},{"line_number":2615,"context_line":"        self.flags(file_backed_memory\u003d1024, group\u003d\"libvirt\")"}],"source_content_type":"text/x-python","patch_set":4,"id":"9fb8cfa7_66401437","line":2612,"range":{"start_line":2606,"start_character":0,"end_line":2612,"end_character":52},"updated":"2019-06-11 16:19:37.000000000","message":"Is it safe to rely on a certain order in cfg.devices?  Seems very brittle to me, but even if it is, it doesn\u0027t make for very readable tests.  So I\u0027d prefer checking by device type/name here.  You could also do this in a for loop.","commit_id":"28b84c6c3dc1319bc4f5ed1c8fcb42aa18e3a92e"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"d7741b8a7ae3136a5f5bb56efb2b5050945d6c47","unresolved":false,"context_lines":[{"line_number":2704,"context_line":"        self.assertTrue(cfg.membacking.locked)"},{"line_number":2705,"context_line":"        # All disks/interfaces/memballoon are expected to be virtio,"},{"line_number":2706,"context_line":"        # thus driver_iommu should be on"},{"line_number":2707,"context_line":"        for i, dev in enumerate(cfg.devices):"},{"line_number":2708,"context_line":"            if isinstance(dev, vconfig.LibvirtConfigGuestInput):"},{"line_number":2709,"context_line":"                continue  # FIXME"},{"line_number":2710,"context_line":"            elif isinstance(dev, vconfig.LibvirtConfigGuestVideo):"},{"line_number":2711,"context_line":"                continue  # FIXME"},{"line_number":2712,"context_line":""},{"line_number":2713,"context_line":"            if hasattr(dev, \u0027driver_iommu\u0027):"},{"line_number":2714,"context_line":"                self.assertTrue(dev.driver_iommu)"},{"line_number":2715,"context_line":""},{"line_number":2716,"context_line":"    def test_get_guest_memory_backing_config_file_backed(self):"},{"line_number":2717,"context_line":"        self.flags(file_backed_memory\u003d1024, group\u003d\"libvirt\")"}],"source_content_type":"text/x-python","patch_set":31,"id":"7faddb67_8cd140fe","line":2714,"range":{"start_line":2707,"start_character":0,"end_line":2714,"end_character":49},"updated":"2019-07-30 20:34:20.000000000","message":"this appears to be WIP?","commit_id":"4918f0b2c8992cc1befc9b5bc503873ce1b8b38a"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"c7b94a89f242c115eeaf01000a31e36f80310446","unresolved":false,"context_lines":[{"line_number":2704,"context_line":"        self.assertTrue(cfg.membacking.locked)"},{"line_number":2705,"context_line":"        # All disks/interfaces/memballoon are expected to be virtio,"},{"line_number":2706,"context_line":"        # thus driver_iommu should be on"},{"line_number":2707,"context_line":"        for i, dev in enumerate(cfg.devices):"},{"line_number":2708,"context_line":"            if isinstance(dev, vconfig.LibvirtConfigGuestInput):"},{"line_number":2709,"context_line":"                continue  # FIXME"},{"line_number":2710,"context_line":"            elif isinstance(dev, vconfig.LibvirtConfigGuestVideo):"},{"line_number":2711,"context_line":"                continue  # FIXME"},{"line_number":2712,"context_line":""},{"line_number":2713,"context_line":"            if hasattr(dev, \u0027driver_iommu\u0027):"},{"line_number":2714,"context_line":"                self.assertTrue(dev.driver_iommu)"},{"line_number":2715,"context_line":""},{"line_number":2716,"context_line":"    def test_get_guest_memory_backing_config_file_backed(self):"},{"line_number":2717,"context_line":"        self.flags(file_backed_memory\u003d1024, group\u003d\"libvirt\")"}],"source_content_type":"text/x-python","patch_set":31,"id":"7faddb67_7e53b56d","line":2714,"range":{"start_line":2707,"start_character":0,"end_line":2714,"end_character":49},"in_reply_to":"7faddb67_8cd140fe","updated":"2019-08-08 21:24:07.000000000","message":"Not really - I had put the FIXMEs in because I wasn\u0027t sure whether the input and video devices need to be virtio, but it turns out they don\u0027t, so the FIXMEs can just be removed.","commit_id":"4918f0b2c8992cc1befc9b5bc503873ce1b8b38a"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"a8fb145265a694e5a627fc925f8666c299514121","unresolved":false,"context_lines":[{"line_number":2659,"context_line":"        drvr \u003d libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True)"},{"line_number":2660,"context_line":"        drvr._host._supports_amd_sev \u003d True"},{"line_number":2661,"context_line":""},{"line_number":2662,"context_line":"        test_instance \u003d copy.deepcopy(self.test_instance)"},{"line_number":2663,"context_line":"        test_instance[\"display_name\"] \u003d \"purple tomVatoes\""},{"line_number":2664,"context_line":"        test_instance[\u0027system_metadata\u0027][\u0027owner_project_name\u0027] \u003d \u0027sweetshop\u0027"},{"line_number":2665,"context_line":"        test_instance[\u0027system_metadata\u0027][\u0027owner_user_name\u0027] \u003d \u0027cupcake\u0027"}],"source_content_type":"text/x-python","patch_set":49,"id":"7faddb67_c2fd9808","line":2662,"updated":"2019-09-03 15:51:38.000000000","message":"no need for copying as test_instance is created in the setUp so it is an independent object for each test case.","commit_id":"285046f01cc5cc4f74c9ffab65c0a8074448b8fb"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"2fe8ed783a148a4e793a73de92f84ee308652990","unresolved":false,"context_lines":[{"line_number":2659,"context_line":"        drvr \u003d libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True)"},{"line_number":2660,"context_line":"        drvr._host._supports_amd_sev \u003d True"},{"line_number":2661,"context_line":""},{"line_number":2662,"context_line":"        test_instance \u003d copy.deepcopy(self.test_instance)"},{"line_number":2663,"context_line":"        test_instance[\"display_name\"] \u003d \"purple tomVatoes\""},{"line_number":2664,"context_line":"        test_instance[\u0027system_metadata\u0027][\u0027owner_project_name\u0027] \u003d \u0027sweetshop\u0027"},{"line_number":2665,"context_line":"        test_instance[\u0027system_metadata\u0027][\u0027owner_user_name\u0027] \u003d \u0027cupcake\u0027"}],"source_content_type":"text/x-python","patch_set":49,"id":"7faddb67_02a730aa","line":2662,"in_reply_to":"7faddb67_c2fd9808","updated":"2019-09-03 16:14:51.000000000","message":"Fixed in next PS.","commit_id":"285046f01cc5cc4f74c9ffab65c0a8074448b8fb"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"a8fb145265a694e5a627fc925f8666c299514121","unresolved":false,"context_lines":[{"line_number":2660,"context_line":"        drvr._host._supports_amd_sev \u003d True"},{"line_number":2661,"context_line":""},{"line_number":2662,"context_line":"        test_instance \u003d copy.deepcopy(self.test_instance)"},{"line_number":2663,"context_line":"        test_instance[\"display_name\"] \u003d \"purple tomVatoes\""},{"line_number":2664,"context_line":"        test_instance[\u0027system_metadata\u0027][\u0027owner_project_name\u0027] \u003d \u0027sweetshop\u0027"},{"line_number":2665,"context_line":"        test_instance[\u0027system_metadata\u0027][\u0027owner_user_name\u0027] \u003d \u0027cupcake\u0027"},{"line_number":2666,"context_line":""},{"line_number":2667,"context_line":"        ctxt \u003d context.RequestContext(project_id\u003d123,"},{"line_number":2668,"context_line":"                                      project_name\u003d\"aubergine\","}],"source_content_type":"text/x-python","patch_set":49,"id":"7faddb67_826a40b2","line":2665,"range":{"start_line":2663,"start_character":0,"end_line":2665,"end_character":71},"updated":"2019-09-03 15:51:38.000000000","message":"This seems unnecessary. At least the test passes without these lines.","commit_id":"285046f01cc5cc4f74c9ffab65c0a8074448b8fb"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"2fe8ed783a148a4e793a73de92f84ee308652990","unresolved":false,"context_lines":[{"line_number":2660,"context_line":"        drvr._host._supports_amd_sev \u003d True"},{"line_number":2661,"context_line":""},{"line_number":2662,"context_line":"        test_instance \u003d copy.deepcopy(self.test_instance)"},{"line_number":2663,"context_line":"        test_instance[\"display_name\"] \u003d \"purple tomVatoes\""},{"line_number":2664,"context_line":"        test_instance[\u0027system_metadata\u0027][\u0027owner_project_name\u0027] \u003d \u0027sweetshop\u0027"},{"line_number":2665,"context_line":"        test_instance[\u0027system_metadata\u0027][\u0027owner_user_name\u0027] \u003d \u0027cupcake\u0027"},{"line_number":2666,"context_line":""},{"line_number":2667,"context_line":"        ctxt \u003d context.RequestContext(project_id\u003d123,"},{"line_number":2668,"context_line":"                                      project_name\u003d\"aubergine\","}],"source_content_type":"text/x-python","patch_set":49,"id":"7faddb67_e2e0d4ce","line":2665,"range":{"start_line":2663,"start_character":0,"end_line":2665,"end_character":71},"in_reply_to":"7faddb67_826a40b2","updated":"2019-09-03 16:14:51.000000000","message":"Yes, I don\u0027t know why Boris added this.  Will remove in next PS.","commit_id":"285046f01cc5cc4f74c9ffab65c0a8074448b8fb"}],"nova/virt/libvirt/config.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"f3924b01af92d8f22ed91203cb8bcb06b8e03353","unresolved":false,"context_lines":[{"line_number":2991,"context_line":"        super(LibvirtConfigGuestRng, self).__init__(root_name\u003d\"rng\","},{"line_number":2992,"context_line":"                                                      **kwargs)"},{"line_number":2993,"context_line":""},{"line_number":2994,"context_line":"        self.device_model \u003d \u0027virtio\u0027"},{"line_number":2995,"context_line":"        self.model \u003d \u0027random\u0027"},{"line_number":2996,"context_line":"        self.backend \u003d None"},{"line_number":2997,"context_line":"        self.rate_period \u003d None"}],"source_content_type":"text/x-python","patch_set":52,"id":"5faad753_c258e357","line":2994,"updated":"2019-09-06 11:39:09.000000000","message":"nit: unrelated?","commit_id":"61268d8f334a734dfe23882b4db5305867fa9caa"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"58ec7bfae62c50d655be579eb238d81a563218fb","unresolved":false,"context_lines":[{"line_number":2991,"context_line":"        super(LibvirtConfigGuestRng, self).__init__(root_name\u003d\"rng\","},{"line_number":2992,"context_line":"                                                      **kwargs)"},{"line_number":2993,"context_line":""},{"line_number":2994,"context_line":"        self.device_model \u003d \u0027virtio\u0027"},{"line_number":2995,"context_line":"        self.model \u003d \u0027random\u0027"},{"line_number":2996,"context_line":"        self.backend \u003d None"},{"line_number":2997,"context_line":"        self.rate_period \u003d None"}],"source_content_type":"text/x-python","patch_set":52,"id":"5faad753_7ff2dc08","line":2994,"in_reply_to":"5faad753_98ed7c27","updated":"2019-09-06 21:58:36.000000000","message":"Actually I was wrong - it *is* still needed, and it was a bug in this patch set that it wasn\u0027t being used.  I was setting iommu on any device of a certain type, regardless of whether virtio was enabled, and this resulted in iommu being incorrectly set on sata config drives.  This was actually discovered by the brand new tempest test I just wrote!\n\nhttps://review.opendev.org/#/c/680777/1/tempest/scenario/test_server_sev.py@117\n\nIt\u0027s the first time I tried a configdrive with an SEV guest, which is why I hadn\u0027t spotted the bug before.\n\nThis is fixed in PS53 and you can see it in use here:\n\nhttps://review.opendev.org/#/c/644565/53/nova/virt/libvirt/designer.py@206","commit_id":"61268d8f334a734dfe23882b4db5305867fa9caa"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"c08bb65991126740f95ca8002fbfc604558fd23a","unresolved":false,"context_lines":[{"line_number":2991,"context_line":"        super(LibvirtConfigGuestRng, self).__init__(root_name\u003d\"rng\","},{"line_number":2992,"context_line":"                                                      **kwargs)"},{"line_number":2993,"context_line":""},{"line_number":2994,"context_line":"        self.device_model \u003d \u0027virtio\u0027"},{"line_number":2995,"context_line":"        self.model \u003d \u0027random\u0027"},{"line_number":2996,"context_line":"        self.backend \u003d None"},{"line_number":2997,"context_line":"        self.rate_period \u003d None"}],"source_content_type":"text/x-python","patch_set":52,"id":"5faad753_98ed7c27","line":2994,"in_reply_to":"5faad753_c258e357","updated":"2019-09-06 12:49:33.000000000","message":"Ah, good catch.  In previous versions of this patch (PS47 and earlier), this was necessary so that check_set_sev() could correctly determine whether the driver was virtio and thus whether it should have iommu enabled:\n\nhttps://review.opendev.org/#/c/644565/47/nova/virt/libvirt/driver.py@4889\n\nHowever we later scrapped that approach in favour of designer.py, so this is not technically needed any more.  However, I think it\u0027s still better this way - more explicitly declarative than hiding hardcoded values in format_dom().  Also if support was ever added for another device model in the future this would make it a tiny teeny teensy bit easier to do.","commit_id":"61268d8f334a734dfe23882b4db5305867fa9caa"}],"nova/virt/libvirt/designer.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"7492ba9961c40eb4e61ff0c9cec7a07ac2a34f35","unresolved":false,"context_lines":[{"line_number":205,"context_line":"                            config.LibvirtConfigGuestVideo,"},{"line_number":206,"context_line":"                            config.LibvirtConfigGuestSerial)"},{"line_number":207,"context_line":""},{"line_number":208,"context_line":"    for i, dev in enumerate(conf.devices):"},{"line_number":209,"context_line":"        for ignored in ignored_driver_types:"},{"line_number":210,"context_line":"            if isinstance(dev, ignored):"},{"line_number":211,"context_line":"                break"}],"source_content_type":"text/x-python","patch_set":49,"id":"7faddb67_d7e0fcce","line":208,"range":{"start_line":208,"start_character":8,"end_line":208,"end_character":9},"updated":"2019-09-03 15:41:43.000000000","message":"this is unused so you don\u0027t need enumerate either:\n\nfor dev in conf.devices:\n    ...","commit_id":"285046f01cc5cc4f74c9ffab65c0a8074448b8fb"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"2fe8ed783a148a4e793a73de92f84ee308652990","unresolved":false,"context_lines":[{"line_number":205,"context_line":"                            config.LibvirtConfigGuestVideo,"},{"line_number":206,"context_line":"                            config.LibvirtConfigGuestSerial)"},{"line_number":207,"context_line":""},{"line_number":208,"context_line":"    for i, dev in enumerate(conf.devices):"},{"line_number":209,"context_line":"        for ignored in ignored_driver_types:"},{"line_number":210,"context_line":"            if isinstance(dev, ignored):"},{"line_number":211,"context_line":"                break"}],"source_content_type":"text/x-python","patch_set":49,"id":"7faddb67_42cf6856","line":208,"range":{"start_line":208,"start_character":8,"end_line":208,"end_character":9},"in_reply_to":"7faddb67_d7e0fcce","updated":"2019-09-03 16:14:51.000000000","message":"Fixed in next PS.","commit_id":"285046f01cc5cc4f74c9ffab65c0a8074448b8fb"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"3276e80af72335bb1c3051309b4fb7f5892fa4df","unresolved":false,"context_lines":[{"line_number":200,"context_line":""},{"line_number":201,"context_line":""},{"line_number":202,"context_line":"def set_driver_iommu_for_sev(conf):"},{"line_number":203,"context_line":"    virtio_attrs \u003d {"},{"line_number":204,"context_line":"        config.LibvirtConfigGuestDisk: \u0027target_bus\u0027,"},{"line_number":205,"context_line":"        config.LibvirtConfigGuestInterface: \u0027model\u0027,"},{"line_number":206,"context_line":"        config.LibvirtConfigGuestRng: \u0027device_model\u0027,"},{"line_number":207,"context_line":"        config.LibvirtConfigMemoryBalloon: \u0027model\u0027,"},{"line_number":208,"context_line":"    }"},{"line_number":209,"context_line":""},{"line_number":210,"context_line":"    for dev in conf.devices:"},{"line_number":211,"context_line":"        virtio_attr \u003d virtio_attrs.get(dev.__class__)"},{"line_number":212,"context_line":"        if virtio_attr and getattr(dev, virtio_attr) \u003d\u003d \u0027virtio\u0027:"},{"line_number":213,"context_line":"            dev.driver_iommu \u003d True"}],"source_content_type":"text/x-python","patch_set":53,"id":"5faad753_d98dfe31","line":213,"range":{"start_line":203,"start_character":0,"end_line":213,"end_character":35},"updated":"2019-09-10 12:37:43.000000000","message":"nit: _Personally_, I\u0027d much rather we played dumb here with something like:\n\n    for dev in conf.devices:\n        if any((isinstance(dev, config.LibvirtConfigGuestDisk) and\n                dev.target_bus \u003d\u003d \u0027virtio\u0027) or\n               (isinstance(dev, config.LibvirtConfigGuestInterface) and\n                dev.target_bus \u003d\u003d \u0027virtio\u0027) or\n               (isinstance(dev, config.LibvirtConfigGuestRng) and\n                dev.target_bus \u003d\u003d \u0027virtio\u0027) or\n               (isinstance(dev, config.LibvirtConfigMemoryBalloon) and\n                dev.target_bus \u003d\u003d \u0027virtio\u0027)):\n            dev.driver_iommu \u003d True\n\nIt\u0027s certainly more verbose and probably uglier, but it\u0027s blatantly obvious what\u0027s going on. This is certainly not the worst instance, but at this point I almost consider the use of getattr to be a code smell since it\u0027s so good at obscuring stuff.\n\nAnother alternative would be to add a \u0027uses_virtio\u0027 property to the base class of these four classes and overrides them here, but I haven\u0027t looked to see if that\u0027s even possible","commit_id":"71316d15bc0ee1dc1afb2a76da5e0fed006c0e0e"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"b83d2116dc9d3c94f1c507627b4d60c2685dbda3","unresolved":false,"context_lines":[{"line_number":200,"context_line":""},{"line_number":201,"context_line":""},{"line_number":202,"context_line":"def set_driver_iommu_for_sev(conf):"},{"line_number":203,"context_line":"    virtio_attrs \u003d {"},{"line_number":204,"context_line":"        config.LibvirtConfigGuestDisk: \u0027target_bus\u0027,"},{"line_number":205,"context_line":"        config.LibvirtConfigGuestInterface: \u0027model\u0027,"},{"line_number":206,"context_line":"        config.LibvirtConfigGuestRng: \u0027device_model\u0027,"},{"line_number":207,"context_line":"        config.LibvirtConfigMemoryBalloon: \u0027model\u0027,"},{"line_number":208,"context_line":"    }"},{"line_number":209,"context_line":""},{"line_number":210,"context_line":"    for dev in conf.devices:"},{"line_number":211,"context_line":"        virtio_attr \u003d virtio_attrs.get(dev.__class__)"},{"line_number":212,"context_line":"        if virtio_attr and getattr(dev, virtio_attr) \u003d\u003d \u0027virtio\u0027:"},{"line_number":213,"context_line":"            dev.driver_iommu \u003d True"}],"source_content_type":"text/x-python","patch_set":53,"id":"5faad753_db0ded3e","line":213,"range":{"start_line":203,"start_character":0,"end_line":213,"end_character":35},"in_reply_to":"5faad753_ca067a87","updated":"2019-09-10 15:30:26.000000000","message":"\u003e That hurts my eyes ;-)  But I take your point.\n\nMe too, heh\n\n \u003e I think the uses_virtio idea is *far* nicer, and yes it\u0027s easily\n \u003e doable.  We\u0027d probably have to add a default \u0027return False\u0027 to the\n \u003e method in the base class LibvirtConfigGuestDevice.\n\nYup, agreed","commit_id":"71316d15bc0ee1dc1afb2a76da5e0fed006c0e0e"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"cd10c1164cf227445620b1267b7c06d552e0cece","unresolved":false,"context_lines":[{"line_number":200,"context_line":""},{"line_number":201,"context_line":""},{"line_number":202,"context_line":"def set_driver_iommu_for_sev(conf):"},{"line_number":203,"context_line":"    virtio_attrs \u003d {"},{"line_number":204,"context_line":"        config.LibvirtConfigGuestDisk: \u0027target_bus\u0027,"},{"line_number":205,"context_line":"        config.LibvirtConfigGuestInterface: \u0027model\u0027,"},{"line_number":206,"context_line":"        config.LibvirtConfigGuestRng: \u0027device_model\u0027,"},{"line_number":207,"context_line":"        config.LibvirtConfigMemoryBalloon: \u0027model\u0027,"},{"line_number":208,"context_line":"    }"},{"line_number":209,"context_line":""},{"line_number":210,"context_line":"    for dev in conf.devices:"},{"line_number":211,"context_line":"        virtio_attr \u003d virtio_attrs.get(dev.__class__)"},{"line_number":212,"context_line":"        if virtio_attr and getattr(dev, virtio_attr) \u003d\u003d \u0027virtio\u0027:"},{"line_number":213,"context_line":"            dev.driver_iommu \u003d True"}],"source_content_type":"text/x-python","patch_set":53,"id":"5faad753_ca067a87","line":213,"range":{"start_line":203,"start_character":0,"end_line":213,"end_character":35},"in_reply_to":"5faad753_d98dfe31","updated":"2019-09-10 13:37:38.000000000","message":"That hurts my eyes ;-)  But I take your point.\n\nI think the uses_virtio idea is *far* nicer, and yes it\u0027s easily doable.  We\u0027d probably have to add a default \u0027return False\u0027 to the method in the base class LibvirtConfigGuestDevice.","commit_id":"71316d15bc0ee1dc1afb2a76da5e0fed006c0e0e"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":13478,"name":"Boris Bobrov","email":"b.bobrov@sap.com","username":"bbobrov"},"change_message_id":"6f4341290ee289241388d4da127c919fa87357cd","unresolved":false,"context_lines":[{"line_number":5380,"context_line":"        # plus 16 MiB"},{"line_number":5381,"context_line":"        memtune \u003d vconfig.LibvirtConfigGuestMemoryTune()"},{"line_number":5382,"context_line":"        memtune.hard_limit \u003d guest.memory + vram + 16 * units.Mi / units.Ki"},{"line_number":5383,"context_line":"        guest.memtune \u003d memtune"},{"line_number":5384,"context_line":""},{"line_number":5385,"context_line":"    def _guest_add_mdevs(self, guest, chosen_mdevs):"},{"line_number":5386,"context_line":"        for chosen_mdev in chosen_mdevs:"}],"source_content_type":"text/x-python","patch_set":1,"id":"5fc1f717_ca46c2c1","line":5383,"updated":"2019-03-19 13:18:04.000000000","message":"this does not reflect the discussion happening in the spec and will have to be fixed","commit_id":"471844e20942bdc9ea525098a71036467ae0261c"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"cb7c9c365fd65b2d2db2ade88a96b65cde4bc7a1","unresolved":false,"context_lines":[{"line_number":878,"context_line":""},{"line_number":879,"context_line":"    @staticmethod"},{"line_number":880,"context_line":"    def _sev_required(flavor):"},{"line_number":881,"context_line":"        sev \u003d flavor.extra_specs.get(\u0027traits:HW_CPU_AMD_SEV\u0027, False)"},{"line_number":882,"context_line":"        return bool(sev)"},{"line_number":883,"context_line":""},{"line_number":884,"context_line":"    def instance_exists(self, instance):"}],"source_content_type":"text/x-python","patch_set":2,"id":"9fb8cfa7_c8d9d922","line":881,"updated":"2019-06-03 12:30:58.000000000","message":"It\u0027s now HW_CPU_X86_AMD_SEV since https://review.opendev.org/#/c/655193/ landed and https://review.opendev.org/#/c/662718/ is on the way.\n\nAlso, don\u0027t you have to check for \"...\u003drequired\", because it could also be \"...\u003dforbidden\"?\n\nFinally I\u0027m wondering if we should be checking for hw:mem_encryption here, but I guess the request filter will have already translated it by this stage, and this method will be consumed by SEV-specific code, so maybe it\u0027s correct to leave it dependent on the SEV trait.  Eric can hopefully confirm this.","commit_id":"786e72d7eab10ed6046aa6a94a18a050eff1fd95"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"55146ddceda17c44b6f118493667865b445f13f4","unresolved":false,"context_lines":[{"line_number":878,"context_line":""},{"line_number":879,"context_line":"    @staticmethod"},{"line_number":880,"context_line":"    def _sev_required(flavor):"},{"line_number":881,"context_line":"        sev \u003d flavor.extra_specs.get(\u0027traits:HW_CPU_AMD_SEV\u0027, False)"},{"line_number":882,"context_line":"        return bool(sev)"},{"line_number":883,"context_line":""},{"line_number":884,"context_line":"    def instance_exists(self, instance):"}],"source_content_type":"text/x-python","patch_set":2,"id":"9fb8cfa7_15a213fd","line":881,"in_reply_to":"9fb8cfa7_c8d9d922","updated":"2019-06-11 16:19:37.000000000","message":"The right approach was clarified in this discussion:\n\nhttp://eavesdrop.openstack.org/irclogs/%23openstack-nova/%23openstack-nova.2019-06-11.log.html#t2019-06-11T16:01:45","commit_id":"786e72d7eab10ed6046aa6a94a18a050eff1fd95"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"c5427bd5733c556404880c3eb401828d436934bd","unresolved":false,"context_lines":[{"line_number":919,"context_line":"        # conversion which will return value of type unicode."},{"line_number":920,"context_line":"        return uri and str(uri)"},{"line_number":921,"context_line":""},{"line_number":922,"context_line":"    def _sev_required(self, flavor, image):"},{"line_number":923,"context_line":"        if not self.supports_amd_sev:"},{"line_number":924,"context_line":"            return False"},{"line_number":925,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"9fb8cfa7_1294d243","line":922,"range":{"start_line":922,"start_character":36,"end_line":922,"end_character":41},"updated":"2019-06-19 21:00:44.000000000","message":"this should be spelled `image_meta`","commit_id":"1ddaf8383af92934309298a815d665aea8192db6"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"7088c26d051fb4de95147e8f38a665036f268254","unresolved":false,"context_lines":[{"line_number":919,"context_line":"        # conversion which will return value of type unicode."},{"line_number":920,"context_line":"        return uri and str(uri)"},{"line_number":921,"context_line":""},{"line_number":922,"context_line":"    def _sev_required(self, flavor, image):"},{"line_number":923,"context_line":"        if not self.supports_amd_sev:"},{"line_number":924,"context_line":"            return False"},{"line_number":925,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"9fb8cfa7_2202db0c","line":922,"range":{"start_line":922,"start_character":36,"end_line":922,"end_character":41},"in_reply_to":"9fb8cfa7_1294d243","updated":"2019-06-20 14:18:52.000000000","message":"Done","commit_id":"1ddaf8383af92934309298a815d665aea8192db6"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"c5427bd5733c556404880c3eb401828d436934bd","unresolved":false,"context_lines":[{"line_number":1623,"context_line":"            disk_info[\u0027unit\u0027] \u003d self._get_scsi_controller_max_unit(guest) + 1"},{"line_number":1624,"context_line":""},{"line_number":1625,"context_line":"        conf \u003d self._get_volume_config(connection_info, disk_info)"},{"line_number":1626,"context_line":"        if \u0027virtio\u0027 in disk_info[\u0027bus\u0027]:"},{"line_number":1627,"context_line":"            conf.driver_iommu \u003d self._sev_required(instance.flavor,"},{"line_number":1628,"context_line":"                                                   instance.image_meta)"},{"line_number":1629,"context_line":""},{"line_number":1630,"context_line":"        self._check_discard_for_attach_volume(conf, instance)"},{"line_number":1631,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"9fb8cfa7_f2a6deab","line":1628,"range":{"start_line":1626,"start_character":0,"end_line":1628,"end_character":71},"updated":"2019-06-19 21:00:44.000000000","message":"I notice (throughout) that you\u0027re explicitly setting $thing to false when sev is off, as opposed to omitting $thing. That may be How It\u0027s Done, just something that stood out","commit_id":"1ddaf8383af92934309298a815d665aea8192db6"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"7088c26d051fb4de95147e8f38a665036f268254","unresolved":false,"context_lines":[{"line_number":1623,"context_line":"            disk_info[\u0027unit\u0027] \u003d self._get_scsi_controller_max_unit(guest) + 1"},{"line_number":1624,"context_line":""},{"line_number":1625,"context_line":"        conf \u003d self._get_volume_config(connection_info, disk_info)"},{"line_number":1626,"context_line":"        if \u0027virtio\u0027 in disk_info[\u0027bus\u0027]:"},{"line_number":1627,"context_line":"            conf.driver_iommu \u003d self._sev_required(instance.flavor,"},{"line_number":1628,"context_line":"                                                   instance.image_meta)"},{"line_number":1629,"context_line":""},{"line_number":1630,"context_line":"        self._check_discard_for_attach_volume(conf, instance)"},{"line_number":1631,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"9fb8cfa7_7d563209","line":1628,"range":{"start_line":1626,"start_character":0,"end_line":1628,"end_character":71},"in_reply_to":"9fb8cfa7_1227f214","updated":"2019-06-20 14:18:52.000000000","message":"This is how Boris wrote it, and I didn\u0027t think to question it.  It\u0027s possible in general I guess, and even if it\u0027s never the case right now, it could happen due to future code additions, so I\u0027ll drop the explicit switch-off.","commit_id":"1ddaf8383af92934309298a815d665aea8192db6"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"b6065af787e673d930f39283615c41ded52d9347","unresolved":false,"context_lines":[{"line_number":1623,"context_line":"            disk_info[\u0027unit\u0027] \u003d self._get_scsi_controller_max_unit(guest) + 1"},{"line_number":1624,"context_line":""},{"line_number":1625,"context_line":"        conf \u003d self._get_volume_config(connection_info, disk_info)"},{"line_number":1626,"context_line":"        if \u0027virtio\u0027 in disk_info[\u0027bus\u0027]:"},{"line_number":1627,"context_line":"            conf.driver_iommu \u003d self._sev_required(instance.flavor,"},{"line_number":1628,"context_line":"                                                   instance.image_meta)"},{"line_number":1629,"context_line":""},{"line_number":1630,"context_line":"        self._check_discard_for_attach_volume(conf, instance)"},{"line_number":1631,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"9fb8cfa7_1227f214","line":1628,"range":{"start_line":1626,"start_character":0,"end_line":1628,"end_character":71},"in_reply_to":"9fb8cfa7_f2a6deab","updated":"2019-06-19 21:04:06.000000000","message":"Oh, looking more closely at the previous patch, the deeper code does the set-to-`on`-iff-True-else-exclude logic. So yeah, here you\u0027re explicitly switching $thing off rather than just letting it default. Is that on purpose? Like, is it possible it was on before, and then you get here and something has changed and you decide you need to turn it off?","commit_id":"1ddaf8383af92934309298a815d665aea8192db6"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"1124a9d3350e16a391b64fe9883b8dd4d12daca3","unresolved":false,"context_lines":[{"line_number":4834,"context_line":"        return True"},{"line_number":4835,"context_line":""},{"line_number":4836,"context_line":"    def _add_video_driver(self, guest, image_meta, flavor):"},{"line_number":4837,"context_line":"        VALID_VIDEO_DEVICES \u003d (\"vga\", \"cirrus\", \"vmvga\","},{"line_number":4838,"context_line":"                               \"xen\", \"qxl\", \"virtio\")"},{"line_number":4839,"context_line":"        video \u003d vconfig.LibvirtConfigGuestVideo()"},{"line_number":4840,"context_line":"        # NOTE(ldbragst): The following logic sets the video.type"}],"source_content_type":"text/x-python","patch_set":18,"id":"7faddb67_a4765e6f","line":4837,"range":{"start_line":4837,"start_character":8,"end_line":4837,"end_character":27},"updated":"2019-07-17 01:00:09.000000000","message":"This parameter is not used.","commit_id":"175f40b057c0c75f87f01ec30bfd084246039fa8"},{"author":{"_account_id":13478,"name":"Boris Bobrov","email":"b.bobrov@sap.com","username":"bbobrov"},"change_message_id":"5eda2a6343a4ef2a447d596d8002249bcc08998f","unresolved":false,"context_lines":[{"line_number":4834,"context_line":"        return True"},{"line_number":4835,"context_line":""},{"line_number":4836,"context_line":"    def _add_video_driver(self, guest, image_meta, flavor):"},{"line_number":4837,"context_line":"        VALID_VIDEO_DEVICES \u003d (\"vga\", \"cirrus\", \"vmvga\","},{"line_number":4838,"context_line":"                               \"xen\", \"qxl\", \"virtio\")"},{"line_number":4839,"context_line":"        video \u003d vconfig.LibvirtConfigGuestVideo()"},{"line_number":4840,"context_line":"        # NOTE(ldbragst): The following logic sets the video.type"}],"source_content_type":"text/x-python","patch_set":18,"id":"7faddb67_4850d80b","line":4837,"range":{"start_line":4837,"start_character":8,"end_line":4837,"end_character":27},"in_reply_to":"7faddb67_a4765e6f","updated":"2019-07-17 11:36:41.000000000","message":"Done","commit_id":"175f40b057c0c75f87f01ec30bfd084246039fa8"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"d7741b8a7ae3136a5f5bb56efb2b5050945d6c47","unresolved":false,"context_lines":[{"line_number":885,"context_line":"        # conversion which will return value of type unicode."},{"line_number":886,"context_line":"        return uri and str(uri)"},{"line_number":887,"context_line":""},{"line_number":888,"context_line":"    def _sev_enabled(self, flavor, image_meta):"},{"line_number":889,"context_line":"        return libvirt_utils.sev_enabled(self._host, flavor, image_meta)"},{"line_number":890,"context_line":""},{"line_number":891,"context_line":"    def instance_exists(self, instance):"}],"source_content_type":"text/x-python","patch_set":31,"id":"7faddb67_56ed19b5","line":888,"updated":"2019-07-30 20:34:20.000000000","message":"This kinda doesn\u0027t seem worth having, versus just inlining the call below.\n\nThat said, I could see the benefit if this method did a hair more, like:\n\n def check_set_sev(self, conf, instance, businfo):\n     if (\u0027virtio\u0027 in businfo and\n             libvirt_utils.sev_enabled(\n                 self._host, instance.flavor, instance.image_meta):\n         conf.driver_iommu \u003d True\n     return conf\n\nThough tbh I\u0027m not sure why you wouldn\u0027t just have that ^ in libvirt_utils itself so you could call it the same way both below and in vif.py","commit_id":"4918f0b2c8992cc1befc9b5bc503873ce1b8b38a"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"790c57b347d608a783885ea2d46a8ec957f8e8bf","unresolved":false,"context_lines":[{"line_number":885,"context_line":"        # conversion which will return value of type unicode."},{"line_number":886,"context_line":"        return uri and str(uri)"},{"line_number":887,"context_line":""},{"line_number":888,"context_line":"    def _sev_enabled(self, flavor, image_meta):"},{"line_number":889,"context_line":"        return libvirt_utils.sev_enabled(self._host, flavor, image_meta)"},{"line_number":890,"context_line":""},{"line_number":891,"context_line":"    def instance_exists(self, instance):"}],"source_content_type":"text/x-python","patch_set":31,"id":"7faddb67_21e0aefa","line":888,"in_reply_to":"7faddb67_56ed19b5","updated":"2019-08-15 18:23:55.000000000","message":"Well, it reduces the number of parameters by one ;-)  But yeah, check_set_sev is a really nice idea to avoid a lot of duplicated code, so I\u0027ll pinch that idea - thanks.","commit_id":"4918f0b2c8992cc1befc9b5bc503873ce1b8b38a"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"d7741b8a7ae3136a5f5bb56efb2b5050945d6c47","unresolved":false,"context_lines":[{"line_number":1571,"context_line":"        conf \u003d self._get_volume_config(connection_info, disk_info)"},{"line_number":1572,"context_line":"        if (\u0027virtio\u0027 in disk_info[\u0027bus\u0027] and"},{"line_number":1573,"context_line":"                self._sev_enabled(instance.flavor, instance.image_meta)):"},{"line_number":1574,"context_line":"            conf.driver_iommu \u003d True"},{"line_number":1575,"context_line":""},{"line_number":1576,"context_line":"        self._check_discard_for_attach_volume(conf, instance)"},{"line_number":1577,"context_line":""}],"source_content_type":"text/x-python","patch_set":31,"id":"7faddb67_8c3f0024","line":1574,"updated":"2019-07-30 20:34:20.000000000","message":"not tested","commit_id":"4918f0b2c8992cc1befc9b5bc503873ce1b8b38a"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"790c57b347d608a783885ea2d46a8ec957f8e8bf","unresolved":false,"context_lines":[{"line_number":1571,"context_line":"        conf \u003d self._get_volume_config(connection_info, disk_info)"},{"line_number":1572,"context_line":"        if (\u0027virtio\u0027 in disk_info[\u0027bus\u0027] and"},{"line_number":1573,"context_line":"                self._sev_enabled(instance.flavor, instance.image_meta)):"},{"line_number":1574,"context_line":"            conf.driver_iommu \u003d True"},{"line_number":1575,"context_line":""},{"line_number":1576,"context_line":"        self._check_discard_for_attach_volume(conf, instance)"},{"line_number":1577,"context_line":""}],"source_content_type":"text/x-python","patch_set":31,"id":"7faddb67_a6380d9f","line":1574,"in_reply_to":"7faddb67_8c3f0024","updated":"2019-08-15 18:23:55.000000000","message":"Done","commit_id":"4918f0b2c8992cc1befc9b5bc503873ce1b8b38a"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"d7741b8a7ae3136a5f5bb56efb2b5050945d6c47","unresolved":false,"context_lines":[{"line_number":1703,"context_line":"        conf \u003d self._get_volume_config(new_connection_info, disk_info)"},{"line_number":1704,"context_line":"        if (\u0027virtio\u0027 in disk_info[\u0027bus\u0027] and"},{"line_number":1705,"context_line":"                self._sev_enabled(instance.flavor, instance.image_meta)):"},{"line_number":1706,"context_line":"            conf.driver_iommu \u003d True"},{"line_number":1707,"context_line":"        if not conf.source_path:"},{"line_number":1708,"context_line":"            self._disconnect_volume(context, new_connection_info, instance)"},{"line_number":1709,"context_line":"            raise NotImplementedError(_(\"Swap only supports host devices\"))"}],"source_content_type":"text/x-python","patch_set":31,"id":"7faddb67_ec0f9485","line":1706,"updated":"2019-07-30 20:34:20.000000000","message":"not tested","commit_id":"4918f0b2c8992cc1befc9b5bc503873ce1b8b38a"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"790c57b347d608a783885ea2d46a8ec957f8e8bf","unresolved":false,"context_lines":[{"line_number":1703,"context_line":"        conf \u003d self._get_volume_config(new_connection_info, disk_info)"},{"line_number":1704,"context_line":"        if (\u0027virtio\u0027 in disk_info[\u0027bus\u0027] and"},{"line_number":1705,"context_line":"                self._sev_enabled(instance.flavor, instance.image_meta)):"},{"line_number":1706,"context_line":"            conf.driver_iommu \u003d True"},{"line_number":1707,"context_line":"        if not conf.source_path:"},{"line_number":1708,"context_line":"            self._disconnect_volume(context, new_connection_info, instance)"},{"line_number":1709,"context_line":"            raise NotImplementedError(_(\"Swap only supports host devices\"))"}],"source_content_type":"text/x-python","patch_set":31,"id":"7faddb67_4f4efec1","line":1706,"in_reply_to":"7faddb67_ec0f9485","updated":"2019-08-15 18:23:55.000000000","message":"Done","commit_id":"4918f0b2c8992cc1befc9b5bc503873ce1b8b38a"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"d7741b8a7ae3136a5f5bb56efb2b5050945d6c47","unresolved":false,"context_lines":[{"line_number":4086,"context_line":"                                 disk_unit\u003ddisk_unit)"},{"line_number":4087,"context_line":"        if (\u0027virtio\u0027 in disk_info[\u0027bus\u0027] and"},{"line_number":4088,"context_line":"                self._sev_enabled(instance.flavor, instance.image_meta)):"},{"line_number":4089,"context_line":"            conf.driver_iommu \u003d True"},{"line_number":4090,"context_line":"        return conf"},{"line_number":4091,"context_line":""},{"line_number":4092,"context_line":"    def _get_guest_fs_config(self, instance, name, image_type\u003dNone):"}],"source_content_type":"text/x-python","patch_set":31,"id":"7faddb67_4c734817","line":4089,"updated":"2019-07-30 20:34:20.000000000","message":"tested ✔","commit_id":"4918f0b2c8992cc1befc9b5bc503873ce1b8b38a"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"d7741b8a7ae3136a5f5bb56efb2b5050945d6c47","unresolved":false,"context_lines":[{"line_number":4224,"context_line":"            cfg \u003d self._get_volume_config(connection_info, info)"},{"line_number":4225,"context_line":"            if (\u0027virtio\u0027 in info[\u0027bus\u0027] and"},{"line_number":4226,"context_line":"                    self._sev_enabled(instance.flavor, instance.image_meta)):"},{"line_number":4227,"context_line":"                cfg.driver_iommu \u003d True"},{"line_number":4228,"context_line":"            devices.append(cfg)"},{"line_number":4229,"context_line":"            vol[\u0027connection_info\u0027] \u003d connection_info"},{"line_number":4230,"context_line":"            vol.save()"}],"source_content_type":"text/x-python","patch_set":31,"id":"7faddb67_6c708409","line":4227,"updated":"2019-07-30 20:34:20.000000000","message":"not tested","commit_id":"4918f0b2c8992cc1befc9b5bc503873ce1b8b38a"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"790c57b347d608a783885ea2d46a8ec957f8e8bf","unresolved":false,"context_lines":[{"line_number":4224,"context_line":"            cfg \u003d self._get_volume_config(connection_info, info)"},{"line_number":4225,"context_line":"            if (\u0027virtio\u0027 in info[\u0027bus\u0027] and"},{"line_number":4226,"context_line":"                    self._sev_enabled(instance.flavor, instance.image_meta)):"},{"line_number":4227,"context_line":"                cfg.driver_iommu \u003d True"},{"line_number":4228,"context_line":"            devices.append(cfg)"},{"line_number":4229,"context_line":"            vol[\u0027connection_info\u0027] \u003d connection_info"},{"line_number":4230,"context_line":"            vol.save()"}],"source_content_type":"text/x-python","patch_set":31,"id":"7faddb67_ad3a1aca","line":4227,"in_reply_to":"7faddb67_6c708409","updated":"2019-08-15 18:23:55.000000000","message":"Done","commit_id":"4918f0b2c8992cc1befc9b5bc503873ce1b8b38a"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"d7741b8a7ae3136a5f5bb56efb2b5050945d6c47","unresolved":false,"context_lines":[{"line_number":4247,"context_line":"            scsi_controller.index \u003d 0"},{"line_number":4248,"context_line":"            if (\u0027virtio\u0027 in hw_scsi_model and"},{"line_number":4249,"context_line":"                    self._sev_enabled(flavor, image_meta)):"},{"line_number":4250,"context_line":"                scsi_controller.driver_iommu \u003d True"},{"line_number":4251,"context_line":"            return scsi_controller"},{"line_number":4252,"context_line":""},{"line_number":4253,"context_line":"    def _get_host_sysinfo_serial_hardware(self):"}],"source_content_type":"text/x-python","patch_set":31,"id":"7faddb67_6cde44db","line":4250,"updated":"2019-07-30 20:34:20.000000000","message":"not tested","commit_id":"4918f0b2c8992cc1befc9b5bc503873ce1b8b38a"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"790c57b347d608a783885ea2d46a8ec957f8e8bf","unresolved":false,"context_lines":[{"line_number":4247,"context_line":"            scsi_controller.index \u003d 0"},{"line_number":4248,"context_line":"            if (\u0027virtio\u0027 in hw_scsi_model and"},{"line_number":4249,"context_line":"                    self._sev_enabled(flavor, image_meta)):"},{"line_number":4250,"context_line":"                scsi_controller.driver_iommu \u003d True"},{"line_number":4251,"context_line":"            return scsi_controller"},{"line_number":4252,"context_line":""},{"line_number":4253,"context_line":"    def _get_host_sysinfo_serial_hardware(self):"}],"source_content_type":"text/x-python","patch_set":31,"id":"7faddb67_4d1606aa","line":4250,"in_reply_to":"7faddb67_6cde44db","updated":"2019-08-15 18:23:55.000000000","message":"Done","commit_id":"4918f0b2c8992cc1befc9b5bc503873ce1b8b38a"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"d7741b8a7ae3136a5f5bb56efb2b5050945d6c47","unresolved":false,"context_lines":[{"line_number":4849,"context_line":"                raise exception.InvalidVideoMode(model\u003dvideo.type)"},{"line_number":4850,"context_line":""},{"line_number":4851,"context_line":"        if \u0027virtio\u0027 in video.type and self._sev_enabled(flavor, image_meta):"},{"line_number":4852,"context_line":"            video.driver_iommu \u003d True"},{"line_number":4853,"context_line":""},{"line_number":4854,"context_line":"        # Set video memory, only if the flavor\u0027s limit is set"},{"line_number":4855,"context_line":"        video_ram \u003d image_meta.properties.get(\u0027hw_video_ram\u0027, 0)"}],"source_content_type":"text/x-python","patch_set":31,"id":"7faddb67_8c04e04b","line":4852,"updated":"2019-07-30 20:34:20.000000000","message":"not tested","commit_id":"4918f0b2c8992cc1befc9b5bc503873ce1b8b38a"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"790c57b347d608a783885ea2d46a8ec957f8e8bf","unresolved":false,"context_lines":[{"line_number":4849,"context_line":"                raise exception.InvalidVideoMode(model\u003dvideo.type)"},{"line_number":4850,"context_line":""},{"line_number":4851,"context_line":"        if \u0027virtio\u0027 in video.type and self._sev_enabled(flavor, image_meta):"},{"line_number":4852,"context_line":"            video.driver_iommu \u003d True"},{"line_number":4853,"context_line":""},{"line_number":4854,"context_line":"        # Set video memory, only if the flavor\u0027s limit is set"},{"line_number":4855,"context_line":"        video_ram \u003d image_meta.properties.get(\u0027hw_video_ram\u0027, 0)"}],"source_content_type":"text/x-python","patch_set":31,"id":"7faddb67_dc3615b5","line":4852,"in_reply_to":"7faddb67_8c04e04b","updated":"2019-08-15 18:23:55.000000000","message":"Done","commit_id":"4918f0b2c8992cc1befc9b5bc503873ce1b8b38a"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"d7741b8a7ae3136a5f5bb56efb2b5050945d6c47","unresolved":false,"context_lines":[{"line_number":4887,"context_line":"        # For KVM, there is only virtio variant of the RNG device, so"},{"line_number":4888,"context_line":"        # there is no need to check its \"model\" attribute"},{"line_number":4889,"context_line":"        if self._sev_enabled(flavor, image_meta):"},{"line_number":4890,"context_line":"            rng_device.driver_iommu \u003d True"},{"line_number":4891,"context_line":"        guest.add_device(rng_device)"},{"line_number":4892,"context_line":""},{"line_number":4893,"context_line":"    def _set_qemu_guest_agent(self, guest, flavor, instance, image_meta):"}],"source_content_type":"text/x-python","patch_set":31,"id":"7faddb67_8cb2a093","line":4890,"updated":"2019-07-30 20:34:20.000000000","message":"not tested","commit_id":"4918f0b2c8992cc1befc9b5bc503873ce1b8b38a"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"790c57b347d608a783885ea2d46a8ec957f8e8bf","unresolved":false,"context_lines":[{"line_number":4887,"context_line":"        # For KVM, there is only virtio variant of the RNG device, so"},{"line_number":4888,"context_line":"        # there is no need to check its \"model\" attribute"},{"line_number":4889,"context_line":"        if self._sev_enabled(flavor, image_meta):"},{"line_number":4890,"context_line":"            rng_device.driver_iommu \u003d True"},{"line_number":4891,"context_line":"        guest.add_device(rng_device)"},{"line_number":4892,"context_line":""},{"line_number":4893,"context_line":"    def _set_qemu_guest_agent(self, guest, flavor, instance, image_meta):"}],"source_content_type":"text/x-python","patch_set":31,"id":"7faddb67_392de594","line":4890,"in_reply_to":"7faddb67_8cb2a093","updated":"2019-08-15 18:23:55.000000000","message":"Done","commit_id":"4918f0b2c8992cc1befc9b5bc503873ce1b8b38a"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"d7741b8a7ae3136a5f5bb56efb2b5050945d6c47","unresolved":false,"context_lines":[{"line_number":4946,"context_line":"        if self._sev_enabled(flavor, image_meta):"},{"line_number":4947,"context_line":"            if not membacking:"},{"line_number":4948,"context_line":"                membacking \u003d vconfig.LibvirtConfigGuestMemoryBacking()"},{"line_number":4949,"context_line":"            membacking.locked \u003d True"},{"line_number":4950,"context_line":""},{"line_number":4951,"context_line":"        return membacking"},{"line_number":4952,"context_line":""}],"source_content_type":"text/x-python","patch_set":31,"id":"7faddb67_4c7d88b0","line":4949,"updated":"2019-07-30 20:34:20.000000000","message":"tested ✔","commit_id":"4918f0b2c8992cc1befc9b5bc503873ce1b8b38a"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"d7741b8a7ae3136a5f5bb56efb2b5050945d6c47","unresolved":false,"context_lines":[{"line_number":5081,"context_line":""},{"line_number":5082,"context_line":"    def _guest_machine_type(self, arch, guest, image_meta, sev_enabled):"},{"line_number":5083,"context_line":"        if arch \u003d\u003d fields.Architecture.X86_64 and sev_enabled:"},{"line_number":5084,"context_line":"            # NOTE(aspiers): as explained in the SEV spec, SEV needs a"},{"line_number":5085,"context_line":"            # q35 machine type in order to bind all the virtio devices"},{"line_number":5086,"context_line":"            # to the PCIe bridge so that they use virtio 1.0 and not"},{"line_number":5087,"context_line":"            # virtio 0.9, since QEMU\u0027s iommu_platform feature was"}],"source_content_type":"text/x-python","patch_set":31,"id":"7faddb67_d158338b","line":5084,"range":{"start_line":5084,"start_character":42,"end_line":5084,"end_character":57},"updated":"2019-07-30 20:34:20.000000000","message":"could link that here","commit_id":"4918f0b2c8992cc1befc9b5bc503873ce1b8b38a"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"790c57b347d608a783885ea2d46a8ec957f8e8bf","unresolved":false,"context_lines":[{"line_number":5081,"context_line":""},{"line_number":5082,"context_line":"    def _guest_machine_type(self, arch, guest, image_meta, sev_enabled):"},{"line_number":5083,"context_line":"        if arch \u003d\u003d fields.Architecture.X86_64 and sev_enabled:"},{"line_number":5084,"context_line":"            # NOTE(aspiers): as explained in the SEV spec, SEV needs a"},{"line_number":5085,"context_line":"            # q35 machine type in order to bind all the virtio devices"},{"line_number":5086,"context_line":"            # to the PCIe bridge so that they use virtio 1.0 and not"},{"line_number":5087,"context_line":"            # virtio 0.9, since QEMU\u0027s iommu_platform feature was"}],"source_content_type":"text/x-python","patch_set":31,"id":"7faddb67_f970ada6","line":5084,"range":{"start_line":5084,"start_character":42,"end_line":5084,"end_character":57},"in_reply_to":"7faddb67_d158338b","updated":"2019-08-15 18:23:55.000000000","message":"Done","commit_id":"4918f0b2c8992cc1befc9b5bc503873ce1b8b38a"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"d7741b8a7ae3136a5f5bb56efb2b5050945d6c47","unresolved":false,"context_lines":[{"line_number":5479,"context_line":""},{"line_number":5480,"context_line":"        self._guest_add_watchdog_action(guest, flavor, image_meta)"},{"line_number":5481,"context_line":""},{"line_number":5482,"context_line":"        self._guest_add_memory_balloon(guest, sev_enabled\u003dself._sev_enabled("},{"line_number":5483,"context_line":"            flavor, image_meta))"},{"line_number":5484,"context_line":""},{"line_number":5485,"context_line":"        if mdevs:"},{"line_number":5486,"context_line":"            self._guest_add_mdevs(guest, mdevs)"}],"source_content_type":"text/x-python","patch_set":31,"id":"7faddb67_9125dbc4","line":5483,"range":{"start_line":5482,"start_character":58,"end_line":5483,"end_character":31},"updated":"2019-07-30 20:34:20.000000000","message":"why not use the already calculated sev_enabled local var here?","commit_id":"4918f0b2c8992cc1befc9b5bc503873ce1b8b38a"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"790c57b347d608a783885ea2d46a8ec957f8e8bf","unresolved":false,"context_lines":[{"line_number":5479,"context_line":""},{"line_number":5480,"context_line":"        self._guest_add_watchdog_action(guest, flavor, image_meta)"},{"line_number":5481,"context_line":""},{"line_number":5482,"context_line":"        self._guest_add_memory_balloon(guest, sev_enabled\u003dself._sev_enabled("},{"line_number":5483,"context_line":"            flavor, image_meta))"},{"line_number":5484,"context_line":""},{"line_number":5485,"context_line":"        if mdevs:"},{"line_number":5486,"context_line":"            self._guest_add_mdevs(guest, mdevs)"}],"source_content_type":"text/x-python","patch_set":31,"id":"7faddb67_b966b5ec","line":5483,"range":{"start_line":5482,"start_character":58,"end_line":5483,"end_character":31},"in_reply_to":"7faddb67_9125dbc4","updated":"2019-08-15 18:23:55.000000000","message":"Done","commit_id":"4918f0b2c8992cc1befc9b5bc503873ce1b8b38a"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"d7741b8a7ae3136a5f5bb56efb2b5050945d6c47","unresolved":false,"context_lines":[{"line_number":5492,"context_line":""},{"line_number":5493,"context_line":"    def _guest_add_sev(self, guest, arch, mach_type):"},{"line_number":5494,"context_line":"        sev \u003d self._find_sev_feature(arch, mach_type)"},{"line_number":5495,"context_line":"        if sev is None:"},{"line_number":5496,"context_line":"            # In theory this should never happen because it should"},{"line_number":5497,"context_line":"            # only get called if SEV was requested, in which case the"},{"line_number":5498,"context_line":"            # guest should only get scheduled on this host if it"},{"line_number":5499,"context_line":"            # supports SEV, and SEV support is dependent on the"},{"line_number":5500,"context_line":"            # presence of this \u003csev\u003e feature.  That said, it\u0027s"},{"line_number":5501,"context_line":"            # conceivable that something could get messed up along the"},{"line_number":5502,"context_line":"            # way, e.g. a mismatch in the choice of machine type.  So"},{"line_number":5503,"context_line":"            # make sure that if it ever does happen, we at least get a"},{"line_number":5504,"context_line":"            # helpful error rather than something cryptic like"},{"line_number":5505,"context_line":"            # \"AttributeError: \u0027NoneType\u0027 object has no attribute \u0027cbitpos\u0027"},{"line_number":5506,"context_line":"            raise exception.MissingDomainCapabilityFeatureException("},{"line_number":5507,"context_line":"                feature\u003d\u0027sev\u0027)"},{"line_number":5508,"context_line":""},{"line_number":5509,"context_line":"        launch_security \u003d vconfig.LibvirtConfigGuestSEVLaunchSecurity()"},{"line_number":5510,"context_line":"        launch_security.cbitpos \u003d sev.cbitpos"}],"source_content_type":"text/x-python","patch_set":31,"id":"7faddb67_0c4eb06e","line":5507,"range":{"start_line":5495,"start_character":8,"end_line":5507,"end_character":30},"updated":"2019-07-30 20:34:20.000000000","message":"recognizing the logic would be slightly more awkward, this part seems like it should happen in _find_sev_feature","commit_id":"4918f0b2c8992cc1befc9b5bc503873ce1b8b38a"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"790c57b347d608a783885ea2d46a8ec957f8e8bf","unresolved":false,"context_lines":[{"line_number":5492,"context_line":""},{"line_number":5493,"context_line":"    def _guest_add_sev(self, guest, arch, mach_type):"},{"line_number":5494,"context_line":"        sev \u003d self._find_sev_feature(arch, mach_type)"},{"line_number":5495,"context_line":"        if sev is None:"},{"line_number":5496,"context_line":"            # In theory this should never happen because it should"},{"line_number":5497,"context_line":"            # only get called if SEV was requested, in which case the"},{"line_number":5498,"context_line":"            # guest should only get scheduled on this host if it"},{"line_number":5499,"context_line":"            # supports SEV, and SEV support is dependent on the"},{"line_number":5500,"context_line":"            # presence of this \u003csev\u003e feature.  That said, it\u0027s"},{"line_number":5501,"context_line":"            # conceivable that something could get messed up along the"},{"line_number":5502,"context_line":"            # way, e.g. a mismatch in the choice of machine type.  So"},{"line_number":5503,"context_line":"            # make sure that if it ever does happen, we at least get a"},{"line_number":5504,"context_line":"            # helpful error rather than something cryptic like"},{"line_number":5505,"context_line":"            # \"AttributeError: \u0027NoneType\u0027 object has no attribute \u0027cbitpos\u0027"},{"line_number":5506,"context_line":"            raise exception.MissingDomainCapabilityFeatureException("},{"line_number":5507,"context_line":"                feature\u003d\u0027sev\u0027)"},{"line_number":5508,"context_line":""},{"line_number":5509,"context_line":"        launch_security \u003d vconfig.LibvirtConfigGuestSEVLaunchSecurity()"},{"line_number":5510,"context_line":"        launch_security.cbitpos \u003d sev.cbitpos"}],"source_content_type":"text/x-python","patch_set":31,"id":"7faddb67_59e22132","line":5507,"range":{"start_line":5495,"start_character":8,"end_line":5507,"end_character":30},"in_reply_to":"7faddb67_0c4eb06e","updated":"2019-08-15 18:23:55.000000000","message":"Not sure I follow. Currently _find_sev_feature() can return None for three different reasons.  So moving this check there would require duplicating this raise three times, or at least making that method substantially more convoluted.\n\nAlso semantically it seems reasonable that other code might want to call _find_sev_feature() and even sometimes expect None to be returned without needing to raise an exception.  In general there\u0027s nothing inherently wrong with not finding the SEV feature present; however in the specific case of _guest_add_sev() there\u0027s something wrong because it should only be called if the feature was already present.\n\nSo I\u0027m not really sure what we could change here to improve things.","commit_id":"4918f0b2c8992cc1befc9b5bc503873ce1b8b38a"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"d7741b8a7ae3136a5f5bb56efb2b5050945d6c47","unresolved":false,"context_lines":[{"line_number":5563,"context_line":"            if virt_type in (\u0027qemu\u0027, \u0027kvm\u0027):"},{"line_number":5564,"context_line":"                balloon.model \u003d \u0027virtio\u0027"},{"line_number":5565,"context_line":"                if sev_enabled:"},{"line_number":5566,"context_line":"                    balloon.driver_iommu \u003d True"},{"line_number":5567,"context_line":"            else:"},{"line_number":5568,"context_line":"                balloon.model \u003d \u0027xen\u0027"},{"line_number":5569,"context_line":"            balloon.period \u003d CONF.libvirt.mem_stats_period_seconds"}],"source_content_type":"text/x-python","patch_set":31,"id":"7faddb67_cc3738fe","line":5566,"updated":"2019-07-30 20:34:20.000000000","message":"tested ✔","commit_id":"4918f0b2c8992cc1befc9b5bc503873ce1b8b38a"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"b6a13e76928a1572601da7fa1788e459b39bea3a","unresolved":false,"context_lines":[{"line_number":4886,"context_line":"        if (rng_path and not os.path.exists(rng_path)):"},{"line_number":4887,"context_line":"            raise exception.RngDeviceNotExist(path\u003drng_path)"},{"line_number":4888,"context_line":"        rng_device.backend \u003d rng_path"},{"line_number":4889,"context_line":"        libvirt_utils.check_set_sev(rng_device, rng_device.device_model,"},{"line_number":4890,"context_line":"                                    self._host, flavor, image_meta)"},{"line_number":4891,"context_line":"        guest.add_device(rng_device)"},{"line_number":4892,"context_line":""},{"line_number":4893,"context_line":"    def _set_qemu_guest_agent(self, guest, flavor, instance, image_meta):"}],"source_content_type":"text/x-python","patch_set":46,"id":"7faddb67_70a14095","line":4890,"range":{"start_line":4889,"start_character":8,"end_line":4890,"end_character":67},"updated":"2019-08-30 05:40:54.000000000","message":"I don\u0027t think we should check this everywhere for opening up the iommu. See my comment in the previous patch https://review.opendev.org/#/c/636318/49/nova/virt/libvirt/config.py@2701","commit_id":"8ebc59eb50a5f0df723ee6d01ba5112734bbff6d"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"def925a3ddf4e9a416786ae0d59a4286a40b7acb","unresolved":false,"context_lines":[{"line_number":4886,"context_line":"        if (rng_path and not os.path.exists(rng_path)):"},{"line_number":4887,"context_line":"            raise exception.RngDeviceNotExist(path\u003drng_path)"},{"line_number":4888,"context_line":"        rng_device.backend \u003d rng_path"},{"line_number":4889,"context_line":"        libvirt_utils.check_set_sev(rng_device, rng_device.device_model,"},{"line_number":4890,"context_line":"                                    self._host, flavor, image_meta)"},{"line_number":4891,"context_line":"        guest.add_device(rng_device)"},{"line_number":4892,"context_line":""},{"line_number":4893,"context_line":"    def _set_qemu_guest_agent(self, guest, flavor, instance, image_meta):"}],"source_content_type":"text/x-python","patch_set":46,"id":"7faddb67_952db590","line":4890,"range":{"start_line":4889,"start_character":8,"end_line":4890,"end_character":67},"in_reply_to":"7faddb67_4c4a68ad","updated":"2019-09-01 16:11:35.000000000","message":"Eric agreed with this:\n\nhttp://eavesdrop.openstack.org/irclogs/%23openstack-nova/%23openstack-nova.2019-08-30.log.html#t2019-08-30T14:09:25\n\nSo I\u0027ve changed it to use this approach from PS48 onwards.","commit_id":"8ebc59eb50a5f0df723ee6d01ba5112734bbff6d"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"8b678c38f1ebf1dc8d2c1ce82a7d8d08229a08e7","unresolved":false,"context_lines":[{"line_number":4886,"context_line":"        if (rng_path and not os.path.exists(rng_path)):"},{"line_number":4887,"context_line":"            raise exception.RngDeviceNotExist(path\u003drng_path)"},{"line_number":4888,"context_line":"        rng_device.backend \u003d rng_path"},{"line_number":4889,"context_line":"        libvirt_utils.check_set_sev(rng_device, rng_device.device_model,"},{"line_number":4890,"context_line":"                                    self._host, flavor, image_meta)"},{"line_number":4891,"context_line":"        guest.add_device(rng_device)"},{"line_number":4892,"context_line":""},{"line_number":4893,"context_line":"    def _set_qemu_guest_agent(self, guest, flavor, instance, image_meta):"}],"source_content_type":"text/x-python","patch_set":46,"id":"7faddb67_4c4a68ad","line":4890,"range":{"start_line":4889,"start_character":8,"end_line":4890,"end_character":67},"in_reply_to":"7faddb67_70a14095","updated":"2019-08-30 13:52:19.000000000","message":"I\u0027ve replied to your comment in the same place.","commit_id":"8ebc59eb50a5f0df723ee6d01ba5112734bbff6d"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"276d79a7bef6c7f6faa1d3de362b56137d48e691","unresolved":false,"context_lines":[{"line_number":5087,"context_line":"                guest.os_init_path \u003d \"/sbin/init\""},{"line_number":5088,"context_line":""},{"line_number":5089,"context_line":"    def _guest_machine_type(self, arch, guest, image_meta, sev_enabled):"},{"line_number":5090,"context_line":"        if arch \u003d\u003d fields.Architecture.X86_64 and sev_enabled:"},{"line_number":5091,"context_line":"            # NOTE(aspiers): As explained in the SEV spec, SEV needs a"},{"line_number":5092,"context_line":"            # q35 machine type in order to bind all the virtio devices"},{"line_number":5093,"context_line":"            # to the PCIe bridge so that they use virtio 1.0 and not"}],"source_content_type":"text/x-python","patch_set":49,"id":"7faddb67_0e3a0fcc","line":5090,"range":{"start_line":5090,"start_character":8,"end_line":5090,"end_character":61},"updated":"2019-09-02 12:46:10.000000000","message":"Per our discussion on #openstack-nova, Freenode, let\u0027s extract this SEV-specifc conditional away into a separate method, _guest_machine_type_for_sev(), and then we call it from here.","commit_id":"285046f01cc5cc4f74c9ffab65c0a8074448b8fb"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"2fe8ed783a148a4e793a73de92f84ee308652990","unresolved":false,"context_lines":[{"line_number":5087,"context_line":"                guest.os_init_path \u003d \"/sbin/init\""},{"line_number":5088,"context_line":""},{"line_number":5089,"context_line":"    def _guest_machine_type(self, arch, guest, image_meta, sev_enabled):"},{"line_number":5090,"context_line":"        if arch \u003d\u003d fields.Architecture.X86_64 and sev_enabled:"},{"line_number":5091,"context_line":"            # NOTE(aspiers): As explained in the SEV spec, SEV needs a"},{"line_number":5092,"context_line":"            # q35 machine type in order to bind all the virtio devices"},{"line_number":5093,"context_line":"            # to the PCIe bridge so that they use virtio 1.0 and not"}],"source_content_type":"text/x-python","patch_set":49,"id":"7faddb67_147c601e","line":5090,"range":{"start_line":5090,"start_character":8,"end_line":5090,"end_character":61},"in_reply_to":"7faddb67_0e3a0fcc","updated":"2019-09-03 16:14:51.000000000","message":"Done","commit_id":"285046f01cc5cc4f74c9ffab65c0a8074448b8fb"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"ab5f2bf8ee79403898d71e9e1ba6f1ab73b5a838","unresolved":false,"context_lines":[{"line_number":5102,"context_line":"                # of the machine type is required"},{"line_number":5103,"context_line":"                return mach_type"},{"line_number":5104,"context_line":"            else:"},{"line_number":5105,"context_line":"                LOG.warning("},{"line_number":5106,"context_line":"                    \"Overriding requested machine type %(mtype)s \""},{"line_number":5107,"context_line":"                    \"for guest %(guest)s to q35 in order to ensure SEV works\","},{"line_number":5108,"context_line":"                    {\u0027mtype\u0027: mach_type, \u0027guest\u0027: \u0027guest\u0027})"}],"source_content_type":"text/x-python","patch_set":49,"id":"7faddb67_ae2bbb7f","line":5105,"updated":"2019-09-02 12:41:54.000000000","message":"Sean said on IRC he thinks we shouldn\u0027t override an explicitly requested machine type here, and instead raise an exception.  If so we would presumably want a check in the API layer too.  I\u0027m 60% likely to agree but would be good to hear other opinions.\n\nA related question is whether this could be changed as a follow-up commit.","commit_id":"285046f01cc5cc4f74c9ffab65c0a8074448b8fb"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"2fe8ed783a148a4e793a73de92f84ee308652990","unresolved":false,"context_lines":[{"line_number":5102,"context_line":"                # of the machine type is required"},{"line_number":5103,"context_line":"                return mach_type"},{"line_number":5104,"context_line":"            else:"},{"line_number":5105,"context_line":"                LOG.warning("},{"line_number":5106,"context_line":"                    \"Overriding requested machine type %(mtype)s \""},{"line_number":5107,"context_line":"                    \"for guest %(guest)s to q35 in order to ensure SEV works\","},{"line_number":5108,"context_line":"                    {\u0027mtype\u0027: mach_type, \u0027guest\u0027: \u0027guest\u0027})"}],"source_content_type":"text/x-python","patch_set":49,"id":"7faddb67_a2ff3c64","line":5105,"in_reply_to":"7faddb67_9c270b64","updated":"2019-09-03 16:14:51.000000000","message":"Yes, I\u0027ve already changed this to raise an exception, as agreed with Kashyap and Sean on IRC.  Will be in next PS.","commit_id":"285046f01cc5cc4f74c9ffab65c0a8074448b8fb"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"7492ba9961c40eb4e61ff0c9cec7a07ac2a34f35","unresolved":false,"context_lines":[{"line_number":5102,"context_line":"                # of the machine type is required"},{"line_number":5103,"context_line":"                return mach_type"},{"line_number":5104,"context_line":"            else:"},{"line_number":5105,"context_line":"                LOG.warning("},{"line_number":5106,"context_line":"                    \"Overriding requested machine type %(mtype)s \""},{"line_number":5107,"context_line":"                    \"for guest %(guest)s to q35 in order to ensure SEV works\","},{"line_number":5108,"context_line":"                    {\u0027mtype\u0027: mach_type, \u0027guest\u0027: \u0027guest\u0027})"}],"source_content_type":"text/x-python","patch_set":49,"id":"7faddb67_9c270b64","line":5105,"in_reply_to":"7faddb67_ae2bbb7f","updated":"2019-09-03 15:41:43.000000000","message":"It is actually in the spec:\n\"If SEV’s requirement of a Q35 machine type cannot be satisfied by hw_machine_type specified by the image (if present), or the value specified by libvirt.hw_machine_type in nova.conf (which is not set by default), then an exception should be raised so that the build fails.\"\n\nSo the question is, why we want to deviate from the spec.\n\nOverall I think it is better to fail than to hide a inconsistency in the request. Is there a combination where the end user providing a image metadata can cause that nova ignore what the admin asked in the flavor? If yes then I\u0027m strongly against ignoring the conflict.","commit_id":"285046f01cc5cc4f74c9ffab65c0a8074448b8fb"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"276d79a7bef6c7f6faa1d3de362b56137d48e691","unresolved":false,"context_lines":[{"line_number":5102,"context_line":"                # of the machine type is required"},{"line_number":5103,"context_line":"                return mach_type"},{"line_number":5104,"context_line":"            else:"},{"line_number":5105,"context_line":"                LOG.warning("},{"line_number":5106,"context_line":"                    \"Overriding requested machine type %(mtype)s \""},{"line_number":5107,"context_line":"                    \"for guest %(guest)s to q35 in order to ensure SEV works\","},{"line_number":5108,"context_line":"                    {\u0027mtype\u0027: mach_type, \u0027guest\u0027: \u0027guest\u0027})"},{"line_number":5109,"context_line":"                return \u0027q35\u0027"},{"line_number":5110,"context_line":""},{"line_number":5111,"context_line":"        # In most cases, just use the machine type that was requested,"},{"line_number":5112,"context_line":"        # or the default."}],"source_content_type":"text/x-python","patch_set":49,"id":"7faddb67_a33434fc","line":5109,"range":{"start_line":5105,"start_character":0,"end_line":5109,"end_character":28},"updated":"2019-09-02 12:46:10.000000000","message":"I\u0027m not sure if we should override the machine type, as it can cause unwanted surprise for the operator.  As you seemed amenable on IRC to \"bail out\" -- let\u0027s just do that, to throw an exception.","commit_id":"285046f01cc5cc4f74c9ffab65c0a8074448b8fb"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"2fe8ed783a148a4e793a73de92f84ee308652990","unresolved":false,"context_lines":[{"line_number":5102,"context_line":"                # of the machine type is required"},{"line_number":5103,"context_line":"                return mach_type"},{"line_number":5104,"context_line":"            else:"},{"line_number":5105,"context_line":"                LOG.warning("},{"line_number":5106,"context_line":"                    \"Overriding requested machine type %(mtype)s \""},{"line_number":5107,"context_line":"                    \"for guest %(guest)s to q35 in order to ensure SEV works\","},{"line_number":5108,"context_line":"                    {\u0027mtype\u0027: mach_type, \u0027guest\u0027: \u0027guest\u0027})"},{"line_number":5109,"context_line":"                return \u0027q35\u0027"},{"line_number":5110,"context_line":""},{"line_number":5111,"context_line":"        # In most cases, just use the machine type that was requested,"},{"line_number":5112,"context_line":"        # or the default."}],"source_content_type":"text/x-python","patch_set":49,"id":"7faddb67_62f5c482","line":5109,"range":{"start_line":5105,"start_character":0,"end_line":5109,"end_character":28},"in_reply_to":"7faddb67_a33434fc","updated":"2019-09-03 16:14:51.000000000","message":"Fixed in next PS.","commit_id":"285046f01cc5cc4f74c9ffab65c0a8074448b8fb"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"566c6434fe8e1e3315804af165b7587cea7bd516","unresolved":false,"context_lines":[{"line_number":5431,"context_line":"                self._get_guest_os_type(virt_type))"},{"line_number":5432,"context_line":"        caps \u003d self._host.get_capabilities()"},{"line_number":5433,"context_line":""},{"line_number":5434,"context_line":"        sev_enabled \u003d libvirt_utils.sev_enabled(self._host, flavor, image_meta)"},{"line_number":5435,"context_line":""},{"line_number":5436,"context_line":"        self._configure_guest_by_virt_type(guest, virt_type, caps, instance,"},{"line_number":5437,"context_line":"                                           image_meta, flavor,"}],"source_content_type":"text/x-python","patch_set":49,"id":"7faddb67_10cc73b9","line":5434,"range":{"start_line":5434,"start_character":8,"end_line":5434,"end_character":79},"updated":"2019-09-02 01:22:35.000000000","message":"another idea is to set a variable call \"iommu_enabled \u003d sev_enabled\", then we pass the iommu_enabled down to each \"self._guest_add_xxx\" method. That is also avoid the risk you said \"the only risk I can\u0027t see us easily avoiding is someone adding code which explicitly disables iommu but forgets to prevent the SEV code from overriding that. \"","commit_id":"285046f01cc5cc4f74c9ffab65c0a8074448b8fb"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"2fe8ed783a148a4e793a73de92f84ee308652990","unresolved":false,"context_lines":[{"line_number":5431,"context_line":"                self._get_guest_os_type(virt_type))"},{"line_number":5432,"context_line":"        caps \u003d self._host.get_capabilities()"},{"line_number":5433,"context_line":""},{"line_number":5434,"context_line":"        sev_enabled \u003d libvirt_utils.sev_enabled(self._host, flavor, image_meta)"},{"line_number":5435,"context_line":""},{"line_number":5436,"context_line":"        self._configure_guest_by_virt_type(guest, virt_type, caps, instance,"},{"line_number":5437,"context_line":"                                           image_meta, flavor,"}],"source_content_type":"text/x-python","patch_set":49,"id":"7faddb67_8299e01e","line":5434,"range":{"start_line":5434,"start_character":8,"end_line":5434,"end_character":79},"in_reply_to":"7faddb67_10cc73b9","updated":"2019-09-03 16:14:51.000000000","message":"I like the idea but not sure how it would work, since each _guest_add_xxx would receive an iommu_enabled parameter but not use it for anything, causing flake8 errors?","commit_id":"285046f01cc5cc4f74c9ffab65c0a8074448b8fb"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"7492ba9961c40eb4e61ff0c9cec7a07ac2a34f35","unresolved":false,"context_lines":[{"line_number":5527,"context_line":"            LOG.warning("},{"line_number":5528,"context_line":"                \"Wanted to add SEV to config for guest with arch %(arch)s \""},{"line_number":5529,"context_line":"                \"but only had domain capabilities for: %(archs)s\","},{"line_number":5530,"context_line":"                {\u0027arch\u0027: arch, \u0027archs\u0027: \u0027 \u0027.join(domain_caps.keys())})"},{"line_number":5531,"context_line":"            return None"},{"line_number":5532,"context_line":""},{"line_number":5533,"context_line":"        if mach_type not in domain_caps[arch]:"}],"source_content_type":"text/x-python","patch_set":49,"id":"7faddb67_97dcc445","line":5530,"range":{"start_line":5530,"start_character":60,"end_line":5530,"end_character":67},"updated":"2019-09-03 15:41:43.000000000","message":"nit: if domina_caps is a dict then the keys() is not needed.","commit_id":"285046f01cc5cc4f74c9ffab65c0a8074448b8fb"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"2fe8ed783a148a4e793a73de92f84ee308652990","unresolved":false,"context_lines":[{"line_number":5527,"context_line":"            LOG.warning("},{"line_number":5528,"context_line":"                \"Wanted to add SEV to config for guest with arch %(arch)s \""},{"line_number":5529,"context_line":"                \"but only had domain capabilities for: %(archs)s\","},{"line_number":5530,"context_line":"                {\u0027arch\u0027: arch, \u0027archs\u0027: \u0027 \u0027.join(domain_caps.keys())})"},{"line_number":5531,"context_line":"            return None"},{"line_number":5532,"context_line":""},{"line_number":5533,"context_line":"        if mach_type not in domain_caps[arch]:"}],"source_content_type":"text/x-python","patch_set":49,"id":"7faddb67_a2341c4a","line":5530,"range":{"start_line":5530,"start_character":60,"end_line":5530,"end_character":67},"in_reply_to":"7faddb67_97dcc445","updated":"2019-09-03 16:14:51.000000000","message":"Nice trick, didn\u0027t know that. Fixed in next PS.","commit_id":"285046f01cc5cc4f74c9ffab65c0a8074448b8fb"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"7492ba9961c40eb4e61ff0c9cec7a07ac2a34f35","unresolved":false,"context_lines":[{"line_number":5536,"context_line":"                \"%(mtype)s but for arch %(arch)s only had domain capabilities \""},{"line_number":5537,"context_line":"                \"for machine types: %(mtypes)s\","},{"line_number":5538,"context_line":"                {\u0027mtype\u0027: mach_type, \u0027arch\u0027: arch,"},{"line_number":5539,"context_line":"                 \u0027mtypes\u0027: \u0027 \u0027.join(domain_caps[arch].keys())})"},{"line_number":5540,"context_line":"            return None"},{"line_number":5541,"context_line":""},{"line_number":5542,"context_line":"        for feature in domain_caps[arch][mach_type].features:"}],"source_content_type":"text/x-python","patch_set":49,"id":"7faddb67_f7ead8d6","line":5539,"range":{"start_line":5539,"start_character":54,"end_line":5539,"end_character":60},"updated":"2019-09-03 15:41:43.000000000","message":"ditto","commit_id":"285046f01cc5cc4f74c9ffab65c0a8074448b8fb"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"2fe8ed783a148a4e793a73de92f84ee308652990","unresolved":false,"context_lines":[{"line_number":5536,"context_line":"                \"%(mtype)s but for arch %(arch)s only had domain capabilities \""},{"line_number":5537,"context_line":"                \"for machine types: %(mtypes)s\","},{"line_number":5538,"context_line":"                {\u0027mtype\u0027: mach_type, \u0027arch\u0027: arch,"},{"line_number":5539,"context_line":"                 \u0027mtypes\u0027: \u0027 \u0027.join(domain_caps[arch].keys())})"},{"line_number":5540,"context_line":"            return None"},{"line_number":5541,"context_line":""},{"line_number":5542,"context_line":"        for feature in domain_caps[arch][mach_type].features:"}],"source_content_type":"text/x-python","patch_set":49,"id":"7faddb67_0243b0b1","line":5539,"range":{"start_line":5539,"start_character":54,"end_line":5539,"end_character":60},"in_reply_to":"7faddb67_f7ead8d6","updated":"2019-09-03 16:14:51.000000000","message":"Fixed in next PS as above.","commit_id":"285046f01cc5cc4f74c9ffab65c0a8074448b8fb"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"566c6434fe8e1e3315804af165b7587cea7bd516","unresolved":false,"context_lines":[{"line_number":5561,"context_line":"            guest.add_device(channel)"},{"line_number":5562,"context_line":""},{"line_number":5563,"context_line":"    @staticmethod"},{"line_number":5564,"context_line":"    def _guest_add_memory_balloon(guest, sev_enabled\u003dFalse):"},{"line_number":5565,"context_line":"        virt_type \u003d guest.virt_type"},{"line_number":5566,"context_line":"        # Memory balloon device only support \u0027qemu/kvm\u0027 and \u0027xen\u0027 hypervisor"},{"line_number":5567,"context_line":"        if (virt_type in (\u0027xen\u0027, \u0027qemu\u0027, \u0027kvm\u0027) and"}],"source_content_type":"text/x-python","patch_set":49,"id":"7faddb67_90b70322","line":5564,"range":{"start_line":5564,"start_character":41,"end_line":5564,"end_character":58},"updated":"2019-09-02 01:22:35.000000000","message":"why this is special not in the designer?\n\nOr we can switch all designer to this way, pass the iommu_enabled flag to each sub method.","commit_id":"285046f01cc5cc4f74c9ffab65c0a8074448b8fb"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"2fe8ed783a148a4e793a73de92f84ee308652990","unresolved":false,"context_lines":[{"line_number":5561,"context_line":"            guest.add_device(channel)"},{"line_number":5562,"context_line":""},{"line_number":5563,"context_line":"    @staticmethod"},{"line_number":5564,"context_line":"    def _guest_add_memory_balloon(guest, sev_enabled\u003dFalse):"},{"line_number":5565,"context_line":"        virt_type \u003d guest.virt_type"},{"line_number":5566,"context_line":"        # Memory balloon device only support \u0027qemu/kvm\u0027 and \u0027xen\u0027 hypervisor"},{"line_number":5567,"context_line":"        if (virt_type in (\u0027xen\u0027, \u0027qemu\u0027, \u0027kvm\u0027) and"}],"source_content_type":"text/x-python","patch_set":49,"id":"7faddb67_d4ea48d7","line":5564,"range":{"start_line":5564,"start_character":41,"end_line":5564,"end_character":58},"in_reply_to":"7faddb67_90b70322","updated":"2019-09-03 16:14:51.000000000","message":"Done","commit_id":"285046f01cc5cc4f74c9ffab65c0a8074448b8fb"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"3276e80af72335bb1c3051309b4fb7f5892fa4df","unresolved":false,"context_lines":[{"line_number":5509,"context_line":"            # conceivable that something could get messed up along the"},{"line_number":5510,"context_line":"            # way, e.g. a mismatch in the choice of machine type.  So"},{"line_number":5511,"context_line":"            # make sure that if it ever does happen, we at least get a"},{"line_number":5512,"context_line":"            # helpful error rather than something cryptic like"},{"line_number":5513,"context_line":"            # \"AttributeError: \u0027NoneType\u0027 object has no attribute \u0027cbitpos\u0027"},{"line_number":5514,"context_line":"            raise exception.MissingDomainCapabilityFeatureException("},{"line_number":5515,"context_line":"                feature\u003d\u0027sev\u0027)"},{"line_number":5516,"context_line":""}],"source_content_type":"text/x-python","patch_set":53,"id":"5faad753_f9e47aae","line":5513,"range":{"start_line":5512,"start_character":0,"end_line":5513,"end_character":75},"updated":"2019-09-10 12:37:43.000000000","message":"Again, personally, I\u0027d have preferred to remove this since the AttributeError still achieves the end goal of highlighting a bug in nova without adding a load of extra code to do it.","commit_id":"71316d15bc0ee1dc1afb2a76da5e0fed006c0e0e"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"3276e80af72335bb1c3051309b4fb7f5892fa4df","unresolved":false,"context_lines":[{"line_number":5529,"context_line":"        \"\"\""},{"line_number":5530,"context_line":"        domain_caps \u003d self._host.get_domain_capabilities()"},{"line_number":5531,"context_line":"        if arch not in domain_caps:"},{"line_number":5532,"context_line":"            LOG.warning("},{"line_number":5533,"context_line":"                \"Wanted to add SEV to config for guest with arch %(arch)s \""},{"line_number":5534,"context_line":"                \"but only had domain capabilities for: %(archs)s\","},{"line_number":5535,"context_line":"                {\u0027arch\u0027: arch, \u0027archs\u0027: \u0027 \u0027.join(domain_caps)})"},{"line_number":5536,"context_line":"            return None"},{"line_number":5537,"context_line":""},{"line_number":5538,"context_line":"        if mach_type not in domain_caps[arch]:"},{"line_number":5539,"context_line":"            LOG.warning("}],"source_content_type":"text/x-python","patch_set":53,"id":"5faad753_5984aeb1","line":5536,"range":{"start_line":5532,"start_character":0,"end_line":5536,"end_character":23},"updated":"2019-09-10 12:37:43.000000000","message":"On that note, why aren\u0027t we just raising the exception here? We don\u0027t have any callers that actually expect this to not find something, right?","commit_id":"71316d15bc0ee1dc1afb2a76da5e0fed006c0e0e"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"cd10c1164cf227445620b1267b7c06d552e0cece","unresolved":false,"context_lines":[{"line_number":5529,"context_line":"        \"\"\""},{"line_number":5530,"context_line":"        domain_caps \u003d self._host.get_domain_capabilities()"},{"line_number":5531,"context_line":"        if arch not in domain_caps:"},{"line_number":5532,"context_line":"            LOG.warning("},{"line_number":5533,"context_line":"                \"Wanted to add SEV to config for guest with arch %(arch)s \""},{"line_number":5534,"context_line":"                \"but only had domain capabilities for: %(archs)s\","},{"line_number":5535,"context_line":"                {\u0027arch\u0027: arch, \u0027archs\u0027: \u0027 \u0027.join(domain_caps)})"},{"line_number":5536,"context_line":"            return None"},{"line_number":5537,"context_line":""},{"line_number":5538,"context_line":"        if mach_type not in domain_caps[arch]:"},{"line_number":5539,"context_line":"            LOG.warning("}],"source_content_type":"text/x-python","patch_set":53,"id":"5faad753_2a44eeb6","line":5536,"range":{"start_line":5532,"start_character":0,"end_line":5536,"end_character":23},"in_reply_to":"5faad753_5984aeb1","updated":"2019-09-10 13:37:38.000000000","message":"Yeah we could do that.","commit_id":"71316d15bc0ee1dc1afb2a76da5e0fed006c0e0e"}],"nova/virt/libvirt/utils.py":[{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"e9d35b8774b034bf7a588d1e9275f3ebc27d1771","unresolved":false,"context_lines":[{"line_number":593,"context_line":""},{"line_number":594,"context_line":""},{"line_number":595,"context_line":"def sev_enabled(host, flavor, image_meta):"},{"line_number":596,"context_line":"    supported \u003d host.supports_amd_sev"},{"line_number":597,"context_line":"    requested \u003d hardware.mem_encryption_requested(flavor, image_meta)"},{"line_number":598,"context_line":"    return supported and requested"}],"source_content_type":"text/x-python","patch_set":18,"id":"7faddb67_64ef06e9","line":596,"updated":"2019-07-17 01:22:44.000000000","message":"nits: Could you please add some notes inline? This is seems like not easy to understacd.","commit_id":"175f40b057c0c75f87f01ec30bfd084246039fa8"},{"author":{"_account_id":13478,"name":"Boris Bobrov","email":"b.bobrov@sap.com","username":"bbobrov"},"change_message_id":"5eda2a6343a4ef2a447d596d8002249bcc08998f","unresolved":false,"context_lines":[{"line_number":593,"context_line":""},{"line_number":594,"context_line":""},{"line_number":595,"context_line":"def sev_enabled(host, flavor, image_meta):"},{"line_number":596,"context_line":"    supported \u003d host.supports_amd_sev"},{"line_number":597,"context_line":"    requested \u003d hardware.mem_encryption_requested(flavor, image_meta)"},{"line_number":598,"context_line":"    return supported and requested"}],"source_content_type":"text/x-python","patch_set":18,"id":"7faddb67_883490df","line":596,"in_reply_to":"7faddb67_64ef06e9","updated":"2019-07-17 11:36:41.000000000","message":"Done","commit_id":"175f40b057c0c75f87f01ec30bfd084246039fa8"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"d7741b8a7ae3136a5f5bb56efb2b5050945d6c47","unresolved":false,"context_lines":[{"line_number":592,"context_line":"    return \"mdev_\" + mdev_uuid.replace(\u0027-\u0027, \u0027_\u0027)"},{"line_number":593,"context_line":""},{"line_number":594,"context_line":""},{"line_number":595,"context_line":"def sev_enabled(host, flavor, image_meta):"},{"line_number":596,"context_line":"    # To enable AMD SEV the following should be true:"},{"line_number":597,"context_line":"    #  a) the supports_amd_sev instance variable in the host is"},{"line_number":598,"context_line":"    #     true"}],"source_content_type":"text/x-python","patch_set":31,"id":"7faddb67_0ca230af","line":595,"range":{"start_line":595,"start_character":4,"end_line":595,"end_character":15},"updated":"2019-07-30 20:34:20.000000000","message":"trivial, yes, but this should have UT","commit_id":"4918f0b2c8992cc1befc9b5bc503873ce1b8b38a"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"790c57b347d608a783885ea2d46a8ec957f8e8bf","unresolved":false,"context_lines":[{"line_number":592,"context_line":"    return \"mdev_\" + mdev_uuid.replace(\u0027-\u0027, \u0027_\u0027)"},{"line_number":593,"context_line":""},{"line_number":594,"context_line":""},{"line_number":595,"context_line":"def sev_enabled(host, flavor, image_meta):"},{"line_number":596,"context_line":"    # To enable AMD SEV the following should be true:"},{"line_number":597,"context_line":"    #  a) the supports_amd_sev instance variable in the host is"},{"line_number":598,"context_line":"    #     true"}],"source_content_type":"text/x-python","patch_set":31,"id":"7faddb67_ab2e5889","line":595,"range":{"start_line":595,"start_character":4,"end_line":595,"end_character":15},"in_reply_to":"7faddb67_0ca230af","updated":"2019-08-15 18:23:55.000000000","message":"Done","commit_id":"4918f0b2c8992cc1befc9b5bc503873ce1b8b38a"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"d7741b8a7ae3136a5f5bb56efb2b5050945d6c47","unresolved":false,"context_lines":[{"line_number":600,"context_line":"    #     memory encryption to be enabled."},{"line_number":601,"context_line":""},{"line_number":602,"context_line":"    supported \u003d host.supports_amd_sev"},{"line_number":603,"context_line":"    requested \u003d hardware.mem_encryption_requested(flavor, image_meta)"},{"line_number":604,"context_line":"    return supported and requested"}],"source_content_type":"text/x-python","patch_set":31,"id":"7faddb67_56a679ad","line":603,"updated":"2019-07-30 20:34:20.000000000","message":"nit, this isn\u0027t a super-expensive operation, but you could potentially shortcut it by returning False immediately if you find `supported` isn\u0027t True above.","commit_id":"4918f0b2c8992cc1befc9b5bc503873ce1b8b38a"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"790c57b347d608a783885ea2d46a8ec957f8e8bf","unresolved":false,"context_lines":[{"line_number":600,"context_line":"    #     memory encryption to be enabled."},{"line_number":601,"context_line":""},{"line_number":602,"context_line":"    supported \u003d host.supports_amd_sev"},{"line_number":603,"context_line":"    requested \u003d hardware.mem_encryption_requested(flavor, image_meta)"},{"line_number":604,"context_line":"    return supported and requested"}],"source_content_type":"text/x-python","patch_set":31,"id":"7faddb67_b9c23584","line":603,"in_reply_to":"7faddb67_56a679ad","updated":"2019-08-15 18:23:55.000000000","message":"Done","commit_id":"4918f0b2c8992cc1befc9b5bc503873ce1b8b38a"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"7492ba9961c40eb4e61ff0c9cec7a07ac2a34f35","unresolved":false,"context_lines":[{"line_number":542,"context_line":"def get_machine_type(image_meta):"},{"line_number":543,"context_line":"    \"\"\"The guest machine type can be set as an image metadata property, or"},{"line_number":544,"context_line":"    otherwise based on architecture-specific defaults. If no defaults are"},{"line_number":545,"context_line":"    found then None will be returned. This will ultimately lead to QEMU using"},{"line_number":546,"context_line":"    its own default which is currently the \u0027pc\u0027 machine type."},{"line_number":547,"context_line":"    \"\"\""},{"line_number":548,"context_line":"    mach_type \u003d get_machine_type_no_default(image_meta)"},{"line_number":549,"context_line":"    if mach_type is not None:"}],"source_content_type":"text/x-python","patch_set":49,"id":"7faddb67_57840cb1","line":546,"range":{"start_line":545,"start_character":38,"end_line":546,"end_character":61},"updated":"2019-09-03 15:41:43.000000000","message":"unrelated, but this might not be true. I think there is qemu version out there with q35 as default.","commit_id":"285046f01cc5cc4f74c9ffab65c0a8074448b8fb"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"ab5fe135491642d56dc31f7401f3161e05084f8a","unresolved":false,"context_lines":[{"line_number":542,"context_line":"def get_machine_type(image_meta):"},{"line_number":543,"context_line":"    \"\"\"The guest machine type can be set as an image metadata property, or"},{"line_number":544,"context_line":"    otherwise based on architecture-specific defaults. If no defaults are"},{"line_number":545,"context_line":"    found then None will be returned. This will ultimately lead to QEMU using"},{"line_number":546,"context_line":"    its own default which is currently the \u0027pc\u0027 machine type."},{"line_number":547,"context_line":"    \"\"\""},{"line_number":548,"context_line":"    mach_type \u003d get_machine_type_no_default(image_meta)"},{"line_number":549,"context_line":"    if mach_type is not None:"}],"source_content_type":"text/x-python","patch_set":49,"id":"7faddb67_f2fbb877","line":546,"range":{"start_line":545,"start_character":38,"end_line":546,"end_character":61},"in_reply_to":"7faddb67_47920c4d","updated":"2019-09-04 09:07:23.000000000","message":"So, as noted above (see the references linked), QEMU upstream default machine type has not changed.  It is still \u0027pc\u0027","commit_id":"285046f01cc5cc4f74c9ffab65c0a8074448b8fb"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"ab5fe135491642d56dc31f7401f3161e05084f8a","unresolved":false,"context_lines":[{"line_number":542,"context_line":"def get_machine_type(image_meta):"},{"line_number":543,"context_line":"    \"\"\"The guest machine type can be set as an image metadata property, or"},{"line_number":544,"context_line":"    otherwise based on architecture-specific defaults. If no defaults are"},{"line_number":545,"context_line":"    found then None will be returned. This will ultimately lead to QEMU using"},{"line_number":546,"context_line":"    its own default which is currently the \u0027pc\u0027 machine type."},{"line_number":547,"context_line":"    \"\"\""},{"line_number":548,"context_line":"    mach_type \u003d get_machine_type_no_default(image_meta)"},{"line_number":549,"context_line":"    if mach_type is not None:"}],"source_content_type":"text/x-python","patch_set":49,"id":"7faddb67_92d124fb","line":546,"range":{"start_line":545,"start_character":38,"end_line":546,"end_character":61},"in_reply_to":"7faddb67_57840cb1","updated":"2019-09-04 09:07:23.000000000","message":"\u003e unrelated, but this might not be true. I think there is qemu\n \u003e version out there with q35 as default.\n\nNo, there\u0027s no upstream QEMU version with \u0027q35\u0027 as the default.  It was discussed at length (see this thread[1][2], but the QEMU default was *not* changed for various reasons.\n\n(IOW, the doc string as it exists is correct :-))\n\n[1] https://lists.nongnu.org/archive/html/qemu-devel/2018-06/msg00605.html\n\n[2] https://lists.nongnu.org/archive/html/qemu-devel/2018-06/msg00702.html","commit_id":"285046f01cc5cc4f74c9ffab65c0a8074448b8fb"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"2fe8ed783a148a4e793a73de92f84ee308652990","unresolved":false,"context_lines":[{"line_number":542,"context_line":"def get_machine_type(image_meta):"},{"line_number":543,"context_line":"    \"\"\"The guest machine type can be set as an image metadata property, or"},{"line_number":544,"context_line":"    otherwise based on architecture-specific defaults. If no defaults are"},{"line_number":545,"context_line":"    found then None will be returned. This will ultimately lead to QEMU using"},{"line_number":546,"context_line":"    its own default which is currently the \u0027pc\u0027 machine type."},{"line_number":547,"context_line":"    \"\"\""},{"line_number":548,"context_line":"    mach_type \u003d get_machine_type_no_default(image_meta)"},{"line_number":549,"context_line":"    if mach_type is not None:"}],"source_content_type":"text/x-python","patch_set":49,"id":"7faddb67_bdfd2109","line":546,"range":{"start_line":545,"start_character":38,"end_line":546,"end_character":61},"in_reply_to":"7faddb67_57840cb1","updated":"2019-09-03 16:14:51.000000000","message":"That information came from Kashyap.  I was aware of plans to change the default from \u0027pc\u0027 to \u0027q35\u0027, which is the reason for this:\n\n  https://bugs.launchpad.net/nova/+bug/1780138\n\nbut I didn\u0027t think that change had happened yet.  Maybe it happened very recently?","commit_id":"285046f01cc5cc4f74c9ffab65c0a8074448b8fb"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"b31dffa929617b0331140a86cef723d7e64b1875","unresolved":false,"context_lines":[{"line_number":542,"context_line":"def get_machine_type(image_meta):"},{"line_number":543,"context_line":"    \"\"\"The guest machine type can be set as an image metadata property, or"},{"line_number":544,"context_line":"    otherwise based on architecture-specific defaults. If no defaults are"},{"line_number":545,"context_line":"    found then None will be returned. This will ultimately lead to QEMU using"},{"line_number":546,"context_line":"    its own default which is currently the \u0027pc\u0027 machine type."},{"line_number":547,"context_line":"    \"\"\""},{"line_number":548,"context_line":"    mach_type \u003d get_machine_type_no_default(image_meta)"},{"line_number":549,"context_line":"    if mach_type is not None:"}],"source_content_type":"text/x-python","patch_set":49,"id":"7faddb67_47920c4d","line":546,"range":{"start_line":545,"start_character":38,"end_line":546,"end_character":61},"in_reply_to":"7faddb67_bdfd2109","updated":"2019-09-04 08:00:29.000000000","message":"I think Kashyap has the best knowledge about these topics so I defer this to him.","commit_id":"285046f01cc5cc4f74c9ffab65c0a8074448b8fb"}],"nova/virt/libvirt/vif.py":[{"author":{"_account_id":13478,"name":"Boris Bobrov","email":"b.bobrov@sap.com","username":"bbobrov"},"change_message_id":"0a7fa8c45c0384ea9076b33ba90c7ca7e24fa6b8","unresolved":false,"context_lines":[{"line_number":575,"context_line":"        return conf"},{"line_number":576,"context_line":""},{"line_number":577,"context_line":"    def get_config(self, instance, vif, image_meta,"},{"line_number":578,"context_line":"                   inst_type, virt_type, host, sev_required):"},{"line_number":579,"context_line":"        vif_type \u003d vif[\u0027type\u0027]"},{"line_number":580,"context_line":"        vnic_type \u003d vif[\u0027vnic_type\u0027]"},{"line_number":581,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"9fb8cfa7_89670252","line":578,"updated":"2019-06-24 10:39:16.000000000","message":"initially (see patchset 5) i had the check that SEV is required inside this method. The reason was that get_config() was involved hot-, cold-plugging, and the initial creation of the vm, making it hard to trace and fix all usages of this method.\n\nIf you really mind re-checking that sev is required in this method, i suggest to remove the sev-related bits from here completely and move them to the places where get_config() is getting called.\n\nPersonally, i think duplicating the check, like it was in ps5, was OK, given how simple it is with harware.mem_encryption_requested().\n\nWhat do you think?","commit_id":"fac775447be02f23f6fbff15cc57295dac8b3679"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"3f6451093441c245f3d96111d351dde05283fd55","unresolved":false,"context_lines":[{"line_number":575,"context_line":"        return conf"},{"line_number":576,"context_line":""},{"line_number":577,"context_line":"    def get_config(self, instance, vif, image_meta,"},{"line_number":578,"context_line":"                   inst_type, virt_type, host, sev_required):"},{"line_number":579,"context_line":"        vif_type \u003d vif[\u0027type\u0027]"},{"line_number":580,"context_line":"        vnic_type \u003d vif[\u0027vnic_type\u0027]"},{"line_number":581,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"9fb8cfa7_766d5001","line":578,"in_reply_to":"9fb8cfa7_34861e68","updated":"2019-07-01 20:41:41.000000000","message":"\u003e I\u0027m OK with that but please check with Eric first.\n\nYeah, I don\u0027t mind redoing the check if it\u0027s cheap, but let\u0027s put it into a helper method (even if it turns out to be a one-liner) that we call from all the places.\n\n \u003e  Also I think\n \u003e your suggestion of moving the host detection from driver.py to\n \u003e host.py might be good, but again Eric will be able to advise on\n \u003e that.\n\nSorry, what suggestion, where? (Though in general putting a thing called \"host detection\" into a module called \"host.py\" sounds sensible to me...)","commit_id":"fac775447be02f23f6fbff15cc57295dac8b3679"},{"author":{"_account_id":13478,"name":"Boris Bobrov","email":"b.bobrov@sap.com","username":"bbobrov"},"change_message_id":"7087b2e4f11bf1519ad9e31a6a8a06846e11eb93","unresolved":false,"context_lines":[{"line_number":575,"context_line":"        return conf"},{"line_number":576,"context_line":""},{"line_number":577,"context_line":"    def get_config(self, instance, vif, image_meta,"},{"line_number":578,"context_line":"                   inst_type, virt_type, host, sev_required):"},{"line_number":579,"context_line":"        vif_type \u003d vif[\u0027type\u0027]"},{"line_number":580,"context_line":"        vnic_type \u003d vif[\u0027vnic_type\u0027]"},{"line_number":581,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"7faddb67_da3fce22","line":578,"in_reply_to":"9fb8cfa7_766d5001","updated":"2019-07-15 19:15:32.000000000","message":"\u003e Yeah, I don\u0027t mind redoing the check if it\u0027s cheap, but let\u0027s put it into a helper method (even if it turns out to be a one-liner) that we call from all the places.\n\nI ended up putting it to utils.py\n\n\u003e Sorry, what suggestion, where? (Though in general putting a thing called \"host detection\" into a module called \"host.py\" sounds sensible to me...)\n\nSorry, it was briefly discussed with Adam in our internal chat, but never articulated here.\n\nThe idea was to move the code that detects whether SEV is supported from driver.py to host.py, so that it could be reused. I did it in https://review.opendev.org/#/c/664420/, PS 17. It was suggested to prevent changing signature of LibvirtGenericVIFDriver:get_config, because it would require a lot of work to fix usage of it, and we already have all the required bits inside get_config to understand whether SEV should be enabled.","commit_id":"fac775447be02f23f6fbff15cc57295dac8b3679"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"de6cd5a97a232b09766ca29c79bf84bc3e640198","unresolved":false,"context_lines":[{"line_number":575,"context_line":"        return conf"},{"line_number":576,"context_line":""},{"line_number":577,"context_line":"    def get_config(self, instance, vif, image_meta,"},{"line_number":578,"context_line":"                   inst_type, virt_type, host, sev_required):"},{"line_number":579,"context_line":"        vif_type \u003d vif[\u0027type\u0027]"},{"line_number":580,"context_line":"        vnic_type \u003d vif[\u0027vnic_type\u0027]"},{"line_number":581,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"9fb8cfa7_34861e68","line":578,"in_reply_to":"9fb8cfa7_89670252","updated":"2019-06-27 19:51:15.000000000","message":"I\u0027m OK with that but please check with Eric first.  Also I think your suggestion of moving the host detection from driver.py to host.py might be good, but again Eric will be able to advise on that.","commit_id":"fac775447be02f23f6fbff15cc57295dac8b3679"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"d7741b8a7ae3136a5f5bb56efb2b5050945d6c47","unresolved":false,"context_lines":[{"line_number":588,"context_line":"            sev_enabled \u003d libvirt_utils.sev_enabled(host, inst_type,"},{"line_number":589,"context_line":"                                                    image_meta)"},{"line_number":590,"context_line":"            if conf and conf.model and \u0027virtio\u0027 in conf.model and sev_enabled:"},{"line_number":591,"context_line":"                conf.driver_iommu \u003d True"},{"line_number":592,"context_line":"            return conf"},{"line_number":593,"context_line":""},{"line_number":594,"context_line":"        # Legacy non-os-vif codepath"}],"source_content_type":"text/x-python","patch_set":31,"id":"7faddb67_0c1b1081","line":591,"updated":"2019-07-30 20:34:20.000000000","message":"tested ✔","commit_id":"4918f0b2c8992cc1befc9b5bc503873ce1b8b38a"}]}