So that Windows conversion can use the same function. Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2149629 Signed-off-by: Laszlo Ersek <lersek(a)redhat.com&gt; --- lib/utils.mli | 5 +++++ convert/convert_linux.ml | 6 ------ lib/utils.ml | 5 +++++ 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/lib/utils.mli b/lib/utils.mli index d431e21f5ce9..5687bf75867e 100644 --- a/lib/utils.mli +++ b/lib/utils.mli @@ -98,3 +98,8 @@ val get_disk_allocated : dir:string -> disknr:int -> int64 option image, according to the "base:allocation" metadata context. If the context is not supported by the NBD server behind the socket, the function returns None. *) + +val get_uefi_arch_suffix : string -> string option +(** [get_uefi_arch_suffix arch] maps [arch] from [inspect.i_arch] representation + to UEFI spec representation. If a mapping cannot be found, [None] is + returned. *) diff --git a/convert/convert_linux.ml b/convert/convert_linux.ml index 5bfdac5aa6d9..cde3b037542f 100644 --- a/convert/convert_linux.ml +++ b/convert/convert_linux.ml @@ -1327,12 +1327,6 @@ let convert (g : G.guestfs) source inspect keep_serial_console _ = info (f_"Can't fix UEFI bootloader. VM may not boot.") in - let get_uefi_arch_suffix = function - | "x86_64" -> Some "X64" - | "i386" -> Some "X32" - | _ -> None - in - match get_uefi_arch_suffix inspect.i_arch with | None -> cant_fix_uefi () | Some suffix -> ( diff --git a/lib/utils.ml b/lib/utils.ml index fd45769415c7..26e7e259001b 100644 --- a/lib/utils.ml +++ b/lib/utils.ml @@ -258,3 +258,8 @@ let get_disk_allocated ~dir ~disknr = Some !allocated ) else None ) + +let get_uefi_arch_suffix = function + | "x86_64" -> Some "X64" + | "i386" -> Some "X32" + | _ -> None

The "fallback" (or "default") boot behavior is described at great length here: https://blog.uncooperative.org/uefi/linux/shim/efi%20system%20partition/2... The gist of it applies to all UEFI OSes, including Windows. For the fallback boot behavior to work, the \EFI\BOOT\BOOTX64.efi boot loader on the EFI system partition must match the installed operating system. We've encountered a physical machine, during a virt-p2v conversion, where (a) \EFI\BOOT\BOOTX64.efi belongs to a previously installed, but now wiped, RHEL (hence shim+grub) deployment, and (b) the currently installed operating system is Windows. Virt-v2v never transfers the UEFI variables (including the UEFI boot options) of the source, therefore the converted VM always relies on the default boot behavior when it is first started up. In the above scenario, where \EFI\BOOT\BOOTX64.efi is actually "shim", the mismatch is triggered at first boot after conversion, and a broken grub shell is reached instead of the Windows boot loader. Detect this situation by investigating \EFI\BOOT\BOOTX64.efi on the EFI system partition of a Windows disk image. If the file is missing, or is not -- as expected -- a duplicate of \EFI\Microsoft\Boot\bootmgfw.efi, then copy the latter to the former. Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2149629 Signed-off-by: Laszlo Ersek <lersek(a)redhat.com&gt; --- Notes: Tested with a freshly installed Win2019 guest whose \EFI\BOOT\BOOTX64.efi binary I manually corrupted. convert/convert_windows.ml | 25 ++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/convert/convert_windows.ml b/convert/convert_windows.ml index 34a5044dd338..57a7ff03398f 100644 --- a/convert/convert_windows.ml +++ b/convert/convert_windows.ml @@ -836,17 +836,42 @@ let convert (g : G.guestfs) _ inspect _ static_ips = ); with Not_found -> () + + and fix_win_uefi_fallback esp_path uefi_arch = + (* [esp_path] is on NTFS, and therefore it is considered case-sensitive; + * refer to + * <https://libguestfs.org/guestfs.3.html#guestfs_case_sensitive_path>;. + * However, the EFI system partition mounted under [esp_path] is FAT32 per + * UEFI spec, and the Linux vfat driver in the libguestfs appliance treats + * pathnames case-insensitively. Therefore, we're free to use any case in + * the ESP-relative pathnames below. + *) + let bootmgfw = sprintf "%s/efi/microsoft/boot/bootmgfw.efi" esp_path in + if g#is_file bootmgfw then + let bootdir = sprintf "%s/efi/boot" esp_path in + let fallback = sprintf "%s/boot%s.efi" bootdir uefi_arch in + if not (g#is_file fallback) || not (g#equal fallback bootmgfw) then ( + info (f_"Fixing UEFI bootloader."); + g#rm_rf bootdir; + g#mkdir_p bootdir; + g#cp_a bootmgfw fallback + ) in match inspect.i_firmware with | I_BIOS -> () | I_UEFI esp_list -> let esp_temp_path = g#mkdtemp "/Windows/Temp/ESP_XXXXXX" in + let uefi_arch = get_uefi_arch_suffix inspect.i_arch in List.iter ( fun dev_path -> g#mount dev_path esp_temp_path; fix_win_uefi_bcd esp_temp_path; + (match uefi_arch with + | Some uefi_arch -> fix_win_uefi_fallback esp_temp_path uefi_arch + | None -> () + ); g#umount esp_temp_path; ) esp_list;

On 12/02/22 13:58, Richard W.M. Jones wrote: Yes, the longest two lines have exactly 80 chars each. For years I've used 79, but I kept finding preexistent code lines in various projects that were 80 chars. The command git grep -E '^.{80}$' -- '*.ml*' returns a good number of lines in v2v as well, and the command git grep -E '^.{80,}$' -- '*.ml*' even more. The parens are required: # open Printf;; # let f () = false;; val f : unit -> bool = <fun> # if not f () then printf "hello\n";; Characters 3-6: if not f () then printf "hello\n";; ^^^ Error: This function has type bool -> bool It is applied to too many arguments; maybe you forgot a `;'. # if not (f ()) then printf "hello\n";; hello - : unit = () Yes I think we *should* delete the \EFI\BOOT directory in this case. We've determined that a Windows OS is installed on the disk, and I've not seen any Windows version yet where \EFI\BOOT contained any other file than BOOTX64.efi (or BOOTX32.efi), with the file being a copy of bootmgfw.efi at that. In the particular problem scenario <https://bugzilla.redhat.com/show_bug.cgi?id=2149629#c6>;, replacing only the BOOTX64.efi file (= shim) with a copy of bootmgfw.efi would produce a \EFI\BOOT directory containing two files: - BOOTX64.efi (= bootmgfw.efi) - fbx64.efi (from the shim installation) While that might not necessarily cause runtime issues, it's certainly confusing for any human reader (does not look like pristine ESP contents with Windows installed). Thanks, I'll push the series in a few minutes. Laszlo