)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"87c8064c7ae9e2b0790a5961b03e84a8198d64a6","unresolved":true,"context_lines":[{"line_number":19,"context_line":"- Else, well, let\u0027s at least try; log that, and good luck!"},{"line_number":20,"context_line":"  (Maybe we should error out? but this is stable-only..)"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"[stable-only]:"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"Wallaby and later are fixed with refactoring, as part of"},{"line_number":25,"context_line":"the Secure Boot implementation, but that\u0027s big/risky; see"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"9ca15fc5_4b08d4ad","line":22,"range":{"start_line":22,"start_character":0,"end_line":22,"end_character":14},"updated":"2022-04-12 10:02:33.000000000","message":"nit this does not need to be repeated.","commit_id":"ebed454cd75db5fe20250e2029854977062c0f0f"},{"author":{"_account_id":28621,"name":"Mauricio Faria de Oliveira","email":"mfo@canonical.com","username":"mfo"},"change_message_id":"9c180f9da89fc0ab0088af555f99d76f054539ef","unresolved":false,"context_lines":[{"line_number":19,"context_line":"- Else, well, let\u0027s at least try; log that, and good luck!"},{"line_number":20,"context_line":"  (Maybe we should error out? but this is stable-only..)"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"[stable-only]:"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"Wallaby and later are fixed with refactoring, as part of"},{"line_number":25,"context_line":"the Secure Boot implementation, but that\u0027s big/risky; see"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"15c47be2_76c810ff","line":22,"range":{"start_line":22,"start_character":0,"end_line":22,"end_character":14},"in_reply_to":"9ca15fc5_4b08d4ad","updated":"2022-04-12 13:32:30.000000000","message":"Ack","commit_id":"ebed454cd75db5fe20250e2029854977062c0f0f"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":28621,"name":"Mauricio Faria de Oliveira","email":"mfo@canonical.com","username":"mfo"},"change_message_id":"355cb204d4c413698922f41d85e123c395f8a956","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"612fe552_d706cd9d","updated":"2022-02-21 18:14:04.000000000","message":"for doc purposes,\n\nthe only (voting) failure is:\n\u003e nova-multi-cell https://zuul.opendev.org/t/openstack/build/bd06aa0ddcec4edc9f5445ef97f90b2a : FAILURE in 1h 08m 51s\n\nwhich is auth fail due to ssh timeout (tempest.lib.exceptions.SSHTimeout) in security groups testing, unrelated to this change.\n\n\n\n","commit_id":"84383af704acda4f94a21fec065d5e6c6c5f80c9"},{"author":{"_account_id":28621,"name":"Mauricio Faria de Oliveira","email":"mfo@canonical.com","username":"mfo"},"change_message_id":"368063fdb3620b2c08d37d706cae4e81edff5c63","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"8cb068cf_f7b7b282","updated":"2022-02-21 18:14:40.000000000","message":"recheck","commit_id":"84383af704acda4f94a21fec065d5e6c6c5f80c9"},{"author":{"_account_id":28621,"name":"Mauricio Faria de Oliveira","email":"mfo@canonical.com","username":"mfo"},"change_message_id":"a3ae184e0c2981164ebeeef9be039931f3cb2f59","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"3e827413_3c7227c7","in_reply_to":"8cb068cf_f7b7b282","updated":"2022-02-21 19:47:04.000000000","message":"Done","commit_id":"84383af704acda4f94a21fec065d5e6c6c5f80c9"},{"author":{"_account_id":28621,"name":"Mauricio Faria de Oliveira","email":"mfo@canonical.com","username":"mfo"},"change_message_id":"fcab835bad5486d7a74fdc7ad5193a60a24e81b3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"5239d830_03cadb83","updated":"2022-04-19 18:39:35.000000000","message":"Thanks for reviewing, Dan!\nAddressing your comments.","commit_id":"ebed454cd75db5fe20250e2029854977062c0f0f"},{"author":{"_account_id":28621,"name":"Mauricio Faria de Oliveira","email":"mfo@canonical.com","username":"mfo"},"change_message_id":"9c180f9da89fc0ab0088af555f99d76f054539ef","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"a642cc66_7ddc2f2e","updated":"2022-04-12 13:32:30.000000000","message":"Thanks for reviewing, and for the suggestions/corrections if a respin is needed; all noted!","commit_id":"ebed454cd75db5fe20250e2029854977062c0f0f"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"19c2e4822b7677441385eb769dc49c57bb3e61aa","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"760b1d92_3d5b3967","updated":"2022-04-19 19:13:01.000000000","message":"Thinking about this more, I think the current change here is really a bit roundabout, still looping through all firmware files and then trying to warn (but not block) at the end.\n\nIt seems like the real bug here is \"we should never have considered the secboot file if platform\u003d\u003dpc\". If that\u0027s the case, it seems like the better algorithm here should be something like:\n\n firmwares \u003d NON_SECBOOT_FIRMWARE_PATH_LIST\n if platform \u003d\u003d \u0027q35\u0027:\n     firmwares.append(SECBOOT_FIRMWARE_PATH)\n for firmware in firmwares:\n     ...\n\nSo that we\u0027re not even iterating the list with the secboot file in it if we know it can\u0027t work on our platform.\n\nEither way, I\u0027m still pretty meh about applying this to such old branches at this point, but I\u0027ll defer to people with stronger opinions.","commit_id":"ebed454cd75db5fe20250e2029854977062c0f0f"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f3df1e81e5e471fa05c0b218c59c3a9cc30fe9a9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"7f4d655c_f2af6b94","updated":"2022-04-19 18:28:20.000000000","message":"This feels like something that\u0027s maybe too invasive for something as old as Ussuri and Victoria, as it\u0027s not a regression, not security, etc. I also question whether it\u0027s generally applicable or specific to where that file will be on ubuntu. Wouldn\u0027t it make more sense to push this into the ubuntu package as an almost-feature-backport sort of thing?","commit_id":"ebed454cd75db5fe20250e2029854977062c0f0f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"87c8064c7ae9e2b0790a5961b03e84a8198d64a6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"8ee1303c_1f2a266d","updated":"2022-04-12 10:02:33.000000000","message":"some minior nits but having discussed this with you i agree that this should be done as a stable only change.","commit_id":"ebed454cd75db5fe20250e2029854977062c0f0f"},{"author":{"_account_id":28621,"name":"Mauricio Faria de Oliveira","email":"mfo@canonical.com","username":"mfo"},"change_message_id":"fcab835bad5486d7a74fdc7ad5193a60a24e81b3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"5e265827_adb6f67d","in_reply_to":"7f4d655c_f2af6b94","updated":"2022-04-19 18:39:35.000000000","message":"Yup, I tried to be conservative w/ the changes; happy to use another structure/take suggestions!\n\nThis is generally applicable / not specific to Ubuntu -- any distro/install with the .secboot.fd file will hit this issue.\n\nHaving this downstream in Ubuntu is indeed an option, but offering this upstream as other distros might hit this (and, honestly, to ask for review/opinions from other developers on good ways to do it) seemed a natural path forward.","commit_id":"ebed454cd75db5fe20250e2029854977062c0f0f"},{"author":{"_account_id":28621,"name":"Mauricio Faria de Oliveira","email":"mfo@canonical.com","username":"mfo"},"change_message_id":"1ea1661d35f1761a3986e6f9846f19f5bd5e2e67","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":5,"id":"17361aef_e4b8a606","updated":"2022-06-14 15:59:35.000000000","message":"Hi Sean, Dan,\n\nSuggestions implemented! Does this look good to you?\n\nThanks!","commit_id":"6d487369125166e96bd514593d6933d5ae1dbfb6"},{"author":{"_account_id":28621,"name":"Mauricio Faria de Oliveira","email":"mfo@canonical.com","username":"mfo"},"change_message_id":"7f79a3628e1726ecef3cd210c8ee29fae0eeb487","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"1161c0f7_b62ce2c9","updated":"2022-06-13 18:12:06.000000000","message":"recheck","commit_id":"6d487369125166e96bd514593d6933d5ae1dbfb6"},{"author":{"_account_id":28621,"name":"Mauricio Faria de Oliveira","email":"mfo@canonical.com","username":"mfo"},"change_message_id":"382f29a40f41e29ed1827ce9366cc6bf50055a82","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"2b1d2347_815ea00a","updated":"2022-06-13 14:32:17.000000000","message":"recheck","commit_id":"6d487369125166e96bd514593d6933d5ae1dbfb6"},{"author":{"_account_id":33350,"name":"Silvano Nogueira Buback","email":"silvano.buback@workday.com","username":"snbuback"},"change_message_id":"684fa79f7a11415105cc98aeee54055f425be214","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"7f14235e_8e8af872","updated":"2023-09-12 15:39:07.000000000","message":"I have tested this patch in my company and it is working like a charm.\nBoth UEFI and legacy images are working.","commit_id":"75e30a5a8cdd5343e65c14dbdc306ec3efee74bd"},{"author":{"_account_id":11805,"name":"Corey Bryant","email":"corey.bryant@canonical.com","username":"coreycb"},"change_message_id":"99457769c40e459fa3a32f966c4451903140d134","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"1fff51d2_feeaf896","updated":"2023-07-26 18:51:55.000000000","message":"This seems like a reasonably safe approach to fixing this. Adding my +1 but ideally I\u0027d really like to see someone with more knowledge in the area review it before we carry it in the Ubuntu package.","commit_id":"75e30a5a8cdd5343e65c14dbdc306ec3efee74bd"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"ba6b7a60c0c9337ab4fa24e04f305c42ec4cbe07","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"37467482_553c3861","updated":"2023-09-13 13:54:09.000000000","message":"i think this looks reasonable","commit_id":"75e30a5a8cdd5343e65c14dbdc306ec3efee74bd"},{"author":{"_account_id":28621,"name":"Mauricio Faria de Oliveira","email":"mfo@canonical.com","username":"mfo"},"change_message_id":"1fa16878fc2e0cd60cbd28f170378abf87989c6e","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":6,"id":"2a4e7581_ed8f8bcc","in_reply_to":"1fff51d2_feeaf896","updated":"2023-07-26 18:59:47.000000000","message":"Hi Corey,\n\nThanks for reviewing!\n\n\u003e This seems like a reasonably safe approach to fixing this. Adding my +1 but ideally I\u0027d really like to see someone with more knowledge in the area review it before we carry it in the Ubuntu package.\n\nIn the end, the (downstream) version I proposed in LP: #1960758 is (very) different than this one, it\u0027s evolved and more generic, specially as `q35` showed to be impacted as well (this patch is only for `pc`).\n\nIf you\u0027d like upstream to take a look at it too, I\u0027m happy to send another revision here; please just let me know.\n\ncheers,\nMauricio","commit_id":"75e30a5a8cdd5343e65c14dbdc306ec3efee74bd"},{"author":{"_account_id":28621,"name":"Mauricio Faria de Oliveira","email":"mfo@canonical.com","username":"mfo"},"change_message_id":"fd1431447ed800456945cfbefdf8cecfa88bea4f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"86975876_d09973fd","in_reply_to":"7f14235e_8e8af872","updated":"2023-09-13 13:41:27.000000000","message":"Thanks for testing!\n\nPlease note this has been addressed with a different, downstream/Ubuntu patch in bug 1960758.\n\nMarking as Abandoned again.","commit_id":"75e30a5a8cdd5343e65c14dbdc306ec3efee74bd"}],"nova/tests/unit/virt/libvirt/test_driver.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"87c8064c7ae9e2b0790a5961b03e84a8198d64a6","unresolved":true,"context_lines":[{"line_number":5286,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.driver.os.path.exists\u0027, return_value\u003dFalse)"},{"line_number":5287,"context_line":"    @test.patch_exists(\u0027/usr/share/OVMF/OVMF_CODE.fd\u0027, True)"},{"line_number":5288,"context_line":"    @test.patch_exists(\u0027/usr/share/OVMF/OVMF_CODE.secboot.fd\u0027, True)"},{"line_number":5289,"context_line":"    def test_get_guest_config_with_uefi_q35_both_loaders(self, mock_exist,"},{"line_number":5290,"context_line":"                                                         mock_warning):"},{"line_number":5291,"context_line":"        cfg \u003d self._test_get_guest_config_with_uefi_machine_loader(\u0027q35\u0027)"},{"line_number":5292,"context_line":"        self.assertEqual(cfg.os_loader, \u0027/usr/share/OVMF/OVMF_CODE.secboot.fd\u0027)"},{"line_number":5293,"context_line":"        mock_warning.assert_not_called()"}],"source_content_type":"text/x-python","patch_set":4,"id":"68be6ac4_1915b39f","line":5290,"range":{"start_line":5289,"start_character":4,"end_line":5290,"end_character":71},"updated":"2022-04-12 10:02:33.000000000","message":"nit: this should be \ndef test_get_guest_config_with_uefi_q35_both_loaders(\n    self, mock_exist, mock_warning):\n\nand elsewhere where you have split args on two lines.\n\nwe have started to try and standardise on this over the last 2 years\nfor new code and slowly stared converting old code.\n\nno need to resping for this just keep it in mind in the future and and if you repsin the patch later for any  reason please go through and reformat\nany lines that you have split across multiple lines to use that format\n\nbasically the rule we apply is if it fits on one line then keep it on one line if not then move all paramters to a new line to minimise the wasted space due to indents.","commit_id":"ebed454cd75db5fe20250e2029854977062c0f0f"},{"author":{"_account_id":28621,"name":"Mauricio Faria de Oliveira","email":"mfo@canonical.com","username":"mfo"},"change_message_id":"9c180f9da89fc0ab0088af555f99d76f054539ef","unresolved":false,"context_lines":[{"line_number":5286,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.driver.os.path.exists\u0027, return_value\u003dFalse)"},{"line_number":5287,"context_line":"    @test.patch_exists(\u0027/usr/share/OVMF/OVMF_CODE.fd\u0027, True)"},{"line_number":5288,"context_line":"    @test.patch_exists(\u0027/usr/share/OVMF/OVMF_CODE.secboot.fd\u0027, True)"},{"line_number":5289,"context_line":"    def test_get_guest_config_with_uefi_q35_both_loaders(self, mock_exist,"},{"line_number":5290,"context_line":"                                                         mock_warning):"},{"line_number":5291,"context_line":"        cfg \u003d self._test_get_guest_config_with_uefi_machine_loader(\u0027q35\u0027)"},{"line_number":5292,"context_line":"        self.assertEqual(cfg.os_loader, \u0027/usr/share/OVMF/OVMF_CODE.secboot.fd\u0027)"},{"line_number":5293,"context_line":"        mock_warning.assert_not_called()"}],"source_content_type":"text/x-python","patch_set":4,"id":"4c299d00_5ac7dc52","line":5290,"range":{"start_line":5289,"start_character":4,"end_line":5290,"end_character":71},"in_reply_to":"68be6ac4_1915b39f","updated":"2022-04-12 13:32:30.000000000","message":"Ack","commit_id":"ebed454cd75db5fe20250e2029854977062c0f0f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"ba6b7a60c0c9337ab4fa24e04f305c42ec4cbe07","unresolved":true,"context_lines":[{"line_number":5224,"context_line":"            self.assertEqual(cfg.os_loader_type, \"pflash\")"},{"line_number":5225,"context_line":""},{"line_number":5226,"context_line":"    @test.patch_exists(CONF.libvirt.rng_dev_path, True)"},{"line_number":5227,"context_line":"    def _test_get_guest_config_with_uefi_machine_loader(self, machine):"},{"line_number":5228,"context_line":"        self._stub_host_capabilities_cpu_arch(fields.Architecture.X86_64)"},{"line_number":5229,"context_line":""},{"line_number":5230,"context_line":"        libvirt_driver.uefi_logged \u003d True"}],"source_content_type":"text/x-python","patch_set":6,"id":"1d4fa1b0_25f90785","line":5227,"updated":"2023-09-13 13:54:09.000000000","message":"ack so you pulled out the commen test funcitonality","commit_id":"75e30a5a8cdd5343e65c14dbdc306ec3efee74bd"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"ba6b7a60c0c9337ab4fa24e04f305c42ec4cbe07","unresolved":true,"context_lines":[{"line_number":5242,"context_line":"        disk_info \u003d blockinfo.get_disk_info(CONF.libvirt.virt_type,"},{"line_number":5243,"context_line":"                                            instance_ref,"},{"line_number":5244,"context_line":"                                            image_meta)"},{"line_number":5245,"context_line":"        cfg \u003d drvr._get_guest_config(instance_ref, [],"},{"line_number":5246,"context_line":"                                     image_meta, disk_info)"},{"line_number":5247,"context_line":"        return cfg"},{"line_number":5248,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"8c244792_12bf10aa","line":5245,"updated":"2023-09-13 13:54:09.000000000","message":"nit: this could just be a return","commit_id":"75e30a5a8cdd5343e65c14dbdc306ec3efee74bd"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f3df1e81e5e471fa05c0b218c59c3a9cc30fe9a9","unresolved":true,"context_lines":[{"line_number":5843,"context_line":"                    for lpath in DEFAULT_UEFI_LOADER_PATH[caps.host.cpu.arch]:"},{"line_number":5844,"context_line":"                        if os.path.exists(lpath):"},{"line_number":5845,"context_line":"                            guest.os_loader \u003d lpath"},{"line_number":5846,"context_line":"                            if lpath !\u003d \u0027/usr/share/OVMF/OVMF_CODE.secboot.fd\u0027:"},{"line_number":5847,"context_line":"                                lpath_not_secboot \u003d lpath"},{"line_number":5848,"context_line":"                    mach_type \u003d libvirt_utils.get_machine_type(image_meta)"},{"line_number":5849,"context_line":"                    if (caps.host.cpu.arch \u003d\u003d \u0027x86_64\u0027 and"}],"source_content_type":"text/x-python","patch_set":4,"id":"e44c9fb3_223601c0","line":5846,"updated":"2022-04-19 18:28:20.000000000","message":"Surely this isn\u0027t the only path where this can live, right? I assume this is where it is on 20.04 (and probably only by default), but it could easily be somewhere else on another system right?\n\nIf this is really an ubuntu-specific fix, then it seems more proper for being a patch in their package.","commit_id":"ebed454cd75db5fe20250e2029854977062c0f0f"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"540faeba9e0eda2b9977a0a6812d85fab0991598","unresolved":false,"context_lines":[{"line_number":5843,"context_line":"                    for lpath in DEFAULT_UEFI_LOADER_PATH[caps.host.cpu.arch]:"},{"line_number":5844,"context_line":"                        if os.path.exists(lpath):"},{"line_number":5845,"context_line":"                            guest.os_loader \u003d lpath"},{"line_number":5846,"context_line":"                            if lpath !\u003d \u0027/usr/share/OVMF/OVMF_CODE.secboot.fd\u0027:"},{"line_number":5847,"context_line":"                                lpath_not_secboot \u003d lpath"},{"line_number":5848,"context_line":"                    mach_type \u003d libvirt_utils.get_machine_type(image_meta)"},{"line_number":5849,"context_line":"                    if (caps.host.cpu.arch \u003d\u003d \u0027x86_64\u0027 and"}],"source_content_type":"text/x-python","patch_set":4,"id":"d9940632_47ec9bcf","line":5846,"in_reply_to":"168422f2_8a5a2b74","updated":"2022-04-19 18:50:23.000000000","message":"Ack, sorry I should have connected that to the array we\u0027re iterating. I still don\u0027t think having the bare string here is reasonable, but I\u0027m also not sure that checking for \"\u0027secboot\u0027 in lpath\" is great, nor assuming it\u0027s the second item in the list either. Some other distribution that has patched the above list and is currently working won\u0027t realize they need to change this as well.\n\nSo, not sure what to suggest really, but as this is stable-only I think it should be pretty airtight. It really feels like a bug-as-a-feature thing to me.","commit_id":"ebed454cd75db5fe20250e2029854977062c0f0f"},{"author":{"_account_id":28621,"name":"Mauricio Faria de Oliveira","email":"mfo@canonical.com","username":"mfo"},"change_message_id":"fcab835bad5486d7a74fdc7ad5193a60a24e81b3","unresolved":false,"context_lines":[{"line_number":5843,"context_line":"                    for lpath in DEFAULT_UEFI_LOADER_PATH[caps.host.cpu.arch]:"},{"line_number":5844,"context_line":"                        if os.path.exists(lpath):"},{"line_number":5845,"context_line":"                            guest.os_loader \u003d lpath"},{"line_number":5846,"context_line":"                            if lpath !\u003d \u0027/usr/share/OVMF/OVMF_CODE.secboot.fd\u0027:"},{"line_number":5847,"context_line":"                                lpath_not_secboot \u003d lpath"},{"line_number":5848,"context_line":"                    mach_type \u003d libvirt_utils.get_machine_type(image_meta)"},{"line_number":5849,"context_line":"                    if (caps.host.cpu.arch \u003d\u003d \u0027x86_64\u0027 and"}],"source_content_type":"text/x-python","patch_set":4,"id":"168422f2_8a5a2b74","line":5846,"in_reply_to":"e44c9fb3_223601c0","updated":"2022-04-19 18:39:35.000000000","message":"Right, but the paths are hardcoded in victoria/ussuri (upstream), so the check is valid / against the value in array DEFAULT_UEFI_LOADER_PATH.\n\nIt happens to be a valid hardcoded path on Ubuntu as well. :)","commit_id":"ebed454cd75db5fe20250e2029854977062c0f0f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"87c8064c7ae9e2b0790a5961b03e84a8198d64a6","unresolved":true,"context_lines":[{"line_number":5844,"context_line":"                        if os.path.exists(lpath):"},{"line_number":5845,"context_line":"                            guest.os_loader \u003d lpath"},{"line_number":5846,"context_line":"                            if lpath !\u003d \u0027/usr/share/OVMF/OVMF_CODE.secboot.fd\u0027:"},{"line_number":5847,"context_line":"                                lpath_not_secboot \u003d lpath"},{"line_number":5848,"context_line":"                    mach_type \u003d libvirt_utils.get_machine_type(image_meta)"},{"line_number":5849,"context_line":"                    if (caps.host.cpu.arch \u003d\u003d \u0027x86_64\u0027 and"},{"line_number":5850,"context_line":"                            mach_type is not None and \u0027q35\u0027 not in mach_type):"}],"source_content_type":"text/x-python","patch_set":4,"id":"33a83288_a6072c02","line":5847,"range":{"start_line":5847,"start_character":32,"end_line":5847,"end_character":57},"updated":"2022-04-12 10:02:33.000000000","message":"you could add a break here to stop checking after you find a non secure boot path but i guess this is also fine.\nthe list is short.","commit_id":"ebed454cd75db5fe20250e2029854977062c0f0f"},{"author":{"_account_id":28621,"name":"Mauricio Faria de Oliveira","email":"mfo@canonical.com","username":"mfo"},"change_message_id":"9c180f9da89fc0ab0088af555f99d76f054539ef","unresolved":false,"context_lines":[{"line_number":5844,"context_line":"                        if os.path.exists(lpath):"},{"line_number":5845,"context_line":"                            guest.os_loader \u003d lpath"},{"line_number":5846,"context_line":"                            if lpath !\u003d \u0027/usr/share/OVMF/OVMF_CODE.secboot.fd\u0027:"},{"line_number":5847,"context_line":"                                lpath_not_secboot \u003d lpath"},{"line_number":5848,"context_line":"                    mach_type \u003d libvirt_utils.get_machine_type(image_meta)"},{"line_number":5849,"context_line":"                    if (caps.host.cpu.arch \u003d\u003d \u0027x86_64\u0027 and"},{"line_number":5850,"context_line":"                            mach_type is not None and \u0027q35\u0027 not in mach_type):"}],"source_content_type":"text/x-python","patch_set":4,"id":"8a2e6eaf_ddda1ddf","line":5847,"range":{"start_line":5847,"start_character":32,"end_line":5847,"end_character":57},"in_reply_to":"33a83288_a6072c02","updated":"2022-04-12 13:32:30.000000000","message":"The lack of \u0027break\u0027 is on purpose in this case, so to keep the same behavior/assignment of \u0027guest.os_loader\u0027 from line 5845 (which doesn\u0027t have a \u0027break\u0027) on line 5852.","commit_id":"ebed454cd75db5fe20250e2029854977062c0f0f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"ba6b7a60c0c9337ab4fa24e04f305c42ec4cbe07","unresolved":true,"context_lines":[{"line_number":5836,"context_line":"                        uefi_logged \u003d True"},{"line_number":5837,"context_line":""},{"line_number":5838,"context_line":"                    loader_paths \u003d DEFAULT_UEFI_LOADER_PATH[caps.host.cpu.arch]"},{"line_number":5839,"context_line":"                    mach_type \u003d libvirt_utils.get_machine_type(image_meta)"},{"line_number":5840,"context_line":""},{"line_number":5841,"context_line":"                    # x86_64: on pc (non-q35) prefer a non-secboot/SB loader"},{"line_number":5842,"context_line":"                    # (SB requires SMM, which requires the q35 machine type)"}],"source_content_type":"text/x-python","patch_set":6,"id":"b4ab0d6d_0f4ca915","line":5839,"updated":"2023-09-13 13:54:09.000000000","message":"this retrives the machinetype form the instnace metadata if set or returs the default based on teh architecture\n\nhttps://github.com/openstack/nova/blob/53012f1c55072c42ced267a2b1adef0a669d9f45/nova/virt/libvirt/utils.py#L584-L594\n\n+1","commit_id":"75e30a5a8cdd5343e65c14dbdc306ec3efee74bd"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"ba6b7a60c0c9337ab4fa24e04f305c42ec4cbe07","unresolved":true,"context_lines":[{"line_number":5841,"context_line":"                    # x86_64: on pc (non-q35) prefer a non-secboot/SB loader"},{"line_number":5842,"context_line":"                    # (SB requires SMM, which requires the q35 machine type)"},{"line_number":5843,"context_line":"                    if (caps.host.cpu.arch \u003d\u003d \u0027x86_64\u0027 and"},{"line_number":5844,"context_line":"                           mach_type is not None and \u0027q35\u0027 not in mach_type):"},{"line_number":5845,"context_line":"                        # Don\u0027t even iterate the list with the secboot loader"},{"line_number":5846,"context_line":"                        loader_paths \u003d loader_paths.copy()"},{"line_number":5847,"context_line":"                        loader_paths.remove(UEFI_LOADER_PATH_x86_64_SECBOOT)"}],"source_content_type":"text/x-python","patch_set":6,"id":"45c45fb9_1b0f3e72","line":5844,"range":{"start_line":5844,"start_character":53,"end_line":5844,"end_character":75},"updated":"2023-09-13 13:54:09.000000000","message":"then you doing a substring match agaisnt the user suppied value.\n\ntechnailsy htey could have typed Q35 and this woudl fail to match however the vm woudl also fail to boot as the value we pass to qemu is case senseitve\n\nso its fine to skip calling .lower() here","commit_id":"75e30a5a8cdd5343e65c14dbdc306ec3efee74bd"}]}