Skip to content

Commit

Permalink
Change save/restore nvram data logic
Browse files Browse the repository at this point in the history
Save NVRAM data not only if BiosType is Q35_SECURE_BOOT
Also save if Q35_OVMF because it also includes EFI boot variables
With Q35_OVMF we unable to boot non-fallback bootloaders
in place different from <esp>/EFI/BOOT/BOOT<arch>.EFI

To check this it needed to:
1) Run VM with installed Linux (it doesn't matter what distro)
2) Add a bootloader via efibbotmgr
   efibootmgr -c -d <boot device> -L "Some Linux" -l \\EFI\\<somevendor>\\grubx64.efi
3) Look at created efi boot variable
   efibootmgr -v
   It's present
4) Shutdown VM
5) Run it again
6) Check efi boot variables again
   efibootmgr -v
7) We got non bootable VM if fallback bootloader is broken or boot default esp/EFI/BOOT/BOOT<arch>.EFI
   otherwise

Signed-off-by: Anton Fadeev <[email protected]>
  • Loading branch information
antonios-f committed Sep 11, 2023
1 parent c5da78e commit c734177
Show file tree
Hide file tree
Showing 4 changed files with 6 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
import org.ovirt.engine.core.common.action.ExternalDataStatus;
import org.ovirt.engine.core.common.action.SaveVmExternalDataParameters;
import org.ovirt.engine.core.common.action.VmExternalDataKind;
import org.ovirt.engine.core.common.businessentities.BiosType;
import org.ovirt.engine.core.common.businessentities.VmDeviceGeneralType;
import org.ovirt.engine.core.common.errors.EngineError;
import org.ovirt.engine.core.common.errors.EngineException;
Expand Down Expand Up @@ -47,8 +46,8 @@ private boolean hasTpmDevice() {
return !vmDeviceDao.getVmDeviceByVmIdAndType(getParameters().getVmId(), VmDeviceGeneralType.TPM).isEmpty();
}

private boolean hasSecureBoot() {
return getVm().getBiosType() == BiosType.Q35_SECURE_BOOT
private boolean isUEFI() {
return getVm().getBiosType().isOvmf()
&& FeatureSupported.isNvramPersistenceSupported(getVm().getCompatibilityVersion());
}

Expand Down Expand Up @@ -80,7 +79,7 @@ protected void executeCommand() {
if (hasTpmDevice()) {
dataToRetrieve.add(VmExternalDataKind.TPM);
}
if (hasSecureBoot()) {
if (isUEFI()) {
dataToRetrieve.add(VmExternalDataKind.NVRAM);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
import org.ovirt.engine.core.bll.utils.VmDeviceUtils;
import org.ovirt.engine.core.common.AuditLogType;
import org.ovirt.engine.core.common.action.VmExternalDataKind;
import org.ovirt.engine.core.common.businessentities.BiosType;
import org.ovirt.engine.core.common.businessentities.Cluster;
import org.ovirt.engine.core.common.businessentities.Quota;
import org.ovirt.engine.core.common.businessentities.Snapshot;
Expand Down Expand Up @@ -352,7 +351,7 @@ private void addVmExternalData(Map<VmExternalDataKind, SecretValue<String>> vmEx
vmExternalData.put(VmExternalDataKind.TPM, tpmData);
}
}
if (vm.getBiosType() == BiosType.Q35_SECURE_BOOT) {
if (vm.getBiosType().isOvmf()) {
SecretValue<String> nvramData = vmDao.getNvramData(vm.getId()).getFirst();
if (!SecretValue.isNull(nvramData) && !nvramData.getValue().equals("")) {
vmExternalData.put(VmExternalDataKind.NVRAM, nvramData);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
import org.ovirt.engine.core.common.action.VmExternalDataKind;
import org.ovirt.engine.core.common.action.VmManagementParametersBase;
import org.ovirt.engine.core.common.businessentities.ArchitectureType;
import org.ovirt.engine.core.common.businessentities.BiosType;
import org.ovirt.engine.core.common.businessentities.ChipsetType;
import org.ovirt.engine.core.common.businessentities.Cluster;
import org.ovirt.engine.core.common.businessentities.ConsoleTargetType;
Expand Down Expand Up @@ -2287,7 +2286,7 @@ public void copyVmExternalData(Guid sourceVmId, Guid targetVmId) {
if (hasTpmDevice(targetVmId)) {
vmDao.copyTpmData(sourceVmId, targetVmId);
}
if (getVmBase(targetVmId).getBiosType() == BiosType.Q35_SECURE_BOOT) {
if (getVmBase(targetVmId).getBiosType().isOvmf()) {
vmDao.copyNvramData(sourceVmId, targetVmId);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@

import javax.inject.Inject;

import org.ovirt.engine.core.common.businessentities.BiosType;
import org.ovirt.engine.core.common.businessentities.VM;
import org.ovirt.engine.core.common.businessentities.VmDevice;
import org.ovirt.engine.core.common.businessentities.storage.DiskImage;
Expand Down Expand Up @@ -78,7 +77,7 @@ private Map<String, Object> createInfo() {
if (!SecretValue.isNull(tpmData)) {
createInfo.put(VdsProperties.tpmData, tpmData.getValue());
}
if (vm.getBiosType() == BiosType.Q35_SECURE_BOOT) {
if (vm.getBiosType().isOvmf()) {
SecretValue<String> nvramData = vmInfoBuildUtils.nvramData(vm.getId());
if (!SecretValue.isNull(nvramData)) {
createInfo.put(VdsProperties.nvramData, nvramData.getValue());
Expand Down

0 comments on commit c734177

Please sign in to comment.