)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"7c103348bd2a63a2ca14b49ac8a2d9c53c9c42f2","unresolved":true,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The \u0027virt_type\u0027 is always going to be accessible from config, while the"},{"line_number":10,"context_line":"\u0027caps\u0027 is accessible from (cached) instance information. There\u0027s no need"},{"line_number":11,"context_line":"to pass these things around."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Blueprint: allow-secure-boot-for-qemu-kvm-guests"},{"line_number":14,"context_line":"Change-Id: Ieeca47ab38c3c9d81814d2f58e5548ae9f3a3963"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"665a6867_0ae6e0c8","line":11,"updated":"2021-03-09 10:27:39.000000000","message":"While I agree, I think we shouldn\u0027t verify the config option directly unless it could be mutable (ie. in case of the operator changes the option value, would we have problems or would it be OK ?)","commit_id":"94d16e7c772b022dce85bb55c028b4956d11e924"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"cdf6bf0f4af658f4ffb887aae9884183aecfcb06","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The \u0027virt_type\u0027 is always going to be accessible from config, while the"},{"line_number":10,"context_line":"\u0027caps\u0027 is accessible from (cached) instance information. There\u0027s no need"},{"line_number":11,"context_line":"to pass these things around."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Blueprint: allow-secure-boot-for-qemu-kvm-guests"},{"line_number":14,"context_line":"Change-Id: Ieeca47ab38c3c9d81814d2f58e5548ae9f3a3963"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"f151ce83_1064adfe","line":11,"in_reply_to":"665a6867_0ae6e0c8","updated":"2021-03-09 11:19:45.000000000","message":"See my previous reply to lyarwood [1]. I don\u0027t think this is an issue. In short, we already have a dependency on this configuration option in many places so we\u0027re simply doing more of the same. Ideally we should make this specific to a guest but we currently don\u0027t properly support guest-specific architectures, let alone guest-specific virt types.\n\n[1] https://review.opendev.org/c/openstack/nova/+/775689/3/nova/virt/libvirt/driver.py#6055","commit_id":"94d16e7c772b022dce85bb55c028b4956d11e924"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"a95a28478a4fe1756532fa499a28112676695c4e","unresolved":true,"context_lines":[{"line_number":6052,"context_line":"        vpmems \u003d self._get_ordered_vpmems(instance, flavor)"},{"line_number":6053,"context_line":""},{"line_number":6054,"context_line":"        guest \u003d vconfig.LibvirtConfigGuest()"},{"line_number":6055,"context_line":"        guest.virt_type \u003d CONF.libvirt.virt_type"},{"line_number":6056,"context_line":"        guest.name \u003d instance.name"},{"line_number":6057,"context_line":"        guest.uuid \u003d instance.uuid"},{"line_number":6058,"context_line":"        # We are using default unit for memory: KiB"}],"source_content_type":"text/x-python","patch_set":3,"id":"4e428d17_dc642655","line":6055,"range":{"start_line":6055,"start_character":8,"end_line":6055,"end_character":48},"updated":"2021-03-04 17:25:58.000000000","message":"So from here why don\u0027t we just ensure guest is passed around?\n\nUsing CONF.libvirt.virt_type when we have guest everywhere just seems wrong and something someone will eventually have to revert if we ever make this dynamic, some instances using qemu while others use kvm etc.","commit_id":"745e21b1fc6e1635c7b358f02c9c78fe4096a0b4"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"d8ae84e4c16f18a9886b50ebd6c7d7695832b0b3","unresolved":true,"context_lines":[{"line_number":6052,"context_line":"        vpmems \u003d self._get_ordered_vpmems(instance, flavor)"},{"line_number":6053,"context_line":""},{"line_number":6054,"context_line":"        guest \u003d vconfig.LibvirtConfigGuest()"},{"line_number":6055,"context_line":"        guest.virt_type \u003d CONF.libvirt.virt_type"},{"line_number":6056,"context_line":"        guest.name \u003d instance.name"},{"line_number":6057,"context_line":"        guest.uuid \u003d instance.uuid"},{"line_number":6058,"context_line":"        # We are using default unit for memory: KiB"}],"source_content_type":"text/x-python","patch_set":3,"id":"52e42d31_1ef378df","line":6055,"range":{"start_line":6055,"start_character":8,"end_line":6055,"end_character":48},"in_reply_to":"4e428d17_dc642655","updated":"2021-03-05 15:54:13.000000000","message":"I was concerned that doing so would make all our other calls dependent on this running first, meaning they couldn\u0027t really act independently i.e. by passing them an empty \u0027vconfig.LibvirtConfigGuest()\u0027 instance. It\u0027s minor concern but seeing as we use the config option elsewhere (see below), I figured I might as well do the more correct thing here too.\n\nMore to the point though, we already have a lot of users of \u0027CONF.libvirt.virt_type\u0027, including some places where the guest isn\u0027t in scope currently and might not even make sense:\n\n  $ ag CONF.libvirt.virt_type nova/virt/libvirt/ --no-heading --no-break | wc -l\n  76\n\nWhile I agree that this will need to be changed if we ever want to make \u0027virt_type\u0027 configurable (perhaps by using the \u0027img_hv_type\u0027 image metadata prop), we\u0027d have to fix all of those other uses also. There\u0027s likely a whole lot other work needed here too, and the feature itself seems unlikely/low priority so perhaps what I\u0027ve done is \"good enough (TM)\"?","commit_id":"745e21b1fc6e1635c7b358f02c9c78fe4096a0b4"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"1d167457902a0782235815ea9b9a4e12281ba897","unresolved":false,"context_lines":[{"line_number":6052,"context_line":"        vpmems \u003d self._get_ordered_vpmems(instance, flavor)"},{"line_number":6053,"context_line":""},{"line_number":6054,"context_line":"        guest \u003d vconfig.LibvirtConfigGuest()"},{"line_number":6055,"context_line":"        guest.virt_type \u003d CONF.libvirt.virt_type"},{"line_number":6056,"context_line":"        guest.name \u003d instance.name"},{"line_number":6057,"context_line":"        guest.uuid \u003d instance.uuid"},{"line_number":6058,"context_line":"        # We are using default unit for memory: KiB"}],"source_content_type":"text/x-python","patch_set":3,"id":"1db155aa_ccd73d9a","line":6055,"range":{"start_line":6055,"start_character":8,"end_line":6055,"end_character":48},"in_reply_to":"52e42d31_1ef378df","updated":"2021-03-05 16:09:12.000000000","message":"Okay fair, I was under the impression that this was always being done first but after looking again I can see that isn\u0027t always the case. For now this is fine and others can look into passing it around via guest if we ever make it per instance configurable.","commit_id":"745e21b1fc6e1635c7b358f02c9c78fe4096a0b4"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"a95a28478a4fe1756532fa499a28112676695c4e","unresolved":true,"context_lines":[{"line_number":6184,"context_line":"            self._guest_add_mdevs(guest, mdevs)"},{"line_number":6185,"context_line":""},{"line_number":6186,"context_line":"        if sev_enabled:"},{"line_number":6187,"context_line":"            caps \u003d self._host.get_capabilities()"},{"line_number":6188,"context_line":"            self._guest_configure_sev(guest, caps.host.cpu.arch,"},{"line_number":6189,"context_line":"                                      guest.os_mach_type)"},{"line_number":6190,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"a3d08e0d_9ca24b66","line":6187,"range":{"start_line":6187,"start_character":12,"end_line":6187,"end_character":48},"updated":"2021-03-04 17:25:58.000000000","message":"Can you avoid repeat calls to get_capabilities in _get_guest_config?\n\n    def _get_guest_config\n        [..]\n        caps \u003d None\n        [..]\n        if something_enabled:\n            if caps is None:\n                caps \u003d self._host.get_capabilities()\n            self._enable_something(caps)","commit_id":"745e21b1fc6e1635c7b358f02c9c78fe4096a0b4"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"d8ae84e4c16f18a9886b50ebd6c7d7695832b0b3","unresolved":true,"context_lines":[{"line_number":6184,"context_line":"            self._guest_add_mdevs(guest, mdevs)"},{"line_number":6185,"context_line":""},{"line_number":6186,"context_line":"        if sev_enabled:"},{"line_number":6187,"context_line":"            caps \u003d self._host.get_capabilities()"},{"line_number":6188,"context_line":"            self._guest_configure_sev(guest, caps.host.cpu.arch,"},{"line_number":6189,"context_line":"                                      guest.os_mach_type)"},{"line_number":6190,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"d36de241_1294dbb0","line":6187,"range":{"start_line":6187,"start_character":12,"end_line":6187,"end_character":48},"in_reply_to":"a3d08e0d_9ca24b66","updated":"2021-03-05 15:54:13.000000000","message":"I can, but why? We\u0027re already memoizing the call [1] and I actually changed that slightly here to make it even more obvious.\n\n[1] https://github.com/openstack/nova/blob/a65e4201cc03f89d37296ddb803934c5a7977a71/nova/virt/libvirt/host.py#L742","commit_id":"745e21b1fc6e1635c7b358f02c9c78fe4096a0b4"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"1d167457902a0782235815ea9b9a4e12281ba897","unresolved":false,"context_lines":[{"line_number":6184,"context_line":"            self._guest_add_mdevs(guest, mdevs)"},{"line_number":6185,"context_line":""},{"line_number":6186,"context_line":"        if sev_enabled:"},{"line_number":6187,"context_line":"            caps \u003d self._host.get_capabilities()"},{"line_number":6188,"context_line":"            self._guest_configure_sev(guest, caps.host.cpu.arch,"},{"line_number":6189,"context_line":"                                      guest.os_mach_type)"},{"line_number":6190,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"5c1bc160_7e9c41bb","line":6187,"range":{"start_line":6187,"start_character":12,"end_line":6187,"end_character":48},"in_reply_to":"d36de241_1294dbb0","updated":"2021-03-05 16:09:12.000000000","message":"Gah sorry I thought I had checked for caching before writing this but obviously not. As written is fine.","commit_id":"745e21b1fc6e1635c7b358f02c9c78fe4096a0b4"}]}