)]}'
{"nova/tests/unit/virt/libvirt/test_host.py":[{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"dde2555b40299a166aec4c3445c111fa32954e0e","unresolved":true,"context_lines":[{"line_number":1493,"context_line":"        loader \u003d self.host.get_loader(\u0027x86_64\u0027, \u0027pc\u0027, has_secure_boot\u003dFalse)"},{"line_number":1494,"context_line":"        self.assertIsNotNone(loader)"},{"line_number":1495,"context_line":""},{"line_number":1496,"context_line":"        # While it should fail here since we want it now"},{"line_number":1497,"context_line":"        self.assertRaises("},{"line_number":1498,"context_line":"            exception.UEFINotSupported,"},{"line_number":1499,"context_line":"            self.host.get_loader,"},{"line_number":1500,"context_line":"            \u0027x86_64\u0027, \u0027pc\u0027, has_secure_boot\u003dTrue)"},{"line_number":1501,"context_line":""},{"line_number":1502,"context_line":""},{"line_number":1503,"context_line":"vc \u003d fakelibvirt.virConnect"}],"source_content_type":"text/x-python","patch_set":1,"id":"334eec61_9b8e7ba8","line":1500,"range":{"start_line":1496,"start_character":0,"end_line":1500,"end_character":49},"updated":"2021-03-09 11:49:06.000000000","message":"Can you update the returned loader and assert the positive case with has_secure_boot\u003dTrue here?","commit_id":"bd14adbe3f19cd0562e13f3b9df8c48ae96c5ccd"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"9bb9ef653da1207afd06038dbb4bf495b88d8422","unresolved":false,"context_lines":[{"line_number":1493,"context_line":"        loader \u003d self.host.get_loader(\u0027x86_64\u0027, \u0027pc\u0027, has_secure_boot\u003dFalse)"},{"line_number":1494,"context_line":"        self.assertIsNotNone(loader)"},{"line_number":1495,"context_line":""},{"line_number":1496,"context_line":"        # While it should fail here since we want it now"},{"line_number":1497,"context_line":"        self.assertRaises("},{"line_number":1498,"context_line":"            exception.UEFINotSupported,"},{"line_number":1499,"context_line":"            self.host.get_loader,"},{"line_number":1500,"context_line":"            \u0027x86_64\u0027, \u0027pc\u0027, has_secure_boot\u003dTrue)"},{"line_number":1501,"context_line":""},{"line_number":1502,"context_line":""},{"line_number":1503,"context_line":"vc \u003d fakelibvirt.virConnect"}],"source_content_type":"text/x-python","patch_set":1,"id":"2daea9ed_028c8cb1","line":1500,"range":{"start_line":1496,"start_character":0,"end_line":1500,"end_character":49},"in_reply_to":"334eec61_9b8e7ba8","updated":"2021-03-09 12:37:33.000000000","message":"Done","commit_id":"bd14adbe3f19cd0562e13f3b9df8c48ae96c5ccd"}],"nova/virt/libvirt/host.py":[{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"dde2555b40299a166aec4c3445c111fa32954e0e","unresolved":true,"context_lines":[{"line_number":151,"context_line":"        self._libvirt_proxy_classes \u003d self._get_libvirt_proxy_classes(libvirt)"},{"line_number":152,"context_line":"        self._libvirt_proxy \u003d self._wrap_libvirt_proxy(libvirt)"},{"line_number":153,"context_line":""},{"line_number":154,"context_line":"        self._loaders: ty.List[dict] \u003d None"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"        # A number of features are conditional on support in the hardware,"},{"line_number":157,"context_line":"        # kernel, QEMU, and/or libvirt. These are determined on demand and"}],"source_content_type":"text/x-python","patch_set":1,"id":"aebd1d7c_746473f2","line":154,"in_reply_to":"375e1f6e_f9f54806","updated":"2021-03-09 11:49:06.000000000","message":"\u003e pep8: error: Incompatible types in assignment (expression has type \"None\", variable has type \"List[Dict[Any, Any]]\")\n\nPlease fix.","commit_id":"bd14adbe3f19cd0562e13f3b9df8c48ae96c5ccd"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"9bb9ef653da1207afd06038dbb4bf495b88d8422","unresolved":false,"context_lines":[{"line_number":151,"context_line":"        self._libvirt_proxy_classes \u003d self._get_libvirt_proxy_classes(libvirt)"},{"line_number":152,"context_line":"        self._libvirt_proxy \u003d self._wrap_libvirt_proxy(libvirt)"},{"line_number":153,"context_line":""},{"line_number":154,"context_line":"        self._loaders: ty.List[dict] \u003d None"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"        # A number of features are conditional on support in the hardware,"},{"line_number":157,"context_line":"        # kernel, QEMU, and/or libvirt. These are determined on demand and"}],"source_content_type":"text/x-python","patch_set":1,"id":"41fd5ab4_dab3dbdf","line":154,"in_reply_to":"aebd1d7c_746473f2","updated":"2021-03-09 12:37:33.000000000","message":"Done","commit_id":"bd14adbe3f19cd0562e13f3b9df8c48ae96c5ccd"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"dde2555b40299a166aec4c3445c111fa32954e0e","unresolved":true,"context_lines":[{"line_number":1301,"context_line":"                continue"},{"line_number":1302,"context_line":""},{"line_number":1303,"context_line":"            for domain in guest.domains:"},{"line_number":1304,"context_line":"                if machine in guest.domains[domain].machines:"},{"line_number":1305,"context_line":"                    return machine"},{"line_number":1306,"context_line":""},{"line_number":1307,"context_line":"                if machine in guest.domains[domain].aliases:"}],"source_content_type":"text/x-python","patch_set":1,"id":"eee84a97_48ec383f","line":1304,"range":{"start_line":1304,"start_character":30,"end_line":1304,"end_character":60},"updated":"2021-03-09 11:49:06.000000000","message":"reviewer note - nova.virt.libvirt.config.LibvirtConfigCapsGuestDomain","commit_id":"bd14adbe3f19cd0562e13f3b9df8c48ae96c5ccd"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"ebd6fddb63f9e9f705d4a1154b4cb7ae60a8b5e0","unresolved":false,"context_lines":[{"line_number":1301,"context_line":"                continue"},{"line_number":1302,"context_line":""},{"line_number":1303,"context_line":"            for domain in guest.domains:"},{"line_number":1304,"context_line":"                if machine in guest.domains[domain].machines:"},{"line_number":1305,"context_line":"                    return machine"},{"line_number":1306,"context_line":""},{"line_number":1307,"context_line":"                if machine in guest.domains[domain].aliases:"}],"source_content_type":"text/x-python","patch_set":1,"id":"5e04429b_f843a3d0","line":1304,"range":{"start_line":1304,"start_character":30,"end_line":1304,"end_character":60},"in_reply_to":"eee84a97_48ec383f","updated":"2021-03-09 11:52:33.000000000","message":"Apologies these should be marked as resolved.","commit_id":"bd14adbe3f19cd0562e13f3b9df8c48ae96c5ccd"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"dde2555b40299a166aec4c3445c111fa32954e0e","unresolved":true,"context_lines":[{"line_number":1304,"context_line":"                if machine in guest.domains[domain].machines:"},{"line_number":1305,"context_line":"                    return machine"},{"line_number":1306,"context_line":""},{"line_number":1307,"context_line":"                if machine in guest.domains[domain].aliases:"},{"line_number":1308,"context_line":"                    return guest.domains[domain].aliases[machine][\u0027canonical\u0027]"},{"line_number":1309,"context_line":""},{"line_number":1310,"context_line":"        msg \u003d _(\u0027Invalid machine type: %s\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"bd92a8b5_608c16c3","line":1307,"range":{"start_line":1307,"start_character":30,"end_line":1307,"end_character":59},"updated":"2021-03-09 11:49:06.000000000","message":"as above","commit_id":"bd14adbe3f19cd0562e13f3b9df8c48ae96c5ccd"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"ebd6fddb63f9e9f705d4a1154b4cb7ae60a8b5e0","unresolved":false,"context_lines":[{"line_number":1304,"context_line":"                if machine in guest.domains[domain].machines:"},{"line_number":1305,"context_line":"                    return machine"},{"line_number":1306,"context_line":""},{"line_number":1307,"context_line":"                if machine in guest.domains[domain].aliases:"},{"line_number":1308,"context_line":"                    return guest.domains[domain].aliases[machine][\u0027canonical\u0027]"},{"line_number":1309,"context_line":""},{"line_number":1310,"context_line":"        msg \u003d _(\u0027Invalid machine type: %s\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"a8094e48_c0c19915","line":1307,"range":{"start_line":1307,"start_character":30,"end_line":1307,"end_character":59},"in_reply_to":"bd92a8b5_608c16c3","updated":"2021-03-09 11:52:33.000000000","message":"As above.","commit_id":"bd14adbe3f19cd0562e13f3b9df8c48ae96c5ccd"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"dde2555b40299a166aec4c3445c111fa32954e0e","unresolved":true,"context_lines":[{"line_number":1451,"context_line":"        This should be removed when libvirt correctly supports switching"},{"line_number":1452,"context_line":"        between loaders with or without secure boot enable."},{"line_number":1453,"context_line":""},{"line_number":1454,"context_line":"        [1] https://github.com/qemu/qemu/blob/v5.2.0/docs/interop/firmware.json"},{"line_number":1455,"context_line":"        [2] https://bugzilla.redhat.com/show_bug.cgi?id\u003d1906500"},{"line_number":1456,"context_line":""},{"line_number":1457,"context_line":"        :returns: An ordered list of loader configuration dictionaries."},{"line_number":1458,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":1,"id":"bfd0dc2d_0ba1f558","line":1455,"range":{"start_line":1454,"start_character":0,"end_line":1455,"end_character":63},"updated":"2021-03-09 11:49:06.000000000","message":"nit - I guess you wanted to reference these in specific places above, something to add if you respin.","commit_id":"bd14adbe3f19cd0562e13f3b9df8c48ae96c5ccd"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"9bb9ef653da1207afd06038dbb4bf495b88d8422","unresolved":false,"context_lines":[{"line_number":1451,"context_line":"        This should be removed when libvirt correctly supports switching"},{"line_number":1452,"context_line":"        between loaders with or without secure boot enable."},{"line_number":1453,"context_line":""},{"line_number":1454,"context_line":"        [1] https://github.com/qemu/qemu/blob/v5.2.0/docs/interop/firmware.json"},{"line_number":1455,"context_line":"        [2] https://bugzilla.redhat.com/show_bug.cgi?id\u003d1906500"},{"line_number":1456,"context_line":""},{"line_number":1457,"context_line":"        :returns: An ordered list of loader configuration dictionaries."},{"line_number":1458,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":1,"id":"43f6e8f9_671cd564","line":1455,"range":{"start_line":1454,"start_character":0,"end_line":1455,"end_character":63},"in_reply_to":"bfd0dc2d_0ba1f558","updated":"2021-03-09 12:37:33.000000000","message":"Done","commit_id":"bd14adbe3f19cd0562e13f3b9df8c48ae96c5ccd"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"422fad055c9bf80b63c5d8f415f896b59a75085a","unresolved":true,"context_lines":[{"line_number":28,"context_line":"\"\"\""},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"from collections import defaultdict"},{"line_number":31,"context_line":"import fnmatch"},{"line_number":32,"context_line":"import glob"},{"line_number":33,"context_line":"import inspect"},{"line_number":34,"context_line":"import operator"}],"source_content_type":"text/x-python","patch_set":2,"id":"d26d386f_287832e2","line":31,"updated":"2021-03-11 10:16:20.000000000","message":"see my comment on L1487 about the usage of stdlib vs. oslo.utils but I think we\u0027re all good.","commit_id":"faad45b6323d7c52d35b7ccc45eacb5580b3b4d3"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"422fad055c9bf80b63c5d8f415f896b59a75085a","unresolved":true,"context_lines":[{"line_number":90,"context_line":"    \u0027/usr/share/qemu/firmware\u0027,"},{"line_number":91,"context_line":"    \u0027/etc/qemu/firmware\u0027,"},{"line_number":92,"context_line":"    # we intentionally ignore \u0027$XDG_CONFIG_HOME/qemu/firmware\u0027"},{"line_number":93,"context_line":"]"},{"line_number":94,"context_line":""},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"def _get_loaders():"}],"source_content_type":"text/x-python","patch_set":2,"id":"7767386f_98357d2e","line":93,"updated":"2021-03-11 10:16:20.000000000","message":"we reproduce the pattern from AMD SEV, I think we\u0027re fine to just have a global object instead of a specific config option since the files shouldn\u0027t be different between OS distributions.","commit_id":"faad45b6323d7c52d35b7ccc45eacb5580b3b4d3"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"fc635057e6493eeeade4d88e8f5c1e5478448588","unresolved":true,"context_lines":[{"line_number":93,"context_line":"]"},{"line_number":94,"context_line":""},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"def _get_loaders():"},{"line_number":97,"context_line":"    if not any("},{"line_number":98,"context_line":"        os.path.exists(path) for path in QEMU_FIRMWARE_DESCRIPTOR_PATHS"},{"line_number":99,"context_line":"    ):"}],"source_content_type":"text/x-python","patch_set":2,"id":"73286c46_1319cbf5","line":96,"range":{"start_line":96,"start_character":4,"end_line":96,"end_character":16},"updated":"2021-03-10 13:53:02.000000000","message":"Suggest to rename this as _get_loader_and_nvram().\n\nI think (and I see the evidence in the test in test_host.py) you\u0027re using the word \"loader[s]\" — here and throughout — to loosely refer to both the OVMF binary blob and the \"VARS\" (VARiableS) file[*], OVMF_CODE.secboot.fd and OVMF_VARS.secboot.fd, respectively.\n\nHowever, as you notice in the guest XML, in libvirt parlance \"loader\" XML element is used for the OVMF binary blob; while the VARS is handled by the \"nvram\" element.\n\n[*] The VARS file a non-volatile memory store that contain OVMF \"variables\" e.g. the keys and certificates enrolled, and other boot-level customizations.","commit_id":"faad45b6323d7c52d35b7ccc45eacb5580b3b4d3"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"422fad055c9bf80b63c5d8f415f896b59a75085a","unresolved":true,"context_lines":[{"line_number":93,"context_line":"]"},{"line_number":94,"context_line":""},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"def _get_loaders():"},{"line_number":97,"context_line":"    if not any("},{"line_number":98,"context_line":"        os.path.exists(path) for path in QEMU_FIRMWARE_DESCRIPTOR_PATHS"},{"line_number":99,"context_line":"    ):"}],"source_content_type":"text/x-python","patch_set":2,"id":"19edbbcd_80edc28f","line":96,"range":{"start_line":96,"start_character":4,"end_line":96,"end_character":16},"in_reply_to":"73286c46_1319cbf5","updated":"2021-03-11 10:16:20.000000000","message":"Just a nit IMHO for a follow-up.","commit_id":"faad45b6323d7c52d35b7ccc45eacb5580b3b4d3"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"422fad055c9bf80b63c5d8f415f896b59a75085a","unresolved":true,"context_lines":[{"line_number":98,"context_line":"        os.path.exists(path) for path in QEMU_FIRMWARE_DESCRIPTOR_PATHS"},{"line_number":99,"context_line":"    ):"},{"line_number":100,"context_line":"        msg \u003d _(\"Failed to locate firmware descriptor files\")"},{"line_number":101,"context_line":"        raise exception.InternalError(msg)"},{"line_number":102,"context_line":""},{"line_number":103,"context_line":"    _loaders \u003d []"},{"line_number":104,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"1da081f3_7fd8652a","line":101,"updated":"2021-03-11 10:16:20.000000000","message":"Well, looks like we don\u0027t hard stop with this exception https://github.com/openstack/nova/blob/88ea40c1f86b7f92a3a96c56a8d1a29a93abeb2c/nova/virt/libvirt/driver.py#L3675\nbut I could be wrong.\n\nI\u0027m torn whether we should stop here the service or just not accepting the guests.","commit_id":"faad45b6323d7c52d35b7ccc45eacb5580b3b4d3"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"d23c02a476df403b7fdcdb45d88e4d0b9b632dee","unresolved":true,"context_lines":[{"line_number":98,"context_line":"        os.path.exists(path) for path in QEMU_FIRMWARE_DESCRIPTOR_PATHS"},{"line_number":99,"context_line":"    ):"},{"line_number":100,"context_line":"        msg \u003d _(\"Failed to locate firmware descriptor files\")"},{"line_number":101,"context_line":"        raise exception.InternalError(msg)"},{"line_number":102,"context_line":""},{"line_number":103,"context_line":"    _loaders \u003d []"},{"line_number":104,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"d65e8fa9_b2770d40","line":101,"in_reply_to":"1da081f3_7fd8652a","updated":"2021-03-11 10:58:10.000000000","message":"I\u0027m not sure what your point is by referencing that code, it doesn\u0027t access Host.loaders at the moment (or in the future of the series AFAICT).\n\nAnyway I don\u0027t think we should stop the service, Host.loaders access is going to be specific to instance requests that have in turn requested secure boot so if that fails we should fail the associated request and not the service.","commit_id":"faad45b6323d7c52d35b7ccc45eacb5580b3b4d3"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"0ee40a0297a506eda0da3c4050ccb576873a56a4","unresolved":true,"context_lines":[{"line_number":98,"context_line":"        os.path.exists(path) for path in QEMU_FIRMWARE_DESCRIPTOR_PATHS"},{"line_number":99,"context_line":"    ):"},{"line_number":100,"context_line":"        msg \u003d _(\"Failed to locate firmware descriptor files\")"},{"line_number":101,"context_line":"        raise exception.InternalError(msg)"},{"line_number":102,"context_line":""},{"line_number":103,"context_line":"    _loaders \u003d []"},{"line_number":104,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"16e16f1f_a455a1c2","line":101,"in_reply_to":"d65e8fa9_b2770d40","updated":"2021-03-11 13:56:12.000000000","message":"OK, after thinking it more, I agree with not stopping.","commit_id":"faad45b6323d7c52d35b7ccc45eacb5580b3b4d3"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"fc635057e6493eeeade4d88e8f5c1e5478448588","unresolved":true,"context_lines":[{"line_number":100,"context_line":"        msg \u003d _(\"Failed to locate firmware descriptor files\")"},{"line_number":101,"context_line":"        raise exception.InternalError(msg)"},{"line_number":102,"context_line":""},{"line_number":103,"context_line":"    _loaders \u003d []"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"    for path in QEMU_FIRMWARE_DESCRIPTOR_PATHS:"},{"line_number":106,"context_line":"        if not os.path.exists(path):"}],"source_content_type":"text/x-python","patch_set":2,"id":"590d6488_517e729a","line":103,"range":{"start_line":103,"start_character":4,"end_line":103,"end_character":12},"updated":"2021-03-10 13:53:02.000000000","message":"Here too: _loader_and_nvram (or even better: _ovmf_binary_and_nvram)","commit_id":"faad45b6323d7c52d35b7ccc45eacb5580b3b4d3"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"f39a800b070288e11e8b39d4fba4c8c2d1c94426","unresolved":true,"context_lines":[{"line_number":1286,"context_line":"        \"\"\"Resolve a machine type to its canonical representation."},{"line_number":1287,"context_line":""},{"line_number":1288,"context_line":"        Libvirt supports machine type aliases. On an x86 host the \u0027pc\u0027 machine"},{"line_number":1289,"context_line":"        type is an alias for e.g. \u0027pc-1440fx-5.1\u0027. Resolve the provided machine"},{"line_number":1290,"context_line":"        type to its canonical representation so that it can be used for other"},{"line_number":1291,"context_line":"        operations."},{"line_number":1292,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"efc969ae_d56ea7a8","line":1289,"updated":"2021-03-10 11:02:01.000000000","message":"I\u0027d rewrite the above sentence to the effect that the \u0027pc\u0027 (and \u0027q35\u0027) aliases map on to the latest QEMU version that is installed on the host. \n\nThe version you cite above (typo: \"pc-1440fx-5.1\" --\u003e \"pc-i440fx-5.1\") is based on Fedora-33:\n\n    $\u003e rpm -q qemu-system-x86\n    qemu-system-x86-5.1.0-7.fc33.x86_64\n\n    $\u003e qemu-system-x86_64 -machine help | grep \"alias of pc\"\n    pc                   Standard PC (i440FX + PIIX, 1996) (alias of pc-i440fx-5.1)\n    q35                  Standard PC (Q35 + ICH9, 2009) (alias of pc-q35-5.1)","commit_id":"faad45b6323d7c52d35b7ccc45eacb5580b3b4d3"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"4c1146b0a19e92fd787f8bd56acb1914797d0999","unresolved":true,"context_lines":[{"line_number":1286,"context_line":"        \"\"\"Resolve a machine type to its canonical representation."},{"line_number":1287,"context_line":""},{"line_number":1288,"context_line":"        Libvirt supports machine type aliases. On an x86 host the \u0027pc\u0027 machine"},{"line_number":1289,"context_line":"        type is an alias for e.g. \u0027pc-1440fx-5.1\u0027. Resolve the provided machine"},{"line_number":1290,"context_line":"        type to its canonical representation so that it can be used for other"},{"line_number":1291,"context_line":"        operations."},{"line_number":1292,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"97006a63_eae45285","line":1289,"in_reply_to":"efc969ae_d56ea7a8","updated":"2021-03-10 12:02:39.000000000","message":"I figured that\u0027s what I was saying. Perhaps it\u0027s clearer if I expand out the e.g.?\n\n  On an x86 host, the \u0027pc\u0027 machine type is an alias for, for example, \u0027pc-i1440fx-5.1\u0027.\n\nI can mention that this is derived from QEMU in a follow-up though","commit_id":"faad45b6323d7c52d35b7ccc45eacb5580b3b4d3"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"422fad055c9bf80b63c5d8f415f896b59a75085a","unresolved":true,"context_lines":[{"line_number":1308,"context_line":"                    return guest.domains[domain].aliases[machine][\u0027canonical\u0027]"},{"line_number":1309,"context_line":""},{"line_number":1310,"context_line":"        msg \u003d _(\u0027Invalid machine type: %s\u0027)"},{"line_number":1311,"context_line":"        raise exception.InternalError(msg % machine)"},{"line_number":1312,"context_line":""},{"line_number":1313,"context_line":"    @property"},{"line_number":1314,"context_line":"    def has_hyperthreading(self) -\u003e bool:"}],"source_content_type":"text/x-python","patch_set":2,"id":"83405bce_5a590225","line":1311,"updated":"2021-03-11 10:16:20.000000000","message":"again, hard-stop AFAIK. LGTM.","commit_id":"faad45b6323d7c52d35b7ccc45eacb5580b3b4d3"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"d23c02a476df403b7fdcdb45d88e4d0b9b632dee","unresolved":true,"context_lines":[{"line_number":1308,"context_line":"                    return guest.domains[domain].aliases[machine][\u0027canonical\u0027]"},{"line_number":1309,"context_line":""},{"line_number":1310,"context_line":"        msg \u003d _(\u0027Invalid machine type: %s\u0027)"},{"line_number":1311,"context_line":"        raise exception.InternalError(msg % machine)"},{"line_number":1312,"context_line":""},{"line_number":1313,"context_line":"    @property"},{"line_number":1314,"context_line":"    def has_hyperthreading(self) -\u003e bool:"}],"source_content_type":"text/x-python","patch_set":2,"id":"330bd9f7_dde5513c","line":1311,"in_reply_to":"83405bce_5a590225","updated":"2021-03-11 10:58:10.000000000","message":"As above, I don\u0027t think this should stop the service just the request.","commit_id":"faad45b6323d7c52d35b7ccc45eacb5580b3b4d3"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"fc635057e6493eeeade4d88e8f5c1e5478448588","unresolved":true,"context_lines":[{"line_number":1444,"context_line":"    def loaders(self) -\u003e ty.List[dict]:"},{"line_number":1445,"context_line":"        \"\"\"Retrieve details of loader configuration for the host."},{"line_number":1446,"context_line":""},{"line_number":1447,"context_line":"        Inspect the firmware metadata files provided by QEMU [1] to retrieve"},{"line_number":1448,"context_line":"        information about the firmware supported by this host. Note that most"},{"line_number":1449,"context_line":"        distros only publish this information for UEFI loaders currently."},{"line_number":1450,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"106098d4_9c07cd1e","line":1447,"updated":"2021-03-10 13:53:02.000000000","message":"Here too: \"provided by QEMU\" can give the impression that the distros pick their OVMF binaries directly from QEMU. As you see, QEMU only provides the firmware interface (and templates), and each distro customizes them as per their EDK2/OVMF builds.  For example SUSE (SLES) generates far more variants (for no good reason, actually) of OVMF binaries than any of Debian or Fedora.  Also each OVMF binary is unique to a given distro, as they\u0027re signed by a distro-specific key.\n\nSo, suggest to use:\n\n    Inspect the firmware metadata files shipped provided by a Linux distro\u0027s \n    OVMF/EDK2 package to retrieve details [...]","commit_id":"faad45b6323d7c52d35b7ccc45eacb5580b3b4d3"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"fc635057e6493eeeade4d88e8f5c1e5478448588","unresolved":true,"context_lines":[{"line_number":1446,"context_line":""},{"line_number":1447,"context_line":"        Inspect the firmware metadata files provided by QEMU [1] to retrieve"},{"line_number":1448,"context_line":"        information about the firmware supported by this host. Note that most"},{"line_number":1449,"context_line":"        distros only publish this information for UEFI loaders currently."},{"line_number":1450,"context_line":""},{"line_number":1451,"context_line":"        This should be removed when libvirt correctly supports switching"},{"line_number":1452,"context_line":"        between loaders with or without secure boot enabled [2]."}],"source_content_type":"text/x-python","patch_set":2,"id":"9302bab2_dc108fc7","line":1449,"range":{"start_line":1449,"start_character":50,"end_line":1449,"end_character":62},"updated":"2021-03-10 13:53:02.000000000","message":"s/UEFI loaders/UEFI binaries/\n\n(I think you mean, only UEFI, but not—yet—SeaBIOS, et al)","commit_id":"faad45b6323d7c52d35b7ccc45eacb5580b3b4d3"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"422fad055c9bf80b63c5d8f415f896b59a75085a","unresolved":true,"context_lines":[{"line_number":1446,"context_line":""},{"line_number":1447,"context_line":"        Inspect the firmware metadata files provided by QEMU [1] to retrieve"},{"line_number":1448,"context_line":"        information about the firmware supported by this host. Note that most"},{"line_number":1449,"context_line":"        distros only publish this information for UEFI loaders currently."},{"line_number":1450,"context_line":""},{"line_number":1451,"context_line":"        This should be removed when libvirt correctly supports switching"},{"line_number":1452,"context_line":"        between loaders with or without secure boot enabled [2]."}],"source_content_type":"text/x-python","patch_set":2,"id":"8c183d24_1c93b96c","line":1449,"range":{"start_line":1449,"start_character":50,"end_line":1449,"end_character":62},"in_reply_to":"9302bab2_dc108fc7","updated":"2021-03-11 10:16:20.000000000","message":"looks good to me for a follow-up","commit_id":"faad45b6323d7c52d35b7ccc45eacb5580b3b4d3"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"fc635057e6493eeeade4d88e8f5c1e5478448588","unresolved":true,"context_lines":[{"line_number":1482,"context_line":"                    continue"},{"line_number":1483,"context_line":""},{"line_number":1484,"context_line":"                for machine_glob in target[\u0027machines\u0027]:"},{"line_number":1485,"context_line":"                    # the \u0027machines\u0027 attribute supports glob patterns (e.g."},{"line_number":1486,"context_line":"                    # \u0027pc-q35-*\u0027) so we need to resolve these"},{"line_number":1487,"context_line":"                    if fnmatch.fnmatch(machine, machine_glob):"},{"line_number":1488,"context_line":"                        break"}],"source_content_type":"text/x-python","patch_set":2,"id":"9043a6b7_97892306","line":1485,"range":{"start_line":1485,"start_character":56,"end_line":1485,"end_character":69},"updated":"2021-03-10 13:53:02.000000000","message":"Thanks for handling this!","commit_id":"faad45b6323d7c52d35b7ccc45eacb5580b3b4d3"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"fae104844ba493314a06e1b4b1d174635714ab59","unresolved":false,"context_lines":[{"line_number":1482,"context_line":"                    continue"},{"line_number":1483,"context_line":""},{"line_number":1484,"context_line":"                for machine_glob in target[\u0027machines\u0027]:"},{"line_number":1485,"context_line":"                    # the \u0027machines\u0027 attribute supports glob patterns (e.g."},{"line_number":1486,"context_line":"                    # \u0027pc-q35-*\u0027) so we need to resolve these"},{"line_number":1487,"context_line":"                    if fnmatch.fnmatch(machine, machine_glob):"},{"line_number":1488,"context_line":"                        break"}],"source_content_type":"text/x-python","patch_set":2,"id":"c026f549_c92fa686","line":1485,"range":{"start_line":1485,"start_character":56,"end_line":1485,"end_character":69},"in_reply_to":"9043a6b7_97892306","updated":"2021-03-15 12:48:58.000000000","message":"Ack","commit_id":"faad45b6323d7c52d35b7ccc45eacb5580b3b4d3"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"422fad055c9bf80b63c5d8f415f896b59a75085a","unresolved":true,"context_lines":[{"line_number":1484,"context_line":"                for machine_glob in target[\u0027machines\u0027]:"},{"line_number":1485,"context_line":"                    # the \u0027machines\u0027 attribute supports glob patterns (e.g."},{"line_number":1486,"context_line":"                    # \u0027pc-q35-*\u0027) so we need to resolve these"},{"line_number":1487,"context_line":"                    if fnmatch.fnmatch(machine, machine_glob):"},{"line_number":1488,"context_line":"                        break"},{"line_number":1489,"context_line":"                else:"},{"line_number":1490,"context_line":"                    continue"}],"source_content_type":"text/x-python","patch_set":2,"id":"00ca4ff3_f15b6b76","line":1487,"updated":"2021-03-11 10:16:20.000000000","message":"this comes from py3 stdlib, so we don\u0027t need to have a reqs.txt dependency here.\n\nFWIW, a thread-safe implementation of fnmatch was made in oslo.utils as the Py2 stdlib fnmatch wasn\u0027t, but given we now no longer support py2, I think we can use the py3 stdlib version.","commit_id":"faad45b6323d7c52d35b7ccc45eacb5580b3b4d3"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"fae104844ba493314a06e1b4b1d174635714ab59","unresolved":false,"context_lines":[{"line_number":1484,"context_line":"                for machine_glob in target[\u0027machines\u0027]:"},{"line_number":1485,"context_line":"                    # the \u0027machines\u0027 attribute supports glob patterns (e.g."},{"line_number":1486,"context_line":"                    # \u0027pc-q35-*\u0027) so we need to resolve these"},{"line_number":1487,"context_line":"                    if fnmatch.fnmatch(machine, machine_glob):"},{"line_number":1488,"context_line":"                        break"},{"line_number":1489,"context_line":"                else:"},{"line_number":1490,"context_line":"                    continue"}],"source_content_type":"text/x-python","patch_set":2,"id":"b7727a12_f4f0caf7","line":1487,"in_reply_to":"00ca4ff3_f15b6b76","updated":"2021-03-15 12:48:58.000000000","message":"Ack","commit_id":"faad45b6323d7c52d35b7ccc45eacb5580b3b4d3"}]}