)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"e21c58637e576eb40a142f0e38c53b7085c8f4d2","unresolved":true,"context_lines":[{"line_number":10,"context_line":"enabled instance must have the iommu attribute enabled. This was done"},{"line_number":11,"context_line":"within the original implementation of the spec for all virtio devices"},{"line_number":12,"context_line":"defined when initially spawning the instance but does not include volume"},{"line_number":13,"context_line":"and interfaces that are later hot plugged."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"This change corrects this and in doing so slightly refactors the"},{"line_number":16,"context_line":"original designer code to make it usable in both cases."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"013a4c3e_7263ea1d","line":13,"updated":"2021-06-07 14:12:46.000000000","message":"ahah, gotcha.","commit_id":"46ae3c8c1a1f4c285ff4cc9392212af5ce740c90"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"e21c58637e576eb40a142f0e38c53b7085c8f4d2","unresolved":true,"context_lines":[{"line_number":16,"context_line":"original designer code to make it usable in both cases."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"[1] https://specs.openstack.org/openstack/nova-specs/specs/train/implemented/amd-sev-libvirt-support.html#proposed-change"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"Closes-Bug: #1930734"},{"line_number":21,"context_line":"Change-Id: I11131a3f90b8af85e7151b519fb26d225629c391"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"46a77da4_09ac4e45","line":19,"updated":"2021-06-07 14:12:46.000000000","message":"fwiw, I\u0027d somehow have appreciated if we could verify the regression and the fix by some functional tests (even if not in this change) but AFAICS, there are no existing func tests and it would need some time for adding them.","commit_id":"46ae3c8c1a1f4c285ff4cc9392212af5ce740c90"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"6e9cd01559177bddcc305237decfcb56feeb929c","unresolved":true,"context_lines":[{"line_number":12,"context_line":"defined when initially spawning the instance but does not include volume"},{"line_number":13,"context_line":"and interfaces that are later hot plugged."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"This change corrects this for both volumes and nics and in doing so"},{"line_number":16,"context_line":"slightly refactors the original designer code to make it usable in both"},{"line_number":17,"context_line":"cases."},{"line_number":18,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"9cca8d4e_028eaa1b","line":15,"range":{"start_line":15,"start_character":47,"end_line":15,"end_character":51},"updated":"2021-06-08 10:53:35.000000000","message":"NICs","commit_id":"4d8bf15fec15dc3416023e577e0f2c277c216506"}],"nova/tests/unit/virt/libvirt/test_driver.py":[{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"8006a79b1df45e28e25765da2b3260f3ace64714","unresolved":true,"context_lines":[{"line_number":8846,"context_line":"            connection_info,"},{"line_number":8847,"context_line":"            mock.sentinel.disk_info)"},{"line_number":8848,"context_line":"        mock_set_cache_mode.assert_called_once_with(returned_config)"},{"line_number":8849,"context_line":"        self.assertEqual(generated_config.to_xml(), returned_config.to_xml())"},{"line_number":8850,"context_line":""},{"line_number":8851,"context_line":"    @mock.patch.object(libvirt_driver.LibvirtDriver, \u0027_sev_enabled\u0027,"},{"line_number":8852,"context_line":"                       new\u003dmock.Mock(return_value\u003dTrue))"}],"source_content_type":"text/x-python","patch_set":2,"id":"93663ef5_f9255ced","line":8849,"updated":"2021-06-04 10:22:38.000000000","message":"I guess if I had to fine one nit with this patch, it\u0027s that we\u0027re not asserting that we *haven\u0027t* added driver_iommu in cases when we shouldn\u0027t.","commit_id":"60450d8514f6c0411458beb3653d232dda2621f7"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"8006a79b1df45e28e25765da2b3260f3ace64714","unresolved":true,"context_lines":[{"line_number":1884,"context_line":"        vol_driver \u003d self._get_volume_driver(connection_info)"},{"line_number":1885,"context_line":"        conf \u003d vol_driver.get_config(connection_info, disk_info)"},{"line_number":1886,"context_line":""},{"line_number":1887,"context_line":"        if self._sev_enabled(instance.flavor, instance.image_meta):"},{"line_number":1888,"context_line":"            designer.set_driver_iommu_for_device(conf)"},{"line_number":1889,"context_line":""},{"line_number":1890,"context_line":"        self._set_cache_mode(conf)"}],"source_content_type":"text/x-python","patch_set":2,"id":"a3ea25e7_af423a2f","line":1887,"updated":"2021-06-04 10:22:38.000000000","message":"Let\u0027s not debate whether this belongs in the main attach_volume(), or here. Both are fine, this is probably a tiny bit cleaner.","commit_id":"60450d8514f6c0411458beb3653d232dda2621f7"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"8006a79b1df45e28e25765da2b3260f3ace64714","unresolved":true,"context_lines":[{"line_number":2695,"context_line":"                                         instance.flavor,"},{"line_number":2696,"context_line":"                                         CONF.libvirt.virt_type)"},{"line_number":2697,"context_line":""},{"line_number":2698,"context_line":"        if self._sev_enabled(instance.flavor, image_meta):"},{"line_number":2699,"context_line":"            designer.set_driver_iommu_for_device(cfg)"},{"line_number":2700,"context_line":""},{"line_number":2701,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":2,"id":"a8132dd5_1fcd3531","line":2698,"updated":"2021-06-04 10:22:38.000000000","message":"(But it definitely doesn\u0027t make sense to move this into the vif driver\u0027s get_config)","commit_id":"60450d8514f6c0411458beb3653d232dda2621f7"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"e21c58637e576eb40a142f0e38c53b7085c8f4d2","unresolved":true,"context_lines":[{"line_number":1885,"context_line":"        conf \u003d vol_driver.get_config(connection_info, disk_info)"},{"line_number":1886,"context_line":""},{"line_number":1887,"context_line":"        if self._sev_enabled(instance.flavor, instance.image_meta):"},{"line_number":1888,"context_line":"            designer.set_driver_iommu_for_device(conf)"},{"line_number":1889,"context_line":""},{"line_number":1890,"context_line":"        self._set_cache_mode(conf)"},{"line_number":1891,"context_line":"        return conf"}],"source_content_type":"text/x-python","patch_set":3,"id":"ad66758a_c3d233aa","line":1888,"updated":"2021-06-07 14:12:46.000000000","message":"fwiw, I would have preferred to look over all existing devices later and just trigger _guest_configure_sev() instead of enabling IOMMU per device but I guess this sounds reasonable here at the least modified path.","commit_id":"46ae3c8c1a1f4c285ff4cc9392212af5ce740c90"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"e21c58637e576eb40a142f0e38c53b7085c8f4d2","unresolved":true,"context_lines":[{"line_number":2696,"context_line":"                                         CONF.libvirt.virt_type)"},{"line_number":2697,"context_line":""},{"line_number":2698,"context_line":"        if self._sev_enabled(instance.flavor, image_meta):"},{"line_number":2699,"context_line":"            designer.set_driver_iommu_for_device(cfg)"},{"line_number":2700,"context_line":""},{"line_number":2701,"context_line":"        try:"},{"line_number":2702,"context_line":"            state \u003d guest.get_power_state(self._host)"}],"source_content_type":"text/x-python","patch_set":3,"id":"f8be5c50_b52ad2fd","line":2699,"updated":"2021-06-07 14:12:46.000000000","message":"here, surely we need to enable IOMMU on the fly per device, since it\u0027s a bind.","commit_id":"46ae3c8c1a1f4c285ff4cc9392212af5ce740c90"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"1825c7f243ec2f8b59bdbf0d78aeeeca0805509a","unresolved":true,"context_lines":[{"line_number":9542,"context_line":"                    # TODO(sahid): It\u0027s not a really good idea to pass"},{"line_number":9543,"context_line":"                    # the method _get_volume_config and we should to find"},{"line_number":9544,"context_line":"                    # a way to avoid this in future."},{"line_number":9545,"context_line":"                    guest, migrate_data, self._get_volume_config,"},{"line_number":9546,"context_line":"                    get_vif_config\u003dget_vif_config, new_resources\u003dnew_resources)"},{"line_number":9547,"context_line":""},{"line_number":9548,"context_line":"            # NOTE(pkoniszewski): Because of precheck which blocks"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa1dc3a7_3d892a54","line":9545,"updated":"2021-06-07 15:04:41.000000000","message":"This is what\u0027s passed to get_updated_guest_xml as the get_volume_config() method, so I think you\u0027re good.","commit_id":"46ae3c8c1a1f4c285ff4cc9392212af5ce740c90"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"90f1908780818b3b9a1c43b89bf2beb998749f87","unresolved":false,"context_lines":[{"line_number":9542,"context_line":"                    # TODO(sahid): It\u0027s not a really good idea to pass"},{"line_number":9543,"context_line":"                    # the method _get_volume_config and we should to find"},{"line_number":9544,"context_line":"                    # a way to avoid this in future."},{"line_number":9545,"context_line":"                    guest, migrate_data, self._get_volume_config,"},{"line_number":9546,"context_line":"                    get_vif_config\u003dget_vif_config, new_resources\u003dnew_resources)"},{"line_number":9547,"context_line":""},{"line_number":9548,"context_line":"            # NOTE(pkoniszewski): Because of precheck which blocks"}],"source_content_type":"text/x-python","patch_set":3,"id":"02fe433d_055f6b21","line":9545,"in_reply_to":"fa1dc3a7_3d892a54","updated":"2021-06-07 15:33:06.000000000","message":"Right the issue was that it would then call this without instance below:\n\nhttps://github.com/openstack/nova/blob/1a1a6a042c5b2295c7ab38b0387f3c234a704462/nova/virt/libvirt/migration.py#L211-L212\n\nI\u0027m not sure why this only failed in the LC job tbh but it should be resolved now, I\u0027ll also look at the above TODO this cycle.","commit_id":"46ae3c8c1a1f4c285ff4cc9392212af5ce740c90"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"6e9cd01559177bddcc305237decfcb56feeb929c","unresolved":false,"context_lines":[{"line_number":1882,"context_line":""},{"line_number":1883,"context_line":"    def _get_volume_config(self, instance, connection_info, disk_info):"},{"line_number":1884,"context_line":"        vol_driver \u003d self._get_volume_driver(connection_info)"},{"line_number":1885,"context_line":"        conf \u003d vol_driver.get_config(connection_info, disk_info)"},{"line_number":1886,"context_line":""},{"line_number":1887,"context_line":"        if self._sev_enabled(instance.flavor, instance.image_meta):"},{"line_number":1888,"context_line":"            designer.set_driver_iommu_for_device(conf)"}],"source_content_type":"text/x-python","patch_set":6,"id":"a61d27fb_0c37162c","line":1885,"updated":"2021-06-08 10:53:35.000000000","message":"Yup, only instance of a call to this API here","commit_id":"4d8bf15fec15dc3416023e577e0f2c277c216506"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"6e9cd01559177bddcc305237decfcb56feeb929c","unresolved":false,"context_lines":[{"line_number":2691,"context_line":"        guest \u003d self._host.get_guest(instance)"},{"line_number":2692,"context_line":""},{"line_number":2693,"context_line":"        self.vif_driver.plug(instance, vif)"},{"line_number":2694,"context_line":"        cfg \u003d self.vif_driver.get_config(instance, vif, image_meta,"},{"line_number":2695,"context_line":"                                         instance.flavor,"},{"line_number":2696,"context_line":"                                         CONF.libvirt.virt_type)"},{"line_number":2697,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"62b8b69f_1f2a0409","line":2694,"updated":"2021-06-08 10:53:35.000000000","message":"There are a couple of instances of this, but this is the only one that\u0027s relevant I think. The others are:\n\n- for detach, from \u0027detach_interface\u0027: not relevant cos it\u0027s detach\n- for PCI passthrough attach, from \u0027_attach_direct_passthrough_ports\u0027: not relevant since these clearly can\u0027t be virtio devices\n- for initial init, from \u0027_get_guest_config\u0027: already handled via a later call to \u0027_guest_configure_sev\u0027 in that method\n- for live migration, from \u0027_live_migration_operation\u0027: already handled via the later call to \u0027get_updated_guest_xml\u0027, which you\u0027ve updated here","commit_id":"4d8bf15fec15dc3416023e577e0f2c277c216506"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"d1acf073d9036c363b604c1e7ec940fb3b1edf25","unresolved":false,"context_lines":[{"line_number":2691,"context_line":"        guest \u003d self._host.get_guest(instance)"},{"line_number":2692,"context_line":""},{"line_number":2693,"context_line":"        self.vif_driver.plug(instance, vif)"},{"line_number":2694,"context_line":"        cfg \u003d self.vif_driver.get_config(instance, vif, image_meta,"},{"line_number":2695,"context_line":"                                         instance.flavor,"},{"line_number":2696,"context_line":"                                         CONF.libvirt.virt_type)"},{"line_number":2697,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"21174765_817e81a2","line":2694,"in_reply_to":"62b8b69f_1f2a0409","updated":"2021-06-08 11:07:29.000000000","message":"so in a follow up i want to move this call into the config generation in vif.py as i discussed with lee but for back porting i think we are ok with this. for now.","commit_id":"4d8bf15fec15dc3416023e577e0f2c277c216506"}],"nova/virt/libvirt/migration.py":[{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"1308bf8148536c3ba4930f42743a5fda6a798cdd","unresolved":true,"context_lines":[{"line_number":193,"context_line":"    return xml_doc"},{"line_number":194,"context_line":""},{"line_number":195,"context_line":""},{"line_number":196,"context_line":"def _update_volume_xml(xml_doc, migrate_data, instance, get_volume_config):"},{"line_number":197,"context_line":"    \"\"\"Update XML using device information of destination host.\"\"\""},{"line_number":198,"context_line":"    migrate_bdm_info \u003d migrate_data.bdms"},{"line_number":199,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"93c706c8_f2b99a8e","line":196,"updated":"2021-06-07 15:35:42.000000000","message":"*facepalm* Right, you meant the method signature.\n\nAs an aside, this kind of \"hidden\" coupling, where we\u0027re passing a method down but still expect the eventual caller to know its arguments is really ugly. I wonder if we could turn it into a functools.partial or something.","commit_id":"1f421d47aeea31964800857c4be26d077a2bc17a"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"7a99a7e3d409a6d626c6d4adc333a2c7531e9492","unresolved":false,"context_lines":[{"line_number":193,"context_line":"    return xml_doc"},{"line_number":194,"context_line":""},{"line_number":195,"context_line":""},{"line_number":196,"context_line":"def _update_volume_xml(xml_doc, migrate_data, instance, get_volume_config):"},{"line_number":197,"context_line":"    \"\"\"Update XML using device information of destination host.\"\"\""},{"line_number":198,"context_line":"    migrate_bdm_info \u003d migrate_data.bdms"},{"line_number":199,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"f5132f30_ecd30a84","line":196,"in_reply_to":"93c706c8_f2b99a8e","updated":"2021-06-07 15:43:28.000000000","message":"Yeah it\u0027s horrid and needs to be refactored, I was just going to generate the required volume configs in the driver and pass them down into this method tbh.","commit_id":"1f421d47aeea31964800857c4be26d077a2bc17a"}]}