)]}'
{"ironic_python_agent/extensions/image.py":[{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"d0769a2f67e4dc8ddb1146cf384dfeb425112acd","unresolved":false,"context_lines":[{"line_number":180,"context_line":""},{"line_number":181,"context_line":""},{"line_number":182,"context_line":"def _verify_secure_boot(device, root_uuid, efi_system_part_uuid):"},{"line_number":183,"context_line":"    if efi_system_part_uuid:"},{"line_number":184,"context_line":"        root_partition \u003d _get_partition(device, uuid\u003droot_uuid)"},{"line_number":185,"context_line":"        efi_partition \u003d None"},{"line_number":186,"context_line":"        efi_partition_mount_point \u003d None"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_383be639","line":183,"range":{"start_line":183,"start_character":7,"end_line":183,"end_character":27},"updated":"2019-12-02 20:07:43.000000000","message":"Are we sure that this is always going to be present?\n\nWhat if we wrote a whole disk image? Are we finding an already created and present EFI boot partition?","commit_id":"fb8cee7225b400b40bdc133d3391e391e3fca90c"},{"author":{"_account_id":15519,"name":"Iury Gregory Melo Ferreira","display_name":"Iury Gregory","email":"iurygregory@gmail.com","username":"iurygregory"},"change_message_id":"a35edd9fe8343211624e918aa76c6bb51c2016bb","unresolved":false,"context_lines":[{"line_number":180,"context_line":""},{"line_number":181,"context_line":""},{"line_number":182,"context_line":"def _verify_secure_boot(device, root_uuid, efi_system_part_uuid):"},{"line_number":183,"context_line":"    if efi_system_part_uuid:"},{"line_number":184,"context_line":"        root_partition \u003d _get_partition(device, uuid\u003droot_uuid)"},{"line_number":185,"context_line":"        efi_partition \u003d None"},{"line_number":186,"context_line":"        efi_partition_mount_point \u003d None"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_f3d1e8cb","line":183,"range":{"start_line":183,"start_character":7,"end_line":183,"end_character":27},"in_reply_to":"3fa7e38b_383be639","updated":"2019-12-03 12:41:13.000000000","message":"I was assuming that will be present, \n`:param efi_system_part_uuid: The UUID of the efi system partition.\n            To be used only for uefi boot mode.  For uefi boot mode, the\n            boot loader will be installed here.`\n\nSo maybe we should check the boot mode (using https://github.com/openstack/ironic-python-agent/blob/4354bc04f95c03fc90d7b596422874e329996660/ironic_python_agent/hardware.py#L660 ), if is `uefi` we would check device to see if is a whole disk image and if we don\u0027t have the partition we would create, does it makes sense?","commit_id":"fb8cee7225b400b40bdc133d3391e391e3fca90c"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"304418866d97c6be092ad0c817c7d71e68f8cf21","unresolved":false,"context_lines":[{"line_number":180,"context_line":""},{"line_number":181,"context_line":""},{"line_number":182,"context_line":"def _verify_secure_boot(device, root_uuid, efi_system_part_uuid):"},{"line_number":183,"context_line":"    if efi_system_part_uuid:"},{"line_number":184,"context_line":"        root_partition \u003d _get_partition(device, uuid\u003droot_uuid)"},{"line_number":185,"context_line":"        efi_partition \u003d None"},{"line_number":186,"context_line":"        efi_partition_mount_point \u003d None"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_8af06b93","line":183,"range":{"start_line":183,"start_character":7,"end_line":183,"end_character":27},"in_reply_to":"3fa7e38b_39ae7e81","updated":"2019-12-03 23:31:28.000000000","message":"so if we\u0027re in uefi mode, check disk contents regardless of if a part id exists and go from there. I think that \"might\" work ? :)  EFI partitions do also have special labels so we could look for that too...  I need to look at Richard\u0027s issue closer though.","commit_id":"fb8cee7225b400b40bdc133d3391e391e3fca90c"},{"author":{"_account_id":15519,"name":"Iury Gregory Melo Ferreira","display_name":"Iury Gregory","email":"iurygregory@gmail.com","username":"iurygregory"},"change_message_id":"3661dedad736fd8cb195982eb5b5754beecf24e9","unresolved":false,"context_lines":[{"line_number":180,"context_line":""},{"line_number":181,"context_line":""},{"line_number":182,"context_line":"def _verify_secure_boot(device, root_uuid, efi_system_part_uuid):"},{"line_number":183,"context_line":"    if efi_system_part_uuid:"},{"line_number":184,"context_line":"        root_partition \u003d _get_partition(device, uuid\u003droot_uuid)"},{"line_number":185,"context_line":"        efi_partition \u003d None"},{"line_number":186,"context_line":"        efi_partition_mount_point \u003d None"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_39ae7e81","line":183,"range":{"start_line":183,"start_character":7,"end_line":183,"end_character":27},"in_reply_to":"3fa7e38b_bef9f44e","updated":"2019-12-03 15:40:09.000000000","message":"I will trust on the boot mode for now, if anyone knows a better approach I will update \u003d)","commit_id":"fb8cee7225b400b40bdc133d3391e391e3fca90c"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"afa85cbed1ce630c8de130e241d39a05f3911055","unresolved":false,"context_lines":[{"line_number":180,"context_line":""},{"line_number":181,"context_line":""},{"line_number":182,"context_line":"def _verify_secure_boot(device, root_uuid, efi_system_part_uuid):"},{"line_number":183,"context_line":"    if efi_system_part_uuid:"},{"line_number":184,"context_line":"        root_partition \u003d _get_partition(device, uuid\u003droot_uuid)"},{"line_number":185,"context_line":"        efi_partition \u003d None"},{"line_number":186,"context_line":"        efi_partition_mount_point \u003d None"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_bef9f44e","line":183,"range":{"start_line":183,"start_character":7,"end_line":183,"end_character":27},"in_reply_to":"3fa7e38b_f3d1e8cb","updated":"2019-12-03 14:19:46.000000000","message":"I too assumed that to be present, although I was chatting with rpioso last night, and I think we\u0027ve got a UEFI whole disk image bug where that field is not populated (which makes sense, we didn\u0027t create it...) so we can\u0027t trust it.\n\nThat is bood mode of the hardware it\u0027s self for the deploy, not post deploy... but maybe we should just \"trust it\"","commit_id":"fb8cee7225b400b40bdc133d3391e391e3fca90c"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"d0769a2f67e4dc8ddb1146cf384dfeb425112acd","unresolved":false,"context_lines":[{"line_number":205,"context_line":"                              efi_partition_mount_point)"},{"line_number":206,"context_line":"                efi_mounted \u003d True"},{"line_number":207,"context_line":""},{"line_number":208,"context_line":"            sb_efivar \u003d utils.execute(\u0027find\u0027, \u0027/sys/firmware/efi/efivars\u0027,"},{"line_number":209,"context_line":"                                      \u0027-name\u0027, \u0027SecureBoot*\u0027)"},{"line_number":210,"context_line":"            sb_files \u003d sb_efivar[0].rstrip().split(\u0027\\n\u0027)"},{"line_number":211,"context_line":"            for sb_file in sb_files:"},{"line_number":212,"context_line":"                od_output \u003d utils.execute(\u0027od\u0027, \u0027--address-radix\u003dn\u0027,"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_386686f0","line":209,"range":{"start_line":208,"start_character":0,"end_line":209,"end_character":61},"updated":"2019-12-02 20:07:43.000000000","message":"Are we sure these would be present if secure boot is running? Also, what does secure boot matter in this context?","commit_id":"fb8cee7225b400b40bdc133d3391e391e3fca90c"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"afa85cbed1ce630c8de130e241d39a05f3911055","unresolved":false,"context_lines":[{"line_number":205,"context_line":"                              efi_partition_mount_point)"},{"line_number":206,"context_line":"                efi_mounted \u003d True"},{"line_number":207,"context_line":""},{"line_number":208,"context_line":"            sb_efivar \u003d utils.execute(\u0027find\u0027, \u0027/sys/firmware/efi/efivars\u0027,"},{"line_number":209,"context_line":"                                      \u0027-name\u0027, \u0027SecureBoot*\u0027)"},{"line_number":210,"context_line":"            sb_files \u003d sb_efivar[0].rstrip().split(\u0027\\n\u0027)"},{"line_number":211,"context_line":"            for sb_file in sb_files:"},{"line_number":212,"context_line":"                od_output \u003d utils.execute(\u0027od\u0027, \u0027--address-radix\u003dn\u0027,"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_7eba9c71","line":209,"range":{"start_line":208,"start_character":0,"end_line":209,"end_character":61},"in_reply_to":"3fa7e38b_33e9c07e","updated":"2019-12-03 14:19:46.000000000","message":"Well, installing grub2 is stomping on the already present bootloader, regardless of if it is grub2. The idea, at least to me, is if it is a whole disk image, and it looks like it should boot, then let it boot, just make sure if it is in efi mode that we update the nvram to point to what appears to be the bootloader.","commit_id":"fb8cee7225b400b40bdc133d3391e391e3fca90c"},{"author":{"_account_id":15519,"name":"Iury Gregory Melo Ferreira","display_name":"Iury Gregory","email":"iurygregory@gmail.com","username":"iurygregory"},"change_message_id":"a35edd9fe8343211624e918aa76c6bb51c2016bb","unresolved":false,"context_lines":[{"line_number":205,"context_line":"                              efi_partition_mount_point)"},{"line_number":206,"context_line":"                efi_mounted \u003d True"},{"line_number":207,"context_line":""},{"line_number":208,"context_line":"            sb_efivar \u003d utils.execute(\u0027find\u0027, \u0027/sys/firmware/efi/efivars\u0027,"},{"line_number":209,"context_line":"                                      \u0027-name\u0027, \u0027SecureBoot*\u0027)"},{"line_number":210,"context_line":"            sb_files \u003d sb_efivar[0].rstrip().split(\u0027\\n\u0027)"},{"line_number":211,"context_line":"            for sb_file in sb_files:"},{"line_number":212,"context_line":"                od_output \u003d utils.execute(\u0027od\u0027, \u0027--address-radix\u003dn\u0027,"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_33e9c07e","line":209,"range":{"start_line":208,"start_character":0,"end_line":209,"end_character":61},"in_reply_to":"3fa7e38b_386686f0","updated":"2019-12-03 12:41:13.000000000","message":"If is using SB we should be able to find the variables there.\nI was thinking that we only want to avoid grub2 when we are using uefi with SB. \n\nSo if is uefi, we don\u0027t want to install grub2 at all right?","commit_id":"fb8cee7225b400b40bdc133d3391e391e3fca90c"},{"author":{"_account_id":15519,"name":"Iury Gregory Melo Ferreira","display_name":"Iury Gregory","email":"iurygregory@gmail.com","username":"iurygregory"},"change_message_id":"3661dedad736fd8cb195982eb5b5754beecf24e9","unresolved":false,"context_lines":[{"line_number":205,"context_line":"                              efi_partition_mount_point)"},{"line_number":206,"context_line":"                efi_mounted \u003d True"},{"line_number":207,"context_line":""},{"line_number":208,"context_line":"            sb_efivar \u003d utils.execute(\u0027find\u0027, \u0027/sys/firmware/efi/efivars\u0027,"},{"line_number":209,"context_line":"                                      \u0027-name\u0027, \u0027SecureBoot*\u0027)"},{"line_number":210,"context_line":"            sb_files \u003d sb_efivar[0].rstrip().split(\u0027\\n\u0027)"},{"line_number":211,"context_line":"            for sb_file in sb_files:"},{"line_number":212,"context_line":"                od_output \u003d utils.execute(\u0027od\u0027, \u0027--address-radix\u003dn\u0027,"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_59e6dae9","line":209,"range":{"start_line":208,"start_character":0,"end_line":209,"end_character":61},"in_reply_to":"3fa7e38b_7eba9c71","updated":"2019-12-03 15:40:09.000000000","message":"I will consider only this scenario an update the patch","commit_id":"fb8cee7225b400b40bdc133d3391e391e3fca90c"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"d0769a2f67e4dc8ddb1146cf384dfeb425112acd","unresolved":false,"context_lines":[{"line_number":210,"context_line":"            sb_files \u003d sb_efivar[0].rstrip().split(\u0027\\n\u0027)"},{"line_number":211,"context_line":"            for sb_file in sb_files:"},{"line_number":212,"context_line":"                od_output \u003d utils.execute(\u0027od\u0027, \u0027--address-radix\u003dn\u0027,"},{"line_number":213,"context_line":"                                          \u0027--format\u003du1\u0027, sb_file)"},{"line_number":214,"context_line":"                if od_output[0].rstrip().split(\u0027\\n\u0027)[0].endswith(\u00271\u0027):"},{"line_number":215,"context_line":"                    return True"},{"line_number":216,"context_line":"            return False"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_78fcfe71","line":213,"updated":"2019-12-02 20:07:43.000000000","message":"so if I\u0027m grokking this, we\u0027re kind of going in reverse. I\u0027m reading this as your trying to look at the firmware and see what is in the efi bootloader nvram, as compared to what is on disk, but we need to identify, and potentially set what we find on disk.\n\nSo... something like\n1) Mount EFI boot partition\n2) Scan for bootx64.efi, likely in order of most complaint to spec to most non-compliant.\n\n(Note, these are all linux style paths)\n(Note2, all of these paths are x86_64. There is some variation for other architectures, so maybe the path to take is find all the files in the boot partition, and hunt for chunks of text in order, and maybe then verify that it is a file on disk? https://wiki.debian.org/UEFI)\n\nSecure boot:\n\n\"\u003cpartition path\u003e/efi/redhat/shimx64-redhat.efi\"\n\"\u003cpartition path\u003e/efi/redhat/shimx64.efi\" \u003c-- This would be \n\"\u003cpartition path\u003e/EFI/debian/grubx64.efi\n\"\u003cpartition path\u003e/EFI/ubuntu/grubx64.efi\n\"\u003cpartition path\u003e/EFI/opensuse/shim.efi\n\"\u003cpartition path\u003e/EFI/opensuse/grubx64.efi\n\nGeneral boot... \n\"\u003cpartition path\u003e/efi/boot/bootx64.efi\" \u003c-- this file may actually be the shim, or it could be grub in unsigned ases.\n\"\u003cpartition path/efi/EFI/BOOT/BOOTX64.efi\" \u003c- The FS should be fat16... so case \"shouldn\u0027t\" matter, the difference is the extra EFI. An additional variation.\n\nWindows boot which should likely be in the secure boot list:\n\n\"\u003cpartition path\u003eWindows/System32/Winload.efi\" \u003c-- Actually for windows, this may be root volume content...\n\nFor arm: The format is bootaa64.efi/shimaa64.efi/grubaa64.efi.\n\n3) Once we\u0027e identified the the contents on the disk or partition, then we can update the efibootmgr contents.","commit_id":"fb8cee7225b400b40bdc133d3391e391e3fca90c"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"afa85cbed1ce630c8de130e241d39a05f3911055","unresolved":false,"context_lines":[{"line_number":210,"context_line":"            sb_files \u003d sb_efivar[0].rstrip().split(\u0027\\n\u0027)"},{"line_number":211,"context_line":"            for sb_file in sb_files:"},{"line_number":212,"context_line":"                od_output \u003d utils.execute(\u0027od\u0027, \u0027--address-radix\u003dn\u0027,"},{"line_number":213,"context_line":"                                          \u0027--format\u003du1\u0027, sb_file)"},{"line_number":214,"context_line":"                if od_output[0].rstrip().split(\u0027\\n\u0027)[0].endswith(\u00271\u0027):"},{"line_number":215,"context_line":"                    return True"},{"line_number":216,"context_line":"            return False"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_be7c94b6","line":213,"in_reply_to":"3fa7e38b_258f3777","updated":"2019-12-03 14:19:46.000000000","message":"That should work and if we see a bootx64.efi out there, then that is likely it. Still would need to eval the entire path I suspect because... there are multiple loaders that sometimes get deposited on system images :\\ Of course, they are often also copies so it may not matter.","commit_id":"fb8cee7225b400b40bdc133d3391e391e3fca90c"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"304418866d97c6be092ad0c817c7d71e68f8cf21","unresolved":false,"context_lines":[{"line_number":210,"context_line":"            sb_files \u003d sb_efivar[0].rstrip().split(\u0027\\n\u0027)"},{"line_number":211,"context_line":"            for sb_file in sb_files:"},{"line_number":212,"context_line":"                od_output \u003d utils.execute(\u0027od\u0027, \u0027--address-radix\u003dn\u0027,"},{"line_number":213,"context_line":"                                          \u0027--format\u003du1\u0027, sb_file)"},{"line_number":214,"context_line":"                if od_output[0].rstrip().split(\u0027\\n\u0027)[0].endswith(\u00271\u0027):"},{"line_number":215,"context_line":"                    return True"},{"line_number":216,"context_line":"            return False"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_6a2bafe9","line":213,"in_reply_to":"3fa7e38b_4f560c08","updated":"2019-12-03 23:31:28.000000000","message":"\u003e \"eval the entire path\" this would be verify if the efibootmgr has\n \u003e an entry for the efi files we have found?\n\nNo, because we might have overriden the nvram by saying \"network boot\" via the bmc, which is what Richard\u0027s machine actually reports. The act of grub-install presently updates the nvram which puts the entry in.\n\nSo in a sense, the case is \"The bootloader has been installed on disk\" but we need to make sure there is an nvram entry.","commit_id":"fb8cee7225b400b40bdc133d3391e391e3fca90c"},{"author":{"_account_id":23851,"name":"Riccardo Pittau","email":"elfosardo@gmail.com","username":"elfosardo"},"change_message_id":"befe9aba02d5fb9d030c653525cb768614d27106","unresolved":false,"context_lines":[{"line_number":210,"context_line":"            sb_files \u003d sb_efivar[0].rstrip().split(\u0027\\n\u0027)"},{"line_number":211,"context_line":"            for sb_file in sb_files:"},{"line_number":212,"context_line":"                od_output \u003d utils.execute(\u0027od\u0027, \u0027--address-radix\u003dn\u0027,"},{"line_number":213,"context_line":"                                          \u0027--format\u003du1\u0027, sb_file)"},{"line_number":214,"context_line":"                if od_output[0].rstrip().split(\u0027\\n\u0027)[0].endswith(\u00271\u0027):"},{"line_number":215,"context_line":"                    return True"},{"line_number":216,"context_line":"            return False"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_70ff24ab","line":213,"in_reply_to":"3fa7e38b_4f7dac6b","updated":"2019-12-03 17:11:36.000000000","message":"of course, I pointed out the secure boot because my understanding was that the entire patch is rotating around that.\nSo we\u0027re looking a bit more than just checking the uefi, not only just looking for \"this is uefi enabled\", but also \"we have a bootloader\" no matter if secure boot or not; in this case any *.efi files in the path should be ok, but still we need to give priority to secure boot enabled bootloader, no?","commit_id":"fb8cee7225b400b40bdc133d3391e391e3fca90c"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"304418866d97c6be092ad0c817c7d71e68f8cf21","unresolved":false,"context_lines":[{"line_number":210,"context_line":"            sb_files \u003d sb_efivar[0].rstrip().split(\u0027\\n\u0027)"},{"line_number":211,"context_line":"            for sb_file in sb_files:"},{"line_number":212,"context_line":"                od_output \u003d utils.execute(\u0027od\u0027, \u0027--address-radix\u003dn\u0027,"},{"line_number":213,"context_line":"                                          \u0027--format\u003du1\u0027, sb_file)"},{"line_number":214,"context_line":"                if od_output[0].rstrip().split(\u0027\\n\u0027)[0].endswith(\u00271\u0027):"},{"line_number":215,"context_line":"                    return True"},{"line_number":216,"context_line":"            return False"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_8a45cbb6","line":213,"in_reply_to":"3fa7e38b_70ff24ab","updated":"2019-12-03 23:31:28.000000000","message":"\u003e of course, I pointed out the secure boot because my understanding\n \u003e was that the entire patch is rotating around that.\n \u003e So we\u0027re looking a bit more than just checking the uefi, not only\n \u003e just looking for \"this is uefi enabled\", but also \"we have a\n \u003e bootloader\" no matter if secure boot or not; in this case any *.efi\n \u003e files in the path should be ok, but still we need to give priority\n \u003e to secure boot enabled bootloader, no?\n\nI don\u0027t think we need to. The standard practice is to put the BOOTX68.EFI file as the shim for Linux/Grub and then put in GRUBX64.EFI which the shim loads. If those files are signed, then in theory, we get secure boot for free. Presently we stomp on them effectively erasing secure boot.","commit_id":"fb8cee7225b400b40bdc133d3391e391e3fca90c"},{"author":{"_account_id":15519,"name":"Iury Gregory Melo Ferreira","display_name":"Iury Gregory","email":"iurygregory@gmail.com","username":"iurygregory"},"change_message_id":"a35edd9fe8343211624e918aa76c6bb51c2016bb","unresolved":false,"context_lines":[{"line_number":210,"context_line":"            sb_files \u003d sb_efivar[0].rstrip().split(\u0027\\n\u0027)"},{"line_number":211,"context_line":"            for sb_file in sb_files:"},{"line_number":212,"context_line":"                od_output \u003d utils.execute(\u0027od\u0027, \u0027--address-radix\u003dn\u0027,"},{"line_number":213,"context_line":"                                          \u0027--format\u003du1\u0027, sb_file)"},{"line_number":214,"context_line":"                if od_output[0].rstrip().split(\u0027\\n\u0027)[0].endswith(\u00271\u0027):"},{"line_number":215,"context_line":"                    return True"},{"line_number":216,"context_line":"            return False"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_258f3777","line":213,"in_reply_to":"3fa7e38b_78fcfe71","updated":"2019-12-03 12:41:13.000000000","message":"1) would be:\n - mount root_partition on the path \u003ctemporary\u003e\n - mount sysfs\n - mount efi_partition on the \u003cpath\u003e + \"boot/efi\"\n\n2) we can just try to find all files with .efi file name extension","commit_id":"fb8cee7225b400b40bdc133d3391e391e3fca90c"},{"author":{"_account_id":23851,"name":"Riccardo Pittau","email":"elfosardo@gmail.com","username":"elfosardo"},"change_message_id":"e3f7a626dcd8df303da1324e8fb8599e2ba0d4e2","unresolved":false,"context_lines":[{"line_number":210,"context_line":"            sb_files \u003d sb_efivar[0].rstrip().split(\u0027\\n\u0027)"},{"line_number":211,"context_line":"            for sb_file in sb_files:"},{"line_number":212,"context_line":"                od_output \u003d utils.execute(\u0027od\u0027, \u0027--address-radix\u003dn\u0027,"},{"line_number":213,"context_line":"                                          \u0027--format\u003du1\u0027, sb_file)"},{"line_number":214,"context_line":"                if od_output[0].rstrip().split(\u0027\\n\u0027)[0].endswith(\u00271\u0027):"},{"line_number":215,"context_line":"                    return True"},{"line_number":216,"context_line":"            return False"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_d8208bb4","line":213,"in_reply_to":"3fa7e38b_78fcfe71","updated":"2019-12-03 09:42:23.000000000","message":"@Julia: for the bootloader efi to be present (in any location, more or less \"standard\"), shouldn\u0027t the installation have happened already?\nIf grub didn\u0027t run, we shouldn\u0027t find any bootloader (talking about linux distro), unless of course the disk has been already prepared for one or more specific OSes.\nIf this is the first run, my understanding is that reading the efi variables will be the only way to determine if secureboot is enabled or not.","commit_id":"fb8cee7225b400b40bdc133d3391e391e3fca90c"},{"author":{"_account_id":21909,"name":"Bob Fournier","email":"bfournie@redhat.com","username":"bfournie"},"change_message_id":"ccaabc2c99a193c9a0dba657b149addbf52b4617","unresolved":false,"context_lines":[{"line_number":210,"context_line":"            sb_files \u003d sb_efivar[0].rstrip().split(\u0027\\n\u0027)"},{"line_number":211,"context_line":"            for sb_file in sb_files:"},{"line_number":212,"context_line":"                od_output \u003d utils.execute(\u0027od\u0027, \u0027--address-radix\u003dn\u0027,"},{"line_number":213,"context_line":"                                          \u0027--format\u003du1\u0027, sb_file)"},{"line_number":214,"context_line":"                if od_output[0].rstrip().split(\u0027\\n\u0027)[0].endswith(\u00271\u0027):"},{"line_number":215,"context_line":"                    return True"},{"line_number":216,"context_line":"            return False"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_4f7dac6b","line":213,"in_reply_to":"3fa7e38b_9e62f814","updated":"2019-12-03 15:42:27.000000000","message":"Agree, I think we should concentrate on taking grub2-install out of the workflow, whether or not secure boot is confgured.","commit_id":"fb8cee7225b400b40bdc133d3391e391e3fca90c"},{"author":{"_account_id":15519,"name":"Iury Gregory Melo Ferreira","display_name":"Iury Gregory","email":"iurygregory@gmail.com","username":"iurygregory"},"change_message_id":"3661dedad736fd8cb195982eb5b5754beecf24e9","unresolved":false,"context_lines":[{"line_number":210,"context_line":"            sb_files \u003d sb_efivar[0].rstrip().split(\u0027\\n\u0027)"},{"line_number":211,"context_line":"            for sb_file in sb_files:"},{"line_number":212,"context_line":"                od_output \u003d utils.execute(\u0027od\u0027, \u0027--address-radix\u003dn\u0027,"},{"line_number":213,"context_line":"                                          \u0027--format\u003du1\u0027, sb_file)"},{"line_number":214,"context_line":"                if od_output[0].rstrip().split(\u0027\\n\u0027)[0].endswith(\u00271\u0027):"},{"line_number":215,"context_line":"                    return True"},{"line_number":216,"context_line":"            return False"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_4f560c08","line":213,"in_reply_to":"3fa7e38b_be7c94b6","updated":"2019-12-03 15:40:09.000000000","message":"\"eval the entire path\" this would be verify if the efibootmgr has an entry for the efi files we have found?","commit_id":"fb8cee7225b400b40bdc133d3391e391e3fca90c"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"afa85cbed1ce630c8de130e241d39a05f3911055","unresolved":false,"context_lines":[{"line_number":210,"context_line":"            sb_files \u003d sb_efivar[0].rstrip().split(\u0027\\n\u0027)"},{"line_number":211,"context_line":"            for sb_file in sb_files:"},{"line_number":212,"context_line":"                od_output \u003d utils.execute(\u0027od\u0027, \u0027--address-radix\u003dn\u0027,"},{"line_number":213,"context_line":"                                          \u0027--format\u003du1\u0027, sb_file)"},{"line_number":214,"context_line":"                if od_output[0].rstrip().split(\u0027\\n\u0027)[0].endswith(\u00271\u0027):"},{"line_number":215,"context_line":"                    return True"},{"line_number":216,"context_line":"            return False"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_9e62f814","line":213,"in_reply_to":"3fa7e38b_d8208bb4","updated":"2019-12-03 14:19:46.000000000","message":"I think we need to not focus on just secure boot, but the general \"if this is uefi, and we have the bits on disk that appear to be there, why stomp on them?","commit_id":"fb8cee7225b400b40bdc133d3391e391e3fca90c"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"292f85ac73bdf6c21b6728f6cea5e7890cfe56fa","unresolved":false,"context_lines":[{"line_number":34,"context_line":""},{"line_number":35,"context_line":"BIND_MOUNTS \u003d (\u0027/dev\u0027, \u0027/proc\u0027, \u0027/run\u0027)"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"BOOTLOADERS_EFI \u003d [\u0027/bootx64.efi\u0027, \u0027/BOOTX64.EFI\u0027, \u0027grubaa64.efi\u0027]"},{"line_number":38,"context_line":""},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"def _get_partition(device, uuid):"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_db0da540","line":37,"range":{"start_line":37,"start_character":51,"end_line":37,"end_character":66},"updated":"2019-12-06 15:56:12.000000000","message":"This is arm64 only.\n\nAlso, the others have leading slashes.","commit_id":"15cd37ce66d0a329b4a753731b54e4a815110fda"},{"author":{"_account_id":23851,"name":"Riccardo Pittau","email":"elfosardo@gmail.com","username":"elfosardo"},"change_message_id":"cca4922c1f0e7f1a3de60e5d75d22a429866c985","unresolved":false,"context_lines":[{"line_number":34,"context_line":""},{"line_number":35,"context_line":"BIND_MOUNTS \u003d (\u0027/dev\u0027, \u0027/proc\u0027, \u0027/run\u0027)"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"BOOTLOADERS_EFI \u003d [\u0027/bootx64.efi\u0027, \u0027/BOOTX64.EFI\u0027, \u0027grubaa64.efi\u0027]"},{"line_number":38,"context_line":""},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"def _get_partition(device, uuid):"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_0d930450","line":37,"range":{"start_line":37,"start_character":19,"end_line":37,"end_character":49},"updated":"2019-12-06 16:36:43.000000000","message":"you should not need both of them, filesystem is caseless and you can always lowercase/uppercase if needed","commit_id":"15cd37ce66d0a329b4a753731b54e4a815110fda"},{"author":{"_account_id":15519,"name":"Iury Gregory Melo Ferreira","display_name":"Iury Gregory","email":"iurygregory@gmail.com","username":"iurygregory"},"change_message_id":"2a234cf9dfefd33fe162a70207636ec3f2cc2199","unresolved":false,"context_lines":[{"line_number":34,"context_line":""},{"line_number":35,"context_line":"BIND_MOUNTS \u003d (\u0027/dev\u0027, \u0027/proc\u0027, \u0027/run\u0027)"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"BOOTLOADERS_EFI \u003d [\u0027/bootx64.efi\u0027, \u0027/BOOTX64.EFI\u0027, \u0027grubaa64.efi\u0027]"},{"line_number":38,"context_line":""},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"def _get_partition(device, uuid):"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_f0367be7","line":37,"range":{"start_line":37,"start_character":19,"end_line":37,"end_character":49},"in_reply_to":"3fa7e38b_0d930450","updated":"2019-12-06 16:40:13.000000000","message":"I will put the find output to lowercase, I just hope it won\u0027t cause problems when running the efibootmgr","commit_id":"15cd37ce66d0a329b4a753731b54e4a815110fda"},{"author":{"_account_id":15519,"name":"Iury Gregory Melo Ferreira","display_name":"Iury Gregory","email":"iurygregory@gmail.com","username":"iurygregory"},"change_message_id":"1ad194e5c67ba22fb8034dd446add07294599bd3","unresolved":false,"context_lines":[{"line_number":34,"context_line":""},{"line_number":35,"context_line":"BIND_MOUNTS \u003d (\u0027/dev\u0027, \u0027/proc\u0027, \u0027/run\u0027)"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"BOOTLOADERS_EFI \u003d [\u0027/bootx64.efi\u0027, \u0027/BOOTX64.EFI\u0027, \u0027grubaa64.efi\u0027]"},{"line_number":38,"context_line":""},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"def _get_partition(device, uuid):"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_ad5af01a","line":37,"range":{"start_line":37,"start_character":51,"end_line":37,"end_character":66},"in_reply_to":"3fa7e38b_db0da540","updated":"2019-12-06 16:09:21.000000000","message":"/me forgot to add the slash, I added since it can be one of the cases for a valid file to look for..","commit_id":"15cd37ce66d0a329b4a753731b54e4a815110fda"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"292f85ac73bdf6c21b6728f6cea5e7890cfe56fa","unresolved":false,"context_lines":[{"line_number":229,"context_line":"            efi_partition \u003d utils.get_efi_part_on_device(device)"},{"line_number":230,"context_line":""},{"line_number":231,"context_line":"        if efi_partition:"},{"line_number":232,"context_line":"            utils.execute(\u0027mount\u0027, root_partition, path)"},{"line_number":233,"context_line":"            utils.execute(\u0027mount\u0027, \u0027-t\u0027, \u0027sysfs\u0027, \u0027none\u0027, path + \u0027/sys\u0027)"},{"line_number":234,"context_line":"            efi_partition_mount_point \u003d os.path.join(path, \"boot/efi\")"},{"line_number":235,"context_line":"            if not os.path.exists(efi_partition_mount_point):"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_fb12e160","line":232,"updated":"2019-12-06 15:56:12.000000000","message":"nit: we may want to mount read-only.","commit_id":"15cd37ce66d0a329b4a753731b54e4a815110fda"},{"author":{"_account_id":15519,"name":"Iury Gregory Melo Ferreira","display_name":"Iury Gregory","email":"iurygregory@gmail.com","username":"iurygregory"},"change_message_id":"1ad194e5c67ba22fb8034dd446add07294599bd3","unresolved":false,"context_lines":[{"line_number":229,"context_line":"            efi_partition \u003d utils.get_efi_part_on_device(device)"},{"line_number":230,"context_line":""},{"line_number":231,"context_line":"        if efi_partition:"},{"line_number":232,"context_line":"            utils.execute(\u0027mount\u0027, root_partition, path)"},{"line_number":233,"context_line":"            utils.execute(\u0027mount\u0027, \u0027-t\u0027, \u0027sysfs\u0027, \u0027none\u0027, path + \u0027/sys\u0027)"},{"line_number":234,"context_line":"            efi_partition_mount_point \u003d os.path.join(path, \"boot/efi\")"},{"line_number":235,"context_line":"            if not os.path.exists(efi_partition_mount_point):"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_2da4c006","line":232,"in_reply_to":"3fa7e38b_fb12e160","updated":"2019-12-06 16:09:21.000000000","message":"ack","commit_id":"15cd37ce66d0a329b4a753731b54e4a815110fda"},{"author":{"_account_id":21909,"name":"Bob Fournier","email":"bfournie@redhat.com","username":"bfournie"},"change_message_id":"ef4a6a56e6540c79be7707d6f1baf621720a0ab1","unresolved":false,"context_lines":[{"line_number":245,"context_line":"        valid_efi_bootloaders \u003d _found_bootloader(efi_partition_mount_point)"},{"line_number":246,"context_line":"        for v_efi_bl_path in valid_efi_bootloaders:"},{"line_number":247,"context_line":"            # Update the nvram using efibootmgr"},{"line_number":248,"context_line":"            utils.execute(\u0027efibootmgr\u0027, \u0027--create\u0027, \u0027--disk\u0027, device,"},{"line_number":249,"context_line":"                          \u0027--part\u0027, efi_partition, \u0027--label\u0027, \u0027efi\u0027,"},{"line_number":250,"context_line":"                          \u0027--loader\u0027, v_efi_bl_path)"},{"line_number":251,"context_line":"        return True"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_ad8c2135","line":248,"range":{"start_line":248,"start_character":27,"end_line":248,"end_character":37},"updated":"2019-12-09 01:35:57.000000000","message":"I think need to run this with chroot, similar to how grub2-install is run (https://github.com/openstack/ironic-python-agent/blob/master/ironic_python_agent/extensions/image.py#L251).  I don\u0027t think the efibootmgr pkg is currently included in the ramdisk.","commit_id":"15cd37ce66d0a329b4a753731b54e4a815110fda"},{"author":{"_account_id":15519,"name":"Iury Gregory Melo Ferreira","display_name":"Iury Gregory","email":"iurygregory@gmail.com","username":"iurygregory"},"change_message_id":"0f1d37d465faf101d5732abfe94a59a9265c3b5f","unresolved":false,"context_lines":[{"line_number":245,"context_line":"        valid_efi_bootloaders \u003d _found_bootloader(efi_partition_mount_point)"},{"line_number":246,"context_line":"        for v_efi_bl_path in valid_efi_bootloaders:"},{"line_number":247,"context_line":"            # Update the nvram using efibootmgr"},{"line_number":248,"context_line":"            utils.execute(\u0027efibootmgr\u0027, \u0027--create\u0027, \u0027--disk\u0027, device,"},{"line_number":249,"context_line":"                          \u0027--part\u0027, efi_partition, \u0027--label\u0027, \u0027efi\u0027,"},{"line_number":250,"context_line":"                          \u0027--loader\u0027, v_efi_bl_path)"},{"line_number":251,"context_line":"        return True"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_c2fda709","line":248,"range":{"start_line":248,"start_character":27,"end_line":248,"end_character":37},"in_reply_to":"3fa7e38b_ad8c2135","updated":"2019-12-09 08:51:28.000000000","message":"About the chroot I asked Julia on the channel [1] and this may not be necessary \n\nSo we need to update the the ipa-builder to include efibootmgr before we move with this? \n\n\n[1]http://eavesdrop.openstack.org/irclogs/%23openstack-ironic/%23openstack-ironic.2019-12-06.log.html#t2019-12-06T14:46:33","commit_id":"15cd37ce66d0a329b4a753731b54e4a815110fda"},{"author":{"_account_id":21909,"name":"Bob Fournier","email":"bfournie@redhat.com","username":"bfournie"},"change_message_id":"428744b879acf801963d618a68d69eda99640175","unresolved":false,"context_lines":[{"line_number":245,"context_line":"        valid_efi_bootloaders \u003d _found_bootloader(efi_partition_mount_point)"},{"line_number":246,"context_line":"        for v_efi_bl_path in valid_efi_bootloaders:"},{"line_number":247,"context_line":"            # Update the nvram using efibootmgr"},{"line_number":248,"context_line":"            utils.execute(\u0027efibootmgr\u0027, \u0027--create\u0027, \u0027--disk\u0027, device,"},{"line_number":249,"context_line":"                          \u0027--part\u0027, efi_partition, \u0027--label\u0027, \u0027efi\u0027,"},{"line_number":250,"context_line":"                          \u0027--loader\u0027, v_efi_bl_path)"},{"line_number":251,"context_line":"        return True"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_ce6cf034","line":248,"range":{"start_line":248,"start_character":27,"end_line":248,"end_character":37},"in_reply_to":"3fa7e38b_c2fda709","updated":"2019-12-09 13:37:42.000000000","message":"Yeah if we don\u0027t use chroot we\u0027ll need to include efibootmgr.","commit_id":"15cd37ce66d0a329b4a753731b54e4a815110fda"},{"author":{"_account_id":15519,"name":"Iury Gregory Melo Ferreira","display_name":"Iury Gregory","email":"iurygregory@gmail.com","username":"iurygregory"},"change_message_id":"e33d1ed8f2b4d8b12aecd3bdfa82157ac9a0d0a4","unresolved":false,"context_lines":[{"line_number":245,"context_line":"        valid_efi_bootloaders \u003d _found_bootloader(efi_partition_mount_point)"},{"line_number":246,"context_line":"        for v_efi_bl_path in valid_efi_bootloaders:"},{"line_number":247,"context_line":"            # Update the nvram using efibootmgr"},{"line_number":248,"context_line":"            utils.execute(\u0027efibootmgr\u0027, \u0027--create\u0027, \u0027--disk\u0027, device,"},{"line_number":249,"context_line":"                          \u0027--part\u0027, efi_partition, \u0027--label\u0027, \u0027efi\u0027,"},{"line_number":250,"context_line":"                          \u0027--loader\u0027, v_efi_bl_path)"},{"line_number":251,"context_line":"        return True"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_892a727b","line":248,"range":{"start_line":248,"start_character":27,"end_line":248,"end_character":37},"in_reply_to":"3fa7e38b_ce6cf034","updated":"2019-12-09 13:59:34.000000000","message":"Ack, Dmitry pointed me some docs","commit_id":"15cd37ce66d0a329b4a753731b54e4a815110fda"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"292f85ac73bdf6c21b6728f6cea5e7890cfe56fa","unresolved":false,"context_lines":[{"line_number":247,"context_line":"            # Update the nvram using efibootmgr"},{"line_number":248,"context_line":"            utils.execute(\u0027efibootmgr\u0027, \u0027--create\u0027, \u0027--disk\u0027, device,"},{"line_number":249,"context_line":"                          \u0027--part\u0027, efi_partition, \u0027--label\u0027, \u0027efi\u0027,"},{"line_number":250,"context_line":"                          \u0027--loader\u0027, v_efi_bl_path)"},{"line_number":251,"context_line":"        return True"},{"line_number":252,"context_line":"    except processutils.ProcessExecutionError as e:"},{"line_number":253,"context_line":"        error_msg \u003d (\u0027Could not verify secure boot on device %(dev)s\u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_ad2c3093","line":250,"updated":"2019-12-06 15:56:12.000000000","message":"Does efibootmgr go ahead and strip the user space relative path for the mount to update the loader? Because if it doesn\u0027t, I suspect this needs to be an absolute path?","commit_id":"15cd37ce66d0a329b4a753731b54e4a815110fda"},{"author":{"_account_id":15519,"name":"Iury Gregory Melo Ferreira","display_name":"Iury Gregory","email":"iurygregory@gmail.com","username":"iurygregory"},"change_message_id":"1ad194e5c67ba22fb8034dd446add07294599bd3","unresolved":false,"context_lines":[{"line_number":247,"context_line":"            # Update the nvram using efibootmgr"},{"line_number":248,"context_line":"            utils.execute(\u0027efibootmgr\u0027, \u0027--create\u0027, \u0027--disk\u0027, device,"},{"line_number":249,"context_line":"                          \u0027--part\u0027, efi_partition, \u0027--label\u0027, \u0027efi\u0027,"},{"line_number":250,"context_line":"                          \u0027--loader\u0027, v_efi_bl_path)"},{"line_number":251,"context_line":"        return True"},{"line_number":252,"context_line":"    except processutils.ProcessExecutionError as e:"},{"line_number":253,"context_line":"        error_msg \u003d (\u0027Could not verify secure boot on device %(dev)s\u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_2d5920fc","line":250,"in_reply_to":"3fa7e38b_ad2c3093","updated":"2019-12-06 16:09:21.000000000","message":"I\u0027ve  being doing some search and the examples I\u0027ve found they don\u0027t use the absolute path\n\nhttps://wiki.gentoo.org/wiki/Efibootmgr/en \nhttps://wiki.debian.org/UEFI\nhttps://wiki.archlinux.org/index.php/EFISTUB\n\nSo if the path was \u0027/boot/efi/EFI/BOOT/BOOTX64.EFI\u0027\nI\u0027m converting this to \u0027\\EFI\\BOOT\\BOOTX64.EFI\u0027to be used on the loader.","commit_id":"15cd37ce66d0a329b4a753731b54e4a815110fda"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"ac759cd15180dfeb278e71da8aeda155befae315","unresolved":false,"context_lines":[{"line_number":34,"context_line":""},{"line_number":35,"context_line":"BIND_MOUNTS \u003d (\u0027/dev\u0027, \u0027/proc\u0027, \u0027/run\u0027)"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"BOOTLOADERS_EFI \u003d [\u0027/bootx64.efi\u0027, \u0027/grubaa64.efi\u0027]"},{"line_number":38,"context_line":""},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"def _get_partition(device, uuid):"}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_24259cbf","line":37,"updated":"2019-12-09 22:16:38.000000000","message":"bonus points to add winload.efi","commit_id":"9b1fc6a3f4133b32cfb2682849d94460a38d302e"},{"author":{"_account_id":15519,"name":"Iury Gregory Melo Ferreira","display_name":"Iury Gregory","email":"iurygregory@gmail.com","username":"iurygregory"},"change_message_id":"7542da01a42df932b33e9bd5ff5b78d33fb16b3e","unresolved":false,"context_lines":[{"line_number":34,"context_line":""},{"line_number":35,"context_line":"BIND_MOUNTS \u003d (\u0027/dev\u0027, \u0027/proc\u0027, \u0027/run\u0027)"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"BOOTLOADERS_EFI \u003d [\u0027/bootx64.efi\u0027, \u0027/grubaa64.efi\u0027]"},{"line_number":38,"context_line":""},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"def _get_partition(device, uuid):"}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_4208be8f","line":37,"in_reply_to":"3fa7e38b_24259cbf","updated":"2019-12-10 15:13:44.000000000","message":"Done","commit_id":"9b1fc6a3f4133b32cfb2682849d94460a38d302e"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"ac759cd15180dfeb278e71da8aeda155befae315","unresolved":false,"context_lines":[{"line_number":198,"context_line":"        efi_files \u003d efi_search[0].rstrip().split(\u0027\\n\u0027)"},{"line_number":199,"context_line":"        valid_bootloaders \u003d []"},{"line_number":200,"context_line":"        for efi_f in efi_files:"},{"line_number":201,"context_line":"            if any(bl in efi_f.lower() for bl in BOOTLOADERS_EFI):"},{"line_number":202,"context_line":"                LOG.debug(\u0027checking if %s has boot sector\u0027, efi_f)"},{"line_number":203,"context_line":"                stdout, stderr \u003d utils.execute(\u0027file\u0027, efi_f)"},{"line_number":204,"context_line":"                if \u0027executable\u0027 in stdout:"}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_1f13095b","line":201,"updated":"2019-12-09 22:16:38.000000000","message":"So do we care at all about paths these files are in. I ask because my work laptop has three BOOTX64.EFI files, and while they are all the same checksum... I just wonder.","commit_id":"9b1fc6a3f4133b32cfb2682849d94460a38d302e"},{"author":{"_account_id":15519,"name":"Iury Gregory Melo Ferreira","display_name":"Iury Gregory","email":"iurygregory@gmail.com","username":"iurygregory"},"change_message_id":"b8c9055aafebcee07406a104511e09002d30fc3a","unresolved":false,"context_lines":[{"line_number":198,"context_line":"        efi_files \u003d efi_search[0].rstrip().split(\u0027\\n\u0027)"},{"line_number":199,"context_line":"        valid_bootloaders \u003d []"},{"line_number":200,"context_line":"        for efi_f in efi_files:"},{"line_number":201,"context_line":"            if any(bl in efi_f.lower() for bl in BOOTLOADERS_EFI):"},{"line_number":202,"context_line":"                LOG.debug(\u0027checking if %s has boot sector\u0027, efi_f)"},{"line_number":203,"context_line":"                stdout, stderr \u003d utils.execute(\u0027file\u0027, efi_f)"},{"line_number":204,"context_line":"                if \u0027executable\u0027 in stdout:"}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_5c1665f4","line":201,"in_reply_to":"3fa7e38b_1f13095b","updated":"2019-12-10 13:13:39.000000000","message":"we will need the path to pass to the efibootmgr without the efi_partition_mount_point","commit_id":"9b1fc6a3f4133b32cfb2682849d94460a38d302e"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"ac759cd15180dfeb278e71da8aeda155befae315","unresolved":false,"context_lines":[{"line_number":199,"context_line":"        valid_bootloaders \u003d []"},{"line_number":200,"context_line":"        for efi_f in efi_files:"},{"line_number":201,"context_line":"            if any(bl in efi_f.lower() for bl in BOOTLOADERS_EFI):"},{"line_number":202,"context_line":"                LOG.debug(\u0027checking if %s has boot sector\u0027, efi_f)"},{"line_number":203,"context_line":"                stdout, stderr \u003d utils.execute(\u0027file\u0027, efi_f)"},{"line_number":204,"context_line":"                if \u0027executable\u0027 in stdout:"},{"line_number":205,"context_line":"                    v_bl \u003d efi_f.replace(\u0027/\u0027, \u0027\\\\\u0027)"}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_9f20599c","line":202,"range":{"start_line":202,"start_character":27,"end_line":202,"end_character":28},"updated":"2019-12-09 22:16:38.000000000","message":"nit s/c/C/\n\nSo, I think what we\u0027re looking for is machine executable, not a boot sector. :)","commit_id":"9b1fc6a3f4133b32cfb2682849d94460a38d302e"},{"author":{"_account_id":15519,"name":"Iury Gregory Melo Ferreira","display_name":"Iury Gregory","email":"iurygregory@gmail.com","username":"iurygregory"},"change_message_id":"b8c9055aafebcee07406a104511e09002d30fc3a","unresolved":false,"context_lines":[{"line_number":199,"context_line":"        valid_bootloaders \u003d []"},{"line_number":200,"context_line":"        for efi_f in efi_files:"},{"line_number":201,"context_line":"            if any(bl in efi_f.lower() for bl in BOOTLOADERS_EFI):"},{"line_number":202,"context_line":"                LOG.debug(\u0027checking if %s has boot sector\u0027, efi_f)"},{"line_number":203,"context_line":"                stdout, stderr \u003d utils.execute(\u0027file\u0027, efi_f)"},{"line_number":204,"context_line":"                if \u0027executable\u0027 in stdout:"},{"line_number":205,"context_line":"                    v_bl \u003d efi_f.replace(\u0027/\u0027, \u0027\\\\\u0027)"}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_65a4ebb8","line":202,"range":{"start_line":202,"start_character":27,"end_line":202,"end_character":28},"in_reply_to":"3fa7e38b_9f20599c","updated":"2019-12-10 13:13:39.000000000","message":"Done","commit_id":"9b1fc6a3f4133b32cfb2682849d94460a38d302e"},{"author":{"_account_id":23851,"name":"Riccardo Pittau","email":"elfosardo@gmail.com","username":"elfosardo"},"change_message_id":"e1372e9a45d71e7b87d9d5463df0cb742fb3e96d","unresolved":false,"context_lines":[{"line_number":214,"context_line":"    efi_mounted \u003d False"},{"line_number":215,"context_line":""},{"line_number":216,"context_line":"    try:"},{"line_number":217,"context_line":"        path \u003d tempfile.mkdtemp()"},{"line_number":218,"context_line":""},{"line_number":219,"context_line":"        if efi_system_part_uuid:"},{"line_number":220,"context_line":"            # Maybe add more logic if the give uuid doesn\u0027t work we can try:"}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_00888568","line":217,"range":{"start_line":217,"start_character":8,"end_line":217,"end_character":12},"updated":"2019-12-10 10:03:29.000000000","message":"maybe use something different from \u0027path\u0027 here","commit_id":"9b1fc6a3f4133b32cfb2682849d94460a38d302e"},{"author":{"_account_id":15519,"name":"Iury Gregory Melo Ferreira","display_name":"Iury Gregory","email":"iurygregory@gmail.com","username":"iurygregory"},"change_message_id":"b8c9055aafebcee07406a104511e09002d30fc3a","unresolved":false,"context_lines":[{"line_number":214,"context_line":"    efi_mounted \u003d False"},{"line_number":215,"context_line":""},{"line_number":216,"context_line":"    try:"},{"line_number":217,"context_line":"        path \u003d tempfile.mkdtemp()"},{"line_number":218,"context_line":""},{"line_number":219,"context_line":"        if efi_system_part_uuid:"},{"line_number":220,"context_line":"            # Maybe add more logic if the give uuid doesn\u0027t work we can try:"}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_7c11e1f7","line":217,"range":{"start_line":217,"start_character":8,"end_line":217,"end_character":12},"in_reply_to":"3fa7e38b_00888568","updated":"2019-12-10 13:13:39.000000000","message":"Done","commit_id":"9b1fc6a3f4133b32cfb2682849d94460a38d302e"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"ac759cd15180dfeb278e71da8aeda155befae315","unresolved":false,"context_lines":[{"line_number":235,"context_line":"                          efi_partition_mount_point)"},{"line_number":236,"context_line":"            efi_mounted \u003d True"},{"line_number":237,"context_line":"        else:"},{"line_number":238,"context_line":"            # If we can\u0027t find the partition we need to decide what should"},{"line_number":239,"context_line":"            # happen"},{"line_number":240,"context_line":"            return False"},{"line_number":241,"context_line":""},{"line_number":242,"context_line":"        valid_efi_bootloaders \u003d _found_bootloader(efi_partition_mount_point)"}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_3facc5ec","line":239,"range":{"start_line":238,"start_character":11,"end_line":239,"end_character":20},"updated":"2019-12-09 22:16:38.000000000","message":"Well, this is a good question. And definitely out of scope of the focus of this patch.","commit_id":"9b1fc6a3f4133b32cfb2682849d94460a38d302e"},{"author":{"_account_id":15519,"name":"Iury Gregory Melo Ferreira","display_name":"Iury Gregory","email":"iurygregory@gmail.com","username":"iurygregory"},"change_message_id":"b8c9055aafebcee07406a104511e09002d30fc3a","unresolved":false,"context_lines":[{"line_number":235,"context_line":"                          efi_partition_mount_point)"},{"line_number":236,"context_line":"            efi_mounted \u003d True"},{"line_number":237,"context_line":"        else:"},{"line_number":238,"context_line":"            # If we can\u0027t find the partition we need to decide what should"},{"line_number":239,"context_line":"            # happen"},{"line_number":240,"context_line":"            return False"},{"line_number":241,"context_line":""},{"line_number":242,"context_line":"        valid_efi_bootloaders \u003d _found_bootloader(efi_partition_mount_point)"}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_65af0b9d","line":239,"range":{"start_line":238,"start_character":11,"end_line":239,"end_character":20},"in_reply_to":"3fa7e38b_3facc5ec","updated":"2019-12-10 13:13:39.000000000","message":"I will keep this as note so we don\u0027t forget, also if this function returns False I\u0027ve put to call _install_grub2 on install_bootloader.","commit_id":"9b1fc6a3f4133b32cfb2682849d94460a38d302e"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"ac759cd15180dfeb278e71da8aeda155befae315","unresolved":false,"context_lines":[{"line_number":240,"context_line":"            return False"},{"line_number":241,"context_line":""},{"line_number":242,"context_line":"        valid_efi_bootloaders \u003d _found_bootloader(efi_partition_mount_point)"},{"line_number":243,"context_line":"        for v_efi_bl_path in valid_efi_bootloaders:"},{"line_number":244,"context_line":"            # Update the nvram using efibootmgr"},{"line_number":245,"context_line":"            # https://linux.die.net/man/8/efibootmgr"},{"line_number":246,"context_line":"            LOG.debug(\u0027Adding loader %s on partition %s of device %s\u0027,"}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_1faf89e0","line":243,"updated":"2019-12-09 22:16:38.000000000","message":"I like!","commit_id":"9b1fc6a3f4133b32cfb2682849d94460a38d302e"},{"author":{"_account_id":21909,"name":"Bob Fournier","email":"bfournie@redhat.com","username":"bfournie"},"change_message_id":"0df7c40c1c8068569367acc7fc876fc01992a359","unresolved":false,"context_lines":[{"line_number":245,"context_line":"            # https://linux.die.net/man/8/efibootmgr"},{"line_number":246,"context_line":"            LOG.debug(\u0027Adding loader %s on partition %s of device %s\u0027,"},{"line_number":247,"context_line":"                      (v_efi_bl_path, device, efi_partition))"},{"line_number":248,"context_line":"            utils.execute(\u0027efibootmgr\u0027, \u0027-c\u0027, \u0027-d\u0027, device,"},{"line_number":249,"context_line":"                          \u0027-p\u0027, efi_partition, \u0027-L\u0027, \u0027efi\u0027,"},{"line_number":250,"context_line":"                          \u0027-l\u0027, v_efi_bl_path)"},{"line_number":251,"context_line":"        return True if valid_efi_bootloaders else False"},{"line_number":252,"context_line":"    except processutils.ProcessExecutionError as e:"},{"line_number":253,"context_line":"        error_msg \u003d (\u0027Could not verify secure boot on device %(dev)s\u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_22b7c2da","line":250,"range":{"start_line":248,"start_character":27,"end_line":250,"end_character":46},"updated":"2019-12-10 15:05:50.000000000","message":"Question - when grub2-install calls efibootmgr it uses the -w flag to write a unique signature (https://github.com/coreos/grub/blob/2.02-coreos/grub-core/osdep/unix/platform.c#L161).  Do we need to do the same here?","commit_id":"9b1fc6a3f4133b32cfb2682849d94460a38d302e"},{"author":{"_account_id":15519,"name":"Iury Gregory Melo Ferreira","display_name":"Iury Gregory","email":"iurygregory@gmail.com","username":"iurygregory"},"change_message_id":"7542da01a42df932b33e9bd5ff5b78d33fb16b3e","unresolved":false,"context_lines":[{"line_number":245,"context_line":"            # https://linux.die.net/man/8/efibootmgr"},{"line_number":246,"context_line":"            LOG.debug(\u0027Adding loader %s on partition %s of device %s\u0027,"},{"line_number":247,"context_line":"                      (v_efi_bl_path, device, efi_partition))"},{"line_number":248,"context_line":"            utils.execute(\u0027efibootmgr\u0027, \u0027-c\u0027, \u0027-d\u0027, device,"},{"line_number":249,"context_line":"                          \u0027-p\u0027, efi_partition, \u0027-L\u0027, \u0027efi\u0027,"},{"line_number":250,"context_line":"                          \u0027-l\u0027, v_efi_bl_path)"},{"line_number":251,"context_line":"        return True if valid_efi_bootloaders else False"},{"line_number":252,"context_line":"    except processutils.ProcessExecutionError as e:"},{"line_number":253,"context_line":"        error_msg \u003d (\u0027Could not verify secure boot on device %(dev)s\u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_020c0691","line":250,"range":{"start_line":248,"start_character":27,"end_line":250,"end_character":46},"in_reply_to":"3fa7e38b_22b7c2da","updated":"2019-12-10 15:13:44.000000000","message":"Interesting, \nwell according to docs  https://linux.die.net/man/8/efibootmgr\n\"-w | --write-signature write unique signature to the MBR if needed\"\n the \"if needed\" doesn\u0027t help \u003d(  \nMaybe would be worth adding...\n\nThe examples I was following didn\u0027t use any -w so I\u0027m not sure tbh\nhttps://wiki.gentoo.org/wiki/Efibootmgr\nhttps://wiki.archlinux.org/index.php/Unified_Extensible_Firmware_Interface#efibootmgr","commit_id":"9b1fc6a3f4133b32cfb2682849d94460a38d302e"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"ac759cd15180dfeb278e71da8aeda155befae315","unresolved":false,"context_lines":[{"line_number":250,"context_line":"                          \u0027-l\u0027, v_efi_bl_path)"},{"line_number":251,"context_line":"        return True if valid_efi_bootloaders else False"},{"line_number":252,"context_line":"    except processutils.ProcessExecutionError as e:"},{"line_number":253,"context_line":"        error_msg \u003d (\u0027Could not verify secure boot on device %(dev)s\u0027"},{"line_number":254,"context_line":"                     \u0027failed with %(err)s.\u0027 % {\u0027dev\u0027: device, \u0027err\u0027: e})"},{"line_number":255,"context_line":"        LOG.error(error_msg)"},{"line_number":256,"context_line":"        raise errors.CommandExecutionError(error_msg)"}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_9ff13903","line":253,"range":{"start_line":253,"start_character":32,"end_line":253,"end_character":51},"updated":"2019-12-09 22:16:38.000000000","message":"This is actually misleading. We\u0027re not focusing on secure boot, we\u0027re focusing on the on-disk content which if it came from a vendor package may be signed and thus secure bootable.","commit_id":"9b1fc6a3f4133b32cfb2682849d94460a38d302e"},{"author":{"_account_id":15519,"name":"Iury Gregory Melo Ferreira","display_name":"Iury Gregory","email":"iurygregory@gmail.com","username":"iurygregory"},"change_message_id":"b8c9055aafebcee07406a104511e09002d30fc3a","unresolved":false,"context_lines":[{"line_number":250,"context_line":"                          \u0027-l\u0027, v_efi_bl_path)"},{"line_number":251,"context_line":"        return True if valid_efi_bootloaders else False"},{"line_number":252,"context_line":"    except processutils.ProcessExecutionError as e:"},{"line_number":253,"context_line":"        error_msg \u003d (\u0027Could not verify secure boot on device %(dev)s\u0027"},{"line_number":254,"context_line":"                     \u0027failed with %(err)s.\u0027 % {\u0027dev\u0027: device, \u0027err\u0027: e})"},{"line_number":255,"context_line":"        LOG.error(error_msg)"},{"line_number":256,"context_line":"        raise errors.CommandExecutionError(error_msg)"}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_e5669bbc","line":253,"range":{"start_line":253,"start_character":32,"end_line":253,"end_character":51},"in_reply_to":"3fa7e38b_9ff13903","updated":"2019-12-10 13:13:39.000000000","message":"I forgot to update this message, doing now","commit_id":"9b1fc6a3f4133b32cfb2682849d94460a38d302e"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"ac759cd15180dfeb278e71da8aeda155befae315","unresolved":false,"context_lines":[{"line_number":287,"context_line":"            else:"},{"line_number":288,"context_line":"                # After everything is umounted we can then remove the"},{"line_number":289,"context_line":"                # temporary directory"},{"line_number":290,"context_line":"                shutil.rmtree(path)"},{"line_number":291,"context_line":""},{"line_number":292,"context_line":""},{"line_number":293,"context_line":"def _install_grub2(device, root_uuid, efi_system_part_uuid\u003dNone,"}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_bfcf55b1","line":290,"updated":"2019-12-09 22:16:38.000000000","message":"For some reason my brain thinks we should issue fire off utils.execute(\u0027sync\u0027) before exiting on normal conditions. Technically we may change FS contents by making a folder, and we\u0027re definitely changing the last access time markers.","commit_id":"9b1fc6a3f4133b32cfb2682849d94460a38d302e"},{"author":{"_account_id":23851,"name":"Riccardo Pittau","email":"elfosardo@gmail.com","username":"elfosardo"},"change_message_id":"e1372e9a45d71e7b87d9d5463df0cb742fb3e96d","unresolved":false,"context_lines":[{"line_number":287,"context_line":"            else:"},{"line_number":288,"context_line":"                # After everything is umounted we can then remove the"},{"line_number":289,"context_line":"                # temporary directory"},{"line_number":290,"context_line":"                shutil.rmtree(path)"},{"line_number":291,"context_line":""},{"line_number":292,"context_line":""},{"line_number":293,"context_line":"def _install_grub2(device, root_uuid, efi_system_part_uuid\u003dNone,"}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_250b532b","line":290,"in_reply_to":"3fa7e38b_bfcf55b1","updated":"2019-12-10 10:03:29.000000000","message":"@Julia umount already completes all pending writes before actually unmounting the FS; just to clarify, are you suggesting to add it after the rmtree command ?","commit_id":"9b1fc6a3f4133b32cfb2682849d94460a38d302e"},{"author":{"_account_id":21909,"name":"Bob Fournier","email":"bfournie@redhat.com","username":"bfournie"},"change_message_id":"0df7c40c1c8068569367acc7fc876fc01992a359","unresolved":false,"context_lines":[{"line_number":348,"context_line":"            binary_name \u003d \"grub2\""},{"line_number":349,"context_line":""},{"line_number":350,"context_line":"        # Add /bin to PATH variable as grub requires it to find efibootmgr"},{"line_number":351,"context_line":"        # when running in uefi boot mode."},{"line_number":352,"context_line":"        # Add /usr/sbin to PATH variable to ensure it is there as we do"},{"line_number":353,"context_line":"        # not use full path to grub binary anymore."},{"line_number":354,"context_line":"        path_variable \u003d os.environ.get(\u0027PATH\u0027, \u0027\u0027)"}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_62e13a2b","line":351,"range":{"start_line":351,"start_character":0,"end_line":351,"end_character":41},"updated":"2019-12-10 15:05:50.000000000","message":"Nit - may want to add another comment here. We should not be in this code and calling grub2-install if in uefi boot mode, unless not able to verify uefi.","commit_id":"9b1fc6a3f4133b32cfb2682849d94460a38d302e"},{"author":{"_account_id":15519,"name":"Iury Gregory Melo Ferreira","display_name":"Iury Gregory","email":"iurygregory@gmail.com","username":"iurygregory"},"change_message_id":"7542da01a42df932b33e9bd5ff5b78d33fb16b3e","unresolved":false,"context_lines":[{"line_number":348,"context_line":"            binary_name \u003d \"grub2\""},{"line_number":349,"context_line":""},{"line_number":350,"context_line":"        # Add /bin to PATH variable as grub requires it to find efibootmgr"},{"line_number":351,"context_line":"        # when running in uefi boot mode."},{"line_number":352,"context_line":"        # Add /usr/sbin to PATH variable to ensure it is there as we do"},{"line_number":353,"context_line":"        # not use full path to grub binary anymore."},{"line_number":354,"context_line":"        path_variable \u003d os.environ.get(\u0027PATH\u0027, \u0027\u0027)"}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_c266ee55","line":351,"range":{"start_line":351,"start_character":0,"end_line":351,"end_character":41},"in_reply_to":"3fa7e38b_62e13a2b","updated":"2019-12-10 15:13:44.000000000","message":"Will do","commit_id":"9b1fc6a3f4133b32cfb2682849d94460a38d302e"},{"author":{"_account_id":21909,"name":"Bob Fournier","email":"bfournie@redhat.com","username":"bfournie"},"change_message_id":"25db4414ec6f78525d90339a698e059c82ead858","unresolved":false,"context_lines":[{"line_number":284,"context_line":"                        {\u0027path\u0027: local_path + \u0027/sys\u0027, \u0027error\u0027: e})"},{"line_number":285,"context_line":""},{"line_number":286,"context_line":"        # If umounting the binds succeed then we can try to delete it"},{"line_number":287,"context_line":"        if not umount_binds_fail:"},{"line_number":288,"context_line":"            try:"},{"line_number":289,"context_line":"                utils.execute(\u0027umount\u0027, local_path, attempts\u003d3,"},{"line_number":290,"context_line":"                              delay_on_retry\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_ab5e2d42","line":287,"range":{"start_line":287,"start_character":7,"end_line":287,"end_character":33},"updated":"2019-12-10 21:49:07.000000000","message":"Nit - can make this an \"else\" and not need \u0027umount_binds_fail\u0027","commit_id":"79edac9f8c8c3917a25da2f313543beaaf147297"},{"author":{"_account_id":15519,"name":"Iury Gregory Melo Ferreira","display_name":"Iury Gregory","email":"iurygregory@gmail.com","username":"iurygregory"},"change_message_id":"c91d27193f9935a91737f8340c6cb61ef5132170","unresolved":false,"context_lines":[{"line_number":284,"context_line":"                        {\u0027path\u0027: local_path + \u0027/sys\u0027, \u0027error\u0027: e})"},{"line_number":285,"context_line":""},{"line_number":286,"context_line":"        # If umounting the binds succeed then we can try to delete it"},{"line_number":287,"context_line":"        if not umount_binds_fail:"},{"line_number":288,"context_line":"            try:"},{"line_number":289,"context_line":"                utils.execute(\u0027umount\u0027, local_path, attempts\u003d3,"},{"line_number":290,"context_line":"                              delay_on_retry\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_ceda8d48","line":287,"range":{"start_line":287,"start_character":7,"end_line":287,"end_character":33},"in_reply_to":"3fa7e38b_ab5e2d42","updated":"2019-12-11 09:47:45.000000000","message":"Done","commit_id":"79edac9f8c8c3917a25da2f313543beaaf147297"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"24a2c780b3631674f2c66da43af4a303cefc3a78","unresolved":false,"context_lines":[{"line_number":34,"context_line":""},{"line_number":35,"context_line":"BIND_MOUNTS \u003d (\u0027/dev\u0027, \u0027/proc\u0027, \u0027/run\u0027)"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"BOOTLOADERS_EFI \u003d [\u0027/bootx64.efi\u0027, \u0027/grubaa64.efi\u0027, \u0027/winload.efi\u0027]"},{"line_number":38,"context_line":""},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"def _get_partition(device, uuid):"}],"source_content_type":"text/x-python","patch_set":5,"id":"3fa7e38b_68490930","line":37,"updated":"2019-12-11 18:37:32.000000000","message":"\u003c3","commit_id":"8e6bd3eeaaef9b878b127878e5024b6039d31384"},{"author":{"_account_id":23851,"name":"Riccardo Pittau","email":"elfosardo@gmail.com","username":"elfosardo"},"change_message_id":"b789d1e91d29cdb1f1d731aece4fe33df3058a60","unresolved":false,"context_lines":[{"line_number":184,"context_line":"def _get_efi_bootloaders(location):"},{"line_number":185,"context_line":"    # Let\u0027s find all files with .efi or .EFI extension"},{"line_number":186,"context_line":"    LOG.debug(\u0027Looking for all efi files\u0027)"},{"line_number":187,"context_line":"    efi_search \u003d utils.execute(\u0027find\u0027, location,"},{"line_number":188,"context_line":"                               \u0027-type\u0027, \u0027f\u0027, \u0027-iname\u0027, \u0027*.efi\u0027)"},{"line_number":189,"context_line":"    efi_files \u003d efi_search[0].rstrip().split(\u0027\\n\u0027)"},{"line_number":190,"context_line":"    valid_bootloaders \u003d []"}],"source_content_type":"text/x-python","patch_set":5,"id":"3fa7e38b_d4545aff","line":187,"updated":"2019-12-11 11:38:37.000000000","message":"just thinking if maybe we can use os.walk() instead of calling find here","commit_id":"8e6bd3eeaaef9b878b127878e5024b6039d31384"},{"author":{"_account_id":15519,"name":"Iury Gregory Melo Ferreira","display_name":"Iury Gregory","email":"iurygregory@gmail.com","username":"iurygregory"},"change_message_id":"73af18261815ff977b6f09224fb8730ec77428dc","unresolved":false,"context_lines":[{"line_number":184,"context_line":"def _get_efi_bootloaders(location):"},{"line_number":185,"context_line":"    # Let\u0027s find all files with .efi or .EFI extension"},{"line_number":186,"context_line":"    LOG.debug(\u0027Looking for all efi files\u0027)"},{"line_number":187,"context_line":"    efi_search \u003d utils.execute(\u0027find\u0027, location,"},{"line_number":188,"context_line":"                               \u0027-type\u0027, \u0027f\u0027, \u0027-iname\u0027, \u0027*.efi\u0027)"},{"line_number":189,"context_line":"    efi_files \u003d efi_search[0].rstrip().split(\u0027\\n\u0027)"},{"line_number":190,"context_line":"    valid_bootloaders \u003d []"}],"source_content_type":"text/x-python","patch_set":5,"id":"3fa7e38b_2217b048","line":187,"in_reply_to":"3fa7e38b_d4545aff","updated":"2019-12-11 14:21:48.000000000","message":"Done","commit_id":"8e6bd3eeaaef9b878b127878e5024b6039d31384"},{"author":{"_account_id":15519,"name":"Iury Gregory Melo Ferreira","display_name":"Iury Gregory","email":"iurygregory@gmail.com","username":"iurygregory"},"change_message_id":"d9ef5fe39b9fceb82c18765ac70b16bc7535a19d","unresolved":false,"context_lines":[{"line_number":184,"context_line":"def _get_efi_bootloaders(location):"},{"line_number":185,"context_line":"    # Let\u0027s find all files with .efi or .EFI extension"},{"line_number":186,"context_line":"    LOG.debug(\u0027Looking for all efi files\u0027)"},{"line_number":187,"context_line":"    efi_search \u003d utils.execute(\u0027find\u0027, location,"},{"line_number":188,"context_line":"                               \u0027-type\u0027, \u0027f\u0027, \u0027-iname\u0027, \u0027*.efi\u0027)"},{"line_number":189,"context_line":"    efi_files \u003d efi_search[0].rstrip().split(\u0027\\n\u0027)"},{"line_number":190,"context_line":"    valid_bootloaders \u003d []"}],"source_content_type":"text/x-python","patch_set":5,"id":"3fa7e38b_f774f89d","line":187,"in_reply_to":"3fa7e38b_d4545aff","updated":"2019-12-11 12:45:10.000000000","message":"I will take a look","commit_id":"8e6bd3eeaaef9b878b127878e5024b6039d31384"},{"author":{"_account_id":23851,"name":"Riccardo Pittau","email":"elfosardo@gmail.com","username":"elfosardo"},"change_message_id":"b789d1e91d29cdb1f1d731aece4fe33df3058a60","unresolved":false,"context_lines":[{"line_number":191,"context_line":"    for efi_f in efi_files:"},{"line_number":192,"context_line":"        if any(bl in efi_f.lower() for bl in BOOTLOADERS_EFI):"},{"line_number":193,"context_line":"            LOG.debug(\u0027Checking if %s is executable\u0027, efi_f)"},{"line_number":194,"context_line":"            stdout, stderr \u003d utils.execute(\u0027file\u0027, efi_f)"},{"line_number":195,"context_line":"            if \u0027executable\u0027 in stdout:"},{"line_number":196,"context_line":"                # Get the bootloader path"},{"line_number":197,"context_line":"                v_bl \u003d efi_f.split(\u0027/boot/efi\u0027)[-1].replace(\u0027/\u0027, \u0027\\\\\u0027)"}],"source_content_type":"text/x-python","patch_set":5,"id":"3fa7e38b_b403de0b","line":194,"updated":"2019-12-11 11:38:37.000000000","message":"maybe we can use os.access() to determine if the file is executable","commit_id":"8e6bd3eeaaef9b878b127878e5024b6039d31384"},{"author":{"_account_id":15519,"name":"Iury Gregory Melo Ferreira","display_name":"Iury Gregory","email":"iurygregory@gmail.com","username":"iurygregory"},"change_message_id":"73af18261815ff977b6f09224fb8730ec77428dc","unresolved":false,"context_lines":[{"line_number":191,"context_line":"    for efi_f in efi_files:"},{"line_number":192,"context_line":"        if any(bl in efi_f.lower() for bl in BOOTLOADERS_EFI):"},{"line_number":193,"context_line":"            LOG.debug(\u0027Checking if %s is executable\u0027, efi_f)"},{"line_number":194,"context_line":"            stdout, stderr \u003d utils.execute(\u0027file\u0027, efi_f)"},{"line_number":195,"context_line":"            if \u0027executable\u0027 in stdout:"},{"line_number":196,"context_line":"                # Get the bootloader path"},{"line_number":197,"context_line":"                v_bl \u003d efi_f.split(\u0027/boot/efi\u0027)[-1].replace(\u0027/\u0027, \u0027\\\\\u0027)"}],"source_content_type":"text/x-python","patch_set":5,"id":"3fa7e38b_820c6439","line":194,"in_reply_to":"3fa7e38b_b403de0b","updated":"2019-12-11 14:21:48.000000000","message":"Done","commit_id":"8e6bd3eeaaef9b878b127878e5024b6039d31384"},{"author":{"_account_id":15519,"name":"Iury Gregory Melo Ferreira","display_name":"Iury Gregory","email":"iurygregory@gmail.com","username":"iurygregory"},"change_message_id":"d9ef5fe39b9fceb82c18765ac70b16bc7535a19d","unresolved":false,"context_lines":[{"line_number":191,"context_line":"    for efi_f in efi_files:"},{"line_number":192,"context_line":"        if any(bl in efi_f.lower() for bl in BOOTLOADERS_EFI):"},{"line_number":193,"context_line":"            LOG.debug(\u0027Checking if %s is executable\u0027, efi_f)"},{"line_number":194,"context_line":"            stdout, stderr \u003d utils.execute(\u0027file\u0027, efi_f)"},{"line_number":195,"context_line":"            if \u0027executable\u0027 in stdout:"},{"line_number":196,"context_line":"                # Get the bootloader path"},{"line_number":197,"context_line":"                v_bl \u003d efi_f.split(\u0027/boot/efi\u0027)[-1].replace(\u0027/\u0027, \u0027\\\\\u0027)"}],"source_content_type":"text/x-python","patch_set":5,"id":"3fa7e38b_1778b48c","line":194,"in_reply_to":"3fa7e38b_b403de0b","updated":"2019-12-11 12:45:10.000000000","message":"I will take a look","commit_id":"8e6bd3eeaaef9b878b127878e5024b6039d31384"},{"author":{"_account_id":23851,"name":"Riccardo Pittau","email":"elfosardo@gmail.com","username":"elfosardo"},"change_message_id":"b789d1e91d29cdb1f1d731aece4fe33df3058a60","unresolved":false,"context_lines":[{"line_number":284,"context_line":"            try:"},{"line_number":285,"context_line":"                utils.execute(\u0027umount\u0027, local_path, attempts\u003d3,"},{"line_number":286,"context_line":"                              delay_on_retry\u003dTrue)"},{"line_number":287,"context_line":"                utils.execute(\u0027sync\u0027)"},{"line_number":288,"context_line":"            except processutils.ProcessExecutionError as e:"},{"line_number":289,"context_line":"                LOG.warning(umount_warn_msg, {\u0027path\u0027: local_path, \u0027error\u0027: e})"},{"line_number":290,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":5,"id":"3fa7e38b_14ebd211","line":287,"updated":"2019-12-11 11:38:37.000000000","message":"I\u0027m not sure having sync after umount is really useful.\nIf we want to have a redundant mechanism we can add it before, even if it\u0027s not really needed as umount already empties buffered data before executing the actual unmount","commit_id":"8e6bd3eeaaef9b878b127878e5024b6039d31384"},{"author":{"_account_id":15519,"name":"Iury Gregory Melo Ferreira","display_name":"Iury Gregory","email":"iurygregory@gmail.com","username":"iurygregory"},"change_message_id":"d9ef5fe39b9fceb82c18765ac70b16bc7535a19d","unresolved":false,"context_lines":[{"line_number":284,"context_line":"            try:"},{"line_number":285,"context_line":"                utils.execute(\u0027umount\u0027, local_path, attempts\u003d3,"},{"line_number":286,"context_line":"                              delay_on_retry\u003dTrue)"},{"line_number":287,"context_line":"                utils.execute(\u0027sync\u0027)"},{"line_number":288,"context_line":"            except processutils.ProcessExecutionError as e:"},{"line_number":289,"context_line":"                LOG.warning(umount_warn_msg, {\u0027path\u0027: local_path, \u0027error\u0027: e})"},{"line_number":290,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":5,"id":"3fa7e38b_976344dd","line":287,"in_reply_to":"3fa7e38b_14ebd211","updated":"2019-12-11 12:45:10.000000000","message":"yesterday when I asked Julia on irc she said after the unmount .-.\n\nhttp://eavesdrop.openstack.org/irclogs/%23openstack-ironic/%23openstack-ironic.2019-12-10.log.html#t2019-12-10T15:46:40","commit_id":"8e6bd3eeaaef9b878b127878e5024b6039d31384"},{"author":{"_account_id":23851,"name":"Riccardo Pittau","email":"elfosardo@gmail.com","username":"elfosardo"},"change_message_id":"33daae252e4dc064f16c0186dad18f3b3b06b4ff","unresolved":false,"context_lines":[{"line_number":205,"context_line":""},{"line_number":206,"context_line":""},{"line_number":207,"context_line":"def _run_efibootmgr(valid_efi_bootloaders, device, efi_partition):"},{"line_number":208,"context_line":"    \"\"\"Executes efibootmgr for a give list of efi bootloaders"},{"line_number":209,"context_line":""},{"line_number":210,"context_line":"    :param valid_efi_bootloaders: the list of valid efi bootloaders"},{"line_number":211,"context_line":"    :param device: the device to be used"}],"source_content_type":"text/x-python","patch_set":6,"id":"3fa7e38b_06e15a2b","line":208,"range":{"start_line":208,"start_character":33,"end_line":208,"end_character":37},"updated":"2019-12-12 09:28:45.000000000","message":"nit: given","commit_id":"d597dbe4cae8d8f521e5dec03eff6fd7cf1cf510"},{"author":{"_account_id":15519,"name":"Iury Gregory Melo Ferreira","display_name":"Iury Gregory","email":"iurygregory@gmail.com","username":"iurygregory"},"change_message_id":"1966d7c030bfcdaaddcf9b58d516a486cf33fcac","unresolved":false,"context_lines":[{"line_number":205,"context_line":""},{"line_number":206,"context_line":""},{"line_number":207,"context_line":"def _run_efibootmgr(valid_efi_bootloaders, device, efi_partition):"},{"line_number":208,"context_line":"    \"\"\"Executes efibootmgr for a give list of efi bootloaders"},{"line_number":209,"context_line":""},{"line_number":210,"context_line":"    :param valid_efi_bootloaders: the list of valid efi bootloaders"},{"line_number":211,"context_line":"    :param device: the device to be used"}],"source_content_type":"text/x-python","patch_set":6,"id":"3fa7e38b_06eb9adc","line":208,"range":{"start_line":208,"start_character":33,"end_line":208,"end_character":37},"in_reply_to":"3fa7e38b_06e15a2b","updated":"2019-12-12 10:33:59.000000000","message":"Done","commit_id":"d597dbe4cae8d8f521e5dec03eff6fd7cf1cf510"},{"author":{"_account_id":23851,"name":"Riccardo Pittau","email":"elfosardo@gmail.com","username":"elfosardo"},"change_message_id":"33daae252e4dc064f16c0186dad18f3b3b06b4ff","unresolved":false,"context_lines":[{"line_number":218,"context_line":"        LOG.debug(\u0027Adding loader %s on partition %s of device %s\u0027,"},{"line_number":219,"context_line":"                  (v_efi_bl_path, device, efi_partition))"},{"line_number":220,"context_line":"        utils.execute(\u0027efibootmgr\u0027, \u0027-c\u0027, \u0027-d\u0027, device,"},{"line_number":221,"context_line":"                      \u0027-p\u0027, efi_partition, \u0027-w\u0027, \u0027-L\u0027, \u0027efi\u0027,"},{"line_number":222,"context_line":"                      \u0027-l\u0027, v_efi_bl_path)"},{"line_number":223,"context_line":""},{"line_number":224,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"3fa7e38b_e6067ecd","line":221,"range":{"start_line":221,"start_character":48,"end_line":221,"end_character":60},"updated":"2019-12-12 09:28:45.000000000","message":"do we care that all the entries will have the same label? They don\u0027t have to be unique, but just wondering if we want to distinguish them in some way.","commit_id":"d597dbe4cae8d8f521e5dec03eff6fd7cf1cf510"},{"author":{"_account_id":23851,"name":"Riccardo Pittau","email":"elfosardo@gmail.com","username":"elfosardo"},"change_message_id":"df47f3ebe53b564c1ee82055e89024d1ec4afe21","unresolved":false,"context_lines":[{"line_number":218,"context_line":"        LOG.debug(\u0027Adding loader %s on partition %s of device %s\u0027,"},{"line_number":219,"context_line":"                  (v_efi_bl_path, device, efi_partition))"},{"line_number":220,"context_line":"        utils.execute(\u0027efibootmgr\u0027, \u0027-c\u0027, \u0027-d\u0027, device,"},{"line_number":221,"context_line":"                      \u0027-p\u0027, efi_partition, \u0027-w\u0027, \u0027-L\u0027, \u0027efi\u0027,"},{"line_number":222,"context_line":"                      \u0027-l\u0027, v_efi_bl_path)"},{"line_number":223,"context_line":""},{"line_number":224,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"3fa7e38b_54ce1883","line":221,"range":{"start_line":221,"start_character":48,"end_line":221,"end_character":60},"in_reply_to":"3fa7e38b_09ee4db4","updated":"2019-12-12 11:26:40.000000000","message":"I think that\u0027s fine, I mean it\u0027s also fine to have all the same, just a bit confusing :)\nideally we\u0027ll have the label based on the operating system, which can be taken from the path for example, but I\u0027m ok if we do the change in a follow-up\nall this if we actually care about differentiating the labels at all","commit_id":"d597dbe4cae8d8f521e5dec03eff6fd7cf1cf510"},{"author":{"_account_id":15519,"name":"Iury Gregory Melo Ferreira","display_name":"Iury Gregory","email":"iurygregory@gmail.com","username":"iurygregory"},"change_message_id":"6f6e9cd8369a756f1fcc356ad01753d9e06a233c","unresolved":false,"context_lines":[{"line_number":218,"context_line":"        LOG.debug(\u0027Adding loader %s on partition %s of device %s\u0027,"},{"line_number":219,"context_line":"                  (v_efi_bl_path, device, efi_partition))"},{"line_number":220,"context_line":"        utils.execute(\u0027efibootmgr\u0027, \u0027-c\u0027, \u0027-d\u0027, device,"},{"line_number":221,"context_line":"                      \u0027-p\u0027, efi_partition, \u0027-w\u0027, \u0027-L\u0027, \u0027efi\u0027,"},{"line_number":222,"context_line":"                      \u0027-l\u0027, v_efi_bl_path)"},{"line_number":223,"context_line":""},{"line_number":224,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"3fa7e38b_b438acec","line":221,"range":{"start_line":221,"start_character":48,"end_line":221,"end_character":60},"in_reply_to":"3fa7e38b_54ce1883","updated":"2020-01-07 10:57:05.000000000","message":"Ack","commit_id":"d597dbe4cae8d8f521e5dec03eff6fd7cf1cf510"},{"author":{"_account_id":15519,"name":"Iury Gregory Melo Ferreira","display_name":"Iury Gregory","email":"iurygregory@gmail.com","username":"iurygregory"},"change_message_id":"1966d7c030bfcdaaddcf9b58d516a486cf33fcac","unresolved":false,"context_lines":[{"line_number":218,"context_line":"        LOG.debug(\u0027Adding loader %s on partition %s of device %s\u0027,"},{"line_number":219,"context_line":"                  (v_efi_bl_path, device, efi_partition))"},{"line_number":220,"context_line":"        utils.execute(\u0027efibootmgr\u0027, \u0027-c\u0027, \u0027-d\u0027, device,"},{"line_number":221,"context_line":"                      \u0027-p\u0027, efi_partition, \u0027-w\u0027, \u0027-L\u0027, \u0027efi\u0027,"},{"line_number":222,"context_line":"                      \u0027-l\u0027, v_efi_bl_path)"},{"line_number":223,"context_line":""},{"line_number":224,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"3fa7e38b_09ee4db4","line":221,"range":{"start_line":221,"start_character":48,"end_line":221,"end_character":60},"in_reply_to":"3fa7e38b_e6067ecd","updated":"2019-12-12 10:33:59.000000000","message":"maybe add a number to it? \"ipa1\" \"ipa2\" so we can identify if it was updated by the ironic-python-agent?","commit_id":"d597dbe4cae8d8f521e5dec03eff6fd7cf1cf510"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"24a2c780b3631674f2c66da43af4a303cefc3a78","unresolved":false,"context_lines":[{"line_number":261,"context_line":""},{"line_number":262,"context_line":"        valid_efi_bootloaders \u003d _get_efi_bootloaders(efi_partition_mount_point)"},{"line_number":263,"context_line":"        if valid_efi_bootloaders:"},{"line_number":264,"context_line":"            _run_efibootmgr(valid_efi_bootloaders, device, efi_partition)"},{"line_number":265,"context_line":"            return True"},{"line_number":266,"context_line":"        else:"},{"line_number":267,"context_line":"            return False"}],"source_content_type":"text/x-python","patch_set":6,"id":"3fa7e38b_782474c3","line":264,"updated":"2019-12-11 18:37:32.000000000","message":"So just a thought: So the fact that we\u0027re running the boot manager here makes me think the overall method\u0027s name is misleading.","commit_id":"d597dbe4cae8d8f521e5dec03eff6fd7cf1cf510"},{"author":{"_account_id":15519,"name":"Iury Gregory Melo Ferreira","display_name":"Iury Gregory","email":"iurygregory@gmail.com","username":"iurygregory"},"change_message_id":"54ce3259d7639b44278f7163c2eeab805dc485db","unresolved":false,"context_lines":[{"line_number":261,"context_line":""},{"line_number":262,"context_line":"        valid_efi_bootloaders \u003d _get_efi_bootloaders(efi_partition_mount_point)"},{"line_number":263,"context_line":"        if valid_efi_bootloaders:"},{"line_number":264,"context_line":"            _run_efibootmgr(valid_efi_bootloaders, device, efi_partition)"},{"line_number":265,"context_line":"            return True"},{"line_number":266,"context_line":"        else:"},{"line_number":267,"context_line":"            return False"}],"source_content_type":"text/x-python","patch_set":6,"id":"3fa7e38b_2ed47646","line":264,"in_reply_to":"3fa7e38b_782474c3","updated":"2019-12-11 19:45:10.000000000","message":"Yeah, the method not only verifies the uefi, but also runs the efibootmgr so maybe this is more like validate uefi?","commit_id":"d597dbe4cae8d8f521e5dec03eff6fd7cf1cf510"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"24a2c780b3631674f2c66da43af4a303cefc3a78","unresolved":false,"context_lines":[{"line_number":492,"context_line":"        device \u003d hardware.dispatch_to_managers(\u0027get_os_install_device\u0027)"},{"line_number":493,"context_line":"        iscsi.clean_up(device)"},{"line_number":494,"context_line":"        boot \u003d hardware.dispatch_to_managers(\u0027get_boot_info\u0027)"},{"line_number":495,"context_line":"        if boot.current_boot_mode \u003d\u003d \u0027uefi\u0027:"},{"line_number":496,"context_line":"            verifed \u003d _verify_uefi(device,"},{"line_number":497,"context_line":"                                   root_uuid\u003droot_uuid,"},{"line_number":498,"context_line":"                                   efi_system_part_uuid\u003defi_system_part_uuid)"}],"source_content_type":"text/x-python","patch_set":6,"id":"3fa7e38b_78fd5407","line":495,"updated":"2019-12-11 18:37:32.000000000","message":"This seems reasonable to me.","commit_id":"d597dbe4cae8d8f521e5dec03eff6fd7cf1cf510"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"f1a8b403e46da53a32e8e622d861b8896dbec355","unresolved":false,"context_lines":[{"line_number":480,"context_line":"class ImageExtension(base.BaseAgentExtension):"},{"line_number":481,"context_line":""},{"line_number":482,"context_line":"    @base.sync_command(\u0027install_bootloader\u0027)"},{"line_number":483,"context_line":"    def install_bootloader(self, root_uuid, efi_system_part_uuid\u003dNone,"},{"line_number":484,"context_line":"                           prep_boot_part_uuid\u003dNone):"},{"line_number":485,"context_line":"        \"\"\"Install the GRUB2 bootloader on the image."},{"line_number":486,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"3fa7e38b_9cc2e734","line":483,"updated":"2019-12-23 21:18:03.000000000","message":"So there are three cases when this... _should_ get called, at least that I know of:\n\n  -iscsi partition image\n  - direct partition image\n  - direct wholedisk image:\n    called at: \nhttps://github.com/openstack/ironic/blob/master/ironic/drivers/modules/agent_base_vendor.py#L856\n    which is called at:\nhttps://github.com/openstack/ironic/blob/master/ironic/drivers/modules/agent_base_vendor.py#L762\n    which is called by:\nhttps://github.com/openstack/ironic/blob/master/ironic/drivers/modules/agent.py#L361\n\nWhich... is a bug in my mind. But we NEED to also call it on iscsi wholedisk images, but we don\u0027t at present.\n\nWe call https://github.com/openstack/ironic/blob/master/ironic/drivers/modules/iscsi_deploy.py#L359 which gets called with a root_uuid, efi_sys_uuid of both \"None\" So it is sort of critical that we gracefully handle None values being passed in. I think what would work is https://review.opendev.org/700447 feel free to squash in and remove the un-needed changes. Also, I\u0027m 95% sure we don\u0027t need to mount /sys as we\u0027re not doing an execution in a chroot, nor do we even need to touch the root filesystem in this case.","commit_id":"8048a17e94a6d1e2bf9405e8be8838dc961108b2"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"3f278c240efea7b107b3706de657ee8ddbc1cc7d","unresolved":false,"context_lines":[{"line_number":253,"context_line":"            # mount the partition read-only"},{"line_number":254,"context_line":"            # We shouldn\u0027t need to mount the base OS or even know about it"},{"line_number":255,"context_line":"            # to update nvram firmware."},{"line_number":256,"context_line":"            # utils.execute(\u0027mount\u0027, \u0027-r\u0027, root_partition, local_path)"},{"line_number":257,"context_line":"            # utils.execute(\u0027mount\u0027, \u0027-t\u0027, \u0027sysfs\u0027, \u0027none\u0027, local_path +"},{"line_number":258,"context_line":"            #               \u0027/sys\u0027)"},{"line_number":259,"context_line":"            efi_partition_mount_point \u003d os.path.join(local_path, \"boot/efi\")"},{"line_number":260,"context_line":"            if not os.path.exists(efi_partition_mount_point):"},{"line_number":261,"context_line":"                os.makedirs(efi_partition_mount_point)"}],"source_content_type":"text/x-python","patch_set":8,"id":"3fa7e38b_a6768dd4","line":258,"range":{"start_line":256,"start_character":0,"end_line":258,"end_character":35},"updated":"2020-01-03 00:42:24.000000000","message":"Comment below also applies.","commit_id":"3602daad1d43f673f10eadc72546210e5ba27482"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"3f278c240efea7b107b3706de657ee8ddbc1cc7d","unresolved":false,"context_lines":[{"line_number":301,"context_line":"            LOG.error(error_msg)"},{"line_number":302,"context_line":"            raise errors.CommandExecutionError(error_msg)"},{"line_number":303,"context_line":""},{"line_number":304,"context_line":"        # try:"},{"line_number":305,"context_line":"        #    utils.execute(\u0027umount\u0027, local_path + \u0027/sys\u0027, attempts\u003d3,"},{"line_number":306,"context_line":"        #                  delay_on_retry\u003dTrue)"},{"line_number":307,"context_line":"        # except processutils.ProcessExecutionError as e:"},{"line_number":308,"context_line":"        #    LOG.warning(umount_warn_msg,"},{"line_number":309,"context_line":"        #                {\u0027path\u0027: local_path + \u0027/sys\u0027, \u0027error\u0027: e})"},{"line_number":310,"context_line":"        else:"},{"line_number":311,"context_line":"            # If umounting the binds succeed then we can try to delete it"},{"line_number":312,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":8,"id":"3fa7e38b_867911c7","line":309,"range":{"start_line":304,"start_character":0,"end_line":309,"end_character":67},"updated":"2020-01-03 00:42:24.000000000","message":"Since bfornie has been testing, I suspect we won\u0027t need the commented out lines from the squashed patch, but we can revise once he tests.","commit_id":"3602daad1d43f673f10eadc72546210e5ba27482"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"3f278c240efea7b107b3706de657ee8ddbc1cc7d","unresolved":false,"context_lines":[{"line_number":310,"context_line":"        else:"},{"line_number":311,"context_line":"            # If umounting the binds succeed then we can try to delete it"},{"line_number":312,"context_line":"            try:"},{"line_number":313,"context_line":"                # utils.execute(\u0027umount\u0027, local_path, attempts\u003d3,"},{"line_number":314,"context_line":"                #              delay_on_retry\u003dTrue)"},{"line_number":315,"context_line":"                utils.execute(\u0027sync\u0027)"},{"line_number":316,"context_line":"            except processutils.ProcessExecutionError as e:"},{"line_number":317,"context_line":"                LOG.warning(umount_warn_msg, {\u0027path\u0027: local_path, \u0027error\u0027: e})"}],"source_content_type":"text/x-python","patch_set":8,"id":"3fa7e38b_4649d98b","line":314,"range":{"start_line":313,"start_character":0,"end_line":314,"end_character":51},"updated":"2020-01-03 00:42:24.000000000","message":"same comment as above.","commit_id":"3602daad1d43f673f10eadc72546210e5ba27482"},{"author":{"_account_id":21909,"name":"Bob Fournier","email":"bfournie@redhat.com","username":"bfournie"},"change_message_id":"6a435d5171af2b600105e6bb673999ea7b76e5f1","unresolved":false,"context_lines":[{"line_number":215,"context_line":"    :param device: the device to be used"},{"line_number":216,"context_line":"    :param efi_partition: the efi partition on the device"},{"line_number":217,"context_line":"    \"\"\""},{"line_number":218,"context_line":""},{"line_number":219,"context_line":"    for v_efi_bl_path in valid_efi_bootloaders:"},{"line_number":220,"context_line":"        # Update the nvram using efibootmgr"},{"line_number":221,"context_line":"        # https://linux.die.net/man/8/efibootmgr"}],"source_content_type":"text/x-python","patch_set":9,"id":"3fa7e38b_5f598158","line":218,"updated":"2020-01-07 00:29:44.000000000","message":"It would be useful to debug log the current boot order here using e.g.:\nstdout, stderr \u003d utils.execute(\u0027efibootmgr\u0027, \u0027-v\u0027)\n\nEspecially since it possible that the entry with the same label may need to be removed before adding the new one, similar to what grub2-install does[0].\n\n[0] https://github.com/coreos/grub/blob/2.02-coreos/grub-core/osdep/unix/platform.c#L82","commit_id":"b79c93fdd2ef836553930a2d491681473ceadcbb"},{"author":{"_account_id":15519,"name":"Iury Gregory Melo Ferreira","display_name":"Iury Gregory","email":"iurygregory@gmail.com","username":"iurygregory"},"change_message_id":"6f6e9cd8369a756f1fcc356ad01753d9e06a233c","unresolved":false,"context_lines":[{"line_number":215,"context_line":"    :param device: the device to be used"},{"line_number":216,"context_line":"    :param efi_partition: the efi partition on the device"},{"line_number":217,"context_line":"    \"\"\""},{"line_number":218,"context_line":""},{"line_number":219,"context_line":"    for v_efi_bl_path in valid_efi_bootloaders:"},{"line_number":220,"context_line":"        # Update the nvram using efibootmgr"},{"line_number":221,"context_line":"        # https://linux.die.net/man/8/efibootmgr"}],"source_content_type":"text/x-python","patch_set":9,"id":"3fa7e38b_25d7d3c2","line":218,"in_reply_to":"3fa7e38b_5f598158","updated":"2020-01-07 10:57:05.000000000","message":"I\u0027ve added the \u0027efibootmgr -v\u0027, about the lables instead of using \"efi\" I\u0027ve changed to used ironic\u003cnumber\u003e so we won\u0027t have duplicated labels in case of multiple entries.\n\nThe link you pointed would remove platform labels, when we are adding we are not looking at the platform, Do we need to remove any entry before adding the new one?","commit_id":"b79c93fdd2ef836553930a2d491681473ceadcbb"},{"author":{"_account_id":21909,"name":"Bob Fournier","email":"bfournie@redhat.com","username":"bfournie"},"change_message_id":"6a435d5171af2b600105e6bb673999ea7b76e5f1","unresolved":false,"context_lines":[{"line_number":221,"context_line":"        # https://linux.die.net/man/8/efibootmgr"},{"line_number":222,"context_line":"        # NOTE(iurygregory): think in a better way to handle labels."},{"line_number":223,"context_line":"        LOG.debug(\u0027Adding loader %s on partition %s of device %s\u0027,"},{"line_number":224,"context_line":"                  (v_efi_bl_path, device, efi_partition))"},{"line_number":225,"context_line":"        utils.execute(\u0027efibootmgr\u0027, \u0027-c\u0027, \u0027-d\u0027, device,"},{"line_number":226,"context_line":"                      \u0027-p\u0027, efi_partition, \u0027-w\u0027, \u0027-L\u0027, \u0027efi\u0027,"},{"line_number":227,"context_line":"                      \u0027-l\u0027, v_efi_bl_path)"}],"source_content_type":"text/x-python","patch_set":9,"id":"3fa7e38b_3e692b25","line":224,"range":{"start_line":224,"start_character":34,"end_line":224,"end_character":55},"updated":"2020-01-07 00:29:44.000000000","message":"Ordering doesn\u0027t match the output string, maybe use something like:\nLOG.debug(\u0027Adding loader %(path)s on partition %(part)s of device %(dev)s\u0027, {\u0027path\u0027: v_efi_bl_path, \u0027part\u0027:efi_partition, \u0027dev\u0027:device})","commit_id":"b79c93fdd2ef836553930a2d491681473ceadcbb"},{"author":{"_account_id":15519,"name":"Iury Gregory Melo Ferreira","display_name":"Iury Gregory","email":"iurygregory@gmail.com","username":"iurygregory"},"change_message_id":"6f6e9cd8369a756f1fcc356ad01753d9e06a233c","unresolved":false,"context_lines":[{"line_number":221,"context_line":"        # https://linux.die.net/man/8/efibootmgr"},{"line_number":222,"context_line":"        # NOTE(iurygregory): think in a better way to handle labels."},{"line_number":223,"context_line":"        LOG.debug(\u0027Adding loader %s on partition %s of device %s\u0027,"},{"line_number":224,"context_line":"                  (v_efi_bl_path, device, efi_partition))"},{"line_number":225,"context_line":"        utils.execute(\u0027efibootmgr\u0027, \u0027-c\u0027, \u0027-d\u0027, device,"},{"line_number":226,"context_line":"                      \u0027-p\u0027, efi_partition, \u0027-w\u0027, \u0027-L\u0027, \u0027efi\u0027,"},{"line_number":227,"context_line":"                      \u0027-l\u0027, v_efi_bl_path)"}],"source_content_type":"text/x-python","patch_set":9,"id":"3fa7e38b_45d44fbb","line":224,"range":{"start_line":224,"start_character":34,"end_line":224,"end_character":55},"in_reply_to":"3fa7e38b_3e692b25","updated":"2020-01-07 10:57:05.000000000","message":"Done","commit_id":"b79c93fdd2ef836553930a2d491681473ceadcbb"},{"author":{"_account_id":21909,"name":"Bob Fournier","email":"bfournie@redhat.com","username":"bfournie"},"change_message_id":"6a435d5171af2b600105e6bb673999ea7b76e5f1","unresolved":false,"context_lines":[{"line_number":248,"context_line":"        # Trust the contents on the disk in the event of a whole disk image."},{"line_number":249,"context_line":"        efi_partition \u003d utils.get_efi_part_on_device(device)"},{"line_number":250,"context_line":"        if not efi_partition:"},{"line_number":251,"context_line":"            efi_partition \u003d _get_partition(device, uuid\u003defi_system_part_uuid)"},{"line_number":252,"context_line":""},{"line_number":253,"context_line":"        if efi_partition:"},{"line_number":254,"context_line":"            efi_partition_mount_point \u003d os.path.join(local_path, \"boot/efi\")"}],"source_content_type":"text/x-python","patch_set":9,"id":"3fa7e38b_de7a57ee","line":251,"range":{"start_line":251,"start_character":28,"end_line":251,"end_character":42},"updated":"2020-01-07 00:29:44.000000000","message":"This should return the partition number, the same way that get_efi_part_on_device does.  Efibootmgr takes the device and partition number separately:\n\nusage: efibootmgr [options]\n\t-d | --disk disk       (defaults to /dev/sda) containing loader\n\t-p | --part part        (defaults to 1) containing loader","commit_id":"b79c93fdd2ef836553930a2d491681473ceadcbb"},{"author":{"_account_id":15519,"name":"Iury Gregory Melo Ferreira","display_name":"Iury Gregory","email":"iurygregory@gmail.com","username":"iurygregory"},"change_message_id":"6f6e9cd8369a756f1fcc356ad01753d9e06a233c","unresolved":false,"context_lines":[{"line_number":248,"context_line":"        # Trust the contents on the disk in the event of a whole disk image."},{"line_number":249,"context_line":"        efi_partition \u003d utils.get_efi_part_on_device(device)"},{"line_number":250,"context_line":"        if not efi_partition:"},{"line_number":251,"context_line":"            efi_partition \u003d _get_partition(device, uuid\u003defi_system_part_uuid)"},{"line_number":252,"context_line":""},{"line_number":253,"context_line":"        if efi_partition:"},{"line_number":254,"context_line":"            efi_partition_mount_point \u003d os.path.join(local_path, \"boot/efi\")"}],"source_content_type":"text/x-python","patch_set":9,"id":"3fa7e38b_0012c555","line":251,"range":{"start_line":251,"start_character":28,"end_line":251,"end_character":42},"in_reply_to":"3fa7e38b_de7a57ee","updated":"2020-01-07 10:57:05.000000000","message":"I will update this to get extract the partition, since the function is already used in other places and it returns /dev/sda1 (for example) \u003d)","commit_id":"b79c93fdd2ef836553930a2d491681473ceadcbb"},{"author":{"_account_id":21909,"name":"Bob Fournier","email":"bfournie@redhat.com","username":"bfournie"},"change_message_id":"3c58584b293e889b1ab9ac522d357e10f784a910","unresolved":false,"context_lines":[{"line_number":218,"context_line":""},{"line_number":219,"context_line":"    # Before updating let\u0027s get information about the bootorder"},{"line_number":220,"context_line":"    LOG.debug(\"Getting information about boot order\")"},{"line_number":221,"context_line":"    utils.execute(\u0027efibootmgr\u0027, \u0027-v\u0027)"},{"line_number":222,"context_line":"    label_id \u003d 1"},{"line_number":223,"context_line":"    for v_efi_bl_path in valid_efi_bootloaders:"},{"line_number":224,"context_line":"        # Update the nvram using efibootmgr"}],"source_content_type":"text/x-python","patch_set":10,"id":"3fa7e38b_df69e7bf","line":221,"range":{"start_line":221,"start_character":30,"end_line":221,"end_character":37},"updated":"2020-01-07 15:30:44.000000000","message":"Thanks for adding this. I did find that the \u0027-v\u0027 is too verbose so recommend removing \u0027-v\u0027.","commit_id":"709948603279a8b131248b846dc636d587cced5e"},{"author":{"_account_id":21909,"name":"Bob Fournier","email":"bfournie@redhat.com","username":"bfournie"},"change_message_id":"1225036ae9956563a4639c8f5662895400289903","unresolved":false,"context_lines":[{"line_number":218,"context_line":""},{"line_number":219,"context_line":"    # Before updating let\u0027s get information about the bootorder"},{"line_number":220,"context_line":"    LOG.debug(\"Getting information about boot order\")"},{"line_number":221,"context_line":"    utils.execute(\u0027efibootmgr\u0027, \u0027-v\u0027)"},{"line_number":222,"context_line":"    label_id \u003d 1"},{"line_number":223,"context_line":"    for v_efi_bl_path in valid_efi_bootloaders:"},{"line_number":224,"context_line":"        # Update the nvram using efibootmgr"}],"source_content_type":"text/x-python","patch_set":10,"id":"3fa7e38b_22e7c248","line":221,"range":{"start_line":221,"start_character":30,"end_line":221,"end_character":37},"in_reply_to":"3fa7e38b_9f012f3c","updated":"2020-01-07 16:32:46.000000000","message":"We only care about the label when removing the entry, not the files, so the -v just provides unnecessary noise.","commit_id":"709948603279a8b131248b846dc636d587cced5e"},{"author":{"_account_id":15519,"name":"Iury Gregory Melo Ferreira","display_name":"Iury Gregory","email":"iurygregory@gmail.com","username":"iurygregory"},"change_message_id":"14555475ec297e41396956f499fcd1f258e809e4","unresolved":false,"context_lines":[{"line_number":218,"context_line":""},{"line_number":219,"context_line":"    # Before updating let\u0027s get information about the bootorder"},{"line_number":220,"context_line":"    LOG.debug(\"Getting information about boot order\")"},{"line_number":221,"context_line":"    utils.execute(\u0027efibootmgr\u0027, \u0027-v\u0027)"},{"line_number":222,"context_line":"    label_id \u003d 1"},{"line_number":223,"context_line":"    for v_efi_bl_path in valid_efi_bootloaders:"},{"line_number":224,"context_line":"        # Update the nvram using efibootmgr"}],"source_content_type":"text/x-python","patch_set":10,"id":"3fa7e38b_9f012f3c","line":221,"range":{"start_line":221,"start_character":30,"end_line":221,"end_character":37},"in_reply_to":"3fa7e38b_df69e7bf","updated":"2020-01-07 16:04:08.000000000","message":"The output with -v could help if we want to remove entries that are using any of the efi valid files.\n\n[root@centos7 ~]# efibootmgr -v\nBootCurrent: 0001\nTimeout: 0 seconds\nBootOrder: 0001,0002,0000,0003\nBoot0000* UiApp\tFvVol(7cb8bdc9-f8eb-4f34-aaea-3ee4af6516a1)/FvFile(462caa21-7614-4503-836e-8ab6f4662331)\nBoot0001* CentOS\tHD(1,GPT,1ab4d86b-9f82-4699-bb1e-e2d3ab5d6e14,0x800,0x64000)/File(\\EFI\\centos\\shimx64.efi)\nBoot0002* UEFI Misc Device\t/Pci(0x2,0x3)/Pci(0x0,0x0)N.....YM....R,Y.\nBoot0003* EFI Internal Shell\tFvVol(7cb8bdc9-f8eb-4f34-aaea-3ee4af6516a1)/FvFile(7c04a583-9e3e-4f1c-ad65-e05268d0b4d1)","commit_id":"709948603279a8b131248b846dc636d587cced5e"},{"author":{"_account_id":21909,"name":"Bob Fournier","email":"bfournie@redhat.com","username":"bfournie"},"change_message_id":"3c58584b293e889b1ab9ac522d357e10f784a910","unresolved":false,"context_lines":[{"line_number":228,"context_line":"        LOG.debug(\"Adding loader %(path)s on partition %(part)s of device \""},{"line_number":229,"context_line":"                  \" %(dev)s\", {\u0027path\u0027: v_efi_bl_path, \u0027part\u0027: efi_partition,"},{"line_number":230,"context_line":"                               \u0027dev\u0027: device})"},{"line_number":231,"context_line":"        utils.execute(\u0027efibootmgr\u0027, \u0027-c\u0027, \u0027-d\u0027, device,"},{"line_number":232,"context_line":"                      \u0027-p\u0027, efi_partition, \u0027-w\u0027, \u0027-L\u0027, label,"},{"line_number":233,"context_line":"                      \u0027-l\u0027, v_efi_bl_path)"},{"line_number":234,"context_line":"        label_id +\u003d 1"}],"source_content_type":"text/x-python","patch_set":10,"id":"3fa7e38b_7fad53e5","line":231,"updated":"2020-01-07 15:30:44.000000000","message":"I think we need to remove existing entries with label \"ironic*\" (similar to what grub2-install does).  As it is now, a new entry in the table will be installed on each deployment.","commit_id":"709948603279a8b131248b846dc636d587cced5e"},{"author":{"_account_id":21909,"name":"Bob Fournier","email":"bfournie@redhat.com","username":"bfournie"},"change_message_id":"3c58584b293e889b1ab9ac522d357e10f784a910","unresolved":false,"context_lines":[{"line_number":506,"context_line":"        device \u003d hardware.dispatch_to_managers(\u0027get_os_install_device\u0027)"},{"line_number":507,"context_line":"        iscsi.clean_up(device)"},{"line_number":508,"context_line":"        boot \u003d hardware.dispatch_to_managers(\u0027get_boot_info\u0027)"},{"line_number":509,"context_line":"        if boot.current_boot_mode \u003d\u003d \u0027uefi\u0027:"},{"line_number":510,"context_line":"            managed \u003d _manage_uefi(device,"},{"line_number":511,"context_line":"                                   # root_uuid\u003droot_uuid,"},{"line_number":512,"context_line":"                                   efi_system_part_uuid\u003defi_system_part_uuid)"}],"source_content_type":"text/x-python","patch_set":10,"id":"3fa7e38b_5f4e5739","line":509,"updated":"2020-01-07 15:30:44.000000000","message":"Recommend a sanity check here that \"efibootmgr\" is available before calling _manage_uefi.  Since the efibootmgr is installed by ipa-builder it would be useful to know it has been installed before attempting to use it.   For example, can execute \"efibootmgr --version\" and handle any exceptions.","commit_id":"709948603279a8b131248b846dc636d587cced5e"},{"author":{"_account_id":21909,"name":"Bob Fournier","email":"bfournie@redhat.com","username":"bfournie"},"change_message_id":"1225036ae9956563a4639c8f5662895400289903","unresolved":false,"context_lines":[{"line_number":506,"context_line":"        device \u003d hardware.dispatch_to_managers(\u0027get_os_install_device\u0027)"},{"line_number":507,"context_line":"        iscsi.clean_up(device)"},{"line_number":508,"context_line":"        boot \u003d hardware.dispatch_to_managers(\u0027get_boot_info\u0027)"},{"line_number":509,"context_line":"        if boot.current_boot_mode \u003d\u003d \u0027uefi\u0027:"},{"line_number":510,"context_line":"            managed \u003d _manage_uefi(device,"},{"line_number":511,"context_line":"                                   # root_uuid\u003droot_uuid,"},{"line_number":512,"context_line":"                                   efi_system_part_uuid\u003defi_system_part_uuid)"}],"source_content_type":"text/x-python","patch_set":10,"id":"3fa7e38b_82db1678","line":509,"in_reply_to":"3fa7e38b_3ff11b6b","updated":"2020-01-07 16:32:46.000000000","message":"The existence of the efibootmgr command doesn\u0027t rely on any mounts being added as it should be there if ipa-builder has installed the efibootmgr package.","commit_id":"709948603279a8b131248b846dc636d587cced5e"},{"author":{"_account_id":15519,"name":"Iury Gregory Melo Ferreira","display_name":"Iury Gregory","email":"iurygregory@gmail.com","username":"iurygregory"},"change_message_id":"14555475ec297e41396956f499fcd1f258e809e4","unresolved":false,"context_lines":[{"line_number":506,"context_line":"        device \u003d hardware.dispatch_to_managers(\u0027get_os_install_device\u0027)"},{"line_number":507,"context_line":"        iscsi.clean_up(device)"},{"line_number":508,"context_line":"        boot \u003d hardware.dispatch_to_managers(\u0027get_boot_info\u0027)"},{"line_number":509,"context_line":"        if boot.current_boot_mode \u003d\u003d \u0027uefi\u0027:"},{"line_number":510,"context_line":"            managed \u003d _manage_uefi(device,"},{"line_number":511,"context_line":"                                   # root_uuid\u003droot_uuid,"},{"line_number":512,"context_line":"                                   efi_system_part_uuid\u003defi_system_part_uuid)"}],"source_content_type":"text/x-python","patch_set":10,"id":"3fa7e38b_3ff11b6b","line":509,"in_reply_to":"3fa7e38b_5f4e5739","updated":"2020-01-07 16:04:08.000000000","message":"I can only check for the efibootmgr after we have the done the mount, so it would be inside _manage_uefi before calling _run_efibootmgr function, it would return False/None so we can run _install_grub2 \u003d)","commit_id":"709948603279a8b131248b846dc636d587cced5e"},{"author":{"_account_id":21909,"name":"Bob Fournier","email":"bfournie@redhat.com","username":"bfournie"},"change_message_id":"c04a01410e0bbc818a44e9b7adcb53a579b46e99","unresolved":false,"context_lines":[{"line_number":219,"context_line":"    # Before updating let\u0027s get information about the bootorder"},{"line_number":220,"context_line":"    LOG.debug(\"Getting information about boot order\")"},{"line_number":221,"context_line":"    utils.execute(\u0027efibootmgr\u0027)"},{"line_number":222,"context_line":"    duplicated_label \u003d re.compile(r\u0027^.*:\\s\\*\\*.*\\*\\*\\s:\\s.*\u0027"},{"line_number":223,"context_line":"                                  r\u0027Boot([0-9a-f-A-F]+)\\s.*$\u0027)"},{"line_number":224,"context_line":"    label_id \u003d 1"},{"line_number":225,"context_line":"    for v_efi_bl_path in valid_efi_bootloaders:"},{"line_number":226,"context_line":"        # Update the nvram using efibootmgr"}],"source_content_type":"text/x-python","patch_set":11,"id":"3fa7e38b_6c07c471","line":223,"range":{"start_line":222,"start_character":4,"end_line":223,"end_character":62},"updated":"2020-01-10 01:09:56.000000000","message":"Please add comment explaining this and the warning message that this is handling.","commit_id":"126e1a93138b29283b5601a8966819f5cc90bc7c"},{"author":{"_account_id":15519,"name":"Iury Gregory Melo Ferreira","display_name":"Iury Gregory","email":"iurygregory@gmail.com","username":"iurygregory"},"change_message_id":"fadbadfc9dbf25cda21d6a747312b6ac70b760da","unresolved":false,"context_lines":[{"line_number":219,"context_line":"    # Before updating let\u0027s get information about the bootorder"},{"line_number":220,"context_line":"    LOG.debug(\"Getting information about boot order\")"},{"line_number":221,"context_line":"    utils.execute(\u0027efibootmgr\u0027)"},{"line_number":222,"context_line":"    duplicated_label \u003d re.compile(r\u0027^.*:\\s\\*\\*.*\\*\\*\\s:\\s.*\u0027"},{"line_number":223,"context_line":"                                  r\u0027Boot([0-9a-f-A-F]+)\\s.*$\u0027)"},{"line_number":224,"context_line":"    label_id \u003d 1"},{"line_number":225,"context_line":"    for v_efi_bl_path in valid_efi_bootloaders:"},{"line_number":226,"context_line":"        # Update the nvram using efibootmgr"}],"source_content_type":"text/x-python","patch_set":11,"id":"3fa7e38b_c166babc","line":223,"range":{"start_line":222,"start_character":4,"end_line":223,"end_character":62},"in_reply_to":"3fa7e38b_6c07c471","updated":"2020-01-10 12:17:50.000000000","message":"Done","commit_id":"126e1a93138b29283b5601a8966819f5cc90bc7c"},{"author":{"_account_id":21909,"name":"Bob Fournier","email":"bfournie@redhat.com","username":"bfournie"},"change_message_id":"c04a01410e0bbc818a44e9b7adcb53a579b46e99","unresolved":false,"context_lines":[{"line_number":225,"context_line":"    for v_efi_bl_path in valid_efi_bootloaders:"},{"line_number":226,"context_line":"        # Update the nvram using efibootmgr"},{"line_number":227,"context_line":"        # https://linux.die.net/man/8/efibootmgr"},{"line_number":228,"context_line":"        # NOTE(iurygregory): think in a better way to handle labels."},{"line_number":229,"context_line":"        label \u003d \u0027ironic\u0027 + str(label_id)"},{"line_number":230,"context_line":"        LOG.debug(\"Adding loader %(path)s on partition %(part)s of device \""},{"line_number":231,"context_line":"                  \" %(dev)s\", {\u0027path\u0027: v_efi_bl_path, \u0027part\u0027: efi_partition,"}],"source_content_type":"text/x-python","patch_set":11,"id":"3fa7e38b_8c0a4075","line":228,"range":{"start_line":228,"start_character":8,"end_line":228,"end_character":68},"updated":"2020-01-10 01:09:56.000000000","message":"Probably don\u0027t need this anymore.","commit_id":"126e1a93138b29283b5601a8966819f5cc90bc7c"},{"author":{"_account_id":15519,"name":"Iury Gregory Melo Ferreira","display_name":"Iury Gregory","email":"iurygregory@gmail.com","username":"iurygregory"},"change_message_id":"fadbadfc9dbf25cda21d6a747312b6ac70b760da","unresolved":false,"context_lines":[{"line_number":225,"context_line":"    for v_efi_bl_path in valid_efi_bootloaders:"},{"line_number":226,"context_line":"        # Update the nvram using efibootmgr"},{"line_number":227,"context_line":"        # https://linux.die.net/man/8/efibootmgr"},{"line_number":228,"context_line":"        # NOTE(iurygregory): think in a better way to handle labels."},{"line_number":229,"context_line":"        label \u003d \u0027ironic\u0027 + str(label_id)"},{"line_number":230,"context_line":"        LOG.debug(\"Adding loader %(path)s on partition %(part)s of device \""},{"line_number":231,"context_line":"                  \" %(dev)s\", {\u0027path\u0027: v_efi_bl_path, \u0027part\u0027: efi_partition,"}],"source_content_type":"text/x-python","patch_set":11,"id":"3fa7e38b_810202b9","line":228,"range":{"start_line":228,"start_character":8,"end_line":228,"end_character":68},"in_reply_to":"3fa7e38b_8c0a4075","updated":"2020-01-10 12:17:50.000000000","message":"Done","commit_id":"126e1a93138b29283b5601a8966819f5cc90bc7c"},{"author":{"_account_id":21909,"name":"Bob Fournier","email":"bfournie@redhat.com","username":"bfournie"},"change_message_id":"c04a01410e0bbc818a44e9b7adcb53a579b46e99","unresolved":false,"context_lines":[{"line_number":237,"context_line":"            match \u003d duplicated_label.match(line)"},{"line_number":238,"context_line":"            if match:"},{"line_number":239,"context_line":"                boot_num \u003d match.group(1)"},{"line_number":240,"context_line":"                LOG.debug(\"Found bootnum %s matching label\", boot_num)"},{"line_number":241,"context_line":"                utils.execute(\u0027efibootmgr\u0027, \u0027-b\u0027, boot_num, \u0027-B\u0027)"},{"line_number":242,"context_line":"        label_id +\u003d 1"},{"line_number":243,"context_line":""},{"line_number":244,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"3fa7e38b_8c7fa0dc","line":241,"range":{"start_line":240,"start_character":16,"end_line":241,"end_character":65},"updated":"2020-01-10 01:09:56.000000000","message":"Could use a unit test for this.","commit_id":"126e1a93138b29283b5601a8966819f5cc90bc7c"},{"author":{"_account_id":15519,"name":"Iury Gregory Melo Ferreira","display_name":"Iury Gregory","email":"iurygregory@gmail.com","username":"iurygregory"},"change_message_id":"fadbadfc9dbf25cda21d6a747312b6ac70b760da","unresolved":false,"context_lines":[{"line_number":237,"context_line":"            match \u003d duplicated_label.match(line)"},{"line_number":238,"context_line":"            if match:"},{"line_number":239,"context_line":"                boot_num \u003d match.group(1)"},{"line_number":240,"context_line":"                LOG.debug(\"Found bootnum %s matching label\", boot_num)"},{"line_number":241,"context_line":"                utils.execute(\u0027efibootmgr\u0027, \u0027-b\u0027, boot_num, \u0027-B\u0027)"},{"line_number":242,"context_line":"        label_id +\u003d 1"},{"line_number":243,"context_line":""},{"line_number":244,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"3fa7e38b_610b46d3","line":241,"range":{"start_line":240,"start_character":16,"end_line":241,"end_character":65},"in_reply_to":"3fa7e38b_8c7fa0dc","updated":"2020-01-10 12:17:50.000000000","message":"Sure!","commit_id":"126e1a93138b29283b5601a8966819f5cc90bc7c"},{"author":{"_account_id":21909,"name":"Bob Fournier","email":"bfournie@redhat.com","username":"bfournie"},"change_message_id":"e7e44679b6b5c500bc379ae17166bbf13f489090","unresolved":false,"context_lines":[{"line_number":526,"context_line":"                if _manage_uefi(device,"},{"line_number":527,"context_line":"                                efi_system_part_uuid\u003defi_system_part_uuid):"},{"line_number":528,"context_line":"                    return"},{"line_number":529,"context_line":""},{"line_number":530,"context_line":"        _install_grub2(device,"},{"line_number":531,"context_line":"                       root_uuid\u003droot_uuid,"},{"line_number":532,"context_line":"                       efi_system_part_uuid\u003defi_system_part_uuid,"}],"source_content_type":"text/x-python","patch_set":11,"id":"3fa7e38b_c1a55aa1","line":529,"updated":"2020-01-10 12:31:50.000000000","message":"Also could use a comment and log msg here indicating that grub2-install is being used to set up boot files.","commit_id":"126e1a93138b29283b5601a8966819f5cc90bc7c"},{"author":{"_account_id":15519,"name":"Iury Gregory Melo Ferreira","display_name":"Iury Gregory","email":"iurygregory@gmail.com","username":"iurygregory"},"change_message_id":"59a91d8b0b11c1498a38334904d3e066618c3d4a","unresolved":false,"context_lines":[{"line_number":526,"context_line":"                if _manage_uefi(device,"},{"line_number":527,"context_line":"                                efi_system_part_uuid\u003defi_system_part_uuid):"},{"line_number":528,"context_line":"                    return"},{"line_number":529,"context_line":""},{"line_number":530,"context_line":"        _install_grub2(device,"},{"line_number":531,"context_line":"                       root_uuid\u003droot_uuid,"},{"line_number":532,"context_line":"                       efi_system_part_uuid\u003defi_system_part_uuid,"}],"source_content_type":"text/x-python","patch_set":11,"id":"3fa7e38b_e7bd06d1","line":529,"in_reply_to":"3fa7e38b_c1a55aa1","updated":"2020-01-10 14:41:34.000000000","message":"Done","commit_id":"126e1a93138b29283b5601a8966819f5cc90bc7c"},{"author":{"_account_id":23851,"name":"Riccardo Pittau","email":"elfosardo@gmail.com","username":"elfosardo"},"change_message_id":"8409aa03b5d3d873fe12b172a7b37f3310fe43bd","unresolved":false,"context_lines":[{"line_number":275,"context_line":"            if not os.path.exists(efi_partition_mount_point):"},{"line_number":276,"context_line":"                os.makedirs(efi_partition_mount_point)"},{"line_number":277,"context_line":""},{"line_number":278,"context_line":"            # The mount need to the device with the partition, in case the"},{"line_number":279,"context_line":"            # device ends with a digit we add a p and the partition number we"},{"line_number":280,"context_line":"            # found, otherwise we just join the device and the partition number"},{"line_number":281,"context_line":"            if device[-1].isdigit():"}],"source_content_type":"text/x-python","patch_set":13,"id":"3fa7e38b_4b82d601","line":278,"range":{"start_line":278,"start_character":14,"end_line":278,"end_character":42},"updated":"2020-01-13 10:16:08.000000000","message":"nit: something missing here ?","commit_id":"c057f6a7778a7429ac22c8a934f62578e6e24d49"},{"author":{"_account_id":15519,"name":"Iury Gregory Melo Ferreira","display_name":"Iury Gregory","email":"iurygregory@gmail.com","username":"iurygregory"},"change_message_id":"d1110bdeeafafe86a26256dad7c3e7488a3bcde3","unresolved":false,"context_lines":[{"line_number":275,"context_line":"            if not os.path.exists(efi_partition_mount_point):"},{"line_number":276,"context_line":"                os.makedirs(efi_partition_mount_point)"},{"line_number":277,"context_line":""},{"line_number":278,"context_line":"            # The mount need to the device with the partition, in case the"},{"line_number":279,"context_line":"            # device ends with a digit we add a p and the partition number we"},{"line_number":280,"context_line":"            # found, otherwise we just join the device and the partition number"},{"line_number":281,"context_line":"            if device[-1].isdigit():"}],"source_content_type":"text/x-python","patch_set":13,"id":"3fa7e38b_0b1e1ebc","line":278,"range":{"start_line":278,"start_character":14,"end_line":278,"end_character":42},"in_reply_to":"3fa7e38b_4b82d601","updated":"2020-01-13 10:22:16.000000000","message":"yeah, updating now","commit_id":"c057f6a7778a7429ac22c8a934f62578e6e24d49"}],"ironic_python_agent/tests/unit/extensions/test_image.py":[{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"24a2c780b3631674f2c66da43af4a303cefc3a78","unresolved":false,"context_lines":[{"line_number":676,"context_line":"        self.assertTrue(result)"},{"line_number":677,"context_line":"        mkdir_mock.assert_called_once_with(self.fake_dir + \u0027/boot/efi\u0027)"},{"line_number":678,"context_line":"        mock_efi_bl.assert_called_once_with(self.fake_dir + \u0027/boot/efi\u0027)"},{"line_number":679,"context_line":"        mock_execute.assert_has_calls(expected)"},{"line_number":680,"context_line":""},{"line_number":681,"context_line":"    @mock.patch.object(os, \u0027walk\u0027, autospec\u003dTrue)"},{"line_number":682,"context_line":"    @mock.patch.object(os, \u0027access\u0027, autospec\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":6,"id":"3fa7e38b_0b75ec97","line":679,"updated":"2019-12-11 18:37:32.000000000","message":"as a bonus, I\u0027d \u003c3 to see one that also considers winload.efi so we can know that the list will continue to do the needful should we need to update it.","commit_id":"d597dbe4cae8d8f521e5dec03eff6fd7cf1cf510"},{"author":{"_account_id":15519,"name":"Iury Gregory Melo Ferreira","display_name":"Iury Gregory","email":"iurygregory@gmail.com","username":"iurygregory"},"change_message_id":"54ce3259d7639b44278f7163c2eeab805dc485db","unresolved":false,"context_lines":[{"line_number":676,"context_line":"        self.assertTrue(result)"},{"line_number":677,"context_line":"        mkdir_mock.assert_called_once_with(self.fake_dir + \u0027/boot/efi\u0027)"},{"line_number":678,"context_line":"        mock_efi_bl.assert_called_once_with(self.fake_dir + \u0027/boot/efi\u0027)"},{"line_number":679,"context_line":"        mock_execute.assert_has_calls(expected)"},{"line_number":680,"context_line":""},{"line_number":681,"context_line":"    @mock.patch.object(os, \u0027walk\u0027, autospec\u003dTrue)"},{"line_number":682,"context_line":"    @mock.patch.object(os, \u0027access\u0027, autospec\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":6,"id":"3fa7e38b_aebf06fa","line":679,"in_reply_to":"3fa7e38b_0b75ec97","updated":"2019-12-11 19:45:10.000000000","message":"I will update one to cover this \u003d)","commit_id":"d597dbe4cae8d8f521e5dec03eff6fd7cf1cf510"},{"author":{"_account_id":21909,"name":"Bob Fournier","email":"bfournie@redhat.com","username":"bfournie"},"change_message_id":"9b4e9004c7cc119c8a62445f006448a1702f5a78","unresolved":false,"context_lines":[{"line_number":188,"context_line":"        mock_utils_efi_part.return_value \u003d \u00271\u0027"},{"line_number":189,"context_line":"        mock_efi_bl.return_value \u003d [\u0027\\\\EFI\\\\BOOT\\\\BOOTX64.EFI\u0027]"},{"line_number":190,"context_line":"        stdeer_msg \u003d \"\"\""},{"line_number":191,"context_line":"efibootmgr: ** Warning ** : Boot0004 has same label ironic\\n"},{"line_number":192,"context_line":"efibootmgr: ** Warning ** : Boot0005 has same label ironic\\n"},{"line_number":193,"context_line":"\"\"\""},{"line_number":194,"context_line":"        mock_execute.side_effect \u003d iter([(\u0027\u0027, \u0027\u0027), (\u0027\u0027, \u0027\u0027),"}],"source_content_type":"text/x-python","patch_set":12,"id":"3fa7e38b_47073a71","line":191,"range":{"start_line":191,"start_character":52,"end_line":191,"end_character":58},"updated":"2020-01-10 14:55:45.000000000","message":"Nit - should be \"ironic1\" (doesn\u0027t affect test)","commit_id":"a7e7ced1d0cb157825ff5272819b3b499dd3b0cb"},{"author":{"_account_id":15519,"name":"Iury Gregory Melo Ferreira","display_name":"Iury Gregory","email":"iurygregory@gmail.com","username":"iurygregory"},"change_message_id":"7b49e2c18ce866b0639330ec0ba5653949669756","unresolved":false,"context_lines":[{"line_number":188,"context_line":"        mock_utils_efi_part.return_value \u003d \u00271\u0027"},{"line_number":189,"context_line":"        mock_efi_bl.return_value \u003d [\u0027\\\\EFI\\\\BOOT\\\\BOOTX64.EFI\u0027]"},{"line_number":190,"context_line":"        stdeer_msg \u003d \"\"\""},{"line_number":191,"context_line":"efibootmgr: ** Warning ** : Boot0004 has same label ironic\\n"},{"line_number":192,"context_line":"efibootmgr: ** Warning ** : Boot0005 has same label ironic\\n"},{"line_number":193,"context_line":"\"\"\""},{"line_number":194,"context_line":"        mock_execute.side_effect \u003d iter([(\u0027\u0027, \u0027\u0027), (\u0027\u0027, \u0027\u0027),"}],"source_content_type":"text/x-python","patch_set":12,"id":"3fa7e38b_c7524a36","line":191,"range":{"start_line":191,"start_character":52,"end_line":191,"end_character":58},"in_reply_to":"3fa7e38b_47073a71","updated":"2020-01-10 15:07:20.000000000","message":"Done","commit_id":"a7e7ced1d0cb157825ff5272819b3b499dd3b0cb"}],"releasenotes/notes/avoid-grub2-using-efibootmgr-bd27c0978d1cf71b.yaml":[{"author":{"_account_id":23851,"name":"Riccardo Pittau","email":"elfosardo@gmail.com","username":"elfosardo"},"change_message_id":"185f4a951b65c3cde2082007619f37d05025ca96","unresolved":false,"context_lines":[{"line_number":2,"context_line":"fixes:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    Fixes the workflow for wholedisk images when using uefi boot mode, when"},{"line_number":5,"context_line":"    possible it will use efibootmgr instead of grub2 to update the nvram. "}],"source_content_type":"text/x-yaml","patch_set":15,"id":"3fa7e38b_86b3a609","line":5,"range":{"start_line":5,"start_character":73,"end_line":5,"end_character":74},"updated":"2020-01-16 09:05:07.000000000","message":"nit: blank space at the end of the line","commit_id":"1a8c3b88727d500206e4cd5494969cfdde718810"},{"author":{"_account_id":15519,"name":"Iury Gregory Melo Ferreira","display_name":"Iury Gregory","email":"iurygregory@gmail.com","username":"iurygregory"},"change_message_id":"b633c536c503b8f751eac78ef755b8b199ae49a0","unresolved":false,"context_lines":[{"line_number":2,"context_line":"fixes:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    Fixes the workflow for wholedisk images when using uefi boot mode, when"},{"line_number":5,"context_line":"    possible it will use efibootmgr instead of grub2 to update the nvram. "}],"source_content_type":"text/x-yaml","patch_set":15,"id":"3fa7e38b_41dcb842","line":5,"range":{"start_line":5,"start_character":73,"end_line":5,"end_character":74},"in_reply_to":"3fa7e38b_86b3a609","updated":"2020-01-16 10:22:37.000000000","message":"Done","commit_id":"1a8c3b88727d500206e4cd5494969cfdde718810"}]}