-
Notifications
You must be signed in to change notification settings - Fork 38
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
virt-v2v 2.5.2 not detecting the firmware type correctly in some flows #52
Comments
I think this is actually a bug in virt-v2v. Here: Lines 96 to 103 in 089d9a2
we attempt to parse the XPath expression Unfortunately we don't log the contents of What happens if it is missing is we force it to Lines 269 to 277 in 089d9a2
What should instead happen is that, in This is a very simple patch if you want to have a go at it. |
Ah, that makes sense. I encountered this issue only with UEFI VMs, and when trying to convert the same VM (without the deleted property), the firmware was detected correctly, so it really looked like a default setting somewhere. Sure, I can post the fix, It sounds pretty straightforward with all the provided info you added :) |
When compiling virt-v2v, some tips:
|
I've added the suggested changes, and now it's really using the I've opened a draft PR in the meantime with the initial changes; let me know what you think: #53. |
Firmware detection is here: https://github.com/libguestfs/libguestfs-common/blob/master/mldrivers/firmware.ml What's happening for this guest is it has a BIOS boot partition on /dev/sda2, which we prioritise to mean it's a BIOS guest (even though it has UEFI ESP etc, so otherwise looks like a UEFI guest). This fallback was added recently in commit 22ec2d2 |
I should say that it's somewhat unusual for us not to have the firmware type from the source hypervisor. I'm fairly sure that VMware-generated OVFs would always have that, and if we're using other methods like VMware VMX then similarly we'd always have that information available. Autodetection is mainly for the fallback case where someone is converting a local disk having "lost" the metadata. |
Yes, you make a valid point. In most cases, we do have this property in the OVF configuration set. However, we've encountered instances where it was missing, leading to migrated VMs not functioning correctly in OpenShift. Since we aim to provide full support for all cases, we've thought to rely on virt-v2v detection for this. So from the commit you added, I understand it should still cover the majority of cases, but we may end up having a small percentage of false detection. I think it may be good enough but I'll talk with Arik and see how and if we want to address it from the MTV side. In the meantime, I've switched the PR with the fix from draft to ready. |
The problem which we were trying to address in the original commit 22ec2d is to detect the correct firmware in case we have both BIOS boot and EFI partitions. This layout doesn't normally happen, and we chose to prioritize BIOS boot in this case. But this logic is still prone to erroneous resolutions, one of which I think we're seeing here. Should we make the firmware detection logic a bit more sophisticated? For instance in case there's a ESP partition present we could mount it, inspect the content and look for EFI application files (BOOTX64.EFI, grubx64.efi etc.). I'm just wondering whether their presence would be an unambiguous indication that we're booting in EFI. |
Thanks Andrey. I think fundamentally the problem is that there can exist guests which can booted both ways! How do we know which is "preferred". But also I like your idea of looking at the EFI partition. In addition I wonder if it's plausible to look at the first sector of the disk, looking for a BIOS bootloader? Before we do anything, @bkhizgiy do you have a way to either send me or for me to create the guest you were trying to convert? |
Hmm, if your assumption is correct and we indeed can have a guest suitable for booting both ways, then this problem seems fundamentally unsolvable to me. We could have both 0xEF02 and 0xEF00 partitions present and each of them would have its own bootloader code stored inside. Then when booting a UEFI based VM (with edk2-ovmf) one bootloader code will be used, and when booting a BIOS based VM (seabios) with the same disk -- another bootloader code will be used. In this case I think we better limit conversions with automatic firmware detection to unambiguous cases only, i.e. only when either BIOS boot or EFI partition contain bootloader (but not both). Otherwise it seems that we're doomed to getting erroneous detections every once in a while. And the normal way should be detecting firmware from the source hypervisor, as you mentioned above. As for the BIOS bootloader detection, it seems that in case of both pure MBR or GPT + protective MBR + BIOS boot partition grub2 bootloader indeed leaves its traces in the first sector, namely in the first 440 bytes Boot Code area [1]. In particular, I can see the string "GRUB \0" in this first sector in case of BIOS boot partition. And in case of UEFI installation this Boot Code area is just filled with zeroes. So I guess we could use the presence of this string as an indication of BIOS bootloader. [1] https://uefi.org/specs/UEFI/2.10/05_GUID_Partition_Table_Format.html#protective-mbr |
So just to clarify, the firmware detection logic I'm suggesting to implement is the following:
|
@rwmjones Sure, I saw it when trying with a RHEL 9 guest. I downloaded the file from Red Hat Cloud using the following link , and selected the VMware-compatible option. I used let me know if you having any issues accessing the files. |
I downloaded them, thanks. |
When converting from OVA file that isnt specifing the firmware type in OVF configuration, the result yaml file contains the wrong firmware type.
attaching some conversion logs of VM with UEFI:
from the conversion steps it didnt recognize the UEFI type:
but from the debug logs it does recognizing UEFI, for example:
and we ended up having this yaml:
expected:
Full conversion logs are attached.
virt-v2v-logs.txt
The text was updated successfully, but these errors were encountered: